Skip to content

Commit 2bb3200

Browse files
authored
Test if LFS object is accessible (#16865)
* Test if object is accessible. * Added more logging.
1 parent d217024 commit 2bb3200

File tree

2 files changed

+60
-19
lines changed

2 files changed

+60
-19
lines changed

integrations/api_repo_lfs_test.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,10 @@ func TestAPILFSBatch(t *testing.T) {
253253
assert.NoError(t, err)
254254
assert.True(t, exist)
255255

256+
repo2 := createLFSTestRepository(t, "batch2")
257+
content := []byte("dummy0")
258+
storeObjectInRepo(t, repo2.ID, &content)
259+
256260
meta, err := repo.GetLFSMetaObjectByOid(p.Oid)
257261
assert.Nil(t, meta)
258262
assert.Equal(t, models.ErrLFSObjectNotExist, err)
@@ -358,13 +362,19 @@ func TestAPILFSUpload(t *testing.T) {
358362
assert.Nil(t, meta)
359363
assert.Equal(t, models.ErrLFSObjectNotExist, err)
360364

361-
req := newRequest(t, p, "")
365+
t.Run("InvalidAccess", func(t *testing.T) {
366+
req := newRequest(t, p, "invalid")
367+
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
368+
})
362369

363-
session.MakeRequest(t, req, http.StatusOK)
370+
t.Run("ValidAccess", func(t *testing.T) {
371+
req := newRequest(t, p, "dummy5")
364372

365-
meta, err = repo.GetLFSMetaObjectByOid(p.Oid)
366-
assert.NoError(t, err)
367-
assert.NotNil(t, meta)
373+
session.MakeRequest(t, req, http.StatusOK)
374+
meta, err = repo.GetLFSMetaObjectByOid(p.Oid)
375+
assert.NoError(t, err)
376+
assert.NotNil(t, meta)
377+
})
368378
})
369379

370380
t.Run("MetaAlreadyExists", func(t *testing.T) {

services/lfs/server.go

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
package lfs
66

77
import (
8+
"crypto/sha256"
89
"encoding/base64"
10+
"encoding/hex"
911
"errors"
1012
"fmt"
1113
"io"
@@ -214,14 +216,22 @@ func BatchHandler(ctx *context.Context) {
214216
}
215217
}
216218

217-
if exists {
218-
if meta == nil {
219+
if exists && meta == nil {
220+
accessible, err := models.LFSObjectAccessible(ctx.User, p.Oid)
221+
if err != nil {
222+
log.Error("Unable to check if LFS MetaObject [%s] is accessible. Error: %v", p.Oid, err)
223+
writeStatus(ctx, http.StatusInternalServerError)
224+
return
225+
}
226+
if accessible {
219227
_, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID})
220228
if err != nil {
221229
log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err)
222230
writeStatus(ctx, http.StatusInternalServerError)
223231
return
224232
}
233+
} else {
234+
exists = false
225235
}
226236
}
227237

@@ -271,29 +281,50 @@ func UploadHandler(ctx *context.Context) {
271281
return
272282
}
273283

274-
meta, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID})
275-
if err != nil {
276-
log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err)
277-
writeStatus(ctx, http.StatusInternalServerError)
278-
return
279-
}
280-
281284
contentStore := lfs_module.NewContentStore()
282-
283285
exists, err := contentStore.Exists(p)
284286
if err != nil {
285287
log.Error("Unable to check if LFS OID[%s] exist. Error: %v", p.Oid, err)
286288
writeStatus(ctx, http.StatusInternalServerError)
287289
return
288290
}
289-
if meta.Existing || exists {
290-
ctx.Resp.WriteHeader(http.StatusOK)
291-
return
291+
292+
uploadOrVerify := func() error {
293+
if exists {
294+
accessible, err := models.LFSObjectAccessible(ctx.User, p.Oid)
295+
if err != nil {
296+
log.Error("Unable to check if LFS MetaObject [%s] is accessible. Error: %v", p.Oid, err)
297+
return err
298+
}
299+
if !accessible {
300+
// The file exists but the user has no access to it.
301+
// The upload gets verified by hashing and size comparison to prove access to it.
302+
hash := sha256.New()
303+
written, err := io.Copy(hash, ctx.Req.Body)
304+
if err != nil {
305+
log.Error("Error creating hash. Error: %v", err)
306+
return err
307+
}
308+
309+
if written != p.Size {
310+
return lfs_module.ErrSizeMismatch
311+
}
312+
if hex.EncodeToString(hash.Sum(nil)) != p.Oid {
313+
return lfs_module.ErrHashMismatch
314+
}
315+
}
316+
} else if err := contentStore.Put(p, ctx.Req.Body); err != nil {
317+
log.Error("Error putting LFS MetaObject [%s] into content store. Error: %v", p.Oid, err)
318+
return err
319+
}
320+
_, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID})
321+
return err
292322
}
293323

294324
defer ctx.Req.Body.Close()
295-
if err := contentStore.Put(meta.Pointer, ctx.Req.Body); err != nil {
325+
if err := uploadOrVerify(); err != nil {
296326
if errors.Is(err, lfs_module.ErrSizeMismatch) || errors.Is(err, lfs_module.ErrHashMismatch) {
327+
log.Error("Upload does not match LFS MetaObject [%s]. Error: %v", p.Oid, err)
297328
writeStatusMessage(ctx, http.StatusUnprocessableEntity, err.Error())
298329
} else {
299330
writeStatus(ctx, http.StatusInternalServerError)

0 commit comments

Comments
 (0)