mirrors
/
forgejo
mirror of
1
Fork 0
Code Issues Releases Activity

refactor auth interface to return error when verify failure (#22119) (#22259) 

backport #22119

This PR changed the Auth interface signature from `Verify(http
*http.Request, w http.ResponseWriter, store DataStore, sess
SessionStore) *user_model.User`
to 
`Verify(http *http.Request, w http.ResponseWriter, store DataStore, sess
SessionStore) (*user_model.User, error)`.

There is a new return argument `error` which means the verification
condition matched but verify process failed, we should stop the auth
process.

Before this PR, when return a `nil` user, we don't know the reason why
it returned `nil`. If the match condition is not satisfied or it
verified failure? For these two different results, we should have
different handler. If the match condition is not satisfied, we should
try next auth method and if there is no more auth method, it's an
anonymous user. If the condition matched but verify failed, the auth
process should be stop and return immediately.

This will fix #20563

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: Jason Song <i@wolfogre.com>
This commit is contained in:
Lunny Xiao 2022-12-29 13:50:09 +08:00 committed by GitHub
parent e9bc2c77c3
commit 900e158064
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 111 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
modules/context/api.go
View File

@ -220,7 +220,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)

8
modules/context/context.go
View File

@ -663,7 +663,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)

17
routers/api/packages/api.go
View File

@ -12,6 +12,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"
@ -57,7 +58,13 @@ func Routes(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
})
@ -317,7 +324,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
})

10
routers/api/packages/conan/auth.go
View File

@ -20,22 +20,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(uid)
if err != nil {
log.Error("GetUserByID: %v", err)
return nil
return nil, err
}
return u
return u, nil
}

12
routers/api/packages/container/auth.go
View File

@ -21,25 +21,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(uid)
if err != nil {
log.Error("GetUserByID: %v", err)
return nil
return nil, err
}
return u
return u, nil
}

9
routers/api/packages/nuget/auth.go
View File

@ -21,19 +21,20 @@ 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(token.UID)
if err != nil {
log.Error("GetUserByID: %v", err)
return nil
return nil, err
}
token.UpdatedUnix = timeutil.TimeStampNow()
@ -41,5 +42,5 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
log.Error("UpdateAccessToken: %v", err)
}
return u
return u, nil
}

22
services/auth/basic.go
View File

@ -41,20 +41,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])
@ -78,11 +78,11 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
u, err := user_model.GetUserByID(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)
@ -91,7 +91,7 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
u, err := user_model.GetUserByID(token.UID)
if err != nil {
log.Error("GetUserByID: %v", err)
return nil
return nil, err
}
token.UpdatedUnix = timeutil.TimeStampNow()
@ -100,13 +100,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)
@ -115,7 +115,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() {
@ -124,5 +124,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
}

17
services/auth/group.go
View File

@ -10,7 +10,6 @@ import (
"reflect"
"strings"
"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user"
)
@ -81,23 +80,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 nil, 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
}

14
services/auth/httpsign.go
View File

