Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auth interface return error when verify failure (#22119) #22259

Merged
merged 2 commits into from
Dec 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion modules/context/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 7 additions & 1 deletion modules/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 15 additions & 2 deletions routers/api/packages/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
})

Expand Down Expand Up @@ -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
})

Expand Down
10 changes: 5 additions & 5 deletions routers/api/packages/conan/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 6 additions & 6 deletions routers/api/packages/container/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 5 additions & 4 deletions routers/api/packages/nuget/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,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(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
}
22 changes: 11 additions & 11 deletions services/auth/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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() {
Expand All @@ -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 changes: 8 additions & 9 deletions services/auth/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"reflect"
"strings"

"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user"
)

Expand Down Expand Up @@ -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 changes: 7 additions & 7 deletions services/auth/httpsign.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -54,36 +54,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(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) {
Expand Down
5 changes: 3 additions & 2 deletions services/auth/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 5 additions & 9 deletions services/auth/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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 {
Expand Down