From ecc0107809e9521aaaa7c6e0c71f6cec82c7ba77 Mon Sep 17 00:00:00 2001 From: wxiaoguang 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. (cherry picked from commit 2b059f493e46b8b0fb52492623e36a8375cb5fbb) --- 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 @@ {{ctx.Locale.Tr "repo.issues.filter_sort.oldest"}} {{ctx.Locale.Tr "repo.issues.label.filter_sort.alphabetically"}} {{ctx.Locale.Tr "repo.issues.label.filter_sort.reverse_alphabetically"}} - {{ctx.Locale.Tr "repo.issues.filter_sort.recentupdate"}} - {{ctx.Locale.Tr "repo.issues.filter_sort.leastupdate"}} 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) + } +}