diff --git a/models/fixtures/repo_redirect.yml b/models/fixtures/repo_redirect.yml index 8850c8d780..82d365c600 100644 --- a/models/fixtures/repo_redirect.yml +++ b/models/fixtures/repo_redirect.yml @@ -3,3 +3,9 @@ owner_id: 2 lower_name: oldrepo1 redirect_repo_id: 1 + +- + id: 2 + owner_id: 17 + lower_name: oldrepo24 + redirect_repo_id: 24 diff --git a/models/fixtures/user_redirect.yml b/models/fixtures/user_redirect.yml index f471e94511..2f7a523c0c 100644 --- a/models/fixtures/user_redirect.yml +++ b/models/fixtures/user_redirect.yml @@ -3,3 +3,15 @@ lower_name: olduser1 redirect_user_id: 1 created_unix: 1730000000 + +- + id: 2 + lower_name: oldorg22 + redirect_user_id: 22 + created_unix: 1730000000 + +- + id: 3 + lower_name: oldorg23 + redirect_user_id: 23 + created_unix: 1730000000 diff --git a/models/repo/redirect.go b/models/repo/redirect.go index 9c44a255d0..e5239a3684 100644 --- a/models/repo/redirect.go +++ b/models/repo/redirect.go @@ -14,8 +14,9 @@ import ( // ErrRedirectNotExist represents a "RedirectNotExist" kind of error. type ErrRedirectNotExist struct { - OwnerID int64 - RepoName string + OwnerID int64 + RepoName string + MissingPermission bool } // IsErrRedirectNotExist check if an error is an ErrRepoRedirectNotExist. @@ -49,8 +50,8 @@ func init() { db.RegisterModel(new(Redirect)) } -// LookupRedirect look up if a repository has a redirect name -func LookupRedirect(ctx context.Context, ownerID int64, repoName string) (int64, error) { +// GetRedirect returns the redirect for a given pair of ownerID and repository name. +func GetRedirect(ctx context.Context, ownerID int64, repoName string) (int64, error) { repoName = strings.ToLower(repoName) redirect := &Redirect{OwnerID: ownerID, LowerName: repoName} if has, err := db.GetEngine(ctx).Get(redirect); err != nil { diff --git a/models/repo/redirect_test.go b/models/repo/redirect_test.go index d84cbbed54..2f2210588f 100644 --- a/models/repo/redirect_test.go +++ b/models/repo/redirect_test.go @@ -10,21 +10,9 @@ import ( repo_model "forgejo.org/models/repo" "forgejo.org/models/unittest" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestLookupRedirect(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - repoID, err := repo_model.LookupRedirect(db.DefaultContext, 2, "oldrepo1") - require.NoError(t, err) - assert.EqualValues(t, 1, repoID) - - _, err = repo_model.LookupRedirect(db.DefaultContext, unittest.NonexistentID, "doesnotexist") - assert.True(t, repo_model.IsErrRedirectNotExist(err)) -} - func TestNewRedirect(t *testing.T) { // redirect to a completely new name require.NoError(t, unittest.PrepareTestDatabase()) diff --git a/models/user/redirect.go b/models/user/redirect.go index 75876f17d2..bcb421d4a1 100644 --- a/models/user/redirect.go +++ b/models/user/redirect.go @@ -21,7 +21,8 @@ import ( // ErrUserRedirectNotExist represents a "UserRedirectNotExist" kind of error. type ErrUserRedirectNotExist struct { - Name string + Name string + MissingPermission bool } // IsErrUserRedirectNotExist check if an error is an ErrUserRedirectNotExist. @@ -81,15 +82,6 @@ func GetUserRedirect(ctx context.Context, userName string) (*Redirect, error) { return redirect, nil } -// LookupUserRedirect look up userID if a user has a redirect name -func LookupUserRedirect(ctx context.Context, userName string) (int64, error) { - redirect, err := GetUserRedirect(ctx, userName) - if err != nil { - return 0, err - } - return redirect.RedirectUserID, nil -} - // NewUserRedirect create a new user redirect func NewUserRedirect(ctx context.Context, ID int64, oldUserName, newUserName string) error { oldUserName = strings.ToLower(oldUserName) diff --git a/models/user/redirect_test.go b/models/user/redirect_test.go deleted file mode 100644 index c598fb045f..0000000000 --- a/models/user/redirect_test.go +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package user_test - -import ( - "testing" - - "forgejo.org/models/db" - "forgejo.org/models/unittest" - user_model "forgejo.org/models/user" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestLookupUserRedirect(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - userID, err := user_model.LookupUserRedirect(db.DefaultContext, "olduser1") - require.NoError(t, err) - assert.EqualValues(t, 1, userID) - - _, err = user_model.LookupUserRedirect(db.DefaultContext, "doesnotexist") - assert.True(t, user_model.IsErrUserRedirectNotExist(err)) -} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 307ed38882..a6d8ff58e7 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -99,6 +99,7 @@ import ( "forgejo.org/services/auth" "forgejo.org/services/context" "forgejo.org/services/forms" + redirect_service "forgejo.org/services/redirect" _ "forgejo.org/routers/api/v1/swagger" // for swagger generation @@ -153,12 +154,12 @@ func repoAssignment() func(ctx *context.APIContext) { owner, err = user_model.GetUserByName(ctx, userName) if err != nil { if user_model.IsErrUserNotExist(err) { - if redirectUserID, err := user_model.LookupUserRedirect(ctx, userName); err == nil { + if redirectUserID, err := redirect_service.LookupUserRedirect(ctx, ctx.Doer, userName); err == nil { context.RedirectToUser(ctx.Base, userName, redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { ctx.NotFound("GetUserByName", err) } else { - ctx.Error(http.StatusInternalServerError, "LookupUserRedirect", err) + ctx.Error(http.StatusInternalServerError, "LookupRedirect", err) } } else { ctx.Error(http.StatusInternalServerError, "GetUserByName", err) @@ -173,7 +174,7 @@ func repoAssignment() func(ctx *context.APIContext) { repo, err := repo_model.GetRepositoryByName(ctx, owner.ID, repoName) if err != nil { if repo_model.IsErrRepoNotExist(err) { - redirectRepoID, err := repo_model.LookupRedirect(ctx, owner.ID, repoName) + redirectRepoID, err := redirect_service.LookupRepoRedirect(ctx, ctx.Doer, owner.ID, repoName) if err == nil { context.RedirectToRepo(ctx.Base, redirectRepoID) } else if repo_model.IsErrRedirectNotExist(err) { @@ -641,13 +642,13 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) { ctx.Org.Organization, err = organization.GetOrgByName(ctx, ctx.Params(":org")) if err != nil { if organization.IsErrOrgNotExist(err) { - redirectUserID, err := user_model.LookupUserRedirect(ctx, ctx.Params(":org")) + redirectUserID, err := redirect_service.LookupUserRedirect(ctx, ctx.Doer, ctx.Params(":org")) if err == nil { context.RedirectToUser(ctx.Base, ctx.Params(":org"), redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { ctx.NotFound("GetOrgByName", err) } else { - ctx.Error(http.StatusInternalServerError, "LookupUserRedirect", err) + ctx.Error(http.StatusInternalServerError, "LookupRedirect", err) } } else { ctx.Error(http.StatusInternalServerError, "GetOrgByName", err) diff --git a/routers/api/v1/user/helper.go b/routers/api/v1/user/helper.go index fe0943091f..1de4d72161 100644 --- a/routers/api/v1/user/helper.go +++ b/routers/api/v1/user/helper.go @@ -8,6 +8,7 @@ import ( user_model "forgejo.org/models/user" "forgejo.org/services/context" + redirect_service "forgejo.org/services/redirect" ) // GetUserByParamsName get user by name @@ -16,7 +17,7 @@ func GetUserByParamsName(ctx *context.APIContext, name string) *user_model.User user, err := user_model.GetUserByName(ctx, username) if err != nil { if user_model.IsErrUserNotExist(err) { - if redirectUserID, err2 := user_model.LookupUserRedirect(ctx, username); err2 == nil { + if redirectUserID, err2 := redirect_service.LookupUserRedirect(ctx, ctx.Doer, username); err2 == nil { context.RedirectToUser(ctx.Base, username, redirectUserID) } else { ctx.NotFound("GetUserByName", err) diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index a60c213113..403245596d 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -31,6 +31,7 @@ import ( "forgejo.org/modules/structs" "forgejo.org/modules/util" "forgejo.org/services/context" + redirect_service "forgejo.org/services/redirect" repo_service "forgejo.org/services/repository" "github.com/go-chi/cors" @@ -111,7 +112,7 @@ func httpBase(ctx *context.Context) *serviceHandler { return nil } - if redirectRepoID, err := repo_model.LookupRedirect(ctx, owner.ID, reponame); err == nil { + if redirectRepoID, err := redirect_service.LookupRepoRedirect(ctx, ctx.Doer, owner.ID, reponame); err == nil { context.RedirectToRepo(ctx.Base, redirectRepoID) return nil } diff --git a/services/context/org.go b/services/context/org.go index 3ddc40b6b3..c7d06b9bcc 100644 --- a/services/context/org.go +++ b/services/context/org.go @@ -16,6 +16,7 @@ import ( "forgejo.org/modules/markup/markdown" "forgejo.org/modules/setting" "forgejo.org/modules/structs" + redirect_service "forgejo.org/services/redirect" ) // Organization contains organization context @@ -48,13 +49,13 @@ func GetOrganizationByParams(ctx *Context) { ctx.Org.Organization, err = organization.GetOrgByName(ctx, orgName) if err != nil { if organization.IsErrOrgNotExist(err) { - redirectUserID, err := user_model.LookupUserRedirect(ctx, orgName) + redirectUserID, err := redirect_service.LookupUserRedirect(ctx, ctx.Doer, orgName) if err == nil { RedirectToUser(ctx.Base, orgName, redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { ctx.NotFound("GetUserByName", err) } else { - ctx.ServerError("LookupUserRedirect", err) + ctx.ServerError("LookupRedirect", err) } } else { ctx.ServerError("GetUserByName", err) diff --git a/services/context/repo.go b/services/context/repo.go index c8876d7166..e01e1ddafc 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -35,6 +35,7 @@ import ( "forgejo.org/modules/setting" "forgejo.org/modules/util" asymkey_service "forgejo.org/services/asymkey" + redirect_service "forgejo.org/services/redirect" "github.com/editorconfig/editorconfig-core-go/v2" ) @@ -477,12 +478,12 @@ func RepoAssignment(ctx *Context) context.CancelFunc { return nil } - if redirectUserID, err := user_model.LookupUserRedirect(ctx, userName); err == nil { + if redirectUserID, err := redirect_service.LookupUserRedirect(ctx, ctx.Doer, userName); err == nil { RedirectToUser(ctx.Base, userName, redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { ctx.NotFound("GetUserByName", nil) } else { - ctx.ServerError("LookupUserRedirect", err) + ctx.ServerError("LookupRedirect", err) } } else { ctx.ServerError("GetUserByName", err) @@ -519,7 +520,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { repo, err := repo_model.GetRepositoryByName(ctx, owner.ID, repoName) if err != nil { if repo_model.IsErrRepoNotExist(err) { - redirectRepoID, err := repo_model.LookupRedirect(ctx, owner.ID, repoName) + redirectRepoID, err := redirect_service.LookupRepoRedirect(ctx, ctx.Doer, owner.ID, repoName) if err == nil { RedirectToRepo(ctx.Base, redirectRepoID) } else if repo_model.IsErrRedirectNotExist(err) { diff --git a/services/context/user.go b/services/context/user.go index a82c90d7a6..63260a49aa 100644 --- a/services/context/user.go +++ b/services/context/user.go @@ -9,6 +9,7 @@ import ( "strings" user_model "forgejo.org/models/user" + redirect_service "forgejo.org/services/redirect" ) // UserAssignmentWeb returns a middleware to handle context-user assignment for web routes @@ -68,12 +69,12 @@ func userAssignment(ctx *Base, doer *user_model.User, errCb func(int, string, an contextUser, err = user_model.GetUserByName(ctx, username) if err != nil { if user_model.IsErrUserNotExist(err) { - if redirectUserID, err := user_model.LookupUserRedirect(ctx, username); err == nil { + if redirectUserID, err := redirect_service.LookupUserRedirect(ctx, doer, username); err == nil { RedirectToUser(ctx, username, redirectUserID) } else if user_model.IsErrUserRedirectNotExist(err) { errCb(http.StatusNotFound, "GetUserByName", err) } else { - errCb(http.StatusInternalServerError, "LookupUserRedirect", err) + errCb(http.StatusInternalServerError, "LookupRedirect", err) } } else { errCb(http.StatusInternalServerError, "GetUserByName", err) diff --git a/services/redirect/main_test.go b/services/redirect/main_test.go new file mode 100644 index 0000000000..7363791caa --- /dev/null +++ b/services/redirect/main_test.go @@ -0,0 +1,18 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT +package redirect + +import ( + "testing" + + "forgejo.org/models/unittest" + + _ "forgejo.org/models" + _ "forgejo.org/models/actions" + _ "forgejo.org/models/activities" + _ "forgejo.org/models/forgefed" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m) +} diff --git a/services/redirect/repo.go b/services/redirect/repo.go new file mode 100644 index 0000000000..7070ab4e2f --- /dev/null +++ b/services/redirect/repo.go @@ -0,0 +1,37 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT +package redirect + +import ( + "context" + + access_model "forgejo.org/models/perm/access" + repo_model "forgejo.org/models/repo" + user_model "forgejo.org/models/user" +) + +// LookupRepoRedirect returns the repository ID if there's a redirect registered for +// the ownerID repository name pair. It checks if the doer has permission to view +// the new repository. +func LookupRepoRedirect(ctx context.Context, doer *user_model.User, ownerID int64, repoName string) (int64, error) { + redirectID, err := repo_model.GetRedirect(ctx, ownerID, repoName) + if err != nil { + return 0, err + } + + redirectRepo, err := repo_model.GetRepositoryByID(ctx, redirectID) + if err != nil { + return 0, err + } + + perm, err := access_model.GetUserRepoPermission(ctx, redirectRepo, doer) + if err != nil { + return 0, err + } + + if !perm.HasAccess() { + return 0, repo_model.ErrRedirectNotExist{OwnerID: ownerID, RepoName: repoName, MissingPermission: true} + } + + return redirectID, nil +} diff --git a/services/redirect/repo_test.go b/services/redirect/repo_test.go new file mode 100644 index 0000000000..cad8414035 --- /dev/null +++ b/services/redirect/repo_test.go @@ -0,0 +1,55 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT +package redirect + +import ( + "testing" + + repo_model "forgejo.org/models/repo" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLookupRepoRedirect(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + normalUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + ownerUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 20}) + + testOk := func(t *testing.T, doer *user_model.User, ownerID int64, repoName string, expectedRedirectID int64) { + t.Helper() + + redirectID, err := LookupRepoRedirect(t.Context(), doer, ownerID, repoName) + require.NoError(t, err) + assert.Equal(t, expectedRedirectID, redirectID) + } + + testFail := func(t *testing.T, doer *user_model.User, ownerID int64, repoName string) { + t.Helper() + + redirectID, err := LookupRepoRedirect(t.Context(), doer, ownerID, repoName) + require.ErrorIs(t, err, repo_model.ErrRedirectNotExist{OwnerID: ownerID, RepoName: repoName, MissingPermission: true}) + assert.Zero(t, redirectID) + } + + t.Run("Public repository", func(t *testing.T) { + ownerID := int64(2) + reponame := "oldrepo1" + + testOk(t, nil, ownerID, reponame, 1) + testOk(t, normalUser, ownerID, reponame, 1) + testOk(t, ownerUser, ownerID, reponame, 1) + }) + + t.Run("Private repository", func(t *testing.T) { + ownerID := int64(17) + reponame := "oldrepo24" + + testFail(t, nil, ownerID, reponame) + testFail(t, normalUser, ownerID, reponame) + testOk(t, ownerUser, ownerID, reponame, 24) + }) +} diff --git a/services/redirect/user.go b/services/redirect/user.go new file mode 100644 index 0000000000..2b1d38bbc0 --- /dev/null +++ b/services/redirect/user.go @@ -0,0 +1,30 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT +package redirect + +import ( + "context" + + user_model "forgejo.org/models/user" +) + +// LookupUserRedirect returns the userID if there's a redirect registered for the +// username. It additionally checks if the doer has permission to view the new +// user. +func LookupUserRedirect(ctx context.Context, doer *user_model.User, userName string) (int64, error) { + redirect, err := user_model.GetUserRedirect(ctx, userName) + if err != nil { + return 0, err + } + + redirectUser, err := user_model.GetUserByID(ctx, redirect.RedirectUserID) + if err != nil { + return 0, err + } + + if !user_model.IsUserVisibleToViewer(ctx, redirectUser, doer) { + return 0, user_model.ErrUserRedirectNotExist{Name: userName, MissingPermission: true} + } + + return redirect.RedirectUserID, nil +} diff --git a/services/redirect/user_test.go b/services/redirect/user_test.go new file mode 100644 index 0000000000..d1ddcc2ebf --- /dev/null +++ b/services/redirect/user_test.go @@ -0,0 +1,65 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT +package redirect + +import ( + "testing" + + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLookupUserRedirect(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + normalUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + testOk := func(t *testing.T, doer *user_model.User, username string, expectedRedirectID int64) { + t.Helper() + + redirectID, err := LookupUserRedirect(t.Context(), doer, username) + require.NoError(t, err) + assert.Equal(t, expectedRedirectID, redirectID) + } + + testFail := func(t *testing.T, doer *user_model.User, username string) { + t.Helper() + + redirectID, err := LookupUserRedirect(t.Context(), doer, username) + require.ErrorIs(t, err, user_model.ErrUserRedirectNotExist{Name: username, MissingPermission: true}) + assert.Zero(t, redirectID) + } + + t.Run("Public visibility", func(t *testing.T) { + username := "olduser1" + redirectID := int64(1) + + testOk(t, nil, username, redirectID) + testOk(t, normalUser, username, redirectID) + testOk(t, adminUser, username, redirectID) + }) + + t.Run("Limited visibility", func(t *testing.T) { + username := "oldorg22" + redirectID := int64(22) + + testFail(t, nil, username) + testOk(t, normalUser, username, redirectID) + testOk(t, adminUser, username, redirectID) + }) + + t.Run("Private visibility", func(t *testing.T) { + orgUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + username := "oldorg23" + redirectID := int64(23) + + testFail(t, nil, username) + testFail(t, normalUser, username) + testOk(t, orgUser, username, redirectID) + testOk(t, adminUser, username, redirectID) + }) +} diff --git a/services/user/user_test.go b/services/user/user_test.go index 0747833557..7240813411 100644 --- a/services/user/user_test.go +++ b/services/user/user_test.go @@ -26,6 +26,7 @@ import ( "forgejo.org/modules/test" "forgejo.org/modules/timeutil" "forgejo.org/services/auth/source/oauth2" + redirect_service "forgejo.org/services/redirect" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -184,7 +185,7 @@ func TestRenameUser(t *testing.T) { require.NoError(t, RenameUser(db.DefaultContext, user, newUsername)) unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID, Name: newUsername, LowerName: strings.ToLower(newUsername)}) - redirectUID, err := user_model.LookupUserRedirect(db.DefaultContext, oldUsername) + redirectUID, err := redirect_service.LookupUserRedirect(db.DefaultContext, user, oldUsername) require.NoError(t, err) assert.Equal(t, user.ID, redirectUID)