@ -40,10 +40,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 (
@ -54,14 +54,14 @@ 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
@ -69,21 +69,21 @@ func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataSt
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(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) {

5
services/auth/interface.go
View File

@ -25,8 +25,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

14
services/auth/oauth2.go
View File

@ -109,18 +109,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)
@ -129,11 +125,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 {

19
services/auth/reverseproxy.go
View File

@ -52,10 +52,10 @@ func (r *ReverseProxy) Name() string {
// If a username is available in the "setting.ReverseProxyAuthUser" header an existing
// user object is returned (populated with username or email found in header).
// Returns nil if header is empty.
func (r *ReverseProxy) getUserFromAuthUser(req *http.Request) *user_model.User {
func (r *ReverseProxy) getUserFromAuthUser(req *http.Request) (*user_model.User, error) {
username := r.getUserName(req)
if len(username) == 0 {
return nil
return nil, nil
}
log.Trace("ReverseProxy Authorization: Found username: %s", username)
@ -63,11 +63,11 @@ func (r *ReverseProxy) getUserFromAuthUser(req *http.Request) *user_model.User {
if err != nil {
if !user_model.IsErrUserNotExist(err) || !r.isAutoRegisterAllowed() {
log.Error("GetUserByName: %v", err)
return nil
return nil, err
}
user = r.newUser(req)
}
return user
return user, nil
}
// getEmail extracts the email from the "setting.ReverseProxyAuthEmail" header
@ -107,12 +107,15 @@ func (r *ReverseProxy) getUserFromAuthEmail(req *http.Request) *user_model.User
// 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).
// Returns nil if the headers are empty or the user is not found.
func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
user := r.getUserFromAuthUser(req)
func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
user, err := r.getUserFromAuthUser(req)
if err != nil {
return nil, err
}
if user == nil {
user = r.getUserFromAuthEmail(req)
if user == nil {
return nil
return nil, nil
}
}
@ -125,7 +128,7 @@ func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, store Da
store.GetData()["IsReverseProxy"] = true
log.Trace("ReverseProxy Authorization: Logged in user %-v", user)
return user
return user, nil
}
// isAutoRegisterAllowed checks if EnableReverseProxyAutoRegister setting is true

6
services/auth/session.go
View File

@ -29,12 +29,12 @@ func (s *Session) Name() string {
// Verify checks if there is a user uid stored in the session and returns the user
// object for that uid.
// Returns nil if there is no user uid stored in the session.
func (s *Session) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
func (s *Session) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
user := SessionUser(sess)
if user != nil {
return user
return user, nil
}
return nil
return nil, nil
}
// SessionUser returns the user object corresponding to the "uid" session variable.

18
services/auth/sspi_windows.go
View File

@ -78,15 +78,15 @@ func (s *SSPI) Free() error {
// If authentication is successful, returns the corresponding user object.
// If negotiation should continue or authentication fails, immediately returns a 401 HTTP
// response code, as required by the SPNEGO protocol.
func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
if !s.shouldAuthenticate(req) {
return nil
return nil, nil
}
cfg, err := s.getConfig()
if err != nil {
log.Error("could not get SSPI config: %v", err)
return nil
return nil, err
}
log.Trace("SSPI Authorization: Attempting to authenticate")
@ -109,7 +109,7 @@ func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore,
log.Error("%v", err)
}
return nil
return nil, err
}
if outToken != "" {
sspiAuth.AppendAuthenticateHeader(w, outToken)
@ -117,7 +117,7 @@ func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore,
username := sanitizeUsername(userInfo.Username, cfg)
if len(username) == 0 {
return nil
return nil, nil
}
log.Info("Authenticated as %s\n", username)
@ -125,16 +125,16 @@ func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore,
if err != nil {
if !user_model.IsErrUserNotExist(err) {
log.Error("GetUserByName: %v", err)
return nil
return nil, err
}
if !cfg.AutoCreateUsers {
log.Error("User '%s' not found", username)
return nil
return nil, nil
}
user, err = s.newUser(username, cfg)
if err != nil {
log.Error("CreateUser: %v", err)
return nil
return nil, err
}
}
@ -144,7 +144,7 @@ func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore,
}
log.Trace("SSPI Authorization: Logged in user %-v", user)
return user
return user, nil
}
// getConfig retrieves the SSPI configuration from login sources

11
services/packages/auth.go
View File

@ -11,6 +11,7 @@ import (
"time"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"github.com/golang-jwt/jwt/v4"
@ -42,9 +43,15 @@ func CreateAuthorizationToken(u *user_model.User) (string, error) {
}
func ParseAuthorizationToken(req *http.Request) (int64, error) {
parts := strings.SplitN(req.Header.Get("Authorization"), " ", 2)
h := req.Header.Get("Authorization")
if h == "" {
return 0, nil
}
parts := strings.SplitN(h, " ", 2)
if len(parts) != 2 {
return 0, fmt.Errorf("no token")
log.Error("split token failed: %s", h)
return 0, fmt.Errorf("split token failed")
}
token, err := jwt.ParseWithClaims(parts[1], &packageClaims{}, func(t *jwt.Token) (interface{}, error) {