From 459ee981361f52217f66ea0f6211a666c30793ef Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 27 Feb 2024 17:10:51 +0800 Subject: [PATCH] Only use supported sort order for "explore/users" page (#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. ![image](https://github.com/go-gitea/gitea/assets/2114189/ce5c39c1-1e86-484a-80c3-33cac6419af8) (cherry picked from commit eedb8f41297c343d6073a7bab46e4df6ee297a90) --- models/user/search.go | 3 ++ routers/web/explore/org.go | 15 +++++++-- routers/web/explore/user.go | 21 ++++++++++-- templates/explore/search.tmpl | 2 -- tests/integration/explore_user_test.go | 44 ++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 6 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 4a468482ae..f8fd6ec38e 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/setting" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/services/context" @@ -24,8 +25,16 @@ func Organizations(ctx *context.Context) { visibleTypes = append(visibleTypes, structs.VisibleTypeLimited, structs.VisibleTypePrivate) } - if ctx.FormString("sort") == "" { - ctx.SetFormString("sort", setting.UI.ExploreDefaultSort) + 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 b67fac2fc1..41f440f9d9 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/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/sitemap" @@ -79,10 +80,16 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, fallthrough default: // in case the sortType is not valid, we set it to recentupdate + sortOrder = "recentupdate" ctx.Data["SortType"] = "recentupdate" orderBy = "`user`.updated_unix DESC" } + if opts.SupportedSortOrders != nil && !opts.SupportedSortOrders.Contains(sortOrder) { + ctx.NotFound("unsupported sort order", nil) + return + } + opts.Keyword = ctx.FormTrim("q") opts.OrderBy = orderBy if len(opts.Keyword) == 0 || isKeywordValid(opts.Keyword) { @@ -132,8 +139,16 @@ func Users(ctx *context.Context) { ctx.Data["PageIsExploreUsers"] = true ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled - if ctx.FormString("sort") == "" { - ctx.SetFormString("sort", setting.UI.ExploreDefaultSort) + supportedSortOrders := container.SetOf( + "newest", + "oldest", + "alphabetically", + "reversealphabetically", + ) + sortOrder := ctx.FormString("sort") + if sortOrder == "" { + sortOrder = "newest" + ctx.SetFormString("sort", sortOrder) } RenderUserSearch(ctx, &user_model.SearchUserOptions{ @@ -142,5 +157,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 74b80436dc..2bb5f319d1 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..046caf378e --- /dev/null +++ b/tests/integration/explore_user_test.go @@ -0,0 +1,44 @@ +// 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).SetHeader("Accept", "text/html") + MakeRequest(t, req, http.StatusNotFound) + } +}