From 6526733a58632086d51ce7211b3a4dc75dbbef90 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 28 Mar 2022 12:46:28 +0800 Subject: [PATCH] Let web and API routes have different auth methods group (#19168) * remove the global methods but create dynamiclly * Fix lint * Fix windows lint * Fix windows lint * some improvements Co-authored-by: wxiaoguang --- routers/api/v1/api.go | 27 ++++++++++++++- routers/api/v1/auth.go | 12 +++++++ routers/api/v1/auth_windows.go | 20 +++++++++++ routers/web/auth.go | 12 +++++++ routers/web/auth_windows.go | 20 +++++++++++ routers/web/web.go | 27 ++++++++++++++- services/auth/auth.go | 62 +++------------------------------- services/auth/group.go | 20 +++++++++++ services/auth/placeholder.go | 10 ------ services/auth/sspi_windows.go | 10 ------ 10 files changed, 140 insertions(+), 80 deletions(-) create mode 100644 routers/api/v1/auth.go create mode 100644 routers/api/v1/auth_windows.go create mode 100644 routers/web/auth.go create mode 100644 routers/web/auth_windows.go delete mode 100644 services/auth/placeholder.go diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 5ac6fba29..3debf58a1 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -563,6 +563,26 @@ func bind(obj interface{}) http.HandlerFunc { }) } +// The OAuth2 plugin is expected to be executed first, as it must ignore the user id stored +// in the session (if there is a user id stored in session other plugins might return the user +// object for that id). +// +// The Session plugin is expected to be executed second, in order to skip authentication +// for users that have already signed in. +func buildAuthGroup() *auth.Group { + group := auth.NewGroup( + &auth.OAuth2{}, + &auth.Basic{}, // FIXME: this should be removed once we don't allow basic auth in API + auth.SharedSession, // FIXME: this should be removed once all UI don't reference API/v1, see https://github.com/go-gitea/gitea/pull/16052 + ) + if setting.Service.EnableReverseProxyAuth { + group.Add(&auth.ReverseProxy{}) + } + specialAdd(group) + + return group +} + // Routes registers all v1 APIs routes to web application. func Routes(sessioner func(http.Handler) http.Handler) *web.Route { m := web.NewRoute() @@ -583,8 +603,13 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route { } m.Use(context.APIContexter()) + group := buildAuthGroup() + if err := group.Init(); err != nil { + log.Error("Could not initialize '%s' auth method, error: %s", group.Name(), err) + } + // Get user from session if logged in. - m.Use(context.APIAuth(auth.NewGroup(auth.Methods()...))) + m.Use(context.APIAuth(group)) m.Use(context.ToggleAPI(&context.ToggleOptions{ SignInRequired: setting.Service.RequireSignInView, diff --git a/routers/api/v1/auth.go b/routers/api/v1/auth.go new file mode 100644 index 000000000..359c9ec56 --- /dev/null +++ b/routers/api/v1/auth.go @@ -0,0 +1,12 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +//go:build !windows +// +build !windows + +package v1 + +import auth_service "code.gitea.io/gitea/services/auth" + +func specialAdd(group *auth_service.Group) {} diff --git a/routers/api/v1/auth_windows.go b/routers/api/v1/auth_windows.go new file mode 100644 index 000000000..d41c4bb22 --- /dev/null +++ b/routers/api/v1/auth_windows.go @@ -0,0 +1,20 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package v1 + +import ( + "code.gitea.io/gitea/models/auth" + auth_service "code.gitea.io/gitea/services/auth" +) + +// specialAdd registers the SSPI auth method as the last method in the list. +// The SSPI plugin is expected to be executed last, as it returns 401 status code if negotiation +// fails (or if negotiation should continue), which would prevent other authentication methods +// to execute at all. +func specialAdd(group *auth_service.Group) { + if auth.IsSSPIEnabled() { + group.Add(&auth_service.SSPI{}) + } +} diff --git a/routers/web/auth.go b/routers/web/auth.go new file mode 100644 index 000000000..4a7fb856b --- /dev/null +++ b/routers/web/auth.go @@ -0,0 +1,12 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +//go:build !windows +// +build !windows + +package web + +import auth_service "code.gitea.io/gitea/services/auth" + +func specialAdd(group *auth_service.Group) {} diff --git a/routers/web/auth_windows.go b/routers/web/auth_windows.go new file mode 100644 index 000000000..f404fd377 --- /dev/null +++ b/routers/web/auth_windows.go @@ -0,0 +1,20 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package web + +import ( + "code.gitea.io/gitea/models/auth" + auth_service "code.gitea.io/gitea/services/auth" +) + +// specialAdd registers the SSPI auth method as the last method in the list. +// The SSPI plugin is expected to be executed last, as it returns 401 status code if negotiation +// fails (or if negotiation should continue), which would prevent other authentication methods +// to execute at all. +func specialAdd(group *auth_service.Group) { + if auth.IsSSPIEnabled() { + group.Add(&auth_service.SSPI{}) + } +} diff --git a/routers/web/web.go b/routers/web/web.go index f4cabaab6..14e90348b 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -73,6 +73,26 @@ func CorsHandler() func(next http.Handler) http.Handler { } } +// The OAuth2 plugin is expected to be executed first, as it must ignore the user id stored +// in the session (if there is a user id stored in session other plugins might return the user +// object for that id). +// +// The Session plugin is expected to be executed second, in order to skip authentication +// for users that have already signed in. +func buildAuthGroup() *auth_service.Group { + group := auth_service.NewGroup( + &auth_service.OAuth2{}, // FIXME: this should be removed and only applied in download and oauth realted routers + &auth_service.Basic{}, // FIXME: this should be removed and only applied in download and git/lfs routers + auth_service.SharedSession, + ) + if setting.Service.EnableReverseProxyAuth { + group.Add(&auth_service.ReverseProxy{}) + } + specialAdd(group) + + return group +} + // Routes returns all web routes func Routes(sessioner func(http.Handler) http.Handler) *web.Route { routes := web.NewRoute() @@ -160,8 +180,13 @@ func Routes(sessioner func(http.Handler) http.Handler) *web.Route { // Removed: toolbox.Toolboxer middleware will provide debug information which seems unnecessary common = append(common, context.Contexter()) + group := buildAuthGroup() + if err := group.Init(); err != nil { + log.Error("Could not initialize '%s' auth method, error: %s", group.Name(), err) + } + // Get user from session if logged in. - common = append(common, context.Auth(auth_service.NewGroup(auth_service.Methods()...))) + common = append(common, context.Auth(group)) // GetHead allows a HEAD request redirect to GET if HEAD method is not defined for that route common = append(common, middleware.GetHead) diff --git a/services/auth/auth.go b/services/auth/auth.go index bdff777f5..a379cb101 100644 --- a/services/auth/auth.go +++ b/services/auth/auth.go @@ -8,7 +8,6 @@ package auth import ( "fmt" "net/http" - "reflect" "regexp" "strings" @@ -21,75 +20,22 @@ import ( "code.gitea.io/gitea/modules/web/middleware" ) -// authMethods contains the list of authentication plugins in the order they are expected to be -// executed. -// -// The OAuth2 plugin is expected to be executed first, as it must ignore the user id stored -// in the session (if there is a user id stored in session other plugins might return the user -// object for that id). -// -// The Session plugin is expected to be executed second, in order to skip authentication -// for users that have already signed in. -var authMethods = []Method{ - &OAuth2{}, - &Basic{}, - &Session{}, -} - // The purpose of the following three function variables is to let the linter know that // those functions are not dead code and are actually being used var ( _ = handleSignIn + + // SharedSession the session auth should only be used by web, but now both web and API/v1 + // will use it. We can remove this after Web removed dependent API/v1 + SharedSession = &Session{} ) -// Methods returns the instances of all registered methods -func Methods() []Method { - return authMethods -} - -// Register adds the specified instance to the list of available methods -func Register(method Method) { - authMethods = append(authMethods, method) -} - // Init should be called exactly once when the application starts to allow plugins // to allocate necessary resources func Init() { - if setting.Service.EnableReverseProxyAuth { - Register(&ReverseProxy{}) - } - specialInit() - for _, method := range Methods() { - initializable, ok := method.(Initializable) - if !ok { - continue - } - - err := initializable.Init() - if err != nil { - log.Error("Could not initialize '%s' auth method, error: %s", reflect.TypeOf(method).String(), err) - } - } - webauthn.Init() } -// Free should be called exactly once when the application is terminating to allow Auth plugins -// to release necessary resources -func Free() { - for _, method := range Methods() { - freeable, ok := method.(Freeable) - if !ok { - continue - } - - err := freeable.Free() - if err != nil { - log.Error("Could not free '%s' auth method, error: %s", reflect.TypeOf(method).String(), err) - } - } -} - // isAttachmentDownload check if request is a file download (GET) with URL to an attachment func isAttachmentDownload(req *http.Request) bool { return strings.HasPrefix(req.URL.Path, "/attachments/") && req.Method == "GET" diff --git a/services/auth/group.go b/services/auth/group.go index bf047338b..0f40e1a76 100644 --- a/services/auth/group.go +++ b/services/auth/group.go @@ -6,6 +6,8 @@ package auth import ( "net/http" + "reflect" + "strings" "code.gitea.io/gitea/models/db" user_model "code.gitea.io/gitea/models/user" @@ -30,6 +32,24 @@ func NewGroup(methods ...Method) *Group { } } +// Add adds a new method to group +func (b *Group) Add(method Method) { + b.methods = append(b.methods, method) +} + +// Name returns group's methods name +func (b *Group) Name() string { + names := make([]string, 0, len(b.methods)) + for _, m := range b.methods { + if n, ok := m.(Named); ok { + names = append(names, n.Name()) + } else { + names = append(names, reflect.TypeOf(m).Elem().Name()) + } + } + return strings.Join(names, ",") +} + // Init does nothing as the Basic implementation does not need to allocate any resources func (b *Group) Init() error { for _, method := range b.methods { diff --git a/services/auth/placeholder.go b/services/auth/placeholder.go deleted file mode 100644 index d9a0ceae7..000000000 --- a/services/auth/placeholder.go +++ /dev/null @@ -1,10 +0,0 @@ -// Copyright 2021 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -//go:build !windows -// +build !windows - -package auth - -func specialInit() {} diff --git a/services/auth/sspi_windows.go b/services/auth/sspi_windows.go index 3a8c8bed4..63e70e61d 100644 --- a/services/auth/sspi_windows.go +++ b/services/auth/sspi_windows.go @@ -244,13 +244,3 @@ func sanitizeUsername(username string, cfg *sspi.Source) string { username = replaceSeparators(username, cfg) return username } - -// specialInit registers the SSPI auth method as the last method in the list. -// The SSPI plugin is expected to be executed last, as it returns 401 status code if negotiation -// fails (or if negotiation should continue), which would prevent other authentication methods -// to execute at all. -func specialInit() { - if auth.IsSSPIEnabled() { - Register(&SSPI{}) - } -}