]> source.dussan.org Git - gitea.git/commitdiff
Let web and API routes have different auth methods group (#19168)
authorLunny Xiao <xiaolunwen@gmail.com>
Mon, 28 Mar 2022 04:46:28 +0000 (12:46 +0800)
committerGitHub <noreply@github.com>
Mon, 28 Mar 2022 04:46:28 +0000 (12:46 +0800)
* remove the global methods but create dynamiclly

* Fix lint

* Fix windows lint

* Fix windows lint

* some improvements

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
routers/api/v1/api.go
routers/api/v1/auth.go [new file with mode: 0644]
routers/api/v1/auth_windows.go [new file with mode: 0644]
routers/web/auth.go [new file with mode: 0644]
routers/web/auth_windows.go [new file with mode: 0644]
routers/web/web.go
services/auth/auth.go
services/auth/group.go
services/auth/placeholder.go [deleted file]
services/auth/sspi_windows.go

index 5ac6fba29be400c7bb4371bf2d26d4083d430ad1..3debf58a17523c8f047fca15d4d3616176214dfd 100644 (file)
@@ -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 (file)
index 0000000..359c9ec
--- /dev/null
@@ -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 (file)
index 0000000..d41c4bb
--- /dev/null
@@ -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 (file)
index 0000000..4a7fb85
--- /dev/null
@@ -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 (file)
index 0000000..f404fd3
--- /dev/null
@@ -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{})
+       }
+}
index f4cabaab6e74f1d96dc67601e69c588ce1be55b9..14e90348b804f52d116aca0c9b85fe9be627cfdc 100644 (file)
@@ -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)
index bdff777f506e49e6520885f08cca91e2bb6e88c2..a379cb101315ddc9c2e2b8a469344b7afa361dfd 100644 (file)
@@ -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
-)
-
-// 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)
-}
+       // 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{}
+)
 
 // 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"
index bf047338bbd594bf6ac9e421d2ec66a07ed838ac..0f40e1a76c9bee33a53af2f22316a5fba9e44c6a 100644 (file)
@@ -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 (file)
index d9a0cea..0000000
+++ /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() {}
index 3a8c8bed443ee09a543bb665fe9f3d296679ce70..63e70e61d4335f653c2010b5ed571349ad9959d2 100644 (file)
@@ -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{})
-       }
-}