diff --git a/modules/web/middleware/request.go b/modules/web/middleware/request.go index 0bb155df70340..b5c2cb71ae0b4 100644 --- a/modules/web/middleware/request.go +++ b/modules/web/middleware/request.go @@ -12,3 +12,7 @@ import ( func IsAPIPath(req *http.Request) bool { return strings.HasPrefix(req.URL.Path, "/api/") } + +func IsLoginPath(req *http.Request) bool { + return strings.HasPrefix(req.URL.Path, "/user/login") +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 39da4be17900e..b58ce7141a8c3 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -418,7 +418,7 @@ authorize_application_description = If you grant the access, it will be able to authorize_title = Authorize "%s" to access your account? authorization_failed = Authorization failed authorization_failed_desc = The authorization failed because we detected an invalid request. Please contact the maintainer of the app you have tried to authorize. -sspi_auth_failed = SSPI authentication failed +authorization.error = Authorization failed. If this error persists, please contact the site administrator. password_pwned = The password you chose is on a list of stolen passwords previously exposed in public data breaches. Please try again with a different password and consider changing this password elsewhere too. password_pwned_err = Could not complete request to HaveIBeenPwned diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index c20a45ebc9721..5052172f7520a 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -163,7 +163,7 @@ func SignIn(ctx *context.Context) { context.SetCaptchaData(ctx) } - ctx.HTML(http.StatusOK, tplSignIn) + ctx.HTML(http.StatusUnauthorized, tplSignIn) } // SignInPost response for sign in request @@ -184,7 +184,7 @@ func SignInPost(ctx *context.Context) { ctx.Data["EnableSSPI"] = auth.IsSSPIEnabled() if ctx.HasError() { - ctx.HTML(http.StatusOK, tplSignIn) + ctx.HTML(http.StatusUnauthorized, tplSignIn) return } diff --git a/services/auth/middleware.go b/services/auth/middleware.go index 4a0b613fa662f..f293799493f6f 100644 --- a/services/auth/middleware.go +++ b/services/auth/middleware.go @@ -21,7 +21,13 @@ func Auth(authMethod Method) func(*context.Context) { ar, err := authShared(ctx.Base, ctx.Session, authMethod) if err != nil { log.Error("Failed to verify user: %v", err) - ctx.Error(http.StatusUnauthorized, "Verify") + + // If the error occurs on the login page, fallthrough and render the login form again with a generic error message. + if middleware.IsLoginPath(ctx.Req) { + ctx.Flash.Error(ctx.Locale.Tr("auth.authorization.error"), true) + } else { + ctx.Error(http.StatusUnauthorized, "Verify") + } return } ctx.Doer = ar.Doer diff --git a/services/auth/sspi_windows.go b/services/auth/sspi_windows.go index a4880c7334604..4728c60d6ea43 100644 --- a/services/auth/sspi_windows.go +++ b/services/auth/sspi_windows.go @@ -6,16 +6,14 @@ package auth import ( "errors" "net/http" + "strconv" "strings" "sync" "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/avatars" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/base" - gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/services/auth/source/sspi" @@ -24,10 +22,6 @@ import ( "github.com/quasoft/websspi" ) -const ( - tplSignIn base.TplName = "user/auth/signin" -) - var ( // sspiAuth is a global instance of the websspi authentication package, // which is used to avoid acquiring the server credential handle on @@ -77,20 +71,9 @@ func (s *SSPI) Verify(req *http.Request, w http.ResponseWriter, store DataStore, log.Trace("SSPI Authorization: Attempting to authenticate") userInfo, outToken, err := sspiAuth.Authenticate(req, w) if err != nil { - log.Warn("Authentication failed with error: %v\n", err) sspiAuth.AppendAuthenticateHeader(w, outToken) - - // Include the user login page in the 401 response to allow the user - // to login with another authentication method if SSPI authentication - // fails - store.GetData()["Flash"] = map[string]string{ - "ErrorMsg": err.Error(), - } - store.GetData()["EnableOpenIDSignIn"] = setting.Service.EnableOpenIDSignIn - store.GetData()["EnableSSPI"] = true - // in this case, the Verify function is called in Gitea's web context - // FIXME: it doesn't look good to render the page here, why not redirect? - gitea_context.GetWebContext(req).HTML(http.StatusUnauthorized, tplSignIn) + // The SSPI workflow requires a 401 StatusUnauthorized response code + // which gets set by the auth routes. return nil, err } if outToken != "" { @@ -145,18 +128,14 @@ func (s *SSPI) getConfig() (*sspi.Source, error) { } func (s *SSPI) shouldAuthenticate(req *http.Request) (shouldAuth bool) { - shouldAuth = false - path := strings.TrimSuffix(req.URL.Path, "/") - if path == "/user/login" { + if middleware.IsLoginPath(req) { if req.FormValue("user_name") != "" && req.FormValue("password") != "" { - shouldAuth = false - } else if req.FormValue("auth_with_sspi") == "1" { - shouldAuth = true + return false } - } else if middleware.IsAPIPath(req) || isAttachmentDownload(req) { - shouldAuth = true + b, _ := strconv.ParseBool(req.FormValue("auth_with_sspi")) + return b } - return shouldAuth + return middleware.IsAPIPath(req) || isAttachmentDownload(req) } // newUser creates a new user object for the purpose of automatic registration @@ -171,11 +150,10 @@ func (s *SSPI) newUser(username string, cfg *sspi.Source) (*user_model.User, err UseCustomAvatar: true, Avatar: avatars.DefaultAvatarLink(), } - emailNotificationPreference := user_model.EmailNotificationsDisabled overwriteDefault := &user_model.CreateUserOverwriteOptions{ IsActive: util.OptionalBoolOf(cfg.AutoActivateUsers), KeepEmailPrivate: util.OptionalBoolTrue, - EmailNotificationsPreference: &emailNotificationPreference, + EmailNotificationsPreference: util.ToPointer(user_model.EmailNotificationsDisabled), } if err := user_model.CreateUser(user, overwriteDefault); err != nil { return nil, err