From 98073ac28d98ee76b73c764452a4f6b18b0ffb9f Mon Sep 17 00:00:00 2001 From: famfo Date: Mon, 29 Sep 2025 15:35:40 +0200 Subject: [PATCH] feat: ability to filter listed accounts by type in admin dashboard (#9455) Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9455 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Reviewed-by: Gusted Co-authored-by: famfo Co-committed-by: famfo --- models/fixtures/user.yml | 38 ++++++++++++++++++++++++++++ models/user/search.go | 5 ++++ models/user/user_test.go | 4 +-- routers/web/admin/users.go | 16 +++++++++++- templates/admin/user/list.tmpl | 5 ++++ tests/integration/admin_user_test.go | 14 +++++++--- tests/integration/api_admin_test.go | 4 ++- 7 files changed, 79 insertions(+), 7 deletions(-) diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 00aa182540..9e9cb2c1a6 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -1562,3 +1562,41 @@ theme: "" keep_activity_private: false created_unix: 1672578400 + +- + id: 42 + lower_name: federated-example.net + name: federated-example.net + full_name: federated + email: f73240e82-c061-41ef-b7d6-4376cb6f2e1c@example.com + keep_email_private: false + email_notifications_preference: enabled + passwd: ZogKvWdyEx:password + passwd_hash_algo: dummy + must_change_password: false + login_source: 0 + login_name: federated-example.net + 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: false + avatar: "" + avatar_email: f73240e82-c061-41ef-b7d6-4376cb6f2e1c@example.com + 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: 1759086716 diff --git a/models/user/search.go b/models/user/search.go index b30422e269..08cf6a14a3 100644 --- a/models/user/search.go +++ b/models/user/search.go @@ -38,6 +38,7 @@ type SearchUserOptions struct { IsRestricted optional.Option[bool] IsTwoFactorEnabled optional.Option[bool] IsProhibitLogin optional.Option[bool] + AccountType optional.Option[UserType] IncludeReserved bool Load2FAStatus bool @@ -123,6 +124,10 @@ func (opts *SearchUserOptions) toSearchQueryBase(ctx context.Context) *xorm.Sess cond = cond.And(builder.Eq{"prohibit_login": opts.IsProhibitLogin.Value()}) } + if opts.AccountType.Has() { + cond = cond.And(builder.Eq{"type": opts.AccountType.Value()}) + } + e := db.GetEngine(ctx) if !opts.IsTwoFactorEnabled.Has() { return e.Where(cond) diff --git a/models/user/user_test.go b/models/user/user_test.go index e4a94cbc57..20665eb813 100644 --- a/models/user/user_test.go +++ b/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, 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, 42, 1041}) testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(false)}, - []int64{9}) + []int64{42, 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}) diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index c4727177ba..987dc38892 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -48,7 +48,7 @@ func Users(ctx *context.Context) { ctx.Data["PageIsAdminUsers"] = true extraParamStrings := map[string]string{} - statusFilterKeys := []string{"is_active", "is_admin", "is_restricted", "is_2fa_enabled", "is_prohibit_login"} + statusFilterKeys := []string{"is_active", "is_admin", "is_restricted", "is_2fa_enabled", "is_prohibit_login", "account_type"} statusFilterMap := map[string]string{} for _, filterKey := range statusFilterKeys { paramKey := "status_filter[" + filterKey + "]" @@ -59,6 +59,19 @@ func Users(ctx *context.Context) { } } + accountType := statusFilterMap["account_type"] + accountTypeFilter := optional.None[user_model.UserType]() + if accountType != "" { + accountTypeInt, err := strconv.ParseInt(accountType, 10, 0) + if err != nil { + ctx.ServerError("account_type int", err) + return + } + + accountTypeFilter = optional.Some(user_model.UserType(accountTypeInt)) + extraParamStrings["account_type"] = accountType + } + sortType := ctx.FormString("sort") if sortType == "" { sortType = UserSearchDefaultAdminSort @@ -81,6 +94,7 @@ func Users(ctx *context.Context) { IsRestricted: optional.ParseBool(statusFilterMap["is_restricted"]), IsTwoFactorEnabled: optional.ParseBool(statusFilterMap["is_2fa_enabled"]), IsProhibitLogin: optional.ParseBool(statusFilterMap["is_prohibit_login"]), + AccountType: accountTypeFilter, IncludeReserved: true, // administrator needs to list all accounts include reserved, bot, remote ones Load2FAStatus: true, ExtraParamStrings: extraParamStrings, diff --git a/templates/admin/user/list.tmpl b/templates/admin/user/list.tmpl index 368e113d24..47df5002d5 100644 --- a/templates/admin/user/list.tmpl +++ b/templates/admin/user/list.tmpl @@ -32,6 +32,11 @@
+
+ + + + diff --git a/tests/integration/admin_user_test.go b/tests/integration/admin_user_test.go index f460cf22f5..9c37c2eb2e 100644 --- a/tests/integration/admin_user_test.go +++ b/tests/integration/admin_user_test.go @@ -39,6 +39,14 @@ func TestAdminViewUsers(t *testing.T) { // 6th column is the 2FA column. // One user that has TOTP and another user that has WebAuthn. assert.Equal(t, 2, htmlDoc.Find(".admin-setting-content table tbody tr td:nth-child(6) .octicon-check").Length()) + + // account type 5 is for remote users (eg. users from the federation) + req = NewRequest(t, "GET", "/admin/users?status_filter[account_type]=5") + resp = session.MakeRequest(t, req, http.StatusOK) + htmlDoc = NewHTMLParser(t, resp.Body) + + // Only one user (id 42) is a remote user + assert.Equal(t, 1, htmlDoc.Find("table tbody tr").Length()) }) t.Run("Normal user", func(t *testing.T) { @@ -148,7 +156,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, "the_34-user.with.all.allowedChars", users[0].UserName) + assert.Equal(t, "federated-example.net", 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) @@ -192,9 +200,9 @@ func TestAdminViewUsersSorted(t *testing.T) { sortType string expectedUsers []string }{ - {0, "alphabetically", []string{"the_34-user.with.all.allowedChars", "user1", "user10", "user11"}}, + {0, "alphabetically", []string{"federated-example.net", "the_34-user.with.all.allowedChars", "user1", "user10"}}, {0, "reversealphabetically", []string{"user9", "user8", "user5", "user40"}}, - {0, "newest", []string{"user40", "user39", "user38", "user37"}}, + {0, "newest", []string{"federated-example.net", "user40", "user39", "user38"}}, {0, "oldest", []string{"user1", "user2", "user4", "user5"}}, {44, "recentupdate", []string{"sorttest1", "sorttest2", "sorttest3", "sorttest4"}}, {44, "leastupdate", []string{"sorttest10", "sorttest9", "sorttest8", "sorttest7"}}, diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 9351dd9c20..5df6e11c7c 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -126,7 +126,9 @@ func TestAPIListUsers(t *testing.T) { } } assert.True(t, found) - numberOfUsers := unittest.GetCount(t, &user_model.User{}, "type = 0") + numberOfUsers := unittest.GetCount(t, &user_model.User{}, "type = 0") + + unittest.GetCount(t, &user_model.User{}, "type = 5") + assert.Len(t, users, numberOfUsers) }