Fix LFS Locks over SSH (#6999)

* Fix LFS Locks over SSH
* Mark test as skipped

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))
+			if !testing.Short() {
-			resp = session.MakeRequest(t, req, http.StatusOK)
+				req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/raw/branch/master/", big))
-			assert.NotEqual(t, bigSize, resp.Body.Len())
+				nilResp := session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
-			assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
+				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))
+			if !testing.Short() {
-			resp = session.MakeRequestNilResponseRecorder(t, req, http.StatusOK)
+				req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-17/media/branch/master/", big))
-			assert.Equal(t, bigSize, resp.Length)
+				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))
+				if !testing.Short() {
-				resp = session.MakeRequest(t, req, http.StatusOK)
+					req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/raw/branch/master/", big))
-				assert.NotEqual(t, bigSize, resp.Body.Len())
+					resp = session.MakeRequest(t, req, http.StatusOK)
-				assert.Contains(t, resp.Body.String(), models.LFSMetaFileIdentifier)
+					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))
+				if !testing.Short() {
-				resp = session.MakeRequest(t, req, http.StatusOK)
+					req = NewRequest(t, "GET", path.Join("/user2/repo-tmp-18/media/branch/master/", big))
-				assert.Equal(t, bigSize, resp.Body.Len())
+					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
+	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", filename).RunInDir(repoPath)
 	assert.NoError(t, err)
 	_, 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("unlock", 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)
 	assert.NoError(t, err)
 }
 

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) {
+		log.Debug("Could not find repository: %s/%s - %s", rv.User, rv.Repo, err)
-			ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
+		writeStatus(ctx, 404)
-			ctx.JSON(401, api.LFSLockError{
+		return
-				Message: "You must have pull access to list locks : " + err.Error(),
+	}
-			})
+	repository.MustOwner()
-			return
+
-		}
+	authenticated := authenticate(ctx, repository, rv.Authorization, false)
-		ctx.JSON(500, api.LFSLockError{
+	if !authenticated {
-			Message: "unable to list lock : " + err.Error(),
+		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)
+		lock, err := models.GetLFSLock(repository, path)
-		handleLockListOut(ctx, lock, err)
+		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 := dec.Decode(&req); err != nil {
-	if 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) {
+		log.Debug("Could not find repository: %s/%s - %s", userName, repoName, err)
-			ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
+		writeStatus(ctx, 404)
-			ctx.JSON(401, api.LFSLockError{
+		return
-				Message: "You must have push access to verify locks : " + err.Error(),
+	}
-			})
+	repository.MustOwner()
-			return
+
-		}
+	authenticated := authenticate(ctx, repository, authorization, true)
-		ctx.JSON(500, api.LFSLockError{
+	if !authenticated {
-			Message: "unable to verify lock : " + err.Error(),
+		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 := dec.Decode(&req); err != nil {
-	if err != nil {
 		writeStatus(ctx, 400)
 		return
 	}

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)
 				})