Skip to content

Commit cb5082f

Browse files
lunnylunny
authored
Fix the bug when ssh clone with redirect user or repository (#36039)
Fix #36026 The redirect should be checked when original user/repo doesn't exist.
1 parent ee365f5 commit cb5082f

File tree

2 files changed

+88
-40
lines changed

2 files changed

+88
-40
lines changed

‎routers/private/serv.go‎

Lines changed: 48 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -108,33 +108,40 @@ func ServCommand(ctx *context.PrivateContext) {
108108
results.RepoName = repoName[:len(repoName)-5]
109109
}
110110

111-
// Check if there is a user redirect for the requested owner
112-
redirectedUserID, err := user_model.LookupUserRedirect(ctx, results.OwnerName)
113-
if err == nil {
114-
owner, err := user_model.GetUserByID(ctx, redirectedUserID)
115-
if err == nil {
116-
log.Info("User %s has been redirected to %s", results.OwnerName, owner.Name)
117-
results.OwnerName = owner.Name
118-
} else {
119-
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)
120-
}
121-
}
122-
123111
owner, err := user_model.GetUserByName(ctx, results.OwnerName)
124112
if err != nil {
125-
if user_model.IsErrUserNotExist(err) {
113+
if !user_model.IsErrUserNotExist(err) {
114+
log.Error("Unable to get repository owner: %s/%s Error: %v", results.OwnerName, results.RepoName, err)
115+
ctx.JSON(http.StatusForbidden, private.Response{
116+
UserMsg: fmt.Sprintf("Unable to get repository owner: %s/%s %v", results.OwnerName, results.RepoName, err),
117+
})
118+
return
119+
}
120+
121+
// Check if there is a user redirect for the requested owner
122+
redirectedUserID, err := user_model.LookupUserRedirect(ctx, results.OwnerName)
123+
if err != nil {
126124
// User is fetching/cloning a non-existent repository
127125
log.Warn("Failed authentication attempt (cannot find repository: %s/%s) from %s", results.OwnerName, results.RepoName, ctx.RemoteAddr())
128126
ctx.JSON(http.StatusNotFound, private.Response{
129127
UserMsg: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName),
130128
})
131129
return
132130
}
133-
log.Error("Unable to get repository owner: %s/%s Error: %v", results.OwnerName, results.RepoName, err)
134-
ctx.JSON(http.StatusForbidden, private.Response{
135-
UserMsg: fmt.Sprintf("Unable to get repository owner: %s/%s %v", results.OwnerName, results.RepoName, err),
136-
})
137-
return
131+
132+
redirectUser, err := user_model.GetUserByID(ctx, redirectedUserID)
133+
if err != nil {
134+
// User is fetching/cloning a non-existent repository
135+
log.Warn("Failed authentication attempt (cannot find repository: %s/%s) from %s", results.OwnerName, results.RepoName, ctx.RemoteAddr())
136+
ctx.JSON(http.StatusNotFound, private.Response{
137+
UserMsg: fmt.Sprintf("Cannot find repository: %s/%s", results.OwnerName, results.RepoName),
138+
})
139+
return
140+
}
141+
142+
log.Info("User %s has been redirected to %s", results.OwnerName, redirectUser.Name)
143+
results.OwnerName = redirectUser.Name
144+
owner = redirectUser
138145
}
139146
if !owner.IsOrganization() && !owner.IsActive {
140147
ctx.JSON(http.StatusForbidden, private.Response{
@@ -143,24 +150,33 @@ func ServCommand(ctx *context.PrivateContext) {
143150
return
144151
}
145152

146-
redirectedRepoID, err := repo_model.LookupRedirect(ctx, owner.ID, results.RepoName)
147-
if err == nil {
148-
redirectedRepo, err := repo_model.GetRepositoryByID(ctx, redirectedRepoID)
149-
if err == nil {
150-
log.Info("Repository %s/%s has been redirected to %s/%s", results.OwnerName, results.RepoName, redirectedRepo.OwnerName, redirectedRepo.Name)
151-
results.RepoName = redirectedRepo.Name
152-
results.OwnerName = redirectedRepo.OwnerName
153-
owner.ID = redirectedRepo.OwnerID
154-
} else {
155-
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)
156-
}
157-
}
158-
159153
// Now get the Repository and set the results section
160154
repoExist := true
161155
repo, err := repo_model.GetRepositoryByName(ctx, owner.ID, results.RepoName)
162156
if err != nil {
163-
if repo_model.IsErrRepoNotExist(err) {
157+
if !repo_model.IsErrRepoNotExist(err) {
158+
log.Error("Unable to get repository: %s/%s Error: %v", results.OwnerName, results.RepoName, err)
159+
ctx.JSON(http.StatusInternalServerError, private.Response{
160+
Err: fmt.Sprintf("Unable to get repository: %s/%s %v", results.OwnerName, results.RepoName, err),
161+
})
162+
return
163+
}
164+
165+
redirectedRepoID, err := repo_model.LookupRedirect(ctx, owner.ID, results.RepoName)
166+
if err == nil {
167+
redirectedRepo, err := repo_model.GetRepositoryByID(ctx, redirectedRepoID)
168+
if err == nil {
169+
log.Info("Repository %s/%s has been redirected to %s/%s", results.OwnerName, results.RepoName, redirectedRepo.OwnerName, redirectedRepo.Name)
170+
results.RepoName = redirectedRepo.Name
171+
results.OwnerName = redirectedRepo.OwnerName
172+
repo = redirectedRepo
173+
owner.ID = redirectedRepo.OwnerID
174+
} else {
175+
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)
176+
}
177+
}
178+
179+
if repo == nil {
164180
repoExist = false
165181
if mode == perm.AccessModeRead {
166182
// User is fetching/cloning a non-existent repository
@@ -170,13 +186,6 @@ func ServCommand(ctx *context.PrivateContext) {
170186
})
171187
return
172188
}
173-
// else fallthrough (push-to-create may kick in below)
174-
} else {
175-
log.Error("Unable to get repository: %s/%s Error: %v", results.OwnerName, results.RepoName, err)
176-
ctx.JSON(http.StatusInternalServerError, private.Response{
177-
Err: fmt.Sprintf("Unable to get repository: %s/%s %v", results.OwnerName, results.RepoName, err),
178-
})
179-
return
180189
}
181190
}
182191

‎tests/integration/git_ssh_redirect_test.go‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,22 @@ package integration
66
import (
77
"fmt"
88
"net/url"
9+
"os"
910
"testing"
1011

1112
auth_model "code.gitea.io/gitea/models/auth"
13+
"code.gitea.io/gitea/modules/structs"
14+
15+
"github.com/stretchr/testify/assert"
1216
)
1317

1418
func TestGitSSHRedirect(t *testing.T) {
1519
onGiteaRun(t, testGitSSHRedirect)
1620
}
1721

1822
func testGitSSHRedirect(t *testing.T, u *url.URL) {
19-
apiTestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
23+
apiTestContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteOrganization)
24+
session := loginUser(t, "user2")
2025

2126
withKeyFile(t, "my-testing-key", func(keyFile string) {
2227
t.Run("CreateUserKey", doAPICreateUserKey(apiTestContext, "test-key", keyFile))
@@ -38,5 +43,39 @@ func testGitSSHRedirect(t *testing.T, u *url.URL) {
3843
t.Run("Clone", doGitClone(t.TempDir(), cloneURL))
3944
})
4045
}
46+
47+
doAPICreateOrganization(apiTestContext, &structs.CreateOrgOption{
48+
UserName: "olduser2",
49+
FullName: "Old User2",
50+
})(t)
51+
52+
cloneURL := createSSHUrl("olduser2/repo1.git", u)
53+
t.Run("Clone Should Fail", doGitCloneFail(cloneURL))
54+
55+
doAPICreateOrganizationRepository(apiTestContext, "olduser2", &structs.CreateRepoOption{
56+
Name: "repo1",
57+
AutoInit: true,
58+
})(t)
59+
testEditFile(t, session, "olduser2", "repo1", "master", "README.md", "This is olduser2's repo1\n")
60+
61+
dstDir := t.TempDir()
62+
t.Run("Clone", doGitClone(dstDir, cloneURL))
63+
readMEContent, err := os.ReadFile(dstDir + "/README.md")
64+
assert.NoError(t, err)
65+
assert.Equal(t, "This is olduser2's repo1\n", string(readMEContent))
66+
67+
apiTestContext2 := NewAPITestContext(t, "user2", "oldrepo1", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteOrganization)
68+
doAPICreateRepository(apiTestContext2, false)(t)
69+
testEditFile(t, session, "user2", "oldrepo1", "master", "README.md", "This is user2's oldrepo1\n")
70+
71+
dstDir = t.TempDir()
72+
cloneURL = createSSHUrl("user2/oldrepo1.git", u)
73+
t.Run("Clone", doGitClone(dstDir, cloneURL))
74+
readMEContent, err = os.ReadFile(dstDir + "/README.md")
75+
assert.NoError(t, err)
76+
assert.Equal(t, "This is user2's oldrepo1\n", string(readMEContent))
77+
78+
cloneURL = createSSHUrl("olduser2/oldrepo1.git", u)
79+
t.Run("Clone Should Fail", doGitCloneFail(cloneURL))
4180
})
4281
}

0 commit comments

Comments
 (0)