From d3f35e8eba602dd963a395b304c778e0a776869d Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Sat, 17 Feb 2024 15:20:39 +0100 Subject: [PATCH] [chore] refactor extractFromCtx a bit (#2646) --- internal/federation/federatingdb/accept.go | 19 +++--- internal/federation/federatingdb/announce.go | 13 +++-- internal/federation/federatingdb/create.go | 17 +++--- internal/federation/federatingdb/delete.go | 15 +++-- internal/federation/federatingdb/reject.go | 15 +++-- internal/federation/federatingdb/undo.go | 14 +++-- internal/federation/federatingdb/update.go | 11 ++-- internal/federation/federatingdb/util.go | 61 ++++++++++++++------ 8 files changed, 106 insertions(+), 59 deletions(-) diff --git a/internal/federation/federatingdb/accept.go b/internal/federation/federatingdb/accept.go index 69fb6d67b..e1d754f2e 100644 --- a/internal/federation/federatingdb/accept.go +++ b/internal/federation/federatingdb/accept.go @@ -41,11 +41,14 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA l.Debug("entering Accept") } - receivingAccount, requestingAccount, internal := extractFromCtx(ctx) - if internal { + activityContext := getActivityContext(ctx) + if activityContext.internal { return nil // Already processed. } + requestingAcct := activityContext.requestingAcct + receivingAcct := activityContext.receivingAcct + // Iterate all provided objects in the activity. for _, object := range ap.ExtractObjects(accept) { @@ -65,13 +68,13 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA // Make sure the creator of the original follow // is the same as whatever inbox this landed in. - if gtsFollow.AccountID != receivingAccount.ID { + if gtsFollow.AccountID != receivingAcct.ID { return errors.New("ACCEPT: follow account and inbox account were not the same") } // Make sure the target of the original follow // is the same as the account making the request. - if gtsFollow.TargetAccountID != requestingAccount.ID { + if gtsFollow.TargetAccountID != requestingAcct.ID { return errors.New("ACCEPT: follow target account and requesting account were not the same") } @@ -84,7 +87,7 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA APObjectType: ap.ActivityFollow, APActivityType: ap.ActivityAccept, GTSModel: follow, - ReceivingAccount: receivingAccount, + ReceivingAccount: receivingAcct, }) } @@ -112,13 +115,13 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA // Make sure the creator of the original follow // is the same as whatever inbox this landed in. - if followReq.AccountID != receivingAccount.ID { + if followReq.AccountID != receivingAcct.ID { return errors.New("ACCEPT: follow account and inbox account were not the same") } // Make sure the target of the original follow // is the same as the account making the request. - if followReq.TargetAccountID != requestingAccount.ID { + if followReq.TargetAccountID != requestingAcct.ID { return errors.New("ACCEPT: follow target account and requesting account were not the same") } @@ -131,7 +134,7 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA APObjectType: ap.ActivityFollow, APActivityType: ap.ActivityAccept, GTSModel: follow, - ReceivingAccount: receivingAccount, + ReceivingAccount: receivingAcct, }) continue diff --git a/internal/federation/federatingdb/announce.go b/internal/federation/federatingdb/announce.go index b1bd51659..2ce6d1c59 100644 --- a/internal/federation/federatingdb/announce.go +++ b/internal/federation/federatingdb/announce.go @@ -41,22 +41,25 @@ func (f *federatingDB) Announce(ctx context.Context, announce vocab.ActivityStre l.Debug("entering Announce") } - receivingAccount, requestingAccount, internal := extractFromCtx(ctx) - if internal { + activityContext := getActivityContext(ctx) + if activityContext.internal { return nil // Already processed. } + requestingAcct := activityContext.requestingAcct + receivingAcct := activityContext.receivingAcct + // Ensure requestingAccount is among // the Actors doing the Announce. // // We don't support Announce forwards. actorIRIs := ap.GetActorIRIs(announce) if !slices.ContainsFunc(actorIRIs, func(actorIRI *url.URL) bool { - return actorIRI.String() == requestingAccount.URI + return actorIRI.String() == requestingAcct.URI }) { return gtserror.Newf( "requestingAccount %s was not among Announce Actors", - requestingAccount.URI, + requestingAcct.URI, ) } @@ -76,7 +79,7 @@ func (f *federatingDB) Announce(ctx context.Context, announce vocab.ActivityStre APObjectType: ap.ActivityAnnounce, APActivityType: ap.ActivityCreate, GTSModel: boost, - ReceivingAccount: receivingAccount, + ReceivingAccount: receivingAcct, }) return nil diff --git a/internal/federation/federatingdb/create.go b/internal/federation/federatingdb/create.go index 279d07c66..e2540b739 100644 --- a/internal/federation/federatingdb/create.go +++ b/internal/federation/federatingdb/create.go @@ -63,27 +63,30 @@ func (f *federatingDB) Create(ctx context.Context, asType vocab.Type) error { Trace("entering Create") } - receivingAccount, requestingAccount, internal := extractFromCtx(ctx) - if internal { + activityContext := getActivityContext(ctx) + if activityContext.internal { return nil // Already processed. } + requestingAcct := activityContext.requestingAcct + receivingAcct := activityContext.receivingAcct + switch asType.GetTypeName() { case ap.ActivityBlock: // BLOCK SOMETHING - return f.activityBlock(ctx, asType, receivingAccount, requestingAccount) + return f.activityBlock(ctx, asType, receivingAcct, requestingAcct) case ap.ActivityCreate: // CREATE SOMETHING - return f.activityCreate(ctx, asType, receivingAccount, requestingAccount) + return f.activityCreate(ctx, asType, receivingAcct, requestingAcct) case ap.ActivityFollow: // FOLLOW SOMETHING - return f.activityFollow(ctx, asType, receivingAccount, requestingAccount) + return f.activityFollow(ctx, asType, receivingAcct, requestingAcct) case ap.ActivityLike: // LIKE SOMETHING - return f.activityLike(ctx, asType, receivingAccount, requestingAccount) + return f.activityLike(ctx, asType, receivingAcct, requestingAcct) case ap.ActivityFlag: // FLAG / REPORT SOMETHING - return f.activityFlag(ctx, asType, receivingAccount, requestingAccount) + return f.activityFlag(ctx, asType, receivingAcct, requestingAcct) } return nil diff --git a/internal/federation/federatingdb/delete.go b/internal/federation/federatingdb/delete.go index cca5fdcad..7390cf0f5 100644 --- a/internal/federation/federatingdb/delete.go +++ b/internal/federation/federatingdb/delete.go @@ -40,30 +40,33 @@ func (f *federatingDB) Delete(ctx context.Context, id *url.URL) error { }...) l.Debug("entering Delete") - receivingAccount, requestingAccount, internal := extractFromCtx(ctx) - if internal { + activityContext := getActivityContext(ctx) + if activityContext.internal { return nil // Already processed. } + requestingAcct := activityContext.requestingAcct + receivingAcct := activityContext.receivingAcct + // in a delete we only get the URI, we can't know if we have a status or a profile or something else, // so we have to try a few different things... - if s, err := f.state.DB.GetStatusByURI(ctx, id.String()); err == nil && requestingAccount.ID == s.AccountID { + if s, err := f.state.DB.GetStatusByURI(ctx, id.String()); err == nil && requestingAcct.ID == s.AccountID { l.Debugf("uri is for STATUS with id: %s", s.ID) f.state.Workers.EnqueueFediAPI(ctx, messages.FromFediAPI{ APObjectType: ap.ObjectNote, APActivityType: ap.ActivityDelete, GTSModel: s, - ReceivingAccount: receivingAccount, + ReceivingAccount: receivingAcct, }) } - if a, err := f.state.DB.GetAccountByURI(ctx, id.String()); err == nil && requestingAccount.ID == a.ID { + if a, err := f.state.DB.GetAccountByURI(ctx, id.String()); err == nil && requestingAcct.ID == a.ID { l.Debugf("uri is for ACCOUNT with id %s", a.ID) f.state.Workers.EnqueueFediAPI(ctx, messages.FromFediAPI{ APObjectType: ap.ObjectProfile, APActivityType: ap.ActivityDelete, GTSModel: a, - ReceivingAccount: receivingAccount, + ReceivingAccount: receivingAcct, }) } diff --git a/internal/federation/federatingdb/reject.go b/internal/federation/federatingdb/reject.go index 738087d62..929559031 100644 --- a/internal/federation/federatingdb/reject.go +++ b/internal/federation/federatingdb/reject.go @@ -40,11 +40,14 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR l.Debug("entering Reject") } - receivingAccount, requestingAccount, internal := extractFromCtx(ctx) - if internal { + activityContext := getActivityContext(ctx) + if activityContext.internal { return nil // Already processed. } + requestingAcct := activityContext.requestingAcct + receivingAcct := activityContext.receivingAcct + for _, obj := range ap.ExtractObjects(reject) { if obj.IsIRI() { @@ -59,13 +62,13 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR // Make sure the creator of the original follow // is the same as whatever inbox this landed in. - if followReq.AccountID != receivingAccount.ID { + if followReq.AccountID != receivingAcct.ID { return errors.New("Reject: follow account and inbox account were not the same") } // Make sure the target of the original follow // is the same as the account making the request. - if followReq.TargetAccountID != requestingAccount.ID { + if followReq.TargetAccountID != requestingAcct.ID { return errors.New("Reject: follow target account and requesting account were not the same") } @@ -89,13 +92,13 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR // Make sure the creator of the original follow // is the same as whatever inbox this landed in. - if gtsFollow.AccountID != receivingAccount.ID { + if gtsFollow.AccountID != receivingAcct.ID { return errors.New("Reject: follow account and inbox account were not the same") } // Make sure the target of the original follow // is the same as the account making the request. - if gtsFollow.TargetAccountID != requestingAccount.ID { + if gtsFollow.TargetAccountID != requestingAcct.ID { return errors.New("Reject: follow target account and requesting account were not the same") } diff --git a/internal/federation/federatingdb/undo.go b/internal/federation/federatingdb/undo.go index ccf6397cd..b3d158ba6 100644 --- a/internal/federation/federatingdb/undo.go +++ b/internal/federation/federatingdb/undo.go @@ -44,13 +44,15 @@ func (f *federatingDB) Undo(ctx context.Context, undo vocab.ActivityStreamsUndo) l.Debug("entering Undo") } - receivingAccount, requestingAccount, internal := extractFromCtx(ctx) - if internal { + activityContext := getActivityContext(ctx) + if activityContext.internal { return nil // Already processed. } - var errs gtserror.MultiError + requestingAcct := activityContext.requestingAcct + receivingAcct := activityContext.receivingAcct + var errs gtserror.MultiError for _, object := range ap.ExtractObjects(undo) { // Try to get object as vocab.Type, // else skip handling (likely) IRI. @@ -61,18 +63,18 @@ func (f *federatingDB) Undo(ctx context.Context, undo vocab.ActivityStreamsUndo) switch objType.GetTypeName() { case ap.ActivityFollow: - if err := f.undoFollow(ctx, receivingAccount, requestingAccount, undo, objType); err != nil { + if err := f.undoFollow(ctx, receivingAcct, requestingAcct, undo, objType); err != nil { errs.Appendf("error undoing follow: %w", err) } case ap.ActivityLike: - if err := f.undoLike(ctx, receivingAccount, requestingAccount, undo, objType); err != nil { + if err := f.undoLike(ctx, receivingAcct, requestingAcct, undo, objType); err != nil { errs.Appendf("error undoing like: %w", err) } case ap.ActivityAnnounce: // TODO: actually handle this ! log.Warn(ctx, "skipped undo announce") case ap.ActivityBlock: - if err := f.undoBlock(ctx, receivingAccount, requestingAccount, undo, objType); err != nil { + if err := f.undoBlock(ctx, receivingAcct, requestingAcct, undo, objType); err != nil { errs.Appendf("error undoing block: %w", err) } } diff --git a/internal/federation/federatingdb/update.go b/internal/federation/federatingdb/update.go index 26ea81f72..bd8ad3106 100644 --- a/internal/federation/federatingdb/update.go +++ b/internal/federation/federatingdb/update.go @@ -53,17 +53,20 @@ func (f *federatingDB) Update(ctx context.Context, asType vocab.Type) error { l.Debug("entering Update") } - receivingAccount, requestingAccount, internal := extractFromCtx(ctx) - if internal { + activityContext := getActivityContext(ctx) + if activityContext.internal { return nil // Already processed. } + requestingAcct := activityContext.requestingAcct + receivingAcct := activityContext.receivingAcct + if accountable, ok := ap.ToAccountable(asType); ok { - return f.updateAccountable(ctx, receivingAccount, requestingAccount, accountable) + return f.updateAccountable(ctx, receivingAcct, requestingAcct, accountable) } if statusable, ok := ap.ToStatusable(asType); ok { - return f.updateStatusable(ctx, receivingAccount, requestingAccount, statusable) + return f.updateStatusable(ctx, receivingAcct, requestingAcct, statusable) } return nil diff --git a/internal/federation/federatingdb/util.go b/internal/federation/federatingdb/util.go index f1c565aeb..43d811914 100644 --- a/internal/federation/federatingdb/util.go +++ b/internal/federation/federatingdb/util.go @@ -220,25 +220,52 @@ func (f *federatingDB) collectIRIs(ctx context.Context, iris []*url.URL) (vocab. return collection, nil } -// extractFromCtx extracts some useful values from a context passed into the federatingDB: -// -// - The account that owns the inbox or URI being interacted with. -// - The account that POSTed a request to the inbox. -// - Whether this is an internal request (one originating not from -// the API but from inside the instance). -// -// If the request is internal, the caller can assume that the activity has -// already been processed elsewhere, and should return with no further action. -func extractFromCtx(ctx context.Context) (receivingAccount *gtsmodel.Account, requestingAccount *gtsmodel.Account, internal bool) { - receivingAccount = gtscontext.ReceivingAccount(ctx) - requestingAccount = gtscontext.RequestingAccount(ctx) +// activityContext represents the context in +// which a call to one of the federatingdb +// functions is taking place, including the +// account who initiated the request via POST +// to an inbox, and the account who received +// the request in their inbox. +type activityContext struct { + // The account that owns the inbox + // or URI being interacted with. + receivingAcct *gtsmodel.Account - // If the receiving account wasn't set on the context, that - // means this request didn't pass through the API, but - // came from inside GtS as the result of a local activity. - internal = receivingAccount == nil + // The account whose keyId was used + // to POST a request to the inbox. + requestingAcct *gtsmodel.Account - return + // Whether this is an internal request, + // ie., one originating not from the + // API but from inside the instance. + // + // If the request is internal, it's + // safe to assume that the activity + // has already been processed elsewhere, + // and we can return with no action. + internal bool +} + +// getActivityContext extracts the context in +// which an Activity is taking place from the +// context.Context passed in to one of the +// federatingdb functions. +func getActivityContext(ctx context.Context) activityContext { + receivingAcct := gtscontext.ReceivingAccount(ctx) + requestingAcct := gtscontext.RequestingAccount(ctx) + + // If the receiving account wasn't set on + // the context, that means this request + // didn't pass through the fedi API, but + // came from inside the instance as the + // result of a local activity. + internal := receivingAcct == nil + + return activityContext{ + receivingAcct: receivingAcct, + requestingAcct: requestingAcct, + internal: internal, + } } func marshalItem(item vocab.Type) (string, error) {