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)
This commit is contained in:
wxiaoguang 2024-03-03 10:28:45 +08:00 committed by Earl Warren
parent 19c5642971
commit ecc0107809
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
5 changed files with 83 additions and 9 deletions

View file

@ -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

View file

@ -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)
}

View file

@ -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)
}

View file

@ -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>

View file

@ -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)
}
}