Fix SSH auth lfs locks (#3152)

* Fix SSH auth LFS locks

* Activate SSH/lock test

* Remove debug

* Follow @lunny recommendation for AfterLoad method

2 files changed, 111 insertions, 100 deletions

diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go
index 2e776c26a0..7a7d3ad42a 100644
--- a/modules/lfs/locks.go
+++ b/modules/lfs/locks.go
@@ -13,24 +13,35 @@ import (
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/sdk/gitea"
-
-	"gopkg.in/macaron.v1"
 )
 
-func checkRequest(req macaron.Request, post bool) int {
+//checkIsValidRequest check if it a valid request in case of bad request it write the response to ctx.
+func checkIsValidRequest(ctx *context.Context, post bool) bool {
 	if !setting.LFS.StartServer {
-		return 404
+		writeStatus(ctx, 404)
+		return false
+	}
+	if !MetaMatcher(ctx.Req) {
+		writeStatus(ctx, 400)
+		return false
 	}
-	if !MetaMatcher(req) {
-		return 400
+	if !ctx.IsSigned {
+		user, _, _, err := parseToken(ctx.Req.Header.Get("Authorization"))
+		if err != nil {
+			ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
+			writeStatus(ctx, 401)
+			return false
+		}
+		ctx.User = user
 	}
 	if post {
-		mediaParts := strings.Split(req.Header.Get("Content-Type"), ";")
+		mediaParts := strings.Split(ctx.Req.Header.Get("Content-Type"), ";")
 		if mediaParts[0] != metaMediaType {
-			return 400
+			writeStatus(ctx, 400)
+			return false
 		}
 	}
-	return 200
+	return true
 }
 
 func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) {
@@ -59,17 +70,16 @@ func handleLockListOut(ctx *context.Context, lock *models.LFSLock, err error) {
 
 // GetListLockHandler list locks
 func GetListLockHandler(ctx *context.Context) {
-	status := checkRequest(ctx.Req, false)
-	if status != 200 {
-		writeStatus(ctx, status)
+	if !checkIsValidRequest(ctx, false) {
 		return
 	}
 	ctx.Resp.Header().Set("Content-Type", metaMediaType)
 
-	err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository.ID, "list")
+	err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository, models.AccessModeRead)
 	if err != nil {
-		if models.IsErrLFSLockUnauthorizedAction(err) {
-			ctx.JSON(403, api.LFSLockError{
+		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
@@ -96,7 +106,7 @@ func GetListLockHandler(ctx *context.Context) {
 
 	path := ctx.Query("path")
 	if path != "" { //Case where we request a specific id
-		lock, err := models.GetLFSLock(ctx.Repo.Repository.ID, path)
+		lock, err := models.GetLFSLock(ctx.Repo.Repository, path)
 		handleLockListOut(ctx, lock, err)
 		return
 	}
@@ -120,9 +130,7 @@ func GetListLockHandler(ctx *context.Context) {
 
 // PostLockHandler create lock
 func PostLockHandler(ctx *context.Context) {
-	status := checkRequest(ctx.Req, true)
-	if status != 200 {
-		writeStatus(ctx, status)
+	if !checkIsValidRequest(ctx, false) {
 		return
 	}
 	ctx.Resp.Header().Set("Content-Type", metaMediaType)
@@ -136,9 +144,9 @@ func PostLockHandler(ctx *context.Context) {
 	}
 
 	lock, err := models.CreateLFSLock(&models.LFSLock{
-		RepoID: ctx.Repo.Repository.ID,
-		Path:   req.Path,
-		Owner:  ctx.User,
+		Repo:  ctx.Repo.Repository,
+		Path:  req.Path,
+		Owner: ctx.User,
 	})
 	if err != nil {
 		if models.IsErrLFSLockAlreadyExist(err) {
@@ -148,8 +156,9 @@ func PostLockHandler(ctx *context.Context) {
 			})
 			return
 		}
-		if models.IsErrLFSLockUnauthorizedAction(err) {
-			ctx.JSON(403, api.LFSLockError{
+		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 create locks : " + err.Error(),
 			})
 			return
@@ -164,18 +173,16 @@ func PostLockHandler(ctx *context.Context) {
 
 // VerifyLockHandler list locks for verification
 func VerifyLockHandler(ctx *context.Context) {
-	status := checkRequest(ctx.Req, true)
-	if status != 200 {
-		writeStatus(ctx, status)
+	if !checkIsValidRequest(ctx, false) {
 		return
 	}
-
 	ctx.Resp.Header().Set("Content-Type", metaMediaType)
 
-	err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository.ID, "verify")
+	err := models.CheckLFSAccessForRepo(ctx.User, ctx.Repo.Repository, models.AccessModeWrite)
 	if err != nil {
-		if models.IsErrLFSLockUnauthorizedAction(err) {
-			ctx.JSON(403, api.LFSLockError{
+		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
@@ -211,9 +218,7 @@ func VerifyLockHandler(ctx *context.Context) {
 
 // UnLockHandler delete locks
 func UnLockHandler(ctx *context.Context) {
-	status := checkRequest(ctx.Req, true)
-	if status != 200 {
-		writeStatus(ctx, status)
+	if !checkIsValidRequest(ctx, false) {
 		return
 	}
 	ctx.Resp.Header().Set("Content-Type", metaMediaType)
@@ -228,8 +233,9 @@ func UnLockHandler(ctx *context.Context) {
 
 	lock, err := models.DeleteLFSLockByID(ctx.ParamsInt64("lid"), ctx.User, req.Force)
 	if err != nil {
-		if models.IsErrLFSLockUnauthorizedAction(err) {
-			ctx.JSON(403, api.LFSLockError{
+		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 delete locks : " + err.Error(),
 			})
 			return
diff --git a/modules/lfs/server.go b/modules/lfs/server.go
index 329d6f00c4..a81d8e5c91 100644
--- a/modules/lfs/server.go
+++ b/modules/lfs/server.go
@@ -473,7 +473,6 @@ func logRequest(r macaron.Request, status int) {
 // authenticate uses the authorization string to determine whether
 // or not to proceed. This server assumes an HTTP Basic auth format.
 func authenticate(ctx *context.Context, repository *models.Repository, authorization string, requireWrite bool) bool {
-
 	accessMode := models.AccessModeRead
 	if requireWrite {
 		accessMode = models.AccessModeWrite
@@ -482,86 +481,92 @@ func authenticate(ctx *context.Context, repository *models.Repository, authoriza
 	if !repository.IsPrivate && !requireWrite {
 		return true
 	}
-
 	if ctx.IsSigned {
 		accessCheck, _ := models.HasAccess(ctx.User.ID, repository, accessMode)
 		return accessCheck
 	}
 
-	if authorization == "" {
-		return false
-	}
-
-	if authenticateToken(repository, authorization, requireWrite) {
-		return true
-	}
-
-	if !strings.HasPrefix(authorization, "Basic ") {
-		return false
-	}
-
-	c, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(authorization, "Basic "))
+	user, repo, opStr, err := parseToken(authorization)
 	if err != nil {
 		return false
 	}
-	cs := string(c)
-	i := strings.IndexByte(cs, ':')
-	if i < 0 {
-		return false
-	}
-	user, password := cs[:i], cs[i+1:]
-
-	userModel, err := models.GetUserByName(user)
-	if err != nil {
-		return false
+	ctx.User = user
+	if opStr == "basic" {
+		accessCheck, _ := models.HasAccess(ctx.User.ID, repository, accessMode)
+		return accessCheck
 	}
-
-	if !userModel.ValidatePassword(password) {
-		return false
+	if repository.ID == repo.ID {
+		if requireWrite && opStr != "upload" {
+			return false
+		}
+		return true
 	}
-
-	accessCheck, _ := models.HasAccess(userModel.ID, repository, accessMode)
-	return accessCheck
+	return false
 }
 
-func authenticateToken(repository *models.Repository, authorization string, requireWrite bool) bool {
-	if !strings.HasPrefix(authorization, "Bearer ") {
-		return false
-	}
-
-	token, err := jwt.Parse(authorization[7:], func(t *jwt.Token) (interface{}, error) {
-		if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok {
-			return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"])
+func parseToken(authorization string) (*models.User, *models.Repository, string, error) {
+	if authorization == "" {
+		return nil, nil, "unknown", fmt.Errorf("No token")
+	}
+	if strings.HasPrefix(authorization, "Bearer ") {
+		token, err := jwt.Parse(authorization[7:], func(t *jwt.Token) (interface{}, error) {
+			if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok {
+				return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"])
+			}
+			return setting.LFS.JWTSecretBytes, nil
+		})
+		if err != nil {
+			return nil, nil, "unknown", err
 		}
-		return setting.LFS.JWTSecretBytes, nil
-	})
-	if err != nil {
-		return false
-	}
-	claims, claimsOk := token.Claims.(jwt.MapClaims)
-	if !token.Valid || !claimsOk {
-		return false
-	}
-
-	opStr, ok := claims["op"].(string)
-	if !ok {
-		return false
-	}
-
-	if requireWrite && opStr != "upload" {
-		return false
-	}
-
-	repoID, ok := claims["repo"].(float64)
-	if !ok {
-		return false
+		claims, claimsOk := token.Claims.(jwt.MapClaims)
+		if !token.Valid || !claimsOk {
+			return nil, nil, "unknown", fmt.Errorf("Token claim invalid")
+		}
+		opStr, ok := claims["op"].(string)
+		if !ok {
+			return nil, nil, "unknown", fmt.Errorf("Token operation invalid")
+		}
+		repoID, ok := claims["repo"].(float64)
+		if !ok {
+			return nil, nil, opStr, fmt.Errorf("Token repository id invalid")
+		}
+		r, err := models.GetRepositoryByID(int64(repoID))
+		if err != nil {
+			return nil, nil, opStr, err
+		}
+		userID, ok := claims["user"].(float64)
+		if !ok {
+			return nil, r, opStr, fmt.Errorf("Token user id invalid")
+		}
+		u, err := models.GetUserByID(int64(userID))
+		if err != nil {
+			return nil, r, opStr, err
+		}
+		return u, r, opStr, nil
 	}
 
-	if repository.ID != int64(repoID) {
-		return false
+	if strings.HasPrefix(authorization, "Basic ") {
+		c, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(authorization, "Basic "))
+		if err != nil {
+			return nil, nil, "basic", err
+		}
+		cs := string(c)
+		i := strings.IndexByte(cs, ':')
+		if i < 0 {
+			return nil, nil, "basic", fmt.Errorf("Basic auth invalid")
+		}
+		user, password := cs[:i], cs[i+1:]
+		u, err := models.GetUserByName(user)
+		if err != nil {
+			return nil, nil, "basic", err
+		}
+		if !u.ValidatePassword(password) {
+			return nil, nil, "basic", fmt.Errorf("Basic auth failed")
+		}
+		return u, nil, "basic", nil
 	}
 
-	return true
+	return nil, nil, "unknown", fmt.Errorf("Token not found")
 }
 
 func requireAuth(ctx *context.Context) {