From 17c5c654a57ecf51c8c7c8ecfc6c86ae313d4000 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 15 May 2021 16:32:09 +0100 Subject: [PATCH] Prevent double-login for Git HTTP and LFS and simplify login (#15303) * Prevent double-login for Git HTTP and LFS and simplify login There are a number of inconsistencies with our current methods for logging in for git and lfs. The first is that there is a double login process. This is particularly evident in 1.13 where there are no less than 4 hash checks for basic authentication due to the previous IsPasswordSet behaviour. This duplicated code had individual inconsistencies that were not helpful and caused confusion. This PR does the following: * Remove the specific login code from the git and lfs handlers except for the lfs special bearer token * Simplify the meaning of DisableBasicAuthentication to allow Token and Oauth2 sign-in. * The removal of the specific code from git and lfs means that these both now have the same login semantics and can - if not DisableBasicAuthentication - login from external services. Further it allows Oauth2 token authentication as per our standard mechanisms. * The change in the recovery handler prevents the service from re-attempting to login - primarily because this could easily cause a further panic and it is wasteful. * add test Signed-off-by: Andrew Thornton Co-authored-by: Andrew Thornton --- integrations/api_repo_lfs_locks_test.go | 2 +- modules/auth/sso/basic.go | 45 +++++---- modules/auth/sso/sso.go | 17 +++- modules/auth/sso/sso_test.go | 124 +++++++++++++++++++++++ modules/context/context.go | 3 + routers/private/internal.go | 2 +- routers/repo/http.go | 129 +++++------------------- routers/routes/base.go | 16 ++- services/lfs/locks.go | 48 +++++---- services/lfs/server.go | 127 ++++++++++------------- 10 files changed, 292 insertions(+), 221 deletions(-) create mode 100644 modules/auth/sso/sso_test.go diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 69981d1c42..ffc239567d 100644 --- a/integrations/api_repo_lfs_locks_test.go +++ b/integrations/api_repo_lfs_locks_test.go @@ -44,7 +44,7 @@ func TestAPILFSLocksNotLogin(t *testing.T) { resp := MakeRequest(t, req, http.StatusUnauthorized) var lfsLockError api.LFSLockError DecodeJSON(t, resp, &lfsLockError) - assert.Equal(t, "Unauthorized", lfsLockError.Message) + assert.Equal(t, "You must have pull access to list locks", lfsLockError.Message) } func TestAPILFSLocksLogged(t *testing.T) { diff --git a/modules/auth/sso/basic.go b/modules/auth/sso/basic.go index d4ac8f8089..a18e127ff9 100644 --- a/modules/auth/sso/basic.go +++ b/modules/auth/sso/basic.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/web/middleware" ) // Ensure the struct implements the interface. @@ -40,7 +41,7 @@ func (b *Basic) Free() error { // IsEnabled returns true as this plugin is enabled by default and its not possible to disable // it from settings. func (b *Basic) IsEnabled() bool { - return setting.Service.EnableBasicAuth + return true } // VerifyAuthData extracts and validates Basic data (username and password/token) from the @@ -48,17 +49,22 @@ func (b *Basic) IsEnabled() bool { // name/token on successful validation. // Returns nil if header is empty or validation fails. func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *models.User { + + // Basic authentication should only fire on API, Download or on Git or LFSPaths + if middleware.IsInternalPath(req) || !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isGitOrLFSPath(req) { + return nil + } + baHead := req.Header.Get("Authorization") if len(baHead) == 0 { return nil } - auths := strings.Fields(baHead) + auths := strings.SplitN(baHead, " ", 2) if len(auths) != 2 || (auths[0] != "Basic" && auths[0] != "basic") { return nil } - var u *models.User uname, passwd, _ := base.BasicAuthDecode(auths[1]) // Check if username or password is a token @@ -76,20 +82,21 @@ func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store D uid := CheckOAuthAccessToken(authToken) if uid != 0 { log.Trace("Basic Authorization: Valid OAuthAccessToken for user[%d]", uid) - var err error - store.GetData()["IsApiToken"] = true - u, err = models.GetUserByID(uid) + u, err := models.GetUserByID(uid) if err != nil { log.Error("GetUserByID: %v", err) return nil } + + store.GetData()["IsApiToken"] = true + return u } + token, err := models.GetAccessTokenBySHA(authToken) if err == nil { log.Trace("Basic Authorization: Valid AccessToken for user[%d]", uid) - - u, err = models.GetUserByID(token.UID) + u, err := models.GetUserByID(token.UID) if err != nil { log.Error("GetUserByID: %v", err) return nil @@ -99,22 +106,24 @@ func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store D if err = models.UpdateAccessToken(token); err != nil { log.Error("UpdateAccessToken: %v", err) } + + store.GetData()["IsApiToken"] = true + return u } else if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { log.Error("GetAccessTokenBySha: %v", err) } - if u == nil { - log.Trace("Basic Authorization: Attempting SignIn for %s", uname) + if !setting.Service.EnableBasicAuth { + return nil + } - u, err = models.UserSignIn(uname, passwd) - if err != nil { - if !models.IsErrUserNotExist(err) { - log.Error("UserSignIn: %v", err) - } - return nil + log.Trace("Basic Authorization: Attempting SignIn for %s", uname) + u, err := models.UserSignIn(uname, passwd) + if err != nil { + if !models.IsErrUserNotExist(err) { + log.Error("UserSignIn: %v", err) } - } else { - store.GetData()["IsApiToken"] = true + return nil } log.Trace("Basic Authorization: Logged in user %-v", u) diff --git a/modules/auth/sso/sso.go b/modules/auth/sso/sso.go index 8785a5f068..2f949cb0f8 100644 --- a/modules/auth/sso/sso.go +++ b/modules/auth/sso/sso.go @@ -9,10 +9,12 @@ import ( "fmt" "net/http" "reflect" + "regexp" "strings" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web/middleware" ) @@ -27,9 +29,9 @@ import ( // for users that have already signed in. var ssoMethods = []SingleSignOn{ &OAuth2{}, + &Basic{}, &Session{}, &ReverseProxy{}, - &Basic{}, } // The purpose of the following three function variables is to let the linter know that @@ -102,6 +104,19 @@ func isAttachmentDownload(req *http.Request) bool { return strings.HasPrefix(req.URL.Path, "/attachments/") && req.Method == "GET" } +var gitPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/(?:(?:git-(?:(?:upload)|(?:receive))-pack$)|(?:info/refs$)|(?:HEAD$)|(?:objects/))`) +var lfsPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/info/lfs/`) + +func isGitOrLFSPath(req *http.Request) bool { + if gitPathRe.MatchString(req.URL.Path) { + return true + } + if setting.LFS.StartServer { + return lfsPathRe.MatchString(req.URL.Path) + } + return false +} + // handleSignIn clears existing session variables and stores new ones for the specified user object func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore, user *models.User) { _ = sess.Delete("openid_verified_uri") diff --git a/modules/auth/sso/sso_test.go b/modules/auth/sso/sso_test.go new file mode 100644 index 0000000000..b6a7f099e3 --- /dev/null +++ b/modules/auth/sso/sso_test.go @@ -0,0 +1,124 @@ +// Copyright 2014 The Gogs Authors. All rights reserved. +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package sso + +import ( + "net/http" + "testing" + + "code.gitea.io/gitea/modules/setting" +) + +func Test_isGitOrLFSPath(t *testing.T) { + + tests := []struct { + path string + + want bool + }{ + { + "/owner/repo/git-upload-pack", + true, + }, + { + "/owner/repo/git-receive-pack", + true, + }, + { + "/owner/repo/info/refs", + true, + }, + { + "/owner/repo/HEAD", + true, + }, + { + "/owner/repo/objects/info/alternates", + true, + }, + { + "/owner/repo/objects/info/http-alternates", + true, + }, + { + "/owner/repo/objects/info/packs", + true, + }, + { + "/owner/repo/objects/info/blahahsdhsdkla", + true, + }, + { + "/owner/repo/objects/01/23456789abcdef0123456789abcdef01234567", + true, + }, + { + "/owner/repo/objects/pack/pack-123456789012345678921234567893124567894.pack", + true, + }, + { + "/owner/repo/objects/pack/pack-0123456789abcdef0123456789abcdef0123456.idx", + true, + }, + { + "/owner/repo/stars", + false, + }, + { + "/notowner", + false, + }, + { + "/owner/repo", + false, + }, + { + "/owner/repo/commit/123456789012345678921234567893124567894", + false, + }, + } + lfsTests := []string{ + "/owner/repo/info/lfs/", + "/owner/repo/info/lfs/objects/batch", + "/owner/repo/info/lfs/objects/oid/filename", + "/owner/repo/info/lfs/objects/oid", + "/owner/repo/info/lfs/objects", + "/owner/repo/info/lfs/verify", + "/owner/repo/info/lfs/locks", + "/owner/repo/info/lfs/locks/verify", + "/owner/repo/info/lfs/locks/123/unlock", + } + + origLFSStartServer := setting.LFS.StartServer + + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + req, _ := http.NewRequest("POST", "http://localhost"+tt.path, nil) + setting.LFS.StartServer = false + if got := isGitOrLFSPath(req); got != tt.want { + t.Errorf("isGitOrLFSPath() = %v, want %v", got, tt.want) + } + setting.LFS.StartServer = true + if got := isGitOrLFSPath(req); got != tt.want { + t.Errorf("isGitOrLFSPath() = %v, want %v", got, tt.want) + } + }) + } + for _, tt := range lfsTests { + t.Run(tt, func(t *testing.T) { + req, _ := http.NewRequest("POST", tt, nil) + setting.LFS.StartServer = false + if got := isGitOrLFSPath(req); got != setting.LFS.StartServer { + t.Errorf("isGitOrLFSPath(%q) = %v, want %v, %v", tt, got, setting.LFS.StartServer, gitPathRe.MatchString(tt)) + } + setting.LFS.StartServer = true + if got := isGitOrLFSPath(req); got != setting.LFS.StartServer { + t.Errorf("isGitOrLFSPath(%q) = %v, want %v", tt, got, setting.LFS.StartServer) + } + }) + } + setting.LFS.StartServer = origLFSStartServer +} diff --git a/modules/context/context.go b/modules/context/context.go index 750941b1d1..d812d7b58c 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -683,6 +683,9 @@ func Contexter() func(next http.Handler) http.Handler { } else { ctx.Data["SignedUserID"] = int64(0) ctx.Data["SignedUserName"] = "" + + // ensure the session uid is deleted + _ = ctx.Session.Delete("uid") } ctx.Resp.Header().Set(`X-Frame-Options`, `SAMEORIGIN`) diff --git a/routers/private/internal.go b/routers/private/internal.go index 15a393c530..c6cc61fc29 100644 --- a/routers/private/internal.go +++ b/routers/private/internal.go @@ -23,7 +23,7 @@ import ( func CheckInternalToken(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { tokens := req.Header.Get("Authorization") - fields := strings.Fields(tokens) + fields := strings.SplitN(tokens, " ", 2) if len(fields) != 2 || fields[0] != "Bearer" || fields[1] != setting.InternalToken { log.Debug("Forbidden attempt to access internal url: Authorization header: %s", tokens) http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) diff --git a/routers/repo/http.go b/routers/repo/http.go index e98f528f36..ef80f7ab02 100644 --- a/routers/repo/http.go +++ b/routers/repo/http.go @@ -22,15 +22,12 @@ import ( "time" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/auth/sso" - "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" repo_service "code.gitea.io/gitea/services/repository" ) @@ -153,11 +150,8 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { // Only public pull don't need auth. isPublicPull := repoExist && !repo.IsPrivate && isPull var ( - askAuth = !isPublicPull || setting.Service.RequireSignInView - authUser *models.User - authUsername string - authPasswd string - environ []string + askAuth = !isPublicPull || setting.Service.RequireSignInView + environ []string ) // don't allow anonymous pulls if organization is not public @@ -172,108 +166,33 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { // check access if askAuth { - authUsername = ctx.Req.Header.Get(setting.ReverseProxyAuthUser) - if setting.Service.EnableReverseProxyAuth && len(authUsername) > 0 { - authUser, err = models.GetUserByName(authUsername) - if err != nil { - ctx.HandleText(401, "reverse proxy login error, got error while running GetUserByName") - return - } - } else { - authHead := ctx.Req.Header.Get("Authorization") - if len(authHead) == 0 { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=\".\"") - ctx.Error(http.StatusUnauthorized) - return - } + // rely on the results of Contexter + if !ctx.IsSigned { + // TODO: support digit auth - which would be Authorization header with digit + ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=\".\"") + ctx.Error(http.StatusUnauthorized) + return + } - auths := strings.Fields(authHead) - // currently check basic auth - // TODO: support digit auth - // FIXME: middlewares/context.go did basic auth check already, - // maybe could use that one. - if len(auths) != 2 || auths[0] != "Basic" { - ctx.HandleText(http.StatusUnauthorized, "no basic auth and digit auth") - return - } - authUsername, authPasswd, err = base.BasicAuthDecode(auths[1]) - if err != nil { - ctx.HandleText(http.StatusUnauthorized, "no basic auth and digit auth") - return - } - - // Check if username or password is a token - isUsernameToken := len(authPasswd) == 0 || authPasswd == "x-oauth-basic" - // Assume username is token - authToken := authUsername - if !isUsernameToken { - // Assume password is token - authToken = authPasswd - } - uid := sso.CheckOAuthAccessToken(authToken) - if uid != 0 { - ctx.Data["IsApiToken"] = true - - authUser, err = models.GetUserByID(uid) - if err != nil { - ctx.ServerError("GetUserByID", err) - return - } - } - // Assume password is a token. - token, err := models.GetAccessTokenBySHA(authToken) + if ctx.IsBasicAuth { + _, err = models.GetTwoFactorByUID(ctx.User.ID) if err == nil { - authUser, err = models.GetUserByID(token.UID) - if err != nil { - ctx.ServerError("GetUserByID", err) - return - } - - token.UpdatedUnix = timeutil.TimeStampNow() - if err = models.UpdateAccessToken(token); err != nil { - ctx.ServerError("UpdateAccessToken", err) - } - } else if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { - log.Error("GetAccessTokenBySha: %v", err) - } - - if authUser == nil { - // Check username and password - authUser, err = models.UserSignIn(authUsername, authPasswd) - if err != nil { - if models.IsErrUserProhibitLogin(err) { - ctx.HandleText(http.StatusForbidden, "User is not permitted to login") - return - } else if !models.IsErrUserNotExist(err) { - ctx.ServerError("UserSignIn error: %v", err) - return - } - } - - if authUser == nil { - ctx.HandleText(http.StatusUnauthorized, fmt.Sprintf("invalid credentials from %s", ctx.RemoteAddr())) - return - } - - _, err = models.GetTwoFactorByUID(authUser.ID) - if err == nil { - // TODO: This response should be changed to "invalid credentials" for security reasons once the expectation behind it (creating an app token to authenticate) is properly documented - ctx.HandleText(http.StatusUnauthorized, "Users with two-factor authentication enabled cannot perform HTTP/HTTPS operations via plain username and password. Please create and use a personal access token on the user settings page") - return - } else if !models.IsErrTwoFactorNotEnrolled(err) { - ctx.ServerError("IsErrTwoFactorNotEnrolled", err) - return - } + // TODO: This response should be changed to "invalid credentials" for security reasons once the expectation behind it (creating an app token to authenticate) is properly documented + ctx.HandleText(http.StatusUnauthorized, "Users with two-factor authentication enabled cannot perform HTTP/HTTPS operations via plain username and password. Please create and use a personal access token on the user settings page") + return + } else if !models.IsErrTwoFactorNotEnrolled(err) { + ctx.ServerError("IsErrTwoFactorNotEnrolled", err) + return } } - if !authUser.IsActive || authUser.ProhibitLogin { + if !ctx.User.IsActive || ctx.User.ProhibitLogin { ctx.HandleText(http.StatusForbidden, "Your account is disabled.") return } if repoExist { - perm, err := models.GetUserRepoPermission(repo, authUser) + perm, err := models.GetUserRepoPermission(repo, ctx.User) if err != nil { ctx.ServerError("GetUserRepoPermission", err) return @@ -293,14 +212,14 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { environ = []string{ models.EnvRepoUsername + "=" + username, models.EnvRepoName + "=" + reponame, - models.EnvPusherName + "=" + authUser.Name, - models.EnvPusherID + fmt.Sprintf("=%d", authUser.ID), + models.EnvPusherName + "=" + ctx.User.Name, + models.EnvPusherID + fmt.Sprintf("=%d", ctx.User.ID), models.EnvIsDeployKey + "=false", models.EnvAppURL + "=" + setting.AppURL, } - if !authUser.KeepEmailPrivate { - environ = append(environ, models.EnvPusherEmail+"="+authUser.Email) + if !ctx.User.KeepEmailPrivate { + environ = append(environ, models.EnvPusherEmail+"="+ctx.User.Email) } if isWiki { @@ -336,7 +255,7 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { return } - repo, err = repo_service.PushCreateRepo(authUser, owner, reponame) + repo, err = repo_service.PushCreateRepo(ctx.User, owner, reponame) if err != nil { log.Error("pushCreateRepo: %v", err) ctx.Status(http.StatusNotFound) diff --git a/routers/routes/base.go b/routers/routes/base.go index 743582d4a5..0b784508a7 100644 --- a/routers/routes/base.go +++ b/routers/routes/base.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth/sso" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/httpcache" @@ -171,8 +172,19 @@ func Recovery() func(next http.Handler) http.Handler { }, } - // Get user from session if logged in. - user, _ := sso.SignedInUser(req, w, &store, sessionStore) + var user *models.User + if apiContext := context.GetAPIContext(req); apiContext != nil { + user = apiContext.User + } + if user == nil { + if ctx := context.GetContext(req); ctx != nil { + user = ctx.User + } + } + if user == nil { + // Get user from session if logged in - do not attempt to sign-in + user = sso.SessionUser(sessionStore) + } if user != nil { store.Data["IsSigned"] = true store.Data["SignedUser"] = user diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 6bbe43d36b..ad204c46e2 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -31,15 +31,6 @@ func checkIsValidRequest(ctx *context.Context) bool { writeStatus(ctx, http.StatusBadRequest) return false } - 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, http.StatusUnauthorized) - return false - } - ctx.User = user - } return true } @@ -73,19 +64,21 @@ func GetListLockHandler(ctx *context.Context) { // Status is written in checkIsValidRequest return } - ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) rv, _ := unpack(ctx) repository, err := models.GetRepositoryByOwnerAndName(rv.User, rv.Repo) if err != nil { log.Debug("Could not find repository: %s/%s - %s", rv.User, rv.Repo, err) - writeStatus(ctx, 404) + 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 } repository.MustOwner() - authenticated := authenticate(ctx, repository, rv.Authorization, false) + authenticated := authenticate(ctx, repository, rv.Authorization, true, false) if !authenticated { ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ @@ -93,6 +86,7 @@ func GetListLockHandler(ctx *context.Context) { }) return } + ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) cursor := ctx.QueryInt("cursor") if cursor < 0 { @@ -160,7 +154,6 @@ func PostLockHandler(ctx *context.Context) { // Status is written in checkIsValidRequest return } - ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) userName := ctx.Params("username") repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git") @@ -169,12 +162,15 @@ func PostLockHandler(ctx *context.Context) { repository, err := models.GetRepositoryByOwnerAndName(userName, repoName) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", userName, repoName, err) - writeStatus(ctx, 404) + 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 } repository.MustOwner() - authenticated := authenticate(ctx, repository, authorization, true) + authenticated := authenticate(ctx, repository, authorization, true, true) if !authenticated { ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ @@ -183,6 +179,8 @@ func PostLockHandler(ctx *context.Context) { return } + ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) + var req api.LFSLockRequest bodyReader := ctx.Req.Body defer bodyReader.Close() @@ -229,7 +227,6 @@ func VerifyLockHandler(ctx *context.Context) { // Status is written in checkIsValidRequest return } - ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) userName := ctx.Params("username") repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git") @@ -238,12 +235,15 @@ func VerifyLockHandler(ctx *context.Context) { repository, err := models.GetRepositoryByOwnerAndName(userName, repoName) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", userName, repoName, err) - writeStatus(ctx, 404) + 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 } repository.MustOwner() - authenticated := authenticate(ctx, repository, authorization, true) + authenticated := authenticate(ctx, repository, authorization, true, true) if !authenticated { ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ @@ -252,6 +252,8 @@ func VerifyLockHandler(ctx *context.Context) { return } + ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) + cursor := ctx.QueryInt("cursor") if cursor < 0 { cursor = 0 @@ -296,7 +298,6 @@ func UnLockHandler(ctx *context.Context) { // Status is written in checkIsValidRequest return } - ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) userName := ctx.Params("username") repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git") @@ -305,12 +306,15 @@ func UnLockHandler(ctx *context.Context) { repository, err := models.GetRepositoryByOwnerAndName(userName, repoName) if err != nil { log.Error("Unable to get repository: %s/%s Error: %v", userName, repoName, err) - writeStatus(ctx, 404) + 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 } repository.MustOwner() - authenticated := authenticate(ctx, repository, authorization, true) + authenticated := authenticate(ctx, repository, authorization, true, true) if !authenticated { ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs") ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ @@ -319,6 +323,8 @@ func UnLockHandler(ctx *context.Context) { return } + ctx.Resp.Header().Set("Content-Type", lfs_module.MediaType) + var req api.LFSLockDeleteRequest bodyReader := ctx.Req.Body defer bodyReader.Close() diff --git a/services/lfs/server.go b/services/lfs/server.go index cd9a3fd7a1..ee7d3bc79a 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -94,7 +94,7 @@ func getAuthenticatedRepoAndMeta(ctx *context.Context, rc *requestContext, p lfs return nil, nil } - if !authenticate(ctx, repository, rc.Authorization, requireWrite) { + if !authenticate(ctx, repository, rc.Authorization, false, requireWrite) { requireAuth(ctx) return nil, nil } @@ -232,7 +232,7 @@ func PostHandler(ctx *context.Context) { return } - if !authenticate(ctx, repository, rc.Authorization, true) { + if !authenticate(ctx, repository, rc.Authorization, false, true) { requireAuth(ctx) return } @@ -322,7 +322,7 @@ func BatchHandler(ctx *context.Context) { requireWrite = true } - if !authenticate(ctx, repository, reqCtx.Authorization, requireWrite) { + if !authenticate(ctx, repository, reqCtx.Authorization, false, requireWrite) { requireAuth(ctx) return } @@ -561,7 +561,7 @@ func logRequest(r *http.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 { +func authenticate(ctx *context.Context, repository *models.Repository, authorization string, requireSigned, requireWrite bool) bool { accessMode := models.AccessModeRead if requireWrite { accessMode = models.AccessModeWrite @@ -575,89 +575,72 @@ func authenticate(ctx *context.Context, repository *models.Repository, authoriza } canRead := perm.CanAccess(accessMode, models.UnitTypeCode) - if canRead { + if canRead && (!requireSigned || ctx.IsSigned) { return true } - user, repo, opStr, err := parseToken(authorization) + user, err := parseToken(authorization, repository, accessMode) if err != nil { // Most of these are Warn level - the true internal server errors are logged in parseToken already log.Warn("Authentication failure for provided token with Error: %v", err) return false } ctx.User = user - if opStr == "basic" { - perm, err = models.GetUserRepoPermission(repository, ctx.User) - if err != nil { - log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", ctx.User, repository) - return false - } - return perm.CanAccess(accessMode, models.UnitTypeCode) - } - if repository.ID == repo.ID { - if requireWrite && opStr != "upload" { - return false - } - return true - } - return false + return true } -func parseToken(authorization string) (*models.User, *models.Repository, string, error) { +func handleLFSToken(tokenSHA string, target *models.Repository, mode models.AccessMode) (*models.User, error) { + if !strings.Contains(tokenSHA, ".") { + return nil, nil + } + token, err := jwt.ParseWithClaims(tokenSHA, &Claims{}, 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 + } + + claims, claimsOk := token.Claims.(*Claims) + if !token.Valid || !claimsOk { + return nil, fmt.Errorf("invalid token claim") + } + + if claims.RepoID != target.ID { + return nil, fmt.Errorf("invalid token claim") + } + + if mode == models.AccessModeWrite && claims.Op != "upload" { + return nil, fmt.Errorf("invalid token claim") + } + + u, err := models.GetUserByID(claims.UserID) + if err != nil { + log.Error("Unable to GetUserById[%d]: Error: %v", claims.UserID, err) + return nil, err + } + return u, nil +} + +func parseToken(authorization string, target *models.Repository, mode models.AccessMode) (*models.User, error) { if authorization == "" { - return nil, nil, "unknown", fmt.Errorf("No token") - } - if strings.HasPrefix(authorization, "Bearer ") { - token, err := jwt.ParseWithClaims(authorization[7:], &Claims{}, 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 { - // The error here is WARN level because it is caused by bad authorization rather than an internal server error - return nil, nil, "unknown", err - } - claims, claimsOk := token.Claims.(*Claims) - if !token.Valid || !claimsOk { - return nil, nil, "unknown", fmt.Errorf("Token claim invalid") - } - r, err := models.GetRepositoryByID(claims.RepoID) - if err != nil { - log.Error("Unable to GetRepositoryById[%d]: Error: %v", claims.RepoID, err) - return nil, nil, claims.Op, err - } - u, err := models.GetUserByID(claims.UserID) - if err != nil { - log.Error("Unable to GetUserById[%d]: Error: %v", claims.UserID, err) - return nil, r, claims.Op, err - } - return u, r, claims.Op, nil + return nil, fmt.Errorf("no token") } - 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 { - log.Error("Unable to GetUserByName[%d]: Error: %v", user, err) - return nil, nil, "basic", err - } - if !u.IsPasswordSet() || !u.ValidatePassword(password) { - return nil, nil, "basic", fmt.Errorf("Basic auth failed") - } - return u, nil, "basic", nil + parts := strings.SplitN(authorization, " ", 2) + if len(parts) != 2 { + return nil, fmt.Errorf("no token") } - - return nil, nil, "unknown", fmt.Errorf("Token not found") + tokenSHA := parts[1] + switch strings.ToLower(parts[0]) { + case "bearer": + fallthrough + case "token": + return handleLFSToken(tokenSHA, target, mode) + } + return nil, fmt.Errorf("token not found") } func requireAuth(ctx *context.Context) {