From 1f5237e0d7214294aceaa4487a98c88d183a243c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 13 Dec 2025 18:14:18 -0800 Subject: [PATCH] Check user visibility when redirecting to a renamed user (#36148) Fix #34169 --- routers/api/v1/api.go | 4 +- routers/api/v1/user/helper.go | 2 +- services/context/context_response.go | 16 ++++++- services/context/org.go | 2 +- services/context/repo.go | 2 +- services/context/user.go | 2 +- tests/integration/user_test.go | 72 ++++++++++++++++++++++++++++ 7 files changed, 92 insertions(+), 8 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 9bce98ac02..fcf9e73057 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -152,7 +152,7 @@ func repoAssignment() func(ctx *context.APIContext) { if err != nil { if user_model.IsErrUserNotExist(err) { if redirectUserID, err := user_model.LookupUserRedirect(ctx, userName); err == nil { - context.RedirectToUser(ctx.Base, userName, redirectUserID) + context.RedirectToUser(ctx.Base, ctx.Doer, userName, redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { ctx.APIErrorNotFound("GetUserByName", err) } else { @@ -612,7 +612,7 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) { if organization.IsErrOrgNotExist(err) { redirectUserID, err := user_model.LookupUserRedirect(ctx, ctx.PathParam("org")) if err == nil { - context.RedirectToUser(ctx.Base, ctx.PathParam("org"), redirectUserID) + context.RedirectToUser(ctx.Base, ctx.Doer, ctx.PathParam("org"), redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { ctx.APIErrorNotFound("GetOrgByName", err) } else { diff --git a/routers/api/v1/user/helper.go b/routers/api/v1/user/helper.go index f49bbbd6db..de3ec089df 100644 --- a/routers/api/v1/user/helper.go +++ b/routers/api/v1/user/helper.go @@ -16,7 +16,7 @@ func GetUserByPathParam(ctx *context.APIContext, name string) *user_model.User { if err != nil { if user_model.IsErrUserNotExist(err) { if redirectUserID, err2 := user_model.LookupUserRedirect(ctx, username); err2 == nil { - context.RedirectToUser(ctx.Base, username, redirectUserID) + context.RedirectToUser(ctx.Base, ctx.Doer, username, redirectUserID) } else { ctx.APIErrorNotFound("GetUserByName", err) } diff --git a/services/context/context_response.go b/services/context/context_response.go index 3f64fc7352..bb896024b1 100644 --- a/services/context/context_response.go +++ b/services/context/context_response.go @@ -20,15 +20,27 @@ import ( "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/web/middleware" ) // RedirectToUser redirect to a differently-named user -func RedirectToUser(ctx *Base, userName string, redirectUserID int64) { +func RedirectToUser(ctx *Base, doer *user_model.User, userName string, redirectUserID int64) { user, err := user_model.GetUserByID(ctx, redirectUserID) if err != nil { - ctx.HTTPError(http.StatusInternalServerError, "unable to get user") + if user_model.IsErrUserNotExist(err) { + ctx.HTTPError(http.StatusNotFound, "user does not exist") + } else { + ctx.HTTPError(http.StatusInternalServerError, "unable to get user") + } + return + } + + // Handle Visibility + if user.Visibility != structs.VisibleTypePublic && doer == nil { + // We must be signed in to see limited or private organizations + ctx.HTTPError(http.StatusNotFound, "user does not exist") return } diff --git a/services/context/org.go b/services/context/org.go index 1cd8923178..d41bd5ea79 100644 --- a/services/context/org.go +++ b/services/context/org.go @@ -49,7 +49,7 @@ func GetOrganizationByParams(ctx *Context) { if organization.IsErrOrgNotExist(err) { redirectUserID, err := user_model.LookupUserRedirect(ctx, orgName) if err == nil { - RedirectToUser(ctx.Base, orgName, redirectUserID) + RedirectToUser(ctx.Base, ctx.Doer, orgName, redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { ctx.NotFound(err) } else { diff --git a/services/context/repo.go b/services/context/repo.go index 64b8695236..5a313e6f15 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -443,7 +443,7 @@ func RepoAssignment(ctx *Context) { } if redirectUserID, err := user_model.LookupUserRedirect(ctx, userName); err == nil { - RedirectToUser(ctx.Base, userName, redirectUserID) + RedirectToUser(ctx.Base, ctx.Doer, userName, redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { ctx.NotFound(nil) } else { diff --git a/services/context/user.go b/services/context/user.go index f1a3035ee9..19c055e2a3 100644 --- a/services/context/user.go +++ b/services/context/user.go @@ -69,7 +69,7 @@ func userAssignment(ctx *Base, doer *user_model.User, errCb func(int, any)) (con if err != nil { if user_model.IsErrUserNotExist(err) { if redirectUserID, err := user_model.LookupUserRedirect(ctx, username); err == nil { - RedirectToUser(ctx, username, redirectUserID) + RedirectToUser(ctx, doer, username, redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { errCb(http.StatusNotFound, err) } else { diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 34692d9cab..54b372dd16 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -45,6 +45,78 @@ func TestRenameUsername(t *testing.T) { unittest.AssertNotExistsBean(t, &user_model.User{Name: "user2"}) } +func TestViewLimitedAndPrivateUserAndRename(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // user 22 is a limited visibility org + org22 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 22}) + req := NewRequest(t, "GET", "/"+org22.Name) + MakeRequest(t, req, http.StatusNotFound) + + session := loginUser(t, "user1") + oldName := org22.Name + newName := "org22_renamed" + req = NewRequestWithValues(t, "POST", "/org/"+oldName+"/settings/rename", map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + "org_name": oldName, + "new_org_name": newName, + }) + session.MakeRequest(t, req, http.StatusOK) + + unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: newName}) + unittest.AssertNotExistsBean(t, &user_model.User{Name: oldName}) + + req = NewRequest(t, "GET", "/"+oldName) + MakeRequest(t, req, http.StatusNotFound) // anonymous user cannot visit limited visibility org via old name + req = NewRequest(t, "GET", "/"+oldName) + session.MakeRequest(t, req, http.StatusTemporaryRedirect) // login user can visit limited visibility org via old name + + // org 23 is a private visibility org + org23 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 23}) + req = NewRequest(t, "GET", "/"+org23.Name) + MakeRequest(t, req, http.StatusNotFound) + + oldName = org23.Name + newName = "org23_renamed" + req = NewRequestWithValues(t, "POST", "/org/"+oldName+"/settings/rename", map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + "org_name": oldName, + "new_org_name": newName, + }) + session.MakeRequest(t, req, http.StatusOK) + + unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: newName}) + unittest.AssertNotExistsBean(t, &user_model.User{Name: oldName}) + + req = NewRequest(t, "GET", "/"+oldName) + MakeRequest(t, req, http.StatusNotFound) // anonymous user cannot visit limited visibility org via old name + req = NewRequest(t, "GET", "/"+oldName) + session.MakeRequest(t, req, http.StatusTemporaryRedirect) // login user can visit limited visibility org via old name + + // user 31 is a private visibility user + user31 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 31}) + req = NewRequest(t, "GET", "/"+user31.Name) + MakeRequest(t, req, http.StatusNotFound) + + oldName = user31.Name + newName = "user31_renamed" + session2 := loginUser(t, oldName) + req = NewRequestWithValues(t, "POST", "/user/settings", map[string]string{ + "_csrf": GetUserCSRFToken(t, session2), + "name": newName, + "visibility": "2", // private + }) + session2.MakeRequest(t, req, http.StatusSeeOther) + + unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: newName}) + unittest.AssertNotExistsBean(t, &user_model.User{Name: oldName}) + + req = NewRequest(t, "GET", "/"+oldName) + MakeRequest(t, req, http.StatusNotFound) // anonymous user cannot visit private visibility user via old name + req = NewRequest(t, "GET", "/"+oldName) + session.MakeRequest(t, req, http.StatusTemporaryRedirect) // login user2 can visit private visibility user via old name +} + func TestRenameInvalidUsername(t *testing.T) { defer tests.PrepareTestEnv(t)()