Skip to content

Commit 2092fb0

Browse files
zeripathjeffliu27
zeripath
authored and
jeffliu27
committed
Fix LFS Locks over SSH (go-gitea#6999)
* Fix LFS Locks over SSH * Mark test as skipped
1 parent a1db9cf commit 2092fb0

File tree

3 files changed

+149
-68
lines changed

3 files changed

+149
-68
lines changed

‎integrations/git_test.go

+65-35
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ func testGit(t *testing.T, u *url.URL) {
6565
little = commitAndPush(t, littleSize, dstPath)
6666
})
6767
t.Run("Big", func(t *testing.T) {
68+
if testing.Short() {
69+
t.Skip("skipping test in short mode.")
70+
return
71+
}
6872
PrintCurrentTest(t)
6973
big = commitAndPush(t, bigSize, dstPath)
7074
})
@@ -85,10 +89,16 @@ func testGit(t *testing.T, u *url.URL) {
8589
t.Run("Little", func(t *testing.T) {
8690
PrintCurrentTest(t)
8791
littleLFS = commitAndPush(t, littleSize, dstPath)
92+
lockFileTest(t, littleLFS, dstPath)
8893
})
8994
t.Run("Big", func(t *testing.T) {
95+
if testing.Short() {
96+
t.Skip("skipping test in short mode.")
97+
return
98+
}
9099
PrintCurrentTest(t)
91100
bigLFS = commitAndPush(t, bigSize, dstPath)
101+
lockFileTest(t, bigLFS, dstPath)
92102
})
93103
})
94104
t.Run("Locks", func(t *testing.T) {
@@ -105,19 +115,21 @@ func testGit(t *testing.T, u *url.URL) {
105115
resp := session.MakeRequest(t, req, http.StatusOK)
106116
assert.Equal(t, littleSize, resp.Body.Len())
107117

108-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", big))
109-
nilResp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
110-
assert.Equal(t, bigSize, nilResp.Length)
111-
112118
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", littleLFS))
113119
resp = session.MakeRequest(t, req, http.StatusOK)
114120
assert.NotEqual(t, littleSize, resp.Body.Len())
115121
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
116122

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)
123+
if !testing.Short() {
124+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", big))
125+
nilResp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
126+
assert.Equal(t, bigSize, nilResp.Length)
127+
128+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", bigLFS))
129+
resp = session.MakeRequest(t, req, http.StatusOK)
130+
assert.NotEqual(t, bigSize, resp.Body.Len())
131+
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
132+
}
121133

122134
})
123135
t.Run("Media", func(t *testing.T) {
@@ -129,17 +141,19 @@ func testGit(t *testing.T, u *url.URL) {
129141
resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
130142
assert.Equal(t, littleSize, resp.Length)
131143

132-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", big))
133-
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
134-
assert.Equal(t, bigSize, resp.Length)
135-
136144
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", littleLFS))
137145
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
138146
assert.Equal(t, littleSize, resp.Length)
139147

140-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", bigLFS))
141-
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
142-
assert.Equal(t, bigSize, resp.Length)
148+
if !testing.Short() {
149+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", big))
150+
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
151+
assert.Equal(t, bigSize, resp.Length)
152+
153+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", bigLFS))
154+
resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
155+
assert.Equal(t, bigSize, resp.Length)
156+
}
143157
})
144158

145159
})
@@ -177,6 +191,10 @@ func testGit(t *testing.T, u *url.URL) {
177191
little = commitAndPush(t, littleSize, dstPath)
178192
})
179193
t.Run("Big", func(t *testing.T) {
194+
if testing.Short() {
195+
t.Skip("skipping test in short mode.")
196+
return
197+
}
180198
PrintCurrentTest(t)
181199
big = commitAndPush(t, bigSize, dstPath)
182200
})
@@ -197,10 +215,17 @@ func testGit(t *testing.T, u *url.URL) {
197215
t.Run("Little", func(t *testing.T) {
198216
PrintCurrentTest(t)
199217
littleLFS = commitAndPush(t, littleSize, dstPath)
218+
lockFileTest(t, littleLFS, dstPath)
219+
200220
})
201221
t.Run("Big", func(t *testing.T) {
222+
if testing.Short() {
223+
return
224+
}
202225
PrintCurrentTest(t)
203226
bigLFS = commitAndPush(t, bigSize, dstPath)
227+
lockFileTest(t, bigLFS, dstPath)
228+
204229
})
205230
})
206231
t.Run("Locks", func(t *testing.T) {
@@ -217,20 +242,21 @@ func testGit(t *testing.T, u *url.URL) {
217242
resp := session.MakeRequest(t, req, http.StatusOK)
218243
assert.Equal(t, littleSize, resp.Body.Len())
219244

220-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", big))
221-
resp = session.MakeRequest(t, req, http.StatusOK)
222-
assert.Equal(t, bigSize, resp.Body.Len())
223-
224245
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", littleLFS))
225246
resp = session.MakeRequest(t, req, http.StatusOK)
226247
assert.NotEqual(t, littleSize, resp.Body.Len())
227248
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
228249

229-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", bigLFS))
230-
resp = session.MakeRequest(t, req, http.StatusOK)
231-
assert.NotEqual(t, bigSize, resp.Body.Len())
232-
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
250+
if !testing.Short() {
251+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", big))
252+
resp = session.MakeRequest(t, req, http.StatusOK)
253+
assert.Equal(t, bigSize, resp.Body.Len())
233254

255+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", bigLFS))
256+
resp = session.MakeRequest(t, req, http.StatusOK)
257+
assert.NotEqual(t, bigSize, resp.Body.Len())
258+
assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
259+
}
234260
})
235261
t.Run("Media", func(t *testing.T) {
236262
PrintCurrentTest(t)
@@ -241,17 +267,19 @@ func testGit(t *testing.T, u *url.URL) {
241267
resp := session.MakeRequest(t, req, http.StatusOK)
242268
assert.Equal(t, littleSize, resp.Body.Len())
243269

244-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", big))
245-
resp = session.MakeRequest(t, req, http.StatusOK)
246-
assert.Equal(t, bigSize, resp.Body.Len())
247-
248270
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", littleLFS))
249271
resp = session.MakeRequest(t, req, http.StatusOK)
250272
assert.Equal(t, littleSize, resp.Body.Len())
251273

252-
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", bigLFS))
253-
resp = session.MakeRequest(t, req, http.StatusOK)
254-
assert.Equal(t, bigSize, resp.Body.Len())
274+
if !testing.Short() {
275+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", big))
276+
resp = session.MakeRequest(t, req, http.StatusOK)
277+
assert.Equal(t, bigSize, resp.Body.Len())
278+
279+
req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", bigLFS))
280+
resp = session.MakeRequest(t, req, http.StatusOK)
281+
assert.Equal(t, bigSize, resp.Body.Len())
282+
}
255283
})
256284

257285
})
@@ -268,15 +296,17 @@ func ensureAnonymousClone(t *testing.T, u *url.URL) {
268296
}
269297

