Skip to content

Commit cbe3ca5

Browse files
6543KN4CK3R
andauthored
Test if LFS object is accessible (#16865) (#16904)
* Test if object is accessible. * Added more logging. Co-authored-by: KN4CK3R <[email protected]>
1 parent 3ac1f35 commit cbe3ca5

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
@@ -254,6 +254,10 @@ func TestAPILFSBatch(t *testing.T) {
254254
assert.NoError(t, err)
255255
assert.True(t, exist)
256256

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

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

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

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

371381
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"
@@ -213,14 +215,22 @@ func BatchHandler(ctx *context.Context) {
213215
}
214216
}
215217

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

@@ -270,29 +280,50 @@ func UploadHandler(ctx *context.Context) {
270280
return
271281
}
272282

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

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

0 commit comments

Comments
 (0)