chore(federation): re-enable nilnil lint (#11253)

First round of patches to re-enable some lints from my side.

This PR also refactors the general key fetching code quite a bit due to the way it currently worked
with relying on some values being nil sometimes.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11253
Reviewed-by: elle <0xllx0@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: famfo <famfo@famfo.xyz>
Co-committed-by: famfo <famfo@famfo.xyz>

.deadcode-out

@@ -58,7 +58,6 @@ forgejo.org/models/user
 	IsErrUserSettingIsNotExist
 	GetUserAllSettings
 	DeleteUserSetting
-	GetFederatedUser
 
 forgejo.org/modules/activitypub
 	NewContext

.golangci.yml

@@ -224,9 +224,6 @@ linters:
       - path: models/dbfs/dbfile.go
         linters:
           - nilnil
-      - path: models/forgefed/federationhost_repository.go
-        linters:
-          - nilnil
       - path: models/forgejo_migrations_legacy/v32.go
         linters:
           - nilnil
@@ -284,9 +281,6 @@ linters:
       - path: models/repo/repo.go
         linters:
           - nilnil
-      - path: models/user/user_repository.go
-        linters:
-          - nilnil
       - path: modules/git/commit.go
         linters:
           - nilnil
@@ -380,9 +374,6 @@ linters:
       - path: routers/api/packages/conan/auth.go
         linters:
           - nilnil
-      - path: services/federation/signature_service.go
-        linters:
-          - nilnil
       - path: services/issue/commit.go
         linters:
           - nilnil

models/forgefed/error.go

@@ -0,0 +1,22 @@
+// Copyright 2026 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package forgefed
+
+import (
+	"fmt"
+)
+
+type ErrFederationHostNotFound struct {
+	SearchKey   string
+	SearchValue string
+}
+
+func (err ErrFederationHostNotFound) Error() string {
+	return fmt.Sprintf("ErrFederationHostNotFound: search key: %s, search value: %s", err.SearchKey, err.SearchValue)
+}
+
+func IsErrFederationHostNotFound(err error) bool {
+	_, ok := err.(ErrFederationHostNotFound)
+	return ok
+}

models/forgefed/federationhost_repository.go

@@ -57,13 +57,13 @@ func GetFederationHost(ctx context.Context, ID int64) (*FederationHost, error) {
 	return host, nil
 }
 
-func findFederationHostFromDB(ctx context.Context, searchKey, searchValue string) (*FederationHost, error) {
+func findFederationHostFromDB(ctx context.Context, searchKey string, searchValue ...any) (*FederationHost, error) {
 	host := new(FederationHost)
-	has, err := db.GetEngine(ctx).Where(searchKey, searchValue).Get(host)
+	has, err := db.GetEngine(ctx).Where(searchKey, searchValue...).Get(host)
 	if err != nil {
 		return nil, err
 	} else if !has {
-		return nil, nil
+		return nil, ErrFederationHostNotFound{SearchKey: searchKey, SearchValue: fmt.Sprintf("%v", searchValue)}
 	}
 	if res, err := validation.IsValid(host); !res {
 		return nil, err
@@ -72,17 +72,7 @@ func findFederationHostFromDB(ctx context.Context, searchKey, searchValue string
 }
 
 func FindFederationHostByFqdnAndPort(ctx context.Context, fqdn string, port uint16) (*FederationHost, error) {
-	host := new(FederationHost)
-	has, err := db.GetEngine(ctx).Where("host_fqdn=? AND host_port=?", fqdn, port).Get(host)
-	if err != nil {
-		return nil, err
-	} else if !has {
-		return nil, nil
-	}
-	if res, err := validation.IsValid(host); !res {
-		return nil, err
-	}
-	return host, nil
+	return findFederationHostFromDB(ctx, "host_fqdn=? AND host_port=?", fqdn, port)
 }
 
 func FindFederationHostByKeyID(ctx context.Context, keyID string) (*FederationHost, error) {

models/user/error.go

@@ -105,3 +105,16 @@ func IsErrUserIsNotLocal(err error) bool {
 	_, ok := err.(ErrUserIsNotLocal)
 	return ok
 }
+
+type ErrFederatedUserNotExists struct {
+	Identifier string
+}
+
+func (err ErrFederatedUserNotExists) Error() string {
+	return fmt.Sprintf("No cached federated user found for identifier %s", err.Identifier)
+}
+
+func IsErrFederatedUserNotExists(err error) bool {
+	_, ok := err.(ErrFederatedUserNotExists)
+	return ok
+}

models/user/user_repository.go

@@ -65,7 +65,7 @@ func FindFederatedUser(ctx context.Context, externalID string, federationHostID
 	if err != nil {
 		return nil, nil, err
 	} else if !has {
-		return nil, nil, nil
+		return nil, nil, ErrFederatedUserNotExists{Identifier: externalID}
 	}
 	has, err = db.GetEngine(ctx).ID(federatedUser.UserID).Get(user)
 	if err != nil {
@@ -134,16 +134,6 @@ func FindFederatedUsersByHostID(ctx context.Context, federationHostID int64, opt
 	return users, nil
 }
 
-func GetFederatedUser(ctx context.Context, externalID string, federationHostID int64) (*User, *FederatedUser, error) {
-	user, federatedUser, err := FindFederatedUser(ctx, externalID, federationHostID)
-	if err != nil {
-		return nil, nil, err
-	} else if federatedUser == nil {
-		return nil, nil, fmt.Errorf("FederatedUser not found (given externalId: %v, federationHostId: %v)", externalID, federationHostID)
-	}
-	return user, federatedUser, nil
-}
-
 func GetFederatedUserByUserID(ctx context.Context, userID int64) (*User, *FederatedUser, error) {
 	federatedUser := new(FederatedUser)
 	user := new(User)
@@ -177,7 +167,7 @@ func FindFederatedUserByKeyID(ctx context.Context, keyID string) (*User, *Federa
 	if err != nil {
 		return nil, nil, err
 	} else if !has {
-		return nil, nil, nil
+		return nil, nil, ErrFederatedUserNotExists{Identifier: keyID}
 	}
 	has, err = db.GetEngine(ctx).ID(federatedUser.UserID).Get(user)
 	if err != nil {

modules/forgefed/actor_person.go

@@ -20,6 +20,8 @@ type PersonID struct {
 const (
 	personIDapiPathV1       = "api/v1/activitypub/user-id"
 	personIDapiPathV1Latest = "api/activitypub/user-id"
+	actorIDapiPathV1        = "api/v1/activitypub"
+	actorIDapiPathLatest    = "api/activitypub"
 )
 
 // Factory function for PersonID. Created struct is asserted to be valid
@@ -88,8 +90,11 @@ func (id PersonID) Validate() []string {
 	result = append(result, validation.ValidateOneOf(id.Source, []any{"forgejo", "gitea", "mastodon", "gotosocial"}, "Source")...)
 	if id.Source == "forgejo" {
 		result = append(result, validation.ValidateNotEmpty(id.Path, "path")...)
-		if strings.ToLower(id.Path) != personIDapiPathV1 && strings.ToLower(id.Path) != personIDapiPathV1Latest {
-			result = append(result, fmt.Sprintf("path: %q has to be a person specific api path", id.Path))
+		lowerPath := strings.ToLower(id.Path)
+		if lowerPath != personIDapiPathV1 && lowerPath != personIDapiPathV1Latest {
+			if lowerPath != actorIDapiPathV1 && lowerPath != actorIDapiPathLatest || id.ID != "actor" {
+				result = append(result, fmt.Sprintf("path: %q has to be a person specific api path", id.Path))
+			}
 		}
 	}
 

modules/forgefed/actor_person_test.go

@@ -159,6 +159,48 @@ func TestPersonIdValidation(t *testing.T) {
 	result, err = validation.IsValid(sut)
 	assert.False(t, result)
 	require.EqualError(t, err, "Validation Error: forgefed.PersonID: Field Source contains the value forgejox, which is not in allowed subset [forgejo gitea mastodon gotosocial]")
+
+	sut = forgefed.PersonID{}
+	sut.ID = "actor"
+	sut.Source = "forgejo"
+	sut.HostSchema = "https"
+	sut.Path = "api/v1/activitypub"
+	sut.Host = "example.com"
+	sut.HostPort = 443
+	sut.IsPortSupplemented = true
+	sut.UnvalidatedInput = "https://example.com/api/v1/activitypub/actor"
+
+	result, err = validation.IsValid(sut)
+	assert.True(t, result)
+	require.NoError(t, err)
+
+	sut = forgefed.PersonID{}
+	sut.ID = "actor"
+	sut.Source = "forgejo"
+	sut.HostSchema = "https"
+	sut.Path = "api/activitypub"
+	sut.Host = "example.com"
+	sut.HostPort = 443
+	sut.IsPortSupplemented = true
+	sut.UnvalidatedInput = "https://example.com/api/activitypub/actor"
+
+	result, err = validation.IsValid(sut)
+	assert.True(t, result)
+	require.NoError(t, err)
+
+	sut = forgefed.PersonID{}
+	sut.ID = "1"
+	sut.Source = "forgejo"
+	sut.HostSchema = "https"
+	sut.Path = "api/v1/activitypub"
+	sut.Host = "example.com"
+	sut.HostPort = 443
+	sut.IsPortSupplemented = true
+	sut.UnvalidatedInput = "https://example.com/api/v1/activitypub/1"
+
+	result, err = validation.IsValid(sut)
+	assert.False(t, result)
+	require.EqualError(t, err, "Validation Error: forgefed.PersonID: path: \"api/v1/activitypub\" has to be a person specific api path")
 }
 
 func TestWebfingerId(t *testing.T) {

routers/api/v1/activitypub/reqsignature.go

@@ -30,13 +30,9 @@ func verifyHTTPSignature(ctx app_context.APIContext) (authenticated bool, err er
 
 	log.Debug("Verify %q, signed by KeyId: %v", r.URL.Path, v.KeyId())
 	signatureAlgorithm := httpsig.Algorithm(setting.Federation.SignatureAlgorithms[0])
-	pubKey, err := federation.FindOrCreateFederatedUserKey(ctx, v.KeyId())
-	if err != nil || pubKey == nil {
-		pubKey, err = federation.FindOrCreateFederationHostKey(ctx, v.KeyId())
-		if err != nil {
-			log.Debug("For %q verification failed: %v", r.URL.Path, err)
-			return false, err
-		}
+	pubKey, err := federation.FindOrCreateActorKey(ctx, v.KeyId())
+	if err != nil {
+		return false, err
 	}
 
 	err = v.Verify(pubKey, signatureAlgorithm)

services/federation/error.go

@@ -42,3 +42,11 @@ func HTTPStatus(err error) int {
 		return http.StatusInternalServerError
 	}
 }
+
+type ErrKeyNotFound struct {
+	KeyID string
+}
+
+func (err ErrKeyNotFound) Error() string {
+	return fmt.Sprintf("No key found for key ID: %s", err.KeyID)
+}

services/federation/federation_service.go

@@ -33,80 +33,92 @@ func FindOrCreateFederationHost(ctx context.Context, actorURI string) (*forgefed
 	if err != nil {
 		return nil, err
 	}
+
 	federationHost, err := forgefed.FindFederationHostByFqdnAndPort(ctx, rawActorID.Host, rawActorID.HostPort)
 	if err != nil {
-		return nil, err
-	}
-	if federationHost == nil {
-		result, err := createFederationHostFromAP(ctx, rawActorID)
-		if err != nil {
+		if !forgefed.IsErrFederationHostNotFound(err) {
 			return nil, err
 		}
-		federationHost = result
+
+		federationHost, err = createFederationHostFromAP(ctx, rawActorID)
 	}
-	return federationHost, nil
+
+	return federationHost, err
 }
 
 func FindOrCreateFederatedUser(ctx context.Context, actorURI string) (*user_model.User, *user_model.FederatedUser, *forgefed.FederationHost, error) {
-	user, federatedUser, federationHost, err := findFederatedUser(ctx, actorURI)
+	federationHost, personID, err := findFederationHost(ctx, actorURI)
 	if err != nil {
 		return nil, nil, nil, err
 	}
 
-	personID, err := fm.NewPersonID(actorURI, string(federationHost.NodeInfo.SoftwareName))
+	user, federatedUser, err := findFederatedUser(ctx, actorURI)
+	if err == nil {
+		log.Trace("Found local user: %v", user.Name)
+		return user, federatedUser, federationHost, nil
+	}
+
+	if !user_model.IsErrFederatedUserNotExists(err) {
+		return nil, nil, nil, err
+	}
+
+	// Fetch the remote user
+	apUser, apFederatedUser, err := fetchUserFromAP(ctx, *personID, federationHost)
 	if err != nil {
 		return nil, nil, nil, err
 	}
 
-	if user != nil {
-		log.Trace("Local ActivityPub user found (actorURI: %#v, user: %v)", actorURI, user.Name)
-	} else {
-		log.Trace("Attempting to create new user and federatedUser for actorURI: %#v", actorURI)
-		apUser, apFederatedUser, err := fetchUserFromAP(ctx, personID, federationHost)
-		if err != nil {
-			return nil, nil, nil, err
-		}
-
-		user, federatedUser, federationHost, err = findFederatedUser(ctx, apFederatedUser.NormalizedOriginalURL)
-		if err != nil {
-			return nil, nil, nil, err
-		}
-
-		if user != nil {
-			log.Trace("Resolved alias %s to %s", actorURI, apFederatedUser.NormalizedOriginalURL)
-		} else {
-			user = apUser
-			federatedUser = apFederatedUser
-
-			err := user_model.CreateFederatedUser(ctx, user, federatedUser)
-			if err != nil {
-				return nil, nil, nil, err
-			}
-
-			log.Trace("Created user %s with federatedUser %s from distant server", user.LogString(), federatedUser.LogString())
-		}
+	// User is an alias, for example in newer Mastodon versions
+	// - example.com/@example
+	// - example.com/users/example
+	// have the ID
+	// - example.com/ap/users/<id>
+	user, federatedUser, err = findFederatedUser(ctx, apFederatedUser.NormalizedOriginalURL)
+	if err == nil {
+		log.Trace("Resolved alias %s to %s", actorURI, apFederatedUser.NormalizedOriginalURL)
+		return user, federatedUser, federationHost, nil
 	}
-	log.Trace("Got user: %v", user.Name)
 
-	return user, federatedUser, federationHost, nil
+	err = user_model.CreateFederatedUser(ctx, apUser, apFederatedUser)
+	if err != nil {
+		return nil, nil, nil, err
+	}
+
+	log.Trace("Created user %s with federatedUser %s from distant server", user.LogString(), federatedUser.LogString())
+	return apUser, apFederatedUser, federationHost, nil
 }
 
-func findFederatedUser(ctx context.Context, actorURI string) (*user_model.User, *user_model.FederatedUser, *forgefed.FederationHost, error) {
+func findFederationHost(ctx context.Context, actorURI string) (*forgefed.FederationHost, *fm.PersonID, error) {
 	federationHost, err := FindOrCreateFederationHost(ctx, actorURI)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
+
 	actorID, err := fm.NewPersonID(actorURI, string(federationHost.NodeInfo.SoftwareName))
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 
-	user, federatedUser, err := user_model.FindFederatedUser(ctx, actorID.ID, federationHost.ID)
+	return federationHost, &actorID, nil
+}
+
+func findFederatedUser(ctx context.Context, actorURI string) (*user_model.User, *user_model.FederatedUser, error) {
+	federationHost, _, err := findFederationHost(ctx, actorURI)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 
-	return user, federatedUser, federationHost, nil
+	actorID, err := fm.NewPersonID(actorURI, string(federationHost.NodeInfo.SoftwareName))
+	if err != nil {
+		return nil, nil, err
+	}
+
+	localUser, federatedUser, err := user_model.FindFederatedUser(ctx, actorID.ID, federationHost.ID)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return localUser, federatedUser, nil
 }
 
 func createFederationHostFromAP(ctx context.Context, actorID fm.ActorID) (*forgefed.FederationHost, error) {

services/federation/person_inbox_create.go

@@ -23,15 +23,11 @@ func processPersonInboxCreate(ctx context.Context, user *user.User, activity *ap
 	}
 
 	actorURI := createAct.Actor.GetLink().String()
-	federatedBaseUser, _, _, err := findFederatedUser(ctx, actorURI)
+	federatedBaseUser, _, err := findFederatedUser(ctx, actorURI)
 	if err != nil {
 		log.Error("Federated user not found (%s): %v", actorURI, err)
 		return ServiceResult{}, NewErrNotAcceptablef("federated user not found (%s): %v", actorURI, err)
 	}
-	if federatedBaseUser == nil {
-		log.Error("Federated user not found (%s): %v", actorURI, err)
-		return ServiceResult{}, NewErrNotAcceptablef("federated user not found (%s): %v", actorURI, err)
-	}
 
 	federatedUserActivity, err := activities.NewFederatedUserActivity(
 		user.ID,

services/federation/person_inbox_undo.go

@@ -20,27 +20,25 @@ func processPersonInboxUndo(ctx context.Context, ctxUser *user.User, activity *a
 	}
 
 	actorURI := activity.Actor.GetLink().String()
-	_, federatedUser, _, err := findFederatedUser(ctx, actorURI)
+	_, federatedUser, err := findFederatedUser(ctx, actorURI)
 	if err != nil {
 		log.Error("User not found: %v", err)
 		return ServiceResult{}, NewErrInternalf("User not found: %v", err)
 	}
 
-	if federatedUser != nil {
-		following, err := user.IsFollowingAp(ctx, ctxUser, federatedUser)
-		if err != nil {
-			log.Error("forgefed.IsFollowing: %v", err)
-			return ServiceResult{}, NewErrInternalf("forgefed.IsFollowing: %v", err)
-		}
-		if !following {
-			// The local user is not following the federated one, nothing to do.
-			log.Trace("Local user[%d] is not following federated user[%d]", ctxUser.ID, federatedUser.ID)
-			return NewServiceResultStatusOnly(http.StatusNoContent), nil
-		}
-		if err := user.RemoveFollower(ctx, ctxUser, federatedUser); err != nil {
-			log.Error("Unable to remove follower", err)
-			return ServiceResult{}, NewErrInternalf("Unable to remove follower: %v", err)
-		}
+	following, err := user.IsFollowingAp(ctx, ctxUser, federatedUser)
+	if err != nil {
+		log.Error("forgefed.IsFollowing: %v", err)
+		return ServiceResult{}, NewErrInternalf("forgefed.IsFollowing: %v", err)
+	}
+	if !following {
+		// The local user is not following the federated one, nothing to do.
+		log.Trace("Local user[%d] is not following federated user[%d]", ctxUser.ID, federatedUser.ID)
+		return NewServiceResultStatusOnly(http.StatusNoContent), nil
+	}
+	if err := user.RemoveFollower(ctx, ctxUser, federatedUser); err != nil {
+		log.Error("Unable to remove follower", err)
+		return ServiceResult{}, NewErrInternalf("Unable to remove follower: %v", err)
 	}
 
 	return NewServiceResultStatusOnly(http.StatusNoContent), nil

services/federation/signature_service.go

@@ -15,180 +15,128 @@ import (
 	"forgejo.org/models/forgefed"
 	"forgejo.org/models/user"
 	"forgejo.org/modules/activitypub"
-	fm "forgejo.org/modules/forgefed"
 	"forgejo.org/modules/log"
 
 	ap "github.com/go-ap/activitypub"
 )
 
-// Factory function for ActorID. Created struct is asserted to be valid
-func NewActorIDFromKeyID(ctx context.Context, uri string) (fm.ActorID, error) {
-	parsedURI, err := url.Parse(uri)
-	if err != nil {
-		return fm.ActorID{}, err
-	}
-
-	parsedURI.Fragment = ""
-
-	actionsUser := user.NewAPServerActor()
-	clientFactory, err := activitypub.GetClientFactory(ctx)
-	if err != nil {
-		return fm.ActorID{}, err
-	}
-
-	apClient, err := clientFactory.WithKeys(ctx, actionsUser, actionsUser.KeyID())
-	if err != nil {
-		return fm.ActorID{}, err
-	}
-
-	userResponse, err := apClient.GetBody(parsedURI.String())
-	if err != nil {
-		return fm.ActorID{}, err
-	}
-
-	var actor ap.Actor
-	err = actor.UnmarshalJSON(userResponse)
-	if err != nil {
-		return fm.ActorID{}, err
-	}
-
-	result, err := fm.NewActorID(actor.PublicKey.Owner.String())
-	return result, err
-}
-
-func FindOrCreateFederatedUserKey(ctx context.Context, keyID string) (pubKey any, err error) {
-	log.Trace("KeyID: %v", keyID)
-	var federatedUser *user.FederatedUser
-	var keyURL *url.URL
-
-	keyURL, err = url.Parse(keyID)
-	if err != nil {
-		return nil, err
-	}
-
-	// Try if the signing actor is an already known federated user
-	_, federatedUser, err = user.FindFederatedUserByKeyID(ctx, keyURL.String())
-	if err != nil {
-		return nil, err
-	}
-
-	if federatedUser == nil {
-		rawActorID, err := NewActorIDFromKeyID(ctx, keyID)
-		if err != nil {
-			return nil, err
-		}
-
-		_, federatedUser, _, err = FindOrCreateFederatedUser(ctx, rawActorID.AsURI())
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	if federatedUser.PublicKey.Valid {
-		pubKey, err := x509.ParsePKIXPublicKey(federatedUser.PublicKey.V)
-		if err != nil {
-			return nil, err
-		}
-		log.Trace("For KeyID %v found pubKey %v", keyID, pubKey)
-		return pubKey, nil
-	}
-
-	// Fetch missing public key
-	pubKey, pubKeyBytes, apPerson, err := fetchKeyFromAp(ctx, *keyURL)
-	if err != nil {
-		return nil, err
-	}
-	if apPerson.Type == ap.ActivityVocabularyType("Person") {
-		// Check federatedUser.id = person.id
-		if federatedUser.ExternalID != apPerson.ID.String() {
-			return nil, fmt.Errorf("federated user fetched (%v) does not match the stored one %v", apPerson, federatedUser)
-		}
-		// update federated user
-		federatedUser.KeyID = sql.NullString{
-			String: apPerson.PublicKey.ID.String(),
-			Valid:  true,
-		}
-		federatedUser.PublicKey = sql.Null[sql.RawBytes]{
-			V:     pubKeyBytes,
-			Valid: true,
-		}
-		err = user.UpdateFederatedUser(ctx, federatedUser)
-		if err != nil {
-			return nil, err
-		}
-		log.Trace("For %v found pubKey %v", keyID, pubKey)
-		return pubKey, nil
-	}
-	log.Trace("For %v found no pubKey", keyID)
-	return nil, nil
-}
-
-func FindOrCreateFederationHostKey(ctx context.Context, keyID string) (pubKey any, err error) {
+func FindOrCreateActorKey(ctx context.Context, keyID string) (pubKey any, err error) {
 	log.Trace("KeyID: %v", keyID)
 	keyURL, err := url.Parse(keyID)
 	if err != nil {
 		return nil, err
 	}
-	rawActorID, err := NewActorIDFromKeyID(ctx, keyID)
-	if err != nil {
-		return nil, err
-	}
 
-	// Is there an already known federation host?
-	federationHost, err := forgefed.FindFederationHostByKeyID(ctx, keyURL.String())
+	// Check for existing user key
+	_, federatedUser, err := user.FindFederatedUserByKeyID(ctx, keyURL.String())
 	if err != nil {
-		return nil, err
-	}
+		if !user.IsErrFederatedUserNotExists(err) {
+			return nil, err
+		}
 
-	if federationHost == nil {
-		federationHost, err = FindOrCreateFederationHost(ctx, rawActorID.AsURI())
+		// Check for existing federation host key
+		federationHost, err := forgefed.FindFederationHostByKeyID(ctx, keyURL.String())
+		if err != nil {
+			if !forgefed.IsErrFederationHostNotFound(err) {
+				return nil, err
+			}
+		} else if federationHost.PublicKey.Valid {
+			pubKey, err := x509.ParsePKIXPublicKey(federationHost.PublicKey.V)
+			if err != nil {
+				return nil, err
+			}
+
+			return pubKey, nil
+		}
+	} else if federatedUser.PublicKey.Valid {
+		pubKey, err := x509.ParsePKIXPublicKey(federatedUser.PublicKey.V)
 		if err != nil {
 			return nil, err
 		}
-	}
 
-	// Is there an already an key?
-	if federationHost.PublicKey.Valid {
-		pubKey, err := x509.ParsePKIXPublicKey(federationHost.PublicKey.V)
-		if err != nil {
-			return nil, err
-		}
-		log.Trace("For %v found pubKey: %v", keyID, pubKey)
 		return pubKey, nil
 	}
 
-	// If not, fetch missing public key
-	pubKey, pubKeyBytes, apPerson, err := fetchKeyFromAp(ctx, *keyURL)
+	// Fetch missing key
+	pubKey, pubKeyBytes, actor, err := fetchKeyFromAp(ctx, *keyURL)
 	if err != nil {
 		return nil, err
 	}
-	if apPerson.Type == ap.ActivityVocabularyType("Application") {
-		// Check federationhost.id = person.id
-		if federationHost.HostPort != rawActorID.HostPort || federationHost.HostFqdn != rawActorID.Host ||
-			federationHost.HostSchema != rawActorID.HostSchema {
-			return nil, fmt.Errorf("federation host fetched (%v) does not match the stored one %v", apPerson, federationHost)
-		}
-		// update federation host
-		federationHost.KeyID = sql.NullString{
-			String: apPerson.PublicKey.ID.String(),
-			Valid:  true,
-		}
-		federationHost.PublicKey = sql.Null[sql.RawBytes]{
-			V:     pubKeyBytes,
-			Valid: true,
-		}
-		err = forgefed.UpdateFederationHost(ctx, federationHost)
+
+	switch actor.Type {
+	case ap.PersonType:
+		_, federatedUser, _, err := FindOrCreateFederatedUser(ctx, actor.ID.String())
 		if err != nil {
 			return nil, err
 		}
-		log.Trace("For %v found pubKey: %v", keyID, pubKey)
-		return pubKey, nil
+
+		err = updateFederatedUserKey(ctx, federatedUser, pubKeyBytes, actor)
+		if err != nil {
+			return nil, err
+		}
+	case ap.ApplicationType:
+		federationHost, err := FindOrCreateFederationHost(ctx, actor.ID.String())
+		if err != nil {
+			return nil, err
+		}
+
+		err = updateFederationHostKey(ctx, federationHost, pubKeyBytes, actor)
+		if err != nil {
+			return nil, err
+		}
+	default:
+		return nil, fmt.Errorf("Fetched actortype (%s) is unhandled", actor.Type)
 	}
-	log.Trace("For %v found no pubKey.", keyID)
-	return nil, nil
+
+	return pubKey, nil
 }
 
-func fetchKeyFromAp(ctx context.Context, keyURL url.URL) (pubKey any, pubKeyBytes []byte, apPerson *ap.Person, err error) {
+func updateFederatedUserKey(ctx context.Context, federatedUser *user.FederatedUser, pubKeyBytes []byte, actor *ap.Actor) (err error) {
+	if actor.Type != ap.PersonType {
+		return fmt.Errorf("Fetched user type (%s) is not of user type Person", actor.Type)
+	}
+
+	if federatedUser.NormalizedOriginalURL != actor.ID.String() {
+		return fmt.Errorf("federated user (%s) does not match the stored one %s", actor.ID, federatedUser.NormalizedOriginalURL)
+	}
+
+	federatedUser.KeyID = sql.NullString{
+		String: actor.PublicKey.ID.String(),
+		Valid:  true,
+	}
+
+	federatedUser.PublicKey = sql.Null[sql.RawBytes]{
+		V:     pubKeyBytes,
+		Valid: true,
+	}
+
+	return user.UpdateFederatedUser(ctx, federatedUser)
+}
+
+func updateFederationHostKey(ctx context.Context, federationHost *forgefed.FederationHost, pubKeyBytes []byte, actor *ap.Actor) (err error) {
+	if actor.Type != ap.ApplicationType {
+		return fmt.Errorf("Fetched user type (%s) is not of user type Application", actor.Type)
+	}
+
+	federationHost.KeyID = sql.NullString{
+		String: actor.PublicKey.ID.String(),
+		Valid:  true,
+	}
+
+	federationHost.PublicKey = sql.Null[sql.RawBytes]{
+		V:     pubKeyBytes,
+		Valid: true,
+	}
+
+	err = forgefed.UpdateFederationHost(ctx, federationHost)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func fetchKeyFromAp(ctx context.Context, keyURL url.URL) (pubKey any, pubKeyBytes []byte, apPerson *ap.Actor, err error) {
 	log.Trace("keyURL %v", keyURL)
 	actionsUser := user.NewAPServerActor()
 
@@ -207,13 +155,17 @@ func fetchKeyFromAp(ctx context.Context, keyURL url.URL) (pubKey any, pubKeyByte
 		return nil, nil, nil, err
 	}
 
-	person := ap.PersonNew(ap.IRI(keyURL.String()))
-	err = person.UnmarshalJSON(b)
+	actor := ap.ActorNew(ap.IRI(keyURL.String()), ap.ActorType)
+	err = actor.UnmarshalJSON(b)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("ActivityStreams type cannot be converted to one known to have publicKey property: %w", err)
+		return nil, nil, nil, fmt.Errorf("ActivityStreams object cannot be converted to actor: %w", err)
+	}
+
+	pubKeyFromAp := actor.PublicKey
+	if pubKeyFromAp.PublicKeyPem == "" {
+		return nil, nil, nil, ErrKeyNotFound{KeyID: keyURL.String()}
 	}
 
-	pubKeyFromAp := person.PublicKey
 	if pubKeyFromAp.ID.String() != keyURL.String() {
 		return nil, nil, nil, fmt.Errorf("cannot find publicKey with id: %v in %v", keyURL, string(b))
 	}
@@ -229,7 +181,7 @@ func fetchKeyFromAp(ctx context.Context, keyURL url.URL) (pubKey any, pubKeyByte
 	}
 
 	log.Trace("For %v fetched pubKey %v", keyURL, pubKey)
-	return pubKey, pubKeyBytes, person, err
+	return pubKey, pubKeyBytes, actor, err
 }
 
 func decodePublicKeyPem(pubKeyPem string) ([]byte, error) {

tests/integration/api_activitypub_actor_test.go

@@ -4,19 +4,13 @@
 package integration
 
 import (
-	"fmt"
 	"io"
 	"net/http"
-	"net/url"
-	"strconv"
 	"testing"
 
-	"forgejo.org/modules/forgefed"
 	"forgejo.org/modules/setting"
 	"forgejo.org/modules/test"
 	"forgejo.org/routers"
-	"forgejo.org/services/contexttest"
-	"forgejo.org/services/federation"
 	"forgejo.org/tests"
 
 	ap "github.com/go-ap/activitypub"
@@ -76,27 +70,3 @@ func TestActivityPubActor(t *testing.T) {
 		assert.Nil(t, outboxCollection.Last)
 	})
 }
-
-func TestActorNewFromKeyId(t *testing.T) {
-	defer test.MockVariableValue(&setting.Federation.Enabled, true)()
-	defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
-
-	onApplicationRun(t, func(t *testing.T, u *url.URL) {
-		ctx, _ := contexttest.MockAPIContext(t, "/api/v1/activitypub/actor")
-		sut, err := federation.NewActorIDFromKeyID(ctx.Base, fmt.Sprintf("%sapi/v1/activitypub/actor#main-key", u))
-		require.NoError(t, err)
-
-		port, err := strconv.ParseUint(u.Port(), 10, 16)
-		require.NoError(t, err)
-
-		assert.Equal(t, forgefed.ActorID{
-			ID:                 "actor",
-			HostSchema:         "http",
-			Path:               "api/v1/activitypub",
-			Host:               setting.Domain,
-			HostPort:           uint16(port),
-			UnvalidatedInput:   fmt.Sprintf("http://%s:%d/api/v1/activitypub/actor", setting.Domain, port),
-			IsPortSupplemented: false,
-		}, sut)
-	})
-}

tests/integration/api_federation_httpsig_test.go

@@ -19,7 +19,6 @@ import (
 	"forgejo.org/modules/test"
 	"forgejo.org/routers"
 	"forgejo.org/services/contexttest"
-	"forgejo.org/services/federation"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -42,12 +41,29 @@ func TestFederationHttpSigValidation(t *testing.T) {
 		apClient, err := clientFactory.WithKeys(ctx, user1, user1.KeyID())
 		require.NoError(t, err)
 
+		// HACK HACK HACK: the host part of the URL gets set to which IP forgejo is
+		// listening on, NOT localhost, which is the Domain given to forgejo which
+		// is then used for eg. the keyID all requests
+		applicationKeyID := fmt.Sprintf("%sapi/v1/activitypub/actor#main-key", setting.AppURL)
+		actorKeyID := fmt.Sprintf("%sapi/v1/activitypub/user-id/1#main-key", setting.AppURL)
+
 		// Unsigned request
 		t.Run("UnsignedRequest", func(t *testing.T) {
 			req := NewRequest(t, "GET", userURL)
 			MakeRequest(t, req, http.StatusBadRequest)
 		})
 
+		// Check for missing public keys
+		t.Run("ValidateEmptyCaches", func(t *testing.T) {
+			_, err := forgefed.FindFederationHostByKeyID(db.DefaultContext, applicationKeyID)
+			require.Error(t, err)
+			assert.True(t, forgefed.IsErrFederationHostNotFound(err))
+
+			_, _, err = user.FindFederatedUserByKeyID(db.DefaultContext, actorKeyID)
+			require.Error(t, err)
+			assert.True(t, user.IsErrFederatedUserNotExists(err))
+		})
+
 		// Signed request
 		t.Run("SignedRequest", func(t *testing.T) {
 			resp, err := apClient.Get(userURL)
@@ -55,12 +71,6 @@ func TestFederationHttpSigValidation(t *testing.T) {
 			assert.Equal(t, http.StatusOK, resp.StatusCode)
 		})
 
-		// HACK HACK HACK: the host part of the URL gets set to which IP forgejo is
-		// listening on, NOT localhost, which is the Domain given to forgejo which
-		// is then used for eg. the keyID all requests
-		applicationKeyID := fmt.Sprintf("%sapi/v1/activitypub/actor#main-key", setting.AppURL)
-		actorKeyID := fmt.Sprintf("%sapi/v1/activitypub/user-id/1#main-key", setting.AppURL)
-
 		// Check for cached public keys
 		t.Run("ValidateCaches", func(t *testing.T) {
 			host, err := forgefed.FindFederationHostByKeyID(db.DefaultContext, applicationKeyID)
@@ -74,14 +84,6 @@ func TestFederationHttpSigValidation(t *testing.T) {
 			assert.True(t, user.PublicKey.Valid)
 		})
 
-		t.Run("ValidateActorFromKeyID", func(t *testing.T) {
-			_, err := federation.NewActorIDFromKeyID(ctx, actorKeyID)
-			require.NoError(t, err)
-
-			_, err = federation.NewActorIDFromKeyID(ctx, "http://bad.url/%^&")
-			require.Error(t, err)
-		})
-
 		// Disable signature validation
 		defer test.MockVariableValue(&setting.Federation.SignatureEnforced, false)()