fix(user): set ActivityPub users to ProhibitLogin (#10434)

* sets all cached ActivityPub users to ProhibitLogin
* creates a new UserType to uniquely identify users from ActivityPub

This has the side effect that ActivityPub users will no longer be listed in the admin view.

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

models/fixtures/user.yml

@@ -1571,13 +1571,13 @@
   email: f73240e82-c061-41ef-b7d6-4376cb6f2e1c@example.com
   keep_email_private: false
   email_notifications_preference: enabled
-  passwd: ZogKvWdyEx:password
-  passwd_hash_algo: dummy
+  passwd: ""
+  passwd_hash_algo: ""
   must_change_password: false
   login_source: 0
   login_name: federated-example.net
-  type: 5
-  salt: ZogKvWdyEx
+  type: 6
+  salt: ""
   max_repo_creation: -1
   is_active: false
   is_admin: false
@@ -1585,7 +1585,7 @@
   allow_git_hook: false
   allow_import_local: false
   allow_create_organization: false
-  prohibit_login: false
+  prohibit_login: true
   avatar: ""
   avatar_email: f73240e82-c061-41ef-b7d6-4376cb6f2e1c@example.com
   use_custom_avatar: false
@@ -1600,3 +1600,41 @@
   theme: ""
   keep_activity_private: false
   created_unix: 1759086716
+
+-
+  id: 43
+  lower_name: imported
+  name: imported
+  full_name: imported
+  email: imported@deprecated-forge.example.org
+  keep_email_private: false
+  email_notifications_preference: enabled
+  passwd: ZogKvWdyEx:password
+  passwd_hash_algo: dummy
+  must_change_password: false
+  login_source: 0
+  login_name: imported
+  type: 5
+  salt: ZogKvWdyEx
+  max_repo_creation: -1
+  is_active: false
+  is_admin: false
+  is_restricted: false
+  allow_git_hook: false
+  allow_import_local: false
+  allow_create_organization: false
+  prohibit_login: true
+  avatar: ""
+  avatar_email: imported@deprecated-forge.example.org
+  use_custom_avatar: false
+  num_followers: 0
+  num_following: 0
+  num_stars: 0
+  num_repos: 0
+  num_teams: 0
+  num_members: 0
+  visibility: 0
+  repo_admin_change_team_access: false
+  theme: ""
+  keep_activity_private: false
+  created_unix: 1765818288

models/forgejo_migrations/v14a_set_remote_user_prohibit_login.go

@@ -0,0 +1,52 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package forgejo_migrations
+
+import (
+	"context"
+
+	"forgejo.org/models/db"
+	user_model "forgejo.org/models/user"
+	"forgejo.org/modules/log"
+
+	"xorm.io/builder"
+	"xorm.io/xorm"
+)
+
+func init() {
+	registerMigration(&Migration{
+		Description: "Set ProhibitLogin and UserTypeActivityPubUser for remote users created from ActivityPub.",
+		Upgrade:     setProhibitLoginActivityPubUser,
+	})
+}
+
+func setProhibitLoginActivityPubUser(x *xorm.Engine) error {
+	return db.WithTx(db.DefaultContext, func(ctx context.Context) error {
+		return db.Iterate(ctx, builder.Eq{"type": 5}, func(ctx context.Context, user *user_model.User) error {
+			log.Info("Checking if user %s is created from ActivityPub", user.LogString())
+
+			// Users created from f3 also have the RemoteUser user type. All
+			// FederatedUser should reference exactly one User.
+			has, err := db.GetEngine(ctx).Table("federated_user").Get(&user_model.FederatedUser{UserID: user.ID})
+			if err != nil {
+				return err
+			}
+
+			if !has {
+				return nil
+			}
+
+			log.Info("Updating user %s", user.LogString())
+			_, err = db.GetEngine(ctx).Table("user").ID(user.ID).Cols("type", "prohibit_login", "passwd", "salt", "passwd_hash_algo").Update(&user_model.User{
+				Type:           user_model.UserTypeActivityPubUser,
+				ProhibitLogin:  true,
+				Passwd:         "",
+				Salt:           "",
+				PasswdHashAlgo: "",
+			})
+
+			return err
+		})
+	})
+}

models/user/user.go

@@ -62,8 +62,11 @@ const (
 	// UserTypeBot defines a bot user
 	UserTypeBot // 4
 
-	// UserTypeRemoteUser defines a remote user for federated users
+	// UserTypeRemoteUser defines a remote user for users created from f3
 	UserTypeRemoteUser // 5
+
+	// UserTypeActivityPubUser defines a user created from ActivityPub
+	UserTypeActivityPubUser // 6
 )
 
 const (
@@ -479,6 +482,10 @@ func (u *User) IsRemote() bool {
 	return u.Type == UserTypeRemoteUser
 }
 
+func (u *User) IsActivityPub() bool {
+	return u.Type == UserTypeActivityPubUser
+}
+
 // DisplayName returns full name if it's not empty,
 // returns username otherwise.
 func (u *User) DisplayName() string {

models/user/user_test.go

@@ -219,10 +219,10 @@ func TestSearchUsers(t *testing.T) {
 	}
 
 	testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
-		[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40, 42, 1041})
+		[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40, 43, 1041})
 
 	testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(false)},
