Skip to content

Commit

Permalink
Log the real reason when authentication fails (but don't show the use…
Browse files Browse the repository at this point in the history
…r) (#25414)
  • Loading branch information
lunny authored Jul 3, 2023
1 parent ad57be0 commit 0403bd9
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 14 deletions.
2 changes: 1 addition & 1 deletion routers/web/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
35 changes: 29 additions & 6 deletions routers/web/auth/linkaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down
6 changes: 1 addition & 5 deletions routers/web/auth/openid.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
39 changes: 37 additions & 2 deletions services/auth/source/db/authenticate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 0403bd9

Please sign in to comment.