[bugfix] Reorder web view logic, other small fixes (#1954)

3 files changed, 121 insertions, 52 deletions

diff --git a/internal/processing/fedi/common.go b/internal/processing/fedi/common.go
index 68a84f303..1331a20e0 100644
--- a/internal/processing/fedi/common.go
+++ b/internal/processing/fedi/common.go
@@ -19,41 +19,61 @@ package fedi
 
 import (
 	"context"
+	"errors"
 	"fmt"
-	"net/url"
 
+	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 )
 
-func (p *Processor) authenticate(ctx context.Context, requestedUsername string) (requestedAccount, requestingAccount *gtsmodel.Account, errWithCode gtserror.WithCode) {
+func (p *Processor) authenticate(ctx context.Context, requestedUsername string) (
+	*gtsmodel.Account, // requestedAccount
+	*gtsmodel.Account, // requestingAccount
+	gtserror.WithCode,
+) {
+	// Get LOCAL account with the requested username.
 	requestedAccount, err := p.state.DB.GetAccountByUsernameDomain(ctx, requestedUsername, "")
 	if err != nil {
-		errWithCode = gtserror.NewErrorNotFound(fmt.Errorf("database error getting account with username %s: %s", requestedUsername, err))
-		return
+		if !errors.Is(err, db.ErrNoEntries) {
+			// Real db error.
+			err = gtserror.Newf("db error getting account %s: %w", requestedUsername, err)
+			return nil, nil, gtserror.NewErrorInternalError(err)
+		}
+
+		// Account just not found in the db.
+		return nil, nil, gtserror.NewErrorNotFound(err)
 	}
 
-	var requestingAccountURI *url.URL
-	requestingAccountURI, errWithCode = p.federator.AuthenticateFederatedRequest(ctx, requestedUsername)
+	// Ensure request signed, and use signature URI to
+	// get requesting account, dereferencing if necessary.
+	requestingAccountURI, errWithCode := p.federator.AuthenticateFederatedRequest(ctx, requestedUsername)
 	if errWithCode != nil {
-		return
+		return nil, nil, errWithCode
 	}
 
-	if requestingAccount, _, err = p.federator.GetAccountByURI(gtscontext.SetFastFail(ctx), requestedUsername, requestingAccountURI); err != nil {
-		errWithCode = gtserror.NewErrorUnauthorized(err)
-		return
+	requestingAccount, _, err := p.federator.GetAccountByURI(
+		gtscontext.SetFastFail(ctx),
+		requestedUsername,
+		requestingAccountURI,
+	)
+	if err != nil {
+		err = gtserror.Newf("error getting account %s: %w", requestingAccountURI, err)
+		return nil, nil, gtserror.NewErrorUnauthorized(err)
 	}
 
+	// Ensure no block exists between requester + requested.
 	blocked, err := p.state.DB.IsEitherBlocked(ctx, requestedAccount.ID, requestingAccount.ID)
 	if err != nil {
-		errWithCode = gtserror.NewErrorInternalError(err)
-		return
+		err = gtserror.Newf("db error getting checking block: %w", err)
+		return nil, nil, gtserror.NewErrorInternalError(err)
 	}
 
 	if blocked {
-		errWithCode = gtserror.NewErrorUnauthorized(fmt.Errorf("block exists between accounts %s and %s", requestedAccount.ID, requestingAccount.ID))
+		err = fmt.Errorf("block exists between accounts %s and %s", requestedAccount.ID, requestingAccount.ID)
+		return nil, nil, gtserror.NewErrorUnauthorized(err)
 	}
 
-	return
+	return requestedAccount, requestingAccount, nil
 }
diff --git a/internal/processing/fedi/status.go b/internal/processing/fedi/status.go
index a9b3e1eee..c2a0e46d6 100644
--- a/internal/processing/fedi/status.go
+++ b/internal/processing/fedi/status.go
@@ -27,9 +27,10 @@ import (
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 )
 
-// StatusGet handles the getting of a fedi/activitypub representation of a particular status, performing appropriate
-// authentication before returning a JSON serializable interface to the caller.
+// StatusGet handles the getting of a fedi/activitypub representation of a local status.
+// It performs appropriate authentication before returning a JSON serializable interface.
 func (p *Processor) StatusGet(ctx context.Context, requestedUsername string, requestedStatusID string) (interface{}, gtserror.WithCode) {
+	// Authenticate using http signature.
 	requestedAccount, requestingAccount, errWithCode := p.authenticate(ctx, requestedUsername)
 	if errWithCode != nil {
 		return nil, errWithCode
@@ -41,15 +42,18 @@ func (p *Processor) StatusGet(ctx context.Context, requestedUsername string, req
 	}
 
 	if status.AccountID != requestedAccount.ID {
-		return nil, gtserror.NewErrorNotFound(fmt.Errorf("status with id %s does not belong to account with id %s", status.ID, requestedAccount.ID))
+		err := fmt.Errorf("status with id %s does not belong to account with id %s", status.ID, requestedAccount.ID)
+		return nil, gtserror.NewErrorNotFound(err)
 	}
 
 	visible, err := p.filter.StatusVisible(ctx, requestingAccount, status)
 	if err != nil {
 		return nil, gtserror.NewErrorInternalError(err)
 	}
+
 	if !visible {
-		return nil, gtserror.NewErrorNotFound(fmt.Errorf("status with id %s not visible to user with id %s", status.ID, requestingAccount.ID))
+		err := fmt.Errorf("status with id %s not visible to user with id %s", status.ID, requestingAccount.ID)
+		return nil, gtserror.NewErrorNotFound(err)
 	}
 
 	asStatus, err := p.tc.StatusToAS(ctx, status)
diff --git a/internal/processing/fedi/user.go b/internal/processing/fedi/user.go
index 4ec780a87..4a55df01f 100644
--- a/internal/processing/fedi/user.go
+++ b/internal/processing/fedi/user.go
@@ -19,65 +19,110 @@ package fedi
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/url"
 
 	"github.com/superseriousbusiness/activity/streams/vocab"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
+	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/uris"
 )
 
-// UserGet handles the getting of a fedi/activitypub representation of a user/account, performing appropriate authentication
-// before returning a JSON serializable interface to the caller.
+// UserGet handles the getting of a fedi/activitypub representation of a user/account,
+// performing authentication before returning a JSON serializable interface to the caller.
 func (p *Processor) UserGet(ctx context.Context, requestedUsername string, requestURL *url.URL) (interface{}, gtserror.WithCode) {
-	// Get the instance-local account the request is referring to.
+	// (Try to) get the requested local account from the db.
 	requestedAccount, err := p.state.DB.GetAccountByUsernameDomain(ctx, requestedUsername, "")
 	if err != nil {
-		return nil, gtserror.NewErrorNotFound(fmt.Errorf("database error getting account with username %s: %s", requestedUsername, err))
-	}
+		if errors.Is(err, db.ErrNoEntries) {
+			// Account just not found w/ this username.
+			err := fmt.Errorf("account with username %s not found in the db", requestedUsername)
+			return nil, gtserror.NewErrorNotFound(err)
+		}
 
-	var requestedPerson vocab.ActivityStreamsPerson
+		// Real db error.
+		err := fmt.Errorf("db error getting account with username %s: %w", requestedUsername, err)
+		return nil, gtserror.NewErrorInternalError(err)
+	}
 
 	if uris.IsPublicKeyPath(requestURL) {
-		// if it's a public key path, we don't need to authenticate but we'll only serve the bare minimum user profile needed for the public key
-		requestedPerson, err = p.tc.AccountToASMinimal(ctx, requestedAccount)
+		// If request is on a public key path, we don't need to
+		// authenticate this request. However, we'll only serve
+		// the bare minimum user profile needed for the pubkey.
+		//
+		// TODO: https://github.com/superseriousbusiness/gotosocial/issues/1186
+		minimalPerson, err := p.tc.AccountToASMinimal(ctx, requestedAccount)
 		if err != nil {
 			return nil, gtserror.NewErrorInternalError(err)
 		}
-	} else {
-		// if it's any other path, we want to fully authenticate the request before we serve any data, and then we can serve a more complete profile
-		requestingAccountURI, errWithCode := p.federator.AuthenticateFederatedRequest(ctx, requestedUsername)
-		if errWithCode != nil {
-			return nil, errWithCode
-		}
 
-		// if we're not already handshaking/dereferencing a remote account, dereference it now
-		if !p.federator.Handshaking(requestedUsername, requestingAccountURI) {
-			requestingAccount, _, err := p.federator.GetAccountByURI(gtscontext.SetFastFail(ctx), requestedUsername, requestingAccountURI)
-			if err != nil {
-				return nil, gtserror.NewErrorUnauthorized(err)
-			}
-
-			blocked, err := p.state.DB.IsEitherBlocked(ctx, requestedAccount.ID, requestingAccount.ID)
-			if err != nil {
-				return nil, gtserror.NewErrorInternalError(err)
-			}
-
-			if blocked {
-				return nil, gtserror.NewErrorUnauthorized(fmt.Errorf("block exists between accounts %s and %s", requestedAccount.ID, requestingAccount.ID))
-			}
-		}
+		// Return early with bare minimum data.
+		return data(minimalPerson)
+	}
 
-		requestedPerson, err = p.tc.AccountToAS(ctx, requestedAccount)
-		if err != nil {
-			return nil, gtserror.NewErrorInternalError(err)
-		}
+	// If the request is not on a public key path, we want to
+	// try to authenticate it before we serve any data, so that
+	// we can serve a more complete profile.
+	requestingAccountURI, errWithCode := p.federator.AuthenticateFederatedRequest(ctx, requestedUsername)
+	if errWithCode != nil {
+		return nil, errWithCode // likely 401
+	}
+
+	// Auth passed, generate the proper AP representation.
+	person, err := p.tc.AccountToAS(ctx, requestedAccount)
+	if err != nil {
+		return nil, gtserror.NewErrorInternalError(err)
 	}
 
+	// If we are currently handshaking with the remote account
+	// making the request, then don't be coy: just serve the AP
+	// representation of the target account.
+	//
+	// This handshake check ensures that we don't get stuck in
+	// a loop with another GtS instance, where each instance is
+	// trying repeatedly to dereference the other account that's
+	// making the request before it will reveal its own account.
+	//
+	// Instead, we end up in an 'I'll show you mine if you show me
+	// yours' situation, where we sort of agree to reveal each
+	// other's profiles at the same time.
+	if p.federator.Handshaking(requestedUsername, requestingAccountURI) {
+		return data(person)
+	}
+
+	// We're not currently handshaking with the requestingAccountURI,
+	// so fetch its details and ensure it's up to date + not blocked.
+	requestingAccount, _, err := p.federator.GetAccountByURI(
+		// On a hot path so fail quickly.
+		gtscontext.SetFastFail(ctx),
+		requestedUsername, requestingAccountURI,
+	)
+	if err != nil {
+		err := gtserror.Newf("error getting account %s: %w", requestingAccountURI, err)
+		return nil, gtserror.NewErrorUnauthorized(err)
+	}
+
+	blocked, err := p.state.DB.IsBlocked(ctx, requestedAccount.ID, requestingAccount.ID)
+	if err != nil {
+		err := gtserror.Newf("error checking block from account %s to account %s: %w", requestedAccount.ID, requestingAccount.ID, err)
+		return nil, gtserror.NewErrorInternalError(err)
+	}
+
+	if blocked {
+		err := fmt.Errorf("account %s blocks account %s", requestedAccount.ID, requestingAccount.ID)
+		return nil, gtserror.NewErrorUnauthorized(err)
+	}
+
+	return data(person)
+}
+
+func data(requestedPerson vocab.ActivityStreamsPerson) (interface{}, gtserror.WithCode) {
 	data, err := ap.Serialize(requestedPerson)
 	if err != nil {
+		err := gtserror.Newf("error serializing person: %w", err)
 		return nil, gtserror.NewErrorInternalError(err)
 	}