diff options
author | Lunny Xiao <xiaolunwen@gmail.com> | 2022-12-28 13:53:28 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-28 13:53:28 +0800 |
commit | ca67c5a8a72f3d26bb0808ba7a63cd4875cd5229 (patch) | |
tree | 8190bc13c43dce1ad3aa78d29920b8e08c0931a8 /routers/api | |
parent | 7cc7db73b922143a1288d26b39eee9e8e59f7106 (diff) | |
download | gitea-ca67c5a8a72f3d26bb0808ba7a63cd4875cd5229.tar.gz gitea-ca67c5a8a72f3d26bb0808ba7a63cd4875cd5229.zip |
refactor auth interface to return error when verify failure (#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>
Diffstat (limited to 'routers/api')
-rw-r--r-- | routers/api/packages/api.go | 17 | ||||
-rw-r--r-- | routers/api/packages/conan/auth.go | 10 | ||||
-rw-r--r-- | routers/api/packages/container/auth.go | 12 | ||||
-rw-r--r-- | routers/api/packages/nuget/auth.go | 9 |
4 files changed, 31 insertions, 17 deletions
diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index b523725c48..78eb5e860b 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/packages/composer" @@ -58,7 +59,13 @@ func CommonRoutes(ctx gocontext.Context) *web.Route { authGroup := auth.NewGroup(authMethods...) r.Use(func(ctx *context.Context) { - ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session) + var err error + ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session) + if err != nil { + log.Error("Verify: %v", err) + ctx.Error(http.StatusUnauthorized, "authGroup.Verify") + return + } ctx.IsSigned = ctx.Doer != nil }) @@ -321,7 +328,13 @@ func ContainerRoutes(ctx gocontext.Context) *web.Route { authGroup := auth.NewGroup(authMethods...) r.Use(func(ctx *context.Context) { - ctx.Doer = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session) + var err error + ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session) + if err != nil { + log.Error("Failed to verify user: %v", err) + ctx.Error(http.StatusUnauthorized, "Verify") + return + } ctx.IsSigned = ctx.Doer != nil }) diff --git a/routers/api/packages/conan/auth.go b/routers/api/packages/conan/auth.go index f3adaf7bee..ca02d61e76 100644 --- a/routers/api/packages/conan/auth.go +++ b/routers/api/packages/conan/auth.go @@ -19,22 +19,22 @@ func (a *Auth) Name() string { } // Verify extracts the user from the Bearer token -func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) *user_model.User { +func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) { uid, err := packages.ParseAuthorizationToken(req) if err != nil { log.Trace("ParseAuthorizationToken: %v", err) - return nil + return nil, err } if uid == 0 { - return nil + return nil, nil } u, err := user_model.GetUserByID(req.Context(), uid) if err != nil { log.Error("GetUserByID: %v", err) - return nil + return nil, err } - return u + return u, nil } diff --git a/routers/api/packages/container/auth.go b/routers/api/packages/container/auth.go index e134f74c8f..33f439ec3e 100644 --- a/routers/api/packages/container/auth.go +++ b/routers/api/packages/container/auth.go @@ -20,25 +20,25 @@ func (a *Auth) Name() string { // Verify extracts the user from the Bearer token // If it's an anonymous session a ghost user is returned -func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) *user_model.User { +func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) { uid, err := packages.ParseAuthorizationToken(req) if err != nil { log.Trace("ParseAuthorizationToken: %v", err) - return nil + return nil, err } if uid == 0 { - return nil + return nil, nil } if uid == -1 { - return user_model.NewGhostUser() + return user_model.NewGhostUser(), nil } u, err := user_model.GetUserByID(req.Context(), uid) if err != nil { log.Error("GetUserByID: %v", err) - return nil + return nil, err } - return u + return u, nil } diff --git a/routers/api/packages/nuget/auth.go b/routers/api/packages/nuget/auth.go index 890c930184..54b33d89c0 100644 --- a/routers/api/packages/nuget/auth.go +++ b/routers/api/packages/nuget/auth.go @@ -20,19 +20,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(req.Context(), token.UID) if err != nil { log.Error("GetUserByID: %v", err) - return nil + return nil, err } token.UpdatedUnix = timeutil.TimeStampNow() @@ -40,5 +41,5 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS log.Error("UpdateAccessToken: %v", err) } - return u + return u, nil } |