-		[]int64{42, 9})
+		[]int64{43, 9})
 
 	testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
 		[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40, 1041})
@@ -241,7 +241,7 @@ func TestSearchUsers(t *testing.T) {
 		[]int64{29})
 
 	testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: optional.Some(true)},
-		[]int64{1041, 37})
+		[]int64{43, 1041, 37})
 
 	testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: optional.Some(true)},
 		[]int64{24, 32})

services/federation/federation_service.go

@@ -13,7 +13,6 @@ import (
 	"forgejo.org/models/forgefed"
 	"forgejo.org/models/user"
 	"forgejo.org/modules/activitypub"
-	"forgejo.org/modules/auth/password"
 	fm "forgejo.org/modules/forgefed"
 	"forgejo.org/modules/log"
 	"forgejo.org/modules/setting"
@@ -181,11 +180,6 @@ func fetchUserFromAP(ctx context.Context, personID fm.PersonID, federationHostID
 		fullName = name
 	}
 
-	password, err := password.Generate(32)
-	if err != nil {
-		return nil, nil, err
-	}
-
 	inbox, err := url.ParseRequestURI(person.Inbox.GetLink().String())
 	if err != nil {
 		return nil, nil, err
@@ -202,10 +196,12 @@ func fetchUserFromAP(ctx context.Context, personID fm.PersonID, federationHostID
 		FullName:                     fullName,
 		Email:                        email,
 		EmailNotificationsPreference: "disabled",
-		Passwd:                       password,
-		MustChangePassword:           false,
+		ProhibitLogin:                true,
+		Passwd:                       "",
+		Salt:                         "",
+		PasswdHashAlgo:               "",
 		LoginName:                    loginName,
-		Type:                         user.UserTypeRemoteUser,
+		Type:                         user.UserTypeActivityPubUser,
 		IsAdmin:                      false,
 	}
 

tests/integration/admin_user_test.go

@@ -45,7 +45,7 @@ func TestAdminViewUsers(t *testing.T) {
 		resp = session.MakeRequest(t, req, http.StatusOK)
 		htmlDoc = NewHTMLParser(t, resp.Body)
 
-		// Only one user (id 42) is a remote user
+		// Only one user (id 43) is a remote user
 		assert.Equal(t, 1, htmlDoc.Find("table tbody tr").Length())
 	})
 
@@ -197,7 +197,7 @@ func TestSourceId(t *testing.T) {
 	resp = session.MakeRequest(t, req, http.StatusOK)
 	DecodeJSON(t, resp, &users)
 	assert.Len(t, users, 1)
-	assert.Equal(t, "federated-example.net", users[0].UserName)
+	assert.Equal(t, "imported", users[0].UserName)
 
 	// Now our new user should be in the list, because we filter by source_id 23
 	req = NewRequest(t, "GET", "/api/v1/admin/users?limit=1&source_id=23").AddTokenAuth(token)
@@ -241,9 +241,9 @@ func TestAdminViewUsersSorted(t *testing.T) {
 		sortType      string
 		expectedUsers []string
 	}{
-		{0, "alphabetically", []string{"federated-example.net", "the_34-user.with.all.allowedChars", "user1", "user10"}},
+		{0, "alphabetically", []string{"imported", "the_34-user.with.all.allowedChars", "user1", "user10"}},
 		{0, "reversealphabetically", []string{"user9", "user8", "user5", "user40"}},
-		{0, "newest", []string{"federated-example.net", "user40", "user39", "user38"}},
+		{0, "newest", []string{"imported", "user40", "user39", "user38"}},
 		{0, "oldest", []string{"user1", "user2", "user4", "user5"}},
 		{44, "recentupdate", []string{"sorttest1", "sorttest2", "sorttest3", "sorttest4"}},
 		{44, "leastupdate", []string{"sorttest10", "sorttest9", "sorttest8", "sorttest7"}},

tests/integration/api_activitypub_person_inbox_follow_test.go

@@ -73,6 +73,13 @@ func TestActivityPubPersonInboxFollow(t *testing.T) {
 			},
 		)
 
+		user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: distantFederatedUser.UserID})
+		assert.Equal(t, user_model.UserTypeActivityPubUser, user.Type)
+		assert.True(t, user.ProhibitLogin)
+		assert.Empty(t, user.Passwd)
+		assert.Empty(t, user.PasswdHashAlgo)
+		assert.Empty(t, user.Salt)
+
 		// distant is informed about accepting follow
 		assert.Contains(t, mock.LastPost, "\"type\":\"Accept\"")