Revert "fix: assorted ActivityPub code only refactors (#8274)" (#8705)

This reverts commit e271c24100.

It was an experiment to verify that adding a delay to the test make a difference. But it does not so... reverting.

@jerger before engaging in a refactor, it is necessary to get to the bottom of this:

- Find the root cause of those failures
- Fix it in a minimal way

Refs https://codeberg.org/forgejo/forgejo/pulls/8274#issuecomment-5987215

---

- https://codeberg.org/forgejo/forgejo/actions/runs/92182/jobs/9
- https://codeberg.org/forgejo/forgejo/actions/runs/92182/jobs/10

```
--- FAIL: TestFederationHttpSigValidation (11.34s)
    testlogger.go:411: 2025/07/28 00:23:46 ...les/storage/local.go:33:NewLocalStorage() [I] Creating new Local Storage at /workspace/forgejo/forgejo/tests/gitea-lfs-meta
    testlogger.go:411: 2025/07/28 00:23:52 ...ypub/reqsignature.go:76:func1() [W] verifyHttpSignatures failed: neither "Signature" nor "Authorization" have signature parameters
    testlogger.go:411: 2025/07/28 00:23:52 ...eb/routing/logger.go:102:func1() [I] router: completed GET http://127.0.0.1:3002/api/v1/activitypub/user-id/2 for test-mock:12345, 400 Bad Request in 5.3ms @ activitypub/reqsignature.go:74(activitypub.ReqHTTPUserOrInstanceSignature)
    testlogger.go:411: 2025/07/28 00:23:52 ...ces/auth/httpsign.go:70:Verify() [W] Failed authentication attempt from 127.0.0.1:43244
    testlogger.go:411: 2025/07/28 00:23:55 ...eb/routing/logger.go:68:func1() [W] router: slow      GET /api/v1/activitypub/user-id/2 for 127.0.0.1:43244, elapsed 3684.7ms @ activitypub/reqsignature.go:74(activitypub.ReqHTTPUserOrInstanceSignature)
    --- FAIL: TestFederationHttpSigValidation/SignedRequest (5.01s)
        api_federation_httpsig_test.go:50:
            	Error Trace:	/workspace/forgejo/forgejo/tests/integration/api_federation_httpsig_test.go:50
            	Error:      	Received unexpected error:
            	            	Get "http://127.0.0.1:3002/api/v1/activitypub/user-id/2": context deadline exceeded (Client.Timeout exceeded while awaiting headers)
            	Test:       	TestFederationHttpSigValidation/SignedRequest
    --- FAIL: TestFederationHttpSigValidation/ValidateCaches (0.00s)
        api_federation_httpsig_test.go:64:
            	Error Trace:	/workspace/forgejo/forgejo/tests/integration/api_federation_httpsig_test.go:64
            	Error:      	Expected value not to be nil.
            	Test:       	TestFederationHttpSigValidation/ValidateCaches
    test_utils.go:247: PrepareTestEnv:Process "GET: /api/v1/activitypub/user-id/2" cancelled
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4cc464a]
```

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8705
Reviewed-by: jerger <jerger@noreply.codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
This commit is contained in:
Earl Warren 2025-07-28 10:55:04 +02:00 committed by Earl Warren
parent 4f0c2ec258
commit b2c8a1cfd3
20 changed files with 69 additions and 741 deletions

View file

@ -1,82 +0,0 @@
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package integration
import (
"fmt"
"net/http"
"net/url"
"testing"
"forgejo.org/models/db"
"forgejo.org/models/forgefed"
"forgejo.org/models/unittest"
"forgejo.org/models/user"
"forgejo.org/modules/activitypub"
"forgejo.org/modules/setting"
"forgejo.org/modules/test"
"forgejo.org/routers"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestFederationHttpSigValidation(t *testing.T) {
defer test.MockVariableValue(&setting.Federation.Enabled, true)()
defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
onGiteaRun(t, func(t *testing.T, u *url.URL) {
userID := 2
userURL := fmt.Sprintf("%sapi/v1/activitypub/user-id/%d", u, userID)
user1 := unittest.AssertExistsAndLoadBean(t, &user.User{ID: 1})
clientFactory, err := activitypub.GetClientFactory(db.DefaultContext)
require.NoError(t, err)
apClient, err := clientFactory.WithKeys(db.DefaultContext, user1, user1.KeyID())
require.NoError(t, err)
// Unsigned request
t.Run("UnsignedRequest", func(t *testing.T) {
req := NewRequest(t, "GET", userURL)
MakeRequest(t, req, http.StatusBadRequest)
})
// Signed request
t.Run("SignedRequest", func(t *testing.T) {
resp, err := apClient.Get(userURL)
require.NoError(t, err)
assert.Equal(t, http.StatusOK, resp.StatusCode)
})
// HACK HACK HACK: the host part of the URL gets set to which IP forgejo is
// listening on, NOT localhost, which is the Domain given to forgejo which
// is then used for eg. the keyID all requests
applicationKeyID := fmt.Sprintf("%sapi/v1/activitypub/actor#main-key", setting.AppURL)
actorKeyID := fmt.Sprintf("%sapi/v1/activitypub/user-id/1#main-key", setting.AppURL)
// Check for cached public keys
t.Run("ValidateCaches", func(t *testing.T) {
host, err := forgefed.FindFederationHostByKeyID(db.DefaultContext, applicationKeyID)
require.NoError(t, err)
assert.NotNil(t, host)
assert.True(t, host.PublicKey.Valid)
_, user, err := user.FindFederatedUserByKeyID(db.DefaultContext, actorKeyID)
require.NoError(t, err)
assert.NotNil(t, user)
assert.True(t, user.PublicKey.Valid)
})
// Disable signature validation
defer test.MockVariableValue(&setting.Federation.SignatureEnforced, false)()
// Unsigned request
t.Run("SignatureValidationDisabled", func(t *testing.T) {
req := NewRequest(t, "GET", userURL)
MakeRequest(t, req, http.StatusOK)
})
})
}