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

internal/api/util/parsequery.go

@@ -43,6 +43,11 @@ const (
 	SearchResolveKey           = "resolve"
 	SearchTypeKey              = "type"
 
+	/* Web endpoint keys */
+
+	WebUsernameKey = "username"
+	WebStatusIDKey = "status"
+
 	/* Domain block keys */
 
 	DomainBlockExportKey = "export"
@@ -75,6 +80,14 @@ func ParseLocal(value string, defaultValue bool) (bool, gtserror.WithCode) {
 	return parseBool(value, defaultValue, LocalKey)
 }
 
+func ParseMaxID(value string, defaultValue string) string {
+	if value == "" {
+		return defaultValue
+	}
+
+	return value
+}
+
 func ParseSearchExcludeUnreviewed(value string, defaultValue bool) (bool, gtserror.WithCode) {
 	return parseBool(value, defaultValue, SearchExcludeUnreviewedKey)
 }
@@ -133,6 +146,26 @@ func ParseSearchQuery(value string) (string, gtserror.WithCode) {
 	return value, nil
 }
 
+func ParseWebUsername(value string) (string, gtserror.WithCode) {
+	key := WebUsernameKey
+
+	if value == "" {
+		return "", requiredError(key)
+	}
+
+	return value, nil
+}
+
+func ParseWebStatusID(value string) (string, gtserror.WithCode) {
+	key := WebStatusIDKey
+
+	if value == "" {
+		return "", requiredError(key)
+	}
+
+	return value, nil
+}
+
 /*
 	Internal functions
 */

internal/federation/dereferencing/dereferencer.go

@@ -95,7 +95,7 @@ type deref struct {
 	derefEmojis         map[string]*media.ProcessingEmoji
 	derefEmojisMu       mutexes.Mutex
 	handshakes          map[string][]*url.URL
-	handshakeSync       sync.Mutex // mutex to lock/unlock when checking or updating the handshakes map
+	handshakesMu        sync.Mutex // mutex to lock/unlock when checking or updating the handshakes map
 }
 
 // NewDereferencer returns a Dereferencer initialized with the given parameters.

internal/federation/dereferencing/handshake.go

@@ -22,68 +22,83 @@ import (
 )
 
 func (d *deref) Handshaking(username string, remoteAccountID *url.URL) bool {
-	d.handshakeSync.Lock()
-	defer d.handshakeSync.Unlock()
+	d.handshakesMu.Lock()
+	defer d.handshakesMu.Unlock()
 
 	if d.handshakes == nil {
-		// handshakes isn't even initialized yet so we can't be handshaking with anyone
+		// Handshakes isn't even initialized yet,
+		// so we can't be handshaking with anyone.
 		return false
 	}
 
 	remoteIDs, ok := d.handshakes[username]
 	if !ok {
-		// user isn't handshaking with anyone, bail
+		// Given username isn't
+		// handshaking with anyone.
 		return false
 	}
 
 	for _, id := range remoteIDs {
 		if id.String() == remoteAccountID.String() {
-			// we are currently handshaking with the remote account, yep
+			// We are currently handshaking
+			// with the remote account.
 			return true
 		}
 	}
 
-	// didn't find it which means we're not handshaking
+	// No results: we're not handshaking
+	// with the remote account.
 	return false
 }
 
 func (d *deref) startHandshake(username string, remoteAccountID *url.URL) {
-	d.handshakeSync.Lock()
-	defer d.handshakeSync.Unlock()
+	d.handshakesMu.Lock()
+	defer d.handshakesMu.Unlock()
 
 	remoteIDs, ok := d.handshakes[username]
 	if !ok {
-		// there was nothing in there yet, so just add this entry and return
+		// No handshakes were stored yet,
+		// so just add this entry and return.
 		d.handshakes[username] = []*url.URL{remoteAccountID}
 		return
 	}
 
-	// add the remote ID to the slice
+	// Add the remote account ID to the slice.
 	remoteIDs = append(remoteIDs, remoteAccountID)
 	d.handshakes[username] = remoteIDs
 }
 
 func (d *deref) stopHandshake(username string, remoteAccountID *url.URL) {
-	d.handshakeSync.Lock()
-	defer d.handshakeSync.Unlock()
+	d.handshakesMu.Lock()
+	defer d.handshakesMu.Unlock()
 
 	remoteIDs, ok := d.handshakes[username]
 	if !ok {
+		// No handshake was in progress,
+		// so there's nothing to stop.
 		return
 	}
 
-	newRemoteIDs := []*url.URL{}
+	// Generate a new remoteIDs slice that
+	// doesn't contain the removed entry.
+	var (
+		remoteAccountIDStr = remoteAccountID.String()
+		newRemoteIDs       = make([]*url.URL, 0, len(remoteIDs)-1)
+	)
+
 	for _, id := range remoteIDs {
-		if id.String() != remoteAccountID.String() {
+		if id.String() != remoteAccountIDStr {
 			newRemoteIDs = append(newRemoteIDs, id)
 		}
 	}
 
 	if len(newRemoteIDs) == 0 {
-		// there are no handshakes so just remove this user entry from the map and save a few bytes
+		// There are no handshakes remaining,
+		// so just remove this username's slice
+		// from the map and save a few bytes.
 		delete(d.handshakes, username)
 	} else {
-		// there are still other handshakes ongoing
+		// There are still other handshakes ongoing.
 		d.handshakes[username] = newRemoteIDs
 	}
 }

internal/processing/account/statuses.go

@@ -197,3 +197,9 @@ func (p *Processor) WebStatusesGet(ctx context.Context, targetAccountID string,
 		NextMaxIDValue: nextMaxIDValue,
 	})
 }
