Skip to content

Commit

Permalink
Fix some user name usages (#33689)
Browse files Browse the repository at this point in the history 
1. GetUserOrgsList should "order by" lower_name
2. GetIssuePostersWithSearch should search in-case-sensitive-ly
3. LoginName should not be used as username

By the way, remove unnecessary "onGiteaRun"
  • Loading branch information
@wxiaoguang
wxiaoguang authored Feb 23, 2025
1 parent f991807 commit 8ae46d9
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 92 deletions.
1 change: 1 addition & 0 deletions models/organization/org_list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg,
if err := db.GetEngine(ctx).Select(columnsStr).
Table("user").
Where(builder.In("`user`.`id`", queryUserOrgIDs(user.ID, true))).
OrderBy("`user`.lower_name ASC").
Find(&orgs); err != nil {
return nil, err
}
Expand Down
5 changes: 3 additions & 2 deletions models/repo/user_repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package repo

import (
"context"
"strings"

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/perm"
Expand Down Expand Up @@ -149,9 +150,9 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us
// If isShowFullName is set to true, also include full name prefix search
func GetIssuePostersWithSearch(ctx context.Context, repo *Repository, isPull bool, search string, isShowFullName bool) ([]*user_model.User, error) {
users := make([]*user_model.User, 0, 30)
var prefixCond builder.Cond = builder.Like{"name", search + "%"}
var prefixCond builder.Cond = builder.Like{"lower_name", strings.ToLower(search) + "%"}
if isShowFullName {
prefixCond = prefixCond.Or(builder.Like{"full_name", "%" + search + "%"})
prefixCond = prefixCond.Or(db.BuildCaseInsensitiveLike("full_name", "%"+search+"%"))
}

cond := builder.In("`user`.id",
Expand Down
17 changes: 17 additions & 0 deletions models/repo/user_repo_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
user_model "code.gitea.io/gitea/models/user"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestRepoAssignees(t *testing.T) {
Expand All @@ -38,3 +39,19 @@ func TestRepoAssignees(t *testing.T) {
assert.NotContains(t, []int64{users[0].ID, users[1].ID, users[2].ID}, 15)
}
}

func TestGetIssuePostersWithSearch(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})

users, err := repo_model.GetIssuePostersWithSearch(db.DefaultContext, repo2, false, "USER", false /* full name */)
require.NoError(t, err)
require.Len(t, users, 1)
assert.Equal(t, "user2", users[0].Name)

users, err = repo_model.GetIssuePostersWithSearch(db.DefaultContext, repo2, false, "TW%O", true /* full name */)
require.NoError(t, err)
require.Len(t, users, 1)
assert.Equal(t, "user2", users[0].Name)
}
6 changes: 4 additions & 2 deletions routers/api/v1/repo/collaborators.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package repo
import (
"errors"
"net/http"
"strings"

"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
Expand Down Expand Up @@ -274,12 +275,13 @@ func GetRepoPermissions(ctx *context.APIContext) {
// "403":
// "$ref": "#/responses/forbidden"

if !ctx.Doer.IsAdmin && ctx.Doer.LoginName != ctx.PathParam("collaborator") && !ctx.IsUserRepoAdmin() {
collaboratorUsername := ctx.PathParam("collaborator")
if !ctx.Doer.IsAdmin && ctx.Doer.LowerName != strings.ToLower(collaboratorUsername) && !ctx.IsUserRepoAdmin() {
ctx.APIError(http.StatusForbidden, "Only admins can query all permissions, repo admins can query all repo permissions, collaborators can query only their own")
return
}

collaborator, err := user_model.GetUserByName(ctx, ctx.PathParam("collaborator"))
collaborator, err := user_model.GetUserByName(ctx, collaboratorUsername)
if err != nil {
if user_model.IsErrUserNotExist(err) {
ctx.APIError(http.StatusNotFound, err)
Expand Down
188 changes: 100 additions & 88 deletions tests/integration/api_repo_collaborator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package integration

import (
"net/http"
"net/url"
"testing"

auth_model "code.gitea.io/gitea/models/auth"
Expand All @@ -14,132 +13,145 @@ import (
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/tests"

"github.com/stretchr/testify/assert"
)

func TestAPIRepoCollaboratorPermission(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) {
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
repo2Owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo2.OwnerID})
defer tests.PrepareTestEnv(t)()
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
repo2Owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo2.OwnerID})

user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
user10 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10})
user11 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 11})
user34 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 34})
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
user10 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10})
user11 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 11})
user34 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 34})

