From 096c517cc332ce2832a8c5dfbd1cd827ab7d0ecb Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 10 Nov 2023 17:16:58 +0100 Subject: [PATCH] [bugfix] Don't try to update suspended accounts (#2348) * [bugfix] Don't try to update suspended accounts * bail early if requesting account suspended --- internal/federation/dereferencing/account.go | 34 ++++++++++++++------ internal/federation/federatingprotocol.go | 7 ++++ internal/processing/fedi/common.go | 9 +++++- internal/processing/fedi/user.go | 9 +++++- 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go index 04efc464f..b980e09a1 100644 --- a/internal/federation/dereferencing/account.go +++ b/internal/federation/dereferencing/account.go @@ -42,6 +42,11 @@ // accountUpToDate returns whether the given account model is both updateable (i.e. // non-instance remote account) and whether it needs an update based on `fetched_at`. func accountUpToDate(account *gtsmodel.Account) bool { + if !account.SuspendedAt.IsZero() { + // Can't update suspended accounts. + return true + } + if account.IsLocal() { // Can't update local accounts. return true @@ -254,12 +259,14 @@ func (d *deref) RefreshAccount(ctx context.Context, requestUser string, account return nil, nil, err } - // This account was updated, enqueue re-dereference featured posts. - d.state.Workers.Federator.MustEnqueueCtx(ctx, func(ctx context.Context) { - if err := d.dereferenceAccountFeatured(ctx, requestUser, account); err != nil { - log.Errorf(ctx, "error fetching account featured collection: %v", err) - } - }) + if apubAcc != nil { + // This account was updated, enqueue re-dereference featured posts. + d.state.Workers.Federator.MustEnqueueCtx(ctx, func(ctx context.Context) { + if err := d.dereferenceAccountFeatured(ctx, requestUser, account); err != nil { + log.Errorf(ctx, "error fetching account featured collection: %v", err) + } + }) + } return latest, apubAcc, nil } @@ -280,21 +287,28 @@ func (d *deref) RefreshAccountAsync(ctx context.Context, requestUser string, acc // Enqueue a worker function to enrich this account async. d.state.Workers.Federator.MustEnqueueCtx(ctx, func(ctx context.Context) { - latest, _, err := d.enrichAccount(ctx, requestUser, uri, account, apubAcc) + latest, apubAcc, err := d.enrichAccount(ctx, requestUser, uri, account, apubAcc) if err != nil { log.Errorf(ctx, "error enriching remote account: %v", err) return } - // This account was updated, re-dereference account featured posts. - if err := d.dereferenceAccountFeatured(ctx, requestUser, latest); err != nil { - log.Errorf(ctx, "error fetching account featured collection: %v", err) + if apubAcc != nil { + // This account was updated, re-dereference account featured posts. + if err := d.dereferenceAccountFeatured(ctx, requestUser, latest); err != nil { + log.Errorf(ctx, "error fetching account featured collection: %v", err) + } } }) } // enrichAccount will enrich the given account, whether a new barebones model, or existing model from the database. It handles necessary dereferencing, webfingering etc. func (d *deref) enrichAccount(ctx context.Context, requestUser string, uri *url.URL, account *gtsmodel.Account, apubAcc ap.Accountable) (*gtsmodel.Account, ap.Accountable, error) { + // Noop if account has been suspended. + if !account.SuspendedAt.IsZero() { + return account, nil, nil + } + // Pre-fetch a transport for requesting username, used by later deref procedures. tsport, err := d.transportController.NewTransportForUsername(ctx, requestUser) if err != nil { diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go index ea19eb651..f8a67ae20 100644 --- a/internal/federation/federatingprotocol.go +++ b/internal/federation/federatingprotocol.go @@ -288,6 +288,13 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr return nil, false, err } + if !requestingAccount.SuspendedAt.IsZero() { + // Account was marked as suspended by a + // local admin action. Stop request early. + w.WriteHeader(http.StatusForbidden) + return ctx, false, nil + } + // We have everything we need now, set the requesting // and receiving accounts on the context for later use. ctx = gtscontext.SetRequestingAccount(ctx, requestingAccount) diff --git a/internal/processing/fedi/common.go b/internal/processing/fedi/common.go index 38c31ffd2..c41f1e00c 100644 --- a/internal/processing/fedi/common.go +++ b/internal/processing/fedi/common.go @@ -63,6 +63,13 @@ func (p *Processor) authenticate(ctx context.Context, requestedUsername string) return nil, nil, gtserror.NewErrorUnauthorized(err) } + if !requestingAccount.SuspendedAt.IsZero() { + // Account was marked as suspended by a + // local admin action. Stop request early. + err = fmt.Errorf("account %s marked as suspended", requestingAccount.ID) + return nil, nil, gtserror.NewErrorForbidden(err) + } + // Ensure no block exists between requester + requested. blocked, err := p.state.DB.IsEitherBlocked(ctx, requestedAccount.ID, requestingAccount.ID) if err != nil { @@ -72,7 +79,7 @@ func (p *Processor) authenticate(ctx context.Context, requestedUsername string) if blocked { err = fmt.Errorf("block exists between accounts %s and %s", requestedAccount.ID, requestingAccount.ID) - return nil, nil, gtserror.NewErrorUnauthorized(err) + return nil, nil, gtserror.NewErrorForbidden(err) } return requestedAccount, requestingAccount, nil diff --git a/internal/processing/fedi/user.go b/internal/processing/fedi/user.go index 67f137f25..17663a8f4 100644 --- a/internal/processing/fedi/user.go +++ b/internal/processing/fedi/user.go @@ -106,6 +106,13 @@ func (p *Processor) UserGet(ctx context.Context, requestedUsername string, reque return nil, gtserror.NewErrorUnauthorized(err) } + if !requestingAccount.SuspendedAt.IsZero() { + // Account was marked as suspended by a + // local admin action. Stop request early. + err = fmt.Errorf("account %s marked as suspended", requestingAccount.ID) + return nil, gtserror.NewErrorForbidden(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) @@ -114,7 +121,7 @@ func (p *Processor) UserGet(ctx context.Context, requestedUsername string, reque if blocked { err := fmt.Errorf("account %s blocks account %s", requestedAccount.ID, requestingAccount.ID) - return nil, gtserror.NewErrorUnauthorized(err) + return nil, gtserror.NewErrorForbidden(err) } return data(person)