+
+// PinnedStatusesGet is a shortcut for getting just an account's pinned statuses.
+// Under the hood, it just calls StatusesGet using mostly default parameters.
+func (p *Processor) PinnedStatusesGet(ctx context.Context, requestingAccount *gtsmodel.Account, targetAccountID string) (*apimodel.PageableResponse, gtserror.WithCode) {
+	return p.StatusesGet(ctx, requestingAccount, targetAccountID, 0, false, false, "", "", true, false, false)
+}

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
 }

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)

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))
-			}
-		}
-
-		requestedPerson, err = p.tc.AccountToAS(ctx, requestedAccount)
-		if err != nil {
-			return nil, gtserror.NewErrorInternalError(err)
-		}
+		// Return early with bare minimum data.
+		return data(minimalPerson)
 	}
 
+	// 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)
 	}
 

internal/web/profile.go

@@ -20,7 +20,6 @@ package web
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -33,91 +32,117 @@ import (
 	"github.com/superseriousbusiness/gotosocial/internal/oauth"
 )
 
-const (
-	// MaxStatusIDKey is for specifying the maximum ID of the status to retrieve.
-	MaxStatusIDKey = "max_id"
-)
-
 func (m *Module) profileGETHandler(c *gin.Context) {
 	ctx := c.Request.Context()
 
+	// We'll need the instance later, and we can also use it
+	// before then to make it easier to return a web error.
+	instance, errWithCode := m.processor.InstanceGetV1(ctx)
+	if errWithCode != nil {
+		apiutil.WebErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
+		return
+	}
+
+	// Return instance we already got from the db,
+	// don't try to fetch it again when erroring.
+	instanceGet := func(ctx context.Context) (*apimodel.InstanceV1, gtserror.WithCode) {
+		return instance, nil
+	}
+
+	// Parse account targetUsername from the URL.
+	targetUsername, errWithCode := apiutil.ParseWebUsername(c.Param(apiutil.WebUsernameKey))
+	if errWithCode != nil {
+		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
+		return
+	}
+
+	// Normalize requested username:
+	//
+	//   - Usernames on our instance are (currently) always lowercase.
+	//
+	// todo: Update this logic when different username patterns
+	// are allowed, and/or when status slugs are introduced.
+	targetUsername = strings.ToLower(targetUsername)
+
+	// Check what type of content is being requested. If we're getting an AP
+	// request on this endpoint we should render the AP representation instead.
+	accept, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	if err != nil {
+		apiutil.WebErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), instanceGet)
+		return
+	}
+
+	if accept == string(apiutil.AppActivityJSON) || accept == string(apiutil.AppActivityLDJSON) {
+		// AP account representation has been requested.
+		m.returnAPAccount(c, targetUsername, accept, instanceGet)
+		return
+	}
+
+	// text/html has been requested. Proceed with getting the web view of the account.
+
+	// Don't require auth for web endpoints, but do take it if it was provided.
+	// authed.Account might end up nil here, but that's fine in case of public pages.
 	authed, err := oauth.Authed(c, false, false, false, false)
 	if err != nil {
 		apiutil.WebErrorHandler(c, gtserror.NewErrorUnauthorized(err, err.Error()), m.processor.InstanceGetV1)
 		return
 	}
 
