Skip to content

Commit dbd0a2e

Browse files
zeripathzeripath
authored
Fix LFS Locks over SSH (#6999) (#7223)
* Fix LFS Locks over SSH * Mark test as skipped
1 parent 7697a28 commit dbd0a2e

File tree

3 files changed

+149
-68
lines changed

3 files changed

+149
-68
lines changed

‎integrations/git_test.go‎

Lines changed: 65 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ func testGit(t *testing.T, u *url.URL) {
6161
little = commitAndPush(t, littleSize, dstPath)
6262
})
6363
t.Run("Big", func(t *testing.T) {
64+
if testing.Short() {
65+
t.Skip("skipping test in short mode.")
66+
return
67+
}
6468
big = commitAndPush(t, bigSize, dstPath)
6569
})
6670
})
@@ -77,9 +81,15 @@ func testGit(t *testing.T, u *url.URL) {
7781

7882
t.Run("Little", func(t *testing.T) {
7983
littleLFS = commitAndPush(t, littleSize, dstPath)
84+
lockFileTest(t, littleLFS, dstPath)
8085
})
8186
t.Run("Big", func(t *testing.T) {
87+
if testing.Short() {
88+
t.Skip("skipping test in short mode.")
89+
return
90+
}
8291
bigLFS = commitAndPush(t, bigSize, dstPath)
92+
lockFileTest(t, bigLFS, dstPath)
8393
})
8494
})
8595
t.Run("Locks", func(t *testing.T) {
@@ -94,19 +104,21 @@ func testGit(t *testing.T, u *url.URL) {
94104
resp := session.MakeRequest(t, req, http.StatusOK)
95105
assert.Equal(t, littleSize, resp.Body.Len())
96106

97-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", big))
98-
nilResp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
99-
assert.Equal(t, bigSize, nilResp.Length)
100-
101107
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", littleLFS))
102108
resp = session.MakeRequest(t, req, http.StatusOK)
103109
assert.NotEqual(t, littleSize, resp.Body.Len())
104110
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
105111

106-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", bigLFS))
107-
resp = session.MakeRequest(t, req, http.StatusOK)
108-
assert.NotEqual(t, bigSize, resp.Body.Len())
109-
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
112+
if !testing.Short() {
113+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", big))
114+
nilResp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
115+
assert.Equal(t, bigSize, nilResp.Length)
116+
117+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", bigLFS))
118+
resp = session.MakeRequest(t, req, http.StatusOK)
119+
assert.NotEqual(t, bigSize, resp.Body.Len())
120+
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
121+
}
110122

111123
})
112124
t.Run("Media", func(t *testing.T) {
@@ -117,17 +129,19 @@ func testGit(t *testing.T, u *url.URL) {
117129
resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
118130
assert.Equal(t, littleSize, resp.Length)
119131

120-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", big))
121-
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
122-
assert.Equal(t, bigSize, resp.Length)
123-
124132
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", littleLFS))
125133
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
126134
assert.Equal(t, littleSize, resp.Length)
127135

128-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", bigLFS))
129-
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
130-
assert.Equal(t, bigSize, resp.Length)
136+
if !testing.Short() {
137+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", big))
138+
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
139+
assert.Equal(t, bigSize, resp.Length)
140+
141+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", bigLFS))
142+
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
143+
assert.Equal(t, bigSize, resp.Length)
144+
}
131145
})
132146

133147
})
@@ -160,6 +174,10 @@ func testGit(t *testing.T, u *url.URL) {
160174
little = commitAndPush(t, littleSize, dstPath)
161175
})
162176
t.Run("Big", func(t *testing.T) {
177+
if testing.Short() {
178+
t.Skip("skipping test in short mode.")
179+
return
180+
}
163181
big = commitAndPush(t, bigSize, dstPath)
164182
})
165183
})
@@ -176,9 +194,16 @@ func testGit(t *testing.T, u *url.URL) {
176194

177195
t.Run("Little", func(t *testing.T) {
178196
littleLFS = commitAndPush(t, littleSize, dstPath)
197+
lockFileTest(t, littleLFS, dstPath)
198+
179199
})
180200
t.Run("Big", func(t *testing.T) {
201+
if testing.Short() {
202+
return
203+
}
181204
bigLFS = commitAndPush(t, bigSize, dstPath)
205+
lockFileTest(t, bigLFS, dstPath)
206+
182207
})
183208
})
184209
t.Run("Locks", func(t *testing.T) {
@@ -193,20 +218,21 @@ func testGit(t *testing.T, u *url.URL) {
193218
resp := session.MakeRequest(t, req, http.StatusOK)
194219
assert.Equal(t, littleSize, resp.Body.Len())
195220

196-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", big))
197-
resp = session.MakeRequest(t, req, http.StatusOK)
198-
assert.Equal(t, bigSize, resp.Body.Len())
199-
200221
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", littleLFS))
201222
resp = session.MakeRequest(t, req, http.StatusOK)
202223
assert.NotEqual(t, littleSize, resp.Body.Len())
203224
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
204225

205-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", bigLFS))
206-
resp = session.MakeRequest(t, req, http.StatusOK)
207-
assert.NotEqual(t, bigSize, resp.Body.Len())
208-
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
226+
if !testing.Short() {
227+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", big))
228+
resp = session.MakeRequest(t, req, http.StatusOK)
229+
assert.Equal(t, bigSize, resp.Body.Len())
209230

231+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", bigLFS))
232+
resp = session.MakeRequest(t, req, http.StatusOK)
233+
assert.NotEqual(t, bigSize, resp.Body.Len())
234+
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
235+
}
210236
})
211237
t.Run("Media", func(t *testing.T) {
212238
session := loginUser(t, "user2")
@@ -216,17 +242,19 @@ func testGit(t *testing.T, u *url.URL) {
216242
resp := session.MakeRequest(t, req, http.StatusOK)
217243
assert.Equal(t, littleSize, resp.Body.Len())
218244

219-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", big))
220-
resp = session.MakeRequest(t, req, http.StatusOK)
221-
assert.Equal(t, bigSize, resp.Body.Len())
222-
223245
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", littleLFS))
224246
resp = session.MakeRequest(t, req, http.StatusOK)
225247
assert.Equal(t, littleSize, resp.Body.Len())
226248

227-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", bigLFS))
228-
resp = session.MakeRequest(t, req, http.StatusOK)
229-
assert.Equal(t, bigSize, resp.Body.Len())
249+
if !testing.Short() {
250+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", big))
251+
resp = session.MakeRequest(t, req, http.StatusOK)
252+
assert.Equal(t, bigSize, resp.Body.Len())
253+
254+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", bigLFS))
255+
resp = session.MakeRequest(t, req, http.StatusOK)
256+
assert.Equal(t, bigSize, resp.Body.Len())
257+
}
230258
})
231259

232260
})
@@ -243,15 +271,17 @@ func ensureAnonymousClone(t *testing.T, u *url.URL) {
243271
}
244272

