From 96e6dadde8aade35f5d18d43d4c63b88f279fbf7 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 20 Mar 2021 14:48:09 +0000 Subject: [PATCH 01/19] Add authentication type information Signed-off-by: Andrew Thornton --- modules/auth/sso/basic.go | 20 +++++++++++--------- modules/auth/sso/oauth2.go | 2 ++ modules/auth/sso/reverseproxy.go | 2 ++ modules/auth/sso/sso.go | 10 ++++++++++ modules/auth/sso/sspi_windows.go | 1 + 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/modules/auth/sso/basic.go b/modules/auth/sso/basic.go index d2d25c6cece65..a2168e64dd86d 100644 --- a/modules/auth/sso/basic.go +++ b/modules/auth/sso/basic.go @@ -74,13 +74,16 @@ func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store D if uid != 0 { var err error store.GetData()["IsApiToken"] = true + store.GetData()["AuthenticationMechanism"] = OAuth2Mechanism u, err = models.GetUserByID(uid) if err != nil { log.Error("GetUserByID: %v", err) return nil } + return u } + token, err := models.GetAccessTokenBySHA(authToken) if err == nil { u, err = models.GetUserByID(token.UID) @@ -93,21 +96,20 @@ 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()["AuthenticationMechanism"] = TokenMechanism + return u } else if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { log.Error("GetAccessTokenBySha: %v", err) } - if u == nil { - u, err = models.UserSignIn(uname, passwd) - if err != nil { - if !models.IsErrUserNotExist(err) { - log.Error("UserSignIn: %v", err) - } - return nil + 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 } + store.GetData()["AuthenticationMechanism"] = BasicAuthenticationMechanism return u } diff --git a/modules/auth/sso/oauth2.go b/modules/auth/sso/oauth2.go index fcd6845b38cc4..e39027f720d2b 100644 --- a/modules/auth/sso/oauth2.go +++ b/modules/auth/sso/oauth2.go @@ -89,6 +89,7 @@ func (o *OAuth2) userIDFromToken(req *http.Request, store DataStore) int64 { uid := CheckOAuthAccessToken(tokenSHA) if uid != 0 { store.GetData()["IsApiToken"] = true + store.GetData()["AuthenticationMechanism"] = OAuth2Mechanism } return uid } @@ -104,6 +105,7 @@ func (o *OAuth2) userIDFromToken(req *http.Request, store DataStore) int64 { log.Error("UpdateAccessToken: %v", err) } store.GetData()["IsApiToken"] = true + store.GetData()["AuthenticationMechanism"] = TokenMechanism return t.UID } diff --git a/modules/auth/sso/reverseproxy.go b/modules/auth/sso/reverseproxy.go index ca9450e71429d..9525128e0a82b 100644 --- a/modules/auth/sso/reverseproxy.go +++ b/modules/auth/sso/reverseproxy.go @@ -66,6 +66,8 @@ func (r *ReverseProxy) VerifyAuthData(req *http.Request, w http.ResponseWriter, return nil } + store.GetData()["AuthenticationMechanism"] = ReverseProxyMechanism + user, err := models.GetUserByName(username) if err != nil { if models.IsErrUserNotExist(err) && r.isAutoRegisterAllowed() { diff --git a/modules/auth/sso/sso.go b/modules/auth/sso/sso.go index e670f1a8a7193..82567e2d81cf5 100644 --- a/modules/auth/sso/sso.go +++ b/modules/auth/sso/sso.go @@ -32,6 +32,16 @@ var ssoMethods = []SingleSignOn{ &Basic{}, } +type AuthenticationMechanism string + +const ( + OAuth2Mechanism AuthenticationMechanism = "OAuth2" + TokenMechanism AuthenticationMechanism = "Token" + ReverseProxyMechanism AuthenticationMechanism = "ReverseProxy" + BasicAuthenticationMechanism AuthenticationMechanism = "Basic" + SSPIMechanism AuthenticationMechanism = "SSPI" +) + // The purpose of the following three function variables is to let the linter know that // those functions are not dead code and are actually being used var ( diff --git a/modules/auth/sso/sspi_windows.go b/modules/auth/sso/sspi_windows.go index 46f7ad9d97a0a..151340ba772cd 100644 --- a/modules/auth/sso/sspi_windows.go +++ b/modules/auth/sso/sspi_windows.go @@ -117,6 +117,7 @@ func (s *SSPI) VerifyAuthData(req *http.Request, w http.ResponseWriter, store Da return nil } log.Info("Authenticated as %s\n", username) + store.GetData()["AuthenticationMechanism"] = SSPIMechanism user, err := models.GetUserByName(username) if err != nil { From 073a5e970ff8e125d6cccc82fb8df6e114b93d01 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 28 Mar 2021 17:53:08 +0100 Subject: [PATCH 02/19] standardise LFS basic authentication Signed-off-by: Andrew Thornton --- modules/auth/sso/basic.go | 16 +-- modules/auth/sso/oauth2.go | 2 +- modules/lfs/locks.go | 40 ++++---- modules/lfs/server.go | 200 +++++++++++++++++++++++++++---------- 4 files changed, 179 insertions(+), 79 deletions(-) diff --git a/modules/auth/sso/basic.go b/modules/auth/sso/basic.go index a2168e64dd86d..6d1c023a79a5e 100644 --- a/modules/auth/sso/basic.go +++ b/modules/auth/sso/basic.go @@ -40,7 +40,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 @@ -53,12 +53,11 @@ func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store D 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 @@ -72,11 +71,10 @@ func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store D uid := CheckOAuthAccessToken(authToken) if uid != 0 { - var err error store.GetData()["IsApiToken"] = true store.GetData()["AuthenticationMechanism"] = OAuth2Mechanism - u, err = models.GetUserByID(uid) + u, err := models.GetUserByID(uid) if err != nil { log.Error("GetUserByID: %v", err) return nil @@ -86,7 +84,7 @@ func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store D token, err := models.GetAccessTokenBySHA(authToken) if err == nil { - u, err = models.GetUserByID(token.UID) + u, err := models.GetUserByID(token.UID) if err != nil { log.Error("GetUserByID: %v", err) return nil @@ -102,7 +100,11 @@ func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store D log.Error("GetAccessTokenBySha: %v", err) } - u, err = models.UserSignIn(uname, passwd) + if !setting.Service.EnableBasicAuth { + return nil + } + + u, err := models.UserSignIn(uname, passwd) if err != nil { if !models.IsErrUserNotExist(err) { log.Error("UserSignIn: %v", err) diff --git a/modules/auth/sso/oauth2.go b/modules/auth/sso/oauth2.go index e39027f720d2b..617d1b732f2f7 100644 --- a/modules/auth/sso/oauth2.go +++ b/modules/auth/sso/oauth2.go @@ -74,7 +74,7 @@ func (o *OAuth2) userIDFromToken(req *http.Request, store DataStore) int64 { // Well, check with header again. auHead := req.Header.Get("Authorization") if len(auHead) > 0 { - auths := strings.Fields(auHead) + auths := strings.SplitN(auHead, " ", 2) if len(auths) == 2 && (auths[0] == "token" || strings.ToLower(auths[0]) == "bearer") { tokenSHA = auths[1] } diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index eaa8305cb4218..246ca7e89ca52 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -30,15 +30,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 } @@ -72,14 +63,16 @@ func GetListLockHandler(ctx *context.Context) { // Status is written in checkIsValidRequest return } - ctx.Resp.Header().Set("Content-Type", metaMediaType) 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() @@ -92,6 +85,7 @@ func GetListLockHandler(ctx *context.Context) { }) return } + ctx.Resp.Header().Set("Content-Type", metaMediaType) cursor := ctx.QueryInt("cursor") if cursor < 0 { @@ -159,7 +153,6 @@ func PostLockHandler(ctx *context.Context) { // Status is written in checkIsValidRequest return } - ctx.Resp.Header().Set("Content-Type", metaMediaType) userName := ctx.Params("username") repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git") @@ -168,7 +161,10 @@ 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() @@ -182,6 +178,8 @@ func PostLockHandler(ctx *context.Context) { return } + ctx.Resp.Header().Set("Content-Type", metaMediaType) + var req api.LFSLockRequest bodyReader := ctx.Req.Body defer bodyReader.Close() @@ -228,7 +226,6 @@ func VerifyLockHandler(ctx *context.Context) { // Status is written in checkIsValidRequest return } - ctx.Resp.Header().Set("Content-Type", metaMediaType) userName := ctx.Params("username") repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git") @@ -237,7 +234,10 @@ 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() @@ -251,6 +251,8 @@ func VerifyLockHandler(ctx *context.Context) { return } + ctx.Resp.Header().Set("Content-Type", metaMediaType) + cursor := ctx.QueryInt("cursor") if cursor < 0 { cursor = 0 @@ -295,7 +297,6 @@ func UnLockHandler(ctx *context.Context) { // Status is written in checkIsValidRequest return } - ctx.Resp.Header().Set("Content-Type", metaMediaType) userName := ctx.Params("username") repoName := strings.TrimSuffix(ctx.Params("reponame"), ".git") @@ -304,7 +305,10 @@ 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() @@ -318,6 +322,8 @@ func UnLockHandler(ctx *context.Context) { return } + ctx.Resp.Header().Set("Content-Type", metaMediaType) + var req api.LFSLockDeleteRequest bodyReader := ctx.Req.Body defer bodyReader.Close() diff --git a/modules/lfs/server.go b/modules/lfs/server.go index 45cba9d9b7512..b70a61cb0616a 100644 --- a/modules/lfs/server.go +++ b/modules/lfs/server.go @@ -16,10 +16,12 @@ import ( "time" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/timeutil" "github.com/dgrijalva/jwt-go" jsoniter "github.com/json-iterator/go" @@ -614,85 +616,175 @@ func authenticate(ctx *context.Context, repository *models.Repository, authoriza 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 true +} + +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 perm.CanAccess(accessMode, models.UnitTypeCode) + 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 } - if repository.ID == repo.ID { - if requireWrite && opStr != "upload" { - return false + return u, nil +} + +func handleOAuth2Token(tokenSHA string) (*models.User, error) { + if !strings.Contains(tokenSHA, ".") { + return nil, nil + } + token, err := models.ParseOAuth2Token(tokenSHA) + if err != nil { + if err.Error() == "invalid token" { + return nil, err } - return true + return nil, nil + } + if token.Type != models.TypeAccessToken || token.ExpiresAt < time.Now().Unix() || token.IssuedAt > time.Now().Unix() { + return nil, fmt.Errorf("invalid token claim") + } + grant, err := models.GetOAuth2GrantByID(token.GrantID) + if err != nil { + log.Error("Unable to GetOAuth2GrantByID[%d]: Error: %v", token.GrantID, err) + return nil, err + } + u, err := models.GetUserByID(grant.UserID) + if err != nil { + log.Error("Unable to GetUserById[%d]: Error: %v", grant.UserID, err) + return nil, err } - return false + + return u, nil +} + +func handleStandardToken(tokenSHA string) (*models.User, error) { + t, err := models.GetAccessTokenBySHA(tokenSHA) + if err != nil { + if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { + log.Error("GetAccessTokenBySHA: %v", err) + } + return nil, err + } + t.UpdatedUnix = timeutil.TimeStampNow() + if err = models.UpdateAccessToken(t); err != nil { + log.Error("UpdateAccessToken: %v", err) + } + u, err := models.GetUserByID(t.UID) + if err != nil { + log.Error("Unable to GetUserById[%d]: Error: %v", t.UID, err) + return nil, err + } + return u, nil } -func parseToken(authorization string) (*models.User, *models.Repository, string, error) { +func parseToken(authorization string, target *models.Repository, mode models.AccessMode) (*models.User, error) { if authorization == "" { - return nil, nil, "unknown", fmt.Errorf("No token") + return nil, 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 + + parts := strings.SplitN(authorization, " ", 2) + if len(parts) != 2 { + return nil, fmt.Errorf("no token") + } + tokenSHA := parts[1] + switch strings.ToLower(parts[0]) { + case "bearer": + fallthrough + case "token": + u, err := handleLFSToken(tokenSHA, target, mode) + if err != nil || u != nil { + return u, err } - claims, claimsOk := token.Claims.(*Claims) - if !token.Valid || !claimsOk { - return nil, nil, "unknown", fmt.Errorf("Token claim invalid") + u, err = handleOAuth2Token(tokenSHA) + if u == nil && err == nil { + u, err = handleStandardToken(tokenSHA) } - 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 + return nil, 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 + if u != nil { + perm, err := models.GetUserRepoPermission(target, u) + if err != nil { + log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", u, target) + return nil, err + } + if perm.CanAccess(mode, models.UnitTypeCode) { + return u, nil + } } - return u, r, claims.Op, nil - } - - if strings.HasPrefix(authorization, "Basic ") { - c, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(authorization, "Basic ")) - if err != nil { - return nil, nil, "basic", err + case "basic": + uname, passwd, _ := base.BasicAuthDecode(tokenSHA) + if len(uname) == 0 && len(passwd) == 0 { + return nil, fmt.Errorf("token not found") + } + // Check if username or password is a token + isUsernameToken := len(passwd) == 0 || passwd == "x-oauth-basic" + // Assume username is token + tokenSHA = uname + if !isUsernameToken { + // Assume password is token + tokenSHA = passwd } - cs := string(c) - i := strings.IndexByte(cs, ':') - if i < 0 { - return nil, nil, "basic", fmt.Errorf("Basic auth invalid") + u, err := handleOAuth2Token(tokenSHA) + if u == nil && err == nil { + u, err = handleStandardToken(tokenSHA) + } + if u == nil && err == nil { + if !setting.Service.EnableBasicAuth { + return nil, fmt.Errorf("token not found") + } + u, err = models.UserSignIn(uname, passwd) } - 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 !models.IsErrUserNotExist(err) { + log.Error("UserSignIn: %v", err) + } + return nil, fmt.Errorf("basic auth failed") } - if !u.IsPasswordSet() || !u.ValidatePassword(password) { - return nil, nil, "basic", fmt.Errorf("Basic auth failed") + if u != nil { + perm, err := models.GetUserRepoPermission(target, u) + if err != nil { + log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", u, target) + return nil, err + } + if perm.CanAccess(mode, models.UnitTypeCode) { + return u, nil + } } - return u, nil, "basic", nil } - - return nil, nil, "unknown", fmt.Errorf("Token not found") + return nil, fmt.Errorf("token not found") } func requireAuth(ctx *context.Context) { From 7119663258cea76905cda548fb0efce1e80d7db6 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 28 Mar 2021 18:31:53 +0100 Subject: [PATCH 03/19] use splitn Signed-off-by: Andrew Thornton --- routers/private/internal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/private/internal.go b/routers/private/internal.go index e541591a3840b..32272210dcf01 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) From 0fdaa3b6c627b4dd29aa0b146ecae486d6354b39 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 28 Mar 2021 18:33:43 +0100 Subject: [PATCH 04/19] standardise git http basic auth Signed-off-by: Andrew Thornton --- routers/repo/http.go | 73 +++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/routers/repo/http.go b/routers/repo/http.go index 0377979e8bb5c..7800057843b30 100644 --- a/routers/repo/http.go +++ b/routers/repo/http.go @@ -151,11 +151,9 @@ 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 + authUser *models.User + environ []string ) // don't allow anonymous pulls if organization is not public @@ -170,14 +168,19 @@ 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 + // FIXME: middlewares/context.go did basic auth check already, + // maybe could use that one. + if setting.Service.EnableReverseProxyAuth { + authUsername := ctx.Req.Header.Get(setting.ReverseProxyAuthUser) + if 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 { + } + if authUser == nil { authHead := ctx.Req.Header.Get("Authorization") if len(authHead) == 0 { ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=\".\"") @@ -185,17 +188,15 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { return } - auths := strings.Fields(authHead) + auths := strings.SplitN(authHead, " ", 2) // 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" { + if len(auths) != 2 || strings.ToLower(auths[0]) != "basic" { ctx.HandleText(http.StatusUnauthorized, "no basic auth and digit auth") return } - authUsername, authPasswd, err = base.BasicAuthDecode(auths[1]) - if err != nil { + authUsername, authPasswd, err := base.BasicAuthDecode(auths[1]) + if err != nil || (len(authUsername) == 0 && len(authPasswd) == 0) { ctx.HandleText(http.StatusUnauthorized, "no basic auth and digit auth") return } @@ -218,21 +219,31 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { return } } - // Assume password is a token. - token, err := models.GetAccessTokenBySHA(authToken) - 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) + if authUser == nil { + // Assume password is a token. + token, err := models.GetAccessTokenBySHA(authToken) + if err == nil { + authUser, err = models.GetUserByID(token.UID) + if err != nil { + ctx.ServerError("GetUserByID", err) + return + } + + ctx.Data["IsApiToken"] = true + + 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) } - } else if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { - log.Error("GetAccessTokenBySha: %v", err) + } + + if authUser == nil && !setting.Service.EnableBasicAuth { + ctx.HandleText(http.StatusUnauthorized, fmt.Sprintf("invalid credentials from %s", ctx.RemoteAddr())) + return } if authUser == nil { From bead56802d5a2e67c2088575192510a8cec2b15c Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 28 Mar 2021 20:14:24 +0100 Subject: [PATCH 05/19] set the reverseproxy auth method Signed-off-by: Andrew Thornton --- modules/auth/sso/reverseproxy.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/auth/sso/reverseproxy.go b/modules/auth/sso/reverseproxy.go index 9525128e0a82b..98cd1b927f5d5 100644 --- a/modules/auth/sso/reverseproxy.go +++ b/modules/auth/sso/reverseproxy.go @@ -66,17 +66,17 @@ func (r *ReverseProxy) VerifyAuthData(req *http.Request, w http.ResponseWriter, return nil } - store.GetData()["AuthenticationMechanism"] = ReverseProxyMechanism - user, err := models.GetUserByName(username) if err != nil { if models.IsErrUserNotExist(err) && r.isAutoRegisterAllowed() { + store.GetData()["AuthenticationMechanism"] = ReverseProxyMechanism return r.newUser(req) } log.Error("GetUserByName: %v", err) return nil } + store.GetData()["AuthenticationMechanism"] = ReverseProxyMechanism return user } From 41c8ccf5ff406972fbf83343480371a6b9bf136f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 28 Mar 2021 21:36:17 +0100 Subject: [PATCH 06/19] Prevent ReverseProxy users from changing username Fix #2407 Signed-off-by: Andrew Thornton --- integrations/api_repo_lfs_locks_test.go | 2 +- modules/auth/sso/basic.go | 3 +++ modules/auth/sso/oauth2.go | 6 ++++++ modules/auth/sso/reverseproxy.go | 4 +++- modules/auth/sso/session.go | 1 + modules/auth/sso/sso.go | 9 +-------- modules/auth/sso/sspi_windows.go | 3 +++ modules/lfs/locks.go | 8 ++++---- modules/lfs/server.go | 10 +++++----- routers/user/setting/profile.go | 5 +++++ templates/user/settings/profile.tmpl | 4 ++-- 11 files changed, 34 insertions(+), 21 deletions(-) diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go index 69981d1c42000..ffc239567dc91 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 6d1c023a79a5e..3512c8f061ee5 100644 --- a/modules/auth/sso/basic.go +++ b/modules/auth/sso/basic.go @@ -16,6 +16,9 @@ import ( "code.gitea.io/gitea/modules/timeutil" ) +// BasicAuthenticationMechanism represents authentication using Basic authentication +const BasicAuthenticationMechanism AuthenticationMechanism = "Basic" + // Ensure the struct implements the interface. var ( _ SingleSignOn = &Basic{} diff --git a/modules/auth/sso/oauth2.go b/modules/auth/sso/oauth2.go index 617d1b732f2f7..21b0cf092144a 100644 --- a/modules/auth/sso/oauth2.go +++ b/modules/auth/sso/oauth2.go @@ -16,6 +16,12 @@ import ( "code.gitea.io/gitea/modules/web/middleware" ) +// OAuth2Mechanism represents authentication using OAuth2 +const OAuth2Mechanism AuthenticationMechanism = "OAuth2" + +// TokenMechanism represents authentication using Token +const TokenMechanism AuthenticationMechanism = "Token" + // Ensure the struct implements the interface. var ( _ SingleSignOn = &OAuth2{} diff --git a/modules/auth/sso/reverseproxy.go b/modules/auth/sso/reverseproxy.go index 98cd1b927f5d5..d015edd48998a 100644 --- a/modules/auth/sso/reverseproxy.go +++ b/modules/auth/sso/reverseproxy.go @@ -16,6 +16,9 @@ import ( gouuid "github.com/google/uuid" ) +// ReverseProxyMechanism represents authentication using ReverseProxy +const ReverseProxyMechanism AuthenticationMechanism = "ReverseProxy" + // Ensure the struct implements the interface. var ( _ SingleSignOn = &ReverseProxy{} @@ -104,7 +107,6 @@ func (r *ReverseProxy) newUser(req *http.Request) *models.User { user := &models.User{ Name: username, Email: email, - Passwd: username, IsActive: true, } if err := models.CreateUser(user); err != nil { diff --git a/modules/auth/sso/session.go b/modules/auth/sso/session.go index 7a546577d86ee..00f42277dd1a6 100644 --- a/modules/auth/sso/session.go +++ b/modules/auth/sso/session.go @@ -42,6 +42,7 @@ func (s *Session) IsEnabled() bool { func (s *Session) VerifyAuthData(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *models.User { user := SessionUser(sess) if user != nil { + store.GetData()["AuthenticationMechanism"] = "" return user } return nil diff --git a/modules/auth/sso/sso.go b/modules/auth/sso/sso.go index 82567e2d81cf5..369927becba53 100644 --- a/modules/auth/sso/sso.go +++ b/modules/auth/sso/sso.go @@ -32,16 +32,9 @@ var ssoMethods = []SingleSignOn{ &Basic{}, } +// AuthenticationMechanism represents possible authentication mechanisms type AuthenticationMechanism string -const ( - OAuth2Mechanism AuthenticationMechanism = "OAuth2" - TokenMechanism AuthenticationMechanism = "Token" - ReverseProxyMechanism AuthenticationMechanism = "ReverseProxy" - BasicAuthenticationMechanism AuthenticationMechanism = "Basic" - SSPIMechanism AuthenticationMechanism = "SSPI" -) - // The purpose of the following three function variables is to let the linter know that // those functions are not dead code and are actually being used var ( diff --git a/modules/auth/sso/sspi_windows.go b/modules/auth/sso/sspi_windows.go index 151340ba772cd..47b05aa55e933 100644 --- a/modules/auth/sso/sspi_windows.go +++ b/modules/auth/sso/sspi_windows.go @@ -23,6 +23,9 @@ import ( const ( tplSignIn base.TplName = "user/auth/signin" + + // SSPIMechanism represents authentication using SSPI + SSPIMechanism AuthenticationMechanism = "SSPI" ) var ( diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go index 246ca7e89ca52..5348b48f17481 100644 --- a/modules/lfs/locks.go +++ b/modules/lfs/locks.go @@ -77,7 +77,7 @@ func GetListLockHandler(ctx *context.Context) { } 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{ @@ -169,7 +169,7 @@ func PostLockHandler(ctx *context.Context) { } 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{ @@ -242,7 +242,7 @@ func VerifyLockHandler(ctx *context.Context) { } 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{ @@ -313,7 +313,7 @@ func UnLockHandler(ctx *context.Context) { } 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{ diff --git a/modules/lfs/server.go b/modules/lfs/server.go index b70a61cb0616a..15e806742db90 100644 --- a/modules/lfs/server.go +++ b/modules/lfs/server.go @@ -141,7 +141,7 @@ func getAuthenticatedRepoAndMeta(ctx *context.Context, rv *RequestVars, requireW return nil, nil } - if !authenticate(ctx, repository, rv.Authorization, requireWrite) { + if !authenticate(ctx, repository, rv.Authorization, false, requireWrite) { requireAuth(ctx) return nil, nil } @@ -268,7 +268,7 @@ func PostHandler(ctx *context.Context) { return } - if !authenticate(ctx, repository, rv.Authorization, true) { + if !authenticate(ctx, repository, rv.Authorization, false, true) { requireAuth(ctx) return } @@ -352,7 +352,7 @@ func BatchHandler(ctx *context.Context) { requireWrite = true } - if !authenticate(ctx, repository, object.Authorization, requireWrite) { + if !authenticate(ctx, repository, object.Authorization, false, requireWrite) { requireAuth(ctx) return } @@ -598,7 +598,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 @@ -612,7 +612,7 @@ func authenticate(ctx *context.Context, repository *models.Repository, authoriza } canRead := perm.CanAccess(accessMode, models.UnitTypeCode) - if canRead { + if canRead && (!requireSigned || ctx.IsSigned) { return true } diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go index c04428261a6d6..2b290f977a505 100644 --- a/routers/user/setting/profile.go +++ b/routers/user/setting/profile.go @@ -15,6 +15,7 @@ import ( "strings" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/auth/sso" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" auth "code.gitea.io/gitea/modules/forms" @@ -51,6 +52,10 @@ func HandleUsernameChange(ctx *context.Context, user *models.User, newName strin // Check if user name has been changed if user.LowerName != strings.ToLower(newName) { + if ctx.Data["AuthenticationMechanism"] == sso.ReverseProxyMechanism { + ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user")) + return fmt.Errorf(ctx.Tr("form.username_change_not_local_user")) + } if err := models.ChangeUserName(user, newName); err != nil { switch { case models.IsErrUserAlreadyExist(err): diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl index ee3cc589041a8..14b739e41090e 100644 --- a/templates/user/settings/profile.tmpl +++ b/templates/user/settings/profile.tmpl @@ -15,8 +15,8 @@ {{.i18n.Tr "settings.change_username_prompt"}} {{.i18n.Tr "settings.change_username_redirect_prompt"}} - - {{if not .SignedUser.IsLocal}} + + {{if or (not .SignedUser.IsLocal) (eq .AuthenticationMechanism "ReverseProxy")}}

{{$.i18n.Tr "settings.password_username_disabled"}}

{{end}} From 213c2062ee76debcb50da7b4d7636d49cff8c8fa Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 2 Apr 2021 02:57:46 +0100 Subject: [PATCH 07/19] move modules/auth/sso to services/auth Signed-off-by: Andrew Thornton --- modules/context/api.go | 4 ++-- modules/context/context.go | 4 ++-- routers/init.go | 4 ++-- routers/repo/http.go | 4 ++-- routers/routes/base.go | 4 ++-- routers/user/setting/account.go | 8 ++++---- routers/user/setting/account_test.go | 4 ++-- routers/user/setting/applications.go | 4 ++-- routers/user/setting/keys.go | 4 ++-- routers/user/setting/oauth2.go | 6 +++--- routers/user/setting/profile.go | 14 +++++++------- routers/user/setting/security_openid.go | 8 ++++---- routers/user/setting/security_twofa.go | 4 ++-- routers/user/setting/security_u2f.go | 6 +++--- .../sso/sso.go => services/auth/authenticators.go | 14 +++++++------- {modules/auth/sso => services/auth}/basic.go | 4 ++-- {modules/auth/sso => services/auth}/interface.go | 6 +++--- {modules/auth/sso => services/auth}/oauth2.go | 4 ++-- .../auth/sso => services/auth}/reverseproxy.go | 4 ++-- {modules/auth/sso => services/auth}/session.go | 4 ++-- .../auth/sso => services/auth}/sspi_windows.go | 2 +- {modules/auth/sso => services/auth}/user.go | 2 +- 22 files changed, 59 insertions(+), 59 deletions(-) rename modules/auth/sso/sso.go => services/auth/authenticators.go (93%) rename {modules/auth/sso => services/auth}/basic.go (98%) rename {modules/auth/sso => services/auth}/interface.go (91%) rename {modules/auth/sso => services/auth}/oauth2.go (98%) rename {modules/auth/sso => services/auth}/reverseproxy.go (98%) rename {modules/auth/sso => services/auth}/session.go (96%) rename {modules/auth/sso => services/auth}/sspi_windows.go (99%) rename {modules/auth/sso => services/auth}/user.go (98%) diff --git a/modules/context/api.go b/modules/context/api.go index cbd90c50e4b8e..db6252f80a273 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -14,11 +14,11 @@ import ( "strings" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/auth/sso" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web/middleware" + "code.gitea.io/gitea/services/auth" "gitea.com/go-chi/session" ) @@ -251,7 +251,7 @@ func APIContexter() func(http.Handler) http.Handler { } // Get user from session if logged in. - ctx.User, ctx.IsBasicAuth = sso.SignedInUser(ctx.Req, ctx.Resp, &ctx, ctx.Session) + ctx.User, ctx.IsBasicAuth = auth.SignedInUser(ctx.Req, ctx.Resp, &ctx, ctx.Session) if ctx.User != nil { ctx.IsSigned = true ctx.Data["IsSigned"] = ctx.IsSigned diff --git a/modules/context/context.go b/modules/context/context.go index b876487d5e004..c38f7ad16ad98 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -21,7 +21,6 @@ import ( "time" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/auth/sso" "code.gitea.io/gitea/modules/base" mc "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/log" @@ -29,6 +28,7 @@ import ( "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/web/middleware" + "code.gitea.io/gitea/services/auth" "gitea.com/go-chi/cache" "gitea.com/go-chi/session" @@ -671,7 +671,7 @@ func Contexter() func(next http.Handler) http.Handler { } // Get user from session if logged in. - ctx.User, ctx.IsBasicAuth = sso.SignedInUser(ctx.Req, ctx.Resp, &ctx, ctx.Session) + ctx.User, ctx.IsBasicAuth = auth.SignedInUser(ctx.Req, ctx.Resp, &ctx, ctx.Session) if ctx.User != nil { ctx.IsSigned = true diff --git a/routers/init.go b/routers/init.go index f5dbfc87d2721..aac736ab97d24 100644 --- a/routers/init.go +++ b/routers/init.go @@ -12,7 +12,6 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/migrations" - "code.gitea.io/gitea/modules/auth/sso" "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/cron" "code.gitea.io/gitea/modules/eventsource" @@ -32,6 +31,7 @@ import ( "code.gitea.io/gitea/modules/svg" "code.gitea.io/gitea/modules/task" "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/mailer" mirror_service "code.gitea.io/gitea/services/mirror" pull_service "code.gitea.io/gitea/services/pull" @@ -189,7 +189,7 @@ func GlobalInit(ctx context.Context) { } else { ssh.Unused() } - sso.Init() + auth.Init() svg.Init() } diff --git a/routers/repo/http.go b/routers/repo/http.go index 7800057843b30..2a0f2bbf5bf2b 100644 --- a/routers/repo/http.go +++ b/routers/repo/http.go @@ -22,7 +22,6 @@ 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" @@ -32,6 +31,7 @@ import ( "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/services/auth" repo_service "code.gitea.io/gitea/services/repository" ) @@ -209,7 +209,7 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { // Assume password is token authToken = authPasswd } - uid := sso.CheckOAuthAccessToken(authToken) + uid := auth.CheckOAuthAccessToken(authToken) if uid != 0 { ctx.Data["IsApiToken"] = true diff --git a/routers/routes/base.go b/routers/routes/base.go index 743582d4a56dc..edc5e541d078b 100644 --- a/routers/routes/base.go +++ b/routers/routes/base.go @@ -15,7 +15,6 @@ import ( "strings" "time" - "code.gitea.io/gitea/modules/auth/sso" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/httpcache" "code.gitea.io/gitea/modules/log" @@ -23,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/web/middleware" + "code.gitea.io/gitea/services/auth" "gitea.com/go-chi/session" ) @@ -172,7 +172,7 @@ func Recovery() func(next http.Handler) http.Handler { } // Get user from session if logged in. - user, _ := sso.SignedInUser(req, w, &store, sessionStore) + user, _ := auth.SignedInUser(req, w, &store, sessionStore) if user != nil { store.Data["IsSigned"] = true store.Data["SignedUser"] = user diff --git a/routers/user/setting/account.go b/routers/user/setting/account.go index 2b2804b53b650..91b49bf0d4c68 100644 --- a/routers/user/setting/account.go +++ b/routers/user/setting/account.go @@ -13,7 +13,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" - auth "code.gitea.io/gitea/modules/forms" + "code.gitea.io/gitea/modules/forms" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" @@ -39,7 +39,7 @@ func Account(ctx *context.Context) { // AccountPost response for change user's password func AccountPost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.ChangePasswordForm) + form := web.GetForm(ctx).(*forms.ChangePasswordForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAccount"] = true @@ -84,7 +84,7 @@ func AccountPost(ctx *context.Context) { // EmailPost response for change user's email func EmailPost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.AddEmailForm) + form := web.GetForm(ctx).(*forms.AddEmailForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAccount"] = true @@ -257,7 +257,7 @@ func DeleteAccount(ctx *context.Context) { // UpdateUIThemePost is used to update users' specific theme func UpdateUIThemePost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.UpdateThemeForm) + form := web.GetForm(ctx).(*forms.UpdateThemeForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAccount"] = true diff --git a/routers/user/setting/account_test.go b/routers/user/setting/account_test.go index 0e7e147b8bc72..24d190d0b109a 100644 --- a/routers/user/setting/account_test.go +++ b/routers/user/setting/account_test.go @@ -9,7 +9,7 @@ import ( "testing" "code.gitea.io/gitea/models" - auth "code.gitea.io/gitea/modules/forms" + "code.gitea.io/gitea/modules/forms" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/web" @@ -86,7 +86,7 @@ func TestChangePassword(t *testing.T) { test.LoadUser(t, ctx, 2) test.LoadRepo(t, ctx, 1) - web.SetForm(ctx, &auth.ChangePasswordForm{ + web.SetForm(ctx, &forms.ChangePasswordForm{ OldPassword: req.OldPassword, Password: req.NewPassword, Retype: req.Retype, diff --git a/routers/user/setting/applications.go b/routers/user/setting/applications.go index 367f2b38c1217..d1853f7621670 100644 --- a/routers/user/setting/applications.go +++ b/routers/user/setting/applications.go @@ -11,7 +11,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" - auth "code.gitea.io/gitea/modules/forms" + "code.gitea.io/gitea/modules/forms" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" ) @@ -32,7 +32,7 @@ func Applications(ctx *context.Context) { // ApplicationsPost response for add user's access token func ApplicationsPost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.NewAccessTokenForm) + form := web.GetForm(ctx).(*forms.NewAccessTokenForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsApplications"] = true diff --git a/routers/user/setting/keys.go b/routers/user/setting/keys.go index 98b7b7413748a..9044a375c0f8d 100644 --- a/routers/user/setting/keys.go +++ b/routers/user/setting/keys.go @@ -11,7 +11,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" - auth "code.gitea.io/gitea/modules/forms" + "code.gitea.io/gitea/modules/forms" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" ) @@ -35,7 +35,7 @@ func Keys(ctx *context.Context) { // KeysPost response for change user's SSH/GPG keys func KeysPost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.AddKeyForm) + form := web.GetForm(ctx).(*forms.AddKeyForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsKeys"] = true ctx.Data["DisableSSH"] = setting.SSH.Disabled diff --git a/routers/user/setting/oauth2.go b/routers/user/setting/oauth2.go index a12f4dc1bade7..2ed4fd5a6a8c7 100644 --- a/routers/user/setting/oauth2.go +++ b/routers/user/setting/oauth2.go @@ -11,7 +11,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" - auth "code.gitea.io/gitea/modules/forms" + "code.gitea.io/gitea/modules/forms" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" @@ -23,7 +23,7 @@ const ( // OAuthApplicationsPost response for adding a oauth2 application func OAuthApplicationsPost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.EditOAuth2ApplicationForm) + form := web.GetForm(ctx).(*forms.EditOAuth2ApplicationForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsApplications"] = true @@ -55,7 +55,7 @@ func OAuthApplicationsPost(ctx *context.Context) { // OAuthApplicationsEdit response for editing oauth2 application func OAuthApplicationsEdit(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.EditOAuth2ApplicationForm) + form := web.GetForm(ctx).(*forms.EditOAuth2ApplicationForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsApplications"] = true diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go index 2b290f977a505..f8edaac31e1f7 100644 --- a/routers/user/setting/profile.go +++ b/routers/user/setting/profile.go @@ -15,15 +15,15 @@ import ( "strings" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/auth/sso" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" - auth "code.gitea.io/gitea/modules/forms" + "code.gitea.io/gitea/modules/forms" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web/middleware" + "code.gitea.io/gitea/services/auth" "github.com/unknwon/i18n" ) @@ -52,7 +52,7 @@ func HandleUsernameChange(ctx *context.Context, user *models.User, newName strin // Check if user name has been changed if user.LowerName != strings.ToLower(newName) { - if ctx.Data["AuthenticationMechanism"] == sso.ReverseProxyMechanism { + if ctx.Data["AuthenticationMechanism"] == auth.ReverseProxyMechanism { ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user")) return fmt.Errorf(ctx.Tr("form.username_change_not_local_user")) } @@ -80,7 +80,7 @@ func HandleUsernameChange(ctx *context.Context, user *models.User, newName strin // ProfilePost response for change user's profile func ProfilePost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.UpdateProfileForm) + form := web.GetForm(ctx).(*forms.UpdateProfileForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsProfile"] = true @@ -132,8 +132,8 @@ func ProfilePost(ctx *context.Context) { // UpdateAvatarSetting update user's avatar // FIXME: limit size. -func UpdateAvatarSetting(ctx *context.Context, form *auth.AvatarForm, ctxUser *models.User) error { - ctxUser.UseCustomAvatar = form.Source == auth.AvatarLocal +func UpdateAvatarSetting(ctx *context.Context, form *forms.AvatarForm, ctxUser *models.User) error { + ctxUser.UseCustomAvatar = form.Source == forms.AvatarLocal if len(form.Gravatar) > 0 { if form.Avatar != nil { ctxUser.Avatar = base.EncodeMD5(form.Gravatar) @@ -181,7 +181,7 @@ func UpdateAvatarSetting(ctx *context.Context, form *auth.AvatarForm, ctxUser *m // AvatarPost response for change user's avatar request func AvatarPost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.AvatarForm) + form := web.GetForm(ctx).(*forms.AvatarForm) if err := UpdateAvatarSetting(ctx, form, ctx.User); err != nil { ctx.Flash.Error(err.Error()) } else { diff --git a/routers/user/setting/security_openid.go b/routers/user/setting/security_openid.go index c5d106e9907c5..ae4e575e635ad 100644 --- a/routers/user/setting/security_openid.go +++ b/routers/user/setting/security_openid.go @@ -10,7 +10,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/auth/openid" "code.gitea.io/gitea/modules/context" - auth "code.gitea.io/gitea/modules/forms" + "code.gitea.io/gitea/modules/forms" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" @@ -18,7 +18,7 @@ import ( // OpenIDPost response for change user's openid func OpenIDPost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.AddOpenIDForm) + form := web.GetForm(ctx).(*forms.AddOpenIDForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsSecurity"] = true @@ -81,7 +81,7 @@ func settingsOpenIDVerify(ctx *context.Context) { id, err := openid.Verify(fullURL) if err != nil { - ctx.RenderWithErr(err.Error(), tplSettingsSecurity, &auth.AddOpenIDForm{ + ctx.RenderWithErr(err.Error(), tplSettingsSecurity, &forms.AddOpenIDForm{ Openid: id, }) return @@ -92,7 +92,7 @@ func settingsOpenIDVerify(ctx *context.Context) { oid := &models.UserOpenID{UID: ctx.User.ID, URI: id} if err = models.AddUserOpenID(oid); err != nil { if models.IsErrOpenIDAlreadyUsed(err) { - ctx.RenderWithErr(ctx.Tr("form.openid_been_used", id), tplSettingsSecurity, &auth.AddOpenIDForm{Openid: id}) + ctx.RenderWithErr(ctx.Tr("form.openid_been_used", id), tplSettingsSecurity, &forms.AddOpenIDForm{Openid: id}) return } ctx.ServerError("AddUserOpenID", err) diff --git a/routers/user/setting/security_twofa.go b/routers/user/setting/security_twofa.go index a830495f54471..4ef5819c6cd32 100644 --- a/routers/user/setting/security_twofa.go +++ b/routers/user/setting/security_twofa.go @@ -15,7 +15,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" - auth "code.gitea.io/gitea/modules/forms" + "code.gitea.io/gitea/modules/forms" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" @@ -168,7 +168,7 @@ func EnrollTwoFactor(ctx *context.Context) { // EnrollTwoFactorPost handles enrolling the user into 2FA. func EnrollTwoFactorPost(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.TwoFactorAuthForm) + form := web.GetForm(ctx).(*forms.TwoFactorAuthForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsSecurity"] = true diff --git a/routers/user/setting/security_u2f.go b/routers/user/setting/security_u2f.go index 040af34b5b309..01c50259b31b4 100644 --- a/routers/user/setting/security_u2f.go +++ b/routers/user/setting/security_u2f.go @@ -10,7 +10,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" - auth "code.gitea.io/gitea/modules/forms" + "code.gitea.io/gitea/modules/forms" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" @@ -20,7 +20,7 @@ import ( // U2FRegister initializes the u2f registration procedure func U2FRegister(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.U2FRegistrationForm) + form := web.GetForm(ctx).(*forms.U2FRegistrationForm) if form.Name == "" { ctx.Error(http.StatusConflict) return @@ -87,7 +87,7 @@ func U2FRegisterPost(ctx *context.Context) { // U2FDelete deletes an security key by id func U2FDelete(ctx *context.Context) { - form := web.GetForm(ctx).(*auth.U2FDeleteForm) + form := web.GetForm(ctx).(*forms.U2FDeleteForm) reg, err := models.GetU2FRegistrationByID(form.ID) if err != nil { if models.IsErrU2FRegistrationNotExist(err) { diff --git a/modules/auth/sso/sso.go b/services/auth/authenticators.go similarity index 93% rename from modules/auth/sso/sso.go rename to services/auth/authenticators.go index 369927becba53..3c56955ee7d0c 100644 --- a/modules/auth/sso/sso.go +++ b/services/auth/authenticators.go @@ -3,7 +3,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package sso +package auth import ( "fmt" @@ -16,7 +16,7 @@ import ( "code.gitea.io/gitea/modules/web/middleware" ) -// ssoMethods contains the list of SSO authentication plugins in the order they are expected to be +// authMethods contains the list of SSO authentication plugins in the order they are expected to be // executed. // // The OAuth2 plugin is expected to be executed first, as it must ignore the user id stored @@ -25,7 +25,7 @@ import ( // // The Session plugin is expected to be executed second, in order to skip authentication // for users that have already signed in. -var ssoMethods = []SingleSignOn{ +var authMethods = []Authenticator{ &OAuth2{}, &Session{}, &ReverseProxy{}, @@ -42,13 +42,13 @@ var ( ) // Methods returns the instances of all registered SSO methods -func Methods() []SingleSignOn { - return ssoMethods +func Methods() []Authenticator { + return authMethods } // Register adds the specified instance to the list of available SSO methods -func Register(method SingleSignOn) { - ssoMethods = append(ssoMethods, method) +func Register(method Authenticator) { + authMethods = append(authMethods, method) } // Init should be called exactly once when the application starts to allow SSO plugins diff --git a/modules/auth/sso/basic.go b/services/auth/basic.go similarity index 98% rename from modules/auth/sso/basic.go rename to services/auth/basic.go index 3512c8f061ee5..adc62ba1a301c 100644 --- a/modules/auth/sso/basic.go +++ b/services/auth/basic.go @@ -3,7 +3,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package sso +package auth import ( "net/http" @@ -21,7 +21,7 @@ const BasicAuthenticationMechanism AuthenticationMechanism = "Basic" // Ensure the struct implements the interface. var ( - _ SingleSignOn = &Basic{} + _ Authenticator = &Basic{} ) // Basic implements the SingleSignOn interface and authenticates requests (API requests diff --git a/modules/auth/sso/interface.go b/services/auth/interface.go similarity index 91% rename from modules/auth/sso/interface.go rename to services/auth/interface.go index 9b1472f2b37fb..60b205c4358e2 100644 --- a/modules/auth/sso/interface.go +++ b/services/auth/interface.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package sso +package auth import ( "net/http" @@ -18,8 +18,8 @@ type DataStore middleware.DataStore // SessionStore represents a session store type SessionStore session.Store -// SingleSignOn represents a SSO authentication method (plugin) for HTTP requests. -type SingleSignOn interface { +// Authenticator represents a SSO authentication method (plugin) for HTTP requests. +type Authenticator interface { // Init should be called exactly once before using any of the other methods, // in order to allow the plugin to allocate necessary resources Init() error diff --git a/modules/auth/sso/oauth2.go b/services/auth/oauth2.go similarity index 98% rename from modules/auth/sso/oauth2.go rename to services/auth/oauth2.go index 21b0cf092144a..e2de0a6729c81 100644 --- a/modules/auth/sso/oauth2.go +++ b/services/auth/oauth2.go @@ -3,7 +3,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package sso +package auth import ( "net/http" @@ -24,7 +24,7 @@ const TokenMechanism AuthenticationMechanism = "Token" // Ensure the struct implements the interface. var ( - _ SingleSignOn = &OAuth2{} + _ Authenticator = &OAuth2{} ) // CheckOAuthAccessToken returns uid of user from oauth token diff --git a/modules/auth/sso/reverseproxy.go b/services/auth/reverseproxy.go similarity index 98% rename from modules/auth/sso/reverseproxy.go rename to services/auth/reverseproxy.go index d015edd48998a..88ca7a14c83ba 100644 --- a/modules/auth/sso/reverseproxy.go +++ b/services/auth/reverseproxy.go @@ -3,7 +3,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package sso +package auth import ( "net/http" @@ -21,7 +21,7 @@ const ReverseProxyMechanism AuthenticationMechanism = "ReverseProxy" // Ensure the struct implements the interface. var ( - _ SingleSignOn = &ReverseProxy{} + _ Authenticator = &ReverseProxy{} ) // ReverseProxy implements the SingleSignOn interface, but actually relies on diff --git a/modules/auth/sso/session.go b/services/auth/session.go similarity index 96% rename from modules/auth/sso/session.go rename to services/auth/session.go index 00f42277dd1a6..c813cbfcea4b3 100644 --- a/modules/auth/sso/session.go +++ b/services/auth/session.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package sso +package auth import ( "net/http" @@ -12,7 +12,7 @@ import ( // Ensure the struct implements the interface. var ( - _ SingleSignOn = &Session{} + _ Authenticator = &Session{} ) // Session checks if there is a user uid stored in the session and returns the user diff --git a/modules/auth/sso/sspi_windows.go b/services/auth/sspi_windows.go similarity index 99% rename from modules/auth/sso/sspi_windows.go rename to services/auth/sspi_windows.go index 47b05aa55e933..f8f5ed9d4a1b8 100644 --- a/modules/auth/sso/sspi_windows.go +++ b/services/auth/sspi_windows.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package sso +package auth import ( "errors" diff --git a/modules/auth/sso/user.go b/services/auth/user.go similarity index 98% rename from modules/auth/sso/user.go rename to services/auth/user.go index 48eebb1e915f0..f6dd285fca1f6 100644 --- a/modules/auth/sso/user.go +++ b/services/auth/user.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package sso +package auth import ( "net/http" From 4b7120e0566e6dd28a918e8e014bbe6b42d981d4 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 2 Apr 2021 03:16:48 +0100 Subject: [PATCH 08/19] Recovery handler should only read user from session The recovery handler should not attempt to reauthenticate the user - as this could have been the site of the panic! --- routers/routes/base.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/routes/base.go b/routers/routes/base.go index edc5e541d078b..8865e2b05cadb 100644 --- a/routers/routes/base.go +++ b/routers/routes/base.go @@ -171,8 +171,8 @@ func Recovery() func(next http.Handler) http.Handler { }, } - // Get user from session if logged in. - user, _ := auth.SignedInUser(req, w, &store, sessionStore) + // Get user from session if logged in - do not attempt to sign-in + user := auth.SessionUser(sessionStore) if user != nil { store.Data["IsSigned"] = true store.Data["SignedUser"] = user From d65433beabfad2f5284fd32b8a516ee90357d5ed Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Apr 2021 14:30:25 +0100 Subject: [PATCH 09/19] Reduce the amount of handlers running for every request by using mounting Signed-off-by: Andrew Thornton --- cmd/web_graceful.go | 9 ++++++++- modules/web/route.go | 13 ++++++++++++ routers/home.go | 2 ++ routers/routes/web.go | 46 +++++++++++++++++++++++-------------------- 4 files changed, 48 insertions(+), 22 deletions(-) diff --git a/cmd/web_graceful.go b/cmd/web_graceful.go index 5db065818a19f..91ac024ddb42f 100644 --- a/cmd/web_graceful.go +++ b/cmd/web_graceful.go @@ -9,9 +9,11 @@ import ( "net" "net/http" "net/http/fcgi" + "strings" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" ) func runHTTP(network, listenAddr, name string, m http.Handler) error { @@ -48,7 +50,12 @@ func runFCGI(network, listenAddr, name string, m http.Handler) error { fcgiServer := graceful.NewServer(network, listenAddr, name) err := fcgiServer.ListenAndServe(func(listener net.Listener) error { - return fcgi.Serve(listener, m) + return fcgi.Serve(listener, http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + if setting.AppSubURL != "" { + req.URL.Path = strings.TrimPrefix(req.URL.Path, setting.AppSubURL) + } + m.ServeHTTP(resp, req) + })) }) if err != nil { log.Fatal("Failed to start FCGI main server: %v", err) diff --git a/modules/web/route.go b/modules/web/route.go index 59e22c5be15f7..d7527d85ea2dd 100644 --- a/modules/web/route.go +++ b/modules/web/route.go @@ -171,6 +171,19 @@ func (r *Route) Use(middlewares ...interface{}) { } } +func convertMiddleware(middleware interface{}) func(http.Handler) http.Handler { + switch t := middleware.(type) { + case func(http.Handler) http.Handler: + return t + case func(*context.Context): + return Middle(t) + case func(*context.APIContext): + return MiddleAPI(t) + default: + panic(fmt.Sprintf("Unsupported middleware type: %#v", t)) + } +} + // Group mounts a sub-Router along a `pattern` string. func (r *Route) Group(pattern string, fn func(), middlewares ...interface{}) { var previousGroupPrefix = r.curGroupPrefix diff --git a/routers/home.go b/routers/home.go index 7eaebc081fd4e..5d5ead784b230 100644 --- a/routers/home.go +++ b/routers/home.go @@ -37,6 +37,8 @@ const ( // Home render home page func Home(ctx *context.Context) { + // stack := log.Stack(1) + // log.Info("In Home: lines %d: stack: %s", strings.Count(stack, "\n"), stack) if ctx.IsSigned { if !ctx.User.IsActive && setting.Service.RegisterEmailConfirm { ctx.Data["Title"] = ctx.Tr("auth.active_your_account") diff --git a/routers/routes/web.go b/routers/routes/web.go index 5b382ecccba9b..d00c648994f8d 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -129,6 +129,7 @@ func NormalRoutes() *web.Route { // WebRoutes returns all web routes func WebRoutes() *web.Route { r := web.NewRoute() + all := r r.Use(session.Sessioner(session.Options{ Provider: setting.SessionConfig.Provider, @@ -143,6 +144,9 @@ func WebRoutes() *web.Route { r.Use(Recovery()) + // TODO: we should consider if there is a way to mount these using r.Get as at present + // these two handlers mean that every request has to hit these "filesystems" twice + // before finally getting to the router. It allows them to override any matching router below. r.Use(public.Custom( &public.Options{ SkipLogging: setting.DisableRouterLog, @@ -155,51 +159,51 @@ func WebRoutes() *web.Route { }, )) - r.Use(storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars)) - r.Use(storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) + // We use r.Route here over r.Use because this prevents requests that are not for avatars having to go through this additional handler + r.Route("/avatars", "GET, HEAD", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars)) + r.Route("/repo-avatars", "GET, HEAD", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) gob.Register(&u2f.Challenge{}) + common := []interface{}{} + if setting.EnableGzip { h, err := gziphandler.GzipHandlerWithOpts(gziphandler.MinSize(GzipMinSize)) if err != nil { log.Fatal("GzipHandlerWithOpts failed: %v", err) } - r.Use(h) - } - - if (setting.Protocol == setting.FCGI || setting.Protocol == setting.FCGIUnix) && setting.AppSubURL != "" { - r.Use(func(next http.Handler) http.Handler { - return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - req.URL.Path = strings.TrimPrefix(req.URL.Path, setting.AppSubURL) - next.ServeHTTP(resp, req) - }) - }) + common = append(common, h) } mailer.InitMailRender(templates.Mailer()) if setting.Service.EnableCaptcha { - r.Use(captcha.Captchaer(context.GetImageCaptcha())) + r.Route("/captcha/*", "GET,HEAD", append(common, captcha.Captchaer(context.GetImageCaptcha()))...) } + // Removed: toolbox.Toolboxer middleware will provide debug informations which seems unnecessary - r.Use(context.Contexter()) + common = append(common, context.Contexter()) // GetHead allows a HEAD request redirect to GET if HEAD method is not defined for that route - r.Use(middleware.GetHead) + common = append(common, middleware.GetHead) if setting.EnableAccessLog { - r.Use(context.AccessLogger()) + common = append(common, context.AccessLogger()) } - r.Use(user.GetNotificationCount) - r.Use(repo.GetActiveStopwatch) - r.Use(func(ctx *context.Context) { + common = append(common, user.GetNotificationCount) + common = append(common, repo.GetActiveStopwatch) + common = append(common, func(ctx *context.Context) { ctx.Data["UnitWikiGlobalDisabled"] = models.UnitTypeWiki.UnitGlobalDisabled() ctx.Data["UnitIssuesGlobalDisabled"] = models.UnitTypeIssues.UnitGlobalDisabled() ctx.Data["UnitPullsGlobalDisabled"] = models.UnitTypePullRequests.UnitGlobalDisabled() ctx.Data["UnitProjectsGlobalDisabled"] = models.UnitTypeProjects.UnitGlobalDisabled() }) + r = web.NewRoute() + for _, middle := range common { + r.Use(middle) + } + // for health check r.Head("/", func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusOK) @@ -234,8 +238,8 @@ func WebRoutes() *web.Route { } RegisterRoutes(r) - - return r + all.Mount("", r) + return all } func goGet(ctx *context.Context) { From 21b6ebf6e3c8c40f5de366b355fee9ca221c8504 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Apr 2021 15:04:18 +0100 Subject: [PATCH 10/19] Move the unit settings stuff to contexter Signed-off-by: Andrew Thornton --- modules/context/context.go | 5 +++++ routers/routes/web.go | 6 ------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/modules/context/context.go b/modules/context/context.go index c38f7ad16ad98..2f4ffce981502 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -707,6 +707,11 @@ func Contexter() func(next http.Handler) http.Handler { ctx.Data["ManifestData"] = setting.ManifestData + ctx.Data["UnitWikiGlobalDisabled"] = models.UnitTypeWiki.UnitGlobalDisabled() + ctx.Data["UnitIssuesGlobalDisabled"] = models.UnitTypeIssues.UnitGlobalDisabled() + ctx.Data["UnitPullsGlobalDisabled"] = models.UnitTypePullRequests.UnitGlobalDisabled() + ctx.Data["UnitProjectsGlobalDisabled"] = models.UnitTypeProjects.UnitGlobalDisabled() + ctx.Data["i18n"] = locale ctx.Data["Tr"] = i18n.Tr ctx.Data["Lang"] = locale.Language() diff --git a/routers/routes/web.go b/routers/routes/web.go index d00c648994f8d..d862e9444ed02 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -192,12 +192,6 @@ func WebRoutes() *web.Route { common = append(common, user.GetNotificationCount) common = append(common, repo.GetActiveStopwatch) - common = append(common, func(ctx *context.Context) { - ctx.Data["UnitWikiGlobalDisabled"] = models.UnitTypeWiki.UnitGlobalDisabled() - ctx.Data["UnitIssuesGlobalDisabled"] = models.UnitTypeIssues.UnitGlobalDisabled() - ctx.Data["UnitPullsGlobalDisabled"] = models.UnitTypePullRequests.UnitGlobalDisabled() - ctx.Data["UnitProjectsGlobalDisabled"] = models.UnitTypeProjects.UnitGlobalDisabled() - }) r = web.NewRoute() for _, middle := range common { From 0f216162e41ac383681d024aa01400c3744a3c28 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Apr 2021 20:43:55 +0100 Subject: [PATCH 11/19] move the access logger earlier in the chain Signed-off-by: Andrew Thornton --- routers/routes/web.go | 48 +++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index d862e9444ed02..cdf51c70759a8 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -90,6 +90,10 @@ func commonMiddlewares() []func(http.Handler) http.Handler { } } + if setting.EnableAccessLog { + handlers = append(handlers, context.AccessLogger()) + } + handlers = append(handlers, func(next http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { // Why we need this? The Recovery() will try to render a beautiful @@ -163,6 +167,15 @@ func WebRoutes() *web.Route { r.Route("/avatars", "GET, HEAD", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars)) r.Route("/repo-avatars", "GET, HEAD", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) + // for health check + r.Head("/", func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + r.Get("/apple-touch-icon.png", func(w http.ResponseWriter, req *http.Request) { + http.Redirect(w, req, path.Join(setting.StaticURLPrefix, "img/apple-touch-icon.png"), 301) + }) + gob.Register(&u2f.Challenge{}) common := []interface{}{} @@ -181,15 +194,22 @@ func WebRoutes() *web.Route { r.Route("/captcha/*", "GET,HEAD", append(common, captcha.Captchaer(context.GetImageCaptcha()))...) } + if setting.HasRobotsTxt { + r.Get("/robots.txt", append(common, func(w http.ResponseWriter, req *http.Request) { + filePath := path.Join(setting.CustomPath, "robots.txt") + fi, err := os.Stat(filePath) + if err == nil && httpcache.HandleTimeCache(req, w, fi) { + return + } + http.ServeFile(w, req, filePath) + })...) + } + // Removed: toolbox.Toolboxer middleware will provide debug informations which seems unnecessary common = append(common, context.Contexter()) // GetHead allows a HEAD request redirect to GET if HEAD method is not defined for that route common = append(common, middleware.GetHead) - if setting.EnableAccessLog { - common = append(common, context.AccessLogger()) - } - common = append(common, user.GetNotificationCount) common = append(common, repo.GetActiveStopwatch) @@ -198,26 +218,6 @@ func WebRoutes() *web.Route { r.Use(middle) } - // for health check - r.Head("/", func(w http.ResponseWriter, req *http.Request) { - w.WriteHeader(http.StatusOK) - }) - - if setting.HasRobotsTxt { - r.Get("/robots.txt", func(w http.ResponseWriter, req *http.Request) { - filePath := path.Join(setting.CustomPath, "robots.txt") - fi, err := os.Stat(filePath) - if err == nil && httpcache.HandleTimeCache(req, w, fi) { - return - } - http.ServeFile(w, req, filePath) - }) - } - - r.Get("/apple-touch-icon.png", func(w http.ResponseWriter, req *http.Request) { - http.Redirect(w, req, path.Join(setting.StaticURLPrefix, "img/apple-touch-icon.png"), 301) - }) - // prometheus metrics endpoint if setting.Metrics.Enabled { c := metrics.NewCollector() From 0793565b7ce585d49fd8af33f5113e79d21ed7e1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Apr 2021 22:24:27 +0100 Subject: [PATCH 12/19] move /api/swagger back into /api routes as it cleans up the sign-in process Signed-off-by: Andrew Thornton --- modules/context/api.go | 3 +++ routers/api/v1/api.go | 11 ++++++----- routers/api/v1/misc/swagger.go | 2 +- routers/routes/web.go | 8 +------- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/modules/context/api.go b/modules/context/api.go index db6252f80a273..d1bdda60d0c87 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/services/auth" @@ -219,6 +220,7 @@ func (ctx *APIContext) CheckForOTP() { // APIContexter returns apicontext as middleware func APIContexter() func(http.Handler) http.Handler { + var rnd = templates.HTMLRenderer() var csrfOpts = getCsrfOpts() return func(next http.Handler) http.Handler { @@ -228,6 +230,7 @@ func APIContexter() func(http.Handler) http.Handler { var ctx = APIContext{ Context: &Context{ Resp: NewResponse(w), + Render: rnd, Data: map[string]interface{}{}, Locale: locale, Session: session.GetSession(req), diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 88355fb2b34ed..f3e0fd30a568b 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -572,15 +572,16 @@ func Routes() *web.Route { } m.Use(context.APIContexter()) - if setting.EnableAccessLog { - m.Use(context.AccessLogger()) - } - m.Use(context.ToggleAPI(&context.ToggleOptions{ SignInRequired: setting.Service.RequireSignInView, })) - m.Group("", func() { + if setting.API.EnableSwagger { + // Note: The route moved back to apiroutes because it simplifies considerably the rest of the code + m.Get("/swagger", misc.Swagger) // Render V1 by default + } + + m.Group("/v1", func() { // Miscellaneous if setting.API.EnableSwagger { m.Get("/swagger", func(ctx *context.APIContext) { diff --git a/routers/api/v1/misc/swagger.go b/routers/api/v1/misc/swagger.go index e46d4194b46e8..247af915b261f 100644 --- a/routers/api/v1/misc/swagger.go +++ b/routers/api/v1/misc/swagger.go @@ -15,7 +15,7 @@ import ( const tplSwagger base.TplName = "swagger/ui" // Swagger render swagger-ui page with v1 json -func Swagger(ctx *context.Context) { +func Swagger(ctx *context.APIContext) { ctx.Data["APIJSONVersion"] = "v1" ctx.HTML(http.StatusOK, tplSwagger) } diff --git a/routers/routes/web.go b/routers/routes/web.go index cdf51c70759a8..73d4eb5db3dc6 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -30,7 +30,6 @@ import ( "code.gitea.io/gitea/routers" "code.gitea.io/gitea/routers/admin" apiv1 "code.gitea.io/gitea/routers/api/v1" - "code.gitea.io/gitea/routers/api/v1/misc" "code.gitea.io/gitea/routers/dev" "code.gitea.io/gitea/routers/events" "code.gitea.io/gitea/routers/org" @@ -125,8 +124,8 @@ func NormalRoutes() *web.Route { } r.Mount("/", WebRoutes()) - r.Mount("/api/v1", apiv1.Routes()) r.Mount("/api/internal", private.Routes()) + r.Mount("/api", apiv1.Routes()) return r } @@ -226,11 +225,6 @@ func WebRoutes() *web.Route { r.Get("/metrics", routers.Metrics) } - if setting.API.EnableSwagger { - // Note: The route moved from apiroutes because it's in fact want to render a web page - r.Get("/api/swagger", misc.Swagger) // Render V1 by default - } - RegisterRoutes(r) all.Mount("", r) return all From c7b50cf582e1ae67edfc6601e4c6cf4ce74dc28a Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 5 Apr 2021 02:19:54 +0100 Subject: [PATCH 13/19] prometheus does not need to be behind the contexter Signed-off-by: Andrew Thornton --- routers/routes/web.go | 51 ++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/routers/routes/web.go b/routers/routes/web.go index 73d4eb5db3dc6..5e64f7a53f170 100644 --- a/routers/routes/web.go +++ b/routers/routes/web.go @@ -131,10 +131,9 @@ func NormalRoutes() *web.Route { // WebRoutes returns all web routes func WebRoutes() *web.Route { - r := web.NewRoute() - all := r + routes := web.NewRoute() - r.Use(session.Sessioner(session.Options{ + routes.Use(session.Sessioner(session.Options{ Provider: setting.SessionConfig.Provider, ProviderConfig: setting.SessionConfig.ProviderConfig, CookieName: setting.SessionConfig.CookieName, @@ -145,17 +144,17 @@ func WebRoutes() *web.Route { Domain: setting.SessionConfig.Domain, })) - r.Use(Recovery()) + routes.Use(Recovery()) // TODO: we should consider if there is a way to mount these using r.Get as at present // these two handlers mean that every request has to hit these "filesystems" twice // before finally getting to the router. It allows them to override any matching router below. - r.Use(public.Custom( + routes.Use(public.Custom( &public.Options{ SkipLogging: setting.DisableRouterLog, }, )) - r.Use(public.Static( + routes.Use(public.Static( &public.Options{ Directory: path.Join(setting.StaticRootPath, "public"), SkipLogging: setting.DisableRouterLog, @@ -163,15 +162,15 @@ func WebRoutes() *web.Route { )) // We use r.Route here over r.Use because this prevents requests that are not for avatars having to go through this additional handler - r.Route("/avatars", "GET, HEAD", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars)) - r.Route("/repo-avatars", "GET, HEAD", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) + routes.Route("/avatars", "GET, HEAD", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars)) + routes.Route("/repo-avatars", "GET, HEAD", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) // for health check - r.Head("/", func(w http.ResponseWriter, req *http.Request) { + routes.Head("/", func(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusOK) }) - r.Get("/apple-touch-icon.png", func(w http.ResponseWriter, req *http.Request) { + routes.Get("/apple-touch-icon.png", func(w http.ResponseWriter, req *http.Request) { http.Redirect(w, req, path.Join(setting.StaticURLPrefix, "img/apple-touch-icon.png"), 301) }) @@ -190,11 +189,11 @@ func WebRoutes() *web.Route { mailer.InitMailRender(templates.Mailer()) if setting.Service.EnableCaptcha { - r.Route("/captcha/*", "GET,HEAD", append(common, captcha.Captchaer(context.GetImageCaptcha()))...) + routes.Route("/captcha/*", "GET,HEAD", append(common, captcha.Captchaer(context.GetImageCaptcha()))...) } if setting.HasRobotsTxt { - r.Get("/robots.txt", append(common, func(w http.ResponseWriter, req *http.Request) { + routes.Get("/robots.txt", append(common, func(w http.ResponseWriter, req *http.Request) { filePath := path.Join(setting.CustomPath, "robots.txt") fi, err := os.Stat(filePath) if err == nil && httpcache.HandleTimeCache(req, w, fi) { @@ -204,30 +203,32 @@ func WebRoutes() *web.Route { })...) } + // prometheus metrics endpoint + if setting.Metrics.Enabled { + c := metrics.NewCollector() + prometheus.MustRegister(c) + + routes.Get("/metrics", append(common, routers.Metrics)...) + } + // Removed: toolbox.Toolboxer middleware will provide debug informations which seems unnecessary common = append(common, context.Contexter()) + // GetHead allows a HEAD request redirect to GET if HEAD method is not defined for that route common = append(common, middleware.GetHead) + // TODO: These really seem like things that could be folded into Contexter or as helper functions common = append(common, user.GetNotificationCount) common = append(common, repo.GetActiveStopwatch) - r = web.NewRoute() + others := web.NewRoute() for _, middle := range common { - r.Use(middle) - } - - // prometheus metrics endpoint - if setting.Metrics.Enabled { - c := metrics.NewCollector() - prometheus.MustRegister(c) - - r.Get("/metrics", routers.Metrics) + others.Use(middle) } - RegisterRoutes(r) - all.Mount("", r) - return all + RegisterRoutes(others) + routes.Mount("", others) + return routes } func goGet(ctx *context.Context) { From 290cf57ebd99d8a9873fcadf877a24bec7ba25b1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 5 Apr 2021 02:20:50 +0100 Subject: [PATCH 14/19] Move flash setup out of Contexter and into its own self contained function Signed-off-by: Andrew Thornton --- modules/context/context.go | 45 +----------------------- modules/context/flash.go | 70 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 44 deletions(-) create mode 100644 modules/context/flash.go diff --git a/modules/context/context.go b/modules/context/context.go index 2f4ffce981502..c8567f7f0bd84 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -617,50 +617,7 @@ func Contexter() func(next http.Handler) http.Handler { ctx.csrf = Csrfer(csrfOpts, &ctx) // Get flash. - flashCookie := ctx.GetCookie("macaron_flash") - vals, _ := url.ParseQuery(flashCookie) - if len(vals) > 0 { - f := &middleware.Flash{ - DataStore: &ctx, - Values: vals, - ErrorMsg: vals.Get("error"), - SuccessMsg: vals.Get("success"), - InfoMsg: vals.Get("info"), - WarningMsg: vals.Get("warning"), - } - ctx.Data["Flash"] = f - } - - f := &middleware.Flash{ - DataStore: &ctx, - Values: url.Values{}, - ErrorMsg: "", - WarningMsg: "", - InfoMsg: "", - SuccessMsg: "", - } - ctx.Resp.Before(func(resp ResponseWriter) { - if flash := f.Encode(); len(flash) > 0 { - middleware.SetCookie(resp, "macaron_flash", flash, 0, - setting.SessionConfig.CookiePath, - middleware.Domain(setting.SessionConfig.Domain), - middleware.HTTPOnly(true), - middleware.Secure(setting.SessionConfig.Secure), - middleware.SameSite(setting.SessionConfig.SameSite), - ) - return - } - - middleware.SetCookie(ctx.Resp, "macaron_flash", "", -1, - setting.SessionConfig.CookiePath, - middleware.Domain(setting.SessionConfig.Domain), - middleware.HTTPOnly(true), - middleware.Secure(setting.SessionConfig.Secure), - middleware.SameSite(setting.SessionConfig.SameSite), - ) - }) - - ctx.Flash = f + setupFlash(&ctx) // If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid. if ctx.Req.Method == "POST" && strings.Contains(ctx.Req.Header.Get("Content-Type"), "multipart/form-data") { diff --git a/modules/context/flash.go b/modules/context/flash.go new file mode 100644 index 0000000000000..a6913c040c5f7 --- /dev/null +++ b/modules/context/flash.go @@ -0,0 +1,70 @@ +// Copyright 2021 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 context + +import ( + "net/url" + + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/web/middleware" +) + +func setupFlash(ctx *Context) { + + // Get the temporary flash cookie from the request + flashCookie := ctx.GetCookie("macaron_flash") + + // Parse its data + vals, _ := url.ParseQuery(flashCookie) + if len(vals) > 0 { + // If there is content then create a flash struct containing this data + f := &middleware.Flash{ + DataStore: ctx, + Values: vals, + ErrorMsg: vals.Get("error"), + SuccessMsg: vals.Get("success"), + InfoMsg: vals.Get("info"), + WarningMsg: vals.Get("warning"), + } + // And stick it in the context datastore + ctx.Data["Flash"] = f + } + + // Now create a new empty Flash struct for this response + f := &middleware.Flash{ + DataStore: ctx, + Values: url.Values{}, + ErrorMsg: "", + WarningMsg: "", + InfoMsg: "", + SuccessMsg: "", + } + + // Add a handler to write/delete the cookie before the response is written + ctx.Resp.Before(func(resp ResponseWriter) { + if flash := f.Encode(); len(flash) > 0 { + // If our flash object contains data - then save it to the cookie + middleware.SetCookie(resp, "macaron_flash", flash, 0, + setting.SessionConfig.CookiePath, + middleware.Domain(setting.SessionConfig.Domain), + middleware.HTTPOnly(true), + middleware.Secure(setting.SessionConfig.Secure), + middleware.SameSite(setting.SessionConfig.SameSite), + ) + return + } + // Otherwise delete the flash cookie + middleware.SetCookie(ctx.Resp, "macaron_flash", "", -1, + setting.SessionConfig.CookiePath, + middleware.Domain(setting.SessionConfig.Domain), + middleware.HTTPOnly(true), + middleware.Secure(setting.SessionConfig.Secure), + middleware.SameSite(setting.SessionConfig.SameSite), + ) + }) + + // Save the new empty Flash as ctx.Flash + ctx.Flash = f +} From 544c359c69afd023006402bfbb77748f0f779374 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 5 Apr 2021 12:51:00 +0100 Subject: [PATCH 15/19] remove unused convertMiddleware Signed-off-by: Andrew Thornton --- modules/web/route.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/modules/web/route.go b/modules/web/route.go index d7527d85ea2dd..293bc6ecb1cc0 100644 --- a/modules/web/route.go +++ b/modules/web/route.go @@ -67,6 +67,7 @@ func Wrap(handlers ...interface{}) http.HandlerFunc { return } case func(http.Handler) http.Handler: + // FIXME: next should be the remaining handlers var next = http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}) t(next).ServeHTTP(resp, req) if r, ok := resp.(context.ResponseWriter); ok && r.Status() > 0 { @@ -171,19 +172,6 @@ func (r *Route) Use(middlewares ...interface{}) { } } -func convertMiddleware(middleware interface{}) func(http.Handler) http.Handler { - switch t := middleware.(type) { - case func(http.Handler) http.Handler: - return t - case func(*context.Context): - return Middle(t) - case func(*context.APIContext): - return MiddleAPI(t) - default: - panic(fmt.Sprintf("Unsupported middleware type: %#v", t)) - } -} - // Group mounts a sub-Router along a `pattern` string. func (r *Route) Group(pattern string, fn func(), middlewares ...interface{}) { var previousGroupPrefix = r.curGroupPrefix From 96aae56d3b2d2b308259d5d97c444f5afdac20c3 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 5 Apr 2021 12:53:27 +0100 Subject: [PATCH 16/19] add some notes Signed-off-by: Andrew Thornton --- modules/context/context.go | 1 + services/auth/sspi_windows.go | 1 + 2 files changed, 2 insertions(+) diff --git a/modules/context/context.go b/modules/context/context.go index c8567f7f0bd84..d5f25122290c3 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -649,6 +649,7 @@ func Contexter() func(next http.Handler) http.Handler { log.Debug("Session ID: %s", ctx.Session.ID()) log.Debug("CSRF Token: %v", ctx.Data["CsrfToken"]) + // FIXME: do we really always need these setting? ctx.Data["IsLandingPageHome"] = setting.LandingPageURL == setting.LandingPageHome ctx.Data["IsLandingPageExplore"] = setting.LandingPageURL == setting.LandingPageExplore ctx.Data["IsLandingPageOrganizations"] = setting.LandingPageURL == setting.LandingPageOrganizations diff --git a/services/auth/sspi_windows.go b/services/auth/sspi_windows.go index f8f5ed9d4a1b8..a304cdedac097 100644 --- a/services/auth/sspi_windows.go +++ b/services/auth/sspi_windows.go @@ -98,6 +98,7 @@ func (s *SSPI) VerifyAuthData(req *http.Request, w http.ResponseWriter, store Da // Include the user login page in the 401 response to allow the user // to login with another authentication method if SSPI authentication // fails + // FIXME: This doesn't work store.GetData()["Flash"] = map[string]string{ "ErrorMsg": err.Error(), } From 08d5087e6df6d57200e71a179600064c7f3ce378 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 5 Apr 2021 19:47:57 +0100 Subject: [PATCH 17/19] Stop doing multiple authentication checks in git and lfs, and ensure that reverseproxy creates a session Signed-off-by: Andrew Thornton --- modules/context/context.go | 3 + modules/lfs/server.go | 62 +---------------- routers/repo/http.go | 119 ++++---------------------------- services/auth/authenticators.go | 18 ++++- services/auth/basic.go | 8 +++ services/auth/oauth2.go | 2 +- services/auth/reverseproxy.go | 15 ++-- 7 files changed, 55 insertions(+), 172 deletions(-) diff --git a/modules/context/context.go b/modules/context/context.go index d5f25122290c3..40a4d533c9409 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -640,6 +640,9 @@ func Contexter() func(next http.Handler) http.Handler { } else { ctx.Data["SignedUserID"] = int64(0) ctx.Data["SignedUserName"] = "" + + // delete the session uid + _ = ctx.Session.Delete("uid") } ctx.Resp.Header().Set(`X-Frame-Options`, `SAMEORIGIN`) diff --git a/modules/lfs/server.go b/modules/lfs/server.go index 15e806742db90..b3b39b0e93240 100644 --- a/modules/lfs/server.go +++ b/modules/lfs/server.go @@ -16,7 +16,6 @@ import ( "time" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -723,66 +722,7 @@ func parseToken(authorization string, target *models.Repository, mode models.Acc case "bearer": fallthrough case "token": - u, err := handleLFSToken(tokenSHA, target, mode) - if err != nil || u != nil { - return u, err - } - u, err = handleOAuth2Token(tokenSHA) - if u == nil && err == nil { - u, err = handleStandardToken(tokenSHA) - } - if err != nil { - return nil, err - } - if u != nil { - perm, err := models.GetUserRepoPermission(target, u) - if err != nil { - log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", u, target) - return nil, err - } - if perm.CanAccess(mode, models.UnitTypeCode) { - return u, nil - } - } - case "basic": - uname, passwd, _ := base.BasicAuthDecode(tokenSHA) - if len(uname) == 0 && len(passwd) == 0 { - return nil, fmt.Errorf("token not found") - } - // Check if username or password is a token - isUsernameToken := len(passwd) == 0 || passwd == "x-oauth-basic" - // Assume username is token - tokenSHA = uname - if !isUsernameToken { - // Assume password is token - tokenSHA = passwd - } - u, err := handleOAuth2Token(tokenSHA) - if u == nil && err == nil { - u, err = handleStandardToken(tokenSHA) - } - if u == nil && err == nil { - if !setting.Service.EnableBasicAuth { - return nil, fmt.Errorf("token not found") - } - u, err = models.UserSignIn(uname, passwd) - } - if err != nil { - if !models.IsErrUserNotExist(err) { - log.Error("UserSignIn: %v", err) - } - return nil, fmt.Errorf("basic auth failed") - } - if u != nil { - perm, err := models.GetUserRepoPermission(target, u) - if err != nil { - log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", u, target) - return nil, err - } - if perm.CanAccess(mode, models.UnitTypeCode) { - return u, nil - } - } + return handleLFSToken(tokenSHA, target, mode) } return nil, fmt.Errorf("token not found") } diff --git a/routers/repo/http.go b/routers/repo/http.go index 2a0f2bbf5bf2b..8a68618a3eed2 100644 --- a/routers/repo/http.go +++ b/routers/repo/http.go @@ -22,16 +22,13 @@ import ( "time" "code.gitea.io/gitea/models" - "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" - "code.gitea.io/gitea/services/auth" repo_service "code.gitea.io/gitea/services/repository" ) @@ -168,111 +165,25 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { // check access if askAuth { - // FIXME: middlewares/context.go did basic auth check already, - // maybe could use that one. - if setting.Service.EnableReverseProxyAuth { - authUsername := ctx.Req.Header.Get(setting.ReverseProxyAuthUser) - if len(authUsername) > 0 { - authUser, err = models.GetUserByName(authUsername) - if err != nil { - ctx.HandleText(401, "reverse proxy login error, got error while running GetUserByName") - 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 } - if authUser == nil { - authHead := ctx.Req.Header.Get("Authorization") - if len(authHead) == 0 { - ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=\".\"") - ctx.Error(http.StatusUnauthorized) - return - } - - auths := strings.SplitN(authHead, " ", 2) - // currently check basic auth - // TODO: support digit auth - if len(auths) != 2 || strings.ToLower(auths[0]) != "basic" { - ctx.HandleText(http.StatusUnauthorized, "no basic auth and digit auth") - return - } - authUsername, authPasswd, err := base.BasicAuthDecode(auths[1]) - if err != nil || (len(authUsername) == 0 && len(authPasswd) == 0) { - 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 := auth.CheckOAuthAccessToken(authToken) - if uid != 0 { - ctx.Data["IsApiToken"] = true - - authUser, err = models.GetUserByID(uid) - if err != nil { - ctx.ServerError("GetUserByID", err) - return - } - } - if authUser == nil { - // Assume password is a token. - token, err := models.GetAccessTokenBySHA(authToken) - if err == nil { - authUser, err = models.GetUserByID(token.UID) - if err != nil { - ctx.ServerError("GetUserByID", err) - return - } - - ctx.Data["IsApiToken"] = true - - 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) - } - } + authUser = ctx.User - if authUser == nil && !setting.Service.EnableBasicAuth { - ctx.HandleText(http.StatusUnauthorized, fmt.Sprintf("invalid credentials from %s", ctx.RemoteAddr())) + if ctx.IsBasicAuth { + _, 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 - } - - 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 - } } } diff --git a/services/auth/authenticators.go b/services/auth/authenticators.go index 3c56955ee7d0c..1f6ce11b058b8 100644 --- a/services/auth/authenticators.go +++ b/services/auth/authenticators.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 authMethods = []Authenticator{ &OAuth2{}, - &Session{}, &ReverseProxy{}, &Basic{}, + &Session{}, } // AuthenticationMechanism represents possible authentication mechanisms @@ -101,6 +103,20 @@ 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 { + log.Info("Checking: %q", req.URL.Path) + 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/services/auth/basic.go b/services/auth/basic.go index adc62ba1a301c..62d1cd3878d51 100644 --- a/services/auth/basic.go +++ b/services/auth/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" ) // BasicAuthenticationMechanism represents authentication using Basic authentication @@ -51,6 +52,13 @@ 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.IsAPIPath(req) && !isAttachmentDownload(req) && !isGitOrLFSPath(req) { + log.Info("Skipping BASIC authentication") + return nil + } + log.Info("Doing BASIC authentication") + baHead := req.Header.Get("Authorization") if len(baHead) == 0 { return nil diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index e2de0a6729c81..b2c8844996f0f 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -130,7 +130,7 @@ func (o *OAuth2) VerifyAuthData(req *http.Request, w http.ResponseWriter, store return nil } - if middleware.IsInternalPath(req) || !middleware.IsAPIPath(req) && !isAttachmentDownload(req) { + if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) { return nil } diff --git a/services/auth/reverseproxy.go b/services/auth/reverseproxy.go index 88ca7a14c83ba..88f837fa3b72d 100644 --- a/services/auth/reverseproxy.go +++ b/services/auth/reverseproxy.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/web/middleware" gouuid "github.com/google/uuid" ) @@ -71,12 +72,16 @@ func (r *ReverseProxy) VerifyAuthData(req *http.Request, w http.ResponseWriter, user, err := models.GetUserByName(username) if err != nil { - if models.IsErrUserNotExist(err) && r.isAutoRegisterAllowed() { - store.GetData()["AuthenticationMechanism"] = ReverseProxyMechanism - return r.newUser(req) + if !models.IsErrUserNotExist(err) || r.isAutoRegisterAllowed() { + log.Error("GetUserByName: %v", err) + return nil } - log.Error("GetUserByName: %v", err) - return nil + user = r.newUser(req) + } + + // Make sure requests to API paths and PWA resources do not create a new session + if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isGitOrLFSPath(req) { + handleSignIn(w, req, sess, user) } store.GetData()["AuthenticationMechanism"] = ReverseProxyMechanism From 511f4cc3c3eb968c0af23cde44927608e8aaff00 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 5 Apr 2021 21:20:44 +0100 Subject: [PATCH 18/19] remove unused functions Signed-off-by: Andrew Thornton --- modules/lfs/server.go | 49 ------------------------------------------- 1 file changed, 49 deletions(-) diff --git a/modules/lfs/server.go b/modules/lfs/server.go index b3b39b0e93240..1f9f5aecf4435 100644 --- a/modules/lfs/server.go +++ b/modules/lfs/server.go @@ -20,7 +20,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" - "code.gitea.io/gitea/modules/timeutil" "github.com/dgrijalva/jwt-go" jsoniter "github.com/json-iterator/go" @@ -660,54 +659,6 @@ func handleLFSToken(tokenSHA string, target *models.Repository, mode models.Acce return u, nil } -func handleOAuth2Token(tokenSHA string) (*models.User, error) { - if !strings.Contains(tokenSHA, ".") { - return nil, nil - } - token, err := models.ParseOAuth2Token(tokenSHA) - if err != nil { - if err.Error() == "invalid token" { - return nil, err - } - return nil, nil - } - if token.Type != models.TypeAccessToken || token.ExpiresAt < time.Now().Unix() || token.IssuedAt > time.Now().Unix() { - return nil, fmt.Errorf("invalid token claim") - } - grant, err := models.GetOAuth2GrantByID(token.GrantID) - if err != nil { - log.Error("Unable to GetOAuth2GrantByID[%d]: Error: %v", token.GrantID, err) - return nil, err - } - u, err := models.GetUserByID(grant.UserID) - if err != nil { - log.Error("Unable to GetUserById[%d]: Error: %v", grant.UserID, err) - return nil, err - } - - return u, nil -} - -func handleStandardToken(tokenSHA string) (*models.User, error) { - t, err := models.GetAccessTokenBySHA(tokenSHA) - if err != nil { - if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) { - log.Error("GetAccessTokenBySHA: %v", err) - } - return nil, err - } - t.UpdatedUnix = timeutil.TimeStampNow() - if err = models.UpdateAccessToken(t); err != nil { - log.Error("UpdateAccessToken: %v", err) - } - u, err := models.GetUserByID(t.UID) - if err != nil { - log.Error("Unable to GetUserById[%d]: Error: %v", t.UID, err) - return nil, err - } - return u, nil -} - func parseToken(authorization string, target *models.Repository, mode models.AccessMode) (*models.User, error) { if authorization == "" { return nil, fmt.Errorf("no token") From e39321b5712d13f4c6ac58590d22b763c1d24ac6 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 5 Apr 2021 21:31:54 +0100 Subject: [PATCH 19/19] fix lint Signed-off-by: Andrew Thornton --- services/auth/basic.go | 2 +- services/auth/oauth2.go | 2 +- services/auth/reverseproxy.go | 2 +- services/auth/sspi_windows.go | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/services/auth/basic.go b/services/auth/basic.go index 62d1cd3878d51..a5c45efb8df4d 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -25,7 +25,7 @@ var ( _ Authenticator = &Basic{} ) -// Basic implements the SingleSignOn interface and authenticates requests (API requests +// Basic implements the Authenticator interface and authenticates requests (API requests // only) by looking for Basic authentication data or "x-oauth-basic" token in the "Authorization" // header. type Basic struct { diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index b2c8844996f0f..7aef2cd53137d 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -51,7 +51,7 @@ func CheckOAuthAccessToken(accessToken string) int64 { return grant.UserID } -// OAuth2 implements the SingleSignOn interface and authenticates requests +// OAuth2 implements the Authenticator interface and authenticates requests // (API requests only) by looking for an OAuth token in query parameters or the // "Authorization" header. type OAuth2 struct { diff --git a/services/auth/reverseproxy.go b/services/auth/reverseproxy.go index 88f837fa3b72d..02b83356e7163 100644 --- a/services/auth/reverseproxy.go +++ b/services/auth/reverseproxy.go @@ -25,7 +25,7 @@ var ( _ Authenticator = &ReverseProxy{} ) -// ReverseProxy implements the SingleSignOn interface, but actually relies on +// ReverseProxy implements the Authenticator interface, but actually relies on // a reverse proxy for authentication of users. // On successful authentication the proxy is expected to populate the username in the // "setting.ReverseProxyAuthUser" header. Optionally it can also populate the email of the diff --git a/services/auth/sspi_windows.go b/services/auth/sspi_windows.go index a304cdedac097..fc827e43c9808 100644 --- a/services/auth/sspi_windows.go +++ b/services/auth/sspi_windows.go @@ -35,10 +35,10 @@ var ( sspiAuth *websspi.Authenticator // Ensure the struct implements the interface. - _ SingleSignOn = &SSPI{} + _ Authenticator = &SSPI{} ) -// SSPI implements the SingleSignOn interface and authenticates requests +// SSPI implements the Authenticator interface and authenticates requests // via the built-in SSPI module in Windows for SPNEGO authentication. // On successful authentication returns a valid user object. // Returns nil if authentication fails.