-	username := strings.ToLower(c.Param(usernameKey))
-	if username == "" {
-		err := errors.New("no account username specified")
-		apiutil.WebErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
-		return
-	}
-
-	instance, err := m.processor.InstanceGetV1(ctx)
-	if err != nil {
-		apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1)
-		return
-	}
-
-	instanceGet := func(ctx context.Context) (*apimodel.InstanceV1, gtserror.WithCode) {
-		return instance, nil
-	}
-
-	account, errWithCode := m.processor.Account().GetLocalByUsername(ctx, authed.Account, username)
+	// Fetch the target account so we can do some checks on it.
+	targetAccount, errWithCode := m.processor.Account().GetLocalByUsername(ctx, authed.Account, targetUsername)
 	if errWithCode != nil {
 		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
 		return
 	}
 
-	// if we're getting an AP request on this endpoint we
-	// should render the account's AP representation instead
-	accept := apiutil.NegotiateFormat(c, string(apiutil.TextHTML), string(apiutil.AppActivityJSON), string(apiutil.AppActivityLDJSON))
-	if accept == string(apiutil.AppActivityJSON) || accept == string(apiutil.AppActivityLDJSON) {
-		m.returnAPProfile(c, username, accept)
+	// If target account is suspended, this page should not be visible.
+	// TODO: change this to 410?
+	if targetAccount.Suspended {
+		err := fmt.Errorf("target account %s is suspended", targetUsername)
+		apiutil.WebErrorHandler(c, gtserror.NewErrorNotFound(err), instanceGet)
 		return
 	}
 
+	// Only generate RSS link if account has RSS enabled.
 	var rssFeed string
-	if account.EnableRSS {
-		rssFeed = "/@" + account.Username + "/feed.rss"
+	if targetAccount.EnableRSS {
+		rssFeed = "/@" + targetAccount.Username + "/feed.rss"
 	}
 
-	// only allow search engines / robots to view this page if account is discoverable
+	// Only allow search engines / robots to
+	// index if account is discoverable.
 	var robotsMeta string
-	if account.Discoverable {
+	if targetAccount.Discoverable {
 		robotsMeta = robotsMetaAllowSome
 	}
 
 	// We need to change our response slightly if the
 	// profile visitor is paging through statuses.
 	var (
-		paging      bool
-		pinnedResp  = &apimodel.PageableResponse{}
-		maxStatusID string
+		maxStatusID    = apiutil.ParseMaxID(c.Query(apiutil.MaxIDKey), "")
+		paging         = maxStatusID != ""
+		pinnedStatuses *apimodel.PageableResponse
 	)
 
-	if maxStatusIDString := c.Query(MaxStatusIDKey); maxStatusIDString != "" {
-		maxStatusID = maxStatusIDString
-		paging = true
-	}
-
-	statusResp, errWithCode := m.processor.Account().WebStatusesGet(ctx, account.ID, maxStatusID)
-	if errWithCode != nil {
-		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
-		return
-	}
-
-	// If we're not paging, then the profile visitor
-	// is currently just opening the bare profile, so
-	// load pinned statuses so we can show them at the
-	// top of the profile.
 	if !paging {
-		pinnedResp, errWithCode = m.processor.Account().StatusesGet(ctx, authed.Account, account.ID, 0, false, false, "", "", true, false, false)
+		// Client opened bare profile (from the top)
+		// so load + display pinned statuses.
+		pinnedStatuses, errWithCode = m.processor.Account().PinnedStatusesGet(ctx, authed.Account, targetAccount.ID)
 		if errWithCode != nil {
 			apiutil.WebErrorHandler(c, errWithCode, instanceGet)
 			return
 		}
+	} else {
+		// Don't load pinned statuses at
+		// the top of profile while paging.
+		pinnedStatuses = new(apimodel.PageableResponse)
+	}
+
+	// Get statuses from maxStatusID onwards (or from top if empty string).
+	statusResp, errWithCode := m.processor.Account().WebStatusesGet(ctx, targetAccount.ID, maxStatusID)
+	if errWithCode != nil {
+		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
+		return
 	}
 
 	stylesheets := []string{
@@ -126,34 +151,41 @@ func (m *Module) profileGETHandler(c *gin.Context) {
 		distPathPrefix + "/profile.css",
 	}
 	if config.GetAccountsAllowCustomCSS() {
-		stylesheets = append(stylesheets, "/@"+account.Username+"/custom.css")
+		stylesheets = append(stylesheets, "/@"+targetAccount.Username+"/custom.css")
 	}
 
 	c.HTML(http.StatusOK, "profile.tmpl", gin.H{
 		"instance":         instance,
-		"account":          account,
-		"ogMeta":           ogBase(instance).withAccount(account),
+		"account":          targetAccount,
+		"ogMeta":           ogBase(instance).withAccount(targetAccount),
 		"rssFeed":          rssFeed,
 		"robotsMeta":       robotsMeta,
 		"statuses":         statusResp.Items,
 		"statuses_next":    statusResp.NextLink,
-		"pinned_statuses":  pinnedResp.Items,
+		"pinned_statuses":  pinnedStatuses.Items,
 		"show_back_to_top": paging,
 		"stylesheets":      stylesheets,
 		"javascript":       []string{distPathPrefix + "/frontend.js"},
 	})
 }
 
-func (m *Module) returnAPProfile(c *gin.Context, username string, accept string) {
-	user, errWithCode := m.processor.Fedi().UserGet(c.Request.Context(), username, c.Request.URL)
+// returnAPAccount returns an ActivityPub representation of
+// target account. It will do http signature authentication.
+func (m *Module) returnAPAccount(
+	c *gin.Context,
+	targetUsername string,
+	accept string,
+	instanceGet func(ctx context.Context) (*apimodel.InstanceV1, gtserror.WithCode),
+) {
+	user, errWithCode := m.processor.Fedi().UserGet(c.Request.Context(), targetUsername, c.Request.URL)
 	if errWithCode != nil {
 		apiutil.WebErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
 		return
 	}
 
-	b, mErr := json.Marshal(user)
-	if mErr != nil {
-		err := fmt.Errorf("could not marshal json: %s", mErr)
+	b, err := json.Marshal(user)
+	if err != nil {
+		err := gtserror.Newf("could not marshal json: %w", err)
 		apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1)
 		return
 	}

internal/web/thread.go

@@ -20,7 +20,6 @@ package web
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -36,66 +35,97 @@ import (
 func (m *Module) threadGETHandler(c *gin.Context) {
 	ctx := c.Request.Context()
 
+	// We'll need the instance later, and we can also use it
+	// before then to make it easier to return a web error.
+	instance, errWithCode := m.processor.InstanceGetV1(ctx)
+	if errWithCode != nil {
+		apiutil.WebErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
+		return
+	}
+
+	// Return instance we already got from the db,
+	// don't try to fetch it again when erroring.
+	instanceGet := func(ctx context.Context) (*apimodel.InstanceV1, gtserror.WithCode) {
+		return instance, nil
+	}
+
+	// Parse account targetUsername and status ID from the URL.
+	targetUsername, errWithCode := apiutil.ParseWebUsername(c.Param(apiutil.WebUsernameKey))
+	if errWithCode != nil {
+		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
+		return
+	}
+
+	targetStatusID, errWithCode := apiutil.ParseWebStatusID(c.Param(apiutil.WebStatusIDKey))
+	if errWithCode != nil {
+		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
+		return
+	}
+
+	// Normalize requested username + status ID:
+	//
+	//   - Usernames on our instance are (currently) always lowercase.
+	//   - StatusIDs on our instance are (currently) always ULIDs.
+	//
+	// todo: Update this logic when different username patterns
+	// are allowed, and/or when status slugs are introduced.
+	targetUsername = strings.ToLower(targetUsername)
+	targetStatusID = strings.ToUpper(targetStatusID)
+
+	// Check what type of content is being requested. If we're getting an AP
+	// request on this endpoint we should render the AP representation instead.
+	accept, err := apiutil.NegotiateAccept(c, apiutil.HTMLOrActivityPubHeaders...)
+	if err != nil {
+		apiutil.WebErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), instanceGet)
+		return
+	}
+
+	if accept == string(apiutil.AppActivityJSON) || accept == string(apiutil.AppActivityLDJSON) {
+		// AP status representation has been requested.
+		m.returnAPStatus(c, targetUsername, targetStatusID, accept, instanceGet)
+		return
+	}
+
+	// text/html has been requested. Proceed with getting the web view of the status.
+
+	// Don't require auth for web endpoints, but do take it if it was provided.
+	// authed.Account might end up nil here, but that's fine in case of public pages.
 	authed, err := oauth.Authed(c, false, false, false, false)
 	if err != nil {
 		apiutil.WebErrorHandler(c, gtserror.NewErrorUnauthorized(err, err.Error()), m.processor.InstanceGetV1)
 		return
 	}
 
-	// usernames on our instance will always be lowercase
-	username := strings.ToLower(c.Param(usernameKey))
-	if username == "" {
-		err := errors.New("no account username specified")
-		apiutil.WebErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
-		return
-	}
-
-	// status ids will always be uppercase
-	statusID := strings.ToUpper(c.Param(statusIDKey))
-	if statusID == "" {
-		err := errors.New("no status id specified")
-		apiutil.WebErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1)
-		return
-	}
-
-	instance, err := m.processor.InstanceGetV1(ctx)
-	if err != nil {
-		apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1)
-		return
-	}
-
-	instanceGet := func(ctx context.Context) (*apimodel.InstanceV1, gtserror.WithCode) {
-		return instance, nil
-	}
-
-	// do this check to make sure the status is actually from a local account,
-	// we shouldn't render threads from statuses that don't belong to us!
-	if _, errWithCode := m.processor.Account().GetLocalByUsername(ctx, authed.Account, username); errWithCode != nil {
-		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
-		return
-	}
-
-	status, errWithCode := m.processor.Status().Get(ctx, authed.Account, statusID)
+	// Fetch the target account so we can do some checks on it.
+	targetAccount, errWithCode := m.processor.Account().GetLocalByUsername(ctx, authed.Account, targetUsername)
 	if errWithCode != nil {
 		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
 		return
 	}
 
-	if !strings.EqualFold(username, status.Account.Username) {
-		err := gtserror.NewErrorNotFound(errors.New("path username not equal to status author username"))
+	// If target account is suspended, this page should not be visible.
+	if targetAccount.Suspended {
+		err := fmt.Errorf("target account %s is suspended", targetUsername)
 		apiutil.WebErrorHandler(c, gtserror.NewErrorNotFound(err), instanceGet)
 		return
 	}
 
-	// if we're getting an AP request on this endpoint we
-	// should render the status's AP representation instead
-	accept := apiutil.NegotiateFormat(c, string(apiutil.TextHTML), string(apiutil.AppActivityJSON), string(apiutil.AppActivityLDJSON))
-	if accept == string(apiutil.AppActivityJSON) || accept == string(apiutil.AppActivityLDJSON) {
-		m.returnAPStatus(c, username, statusID, accept)
+	// Get the status itself from the processor using provided ID and authorization (if any).
+	status, errWithCode := m.processor.Status().Get(ctx, authed.Account, targetStatusID)
+	if errWithCode != nil {
+		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
 		return
 	}
 
-	context, errWithCode := m.processor.Status().ContextGet(ctx, authed.Account, statusID)
+	// Ensure status actually belongs to target account.
+	if status.GetAccountID() != targetAccount.ID {
+		err := fmt.Errorf("target account %s does not own status %s", targetUsername, targetStatusID)
+		apiutil.WebErrorHandler(c, gtserror.NewErrorNotFound(err), instanceGet)
+		return
+	}
+
+	// Fill in the rest of the thread context.
+	context, errWithCode := m.processor.Status().ContextGet(ctx, authed.Account, targetStatusID)
 	if errWithCode != nil {
 		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
 		return
@@ -106,7 +136,7 @@ func (m *Module) threadGETHandler(c *gin.Context) {
 		distPathPrefix + "/status.css",
 	}
 	if config.GetAccountsAllowCustomCSS() {
-		stylesheets = append(stylesheets, "/@"+username+"/custom.css")
+		stylesheets = append(stylesheets, "/@"+targetUsername+"/custom.css")
 	}
 
 	c.HTML(http.StatusOK, "thread.tmpl", gin.H{
@@ -119,17 +149,25 @@ func (m *Module) threadGETHandler(c *gin.Context) {
 	})
 }
 
-func (m *Module) returnAPStatus(c *gin.Context, username string, statusID string, accept string) {
-	status, errWithCode := m.processor.Fedi().StatusGet(c.Request.Context(), username, statusID)
+// returnAPStatus returns an ActivityPub representation of target status,
+// created by targetUsername. It will do http signature authentication.
+func (m *Module) returnAPStatus(
+	c *gin.Context,
+	targetUsername string,
+	targetStatusID string,
+	accept string,
+	instanceGet func(ctx context.Context) (*apimodel.InstanceV1, gtserror.WithCode),
+) {
+	status, errWithCode := m.processor.Fedi().StatusGet(c.Request.Context(), targetUsername, targetStatusID)
 	if errWithCode != nil {
-		apiutil.WebErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
+		apiutil.WebErrorHandler(c, errWithCode, instanceGet)
 		return
 	}
 
-	b, mErr := json.Marshal(status)
-	if mErr != nil {
-		err := fmt.Errorf("could not marshal json: %s", mErr)
-		apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1)
+	b, err := json.Marshal(status)
+	if err != nil {
+		err := gtserror.Newf("could not marshal json: %w", err)
+		apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), instanceGet)
 		return
 	}