From 2b059f493e46b8b0fb52492623e36a8375cb5fbb Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Sun, 3 Mar 2024 10:28:45 +0800
Subject: [PATCH] Only use supported sort order for "explore/users" page
(#29430) (#29443)
Backport #29430
Thanks to inferenceus : some sort orders on the "explore/users" page
could list users by their lastlogintime/updatetime.
It leaks user's activity unintentionally. This PR makes that page only
use "supported" sort orders.
Removing the "sort orders" could also be a good solution, while IMO at
the moment keeping the "create time" and "name" orders is also fine, in
case some users would like to find a target user in the search result,
the "sort order" might help.
---
models/user/search.go | 3 ++
routers/web/explore/org.go | 15 +++++++--
routers/web/explore/user.go | 27 +++++++++++++---
templates/explore/search.tmpl | 2 --
tests/integration/explore_user_test.go | 45 ++++++++++++++++++++++++++
5 files changed, 83 insertions(+), 9 deletions(-)
create mode 100644 tests/integration/explore_user_test.go
diff --git a/models/user/search.go b/models/user/search.go
index 0fa278c257..9484bf4425 100644
--- a/models/user/search.go
+++ b/models/user/search.go
@@ -9,6 +9,7 @@ import (
"strings"
"code.gitea.io/gitea/models/db"
+ "code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
@@ -30,6 +31,8 @@ type SearchUserOptions struct {
Actor *User // The user doing the search
SearchByEmail bool // Search by email as well as username/full name
+ SupportedSortOrders container.Set[string] // if not nil, only allow to use the sort orders in this set
+
IsActive util.OptionalBool
IsAdmin util.OptionalBool
IsRestricted util.OptionalBool
diff --git a/routers/web/explore/org.go b/routers/web/explore/org.go
index e37bce6b40..b620237020 100644
--- a/routers/web/explore/org.go
+++ b/routers/web/explore/org.go
@@ -6,6 +6,7 @@ package explore
import (
"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user"
+ "code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/structs"
@@ -24,8 +25,16 @@ func Organizations(ctx *context.Context) {
visibleTypes = append(visibleTypes, structs.VisibleTypeLimited, structs.VisibleTypePrivate)
}
- if ctx.FormString("sort") == "" {
- ctx.SetFormString("sort", UserSearchDefaultSortType)
+ supportedSortOrders := container.SetOf(
+ "newest",
+ "oldest",
+ "alphabetically",
+ "reversealphabetically",
+ )
+ sortOrder := ctx.FormString("sort")
+ if sortOrder == "" {
+ sortOrder = "newest"
+ ctx.SetFormString("sort", sortOrder)
}
RenderUserSearch(ctx, &user_model.SearchUserOptions{
@@ -33,5 +42,7 @@ func Organizations(ctx *context.Context) {
Type: user_model.UserTypeOrganization,
ListOptions: db.ListOptions{PageSize: setting.UI.ExplorePagingNum},
Visible: visibleTypes,
+
+ SupportedSortOrders: supportedSortOrders,
}, tplExploreUsers)
}
diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go
index c760004088..d987bc75e6 100644
--- a/routers/web/explore/user.go
+++ b/routers/web/explore/user.go
@@ -10,6 +10,7 @@ import (
"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base"
+ "code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
@@ -60,8 +61,8 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions,
// we can not set orderBy to `models.SearchOrderByXxx`, because there may be a JOIN in the statement, different tables may have the same name columns
- ctx.Data["SortType"] = ctx.FormString("sort")
- switch ctx.FormString("sort") {
+ sortOrder := ctx.FormString("sort")
+ switch sortOrder {
case "newest":
orderBy = "`user`.id DESC"
case "oldest":
@@ -80,9 +81,15 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions,
fallthrough
default:
// in case the sortType is not valid, we set it to recentupdate
- ctx.Data["SortType"] = "recentupdate"
+ sortOrder = "recentupdate"
orderBy = "`user`.updated_unix DESC"
}
+ ctx.Data["SortType"] = sortOrder
+
+ if opts.SupportedSortOrders != nil && !opts.SupportedSortOrders.Contains(sortOrder) {
+ ctx.NotFound("unsupported sort order", nil)
+ return
+ }
opts.Keyword = ctx.FormTrim("q")
opts.OrderBy = orderBy
@@ -133,8 +140,16 @@ func Users(ctx *context.Context) {
ctx.Data["PageIsExploreUsers"] = true
ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled
- if ctx.FormString("sort") == "" {
- ctx.SetFormString("sort", UserSearchDefaultSortType)
+ supportedSortOrders := container.SetOf(
+ "newest",
+ "oldest",
+ "alphabetically",
+ "reversealphabetically",
+ )
+ sortOrder := ctx.FormString("sort")
+ if sortOrder == "" {
+ sortOrder = "newest"
+ ctx.SetFormString("sort", sortOrder)
}
RenderUserSearch(ctx, &user_model.SearchUserOptions{
@@ -143,5 +158,7 @@ func Users(ctx *context.Context) {
ListOptions: db.ListOptions{PageSize: setting.UI.ExplorePagingNum},
IsActive: util.OptionalBoolTrue,
Visible: []structs.VisibleType{structs.VisibleTypePublic, structs.VisibleTypeLimited, structs.VisibleTypePrivate},
+
+ SupportedSortOrders: supportedSortOrders,
}, tplExploreUsers)
}
diff --git a/templates/explore/search.tmpl b/templates/explore/search.tmpl
index 63b842cbbf..9597c79449 100644
--- a/templates/explore/search.tmpl
+++ b/templates/explore/search.tmpl
@@ -16,8 +16,6 @@
<a class="{{if eq .SortType "oldest"}}active {{end}}item" href="{{$.Link}}?sort=oldest&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.issues.filter_sort.oldest"}}</a>
<a class="{{if eq .SortType "alphabetically"}}active {{end}}item" href="{{$.Link}}?sort=alphabetically&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.issues.label.filter_sort.alphabetically"}}</a>
<a class="{{if eq .SortType "reversealphabetically"}}active {{end}}item" href="{{$.Link}}?sort=reversealphabetically&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.issues.label.filter_sort.reverse_alphabetically"}}</a>
- <a class="{{if eq .SortType "recentupdate"}}active {{end}}item" href="{{$.Link}}?sort=recentupdate&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.issues.filter_sort.recentupdate"}}</a>
- <a class="{{if eq .SortType "leastupdate"}}active {{end}}item" href="{{$.Link}}?sort=leastupdate&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.issues.filter_sort.leastupdate"}}</a>
</div>
</div>
</div>
diff --git a/tests/integration/explore_user_test.go b/tests/integration/explore_user_test.go
new file mode 100644
index 0000000000..33d1cfb41f
--- /dev/null
+++ b/tests/integration/explore_user_test.go
@@ -0,0 +1,45 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+ "net/http"
+ "testing"
+
+ "code.gitea.io/gitea/tests"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestExploreUser(t *testing.T) {
+ defer tests.PrepareTestEnv(t)()
+
+ cases := []struct{ sortOrder, expected string }{
+ {"", "/explore/users?sort=newest&q="},
+ {"newest", "/explore/users?sort=newest&q="},
+ {"oldest", "/explore/users?sort=oldest&q="},
+ {"alphabetically", "/explore/users?sort=alphabetically&q="},
+ {"reversealphabetically", "/explore/users?sort=reversealphabetically&q="},
+ }
+ for _, c := range cases {
+ req := NewRequest(t, "GET", "/explore/users?sort="+c.sortOrder)
+ resp := MakeRequest(t, req, http.StatusOK)
+ h := NewHTMLParser(t, resp.Body)
+ href, _ := h.Find(`.ui.dropdown .menu a.active.item[href^="/explore/users"]`).Attr("href")
+ assert.Equal(t, c.expected, href)
+ }
+
+ // these sort orders shouldn't be supported, to avoid leaking user activity
+ cases404 := []string{
+ "/explore/users?sort=lastlogin",
+ "/explore/users?sort=reverselastlogin",
+ "/explore/users?sort=leastupdate",
+ "/explore/users?sort=reverseleastupdate",
+ }
+ for _, c := range cases404 {
+ req := NewRequest(t, "GET", c)
+ req.Header.Get("Accept: text/html")
+ MakeRequest(t, req, http.StatusNotFound)
+ }
+}