From c67cc791c4f98275869fe3720f59548fc970e6af Mon Sep 17 00:00:00 2001 From: Michael Jerger Date: Mon, 21 Jul 2025 13:23:27 +0200 Subject: [PATCH] limit ctx access on service level --- routers/api/v1/activitypub/repository.go | 9 ++-- routers/api/v1/activitypub/reqsignature.go | 26 +++++------ routers/api/v1/activitypub/response.go | 34 ++++++++++++++ routers/api/v1/api.go | 3 +- services/federation/error.go | 44 +++++++++++++++++++ services/federation/federation_service.go | 19 ++++---- ...{repo_like.go => repository_inbox_like.go} | 33 +++++++------- services/federation/repository_service.go | 19 ++++++++ services/federation/result.go | 35 +++++++++++++++ services/federation/signature_service.go | 11 ++--- 10 files changed, 184 insertions(+), 49 deletions(-) create mode 100644 services/federation/error.go rename services/federation/{repo_like.go => repository_inbox_like.go} (73%) create mode 100644 services/federation/repository_service.go create mode 100644 services/federation/result.go diff --git a/routers/api/v1/activitypub/repository.go b/routers/api/v1/activitypub/repository.go index c506840f1c..3eaa6b82c5 100644 --- a/routers/api/v1/activitypub/repository.go +++ b/routers/api/v1/activitypub/repository.go @@ -71,10 +71,11 @@ func RepositoryInbox(ctx *context.APIContext) { repository := ctx.Repo.Repository log.Info("RepositoryInbox: repo: %v", repository) form := web.GetForm(ctx) - // TODO: Decide between like/undo{like} activity - httpStatus, title, err := federation.ProcessLikeActivity(ctx, form, repository.ID) + activity := form.(*ap.Activity) + result, err := federation.ProcessRepositoryInbox(ctx, activity, repository.ID) if err != nil { - ctx.Error(httpStatus, title, err) + ctx.Error(federation.HTTPStatus(err), "Processing Repository Inbox failed", result) + return } - ctx.Status(http.StatusNoContent) + responseServiceResult(ctx, result) } diff --git a/routers/api/v1/activitypub/reqsignature.go b/routers/api/v1/activitypub/reqsignature.go index 91274249ec..38cb067b89 100644 --- a/routers/api/v1/activitypub/reqsignature.go +++ b/routers/api/v1/activitypub/reqsignature.go @@ -8,13 +8,13 @@ import ( "forgejo.org/modules/log" "forgejo.org/modules/setting" - gitea_context "forgejo.org/services/context" + services_context "forgejo.org/services/context" "forgejo.org/services/federation" "github.com/42wim/httpsig" ) -func verifyHTTPUserOrInstanceSignature(ctx *gitea_context.APIContext) (authenticated bool, err error) { +func verifyHTTPUserOrInstanceSignature(ctx services_context.APIContext) (authenticated bool, err error) { if !setting.Federation.SignatureEnforced { return true, nil } @@ -28,9 +28,9 @@ func verifyHTTPUserOrInstanceSignature(ctx *gitea_context.APIContext) (authentic } signatureAlgorithm := httpsig.Algorithm(setting.Federation.SignatureAlgorithms[0]) - pubKey, err := federation.FindOrCreateFederatedUserKey(ctx.Base, v.KeyId()) + pubKey, err := federation.FindOrCreateFederatedUserKey(ctx, v.KeyId()) if err != nil || pubKey == nil { - pubKey, err = federation.FindOrCreateFederationHostKey(ctx.Base, v.KeyId()) + pubKey, err = federation.FindOrCreateFederationHostKey(ctx, v.KeyId()) if err != nil { return false, err } @@ -43,7 +43,7 @@ func verifyHTTPUserOrInstanceSignature(ctx *gitea_context.APIContext) (authentic return true, nil } -func verifyHTTPUserSignature(ctx *gitea_context.APIContext) (authenticated bool, err error) { +func verifyHTTPUserSignature(ctx services_context.APIContext) (authenticated bool, err error) { if !setting.Federation.SignatureEnforced { return true, nil } @@ -57,7 +57,7 @@ func verifyHTTPUserSignature(ctx *gitea_context.APIContext) (authenticated bool, } signatureAlgorithm := httpsig.Algorithm(setting.Federation.SignatureAlgorithms[0]) - pubKey, err := federation.FindOrCreateFederatedUserKey(ctx.Base, v.KeyId()) + pubKey, err := federation.FindOrCreateFederatedUserKey(ctx, v.KeyId()) if err != nil { return false, err } @@ -70,9 +70,9 @@ func verifyHTTPUserSignature(ctx *gitea_context.APIContext) (authenticated bool, } // ReqHTTPSignature function -func ReqHTTPUserOrInstanceSignature() func(ctx *gitea_context.APIContext) { - return func(ctx *gitea_context.APIContext) { - if authenticated, err := verifyHTTPUserOrInstanceSignature(ctx); err != nil { +func ReqHTTPUserOrInstanceSignature() func(ctx *services_context.APIContext) { + return func(ctx *services_context.APIContext) { + if authenticated, err := verifyHTTPUserOrInstanceSignature(*ctx); err != nil { log.Warn("verifyHttpSignatures failed: %v", err) ctx.Error(http.StatusBadRequest, "reqSignature", "request signature verification failed") } else if !authenticated { @@ -81,10 +81,10 @@ func ReqHTTPUserOrInstanceSignature() func(ctx *gitea_context.APIContext) { } } -// ReqHTTPSignature function -func ReqHTTPUserSignature() func(ctx *gitea_context.APIContext) { - return func(ctx *gitea_context.APIContext) { - if authenticated, err := verifyHTTPUserSignature(ctx); err != nil { +// ReqHTTPUserSignature function +func ReqHTTPUserSignature() func(ctx *services_context.APIContext) { + return func(ctx *services_context.APIContext) { + if authenticated, err := verifyHTTPUserSignature(*ctx); err != nil { log.Warn("verifyHttpSignatures failed: %v", err) ctx.Error(http.StatusBadRequest, "reqSignature", "request signature verification failed") } else if !authenticated { diff --git a/routers/api/v1/activitypub/response.go b/routers/api/v1/activitypub/response.go index a97f363cc2..64413cebb1 100644 --- a/routers/api/v1/activitypub/response.go +++ b/routers/api/v1/activitypub/response.go @@ -10,12 +10,46 @@ import ( "forgejo.org/modules/forgefed" "forgejo.org/modules/log" "forgejo.org/services/context" + "forgejo.org/services/federation" ap "github.com/go-ap/activitypub" "github.com/go-ap/jsonld" ) // Respond with an ActivityStreams object +func responseServiceResult(ctx *context.APIContext, result federation.ServiceResult) { + ctx.Resp.Header().Add("Content-Type", activitypub.ActivityStreamsContentType) + + switch { + case result.StatusOnly(): + ctx.Resp.WriteHeader(result.HTTPStatus) + return + case result.WithBytes(): + ctx.Resp.WriteHeader(result.HTTPStatus) + if _, err := ctx.Resp.Write(result.Bytes); err != nil { + log.Error("Error writing a response: %v", err) + ctx.Error(http.StatusInternalServerError, "Error writing a response", err) + return + } + case result.WithActivity(): + binary, err := jsonld.WithContext( + jsonld.IRI(ap.ActivityBaseURI), + jsonld.IRI(ap.SecurityContextURI), + jsonld.IRI(forgefed.ForgeFedNamespaceURI), + ).Marshal(result.Activity) + if err != nil { + ctx.ServerError("Marshal", err) + return + } + ctx.Resp.WriteHeader(result.HTTPStatus) + if _, err = ctx.Resp.Write(binary); err != nil { + log.Error("write to resp err: %v", err) + } + } +} + +// Respond with an ActivityStreams object +// Deprecated func response(ctx *context.APIContext, v any) { binary, err := jsonld.WithContext( jsonld.IRI(ap.ActivityBaseURI), diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 3b66d02fba..7437f68b5f 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -69,7 +69,6 @@ import ( repo_model "forgejo.org/models/repo" "forgejo.org/models/unit" user_model "forgejo.org/models/user" - "forgejo.org/modules/forgefed" "forgejo.org/modules/log" "forgejo.org/modules/setting" api "forgejo.org/modules/structs" @@ -841,7 +840,7 @@ func Routes() *web.Route { m.Group("/repository-id/{repository-id}", func() { m.Get("", activitypub.ReqHTTPUserSignature(), activitypub.Repository) m.Post("/inbox", - bind(forgefed.ForgeLike{}), + bind(ap.Activity{}), activitypub.ReqHTTPUserSignature(), activitypub.RepositoryInbox) }, context.RepositoryIDAssignmentAPI()) diff --git a/services/federation/error.go b/services/federation/error.go new file mode 100644 index 0000000000..425035d0d5 --- /dev/null +++ b/services/federation/error.go @@ -0,0 +1,44 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package federation + +import ( + "fmt" + "net/http" +) + +type ErrNotAcceptable struct { + Message string +} + +func NewErrNotAcceptablef(format string, a ...any) ErrNotAcceptable { + message := fmt.Sprintf(format, a...) + return ErrNotAcceptable{Message: message} +} + +func (err ErrNotAcceptable) Error() string { + return fmt.Sprintf("NotAcceptable: %v", err.Message) +} + +type ErrInternal struct { + Message string +} + +func NewErrInternalf(format string, a ...any) ErrInternal { + message := fmt.Sprintf(format, a...) + return ErrInternal{Message: message} +} + +func (err ErrInternal) Error() string { + return fmt.Sprintf("InternalServerError: %v", err.Message) +} + +func HTTPStatus(err error) int { + switch err.(type) { + case ErrNotAcceptable: + return http.StatusNotAcceptable + default: + return http.StatusInternalServerError + } +} diff --git a/services/federation/federation_service.go b/services/federation/federation_service.go index b71d8d2575..36788e725a 100644 --- a/services/federation/federation_service.go +++ b/services/federation/federation_service.go @@ -18,7 +18,6 @@ import ( "forgejo.org/modules/log" "forgejo.org/modules/setting" "forgejo.org/modules/validation" - context_service "forgejo.org/services/context" "github.com/google/uuid" ) @@ -27,7 +26,7 @@ func Init() error { return nil } -func FindOrCreateFederationHost(ctx *context_service.Base, actorURI string) (*forgefed.FederationHost, error) { +func FindOrCreateFederationHost(ctx context.Context, actorURI string) (*forgefed.FederationHost, error) { rawActorID, err := fm.NewActorID(actorURI) if err != nil { return nil, err @@ -46,7 +45,7 @@ func FindOrCreateFederationHost(ctx *context_service.Base, actorURI string) (*fo return federationHost, nil } -func FindOrCreateFederatedUser(ctx *context_service.Base, actorURI string) (*user.User, *user.FederatedUser, *forgefed.FederationHost, error) { +func FindOrCreateFederatedUser(ctx context.Context, actorURI string) (*user.User, *user.FederatedUser, *forgefed.FederationHost, error) { user, federatedUser, federationHost, err := findFederatedUser(ctx, actorURI) if err != nil { return nil, nil, nil, err @@ -57,20 +56,21 @@ func FindOrCreateFederatedUser(ctx *context_service.Base, actorURI string) (*use } if user != nil { - log.Trace("Found local federatedUser: %#v", user) + log.Trace("Local ActivityPub user found (actorURI: %#v, user: %#v)", actorURI, user) } else { + log.Trace("Attempting to create new user and federatedUser for actorURI: %#v", actorURI) user, federatedUser, err = createUserFromAP(ctx, personID, federationHost.ID) if err != nil { return nil, nil, nil, err } - log.Trace("Created federatedUser from ap: %#v", user) + log.Trace("Created user %#v with federatedUser %#v from distant server", user, federatedUser) } log.Trace("Got user: %v", user.Name) return user, federatedUser, federationHost, nil } -func findFederatedUser(ctx *context_service.Base, actorURI string) (*user.User, *user.FederatedUser, *forgefed.FederationHost, error) { +func findFederatedUser(ctx context.Context, actorURI string) (*user.User, *user.FederatedUser, *forgefed.FederationHost, error) { federationHost, err := FindOrCreateFederationHost(ctx, actorURI) if err != nil { return nil, nil, nil, err @@ -90,6 +90,7 @@ func findFederatedUser(ctx *context_service.Base, actorURI string) (*user.User, func createFederationHostFromAP(ctx context.Context, actorID fm.ActorID) (*forgefed.FederationHost, error) { actionsUser := user.NewAPServerActor() + clientFactory, err := activitypub.GetClientFactory(ctx) if err != nil { return nil, err @@ -161,7 +162,7 @@ func fetchUserFromAP(ctx context.Context, personID fm.PersonID, federationHostID return nil, nil, err } - log.Info("Fetched valid person:%q", person) + log.Info("Fetched valid person from distant server: %q", person) localFqdn, err := url.ParseRequestURI(setting.AppURL) if err != nil { @@ -220,7 +221,7 @@ func fetchUserFromAP(ctx context.Context, personID fm.PersonID, federationHostID }, } - log.Info("Fetch federatedUser:%q", federatedUser) + log.Info("Fetched person's %q federatedUser from distant server: %q", person, federatedUser) return &newUser, &federatedUser, nil } @@ -234,6 +235,6 @@ func createUserFromAP(ctx context.Context, personID fm.PersonID, federationHostI return nil, nil, err } - log.Info("Created federatedUser:%q", federatedUser) + log.Info("Created federatedUser: %q", federatedUser) return newUser, federatedUser, nil } diff --git a/services/federation/repo_like.go b/services/federation/repository_inbox_like.go similarity index 73% rename from services/federation/repo_like.go rename to services/federation/repository_inbox_like.go index c1e6500c61..2eef0d0a55 100644 --- a/services/federation/repo_like.go +++ b/services/federation/repository_inbox_like.go @@ -5,11 +5,12 @@ package federation import ( "context" - "errors" "fmt" "net/http" "time" + ap "github.com/go-ap/activitypub" + "forgejo.org/models/forgefed" "forgejo.org/models/repo" "forgejo.org/models/user" @@ -27,32 +28,32 @@ import ( // Validation of incoming RepositoryID against Local RepositoryID // Star the repo if it wasn't already stared // Do some mitigation against out of order attacks -func ProcessLikeActivity(ctx *context_service.APIContext, form any, repositoryID int64) (int, string, error) { - activity := form.(*fm.ForgeLike) - if res, err := validation.IsValid(activity); !res { - return http.StatusNotAcceptable, "Invalid activity", err +func ProcessLikeActivity(ctx context.Context, activity *ap.Activity, repositoryID int64) (ServiceResult, error) { + constructorLikeActivity, _ := fm.NewForgeLike(activity.Actor.GetLink().String(), activity.Object.GetLink().String(), activity.StartTime) + if res, err := validation.IsValid(constructorLikeActivity); !res { + return ServiceResult{}, NewErrNotAcceptablef("Invalid activity: %v", err) } log.Trace("Activity validated: %#v", activity) // parse actorID (person) actorURI := activity.Actor.GetID().String() - user, _, federationHost, err := FindOrCreateFederatedUser(ctx.Base, actorURI) + user, _, federationHost, err := FindOrCreateFederatedUser(ctx, actorURI) if err != nil { - ctx.Error(http.StatusNotAcceptable, "Federated user not found", err) - return http.StatusInternalServerError, "FindOrCreateFederatedUser", err + log.Error("Federated user not found (%s): %v", actorURI, err) + return ServiceResult{}, NewErrNotAcceptablef("FindOrCreateFederatedUser failed: %v", err) } - if !activity.IsNewer(federationHost.LatestActivity) { - return http.StatusNotAcceptable, "Activity out of order.", errors.New("Activity already processed") + if !constructorLikeActivity.IsNewer(federationHost.LatestActivity) { + return ServiceResult{}, NewErrNotAcceptablef("LatestActivity: activity already processed: %v", err) } // parse objectID (repository) - objectID, err := fm.NewRepositoryID(activity.Object.GetID().String(), string(forgefed.ForgejoSourceType)) + objectID, err := fm.NewRepositoryID(constructorLikeActivity.Object.GetID().String(), string(forgefed.ForgejoSourceType)) if err != nil { - return http.StatusNotAcceptable, "Invalid objectId", err + return ServiceResult{}, NewErrNotAcceptablef("Parsing repo objectID failed: %v", err) } if objectID.ID != fmt.Sprint(repositoryID) { - return http.StatusNotAcceptable, "Invalid objectId", err + return ServiceResult{}, NewErrNotAcceptablef("Invalid repoId: %v", err) } log.Trace("Object accepted: %#v", objectID) @@ -61,16 +62,16 @@ func ProcessLikeActivity(ctx *context_service.APIContext, form any, repositoryID if !alreadyStared { err = repo.StarRepo(ctx, user.ID, repositoryID, true) if err != nil { - return http.StatusNotAcceptable, "Error staring", err + return ServiceResult{}, NewErrNotAcceptablef("Staring failed: %v", err) } } federationHost.LatestActivity = activity.StartTime err = forgefed.UpdateFederationHost(ctx, federationHost) if err != nil { - return http.StatusNotAcceptable, "Error updating federatedHost", err + return ServiceResult{}, NewErrNotAcceptablef("Updating federatedHost failed: %v", err) } - return 0, "", nil + return NewServiceResultStatusOnly(http.StatusNoContent), nil } // Create or update a list of FollowingRepo structs diff --git a/services/federation/repository_service.go b/services/federation/repository_service.go new file mode 100644 index 0000000000..7891d786e2 --- /dev/null +++ b/services/federation/repository_service.go @@ -0,0 +1,19 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package federation + +import ( + "context" + + ap "github.com/go-ap/activitypub" +) + +func ProcessRepositoryInbox(ctx context.Context, activity *ap.Activity, repositoryID int64) (ServiceResult, error) { + switch activity.Type { + case ap.LikeType: + return ProcessLikeActivity(ctx, activity, repositoryID) + default: + return ServiceResult{}, NewErrNotAcceptablef("Not a like activity: %v", activity.Type) + } +} diff --git a/services/federation/result.go b/services/federation/result.go new file mode 100644 index 0000000000..47afb2bdf6 --- /dev/null +++ b/services/federation/result.go @@ -0,0 +1,35 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package federation + +import "github.com/go-ap/activitypub" + +type ServiceResult struct { + HTTPStatus int + Bytes []byte + Activity activitypub.Activity + withBytes bool + withActivity bool + statusOnly bool +} + +func NewServiceResultStatusOnly(status int) ServiceResult { + return ServiceResult{HTTPStatus: status, statusOnly: true} +} + +func NewServiceResultWithBytes(status int, bytes []byte) ServiceResult { + return ServiceResult{HTTPStatus: status, Bytes: bytes, withBytes: true} +} + +func (serviceResult ServiceResult) WithBytes() bool { + return serviceResult.withBytes +} + +func (serviceResult ServiceResult) WithActivity() bool { + return serviceResult.withActivity +} + +func (serviceResult ServiceResult) StatusOnly() bool { + return serviceResult.statusOnly +} diff --git a/services/federation/signature_service.go b/services/federation/signature_service.go index e5102b89d8..fd8cbb39cd 100644 --- a/services/federation/signature_service.go +++ b/services/federation/signature_service.go @@ -4,6 +4,7 @@ package federation import ( + "context" "crypto/x509" "database/sql" "encoding/pem" @@ -15,13 +16,12 @@ import ( "forgejo.org/models/user" "forgejo.org/modules/activitypub" fm "forgejo.org/modules/forgefed" - context_service "forgejo.org/services/context" ap "github.com/go-ap/activitypub" ) // Factory function for ActorID. Created struct is asserted to be valid -func NewActorIDFromKeyID(ctx *context_service.Base, uri string) (fm.ActorID, error) { +func NewActorIDFromKeyID(ctx context.Context, uri string) (fm.ActorID, error) { parsedURI, err := url.Parse(uri) parsedURI.Fragment = "" if err != nil { @@ -54,7 +54,7 @@ func NewActorIDFromKeyID(ctx *context_service.Base, uri string) (fm.ActorID, err return result, err } -func FindOrCreateFederatedUserKey(ctx *context_service.Base, keyID string) (pubKey any, err error) { +func FindOrCreateFederatedUserKey(ctx context.Context, keyID string) (pubKey any, err error) { var federatedUser *user.FederatedUser var keyURL *url.URL @@ -122,7 +122,7 @@ func FindOrCreateFederatedUserKey(ctx *context_service.Base, keyID string) (pubK return nil, nil } -func FindOrCreateFederationHostKey(ctx *context_service.Base, keyID string) (pubKey any, err error) { +func FindOrCreateFederationHostKey(ctx context.Context, keyID string) (pubKey any, err error) { keyURL, err := url.Parse(keyID) if err != nil { return nil, err @@ -183,8 +183,9 @@ func FindOrCreateFederationHostKey(ctx *context_service.Base, keyID string) (pub return nil, nil } -func fetchKeyFromAp(ctx *context_service.Base, keyURL url.URL) (pubKey any, pubKeyBytes []byte, apPerson *ap.Person, err error) { +func fetchKeyFromAp(ctx context.Context, keyURL url.URL) (pubKey any, pubKeyBytes []byte, apPerson *ap.Person, err error) { actionsUser := user.NewAPServerActor() + clientFactory, err := activitypub.GetClientFactory(ctx) if err != nil { return nil, nil, nil, err