fix

Author: wxiaoguang

routers/api/v1/api.go

@@ -705,7 +705,10 @@ func buildAuthGroup() *auth.Group {
 	if setting.Service.EnableReverseProxyAuthAPI {
 		group.Add(&auth.ReverseProxy{})
 	}
-	specialAdd(group)
+
+	if setting.IsWindows && auth_model.IsSSPIEnabled() {
+		group.Add(&auth.SSPI{}) // it MUST be the last, see the comment of SSPI
+	}
 
 	return group
 }

routers/api/v1/auth.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"strings"
 
+	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/perm"
 	"code.gitea.io/gitea/models/unit"
 	"code.gitea.io/gitea/modules/context"
@@ -92,7 +93,10 @@ func buildAuthGroup() *auth_service.Group {
 	if setting.Service.EnableReverseProxyAuth {
 		group.Add(&auth_service.ReverseProxy{})
 	}
-	specialAdd(group)
+
+	if setting.IsWindows && auth_model.IsSSPIEnabled() {
+		group.Add(&auth_service.SSPI{}) // it MUST be the last, see the comment of SSPI
+	}
 
 	return group
 }

routers/api/v1/auth_windows.go

@@ -21,18 +21,19 @@ import (
 	"code.gitea.io/gitea/services/auth/source/sspi"
 
 	gouuid "github.com/google/uuid"
-	"github.com/quasoft/websspi"
 )
 
 const (
 	tplSignIn base.TplName = "user/auth/signin"
 )
 
+type SSPIAuth interface {
+	AppendAuthenticateHeader(w http.ResponseWriter, data string)
+	Authenticate(r *http.Request, w http.ResponseWriter) (userInfo *UserInfo, outToken string, err error)
+}
+
 var (
-	// sspiAuth is a global instance of the websspi authentication package,
-	// which is used to avoid acquiring the server credential handle on
-	// every request
-	sspiAuth     *websspi.Authenticator
+	sspiAuth     SSPIAuth // a global instance of the websspi authenticator to avoid acquiring the server credential handle on every request
 	sspiAuthOnce sync.Once
 
 	// Ensure the struct implements the interface.
@@ -41,8 +42,9 @@ var (
 
 // SSPI implements the SingleSignOn interface and authenticates requests
 // via the built-in SSPI module in Windows for SPNEGO authentication.
-// On successful authentication returns a valid user object.
-// Returns nil if authentication fails.
+// 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.
 type SSPI struct{}
 
 // Name represents the name of auth method
@@ -55,14 +57,7 @@ func (s *SSPI) Name() string {
 // If negotiation should continue or authentication fails, immediately returns a 401 HTTP
 // response code, as required by the SPNEGO protocol.
 func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
-	var errInit error
-	sspiAuthOnce.Do(func() {
-		config := websspi.NewConfig()
-		sspiAuth, errInit = websspi.New(config)
-	})
-	if errInit != nil {
-		return nil, errInit
-	}
+	sspiAuthOnce.Do(sspiAuthInit)
 
 	if !s.shouldAuthenticate(req) {
 		return nil, nil

routers/web/auth.go

@@ -0,0 +1,29 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+//go:build !windows
+
+package auth
+
+import (
+	"errors"
+	"net/http"
+)
+
+type UserInfo struct {
+	Username string   // Name of user, usually in the form DOMAIN\User
+	Groups   []string // The global groups the user is a member of
+}
+
+type sspiAuthMock struct{}
+
+func (s sspiAuthMock) AppendAuthenticateHeader(w http.ResponseWriter, data string) {
+}
+
+func (s sspiAuthMock) Authenticate(r *http.Request, w http.ResponseWriter) (userInfo *UserInfo, outToken string, err error) {
+	return nil, "", errors.New("not implemented")
+}
+
+func sspiAuthInit() {
+	sspiAuth = &sspiAuthMock{} // TODO: we can mock the SSPI auth in tests
+}

routers/web/auth_windows.go

@@ -0,0 +1,20 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+//go:build windows
+
+package auth
+
+import (
+	"github.com/quasoft/websspi"
+)
+
+type UserInfo = websspi.UserInfo
+
+func sspiAuthInit() {
+	var err error
+	config := websspi.NewConfig()
+	if sspiAuth, err = websspi.New(config); err != nil {
+		panic(err) // this init is called by a sync.Once, maybe "panic" is the simplest way to handle errors
+	}
+}