testCtx := NewAPITestContext(t, repo2Owner.Name, repo2.Name, auth_model.AccessTokenScopeWriteRepository)
testCtx := NewAPITestContext(t, repo2Owner.Name, repo2.Name, auth_model.AccessTokenScopeWriteRepository)

t.Run("RepoOwnerShouldBeOwner", func(t *testing.T) {
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, repo2Owner.Name).
AddTokenAuth(testCtx.Token)
resp := MakeRequest(t, req, http.StatusOK)
t.Run("RepoOwnerShouldBeOwner", func(t *testing.T) {
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, repo2Owner.Name).
AddTokenAuth(testCtx.Token)
resp := MakeRequest(t, req, http.StatusOK)

var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)
var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)

assert.Equal(t, "owner", repoPermission.Permission)
})
assert.Equal(t, "owner", repoPermission.Permission)
})

t.Run("CollaboratorWithReadAccess", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithReadAccess", doAPIAddCollaborator(testCtx, user4.Name, perm.AccessModeRead))
t.Run("CollaboratorWithReadAccess", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithReadAccess", doAPIAddCollaborator(testCtx, user4.Name, perm.AccessModeRead))

req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user4.Name).
AddTokenAuth(testCtx.Token)
resp := MakeRequest(t, req, http.StatusOK)
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user4.Name).
AddTokenAuth(testCtx.Token)
resp := MakeRequest(t, req, http.StatusOK)

var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)
var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)

assert.Equal(t, "read", repoPermission.Permission)
})
assert.Equal(t, "read", repoPermission.Permission)
})

t.Run("CollaboratorWithWriteAccess", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithWriteAccess", doAPIAddCollaborator(testCtx, user4.Name, perm.AccessModeWrite))
t.Run("CollaboratorWithWriteAccess", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithWriteAccess", doAPIAddCollaborator(testCtx, user4.Name, perm.AccessModeWrite))

req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user4.Name).
AddTokenAuth(testCtx.Token)
resp := MakeRequest(t, req, http.StatusOK)
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user4.Name).
AddTokenAuth(testCtx.Token)
resp := MakeRequest(t, req, http.StatusOK)

var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)
var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)

assert.Equal(t, "write", repoPermission.Permission)
})
assert.Equal(t, "write", repoPermission.Permission)
})

t.Run("CollaboratorWithAdminAccess", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithAdminAccess", doAPIAddCollaborator(testCtx, user4.Name, perm.AccessModeAdmin))
t.Run("CollaboratorWithAdminAccess", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithAdminAccess", doAPIAddCollaborator(testCtx, user4.Name, perm.AccessModeAdmin))

req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user4.Name).
AddTokenAuth(testCtx.Token)
resp := MakeRequest(t, req, http.StatusOK)
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user4.Name).
AddTokenAuth(testCtx.Token)
resp := MakeRequest(t, req, http.StatusOK)

var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)
var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)

assert.Equal(t, "admin", repoPermission.Permission)
})
assert.Equal(t, "admin", repoPermission.Permission)
})

t.Run("CollaboratorNotFound", func(t *testing.T) {
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, "non-existent-user").
AddTokenAuth(testCtx.Token)
MakeRequest(t, req, http.StatusNotFound)
})
t.Run("CollaboratorNotFound", func(t *testing.T) {
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, "non-existent-user").
AddTokenAuth(testCtx.Token)
MakeRequest(t, req, http.StatusNotFound)
})

t.Run("CollaboratorBlocked", func(t *testing.T) {
ctx := NewAPITestContext(t, repo2Owner.Name, repo2.Name, auth_model.AccessTokenScopeWriteRepository)
ctx.ExpectedCode = http.StatusForbidden
doAPIAddCollaborator(ctx, user34.Name, perm.AccessModeAdmin)(t)
})
t.Run("CollaboratorBlocked", func(t *testing.T) {
ctx := NewAPITestContext(t, repo2Owner.Name, repo2.Name, auth_model.AccessTokenScopeWriteRepository)
ctx.ExpectedCode = http.StatusForbidden
doAPIAddCollaborator(ctx, user34.Name, perm.AccessModeAdmin)(t)
})

