Log the real reason when authentication fails (but don't show the use…

…r) (#25414) (#25660) Backport #25414 by @lunny Fix #24498 Co-authored-by: Lunny Xiao <[email protected]>
go-gitea · Jul 3, 2023 · 69bdcf4 · 69bdcf4
1 parent e610b03
commit 69bdcf4
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 14 deletions.
diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go
@@ -201,7 +201,7 @@ func SignInPost(ctx *context.Context) {
 
 	u, source, err := auth_service.UserSignIn(form.UserName, form.Password)
 	if err != nil {
-		if user_model.IsErrUserNotExist(err) || user_model.IsErrEmailAddressNotExist(err) {
+		if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrInvalidArgument) {
 			ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplSignIn, &form)
 			log.Info("Failed authentication attempt for %s from %s: %v", form.UserName, ctx.RemoteAddr(), err)
 		} else if user_model.IsErrEmailAlreadyUsed(err) {

diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go
@@ -13,7 +13,9 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/base"
 	"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"
 	auth_service "code.gitea.io/gitea/services/auth"
 	"code.gitea.io/gitea/services/auth/source/oauth2"
@@ -81,6 +83,32 @@ func LinkAccount(ctx *context.Context) {
 	ctx.HTML(http.StatusOK, tplLinkAccount)
 }
 
+func handleSignInError(ctx *context.Context, userName string, ptrForm any, tmpl base.TplName, invoker string, err error) {
+	if errors.Is(err, util.ErrNotExist) {
+		ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tmpl, ptrForm)
+	} else if errors.Is(err, util.ErrInvalidArgument) {
+		ctx.Data["user_exists"] = true
+		ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tmpl, ptrForm)
+	} else if user_model.IsErrUserProhibitLogin(err) {
+		ctx.Data["user_exists"] = true
+		log.Info("Failed authentication attempt for %s from %s: %v", userName, ctx.RemoteAddr(), err)
+		ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
+		ctx.HTML(http.StatusOK, "user/auth/prohibit_login")
+	} else if user_model.IsErrUserInactive(err) {
+		ctx.Data["user_exists"] = true
+		if setting.Service.RegisterEmailConfirm {
+			ctx.Data["Title"] = ctx.Tr("auth.active_your_account")
+			ctx.HTML(http.StatusOK, TplActivate)
+		} else {
+			log.Info("Failed authentication attempt for %s from %s: %v", userName, ctx.RemoteAddr(), err)
+			ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
+			ctx.HTML(http.StatusOK, "user/auth/prohibit_login")
+		}
+	} else {
+		ctx.ServerError(invoker, err)
+	}
+}
+
 // LinkAccountPostSignIn handle the coupling of external account with another account using signIn
 func LinkAccountPostSignIn(ctx *context.Context) {
 	signInForm := web.GetForm(ctx).(*forms.SignInForm)
@@ -116,12 +144,7 @@ func LinkAccountPostSignIn(ctx *context.Context) {
 
 	u, _, err := auth_service.UserSignIn(signInForm.UserName, signInForm.Password)
 	if err != nil {
-		if user_model.IsErrUserNotExist(err) {
-			ctx.Data["user_exists"] = true
-			ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplLinkAccount, &signInForm)
-		} else {
-			ctx.ServerError("UserLinkAccount", err)
-		}
+		handleSignInError(ctx, signInForm.UserName, &signInForm, tplLinkAccount, "UserLinkAccount", err)
 		return
 	}
 

diff --git a/routers/web/auth/openid.go b/routers/web/auth/openid.go
@@ -282,11 +282,7 @@ func ConnectOpenIDPost(ctx *context.Context) {
 
 	u, _, err := auth.UserSignIn(form.UserName, form.Password)
 	if err != nil {
-		if user_model.IsErrUserNotExist(err) {
-			ctx.RenderWithErr(ctx.Tr("form.username_password_incorrect"), tplConnectOID, &form)
-		} else {
-			ctx.ServerError("ConnectOpenIDPost", err)
-		}
+		handleSignInError(ctx, form.UserName, &form, tplConnectOID, "ConnectOpenIDPost", err)
 		return
 	}
 

diff --git a/services/auth/source/db/authenticate.go b/services/auth/source/db/authenticate.go
@@ -4,19 +4,54 @@
 package db
 
 import (
+	"fmt"
+
 	"code.gitea.io/gitea/models/db"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 )
 
+// ErrUserPasswordNotSet represents a "ErrUserPasswordNotSet" kind of error.
+type ErrUserPasswordNotSet struct {
+	UID  int64
+	Name string
+}
+
+func (err ErrUserPasswordNotSet) Error() string {
+	return fmt.Sprintf("user's password isn't set [uid: %d, name: %s]", err.UID, err.Name)
+}
+
+// Unwrap unwraps this error as a ErrInvalidArgument error
+func (err ErrUserPasswordNotSet) Unwrap() error {
+	return util.ErrInvalidArgument
+}
+
+// ErrUserPasswordInvalid represents a "ErrUserPasswordInvalid" kind of error.
+type ErrUserPasswordInvalid struct {
+	UID  int64
+	Name string
+}
+
+func (err ErrUserPasswordInvalid) Error() string {
+	return fmt.Sprintf("user's password is invalid [uid: %d, name: %s]", err.UID, err.Name)
+}
+
+// Unwrap unwraps this error as a ErrInvalidArgument error
+func (err ErrUserPasswordInvalid) Unwrap() error {
+	return util.ErrInvalidArgument
+}
+
 // Authenticate authenticates the provided user against the DB
 func Authenticate(user *user_model.User, login, password string) (*user_model.User, error) {
 	if user == nil {
 		return nil, user_model.ErrUserNotExist{Name: login}
 	}
 
-	if !user.IsPasswordSet() || !user.ValidatePassword(password) {
-		return nil, user_model.ErrUserNotExist{UID: user.ID, Name: user.Name}
+	if !user.IsPasswordSet() {
+		return nil, ErrUserPasswordNotSet{UID: user.ID, Name: user.Name}
+	} else if !user.ValidatePassword(password) {
+		return nil, ErrUserPasswordInvalid{UID: user.ID, Name: user.Name}
 	}
 
 	// Update password hash if server password hash algorithm have changed