From 0403bd989f60ab84497eb5e04366496b3c9d2534 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 4 Jul 2023 06:39:38 +0800 Subject: [PATCH] Log the real reason when authentication fails (but don't show the user) (#25414) --- routers/web/auth/auth.go | 2 +- routers/web/auth/linkaccount.go | 35 ++++++++++++++++++---- routers/web/auth/openid.go | 6 +--- services/auth/source/db/authenticate.go | 39 +++++++++++++++++++++++-- 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index bc8f6d58c9..9dcc38247d 100644 --- 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 index 865bcca152..522e78a60a 100644 --- 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 index 5e0e7b258f..7a4bb7f209 100644 --- 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 index 773ec601ba..34a0459149 100644 --- 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