From 59dfab2d8bafd91ce3604c49ffa84c94e989ccca Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Wed, 29 Jan 2025 17:57:55 +0000
Subject: [PATCH] chore: load 2fa status for user search when needed (#6727)

- Don't make an extra database call to gather the 2FA status of the users returned from the search. Only load it for the admin's user list page.
- Integration testing added.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6727
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
---
 models/user/search.go                |  1 +
 routers/web/admin/users.go           |  1 +
 routers/web/explore/user.go          |  4 +++-
 tests/integration/admin_user_test.go | 34 +++++++++++++++++++++++-----
 4 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/models/user/search.go b/models/user/search.go
index cb90ca850e..410222aa08 100644
--- a/models/user/search.go
+++ b/models/user/search.go
@@ -40,6 +40,7 @@ type SearchUserOptions struct {
 	IsProhibitLogin    optional.Option[bool]
 	IncludeReserved    bool
 
+	Load2FAStatus     bool
 	ExtraParamStrings map[string]string
 }
 
diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go
index 6bfc35cb99..36ce8d286c 100644
--- a/routers/web/admin/users.go
+++ b/routers/web/admin/users.go
@@ -83,6 +83,7 @@ func Users(ctx *context.Context) {
 		IsTwoFactorEnabled: util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]),
 		IsProhibitLogin:    util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]),
 		IncludeReserved:    true, // administrator needs to list all accounts include reserved, bot, remote ones
+		Load2FAStatus:      true,
 		ExtraParamStrings:  extraParamStrings,
 	}, tplUsers)
 }
diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go
index 15c60f546f..6f2683b5da 100644
--- a/routers/web/explore/user.go
+++ b/routers/web/explore/user.go
@@ -114,7 +114,9 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions,
 	ctx.Data["Keyword"] = opts.Keyword
 	ctx.Data["Total"] = count
 	ctx.Data["Users"] = users
-	ctx.Data["UsersTwoFaStatus"] = user_model.UserList(users).GetTwoFaStatus(ctx)
+	if opts.Load2FAStatus {
+		ctx.Data["UsersTwoFaStatus"] = user_model.UserList(users).GetTwoFaStatus(ctx)
+	}
 	ctx.Data["ShowUserEmail"] = setting.UI.ShowUserEmail
 	ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled
 
diff --git a/tests/integration/admin_user_test.go b/tests/integration/admin_user_test.go
index 93499e9139..eafa713e68 100644
--- a/tests/integration/admin_user_test.go
+++ b/tests/integration/admin_user_test.go
@@ -26,13 +26,35 @@ import (
 func TestAdminViewUsers(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
-	session := loginUser(t, "user1")
-	req := NewRequest(t, "GET", "/admin/users")
-	session.MakeRequest(t, req, http.StatusOK)
+	t.Run("Admin user", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
 
-	session = loginUser(t, "user2")
-	req = NewRequest(t, "GET", "/admin/users")
-	session.MakeRequest(t, req, http.StatusForbidden)
+		session := loginUser(t, "user1")
+		req := NewRequest(t, "GET", "/admin/users")
+		session.MakeRequest(t, req, http.StatusOK)
+
+		req = NewRequest(t, "GET", "/admin/users?status_filter[is_2fa_enabled]=1")
+		resp := session.MakeRequest(t, req, http.StatusOK)
+		htmlDoc := NewHTMLParser(t, resp.Body)
+
+		// 6th column is the 2FA column.
+		htmlDoc.AssertElement(t, ".admin-setting-content table tbody tr td:nth-child(6) .octicon-check", true)
+	})
+
+	t.Run("Normal user", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		session := loginUser(t, "user2")
+		req := NewRequest(t, "GET", "/admin/users")
+		session.MakeRequest(t, req, http.StatusForbidden)
+	})
+
+	t.Run("Anonymous user", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		req := NewRequest(t, "GET", "/admin/users")
+		MakeRequest(t, req, http.StatusSeeOther)
+	})
 }
 
 func TestAdminViewUser(t *testing.T) {