[chore] Better handling of Gone accounts during `PostInbox` (return 202 Accepted instead of 401 Unauthorized) (#4506)v0.20.1

Follow up of https://codeberg.org/superseriousbusiness/gotosocial/pulls/4503 addressing something I missed!

Reviewed-on: https://codeberg.org/superseriousbusiness/gotosocial/pulls/4506
Co-authored-by: tobi <tobi.smethurst@protonmail.com>
Co-committed-by: tobi <tobi.smethurst@protonmail.com>

2 files changed, 38 insertions, 16 deletions

diff --git a/internal/federation/federatingactor.go b/internal/federation/federatingactor.go
index 316835036..719520ea6 100644
--- a/internal/federation/federatingactor.go
+++ b/internal/federation/federatingactor.go
@@ -145,22 +145,43 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr
 	// 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 errorsv2.AsV2[gtserror.WithCode](err) != nil {
-		// 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.
+	ctx, didAuthentication, err := f.sideEffectActor.AuthenticatePostInbox(ctx, w, r)
+	switch {
+
+	// If error 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.
+	//
+	// In most cases, the returned error will be
+	// 401 Unauthorized, to indicate that the
+	// signature on the request was invalid.
+	case errorsv2.AsV2[gtserror.WithCode](err) != nil:
 		return false, err
-	} else if err != nil {
+
+	// If error was returned but wasn't wrapped,
+	// wrap it into a 500 Internal Server Error.
+	case err != nil:
 		err := gtserror.Newf("error authenticating post inbox: %w", err)
 		return false, gtserror.NewErrorInternalError(err)
-	}
 
-	if !authenticated {
-		const text = "not authenticated"
-		return false, gtserror.NewErrorUnauthorized(errors.New(text), text)
+	// If we got no error but we didn't do auth,
+	// this means we were either still handshaking
+	// during auth (very rare) or more likely the
+	// remote account was marked as gone/deleted
+	// so we couldn't even get their key to verify.
+	//
+	// If this is so, we know we've already handled any
+	// necessary deletes, and AuthenticatePostInbox
+	// will have written 202 Accepted, so we can bail.
+	case !didAuthentication:
+		log.Debug(ctx, "requesting account is gone or we're still handshaking with it, ignoring inbox post")
+		return true, nil
+
+	// No problem.
+	default:
+		log.Trace(ctx, "AuthenticatePostInbox was successful, continuing with request")
 	}
 
 	// Ensure requester is not suspended.
diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go
index 2f6953257..990ce6a52 100644
--- a/internal/federation/federatingprotocol.go
+++ b/internal/federation/federatingprotocol.go
@@ -227,12 +227,13 @@ func (f *Federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr
 	pubKeyAuth, errWithCode := f.AuthenticateFederatedRequest(ctx, receiver.Username)
 	if errWithCode != nil {
 
-		// Check if we got an error code from a remote
+		// Check if we got code 410 Gone from a remote
 		// instance while trying to dereference the pub
-		// key owner who's trying to post to this inbox.
+		// key owner who's trying to post to this inbox,
+		// or if we already had a tombstone stored for them.
 		if gtserror.StatusCode(errWithCode) == http.StatusGone {
-			// If the pub key owner's key/account has gone
-			// (410) then likely inbox post was a Delete.
+			// If the pub key owner's key/account has
+			// gone, then inbox post was likely a Delete.
 			//
 			// If so, we can just write 202 and leave, as
 			// either we'll have already processed any Deletes