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

1 files changed, 87 insertions, 55 deletions

diff --git a/internal/web/profile.go b/internal/web/profile.go
index 2ffc7411e..c16965adc 100644
--- a/internal/web/profile.go
+++ b/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()
 
-	authed, err := oauth.Authed(c, false, false, false, false)
-	if err != nil {
-		apiutil.WebErrorHandler(c, gtserror.NewErrorUnauthorized(err, err.Error()), m.processor.InstanceGetV1)
+	// 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
 	}
 
-	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 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
 	}
 
-	instance, err := m.processor.InstanceGetV1(ctx)
+	// 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.NewErrorInternalError(err), m.processor.InstanceGetV1)
+		apiutil.WebErrorHandler(c, gtserror.NewErrorNotAcceptable(err, err.Error()), instanceGet)
 		return
 	}
 
-	instanceGet := func(ctx context.Context) (*apimodel.InstanceV1, gtserror.WithCode) {
-		return instance, nil
+	if accept == string(apiutil.AppActivityJSON) || accept == string(apiutil.AppActivityLDJSON) {
+		// AP account representation has been requested.
+		m.returnAPAccount(c, targetUsername, accept, instanceGet)
+		return
 	}
 
-	account, errWithCode := m.processor.Account().GetLocalByUsername(ctx, authed.Account, username)
+	// 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
+	}
+
+	// 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
 	}