diff --git a/routers/private/serv.go b/routers/private/serv.go index 3dfe4d21da7a5..b752556c238dd 100644 --- a/routers/private/serv.go +++ b/routers/private/serv.go @@ -108,21 +108,19 @@ func ServCommand(ctx *context.PrivateContext) { results.RepoName = repoName[:len(repoName)-5] } - // Check if there is a user redirect for the requested owner - redirectedUserID, err := user_model.LookupUserRedirect(ctx, results.OwnerName) - if err == nil { - owner, err := user_model.GetUserByID(ctx, redirectedUserID) - if err == nil { - log.Info("User %s has been redirected to %s", results.OwnerName, owner.Name) - results.OwnerName = owner.Name - } else { - log.Warn("User %s has a redirect to user with ID %d, but no user with this ID could be found. Trying without redirect...", results.OwnerName, redirectedUserID) - } - } - owner, err := user_model.GetUserByName(ctx, results.OwnerName) if err != nil { - if user_model.IsErrUserNotExist(err) { + if !user_model.IsErrUserNotExist(err) { + log.Error("Unable to get repository owner: %s/%s Error: %v", results.OwnerName, results.RepoName, err) + ctx.JSON(http.StatusForbidden, private.Response{ + UserMsg: fmt.Sprintf("Unable to get repository owner: %s/%s %v", results.OwnerName, results.RepoName, err), + }) + return + } + + // Check if there is a user redirect for the requested owner + redirectedUserID, err := user_model.LookupUserRedirect(ctx, results.OwnerName) + if err != nil { // User is fetching/cloning a non-existent repository log.Warn("Failed authentication attempt (cannot find repository: %s/%s) from %s", results.OwnerName, results.RepoName, ctx.RemoteAddr()) ctx.JSON(http.StatusNotFound, private.Response{ @@ -130,11 +128,20 @@ func ServCommand(ctx *context.PrivateContext) { }) return } - log.Error("Unable to get repository owner: %s/%s Error: %v", results.OwnerName, results.RepoName, err) - ctx.JSON(http.StatusForbidden, private.Response{ - UserMsg: fmt.Sprintf("Unable to get repository owner: %s/%s %v", results.OwnerName, results.RepoName, err), - }) - return + + redirectUser, err := user_model.GetUserByID(ctx, redirectedUserID) + if err != nil { + // User is fetching/cloning a non-existent repository + log.Warn("Failed authentication attempt (cannot find repository: %s/%s) from %s", results.OwnerName, results.RepoName, ctx.RemoteAddr()) + ctx.JSON(http.StatusNotFound, private.Response{ + UserMsg: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName), + }) + return + } + + log.Info("User %s has been redirected to %s", results.OwnerName, redirectUser.Name) + results.OwnerName = redirectUser.Name + owner = redirectUser } if !owner.IsOrganization() && !owner.IsActive { ctx.JSON(http.StatusForbidden, private.Response{ @@ -143,24 +150,33 @@ func ServCommand(ctx *context.PrivateContext) { return } - redirectedRepoID, err := repo_model.LookupRedirect(ctx, owner.ID, results.RepoName) - if err == nil { - redirectedRepo, err := repo_model.GetRepositoryByID(ctx, redirectedRepoID) - if err == nil { - log.Info("Repository %s/%s has been redirected to %s/%s", results.OwnerName, results.RepoName, redirectedRepo.OwnerName, redirectedRepo.Name) - results.RepoName = redirectedRepo.Name - results.OwnerName = redirectedRepo.OwnerName - owner.ID = redirectedRepo.OwnerID - } else { - log.Warn("Repo %s/%s has a redirect to repo with ID %d, but no repo with this ID could be found. Trying without redirect...", results.OwnerName, results.RepoName, redirectedRepoID) - } - } - // Now get the Repository and set the results section repoExist := true repo, err := repo_model.GetRepositoryByName(ctx, owner.ID, results.RepoName) if err != nil { - if repo_model.IsErrRepoNotExist(err) { + if !repo_model.IsErrRepoNotExist(err) { + log.Error("Unable to get repository: %s/%s Error: %v", results.OwnerName, results.RepoName, err) + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to get repository: %s/%s %v", results.OwnerName, results.RepoName, err), + }) + return + } + + redirectedRepoID, err := repo_model.LookupRedirect(ctx, owner.ID, results.RepoName) + if err == nil { + redirectedRepo, err := repo_model.GetRepositoryByID(ctx, redirectedRepoID) + if err == nil { + log.Info("Repository %s/%s has been redirected to %s/%s", results.OwnerName, results.RepoName, redirectedRepo.OwnerName, redirectedRepo.Name) + results.RepoName = redirectedRepo.Name + results.OwnerName = redirectedRepo.OwnerName + repo = redirectedRepo + owner.ID = redirectedRepo.OwnerID + } else { + log.Warn("Repo %s/%s has a redirect to repo with ID %d, but no repo with this ID could be found. Trying without redirect...", results.OwnerName, results.RepoName, redirectedRepoID) + } + } + + if repo == nil { repoExist = false if mode == perm.AccessModeRead { // User is fetching/cloning a non-existent repository @@ -170,13 +186,6 @@ func ServCommand(ctx *context.PrivateContext) { }) return } - // else fallthrough (push-to-create may kick in below) - } else { - log.Error("Unable to get repository: %s/%s Error: %v", results.OwnerName, results.RepoName, err) - ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Unable to get repository: %s/%s %v", results.OwnerName, results.RepoName, err), - }) - return } } diff --git a/tests/integration/git_ssh_redirect_test.go b/tests/integration/git_ssh_redirect_test.go index 5e35ed2a7442f..3ae2652412c9e 100644 --- a/tests/integration/git_ssh_redirect_test.go +++ b/tests/integration/git_ssh_redirect_test.go @@ -6,9 +6,13 @@ package integration import ( "fmt" "net/url" + "os" "testing" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" ) func TestGitSSHRedirect(t *testing.T) { @@ -16,7 +20,8 @@ func TestGitSSHRedirect(t *testing.T) { } func testGitSSHRedirect(t *testing.T, u *url.URL) { - apiTestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + apiTestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteOrganization) + session := loginUser(t, "user2") withKeyFile(t, "my-testing-key", func(keyFile string) { t.Run("CreateUserKey", doAPICreateUserKey(apiTestContext, "test-key", keyFile)) @@ -38,5 +43,39 @@ func testGitSSHRedirect(t *testing.T, u *url.URL) { t.Run("Clone", doGitClone(t.TempDir(), cloneURL)) }) } + + doAPICreateOrganization(apiTestContext, &structs.CreateOrgOption{ + UserName: "olduser2", + FullName: "Old User2", + })(t) + + cloneURL := createSSHUrl("olduser2/repo1.git", u) + t.Run("Clone Should Fail", doGitCloneFail(cloneURL)) + + doAPICreateOrganizationRepository(apiTestContext, "olduser2", &structs.CreateRepoOption{ + Name: "repo1", + AutoInit: true, + })(t) + testEditFile(t, session, "olduser2", "repo1", "master", "README.md", "This is olduser2's repo1\n") + + dstDir := t.TempDir() + t.Run("Clone", doGitClone(dstDir, cloneURL)) + readMEContent, err := os.ReadFile(dstDir + "/README.md") + assert.NoError(t, err) + assert.Equal(t, "This is olduser2's repo1\n", string(readMEContent)) + + apiTestContext2 := NewAPITestContext(t, "user2", "oldrepo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteOrganization) + doAPICreateRepository(apiTestContext2, false)(t) + testEditFile(t, session, "user2", "oldrepo1", "master", "README.md", "This is user2's oldrepo1\n") + + dstDir = t.TempDir() + cloneURL = createSSHUrl("user2/oldrepo1.git", u) + t.Run("Clone", doGitClone(dstDir, cloneURL)) + readMEContent, err = os.ReadFile(dstDir + "/README.md") + assert.NoError(t, err) + assert.Equal(t, "This is user2's oldrepo1\n", string(readMEContent)) + + cloneURL = createSSHUrl("olduser2/oldrepo1.git", u) + t.Run("Clone Should Fail", doGitCloneFail(cloneURL)) }) }