245273
func lockTest(t *testing.T, remote, repoPath string) {
246-
_, err := git.NewCommand("remote").AddArguments("set-url", "origin", remote).RunInDir(repoPath) //TODO add test ssh git-lfs-creds
247-
assert.NoError(t, err)
248-
_, err = git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath)
274+
lockFileTest(t, "README.md", repoPath)
275+
}
276+
277+
func lockFileTest(t *testing.T, filename, repoPath string) {
278+
_, err := git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath)
249279
assert.NoError(t, err)
250-
_, err = git.NewCommand("lfs").AddArguments("lock", "README.md").RunInDir(repoPath)
280+
_, err = git.NewCommand("lfs").AddArguments("lock", filename).RunInDir(repoPath)
251281
assert.NoError(t, err)
252282
_, err = git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath)
253283
assert.NoError(t, err)
254-
_, err = git.NewCommand("lfs").AddArguments("unlock", "README.md").RunInDir(repoPath)
284+
_, err = git.NewCommand("lfs").AddArguments("unlock", filename).RunInDir(repoPath)
255285
assert.NoError(t, err)
256286
}
257287

‎modules/lfs/locks.go‎

Lines changed: 83 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"code.gitea.io/gitea/models"
1313
"code.gitea.io/gitea/modules/context"
14+
"code.gitea.io/gitea/modules/log"
1415
"code.gitea.io/gitea/modules/setting"
1516
api "code.gitea.io/sdk/gitea"
1617
)
@@ -44,7 +45,7 @@ func checkIsValidRequest(ctx *context.Context, post bool) bool {
4445
return true
4546
}
4647

