From 31557b12744410633ceb6fc12b53fb09038cee35 Mon Sep 17 00:00:00 2001 From: zeripath Date: Tue, 28 May 2019 11:32:41 +0100 Subject: [PATCH] Fix LFS Locks over SSH (#6999) * Fix LFS Locks over SSH * Mark test as skipped --- integrations/git_test.go | 100 ++++++++++++++++++++++------------ modules/lfs/locks.go | 115 ++++++++++++++++++++++++++++----------- routers/routes/routes.go | 2 +- 3 files changed, 149 insertions(+), 68 deletions(-) diff --git a/integrations/git_test.go b/integrations/git_test.go index ebbf04f9d..0554f9a5a 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -65,6 +65,10 @@ func testGit(t *testing.T, u *url.URL) { little = commitAndPush(t, littleSize, dstPath) }) t.Run("Big", func(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in short mode.") + return + } PrintCurrentTest(t) big = commitAndPush(t, bigSize, dstPath) }) @@ -85,10 +89,16 @@ func testGit(t *testing.T, u *url.URL) { t.Run("Little", func(t *testing.T) { PrintCurrentTest(t) littleLFS = commitAndPush(t, littleSize, dstPath) + lockFileTest(t, littleLFS, dstPath) }) t.Run("Big", func(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in short mode.") + return + } PrintCurrentTest(t) bigLFS = commitAndPush(t, bigSize, dstPath) + lockFileTest(t, bigLFS, dstPath) }) }) t.Run("Locks", func(t *testing.T) { @@ -105,19 +115,21 @@ func testGit(t *testing.T, u *url.URL) { resp := session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Body.Len()) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", big)) - nilResp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, bigSize, nilResp.Length) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", littleLFS)) resp = session.MakeRequest(t, req, http.StatusOK) assert.NotEqual(t, littleSize, resp.Body.Len()) assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", bigLFS)) - resp = session.MakeRequest(t, req, http.StatusOK) - assert.NotEqual(t, bigSize, resp.Body.Len()) - assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) + if !testing.Short() { + req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", big)) + nilResp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) + assert.Equal(t, bigSize, nilResp.Length) + + req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", bigLFS)) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.NotEqual(t, bigSize, resp.Body.Len()) + assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) + } }) t.Run("Media", func(t *testing.T) { @@ -129,17 +141,19 @@ func testGit(t *testing.T, u *url.URL) { resp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Length) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", big)) - resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, bigSize, resp.Length) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", littleLFS)) resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Length) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", bigLFS)) - resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) - assert.Equal(t, bigSize, resp.Length) + if !testing.Short() { + req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", big)) + resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) + assert.Equal(t, bigSize, resp.Length) + + req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", bigLFS)) + resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK) + assert.Equal(t, bigSize, resp.Length) + } }) }) @@ -177,6 +191,10 @@ func testGit(t *testing.T, u *url.URL) { little = commitAndPush(t, littleSize, dstPath) }) t.Run("Big", func(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in short mode.") + return + } PrintCurrentTest(t) big = commitAndPush(t, bigSize, dstPath) }) @@ -197,10 +215,17 @@ func testGit(t *testing.T, u *url.URL) { t.Run("Little", func(t *testing.T) { PrintCurrentTest(t) littleLFS = commitAndPush(t, littleSize, dstPath) + lockFileTest(t, littleLFS, dstPath) + }) t.Run("Big", func(t *testing.T) { + if testing.Short() { + return + } PrintCurrentTest(t) bigLFS = commitAndPush(t, bigSize, dstPath) + lockFileTest(t, bigLFS, dstPath) + }) }) t.Run("Locks", func(t *testing.T) { @@ -217,20 +242,21 @@ func testGit(t *testing.T, u *url.URL) { resp := session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Body.Len()) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", big)) - resp = session.MakeRequest(t, req, http.StatusOK) - assert.Equal(t, bigSize, resp.Body.Len()) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", littleLFS)) resp = session.MakeRequest(t, req, http.StatusOK) assert.NotEqual(t, littleSize, resp.Body.Len()) assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", bigLFS)) - resp = session.MakeRequest(t, req, http.StatusOK) - assert.NotEqual(t, bigSize, resp.Body.Len()) - assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) + if !testing.Short() { + req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", big)) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, bigSize, resp.Body.Len()) + req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", bigLFS)) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.NotEqual(t, bigSize, resp.Body.Len()) + assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier) + } }) t.Run("Media", func(t *testing.T) { PrintCurrentTest(t) @@ -241,17 +267,19 @@ func testGit(t *testing.T, u *url.URL) { resp := session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Body.Len()) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", big)) - resp = session.MakeRequest(t, req, http.StatusOK) - assert.Equal(t, bigSize, resp.Body.Len()) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", littleLFS)) resp = session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, littleSize, resp.Body.Len()) - req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", bigLFS)) - resp = session.MakeRequest(t, req, http.StatusOK) - assert.Equal(t, bigSize, resp.Body.Len()) + if !testing.Short() { + req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", big)) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, bigSize, resp.Body.Len()) + + req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", bigLFS)) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, bigSize, resp.Body.Len()) + } }) }) @@ -268,15 +296,17 @@ func ensureAnonymousClone(t *testing.T, u *url.URL) { } func lockTest(t *testing.T, remote, repoPath string) { - _, err := git.NewCommand("remote").AddArguments("set-url", "origin", remote).RunInDir(repoPath) //TODO add test ssh git-lfs-creds - assert.NoError(t, err) - _, err = git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath) + lockFileTest(t, "README.md", repoPath) +} + +func lockFileTest(t *testing.T, filename, repoPath string) { + _, err := git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath) assert.NoError(t, err) - _, err = git.NewCommand("lfs").AddArguments("lock", "README.md").RunInDir(repoPath) + _, err = git.NewCommand("lfs").AddArguments("lock", filename).RunInDir(repoPath) assert.NoError(t, err) _, err = git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath) assert.NoError(t, err) - _, err = git.NewCommand("lfs").AddArguments("unlock", "README.md").RunInDir(repoPath) + _, err = git.NewCommand("lfs").AddArguments("unlock", filename).RunInDir(repoPath) assert.NoError(t, err) } diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 525a93645..b1ca2f094 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" ) @@ -44,7 +45,7 @@ func checkIsValidRequest(ctx *context.Context, post bool) bool { return true } -func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) { +func handleLockListOut(ctx *context.Context, repo *models.Repository, lock *models.LFSLock, err error) { if err != nil { if models.IsErrLFSLockNotExist(err) { ctx.JSON(200, api.LFSLockList{ @@ -57,7 +58,7 @@ func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) { }) return } - if ctx.Repo.Repository.ID != lock.RepoID { + if repo.ID != lock.RepoID { ctx.JSON(200, api.LFSLockList{ Locks: []*api.LFSLock{}, }) @@ -75,17 +76,21 @@ func GetListLockHandler(ctx *context.Context) { } ctx.Resp.Header().Set("Content-Type", metaMediaType) - err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository, models.AccessModeRead) + rv := unpack(ctx) + + repository, err := models.GetRepositoryByOwnerAndName(rv.User, rv.Repo) if err != nil { - if models.IsErrLFSUnauthorizedAction(err) { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") - ctx.JSON(401, api.LFSLockError{ - Message: "You must have pull access to list locks : " + err.Error(), - }) - return - } - ctx.JSON(500, api.LFSLockError{ - Message: "unable to list lock : " + err.Error(), + log.Debug("Could not find repository: %s/%s - %s", rv.User, rv.Repo, err) + writeStatus(ctx, 404) + return + } + repository.MustOwner() + + authenticated := authenticate(ctx, repository, rv.Authorization, false) + if !authenticated { + ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.JSON(401, api.LFSLockError{ + Message: "You must have pull access to list locks", }) return } @@ -100,19 +105,19 @@ func GetListLockHandler(ctx *context.Context) { return } lock, err := models.GetLFSLockByID(int64(v)) - handleLockListOut(ctx, lock, err) + handleLockListOut(ctx, repository, lock, err) return } path := ctx.Query("path") if path != "" { //Case where we request a specific id - lock, err := models.GetLFSLock(ctx.Repo.Repository, path) - handleLockListOut(ctx, lock, err) + lock, err := models.GetLFSLock(repository, path) + handleLockListOut(ctx, repository, lock, err) return } //If no query params path or id - lockList, err := models.GetLFSLockByRepoID(ctx.Repo.Repository.ID) + lockList, err := models.GetLFSLockByRepoID(repository.ID) if err != nil { ctx.JSON(500, api.LFSLockError{ Message: "unable to list locks : " + err.Error(), @@ -135,16 +140,36 @@ func PostLockHandler(ctx *context.Context) { } ctx.Resp.Header().Set("Content-Type", metaMediaType) + userName := ctx.Params("username") + repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git") + authorization := ctx.Req.Header.Get("Authorization") + + repository, err := models.GetRepositoryByOwnerAndName(userName, repoName) + if err != nil { + log.Debug("Could not find repository: %s/%s - %s", userName, repoName, err) + writeStatus(ctx, 404) + return + } + repository.MustOwner() + + authenticated := authenticate(ctx, repository, authorization, true) + if !authenticated { + ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.JSON(401, api.LFSLockError{ + Message: "You must have push access to create locks", + }) + return + } + var req api.LFSLockRequest dec := json.NewDecoder(ctx.Req.Body().ReadCloser()) - err := dec.Decode(&req) - if err != nil { + if err := dec.Decode(&req); err != nil { writeStatus(ctx, 400) return } lock, err := models.CreateLFSLock(&models.LFSLock{ - Repo: ctx.Repo.Repository, + Repo: repository, Path: req.Path, Owner: ctx.User, }) @@ -178,23 +203,29 @@ func VerifyLockHandler(ctx *context.Context) { } ctx.Resp.Header().Set("Content-Type", metaMediaType) - err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository, models.AccessModeWrite) + userName := ctx.Params("username") + repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git") + authorization := ctx.Req.Header.Get("Authorization") + + repository, err := models.GetRepositoryByOwnerAndName(userName, repoName) if err != nil { - if models.IsErrLFSUnauthorizedAction(err) { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") - ctx.JSON(401, api.LFSLockError{ - Message: "You must have push access to verify locks : " + err.Error(), - }) - return - } - ctx.JSON(500, api.LFSLockError{ - Message: "unable to verify lock : " + err.Error(), + log.Debug("Could not find repository: %s/%s - %s", userName, repoName, err) + writeStatus(ctx, 404) + return + } + repository.MustOwner() + + authenticated := authenticate(ctx, repository, authorization, true) + if !authenticated { + ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.JSON(401, api.LFSLockError{ + Message: "You must have push access to verify locks", }) return } //TODO handle body json cursor and limit - lockList, err := models.GetLFSLockByRepoID(ctx.Repo.Repository.ID) + lockList, err := models.GetLFSLockByRepoID(repository.ID) if err != nil { ctx.JSON(500, api.LFSLockError{ Message: "unable to list locks : " + err.Error(), @@ -223,10 +254,30 @@ func UnLockHandler(ctx *context.Context) { } ctx.Resp.Header().Set("Content-Type", metaMediaType) + userName := ctx.Params("username") + repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git") + authorization := ctx.Req.Header.Get("Authorization") + + repository, err := models.GetRepositoryByOwnerAndName(userName, repoName) + if err != nil { + log.Debug("Could not find repository: %s/%s - %s", userName, repoName, err) + writeStatus(ctx, 404) + return + } + repository.MustOwner() + + authenticated := authenticate(ctx, repository, authorization, true) + if !authenticated { + ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") + ctx.JSON(401, api.LFSLockError{ + Message: "You must have push access to delete locks", + }) + return + } + var req api.LFSLockDeleteRequest dec := json.NewDecoder(ctx.Req.Body().ReadCloser()) - err := dec.Decode(&req) - if err != nil { + if err := dec.Decode(&req); err != nil { writeStatus(ctx, 400) return } diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 5a5fc518b..d19823714 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -923,7 +923,7 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/", lfs.PostLockHandler) m.Post("/verify", lfs.VerifyLockHandler) m.Post("/:lid/unlock", lfs.UnLockHandler) - }, context.RepoAssignment()) + }) m.Any("/*", func(ctx *context.Context) { ctx.NotFound("", nil) })