Skip to content

Commit 1f5237e

Browse files
lunnylunny
authored
Check user visibility when redirecting to a renamed user (#36148)
Fix #34169
1 parent 29057ea commit 1f5237e

File tree

7 files changed

+92
-8
lines changed

7 files changed

+92
-8
lines changed

‎routers/api/v1/api.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func repoAssignment() func(ctx *context.APIContext) {
152152
if err != nil {
153153
if user_model.IsErrUserNotExist(err) {
154154
if redirectUserID, err := user_model.LookupUserRedirect(ctx, userName); err == nil {
155-
context.RedirectToUser(ctx.Base, userName, redirectUserID)
155+
context.RedirectToUser(ctx.Base, ctx.Doer, userName, redirectUserID)
156156
} else if user_model.IsErrUserRedirectNotExist(err) {
157157
ctx.APIErrorNotFound("GetUserByName", err)
158158
} else {
@@ -612,7 +612,7 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) {
612612
if organization.IsErrOrgNotExist(err) {
613613
redirectUserID, err := user_model.LookupUserRedirect(ctx, ctx.PathParam("org"))
614614
if err == nil {
615-
context.RedirectToUser(ctx.Base, ctx.PathParam("org"), redirectUserID)
615+
context.RedirectToUser(ctx.Base, ctx.Doer, ctx.PathParam("org"), redirectUserID)
616616
} else if user_model.IsErrUserRedirectNotExist(err) {
617617
ctx.APIErrorNotFound("GetOrgByName", err)
618618
} else {

‎routers/api/v1/user/helper.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func GetUserByPathParam(ctx *context.APIContext, name string) *user_model.User {
1616
if err != nil {
1717
if user_model.IsErrUserNotExist(err) {
1818
if redirectUserID, err2 := user_model.LookupUserRedirect(ctx, username); err2 == nil {
19-
context.RedirectToUser(ctx.Base, username, redirectUserID)
19+
context.RedirectToUser(ctx.Base, ctx.Doer, username, redirectUserID)
2020
} else {
2121
ctx.APIErrorNotFound("GetUserByName", err)
2222
}

‎services/context/context_response.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,27 @@ import (
2020
"code.gitea.io/gitea/modules/httplib"
2121
"code.gitea.io/gitea/modules/log"
2222
"code.gitea.io/gitea/modules/setting"
23+
"code.gitea.io/gitea/modules/structs"
2324
"code.gitea.io/gitea/modules/templates"
2425
"code.gitea.io/gitea/modules/web/middleware"
2526
)
2627

2728
// RedirectToUser redirect to a differently-named user
28-
func RedirectToUser(ctx *Base, userName string, redirectUserID int64) {
29+
func RedirectToUser(ctx *Base, doer *user_model.User, userName string, redirectUserID int64) {
2930
user, err := user_model.GetUserByID(ctx, redirectUserID)
3031
if err != nil {
31-
ctx.HTTPError(http.StatusInternalServerError, "unable to get user")
32+
if user_model.IsErrUserNotExist(err) {
33+
ctx.HTTPError(http.StatusNotFound, "user does not exist")
34+
} else {
35+
ctx.HTTPError(http.StatusInternalServerError, "unable to get user")
36+
}
37+
return
38+
}
39+
40+
// Handle Visibility
41+
if user.Visibility != structs.VisibleTypePublic && doer == nil {
42+
// We must be signed in to see limited or private organizations
43+
ctx.HTTPError(http.StatusNotFound, "user does not exist")
3244
return
3345
}
3446

‎services/context/org.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func GetOrganizationByParams(ctx *Context) {
4949
if organization.IsErrOrgNotExist(err) {
5050
redirectUserID, err := user_model.LookupUserRedirect(ctx, orgName)
5151
if err == nil {
52-
RedirectToUser(ctx.Base, orgName, redirectUserID)
52+
RedirectToUser(ctx.Base, ctx.Doer, orgName, redirectUserID)
5353
} else if user_model.IsErrUserRedirectNotExist(err) {
5454
ctx.NotFound(err)
5555
} else {

‎services/context/repo.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ func RepoAssignment(ctx *Context) {
443443
}
444444

445445
if redirectUserID, err := user_model.LookupUserRedirect(ctx, userName); err == nil {
446-
RedirectToUser(ctx.Base, userName, redirectUserID)
446+
RedirectToUser(ctx.Base, ctx.Doer, userName, redirectUserID)
447447
} else if user_model.IsErrUserRedirectNotExist(err) {
448448
ctx.NotFound(nil)
449449
} else {

‎services/context/user.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func userAssignment(ctx *Base, doer *user_model.User, errCb func(int, any)) (con
6969
if err != nil {
7070
if user_model.IsErrUserNotExist(err) {
7171
if redirectUserID, err := user_model.LookupUserRedirect(ctx, username); err == nil {
72-
RedirectToUser(ctx, username, redirectUserID)
72+
RedirectToUser(ctx, doer, username, redirectUserID)
7373
} else if user_model.IsErrUserRedirectNotExist(err) {
7474
errCb(http.StatusNotFound, err)
7575
} else {

‎tests/integration/user_test.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,78 @@ func TestRenameUsername(t *testing.T) {
4545
unittest.AssertNotExistsBean(t, &user_model.User{Name: "user2"})
4646
}
4747

48+
func TestViewLimitedAndPrivateUserAndRename(t *testing.T) {
49+
defer tests.PrepareTestEnv(t)()
50+
51+
// user 22 is a limited visibility org
52+
org22 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 22})
53+
req := NewRequest(t, "GET", "/"+org22.Name)
54+
MakeRequest(t, req, http.StatusNotFound)
55+
56+
session := loginUser(t, "user1")
57+
oldName := org22.Name
58+
newName := "org22_renamed"
59+
req = NewRequestWithValues(t, "POST", "/org/"+oldName+"/settings/rename", map[string]string{
60+
"_csrf": GetUserCSRFToken(t, session),
61+
"org_name": oldName,
62+
"new_org_name": newName,
63+
})
64+
session.MakeRequest(t, req, http.StatusOK)
65+
66+
unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: newName})
67+
unittest.AssertNotExistsBean(t, &user_model.User{Name: oldName})
68+
69+
req = NewRequest(t, "GET", "/"+oldName)
70+
MakeRequest(t, req, http.StatusNotFound) // anonymous user cannot visit limited visibility org via old name
71+
req = NewRequest(t, "GET", "/"+oldName)
72+
session.MakeRequest(t, req, http.StatusTemporaryRedirect) // login user can visit limited visibility org via old name
73+
74+
// org 23 is a private visibility org
75+
org23 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 23})
76+
req = NewRequest(t, "GET", "/"+org23.Name)
77+
MakeRequest(t, req, http.StatusNotFound)
78+
79+
oldName = org23.Name
80+
newName = "org23_renamed"
81+
req = NewRequestWithValues(t, "POST", "/org/"+oldName+"/settings/rename", map[string]string{
82+
"_csrf": GetUserCSRFToken(t, session),
83+
"org_name": oldName,
84+
"new_org_name": newName,
85+
})
86+
session.MakeRequest(t, req, http.StatusOK)
87+
88+
unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: newName})
89+
unittest.AssertNotExistsBean(t, &user_model.User{Name: oldName})
90+
91+
req = NewRequest(t, "GET", "/"+oldName)
92+
MakeRequest(t, req, http.StatusNotFound) // anonymous user cannot visit limited visibility org via old name
93+
req = NewRequest(t, "GET", "/"+oldName)
94+
session.MakeRequest(t, req, http.StatusTemporaryRedirect) // login user can visit limited visibility org via old name
95+
96+
// user 31 is a private visibility user
97+
user31 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 31})
98+
req = NewRequest(t, "GET", "/"+user31.Name)
99+
MakeRequest(t, req, http.StatusNotFound)
100+
101+
oldName = user31.Name
102+
newName = "user31_renamed"
103+
session2 := loginUser(t, oldName)
104+
req = NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
105+
"_csrf": GetUserCSRFToken(t, session2),
106+
"name": newName,
107+
"visibility": "2", // private
108+
})
109+
session2.MakeRequest(t, req, http.StatusSeeOther)
110+
111+
unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: newName})
112+
unittest.AssertNotExistsBean(t, &user_model.User{Name: oldName})
113+
114+
req = NewRequest(t, "GET", "/"+oldName)
115+
MakeRequest(t, req, http.StatusNotFound) // anonymous user cannot visit private visibility user via old name
116+
req = NewRequest(t, "GET", "/"+oldName)
117+
session.MakeRequest(t, req, http.StatusTemporaryRedirect) // login user2 can visit private visibility user via old name
118+
}
119+
48120
func TestRenameInvalidUsername(t *testing.T) {
49121
defer tests.PrepareTestEnv(t)()
50122

0 commit comments

Comments
 (0)