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)

diff --git a/models/user/search.go b/models/user/search.go
index 0fa278c257217ea7aca1cba014422fb55d20f07e..9484bf4425988b3de59ade14a7964268dc8b317d 100644 (file)
--- 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 4a468482ae1931290398012b7c9e696e00e46de1..f8fd6ec38efb6bb6f7d47b9a33e46257ef7cb79a 100644 (file)
--- 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 b67fac2fc133e64fdeefa399a895c446b304b199..41f440f9d963c63e78051b0fe4fe0918f876ae9c 100644 (file)
--- 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 74b80436dc45091a020344ed457a6abe1bf58a4c..2bb5f319d13fbd2914ddf990788a7adc5479e7c7 100644 (file)
--- 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 (file)
index 0000000..046caf3
--- /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)
+       }
+}