feat(activitypub): use structure @PreferredUsername@host.tld:port for actors (#9254)

This modifies usernames of ActivityPub accounts to use the @example@example.tld
format with an additional optional port component (e.g. @user@example.tld:42).
This allows accounts from ActivityPub servers with more relaxed username
requirements than those of Forgejo's to interact with Forgejo. Forgejo would
also follow a "de facto" standard of ActivityPub implementations.

By separating different information using @'s, we also gain future
opportunities to store more information about ActivityPub accounts internally,
so that we won't have to rely on e.g. the amount of dashes in a username as
my migration currently does.

Continuation of Aravinth's work: https://codeberg.org/forgejo/forgejo/pulls/4778

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9254
Reviewed-by: jerger <jerger@noreply.codeberg.org>
Reviewed-by: Ellen Εμιλία Άννα Zscheile <fogti@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Panagiotis "Ivory" Vasilopoulos <git@n0toose.net>
Co-committed-by: Panagiotis "Ivory" Vasilopoulos <git@n0toose.net>

models/db/name.go

@@ -80,6 +80,31 @@ func (err ErrNameCharsNotAllowed) Unwrap() error {
 	return util.ErrInvalidArgument
 }
 
+// ErrNameActivityPubInvalid represents an error for usernames which cannot
+// belong to ActivityPub accounts.
+type ErrNameActivityPubInvalid struct {
+	Name string
+}
+
+// Similarly to IsErrNameCharsNotAllowed, IsErrNameActivityPubInvalid checks if
+// an error is an ErrNameActivityPubInvalid.
+func IsErrNameActivityPubInvalid(err error) bool {
+	_, ok := err.(ErrNameActivityPubInvalid)
+	return ok
+}
+
+func (err ErrNameActivityPubInvalid) Error() string {
+	return fmt.Sprintf(
+		"name is invalid [%s]: not acceptable for users from federated activitypub instances (e.g. @username@domain.example)",
+		err.Name,
+	)
+}
+
+// Unwrap unwraps this as a ErrInvalidArgument err
+func (err ErrNameActivityPubInvalid) Unwrap() error {
+	return util.ErrInvalidArgument
+}
+
 // IsUsableName checks if name is reserved or pattern of name is not allowed
 // based on given reserved names and patterns.
 // Names are exact match, patterns can be prefix or suffix match with placeholder '*'.

models/fixtures/user.yml

@@ -1566,9 +1566,9 @@
 
 -
   id: 42
-  lower_name: federated-example.net
-  name: federated-example.net
-  full_name: federated
+  lower_name: "@federated@example.net"
+  name: "@federated@example.net"
+  full_name: "@federated@example.net"
   email: f73240e82-c061-41ef-b7d6-4376cb6f2e1c@example.com
   keep_email_private: false
   email_notifications_preference: enabled
@@ -1576,7 +1576,7 @@
   passwd_hash_algo: ""
   must_change_password: false
   login_source: 0
-  login_name: federated-example.net
+  login_name: "@federated@example.net"
   type: 6
   salt: ""
   max_repo_creation: -1

models/forgejo_migrations/v14a_ap-change-fedi-handle-structure.go

@@ -0,0 +1,150 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package forgejo_migrations
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"forgejo.org/models/db"
+	"forgejo.org/models/forgefed"
+	user_model "forgejo.org/models/user"
+	"forgejo.org/modules/log"
+	"forgejo.org/modules/validation"
+	user_service "forgejo.org/services/user"
+
+	"xorm.io/xorm"
+)
+
+func init() {
+	registerMigration(&Migration{
+		Description: "use structure @PreferredUsername@host.tld:port for actors",
+		Upgrade:     changeActivityPubUsernameFormat,
+	})
+}
+
+func changeActivityPubUsernameFormat(x *xorm.Engine) error {
+	// Normally, the db.WithTx statement ensures that the database transaction (aka. all changes made
+	// by this migration) will only be committed if the SQL operations inside of the iteration
+	// (db.Iterate) don't return an error.
+	//
+	// This migration was originally authored with those cases in mind, but it was later agreed that
+	// migrations concerning Forgejo's federation-related components should not return any errors at
+	// this point in time, as federation is not considered to be stable at the moment. For more
+	// information, check the relevant discussion here:
+	// https://codeberg.org/forgejo-contrib/federation/issues/67
+	//
+	// Nevertheless, this structure involves some useful boilerplate that can be used for future
+	// migrations at a later point and has been kept as-is.
+	return db.WithTx(db.DefaultContext, func(ctx context.Context) error {
+		// The transaction is committed only if modifying all federated users is possible.
+		return db.Iterate(ctx, nil, func(ctx context.Context, federatedUser *user_model.FederatedUser) error {
+			// localUser represents the "local" representation of an ActivityPub (federated) user
+			localUser := &user_model.User{}
+			has, err := db.GetEngine(ctx).ID(federatedUser.UserID).Get(localUser)
+			if err != nil {
+				log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: Database error occurred while getting local user (ID: %d), ignoring...: %e", federatedUser.UserID, err)
+				return nil
+			}
+
+			if !has {
+				log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: User missing for federated user: %v", federatedUser)
+				err := user_model.DeleteFederatedUser(ctx, federatedUser.UserID)
+				if err != nil {
+					log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: Database error occurred while deleting federated user (%s), ignoring...: %e", federatedUser, err)
+					return nil
+				}
+			}
+
+			if validation.IsValidActivityPubUsername(localUser.Name) {
+				log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: FederatedUser was already migrated: %v", federatedUser)
+			} else {
+				// Copied from models/forgefed/federationhost_repository.go (forgefed.GetFederationHost),
+				// minus some validation code for FederationHost which we do not otherwise manipulate here.
+				federationHost := new(forgefed.FederationHost)
+				has, err := db.GetEngine(ctx).ID(federatedUser.FederationHostID).Get(federationHost)
+				if err != nil {
+					log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: Database error occurred while looking up federation host info (for %v), ignoring...: %e", federatedUser, err)
+					return nil
+				} else if !has {
+					log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: Federation host for federated user missing, deleting: %v", federatedUser)
+					err := user_model.DeleteFederatedUser(ctx, federatedUser.UserID)
+					if err != nil {
+						log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: Database error occurred while deleting federated user (%v), ignoring...: %e", federatedUser, err)
+						return nil
+					}
+
+					err = user_service.DeleteUser(ctx, localUser, true)
+					if err != nil {
+						log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: Database error occurred while deleting user (%s), ignoring...: %v", localUser.LogString(), err)
+					}
+
+					return nil
+				}
+
+				// Take part of the username before the first dash, reconstruct the rest
+				// of it using whatever we have in FederationHost. Before this migration,
+				// usernames of ActivityPub accounts have an expected format of
+				// "username-subdomain-domain-tld-port". We don't know how many subdomains
+				// there are, but that doesn't matter. We can always get the username unless
+				// if the username of an ActivityPub account was manually changed by an admin,
+				// in which case they should either delete the account or change it back.
+				s := strings.Split(localUser.Name, "-")
+				if len(s) == 0 {
+					log.Warn(
+						"Migration[v14a_ap-change-fedi-handle-structure]: Username %s belonging to federatedUser %v does not contain any dashes, can't construct new username",
+						localUser.Name,
+						federatedUser,
+					)
+					return nil
+				}
+
+				// Were a running Forgejo instance to create a new federated account, would the port
+				// have been marked as "supplemented" (thus leading to its omission)?
+				var newUsername string
+				if (federationHost.HostPort == 443 && federationHost.HostSchema == "https") || (federationHost.HostPort == 80 && federationHost.HostSchema == "http") {
+					newUsername = fmt.Sprintf("@%s@%s", s[0], federationHost.HostFqdn)
+				} else {
+					newUsername = fmt.Sprintf("@%s@%s:%d", s[0], federationHost.HostFqdn, federationHost.HostPort)
+				}
+
+				// Implicitly assumes that there won't be a lower name unique constraint violation.
+				// Potentially a bit paranoid, but why not?
+				userThatShouldntExist := &user_model.User{}
+				lowernameTaken, err := db.GetEngine(ctx).Where("lower_name = ?", strings.ToLower(newUsername)).Table("user").Get(userThatShouldntExist)
+				if err != nil {
+					log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: Database error occurred, skipping migration of %s: %e", localUser.LogString(), err)
+					return nil
+				}
+
+				if lowernameTaken {
+					log.Warn(
+						"Migration[v14a_ap-change-fedi-handle-structure]: New username %s for %s already taken by %s, deleting the former...",
+						newUsername,
+						localUser.LogString(),
+						userThatShouldntExist.LogString(),
+					)
+					err := user_model.DeleteFederatedUser(ctx, localUser.ID)
+					if err != nil {
+						log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: Database error occurred while deleting federated user (%s), ignoring...: %e", localUser.LogString(), err)
+					}
+					return nil
+				}
+
+				// Safe to assume that the following operations should just work now.
+				log.Info("Migration[v14a_ap-change-fedi-handle-structure]: Updating username of %s to %s", localUser.LogString(), newUsername)
+				if _, err := db.GetEngine(ctx).ID(localUser.ID).Cols("lower_name", "name").Update(&user_model.User{
+					LowerName: strings.ToLower(newUsername),
+					Name:      newUsername,
+				}); err != nil {
+					log.Warn("Migration[v14a_ap-change-fedi-handle-structure]: Database error occurred when updating federated user's username (%s), ignoring...: %e", localUser.LogString(), err)
+					return nil
+				}
+			}
+
+			return nil
+		})
+	})
+}