47-
func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) {
48+
func handleLockListOut(ctx *context.Context, repo *models.Repository, lock *models.LFSLock, err error) {
4849
if err != nil {
4950
if models.IsErrLFSLockNotExist(err) {
5051
ctx.JSON(200, api.LFSLockList{
@@ -57,7 +58,7 @@ func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) {
5758
})
5859
return
5960
}
60-
if ctx.Repo.Repository.ID != lock.RepoID {
61+
if repo.ID != lock.RepoID {
6162
ctx.JSON(200, api.LFSLockList{
6263
Locks: []*api.LFSLock{},
6364
})
@@ -75,17 +76,21 @@ func GetListLockHandler(ctx *context.Context) {
7576
}
7677
ctx.Resp.Header().Set("Content-Type", metaMediaType)
7778

78-
err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository, models.AccessModeRead)
79+
rv := unpack(ctx)
80+
81+
repository, err := models.GetRepositoryByOwnerAndName(rv.User, rv.Repo)
7982
if err != nil {
80-
if models.IsErrLFSUnauthorizedAction(err) {
81-
ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
82-
ctx.JSON(401, api.LFSLockError{
83-
Message: "You must have pull access to list locks : " + err.Error(),
84-
})
85-
return
86-
}
87-
ctx.JSON(500, api.LFSLockError{
88-
Message: "unable to list lock : " + err.Error(),
83+
log.Debug("Could not find repository: %s/%s - %s", rv.User, rv.Repo, err)
84+
writeStatus(ctx, 404)
85+
return
86+
}
87+
repository.MustOwner()
88+
89+
authenticated := authenticate(ctx, repository, rv.Authorization, false)
90+
if !authenticated {
91+
ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
92+
ctx.JSON(401, api.LFSLockError{
93+
Message: "You must have pull access to list locks",
8994
})
9095
return
9196
}
@@ -100,19 +105,19 @@ func GetListLockHandler(ctx *context.Context) {
100105
return
101106
}
102107
lock, err := models.GetLFSLockByID(int64(v))
103-
handleLockListOut(ctx, lock, err)
108+
handleLockListOut(ctx, repository, lock, err)
104109
return
105110
}
106111

107112
path := ctx.Query("path")
108113
if path != "" { //Case where we request a specific id
109-
lock, err := models.GetLFSLock(ctx.Repo.Repository, path)
110-
handleLockListOut(ctx, lock, err)
114+
lock, err := models.GetLFSLock(repository, path)
115+
handleLockListOut(ctx, repository, lock, err)
111116
return
112117
}
113118

114119
//If no query params path or id
115-
lockList, err := models.GetLFSLockByRepoID(ctx.Repo.Repository.ID)
120+
lockList, err := models.GetLFSLockByRepoID(repository.ID)
116121
if err != nil {
117122
ctx.JSON(500, api.LFSLockError{
118123
Message: "unable to list locks : " + err.Error(),
@@ -135,16 +140,36 @@ func PostLockHandler(ctx *context.Context) {
135140
}
136141
ctx.Resp.Header().Set("Content-Type", metaMediaType)
137142

143+
userName := ctx.Params("username")
144+
repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git")
145+
authorization := ctx.Req.Header.Get("Authorization")
146+
147+
repository, err := models.GetRepositoryByOwnerAndName(userName, repoName)
148+
if err != nil {
149+
log.Debug("Could not find repository: %s/%s - %s", userName, repoName, err)
150+
writeStatus(ctx, 404)
151+
return
152+
}
153+
repository.MustOwner()
154+
155+
authenticated := authenticate(ctx, repository, authorization, true)
156+
if !authenticated {
157+
ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
158+
ctx.JSON(401, api.LFSLockError{
159+
Message: "You must have push access to create locks",
160+
})
161+
return
162+
}
163+
138164
var req api.LFSLockRequest
139165
dec := json.NewDecoder(ctx.Req.Body().ReadCloser())
140-
err := dec.Decode(&req)
141-
if err != nil {
166+
if err := dec.Decode(&req); err != nil {
142167
writeStatus(ctx, 400)
143168
return
144169
}
145170

146171
lock, err := models.CreateLFSLock(&models.LFSLock{
147-
Repo: ctx.Repo.Repository,
172+
Repo: repository,
148173
Path: req.Path,
149174
Owner: ctx.User,
150175
})
@@ -178,23 +203,29 @@ func VerifyLockHandler(ctx *context.Context) {
178203
}
179204
ctx.Resp.Header().Set("Content-Type", metaMediaType)
180205

181-
err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository, models.AccessModeWrite)
206+
userName := ctx.Params("username")
207+
repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git")
208+
authorization := ctx.Req.Header.Get("Authorization")
209+
210+
repository, err := models.GetRepositoryByOwnerAndName(userName, repoName)
182211
if err != nil {
183-
if models.IsErrLFSUnauthorizedAction(err) {
184-
ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
185-
ctx.JSON(401, api.LFSLockError{
186-
Message: "You must have push access to verify locks : " + err.Error(),
187-
})
188-
return
189-
}
190-
ctx.JSON(500, api.LFSLockError{
191-
Message: "unable to verify lock : " + err.Error(),
212+
log.Debug("Could not find repository: %s/%s - %s", userName, repoName, err)
213+
writeStatus(ctx, 404)
214+
return
215+
}
216+
repository.MustOwner()
217+
218+
authenticated := authenticate(ctx, repository, authorization, true)
219+
if !authenticated {
220+
ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
221+
ctx.JSON(401, api.LFSLockError{
222+
Message: "You must have push access to verify locks",
192223
})
193224
return
194225
}
195226

196227
//TODO handle body json cursor and limit
197-
lockList, err := models.GetLFSLockByRepoID(ctx.Repo.Repository.ID)
228+
lockList, err := models.GetLFSLockByRepoID(repository.ID)
198229
if err != nil {
199230
ctx.JSON(500, api.LFSLockError{
200231
Message: "unable to list locks : " + err.Error(),
@@ -223,10 +254,30 @@ func UnLockHandler(ctx *context.Context) {
223254
}
224255
ctx.Resp.Header().Set("Content-Type", metaMediaType)
225256

257+
userName := ctx.Params("username")
258+
repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git")
259+
authorization := ctx.Req.Header.Get("Authorization")
260+
261+
repository, err := models.GetRepositoryByOwnerAndName(userName, repoName)
262+
if err != nil {
263+
log.Debug("Could not find repository: %s/%s - %s", userName, repoName, err)
264+
writeStatus(ctx, 404)
265+
return
266+
}
267+
repository.MustOwner()
268+
269+
authenticated := authenticate(ctx, repository, authorization, true)
270+
if !authenticated {
271+
ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
272+
ctx.JSON(401, api.LFSLockError{
273+
Message: "You must have push access to delete locks",
274+
})
275+
return
276+
}
277+
226278
var req api.LFSLockDeleteRequest
227279
dec := json.NewDecoder(ctx.Req.Body().ReadCloser())
228-
err := dec.Decode(&req)
229-
if err != nil {
280+
if err := dec.Decode(&req); err != nil {
230281
writeStatus(ctx, 400)
231282
return
232283
}

‎routers/routes/routes.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,7 @@ func RegisterRoutes(m *macaron.Macaron) {
827827
m.Post("/", lfs.PostLockHandler)
828828
m.Post("/verify", lfs.VerifyLockHandler)
829829
m.Post("/:lid/unlock", lfs.UnLockHandler)
830-
}, context.RepoAssignment())
830+
})
831831
m.Any("/*", func(ctx *context.Context) {
832832
ctx.NotFound("", nil)
833833
})

0 commit comments

Comments
 (0)