270298
func lockTest(t *testing.T, remote, repoPath string) {
271-
_, err := git.NewCommand("remote").AddArguments("set-url", "origin", remote).RunInDir(repoPath) //TODO add test ssh git-lfs-creds
272-
assert.NoError(t, err)
273-
_, err = git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath)
299+
lockFileTest(t, "README.md", repoPath)
300+
}
301+
302+
func lockFileTest(t *testing.T, filename, repoPath string) {
303+
_, err := git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath)
274304
assert.NoError(t, err)
275-
_, err = git.NewCommand("lfs").AddArguments("lock", "README.md").RunInDir(repoPath)
305+
_, err = git.NewCommand("lfs").AddArguments("lock", filename).RunInDir(repoPath)
276306
assert.NoError(t, err)
277307
_, err = git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath)
278308
assert.NoError(t, err)
279-
_, err = git.NewCommand("lfs").AddArguments("unlock", "README.md").RunInDir(repoPath)
309+
_, err = git.NewCommand("lfs").AddArguments("unlock", filename).RunInDir(repoPath)
280310
assert.NoError(t, err)
281311
}
282312

‎modules/lfs/locks.go

+83-32
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/gitea/modules/structs"
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

+1-1
Original file line numberDiff line numberDiff line change
@@ -923,7 +923,7 @@ func RegisterRoutes(m *macaron.Macaron) {
923923
m.Post("/", lfs.PostLockHandler)
924924
m.Post("/verify", lfs.VerifyLockHandler)
925925
m.Post("/:lid/unlock", lfs.UnLockHandler)
926-
}, context.RepoAssignment())
926+
})
927927
m.Any("/*", func(ctx *context.Context) {
928928
ctx.NotFound("", nil)
929929
})

0 commit comments

Comments
 (0)