t.Run("CollaboratorCanQueryItsPermissions", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithReadAccess", doAPIAddCollaborator(testCtx, user5.Name, perm.AccessModeRead))

_session := loginUser(t, user5.Name)
_testCtx := NewAPITestContext(t, user5.Name, repo2.Name, auth_model.AccessTokenScopeReadRepository)

req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user5.Name).
AddTokenAuth(_testCtx.Token)
resp := _session.MakeRequest(t, req, http.StatusOK)

t.Run("CollaboratorCanQueryItsPermissions", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithReadAccess", doAPIAddCollaborator(testCtx, user5.Name, perm.AccessModeRead))
var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)

_session := loginUser(t, user5.Name)
_testCtx := NewAPITestContext(t, user5.Name, repo2.Name, auth_model.AccessTokenScopeReadRepository)
assert.Equal(t, "read", repoPermission.Permission)

req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user5.Name).
AddTokenAuth(_testCtx.Token)
resp := _session.MakeRequest(t, req, http.StatusOK)
t.Run("CollaboratorCanReadOwnPermission", func(t *testing.T) {
session := loginUser(t, user5.Name)
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository)

var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)
req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user5.Name).AddTokenAuth(token)
resp = MakeRequest(t, req, http.StatusOK)

assert.Equal(t, "read", repoPermission.Permission)
repoCollPerm := api.RepoCollaboratorPermission{}
DecodeJSON(t, resp, &repoCollPerm)

assert.Equal(t, "read", repoCollPerm.Permission)
})
})

t.Run("CollaboratorCanQueryItsPermissions", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithReadAccess", doAPIAddCollaborator(testCtx, user5.Name, perm.AccessModeRead))
t.Run("CollaboratorCanQueryItsPermissions", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithReadAccess", doAPIAddCollaborator(testCtx, user5.Name, perm.AccessModeRead))

_session := loginUser(t, user5.Name)
_testCtx := NewAPITestContext(t, user5.Name, repo2.Name, auth_model.AccessTokenScopeReadRepository)
_session := loginUser(t, user5.Name)
_testCtx := NewAPITestContext(t, user5.Name, repo2.Name, auth_model.AccessTokenScopeReadRepository)

req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user5.Name).
AddTokenAuth(_testCtx.Token)
resp := _session.MakeRequest(t, req, http.StatusOK)
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user5.Name).
AddTokenAuth(_testCtx.Token)
resp := _session.MakeRequest(t, req, http.StatusOK)

var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)
var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)

assert.Equal(t, "read", repoPermission.Permission)
})
assert.Equal(t, "read", repoPermission.Permission)
})

t.Run("RepoAdminCanQueryACollaboratorsPermissions", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithAdminAccess", doAPIAddCollaborator(testCtx, user10.Name, perm.AccessModeAdmin))
t.Run("AddUserAsCollaboratorWithReadAccess", doAPIAddCollaborator(testCtx, user11.Name, perm.AccessModeRead))
t.Run("RepoAdminCanQueryACollaboratorsPermissions", func(t *testing.T) {
t.Run("AddUserAsCollaboratorWithAdminAccess", doAPIAddCollaborator(testCtx, user10.Name, perm.AccessModeAdmin))
t.Run("AddUserAsCollaboratorWithReadAccess", doAPIAddCollaborator(testCtx, user11.Name, perm.AccessModeRead))

_session := loginUser(t, user10.Name)
_testCtx := NewAPITestContext(t, user10.Name, repo2.Name, auth_model.AccessTokenScopeReadRepository)
_session := loginUser(t, user10.Name)
_testCtx := NewAPITestContext(t, user10.Name, repo2.Name, auth_model.AccessTokenScopeReadRepository)

req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user11.Name).
AddTokenAuth(_testCtx.Token)
resp := _session.MakeRequest(t, req, http.StatusOK)
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/collaborators/%s/permission", repo2Owner.Name, repo2.Name, user11.Name).
AddTokenAuth(_testCtx.Token)
resp := _session.MakeRequest(t, req, http.StatusOK)

var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)
var repoPermission api.RepoCollaboratorPermission
DecodeJSON(t, resp, &repoPermission)

assert.Equal(t, "read", repoPermission.Permission)
})
assert.Equal(t, "read", repoPermission.Permission)
})
}

0 comments on commit 8ae46d9

Please sign in to comment.