Skip to content

Commit 7088b56

Browse files
zjjhotzjjhot
committed
Merge remote-tracking branch 'upstream/main'
* upstream/main: refactor auth interface to return error when verify failure (go-gitea#22119)
2 parents 5315392 + ca67c5a commit 7088b56

File tree

15 files changed

+111
-79
lines changed

15 files changed

+111
-79
lines changed

‎modules/context/api.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,13 @@ func (ctx *APIContext) CheckForOTP() {
219219
func APIAuth(authMethod auth_service.Method) func(*APIContext) {
220220
return func(ctx *APIContext) {
221221
// Get user from session if logged in.
222-
ctx.Doer = authMethod.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
222+
var err error
223+
ctx.Doer, err = authMethod.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
224+
if err != nil {
225+
ctx.Error(http.StatusUnauthorized, "APIAuth", err)
226+
return
227+
}
228+
223229
if ctx.Doer != nil {
224230
if ctx.Locale.Language() != ctx.Doer.Language {
225231
ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req)

‎modules/context/context.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,13 @@ func getCsrfOpts() CsrfOptions {
662662
// Auth converts auth.Auth as a middleware
663663
func Auth(authMethod auth.Method) func(*Context) {
664664
return func(ctx *Context) {
665-
ctx.Doer = authMethod.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
665+
var err error
666+
ctx.Doer, err = authMethod.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
667+
if err != nil {
668+
log.Error("Failed to verify user %v: %v", ctx.Req.RemoteAddr, err)
669+
ctx.Error(http.StatusUnauthorized, "Verify")
670+
return
671+
}
666672
if ctx.Doer != nil {
667673
if ctx.Locale.Language() != ctx.Doer.Language {
668674
ctx.Locale = middleware.Locale(ctx.Resp, ctx.Req)

‎routers/api/packages/api.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"code.gitea.io/gitea/models/perm"
1313
"code.gitea.io/gitea/modules/context"
14+
"code.gitea.io/gitea/modules/log"
1415
"code.gitea.io/gitea/modules/setting"
1516
"code.gitea.io/gitea/modules/web"
1617
"code.gitea.io/gitea/routers/api/packages/composer"
@@ -58,7 +59,13 @@ func CommonRoutes(ctx gocontext.Context) *web.Route {
5859

5960
authGroup := auth.NewGroup(authMethods...)
6061
r.Use(func(ctx *context.Context) {
61-
ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
62+
var err error
63+
ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
64+
if err != nil {
65+
log.Error("Verify: %v", err)
66+
ctx.Error(http.StatusUnauthorized, "authGroup.Verify")
67+
return
68+
}
6269
ctx.IsSigned = ctx.Doer != nil
6370
})
6471

@@ -321,7 +328,13 @@ func ContainerRoutes(ctx gocontext.Context) *web.Route {
321328

322329
authGroup := auth.NewGroup(authMethods...)
323330
r.Use(func(ctx *context.Context) {
324-
ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
331+
var err error
332+
ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
333+
if err != nil {
334+
log.Error("Failed to verify user: %v", err)
335+
ctx.Error(http.StatusUnauthorized, "Verify")
336+
return
337+
}
325338
ctx.IsSigned = ctx.Doer != nil
326339
})
327340

‎routers/api/packages/conan/auth.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,22 @@ func (a *Auth) Name() string {
1919
}
2020

2121
// Verify extracts the user from the Bearer token
22-
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) *user_model.User {
22+
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
2323
uid, err := packages.ParseAuthorizationToken(req)
2424
if err != nil {
2525
log.Trace("ParseAuthorizationToken: %v", err)
26-
return nil
26+
return nil, err
2727
}
2828

2929
if uid == 0 {
30-
return nil
30+
return nil, nil
3131
}
3232

3333
u, err := user_model.GetUserByID(req.Context(), uid)
3434
if err != nil {
3535
log.Error("GetUserByID: %v", err)
36-
return nil
36+
return nil, err
3737
}
3838

39-
return u
39+
return u, nil
4040
}

‎routers/api/packages/container/auth.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,25 +20,25 @@ func (a *Auth) Name() string {
2020

2121
// Verify extracts the user from the Bearer token
2222
// If it's an anonymous session a ghost user is returned
23-
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) *user_model.User {
23+
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
2424
uid, err := packages.ParseAuthorizationToken(req)
2525
if err != nil {
2626
log.Trace("ParseAuthorizationToken: %v", err)
27-
return nil
27+
return nil, err
2828
}
2929

3030
if uid == 0 {
31-
return nil
31+
return nil, nil
3232
}
3333
if uid == -1 {
34-
return user_model.NewGhostUser()
34+
return user_model.NewGhostUser(), nil
3535
}
3636

3737
u, err := user_model.GetUserByID(req.Context(), uid)
3838
if err != nil {
3939
log.Error("GetUserByID: %v", err)
40-
return nil
40+
return nil, err
4141
}
4242

43-
return u
43+
return u, nil
4444
}

‎routers/api/packages/nuget/auth.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,25 +20,26 @@ func (a *Auth) Name() string {
2020
}
2121

2222
// https://docs.microsoft.com/en-us/nuget/api/package-publish-resource#request-parameters
23-
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) *user_model.User {
23+
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
2424
token, err := auth_model.GetAccessTokenBySHA(req.Header.Get("X-NuGet-ApiKey"))
2525
if err != nil {
2626
if !(auth_model.IsErrAccessTokenNotExist(err) || auth_model.IsErrAccessTokenEmpty(err)) {
2727
log.Error("GetAccessTokenBySHA: %v", err)
28+
return nil, err
2829
}
29-
return nil
30+
return nil, nil
3031
}
3132

3233
u, err := user_model.GetUserByID(req.Context(), token.UID)
3334
if err != nil {
3435
log.Error("GetUserByID: %v", err)
35-
return nil
36+
return nil, err
3637
}
3738

3839
token.UpdatedUnix = timeutil.TimeStampNow()
3940
if err := auth_model.UpdateAccessToken(token); err != nil {
4041
log.Error("UpdateAccessToken: %v", err)
4142
}
4243

43-
return u
44+
return u, nil
4445
}

‎services/auth/basic.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,20 @@ func (b *Basic) Name() string {
4040
// "Authorization" header of the request and returns the corresponding user object for that
4141
// name/token on successful validation.
4242
// Returns nil if header is empty or validation fails.
43-
func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
43+
func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
4444
// Basic authentication should only fire on API, Download or on Git or LFSPaths
4545
if !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawReleaseOrLFSPath(req) {
46-
return nil
46+
return nil, nil
4747
}
4848

4949
baHead := req.Header.Get("Authorization")
5050
if len(baHead) == 0 {
51-
return nil
51+
return nil, nil
5252
}
5353

5454
auths := strings.SplitN(baHead, " ", 2)
5555
if len(auths) != 2 || (strings.ToLower(auths[0]) != "basic") {
56-
return nil
56+
return nil, nil
5757
}
5858

5959
uname, passwd, _ := base.BasicAuthDecode(auths[1])
@@ -77,11 +77,11 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
7777
u, err := user_model.GetUserByID(req.Context(), uid)
7878
if err != nil {
7979
log.Error("GetUserByID: %v", err)
80-
return nil
80+
return nil, err
8181
}
8282

8383
store.GetData()["IsApiToken"] = true
84-
return u
84+
return u, nil
8585
}
8686

8787
token, err := auth_model.GetAccessTokenBySHA(authToken)
@@ -90,7 +90,7 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
9090
u, err := user_model.GetUserByID(req.Context(), token.UID)
9191
if err != nil {
9292
log.Error("GetUserByID: %v", err)
93-
return nil
93+
return nil, err
9494
}
9595

9696
token.UpdatedUnix = timeutil.TimeStampNow()
@@ -99,13 +99,13 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
9999
}
100100

101101
store.GetData()["IsApiToken"] = true
102-
return u
102+
return u, nil
103103
} else if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) {
104104
log.Error("GetAccessTokenBySha: %v", err)
105105
}
106106

107107
if !setting.Service.EnableBasicAuth {
108-
return nil
108+
return nil, nil
109109
}
110110

111111
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
114114
if !user_model.IsErrUserNotExist(err) {
115115
log.Error("UserSignIn: %v", err)
116116
}
117-
return nil
117+
return nil, err
118118
}
119119

120120
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
123123

124124
log.Trace("Basic Authorization: Logged in user %-v", u)
125125

126-
return u
126+
return u, nil
127127
}

‎services/auth/group.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"reflect"
1010
"strings"
1111

12-
"code.gitea.io/gitea/models/db"
1312
user_model "code.gitea.io/gitea/models/user"
1413
)
1514

@@ -80,23 +79,23 @@ func (b *Group) Free() error {
8079
}
8180

8281
// Verify extracts and validates
83-
func (b *Group) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
84-
if !db.HasEngine {
85-
return nil
86-
}
87-
82+
func (b *Group) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
8883
// Try to sign in with each of the enabled plugins
8984
for _, ssoMethod := range b.methods {
90-
user := ssoMethod.Verify(req, w, store, sess)
85+
user, err := ssoMethod.Verify(req, w, store, sess)
86+
if err != nil {
87+
return nil, err
88+
}
89+
9190
if user != nil {
9291
if store.GetData()["AuthedMethod"] == nil {
9392
if named, ok := ssoMethod.(Named); ok {
9493
store.GetData()["AuthedMethod"] = named.Name()
9594
}
9695
}
97-
return user
96+
return user, nil
9897
}
9998
}
10099

101-
return nil
100+
return nil, nil
102101
}

‎services/auth/httpsign.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ func (h *HTTPSign) Name() string {
3939
// Verify extracts and validates HTTPsign from the Signature header of the request and returns
4040
// the corresponding user object on successful validation.
4141
// Returns nil if header is empty or validation fails.
42-
func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
42+
func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
4343
sigHead := req.Header.Get("Signature")
4444
if len(sigHead) == 0 {
45-
return nil
45+
return nil, nil
4646
}
4747

4848
var (
@@ -53,36 +53,36 @@ func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataSt
5353
if len(req.Header.Get("X-Ssh-Certificate")) != 0 {
5454
// Handle Signature signed by SSH certificates
5555
if len(setting.SSH.TrustedUserCAKeys) == 0 {
56-
return nil
56+
return nil, nil
5757
}
5858

5959
publicKey, err = VerifyCert(req)
6060
if err != nil {
6161
log.Debug("VerifyCert on request from %s: failed: %v", req.RemoteAddr, err)
6262
log.Warn("Failed authentication attempt from %s", req.RemoteAddr)
63-
return nil
63+
return nil, nil
6464
}
6565
} else {
6666
// Handle Signature signed by Public Key
6767
publicKey, err = VerifyPubKey(req)
6868
if err != nil {
6969
log.Debug("VerifyPubKey on request from %s: failed: %v", req.RemoteAddr, err)
7070
log.Warn("Failed authentication attempt from %s", req.RemoteAddr)
71-
return nil
71+
return nil, nil
7272
}
7373
}
7474

7575
u, err := user_model.GetUserByID(req.Context(), publicKey.OwnerID)
7676
if err != nil {
7777
log.Error("GetUserByID: %v", err)
78-
return nil
78+
return nil, err
7979
}
8080

8181
store.GetData()["IsApiToken"] = true
8282

8383
log.Trace("HTTP Sign: Logged in user %-v", u)
8484

85-
return u
85+
return u, nil
8686
}
8787

8888
func VerifyPubKey(r *http.Request) (*asymkey_model.PublicKey, error) {

‎services/auth/interface.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ type Method interface {
2424
// If verification is successful returns either an existing user object (with id > 0)
2525
// or a new user object (with id = 0) populated with the information that was found
2626
// in the authentication data (username or email).
27-
// Returns nil if verification fails.
28-
Verify(http *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User
27+
// Second argument returns err if verification fails, otherwise
28+
// First return argument returns nil if no matched verification condition
29+
Verify(http *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error)
2930
}
3031

3132
// Initializable represents a structure that requires initialization

‎services/auth/oauth2.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,18 +108,14 @@ func (o *OAuth2) userIDFromToken(req *http.Request, store DataStore) int64 {
108108
// or the "Authorization" header and returns the corresponding user object for that ID.
109109
// If verification is successful returns an existing user object.
110110
// Returns nil if verification fails.
111-
func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
112-
if !db.HasEngine {
113-
return nil
114-
}
115-
111+
func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
116112
if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) {
117-
return nil
113+
return nil, nil
118114
}
119115

120116
id := o.userIDFromToken(req, store)
121117
if id <= 0 {
122-
return nil
118+
return nil, nil
123119
}
124120
log.Trace("OAuth2 Authorization: Found token for user[%d]", id)
125121

@@ -128,11 +124,11 @@ func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStor
128124
if !user_model.IsErrUserNotExist(err) {
129125
log.Error("GetUserByName: %v", err)
130126
}
131-
return nil
127+
return nil, err
132128
}
133129

134130
log.Trace("OAuth2 Authorization: Logged in user %-v", user)
135-
return user
131+
return user, nil
136132
}
137133

138134
func isAuthenticatedTokenRequest(req *http.Request) bool {

0 commit comments

Comments
 (0)