refactor auth interface to return error when verify failure

modules/context/api.go

@@ -219,7 +219,13 @@ func (ctx *APIContext) CheckForOTP() {
 func APIAuth(authMethod auth_service.Method) func(*APIContext) {
 return func(ctx *APIContext) {
 // Get user from session if logged in.
-ctx.Doer = authMethod.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+var err error
+ctx.Doer, err = authMethod.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+if err != nil {
+ctx.Error(http.StatusUnauthorized, "APIAuth", err)
+return
+}
+
 if ctx.Doer != nil {
 if ctx.Locale.Language() != ctx.Doer.Language {
 ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req)

modules/context/context.go

@@ -662,7 +662,13 @@ func getCsrfOpts() CsrfOptions {
 // Auth converts auth.Auth as a middleware
 func Auth(authMethod auth.Method) func(*Context) {
 return func(ctx *Context) {
-ctx.Doer = authMethod.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+var err error
+ctx.Doer, err = authMethod.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+if err != nil {
+log.Error("Failed to verify user %v: %v", ctx.Req.RemoteAddr, err)
+ctx.Error(http.StatusUnauthorized, "Verify")
+return
+}
 if ctx.Doer != nil {
 if ctx.Locale.Language() != ctx.Doer.Language {
 ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req)

routers/api/packages/api.go

@@ -11,6 +11,7 @@ import (
 
 "code.gitea.io/gitea/models/perm"
 "code.gitea.io/gitea/modules/context"
+"code.gitea.io/gitea/modules/log"
 "code.gitea.io/gitea/modules/setting"
 "code.gitea.io/gitea/modules/web"
 "code.gitea.io/gitea/routers/api/packages/composer"
@@ -58,7 +59,13 @@ func CommonRoutes(ctx gocontext.Context) *web.Route {
 
 authGroup := auth.NewGroup(authMethods...)
 r.Use(func(ctx *context.Context) {
-ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+var err error
+ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+if err != nil {
+log.Error("Verify: %v", err)
+ctx.Error(http.StatusUnauthorized, "authGroup.Verify")
+return
+}
 ctx.IsSigned = ctx.Doer != nil
 })
 
@@ -321,7 +328,13 @@ func ContainerRoutes(ctx gocontext.Context) *web.Route {
 
 authGroup := auth.NewGroup(authMethods...)
 r.Use(func(ctx *context.Context) {
-ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+var err error
+ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
+if err != nil {
+log.Error("Failed to verify user: %v", err)
+ctx.Error(http.StatusUnauthorized, "Verify")
+return
+}
 ctx.IsSigned = ctx.Doer != nil
 })
 

routers/api/packages/conan/auth.go

@@ -19,22 +19,22 @@ func (a *Auth) Name() string {
 }
 
 // Verify extracts the user from the Bearer token
-func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) *user_model.User {
+func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
 uid, err := packages.ParseAuthorizationToken(req)
 if err != nil {
 log.Trace("ParseAuthorizationToken: %v", err)
-return nil
+return nil, err
 }
 
 if uid == 0 {
-return nil
+return nil, nil
 }
 
 u, err := user_model.GetUserByID(req.Context(), uid)
 if err != nil {
 log.Error("GetUserByID: %v", err)
-return nil
+return nil, err
 }
 
-return u
+return u, nil
 }

routers/api/packages/container/auth.go

@@ -20,25 +20,25 @@ func (a *Auth) Name() string {
 
 // Verify extracts the user from the Bearer token
 // If it's an anonymous session a ghost user is returned
-func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) *user_model.User {
+func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
 uid, err := packages.ParseAuthorizationToken(req)
 if err != nil {
 log.Trace("ParseAuthorizationToken: %v", err)
-return nil
+return nil, err
 }
 
 if uid == 0 {
-return nil
+return nil, nil
 }
 if uid == -1 {
-return user_model.NewGhostUser()
+return user_model.NewGhostUser(), nil
 }
 
 u, err := user_model.GetUserByID(req.Context(), uid)
 if err != nil {
 log.Error("GetUserByID: %v", err)
-return nil
+return nil, err
 }
 
-return u
+return u, nil
 }

routers/api/packages/nuget/auth.go

@@ -20,25 +20,26 @@ func (a *Auth) Name() string {
 }
 
 // https://docs.microsoft.com/en-us/nuget/api/package-publish-resource#request-parameters
-func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) *user_model.User {
+func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
 token, err := auth_model.GetAccessTokenBySHA(req.Header.Get("X-NuGet-ApiKey"))
 if err != nil {
 if !(auth_model.IsErrAccessTokenNotExist(err) || auth_model.IsErrAccessTokenEmpty(err)) {
 log.Error("GetAccessTokenBySHA: %v", err)
+return nil, err
 }
-return nil
+return nil, nil
 }
 
 u, err := user_model.GetUserByID(req.Context(), token.UID)
 if err != nil {
 log.Error("GetUserByID: %v", err)
-return nil
+return nil, err
 }
 
 token.UpdatedUnix = timeutil.TimeStampNow()
 if err := auth_model.UpdateAccessToken(token); err != nil {
 log.Error("UpdateAccessToken: %v", err)
 }
 
-return u
+return u, nil
 }

services/auth/basic.go

@@ -40,20 +40,20 @@ func (b *Basic) Name() string {
 // "Authorization" header of the request and returns the corresponding user object for that
 // name/token on successful validation.
 // Returns nil if header is empty or validation fails.
-func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
+func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
 // Basic authentication should only fire on API, Download or on Git or LFSPaths
 if !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawReleaseOrLFSPath(req) {
-return nil
+return nil, nil
 }
 
 baHead := req.Header.Get("Authorization")
 if len(baHead) == 0 {
-return nil
+return nil, nil
 }
 
 auths := strings.SplitN(baHead, " ", 2)
 if len(auths) != 2 || (strings.ToLower(auths[0]) != "basic") {
-return nil
+return nil, nil
 }
 
 uname, passwd, _ := base.BasicAuthDecode(auths[1])
@@ -77,11 +77,11 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
 u, err := user_model.GetUserByID(req.Context(), uid)
 if err != nil {
 log.Error("GetUserByID: %v", err)
-return nil
+return nil, err
 }
 
 store.GetData()["IsApiToken"] = true
-return u
+return u, nil
 }
 
 token, err := auth_model.GetAccessTokenBySHA(authToken)
@@ -90,7 +90,7 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
 u, err := user_model.GetUserByID(req.Context(), token.UID)
 if err != nil {
 log.Error("GetUserByID: %v", err)
-return nil
+return nil, err
 }
 
 token.UpdatedUnix = timeutil.TimeStampNow()
@@ -99,13 +99,13 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
 }
 
 store.GetData()["IsApiToken"] = true
-return u
+return u, nil
 } else if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) {
 log.Error("GetAccessTokenBySha: %v", err)
 }
 
 if !setting.Service.EnableBasicAuth {
-return nil
+return nil, nil
 }
 
 log.Trace("Basic Authorization: Attempting SignIn for %s", uname)
@@ -114,7 +114,7 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
 if !user_model.IsErrUserNotExist(err) {
 log.Error("UserSignIn: %v", err)
 }
-return nil
+return nil, err
 }
 
 if skipper, ok := source.Cfg.(LocalTwoFASkipper); ok && skipper.IsSkipLocalTwoFA() {
@@ -123,5 +123,5 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
 
 log.Trace("Basic Authorization: Logged in user %-v", u)
 
-return u
+return u, nil
 }

services/auth/group.go

@@ -9,7 +9,6 @@ import (
 "reflect"
 "strings"
 
-"code.gitea.io/gitea/models/db"
 user_model "code.gitea.io/gitea/models/user"
 )
 
@@ -80,23 +79,23 @@ func (b *Group) Free() error {
 }
 
 // Verify extracts and validates
-func (b *Group) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
-if !db.HasEngine {
-return nil
-}
-
+func (b *Group) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
 // Try to sign in with each of the enabled plugins
 for _, ssoMethod := range b.methods {
-user := ssoMethod.Verify(req, w, store, sess)
+user, err := ssoMethod.Verify(req, w, store, sess)
+if err != nil {
+return user, err
+}
+
 if user != nil {
 if store.GetData()["AuthedMethod"] == nil {
 if named, ok := ssoMethod.(Named); ok {
 store.GetData()["AuthedMethod"] = named.Name()
 }
 }
-return user
+return user, nil
 }
 }
 
-return nil
+return nil, nil
 }

services/auth/httpsign.go

@@ -39,10 +39,10 @@ func (h *HTTPSign) Name() string {
 // Verify extracts and validates HTTPsign from the Signature header of the request and returns
 // the corresponding user object on successful validation.
 // Returns nil if header is empty or validation fails.
-func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
+func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
 sigHead := req.Header.Get("Signature")
 if len(sigHead) == 0 {
-return nil
+return nil, nil
 }
 
 var (
@@ -53,36 +53,36 @@ func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataSt
 if len(req.Header.Get("X-Ssh-Certificate")) != 0 {
 // Handle Signature signed by SSH certificates
 if len(setting.SSH.TrustedUserCAKeys) == 0 {
-return nil
+return nil, nil
 }
 
 publicKey, err = VerifyCert(req)
 if err != nil {
 log.Debug("VerifyCert on request from %s: failed: %v", req.RemoteAddr, err)
 log.Warn("Failed authentication attempt from %s", req.RemoteAddr)
-return nil
+return nil, nil
 }
 } else {
 // Handle Signature signed by Public Key
 publicKey, err = VerifyPubKey(req)
 if err != nil {
 log.Debug("VerifyPubKey on request from %s: failed: %v", req.RemoteAddr, err)
 log.Warn("Failed authentication attempt from %s", req.RemoteAddr)
-return nil
+return nil, nil
 }
 }
 
 u, err := user_model.GetUserByID(req.Context(), publicKey.OwnerID)
 if err != nil {
 log.Error("GetUserByID: %v", err)
-return nil
+return nil, err
 }
 
 store.GetData()["IsApiToken"] = true
 
 log.Trace("HTTP Sign: Logged in user %-v", u)
 
-return u
+return u, nil
 }
 
 func VerifyPubKey(r *http.Request) (*asymkey_model.PublicKey, error) {

services/auth/interface.go

@@ -24,8 +24,9 @@ type Method interface {
 // If verification is successful returns either an existing user object (with id > 0)
 // or a new user object (with id = 0) populated with the information that was found
 // in the authentication data (username or email).
-// Returns nil if verification fails.
-Verify(http *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User
+// Second argument returns err if verification fails, otherwise
+// First return argument returns nil if no matched verification condition
+Verify(http *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error)
 }
 
 // Initializable represents a structure that requires initialization

services/auth/oauth2.go

@@ -108,18 +108,14 @@ func (o *OAuth2) userIDFromToken(req *http.Request, store DataStore) int64 {
 // or the "Authorization" header and returns the corresponding user object for that ID.
 // If verification is successful returns an existing user object.
 // Returns nil if verification fails.
-func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
-if !db.HasEngine {
-return nil
-}
-
+func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
 if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) {
-return nil
+return nil, nil
 }
 
 id := o.userIDFromToken(req, store)
 if id <= 0 {
-return nil
+return nil, nil
 }
 log.Trace("OAuth2 Authorization: Found token for user[%d]", id)
 
@@ -128,11 +124,11 @@ func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStor
 if !user_model.IsErrUserNotExist(err) {
 log.Error("GetUserByName: %v", err)
 }
-return nil
+return nil, err
 }
 
 log.Trace("OAuth2 Authorization: Logged in user %-v", user)
-return user
+return user, nil
 }
 
 func isAuthenticatedTokenRequest(req *http.Request) bool {