From ac564c18624aea229defc38bc1cc516d6c787520 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 7 Jul 2023 14:58:53 +0200 Subject: [PATCH] [bugfix] Reorder web view logic, other small fixes (#1954) --- internal/api/util/parsequery.go | 33 ++++ .../federation/dereferencing/dereferencer.go | 2 +- .../federation/dereferencing/handshake.go | 47 ++++-- internal/processing/account/statuses.go | 6 + internal/processing/fedi/common.go | 48 ++++-- internal/processing/fedi/status.go | 12 +- internal/processing/fedi/user.go | 115 +++++++++---- internal/web/profile.go | 154 +++++++++++------- internal/web/thread.go | 140 ++++++++++------ 9 files changed, 375 insertions(+), 182 deletions(-) diff --git a/internal/api/util/parsequery.go b/internal/api/util/parsequery.go index f5966bca1..59b07f7ee 100644 --- a/internal/api/util/parsequery.go +++ b/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 */ diff --git a/internal/federation/dereferencing/dereferencer.go b/internal/federation/dereferencing/dereferencer.go index 170fb6119..8beba9947 100644 --- a/internal/federation/dereferencing/dereferencer.go +++ b/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. diff --git a/internal/federation/dereferencing/handshake.go b/internal/federation/dereferencing/handshake.go index 749625c2d..96d8d349f 100644 --- a/internal/federation/dereferencing/handshake.go +++ b/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 } } diff --git a/internal/processing/account/statuses.go b/internal/processing/account/statuses.go index cd6859b21..df7064b79 100644 --- a/internal/processing/account/statuses.go +++ b/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) +} 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)) - } - } - - 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) } 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() + // 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 } diff --git a/internal/web/thread.go b/internal/web/thread.go index 41f81eb14..d2ae29c07 100644 --- a/internal/web/thread.go +++ b/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 }