[v15.0/forgejo] refactor: clarify four different outputs that authentication methods provide (#12468)

**Backport:** https://codeberg.org/forgejo/forgejo/pulls/12231

#12202 began a refactor of Forgejo's authentication implementations by providing structured data on an authentication success.  However, error cases were maintained as-is in that refactor, leaving a complex situation: what does returning an error from an authentication method mean?; does it mean that the authentication failed, or that a server error occurred?  Can another authentication still be tried?

This PR changes authentication methods so that they can return one of four things:
- `AuthenticationSuccess` with an authentication result.
- `AuthenticationNotAttempted` which indicates that no credentials relevant for this authentication method were presented.  If every method returned `AuthenticationNotAttempted`, then you would have an unauthenticated access.
- `AuthenticationAttemptedIncorrectCredential` which indicates that credentials were present and failed validation -- a situation indicating a `401 Unauthorized`.
- `AuthenticationError` which indicates that an internal server error occurred and failed authentication -- indicating a `500 Internal Server Error`.

This paves the way for one more refactor coming next: `basic.go` and `oauth2.go` perform 3-4 different authentications each (access tokens, oauth JWTs, actions tokens, actions JWTs, and username/password).  With the capability to return these more precise responses, these authentication methods can be split up into separate logic that isn't intertwined together.

Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12468
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This commit is contained in:
forgejo-backport-action 2026-05-08 07:31:33 +02:00 committed by Gusted
parent 0aa1b45956
commit a1222ebb5b
17 changed files with 340 additions and 229 deletions

View file

@ -4,6 +4,7 @@
package packages package packages
import ( import (
"errors"
"net/http" "net/http"
"regexp" "regexp"
"strings" "strings"
@ -119,19 +120,30 @@ func verifyAuth(r *web.Route, authMethods []auth.Method) {
authGroup := auth_method.NewGroup(authMethods...) authGroup := auth_method.NewGroup(authMethods...)
r.Use(func(ctx *context.Context) { r.Use(func(ctx *context.Context) {
authResult, err := authGroup.Verify(ctx.Req, ctx.Resp, ctx.Session) output := authGroup.Verify(ctx.Req, ctx.Resp, ctx.Session)
if err != nil { var ar auth.AuthenticationResult
log.Info("Failed to verify user: %v", err) switch v := output.(type) {
case *auth.AuthenticationSuccess:
ar = v.Result
case *auth.AuthenticationNotAttempted:
ar = &auth.UnauthenticatedResult{}
case *auth.AuthenticationError:
ctx.ServerError("authentication error", v.Error)
return
case *auth.AuthenticationAttemptedIncorrectCredential:
ctx.Error(http.StatusUnauthorized, "authGroup.Verify") ctx.Error(http.StatusUnauthorized, "authGroup.Verify")
return return
} default:
if authResult == nil { ctx.ServerError("Verify failed", errors.New("unexpected result from Method.Verify"))
ctx.Error(http.StatusInternalServerError, "verifyAuth nil authentication result")
return return
} }
ctx.Doer = authResult.User() if ar == nil {
ctx.ServerError("nil authentication result", errors.New("nil authentication result"))
return
}
ctx.Doer = ar.User()
ctx.IsSigned = ctx.Doer != nil ctx.IsSigned = ctx.Doer != nil
ctx.Authentication = authResult ctx.Authentication = ar
}) })
} }
@ -142,20 +154,32 @@ func verifyContainerAuth(r *web.Route, authMethods []auth.Method) {
authGroup := auth_method.NewGroup(authMethods...) authGroup := auth_method.NewGroup(authMethods...)
r.Use(func(ctx *context.Context) { r.Use(func(ctx *context.Context) {
authResult, err := authGroup.Verify(ctx.Req, ctx.Resp, ctx.Session) output := authGroup.Verify(ctx.Req, ctx.Resp, ctx.Session)
if err != nil { var ar auth.AuthenticationResult
log.Info("Failed to verify user: %v", err) switch v := output.(type) {
case *auth.AuthenticationSuccess:
ar = v.Result
case *auth.AuthenticationNotAttempted:
ar = &auth.UnauthenticatedResult{}
case *auth.AuthenticationError:
ctx.ServerError("authentication error", v.Error)
return
case *auth.AuthenticationAttemptedIncorrectCredential:
log.Info("Failed to verify user: %v", v.Error)
container.APIUnauthorizedError(ctx) container.APIUnauthorizedError(ctx)
ctx.Error(http.StatusUnauthorized, "authGroup.Verify") ctx.Error(http.StatusUnauthorized, "authGroup.Verify")
return return
} default:
if authResult == nil { ctx.ServerError("Verify failed", errors.New("unexpected result from Method.Verify"))
ctx.Error(http.StatusInternalServerError, "verifyContainerAuth nil authentication result")
return return
} }
ctx.Doer = authResult.User() if ar == nil {
ctx.ServerError("nil authentication result", errors.New("nil authentication result"))
return
}
ctx.Doer = ar.User()
ctx.IsSigned = ctx.Doer != nil ctx.IsSigned = ctx.Doer != nil
ctx.Authentication = authResult ctx.Authentication = ar
}) })
} }

View file

@ -65,34 +65,37 @@ func (a *Auth) Name() string {
// Verify extracts the user from the signed request // Verify extracts the user from the signed request
// If the request is signed with the user private key the user is verified. // If the request is signed with the user private key the user is verified.
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) { func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) auth.MethodOutput {
u, err := getUserFromRequest(req) u, err := getUserFromRequest(req)
if err != nil { if err != nil {
return nil, err if !user_model.IsErrUserNotExist(err) {
return &auth.AuthenticationError{Error: fmt.Errorf("chef auth getUserFromRequest: %w", err)}
}
return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
if u == nil { if u == nil {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
pub, err := getUserPublicKey(req.Context(), u) pub, err := getUserPublicKey(req.Context(), u)
if err != nil { if err != nil {
return nil, err return &auth.AuthenticationError{Error: fmt.Errorf("chef auth getUserPublicKey: %w", err)}
} }
if err := verifyTimestamp(req); err != nil { if err := verifyTimestamp(req); err != nil {
return nil, err return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
version, err := getSignVersion(req) version, err := getSignVersion(req)
if err != nil { if err != nil {
return nil, err return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
if err := verifySignedHeaders(req, version, pub.(*rsa.PublicKey)); err != nil { if err := verifySignedHeaders(req, version, pub.(*rsa.PublicKey)); err != nil {
return nil, err return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
return &chefAuthenticationResult{user: u}, nil return &auth.AuthenticationSuccess{Result: &chefAuthenticationResult{user: u}}
} }
func getUserFromRequest(req *http.Request) (*user_model.User, error) { func getUserFromRequest(req *http.Request) (*user_model.User, error) {

View file

@ -4,6 +4,7 @@
package conan package conan
import ( import (
"fmt"
"net/http" "net/http"
auth_model "forgejo.org/models/auth" auth_model "forgejo.org/models/auth"
@ -40,15 +41,18 @@ func (a *Auth) Name() string {
} }
// Verify extracts the user from the Bearer token // Verify extracts the user from the Bearer token
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) { func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) auth.MethodOutput {
uid, scope, err := packages.ParseAuthorizationToken(req) uid, scope, err := packages.ParseAuthorizationToken(req)
if err != nil { if err != nil {
log.Trace("ParseAuthorizationToken: %v", err) log.Trace("ParseAuthorizationToken: %v", err)
return nil, err // Errors from ParseAuthorizationToken are almost all from malformed incoming input, which we'll consider an
} // auth failure:
// - `Authorization` header was present for all cases, so it's not `AuthenticationNotAttempted`
if uid == 0 { // - it's not `AuthenticationError` because malformed headers would cause errors, and this is intended for
return &auth.UnauthenticatedResult{}, nil // server errors which should cause 500s
return &auth.AuthenticationAttemptedIncorrectCredential{Error: fmt.Errorf("conan auth JWT error: %w", err)}
} else if uid == 0 {
return &auth.AuthenticationNotAttempted{}
} }
// Propagate scope of the authorization token. // Propagate scope of the authorization token.
@ -60,8 +64,8 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.Sessio
u, err := user_model.GetUserByID(req.Context(), uid) u, err := user_model.GetUserByID(req.Context(), uid)
if err != nil { if err != nil {
log.Error("GetUserByID: %v", err) log.Error("GetUserByID: %v", err)
return nil, err return &auth.AuthenticationError{Error: fmt.Errorf("conan auth GetUserByID failed: %w", err)}
} }
return &conanAuthenticationResult{user: u, scope: authScope}, nil return &auth.AuthenticationSuccess{Result: &conanAuthenticationResult{user: u, scope: authScope}}
} }

View file

@ -4,6 +4,7 @@
package container package container
import ( import (
"fmt"
"net/http" "net/http"
auth_model "forgejo.org/models/auth" auth_model "forgejo.org/models/auth"
@ -41,15 +42,18 @@ func (a *Auth) Name() string {
// Verify extracts the user from the Bearer token // Verify extracts the user from the Bearer token
// If it's an anonymous session a ghost user is returned // If it's an anonymous session a ghost user is returned
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) { func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) auth.MethodOutput {
uid, scope, err := packages.ParseAuthorizationToken(req) uid, scope, err := packages.ParseAuthorizationToken(req)
if err != nil { if err != nil {
log.Trace("ParseAuthorizationToken: %v", err) log.Trace("ParseAuthorizationToken: %v", err)
return nil, err // Errors from ParseAuthorizationToken are almost all from malformed incoming input, which we'll consider an
} // auth failure:
// - `Authorization` header was present for all cases, so it's not `AuthenticationNotAttempted`
if uid == 0 { // - it's not `AuthenticationError` because malformed headers would cause errors, and this is intended for
return &auth.UnauthenticatedResult{}, nil // server errors which should cause 500s
return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} else if uid == 0 {
return &auth.AuthenticationNotAttempted{}
} }
// Propagate scope of the authorization token. // Propagate scope of the authorization token.
@ -60,9 +64,8 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.Sessio
u, err := user_model.GetPossibleUserByID(req.Context(), uid) u, err := user_model.GetPossibleUserByID(req.Context(), uid)
if err != nil { if err != nil {
log.Error("GetPossibleUserByID: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("container auth GetPossibleUserByID: %w", err)}
return nil, err
} }
return &containerAuthenticationResult{user: u, scope: authScope}, nil return &auth.AuthenticationSuccess{Result: &containerAuthenticationResult{user: u, scope: authScope}}
} }

View file

@ -4,6 +4,7 @@
package nuget package nuget
import ( import (
"fmt"
"net/http" "net/http"
auth_model "forgejo.org/models/auth" auth_model "forgejo.org/models/auth"
@ -26,34 +27,30 @@ func (r *nugetAuthenticationResult) User() *user_model.User {
return r.user return r.user
} }
var _ auth.Method = &Auth{}
type Auth struct{} type Auth struct{}
func (a *Auth) Name() string {
return "nuget"
}
// https://docs.microsoft.com/en-us/nuget/api/package-publish-resource#request-parameters // https://docs.microsoft.com/en-us/nuget/api/package-publish-resource#request-parameters
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) { func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) auth.MethodOutput {
token, err := auth_model.GetAccessTokenBySHA(req.Context(), req.Header.Get("X-NuGet-ApiKey")) apiKey := req.Header.Get("X-NuGet-ApiKey")
if apiKey == "" {
return &auth.AuthenticationNotAttempted{}
}
token, err := auth_model.GetAccessTokenBySHA(req.Context(), apiKey)
if err != nil { if err != nil {
if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) { if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) {
log.Error("GetAccessTokenBySHA: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("nuget auth GetAccessTokenBySHA: %w", err)}
return nil, err
} }
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
u, err := user_model.GetUserByID(req.Context(), token.UID) u, err := user_model.GetUserByID(req.Context(), token.UID)
if err != nil { if err != nil {
log.Error("GetUserByID: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("nuget auth GetUserByID: %w", err)}
return nil, err
} }
if err := token.UpdateLastUsed(req.Context()); err != nil { if err := token.UpdateLastUsed(req.Context()); err != nil {
log.Error("UpdateLastUsed: %v", err) log.Error("UpdateLastUsed: %v", err)
} }
return &nugetAuthenticationResult{user: u}, nil return &auth.AuthenticationSuccess{Result: &nugetAuthenticationResult{user: u}}
} }

View file

@ -60,13 +60,25 @@ func buildAuthGroup() *auth_method.Group {
func apiAuthentication(authMethod auth.Method) func(*context.APIContext) { func apiAuthentication(authMethod auth.Method) func(*context.APIContext) {
return func(ctx *context.APIContext) { return func(ctx *context.APIContext) {
ar, err := common.AuthShared(ctx.Base, nil, authMethod) output := common.AuthShared(ctx.Base, nil, authMethod)
if err != nil { var ar auth.AuthenticationResult
ctx.Error(http.StatusUnauthorized, "APIAuth", err) switch v := output.(type) {
case *auth.AuthenticationSuccess:
ar = v.Result
case *auth.AuthenticationNotAttempted:
ar = &auth.UnauthenticatedResult{}
case *auth.AuthenticationAttemptedIncorrectCredential:
ctx.Error(http.StatusUnauthorized, "APIAuth", v.Error)
return
case *auth.AuthenticationError:
ctx.ServerError("authentication error", v.Error)
return
default:
ctx.ServerError("authentication error", errors.New("unexpected result from common.AuthShared"))
return return
} }
if ar == nil { if ar == nil {
ctx.Error(http.StatusInternalServerError, "apiAuthentication nil authentication result", errors.New("nil authentication result")) ctx.ServerError("nil authentication result", errors.New("nil authentication result"))
return return
} }
ctx.Doer = ar.User() ctx.Doer = ar.User()

View file

@ -4,35 +4,37 @@
package common package common
import ( import (
"errors"
"forgejo.org/modules/web/middleware" "forgejo.org/modules/web/middleware"
auth_service "forgejo.org/services/auth" auth_service "forgejo.org/services/auth"
"forgejo.org/services/context" "forgejo.org/services/context"
) )
func AuthShared(ctx *context.Base, sessionStore auth_service.SessionStore, authMethod auth_service.Method) (ar auth_service.AuthenticationResult, err error) { func AuthShared(ctx *context.Base, sessionStore auth_service.SessionStore, authMethod auth_service.Method) auth_service.MethodOutput {
ar, err = authMethod.Verify(ctx.Req, ctx.Resp, sessionStore) output := authMethod.Verify(ctx.Req, ctx.Resp, sessionStore)
if err != nil {
return ar, err var ar auth_service.AuthenticationResult
switch v := output.(type) {
case *auth_service.AuthenticationSuccess:
ar = v.Result
case *auth_service.AuthenticationNotAttempted:
ar = &auth_service.UnauthenticatedResult{}
} }
if ar == nil { if ar != nil {
return nil, errors.New("failure to retrieve AuthenticationResult - nil value") doer := ar.User()
} if doer != nil {
doer := ar.User() if ctx.Locale.Language() != doer.Language {
if doer != nil { ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req)
if ctx.Locale.Language() != doer.Language { }
ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req) ctx.Data["IsSigned"] = true
ctx.Data[middleware.ContextDataKeySignedUser] = doer
ctx.Data["SignedUserID"] = doer.ID
ctx.Data["IsAdmin"] = doer.IsAdmin
} else {
ctx.Data["IsSigned"] = false
ctx.Data["SignedUserID"] = int64(0)
} }
ctx.Data["IsSigned"] = true
ctx.Data[middleware.ContextDataKeySignedUser] = doer
ctx.Data["SignedUserID"] = doer.ID
ctx.Data["IsAdmin"] = doer.IsAdmin
} else {
ctx.Data["IsSigned"] = false
ctx.Data["SignedUserID"] = int64(0)
} }
return ar, nil return output
} }
// VerifyOptions contains required or check options // VerifyOptions contains required or check options

View file

@ -6,6 +6,7 @@ package web
import ( import (
gocontext "context" gocontext "context"
"errors"
"fmt" "fmt"
"net/http" "net/http"
"strings" "strings"
@ -120,14 +121,28 @@ func buildAuthGroup() *auth_method.Group {
func webAuth(authMethod auth_service.Method) func(*context.Context) { func webAuth(authMethod auth_service.Method) func(*context.Context) {
return func(ctx *context.Context) { return func(ctx *context.Context) {
ar, err := common.AuthShared(ctx.Base, ctx.Session, authMethod) output := common.AuthShared(ctx.Base, ctx.Session, authMethod)
if err != nil { var ar auth_service.AuthenticationResult
log.Info("Failed to verify user: %v", err) switch v := output.(type) {
case *auth_service.AuthenticationSuccess:
ar = v.Result
case *auth_service.AuthenticationNotAttempted:
ar = &auth_service.UnauthenticatedResult{}
case *auth_service.AuthenticationAttemptedIncorrectCredential:
ctx.Error(http.StatusUnauthorized, ctx.Locale.TrString("auth.unauthorized_credentials", "https://codeberg.org/forgejo/forgejo/issues/2809")) ctx.Error(http.StatusUnauthorized, ctx.Locale.TrString("auth.unauthorized_credentials", "https://codeberg.org/forgejo/forgejo/issues/2809"))
return return
case *auth_service.AuthenticationError:
// Don't reveal the internal server error details to the user as they may contain sensitive details -- log
// the details, return a generic error.
log.Error("internal error during authentication: %v", v.Error)
ctx.ServerError("authentication error", errors.New("internal server error in authentication"))
return
default:
ctx.ServerError("authentication error", errors.New("unexpected result from common.AuthShared"))
return
} }
if ar == nil { if ar == nil {
ctx.Error(http.StatusInternalServerError, "webAuth nil authentication result") ctx.ServerError("nil authentication result", errors.New("nil authentication result"))
return return
} }
ctx.Doer = ar.User() ctx.Doer = ar.User()

View file

@ -23,13 +23,49 @@ type SessionStore session.Store
// Method represents an authentication method (plugin) for HTTP requests. // Method represents an authentication method (plugin) for HTTP requests.
type Method interface { type Method interface {
// Verify tries to verify the authentication data contained in the request. If verification is successful returns an // Verify tries to validate credentials provided in the request, and returns one of the [MethodOutput] results
// AuthenticationResult implementation with details about the authentication, or, may return an // indicating the result of its validation.
// AnonymousAuthentication if the authentication method doesn't indicate that the request is authenticated. An error Verify(http *http.Request, w http.ResponseWriter, sess SessionStore) MethodOutput
// is only returned if a failure occurred while checking authentication.
Verify(http *http.Request, w http.ResponseWriter, sess SessionStore) (AuthenticationResult, error)
} }
// When attempting to authenticate with an authentication [Method], one of the MethodOutput implementations must be
// returned. This interface serves as a enum of supported outputs, plus related values for each enum. Outputs are
// [AuthenticationSuccess], [AuthenticationNotAttempted], [AuthenticationAttemptedWithFailure], and
// [AuthenticationError].
type MethodOutput interface {
isMethodOutput()
}
// Authentication positively identified the incoming request as successfully authenticated with the result in the
// attached [AuthenticationResult]. For example, if a username and password were provided and we confirmed that those
// credentials matched an existing user in the database, then AuthenticationSuccess would be returned.
type AuthenticationSuccess struct {
Result AuthenticationResult
}
// Authentication method was not found to be applicable for the given request. For example, if a request did not contain
// `Authorization: Basic ...`, then a basic authentication method would return AuthenticationNotAttempted to indicate
// that this method didn't apply to the incoming request.
type AuthenticationNotAttempted struct{}
// Authentication method was attempted against the request and positively identified to be an incorrect credential. For
// example, if a request contained `Authorization: Basic ...`, and the username and password that were provided were
// found to not match any user, AuthenticationAttemptedIncorrectCredential would be returned. This is typically an
// indicator of a `401 Unauthorized ...` response.
type AuthenticationAttemptedIncorrectCredential struct {
Error error
}
// Authentication was attempted and an unexpected internal error occurred.
type AuthenticationError struct {
Error error
}
func (*AuthenticationSuccess) isMethodOutput() {}
func (*AuthenticationNotAttempted) isMethodOutput() {}
func (*AuthenticationAttemptedIncorrectCredential) isMethodOutput() {}
func (*AuthenticationError) isMethodOutput() {}
// PasswordAuthenticator represents a source of authentication // PasswordAuthenticator represents a source of authentication
type PasswordAuthenticator interface { type PasswordAuthenticator interface {
Authenticate(ctx context.Context, user *user_model.User, login, password string) (*user_model.User, error) Authenticate(ctx context.Context, user *user_model.User, login, password string) (*user_model.User, error)

View file

@ -6,6 +6,7 @@ package method
import ( import (
"errors" "errors"
"fmt"
"net/http" "net/http"
"strings" "strings"
@ -19,6 +20,7 @@ import (
"forgejo.org/modules/util" "forgejo.org/modules/util"
"forgejo.org/modules/web/middleware" "forgejo.org/modules/web/middleware"
"forgejo.org/services/auth" "forgejo.org/services/auth"
"forgejo.org/services/auth/source/db"
"forgejo.org/services/authz" "forgejo.org/services/authz"
) )
@ -36,20 +38,20 @@ type Basic struct{}
// "Authorization" header of the request and returns the corresponding user object for that // "Authorization" header of the request and returns the corresponding user object for that
// name/token on successful validation. // name/token on successful validation.
// Returns nil if header is empty or validation fails. // Returns nil if header is empty or validation fails.
func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) (auth.AuthenticationResult, error) { func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) auth.MethodOutput {
// Basic authentication should only fire on API, Download or on Git or LFSPaths // Basic authentication should only fire on API, Download or on Git or LFSPaths
if !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) { if !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
baHead := req.Header.Get("Authorization") baHead := req.Header.Get("Authorization")
if len(baHead) == 0 { if len(baHead) == 0 {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
auths := strings.SplitN(baHead, " ", 2) auths := strings.SplitN(baHead, " ", 2)
if len(auths) != 2 || (strings.ToLower(auths[0]) != "basic") { if len(auths) != 2 || (strings.ToLower(auths[0]) != "basic") {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
uname, passwd, _ := base.BasicAuthDecode(auths[1]) uname, passwd, _ := base.BasicAuthDecode(auths[1])
@ -73,8 +75,7 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionS
u, err := user_model.GetUserByID(req.Context(), uid) u, err := user_model.GetUserByID(req.Context(), uid)
if err != nil { if err != nil {
log.Error("GetUserByID: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("basic auth GetUserByID: %w", err)}
return nil, err
} }
var scope auth_model.AccessTokenScope var scope auth_model.AccessTokenScope
@ -83,17 +84,16 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionS
} else { } else {
scope = auth_model.AccessTokenScopeAll // fallback to all scope = auth_model.AccessTokenScopeAll // fallback to all
} }
return &oAuth2JWTAuthenticationResult{user: u, scope: optional.Some(scope)}, nil return &auth.AuthenticationSuccess{Result: &oAuth2JWTAuthenticationResult{user: u, scope: optional.Some(scope)}}
} }
// check personal access token // check personal access token
token, err := auth_model.GetAccessTokenBySHA(req.Context(), authToken) token, err := auth_model.GetAccessTokenBySHA(req.Context(), authToken)
if err == nil { if err == nil {
log.Trace("Basic Authorization: Valid AccessToken for user[%d]", uid) log.Trace("Basic Authorization: Valid AccessToken for user[%d]", token.UID)
u, err := user_model.GetUserByID(req.Context(), token.UID) u, err := user_model.GetUserByID(req.Context(), token.UID)
if err != nil { if err != nil {
log.Error("GetUserByID: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("basic auth GetUserByID for access token: %w", err)}
return nil, err
} }
if err = token.UpdateLastUsed(req.Context()); err != nil { if err = token.UpdateLastUsed(req.Context()); err != nil {
@ -102,11 +102,10 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionS
reducer, err := authz.GetAuthorizationReducerForAccessToken(req.Context(), token) reducer, err := authz.GetAuthorizationReducerForAccessToken(req.Context(), token)
if err != nil { if err != nil {
log.Error("authz.GetAuthorizationReducerForAccessToken: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("basic auth GetAuthorizationReducerForAccessToken: %w", err)}
return nil, err
} }
return &accessTokenAuthenticationResult{user: u, scope: token.Scope, reducer: reducer}, nil return &auth.AuthenticationSuccess{Result: &accessTokenAuthenticationResult{user: u, scope: token.Scope, reducer: reducer}}
} else if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) { } else if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) {
log.Error("GetAccessTokenBySha: %v", err) log.Error("GetAccessTokenBySha: %v", err)
} }
@ -115,41 +114,41 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionS
task, err := actions_model.GetRunningTaskByToken(req.Context(), authToken) task, err := actions_model.GetRunningTaskByToken(req.Context(), authToken)
if err == nil && task != nil { if err == nil && task != nil {
log.Trace("Basic Authorization: Valid AccessToken for task[%d]", task.ID) log.Trace("Basic Authorization: Valid AccessToken for task[%d]", task.ID)
return &actionsTaskTokenAuthenticationResult{user: user_model.NewActionsUser(), taskID: task.ID}, nil return &auth.AuthenticationSuccess{Result: &actionsTaskTokenAuthenticationResult{user: user_model.NewActionsUser(), taskID: task.ID}}
} }
if !setting.Service.EnableBasicAuth { if !setting.Service.EnableBasicAuth {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationAttemptedIncorrectCredential{Error: errors.New("basic authentication by username & password is disabled")}
} }
log.Trace("Basic Authorization: Attempting SignIn for %s", uname) log.Trace("Basic Authorization: Attempting SignIn for %s", uname)
u, source, err := UserSignIn(req.Context(), uname, passwd) u, source, err := UserSignIn(req.Context(), uname, passwd)
if err != nil { if err != nil {
if !user_model.IsErrUserNotExist(err) { if user_model.IsErrUserNotExist(err) || user_model.IsErrUserProhibitLogin(err) ||
log.Error("UserSignIn: %v", err) errors.As(err, &db.ErrUserPasswordInvalid{}) || errors.As(err, &db.ErrUserPasswordNotSet{}) {
return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
return nil, err return &auth.AuthenticationError{Error: fmt.Errorf("basic auth UserSignIn: %w", err)}
} }
hashWebAuthn, err := auth_model.HasWebAuthnRegistrationsByUID(req.Context(), u.ID) hashWebAuthn, err := auth_model.HasWebAuthnRegistrationsByUID(req.Context(), u.ID)
if err != nil { if err != nil {
log.Error("HasWebAuthnRegistrationsByUID: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("basic auth HasWebAuthnRegistrationsByUID: %w", err)}
return nil, err
} }
if hashWebAuthn { if hashWebAuthn {
return nil, errors.New("Basic authorization is not allowed while having security keys enrolled") return &auth.AuthenticationAttemptedIncorrectCredential{Error: errors.New("Basic authorization is not allowed while having security keys enrolled")}
} }
if skipper, ok := source.Cfg.(auth.LocalTwoFASkipper); !ok || !skipper.IsSkipLocalTwoFA() { if skipper, ok := source.Cfg.(auth.LocalTwoFASkipper); !ok || !skipper.IsSkipLocalTwoFA() {
if err := validateTOTP(req, u); err != nil { if err := validateTOTP(req, u); err != nil {
return nil, err return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
} }
log.Trace("Basic Authorization: Logged in user %-v", u) log.Trace("Basic Authorization: Logged in user %-v", u)
return &basicPaswordAuthenticationResult{user: u}, nil return &auth.AuthenticationSuccess{Result: &basicPaswordAuthenticationResult{user: u}}
} }
func getOtpHeader(header http.Header) string { func getOtpHeader(header http.Header) string {

View file

@ -4,6 +4,8 @@
package method package method
import ( import (
"errors"
"fmt"
"net/http" "net/http"
"forgejo.org/services/auth" "forgejo.org/services/auth"
@ -31,33 +33,34 @@ func (b *Group) Add(method auth.Method) {
b.methods = append(b.methods, method) b.methods = append(b.methods, method)
} }
func (b *Group) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) { func (b *Group) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) auth.MethodOutput {
// Try to sign in with each of the enabled plugins var incorrectCredentials []error
var retErr error
for _, m := range b.methods { for _, m := range b.methods {
authResult, err := m.Verify(req, w, sess) output := m.Verify(req, w, sess)
if err != nil {
if retErr == nil { switch v := output.(type) {
retErr = err case *auth.AuthenticationSuccess, *auth.AuthenticationError:
} return v
// Try other methods if this one failed.
// Some methods may share the same protocol to detect if they are matched. case *auth.AuthenticationNotAttempted:
// For example, OAuth2 and conan.Auth both read token from "Authorization: Bearer <token>" header, // Move on to the next supported authentication method.
// If OAuth2 returns error, we should give conan.Auth a chance to try.
continue continue
}
// If any method returns an authenticated result, we can stop trying. Return and ignore any error returned by case *auth.AuthenticationAttemptedIncorrectCredential:
// previous methods. // Move on to the next supported authentication method, but keep a record of this error. If none of the
if authResult.User() != nil { // other methods are able to authenticate the user, we'll report this as an incorrect credential (401) case.
return authResult, nil incorrectCredentials = append(incorrectCredentials, v.Error)
continue
default:
return &auth.AuthenticationError{Error: fmt.Errorf("unexpected result from Method.Verify on method %v: %v", m, v)}
} }
} }
if retErr != nil { if len(incorrectCredentials) != 0 {
// If no method returns a user, return the error returned by the first method. return &auth.AuthenticationAttemptedIncorrectCredential{Error: errors.Join(incorrectCredentials...)}
return nil, retErr
} }
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }

View file

@ -35,10 +35,10 @@ type HTTPSign struct{}
// Verify extracts and validates HTTPsign from the Signature header of the request and returns // Verify extracts and validates HTTPsign from the Signature header of the request and returns
// the corresponding user object on successful validation. // the corresponding user object on successful validation.
// Returns nil if header is empty or validation fails. // Returns nil if header is empty or validation fails.
func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) (auth.AuthenticationResult, error) { func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) auth.MethodOutput {
sigHead := req.Header.Get("Signature") sigHead := req.Header.Get("Signature")
if len(sigHead) == 0 { if len(sigHead) == 0 {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
var ( var (
@ -49,14 +49,14 @@ func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, _ auth.Sessi
if len(req.Header.Get("X-Ssh-Certificate")) != 0 { if len(req.Header.Get("X-Ssh-Certificate")) != 0 {
// Handle Signature signed by SSH certificates // Handle Signature signed by SSH certificates
if len(setting.SSH.TrustedUserCAKeys) == 0 { if len(setting.SSH.TrustedUserCAKeys) == 0 {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
publicKey, err = VerifyCert(req) publicKey, err = VerifyCert(req)
if err != nil { if err != nil {
log.Debug("VerifyCert on request from %s: failed: %v", req.RemoteAddr, err) log.Debug("VerifyCert on request from %s: failed: %v", req.RemoteAddr, err)
log.Warn("Failed authentication attempt from %s", req.RemoteAddr) log.Warn("Failed authentication attempt from %s", req.RemoteAddr)
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{} // 401 is not expected on signature validation miss; return not attempted
} }
} else { } else {
// Handle Signature signed by Public Key // Handle Signature signed by Public Key
@ -64,18 +64,17 @@ func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, _ auth.Sessi
if err != nil { if err != nil {
log.Debug("VerifyPubKey on request from %s: failed: %v", req.RemoteAddr, err) log.Debug("VerifyPubKey on request from %s: failed: %v", req.RemoteAddr, err)
log.Warn("Failed authentication attempt from %s", req.RemoteAddr) log.Warn("Failed authentication attempt from %s", req.RemoteAddr)
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{} // 401 is not expected on signature validation miss; return not attempted
} }
} }
u, err := user_model.GetUserByID(req.Context(), publicKey.OwnerID) u, err := user_model.GetUserByID(req.Context(), publicKey.OwnerID)
if err != nil { if err != nil {
log.Error("GetUserByID: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("httpsign GetUserByID: %w", err)}
return nil, err
} }
log.Trace("HTTP Sign: Logged in user %-v", u) log.Trace("HTTP Sign: Logged in user %-v", u)
return &httpSignAuthenticationResult{user: u}, nil return &auth.AuthenticationSuccess{Result: &httpSignAuthenticationResult{user: u}}
} }
func VerifyPubKey(r *http.Request) (*asymkey_model.PublicKey, error) { func VerifyPubKey(r *http.Request) (*asymkey_model.PublicKey, error) {

View file

@ -6,7 +6,7 @@ package method
import ( import (
"context" "context"
"errors" "fmt"
"net/http" "net/http"
"slices" "slices"
"strings" "strings"
@ -146,16 +146,16 @@ func parseToken(req *http.Request) (string, bool) {
// userIDFromToken returns the user id corresponding to the OAuth token. // userIDFromToken returns the user id corresponding to the OAuth token.
// It will set 'IsApiToken' to true if the token is an API token and // It will set 'IsApiToken' to true if the token is an API token and
// set 'ApiTokenScope' to the scope of the access token // set 'ApiTokenScope' to the scope of the access token
func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string) (auth.AuthenticationResult, error) { func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string) auth.MethodOutput {
if tokenSHA == "" { if tokenSHA == "" {
return nil, auth_model.ErrAccessTokenEmpty{} return &auth.AuthenticationAttemptedIncorrectCredential{Error: auth_model.ErrAccessTokenEmpty{}}
} }
// Let's see if token is valid. // Let's see if token is valid.
if strings.Contains(tokenSHA, ".") { if strings.Contains(tokenSHA, ".") {
// First attempt to decode an actions JWT, returning the actions user // First attempt to decode an actions JWT, returning the actions user
if taskID, err := actions.TokenToTaskID(tokenSHA); err == nil { if taskID, err := actions.TokenToTaskID(tokenSHA); err == nil {
if CheckTaskIsRunning(ctx, taskID) { if CheckTaskIsRunning(ctx, taskID) {
return &actionsTaskTokenAuthenticationResult{user: user_model.NewActionsUser(), taskID: taskID}, nil return &auth.AuthenticationSuccess{Result: &actionsTaskTokenAuthenticationResult{user: user_model.NewActionsUser(), taskID: taskID}}
} }
} }
@ -171,12 +171,12 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string) (auth.Aut
} }
user, err := user_model.GetPossibleUserByID(ctx, uid) user, err := user_model.GetPossibleUserByID(ctx, uid)
if err != nil { if err != nil {
if !user_model.IsErrUserNotExist(err) { if user_model.IsErrUserNotExist(err) {
log.Error("GetUserByName: %v", err) return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
return nil, err return &auth.AuthenticationError{Error: fmt.Errorf("oauth2 GetPossibleUserByID: %w", err)}
} }
return &oAuth2JWTAuthenticationResult{user: user, scope: accessTokenScope}, nil return &auth.AuthenticationSuccess{Result: &oAuth2JWTAuthenticationResult{user: user, scope: accessTokenScope}}
} }
t, err := auth_model.GetAccessTokenBySHA(ctx, tokenSHA) t, err := auth_model.GetAccessTokenBySHA(ctx, tokenSHA)
@ -186,63 +186,50 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string) (auth.Aut
task, err := actions_model.GetRunningTaskByToken(ctx, tokenSHA) task, err := actions_model.GetRunningTaskByToken(ctx, tokenSHA)
if err == nil && task != nil { if err == nil && task != nil {
log.Trace("Basic Authorization: Valid AccessToken for task[%d]", task.ID) log.Trace("Basic Authorization: Valid AccessToken for task[%d]", task.ID)
return &actionsTaskTokenAuthenticationResult{user: user_model.NewActionsUser(), taskID: task.ID}, nil return &auth.AuthenticationSuccess{Result: &actionsTaskTokenAuthenticationResult{user: user_model.NewActionsUser(), taskID: task.ID}}
} }
} else if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) { } else if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) {
log.Error("GetAccessTokenBySHA: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("oauth2 GetAccessTokenBySHA: %w", err)}
return nil, err
} }
return nil, err return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
if err := t.UpdateLastUsed(ctx); err != nil { if err := t.UpdateLastUsed(ctx); err != nil {
log.Error("UpdateLastUsed: %v", err) log.Error("UpdateLastUsed: %v", err)
} }
if t.UID == 0 { if t.UID == 0 {
return nil, auth_model.ErrAccessTokenNotExist{} return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
reducer, err := authz.GetAuthorizationReducerForAccessToken(ctx, t) reducer, err := authz.GetAuthorizationReducerForAccessToken(ctx, t)
if err != nil { if err != nil {
log.Error("authz.GetAuthorizationReducerForAccessToken: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("oauth2 GetAuthorizationReducerForAccessToken: %w", err)}
return nil, err
} }
u, err := user_model.GetUserByID(ctx, t.UID) u, err := user_model.GetUserByID(ctx, t.UID)
if err != nil { if err != nil {
log.Error("GetUserByID: %v", err) return &auth.AuthenticationError{Error: fmt.Errorf("oauth2 GetUserByID: %w", err)}
return nil, err
} }
return &accessTokenAuthenticationResult{user: u, scope: t.Scope, reducer: reducer}, nil return &auth.AuthenticationSuccess{Result: &accessTokenAuthenticationResult{user: u, scope: t.Scope, reducer: reducer}}
} }
// Verify extracts the user ID from the OAuth token in the query parameters // Verify extracts the user ID from the OAuth token in the query parameters
// or the "Authorization" header and returns the corresponding user object for that ID. // or the "Authorization" header and returns the corresponding user object for that ID.
// If verification is successful returns an existing user object. // If verification is successful returns an existing user object.
// Returns nil if verification fails. // Returns nil if verification fails.
func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) (auth.AuthenticationResult, error) { func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) auth.MethodOutput {
// These paths are not API paths, but we still want to check for tokens because they maybe in the API returned URLs // These paths are not API paths, but we still want to check for tokens because they maybe in the API returned URLs
if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) && if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) &&
!isGitRawOrAttachPath(req) && !isArchivePath(req) { !isGitRawOrAttachPath(req) && !isArchivePath(req) {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
token, ok := parseToken(req) token, ok := parseToken(req)
if !ok { if !ok {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
auth, err := o.userIDFromToken(req.Context(), token) return o.userIDFromToken(req.Context(), token)
if err != nil {
return nil, err
} else if auth.User() == nil {
// Having successfully found a token, it's now expected that the only valid outcomes are either errors that
// result in 401s (if the token wasn't valid), or valid authentication as a user. If we reach here, we've missed
// those expected outcomes and somehow returned an unauthenticated response even though a token was provided.
return nil, errors.New("unexpected unauthenticated result from userIDFromToken")
}
log.Trace("OAuth2 Authorization: Logged in user %-v", auth.User())
return auth, nil
} }
func isAuthenticatedTokenRequest(req *http.Request) bool { func isAuthenticatedTokenRequest(req *http.Request) bool {

View file

@ -12,6 +12,7 @@ import (
"forgejo.org/models/unittest" "forgejo.org/models/unittest"
user_model "forgejo.org/models/user" user_model "forgejo.org/models/user"
"forgejo.org/services/actions" "forgejo.org/services/actions"
auth_service "forgejo.org/services/auth"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -33,8 +34,10 @@ func TestUserIDFromToken(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
o := OAuth2{} o := OAuth2{}
authResult, err := o.userIDFromToken(t.Context(), token) output := o.userIDFromToken(t.Context(), token)
require.NoError(t, err) ar, authSuccess := output.(*auth_service.AuthenticationSuccess)
require.True(t, authSuccess, "expected type AuthenticationSuccess, but was: %#v", output)
authResult := ar.Result
assert.Equal(t, int64(user_model.ActionsUserID), authResult.User().ID) assert.Equal(t, int64(user_model.ActionsUserID), authResult.User().ID)
isActionsToken, authTaskID := authResult.ActionsTaskID().Get() isActionsToken, authTaskID := authResult.ActionsTaskID().Get()
assert.True(t, isActionsToken) assert.True(t, isActionsToken)
@ -53,9 +56,13 @@ func TestUserIDFromToken(t *testing.T) {
o := OAuth2{} o := OAuth2{}
for name, c := range cases { for name, c := range cases {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
authResult, err := o.userIDFromToken(t.Context(), c.Token) output := o.userIDFromToken(t.Context(), c.Token)
ar, authFailure := output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.True(t, authFailure, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
err := ar.Error
require.ErrorIs(t, err, c.Error) require.ErrorIs(t, err, c.Error)
assert.Nil(t, authResult)
}) })
} }
}) })

View file

@ -6,6 +6,7 @@ package method
import ( import (
"errors" "errors"
"fmt"
"net/http" "net/http"
"strings" "strings"
@ -77,13 +78,13 @@ func (r *ReverseProxy) getEmail(req *http.Request) string {
// If an email is available in the "setting.ReverseProxyAuthEmail" header an existing // If an email is available in the "setting.ReverseProxyAuthEmail" header an existing
// user object is returned (populated with the email found in header). // user object is returned (populated with the email found in header).
// Returns nil if header is empty or if "setting.EnableReverseProxyEmail" is disabled. // Returns nil if header is empty or if "setting.EnableReverseProxyEmail" is disabled.
func (r *ReverseProxy) getUserFromAuthEmail(req *http.Request) *user_model.User { func (r *ReverseProxy) getUserFromAuthEmail(req *http.Request) (*user_model.User, error) {
if !setting.Service.EnableReverseProxyEmail { if !setting.Service.EnableReverseProxyEmail {
return nil return nil, util.ErrNotExist
} }
email := r.getEmail(req) email := r.getEmail(req)
if len(email) == 0 { if len(email) == 0 {
return nil return nil, util.ErrNotExist
} }
log.Trace("ReverseProxy Authorization: Found email: %s", email) log.Trace("ReverseProxy Authorization: Found email: %s", email)
@ -93,24 +94,29 @@ func (r *ReverseProxy) getUserFromAuthEmail(req *http.Request) *user_model.User
if !user_model.IsErrUserNotExist(err) { if !user_model.IsErrUserNotExist(err) {
log.Error("GetUserByEmail: %v", err) log.Error("GetUserByEmail: %v", err)
} }
return nil return nil, err
} }
return user return user, nil
} }
// Verify attempts to load a user object based on headers sent by the reverse proxy. // Verify attempts to load a user object based on headers sent by the reverse proxy.
// First it will attempt to load it based on the username (see docs for getUserFromAuthUser), // First it will attempt to load it based on the username (see docs for getUserFromAuthUser),
// and failing that it will attempt to load it based on the email (see docs for getUserFromAuthEmail). // and failing that it will attempt to load it based on the email (see docs for getUserFromAuthEmail).
// Returns nil if the headers are empty or the user is not found. // Returns nil if the headers are empty or the user is not found.
func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) { func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) auth.MethodOutput {
user, err := r.getUserFromAuthUser(req) user, err := r.getUserFromAuthUser(req)
if err != nil && !errors.Is(err, util.ErrNotExist) { if err != nil && !errors.Is(err, util.ErrNotExist) {
return nil, err return &auth.AuthenticationError{Error: fmt.Errorf("reverse proxy getUserFromAuthUser: %w", err)}
} }
if user == nil { if user == nil {
user = r.getUserFromAuthEmail(req) user, err = r.getUserFromAuthEmail(req)
if user == nil { if user == nil {
return &auth.UnauthenticatedResult{}, nil if errors.Is(err, util.ErrNotExist) {
// Not attempted is returned when no HTTP headers were provided, which is the cases that ErrNotExist
// represents:
return &auth.AuthenticationNotAttempted{}
}
return &auth.AuthenticationAttemptedIncorrectCredential{Error: errors.New("user not found")}
} }
} }
@ -122,7 +128,7 @@ func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, sess aut
} }
log.Trace("ReverseProxy Authorization: Logged in user %-v", user) log.Trace("ReverseProxy Authorization: Logged in user %-v", user)
return &reverseProxyAuthenticationResult{user: user}, nil return &auth.AuthenticationSuccess{Result: &reverseProxyAuthenticationResult{user: user}}
} }
// isAutoRegisterAllowed checks if EnableReverseProxyAutoRegister setting is true // isAutoRegisterAllowed checks if EnableReverseProxyAutoRegister setting is true

View file

@ -4,6 +4,7 @@
package method package method
import ( import (
"fmt"
"net/http" "net/http"
user_model "forgejo.org/models/user" user_model "forgejo.org/models/user"
@ -23,34 +24,33 @@ type Session struct{}
// Verify checks if there is a user uid stored in the session and returns the user // Verify checks if there is a user uid stored in the session and returns the user
// object for that uid. // object for that uid.
// Returns nil if there is no user uid stored in the session. // Returns nil if there is no user uid stored in the session.
func (s *Session) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) { func (s *Session) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) auth.MethodOutput {
if sess == nil { if sess == nil {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
// Get user ID // Get user ID
uid := sess.Get("uid") uid := sess.Get("uid")
if uid == nil { if uid == nil {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
log.Trace("Session Authorization: Found user[%d]", uid) log.Trace("Session Authorization: Found user[%d]", uid)
id, ok := uid.(int64) id, ok := uid.(int64)
if !ok { if !ok {
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationNotAttempted{}
} }
// Get user object // Get user object
user, err := user_model.GetUserByID(req.Context(), id) user, err := user_model.GetUserByID(req.Context(), id)
if err != nil { if err != nil {
if !user_model.IsErrUserNotExist(err) { if !user_model.IsErrUserNotExist(err) {
log.Error("GetUserByID: %v", err)
// Return the err as-is to keep current signed-in session, in case the err is something like context.Canceled. Otherwise non-existing user (nil, nil) will make the caller clear the signed-in session. // Return the err as-is to keep current signed-in session, in case the err is something like context.Canceled. Otherwise non-existing user (nil, nil) will make the caller clear the signed-in session.
return nil, err return &auth.AuthenticationError{Error: fmt.Errorf("session auth GetUserByID: %w", err)}
} }
return &auth.UnauthenticatedResult{}, nil return &auth.AuthenticationAttemptedIncorrectCredential{Error: err}
} }
log.Trace("Session Authorization: Logged in user %-v", user) log.Trace("Session Authorization: Logged in user %-v", user)
return &sessionAuthenticationResult{user: user}, nil return &auth.AuthenticationSuccess{Result: &sessionAuthenticationResult{user: user}}
} }

View file

@ -32,6 +32,7 @@ import (
chef_module "forgejo.org/modules/packages/chef" chef_module "forgejo.org/modules/packages/chef"
"forgejo.org/modules/setting" "forgejo.org/modules/setting"
chef_router "forgejo.org/routers/api/packages/chef" chef_router "forgejo.org/routers/api/packages/chef"
auth_service "forgejo.org/services/auth"
"forgejo.org/tests" "forgejo.org/tests"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@ -94,10 +95,9 @@ nwIDAQAB
defer tests.PrintCurrentTest(t)() defer tests.PrintCurrentTest(t)()
req := NewRequest(t, "POST", "/dummy") req := NewRequest(t, "POST", "/dummy")
u, err := auth.Verify(req.Request, nil, nil) output := auth.Verify(req.Request, nil, nil)
require.NoError(t, err) _, authNotAttempted := output.(*auth_service.AuthenticationNotAttempted)
require.NotNil(t, u) assert.True(t, authNotAttempted, "expected type AuthenticationNotAttempted, but was: %#v", output)
assert.Nil(t, u.User())
}) })
t.Run("NotExistingUser", func(t *testing.T) { t.Run("NotExistingUser", func(t *testing.T) {
@ -105,9 +105,11 @@ nwIDAQAB
req := NewRequest(t, "POST", "/dummy"). req := NewRequest(t, "POST", "/dummy").
SetHeader("X-Ops-Userid", "not-existing-user") SetHeader("X-Ops-Userid", "not-existing-user")
u, err := auth.Verify(req.Request, nil, nil)
assert.Nil(t, u) output := auth.Verify(req.Request, nil, nil)
require.Error(t, err) ar, authError := output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.True(t, authError, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
require.ErrorContains(t, ar.Error, "user does not exist")
}) })
t.Run("Timestamp", func(t *testing.T) { t.Run("Timestamp", func(t *testing.T) {
@ -115,14 +117,16 @@ nwIDAQAB
req := NewRequest(t, "POST", "/dummy"). req := NewRequest(t, "POST", "/dummy").
SetHeader("X-Ops-Userid", user.Name) SetHeader("X-Ops-Userid", user.Name)
u, err := auth.Verify(req.Request, nil, nil) output := auth.Verify(req.Request, nil, nil)
assert.Nil(t, u) ar, authError := output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.Error(t, err) require.True(t, authError, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
require.ErrorContains(t, ar.Error, "X-Ops-Timestamp header missing")
req.SetHeader("X-Ops-Timestamp", "2023-01-01T00:00:00Z") req.SetHeader("X-Ops-Timestamp", "2023-01-01T00:00:00Z")
u, err = auth.Verify(req.Request, nil, nil) output = auth.Verify(req.Request, nil, nil)
assert.Nil(t, u) ar, authError = output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.Error(t, err) require.True(t, authError, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
require.ErrorContains(t, ar.Error, "time difference")
}) })
t.Run("SigningVersion", func(t *testing.T) { t.Run("SigningVersion", func(t *testing.T) {
@ -131,29 +135,35 @@ nwIDAQAB
req := NewRequest(t, "POST", "/dummy"). req := NewRequest(t, "POST", "/dummy").
SetHeader("X-Ops-Userid", user.Name). SetHeader("X-Ops-Userid", user.Name).
SetHeader("X-Ops-Timestamp", time.Now().UTC().Format(time.RFC3339)) SetHeader("X-Ops-Timestamp", time.Now().UTC().Format(time.RFC3339))
u, err := auth.Verify(req.Request, nil, nil)
assert.Nil(t, u) output := auth.Verify(req.Request, nil, nil)
require.Error(t, err) ar, authError := output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.True(t, authError, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
require.ErrorContains(t, ar.Error, "X-Ops-Sign header missing")
req.SetHeader("X-Ops-Sign", "version=none") req.SetHeader("X-Ops-Sign", "version=none")
u, err = auth.Verify(req.Request, nil, nil) output = auth.Verify(req.Request, nil, nil)
assert.Nil(t, u) ar, authError = output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.Error(t, err) require.True(t, authError, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
require.ErrorContains(t, ar.Error, "invalid X-Ops-Sign header")
req.SetHeader("X-Ops-Sign", "version=1.4") req.SetHeader("X-Ops-Sign", "version=1.4")
u, err = auth.Verify(req.Request, nil, nil) output = auth.Verify(req.Request, nil, nil)
assert.Nil(t, u) ar, authError = output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.Error(t, err) require.True(t, authError, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
require.ErrorContains(t, ar.Error, "unsupported version")
req.SetHeader("X-Ops-Sign", "version=1.0;algorithm=sha2") req.SetHeader("X-Ops-Sign", "version=1.0;algorithm=sha2")
u, err = auth.Verify(req.Request, nil, nil) output = auth.Verify(req.Request, nil, nil)
assert.Nil(t, u) ar, authError = output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.Error(t, err) require.True(t, authError, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
require.ErrorContains(t, ar.Error, "unsupported algorithm")
req.SetHeader("X-Ops-Sign", "version=1.0;algorithm=sha256") req.SetHeader("X-Ops-Sign", "version=1.0;algorithm=sha256")
u, err = auth.Verify(req.Request, nil, nil) output = auth.Verify(req.Request, nil, nil)
assert.Nil(t, u) ar, authError = output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.Error(t, err) require.True(t, authError, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
require.ErrorContains(t, ar.Error, "unsupported algorithm")
}) })
t.Run("SignedHeaders", func(t *testing.T) { t.Run("SignedHeaders", func(t *testing.T) {
@ -167,9 +177,10 @@ nwIDAQAB
SetHeader("X-Ops-Sign", "version=1.0;algorithm=sha1"). SetHeader("X-Ops-Sign", "version=1.0;algorithm=sha1").
SetHeader("X-Ops-Content-Hash", "unused"). SetHeader("X-Ops-Content-Hash", "unused").
SetHeader("X-Ops-Authorization-4", "dummy") SetHeader("X-Ops-Authorization-4", "dummy")
u, err := auth.Verify(req.Request, nil, nil) output := auth.Verify(req.Request, nil, nil)
assert.Nil(t, u) ar, authError := output.(*auth_service.AuthenticationAttemptedIncorrectCredential)
require.Error(t, err) require.True(t, authError, "expected type AuthenticationAttemptedIncorrectCredential, but was: %#v", output)
require.ErrorContains(t, ar.Error, "invalid X-Ops-Authorization headers")
signRequest := func(rw *RequestWrapper, version string) { signRequest := func(rw *RequestWrapper, version string) {
req := rw.Request req := rw.Request
@ -258,9 +269,12 @@ nwIDAQAB
defer tests.PrintCurrentTest(t)() defer tests.PrintCurrentTest(t)()
signRequest(req, v) signRequest(req, v)
u, err = auth.Verify(req.Request, nil, nil) output := auth.Verify(req.Request, nil, nil)
assert.NotNil(t, u) ar, authSuccess := output.(*auth_service.AuthenticationSuccess)
require.NoError(t, err) require.True(t, authSuccess, "expected type AuthenticationSuccess, but was: %#v", output)
assert.NotNil(t, ar)
assert.NotNil(t, ar.Result)
assert.EqualValues(t, 2, ar.Result.User().ID)
}) })
} }
}) })