From 7c2cb4bc734e75e871a8ca1b6fe54862ada07431 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 27 Feb 2024 01:15:56 +0800 Subject: [PATCH 1/3] fix --- models/user/search.go | 3 ++ routers/web/explore/user.go | 25 ++++++++++++-- templates/explore/search.tmpl | 2 -- tests/integration/explore_user_test.go | 46 ++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 tests/integration/explore_user_test.go diff --git a/models/user/search.go b/models/user/search.go index 0fa278c257217..9484bf4425988 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/user.go b/routers/web/explore/user.go index 09d31f95ef47d..81b64bf84253e 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" @@ -79,10 +80,15 @@ 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) { + return + } + opts.Keyword = ctx.FormTrim("q") opts.OrderBy = orderBy if len(opts.Keyword) == 0 || isKeywordValid(opts.Keyword) { @@ -132,8 +138,21 @@ 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) + } + + if !supportedSortOrders.Contains(sortOrder) { + ctx.NotFound("unsupported sort order", nil) + return } RenderUserSearch(ctx, &user_model.SearchUserOptions{ @@ -142,5 +161,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 74b80436dc450..2bb5f319d13fb 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 0000000000000..541b31413e5ee --- /dev/null +++ b/tests/integration/explore_user_test.go @@ -0,0 +1,46 @@ +// 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)() + + req := NewRequest(t, "GET", "/explore/users") + resp := MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), `Newest`) + + req = NewRequest(t, "GET", "/explore/users?sort=oldest") + resp = MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), `Oldest`) + + req = NewRequest(t, "GET", "/explore/users?sort=alphabetically") + resp = MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), `Alphabetically`) + + req = NewRequest(t, "GET", "/explore/users?sort=reversealphabetically") + resp = MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), `Reverse alphabetically`) + + // these sort orders shouldn't be supported, to avoid leaking user activity + cases := []string{ + "/explore/users?sort=lastlogin", + "/explore/users?sort=reverselastlogin", + "/explore/users?sort=leastupdate", + "/explore/users?sort=reverseleastupdate", + } + for _, c := range cases { + req = NewRequest(t, "GET", c).SetHeader("Accept", "text/html") + resp = MakeRequest(t, req, http.StatusNotFound) + assert.Contains(t, resp.Body.String(), `Page Not Found`) + } +} From f6b68be74862c6b9da44879fde1d90506080a7e9 Mon Sep 17 00:00:00 2001 From: wxiaoguang <wxiaoguang@gmail.com> Date: Tue, 27 Feb 2024 01:31:22 +0800 Subject: [PATCH 2/3] rearrange NotFound call --- routers/web/explore/user.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index 81b64bf84253e..0ea4d7613c4a7 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -86,6 +86,7 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, } if opts.SupportedSortOrders != nil && !opts.SupportedSortOrders.Contains(sortOrder) { + ctx.NotFound("unsupported sort order", nil) return } @@ -150,11 +151,6 @@ func Users(ctx *context.Context) { ctx.SetFormString("sort", sortOrder) } - if !supportedSortOrders.Contains(sortOrder) { - ctx.NotFound("unsupported sort order", nil) - return - } - RenderUserSearch(ctx, &user_model.SearchUserOptions{ Actor: ctx.Doer, Type: user_model.UserTypeIndividual, From 7b996ca2b412ba80fb2a20866ad9efdbf469512c Mon Sep 17 00:00:00 2001 From: wxiaoguang <wxiaoguang@gmail.com> Date: Tue, 27 Feb 2024 01:50:48 +0800 Subject: [PATCH 3/3] fix org, rewrite tests --- routers/web/explore/org.go | 15 ++++++++-- tests/integration/explore_user_test.go | 38 ++++++++++++-------------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/routers/web/explore/org.go b/routers/web/explore/org.go index dc1318beefec0..b6202370207c1 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", 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/tests/integration/explore_user_test.go b/tests/integration/explore_user_test.go index 541b31413e5ee..046caf378eff5 100644 --- a/tests/integration/explore_user_test.go +++ b/tests/integration/explore_user_test.go @@ -15,32 +15,30 @@ import ( func TestExploreUser(t *testing.T) { defer tests.PrepareTestEnv(t)() - req := NewRequest(t, "GET", "/explore/users") - resp := MakeRequest(t, req, http.StatusOK) - assert.Contains(t, resp.Body.String(), `<a class="active item" href="/explore/users?sort=newest&q=">Newest</a>`) - - req = NewRequest(t, "GET", "/explore/users?sort=oldest") - resp = MakeRequest(t, req, http.StatusOK) - assert.Contains(t, resp.Body.String(), `<a class="active item" href="/explore/users?sort=oldest&q=">Oldest</a>`) - - req = NewRequest(t, "GET", "/explore/users?sort=alphabetically") - resp = MakeRequest(t, req, http.StatusOK) - assert.Contains(t, resp.Body.String(), `<a class="active item" href="/explore/users?sort=alphabetically&q=">Alphabetically</a>`) - - req = NewRequest(t, "GET", "/explore/users?sort=reversealphabetically") - resp = MakeRequest(t, req, http.StatusOK) - assert.Contains(t, resp.Body.String(), `<a class="active item" href="/explore/users?sort=reversealphabetically&q=">Reverse alphabetically</a>`) + 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 - cases := []string{ + cases404 := []string{ "/explore/users?sort=lastlogin", "/explore/users?sort=reverselastlogin", "/explore/users?sort=leastupdate", "/explore/users?sort=reverseleastupdate", } - for _, c := range cases { - req = NewRequest(t, "GET", c).SetHeader("Accept", "text/html") - resp = MakeRequest(t, req, http.StatusNotFound) - assert.Contains(t, resp.Body.String(), `<title>Page Not Found`) + for _, c := range cases404 { + req := NewRequest(t, "GET", c).SetHeader("Accept", "text/html") + MakeRequest(t, req, http.StatusNotFound) } }