diff options
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/api/activitypub/users/inboxpost_test.go | 2 | ||||
| -rw-r--r-- | internal/federation/federatingactor.go | 17 | ||||
| -rw-r--r-- | internal/federation/federatingprotocol.go | 9 | ||||
| -rw-r--r-- | internal/federation/federatingprotocol_test.go | 11 |
4 files changed, 31 insertions, 8 deletions
diff --git a/internal/api/activitypub/users/inboxpost_test.go b/internal/api/activitypub/users/inboxpost_test.go index 89e1cec41..9d863f234 100644 --- a/internal/api/activitypub/users/inboxpost_test.go +++ b/internal/api/activitypub/users/inboxpost_test.go @@ -590,7 +590,7 @@ func (suite *InboxPostTestSuite) TestPostUnauthorized() { requestingAccount, targetAccount, http.StatusUnauthorized, - `{"error":"Unauthorized: not authenticated"}`, + `{"error":"Unauthorized: http request wasn't signed or http signature was invalid: (verifier)"}`, // Omit signature check middleware. ) } diff --git a/internal/federation/federatingactor.go b/internal/federation/federatingactor.go index bf54962db..18cee1666 100644 --- a/internal/federation/federatingactor.go +++ b/internal/federation/federatingactor.go @@ -80,8 +80,23 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr } // Authenticate request by checking http signature. + // + // NOTE: the behaviour here is a little strange as we have + // the competing code styles of the go-fed interface expecting + // that any 'err' is fatal, but 'authenticated' bool is intended to + // be the main passer of whether failed auth occurred, but we in + // the gts codebase use errors to pass-back non-200 status codes, + // so we specifically have to check for already wrapped with code. + // ctx, authenticated, err := f.sideEffectActor.AuthenticatePostInbox(ctx, w, r) - if err != nil { + if errors.As(err, new(gtserror.WithCode)) { + // If it was already wrapped with an + // HTTP code then don't bother rewrapping + // it, just return it as-is for caller to + // handle. AuthenticatePostInbox already + // calls WriteHeader() in some situations. + return false, err + } else if err != nil { err := gtserror.Newf("error authenticating post inbox: %w", err) return false, gtserror.NewErrorInternalError(err) } diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go index 2c2da7b7b..f8e5b4c09 100644 --- a/internal/federation/federatingprotocol.go +++ b/internal/federation/federatingprotocol.go @@ -216,7 +216,6 @@ func (f *Federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr // If codes 400, 401, or 403, obey the go-fed // interface by writing the header and bailing. w.WriteHeader(errWithCode.Code()) - return ctx, false, nil case http.StatusGone: // If the requesting account's key has gone // (410) then likely inbox post was a delete. @@ -225,11 +224,11 @@ func (f *Federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr // know about the account anyway, so we can't // do any further processing. w.WriteHeader(http.StatusAccepted) - return ctx, false, nil - default: - // Proper error. - return ctx, false, err } + + // We still return the error + // for later request logging. + return ctx, false, errWithCode } if pubKeyAuth.Handshaking { diff --git a/internal/federation/federatingprotocol_test.go b/internal/federation/federatingprotocol_test.go index 999569c85..f975cd7d6 100644 --- a/internal/federation/federatingprotocol_test.go +++ b/internal/federation/federatingprotocol_test.go @@ -21,6 +21,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "io" "net/http" "net/http/httptest" @@ -30,6 +31,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/internal/gtscontext" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/testrig" "github.com/superseriousbusiness/httpsig" @@ -99,7 +101,14 @@ func (suite *FederatingProtocolTestSuite) authenticatePostInbox( recorder := httptest.NewRecorder() newContext, authed, err := suite.federator.AuthenticatePostInbox(ctx, recorder, request) - if err != nil { + if withCode := new(gtserror.WithCode); (errors.As(err, withCode) && + (*withCode).Code() >= 500) || (err != nil && (*withCode) == nil) { + // NOTE: the behaviour here is a little strange as we have + // the competing code styles of the go-fed interface expecting + // that any err is a no-go, but authed bool is intended to be + // the main passer of whether failed auth occurred, but we in + // the gts codebase use errors to pass-back non-200 status codes, + // so we specifically have to check for an internal error code. suite.FailNow(err.Error()) } |