models/user/user.go

@@ -698,6 +698,14 @@ func IsUsableUsername(name string) error {
 	return db.IsUsableName(reservedUsernames, reservedUserPatterns, name)
 }
 
+// IsActivityPubUsername returns an error if a fediverse handle (referred to as a username) cannot exist
+func IsActivityPubUsername(name string) error {
+	if !validation.IsValidActivityPubUsername(name) {
+		return db.ErrNameActivityPubInvalid{Name: name}
+	}
+	return db.IsUsableName(reservedUsernames, reservedUserPatterns, name)
+}
+
 // CreateUserOverwriteOptions are an optional options who overwrite system defaults on user creation
 type CreateUserOverwriteOptions struct {
 	KeepEmailPrivate             optional.Option[bool]
@@ -708,6 +716,7 @@ type CreateUserOverwriteOptions struct {
 	Theme                        *string
 	IsRestricted                 optional.Option[bool]
 	IsActive                     optional.Option[bool]
+	IsActivityPub                optional.Option[bool]
 }
 
 // CreateUser creates record of a new user.
@@ -722,12 +731,26 @@ func AdminCreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUs
 
 // createUser creates record of a new user.
 func createUser(ctx context.Context, u *User, createdByAdmin bool, overwriteDefault ...*CreateUserOverwriteOptions) (err error) {
-	if err = IsUsableUsername(u.Name); err != nil {
+	overwriteDefaultPresent := len(overwriteDefault) != 0 && overwriteDefault[0] != nil
+
+	// If a username is invalid as-is, check whether the username is meant
+	// for an ActivityPub account. Username constraints that belong to "foreign"
+	// ActivityPub servers, whose implementations we cannot control, are expected
+	// to be much less restrictive than those of Forgejo itself.
+	if overwriteDefaultPresent && overwriteDefault[0].IsActivityPub.Has() {
+		if err = IsActivityPubUsername(u.Name); err != nil {
+			return err
+		}
+	} else if err := IsUsableUsername(u.Name); err != nil {
 		return err
 	}
 
 	// Check if the new username can be claimed.
 	// Skip this check if done by an admin.
+	//
+	// Note: This skip should not currently cover usernames that could belong to
+	// fediverse accounts. This "defensive programming" is in place to prevent future
+	// breakage until the ActivityPub component matures more.
 	if !createdByAdmin {
 		if ok, expireTime, err := CanClaimUsername(ctx, u.Name, -1); err != nil {
 			return err
@@ -754,7 +777,7 @@ func createUser(ctx context.Context, u *User, createdByAdmin bool, overwriteDefa
 	}
 
 	// overwrite defaults if set
-	if len(overwriteDefault) != 0 && overwriteDefault[0] != nil {
+	if overwriteDefaultPresent {
 		overwrite := overwriteDefault[0]
 		if overwrite.KeepEmailPrivate.Has() {
 			u.KeepEmailPrivate = overwrite.KeepEmailPrivate.Value()

models/user/user_repository.go

@@ -23,8 +23,9 @@ func CreateFederatedUser(ctx context.Context, user *User, federatedUser *Federat
 		return err
 	}
 	overwrite := CreateUserOverwriteOptions{
-		IsActive:     optional.Some(false),
-		IsRestricted: optional.Some(false),
+		IsActive:      optional.Some(false),
+		IsRestricted:  optional.Some(false),
+		IsActivityPub: optional.Some(true),
 	}
 
 	// Begin transaction

models/user/user_test.go

@@ -440,6 +440,63 @@ func TestCreateUserClaimingUsername(t *testing.T) {
 	})
 }
 
+// Attempts to create a username with a fediverse-format handle, which should
+// fail (without the override IsActivityPub, which is set by CreateFederatedUser)
+func TestCreateUserPlainWithFediverseHandle(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	_, err := db.GetEngine(db.DefaultContext).NoAutoTime().Insert(&user_model.Redirect{RedirectUserID: 1, LowerName: "redirecting", CreatedUnix: timeutil.TimeStampNow()})
+	require.NoError(t, err)
+
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+
+	user.Name = "@example@example.tld"
+	user.LowerName = strings.ToLower(user.Name)
+	user.ID = 0
+	user.Email = "unique@example.com"
+
+	t.Run("Normal creation (without ActivityPub override)", func(t *testing.T) {
+		err = user_model.CreateUser(db.DefaultContext, user)
+		require.Error(t, err)
+		assert.True(t, db.IsErrNameCharsNotAllowed(err))
+	})
+
+	t.Run("Creation as admin (without ActivityPub override)", func(t *testing.T) {
+		err = user_model.AdminCreateUser(db.DefaultContext, user)
+		require.Error(t, err)
+		assert.True(t, db.IsErrNameCharsNotAllowed(err))
+	})
+
+	// Logic borrowed from CreateFederatedUser (which invokes CreateUser), but
+	// we "lend" this here to verify CreateUser's paths.
+	overwrite := user_model.CreateUserOverwriteOptions{
+		IsActive:      optional.Some(false),
+		IsRestricted:  optional.Some(false),
+		IsActivityPub: optional.Some(true),
+	}
+
+	t.Run("Normal creation (with ActivityPub override, invalid format)", func(t *testing.T) {
+		user.Name = "invalid-format-for-an-activitypub-account"
+		user.LowerName = strings.ToLower(user.Name)
+
+		err = user_model.CreateUser(db.DefaultContext, user, &overwrite)
+		require.Error(t, err)
+		assert.True(t, db.IsErrNameActivityPubInvalid(err))
+	})
+
+	t.Run("Normal creation (with ActivityPub override)", func(t *testing.T) {
+		user.Name = "@valid@example.tld"
+		user.LowerName = strings.ToLower(user.Name)
+
+		err = user_model.CreateUser(db.DefaultContext, user, &overwrite)
+		require.NoError(t, err)
+	})
+
+	// Note: We don't expect that admins are able to access any front-facing
+	// function that sets the overwrite (i.e. CreateFederatedUser), hence it
+	// has been omitted for now.
+}
+
 func TestGetUserIDsByNames(t *testing.T) {
 	require.NoError(t, unittest.PrepareTestDatabase())