From 9d3690f8df4f167014bef4ca41f327978dbb536f Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Sun, 7 Oct 2018 17:28:34 +0000 Subject: [PATCH] handle reset password edge cases properly and consistently --- options/locale/locale_en-US.ini | 3 +- routers/user/auth.go | 125 ++++++++++++++------------ templates/user/auth/reset_passwd.tmpl | 15 ++++ 3 files changed, 87 insertions(+), 56 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ebc84aec4..c2ae05250 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -211,7 +211,8 @@ email_not_associate = The email address is not associated with any account. send_reset_mail = Click here to resend your password reset email reset_password = Reset Your Password invalid_code = Your confirmation code is invalid or has expired. -reset_password_helper = Click here to reset your password +reset_password_helper = Reset Password +reset_password_wrong_user = You are signed in as %s, but the password reset link is for %s password_too_short = Password length cannot be less than %d characters. non_local_account = Non-local users can not update their password through the Gitea web interface. verify = Verify diff --git a/routers/user/auth.go b/routers/user/auth.go index 9b59725b5..aae814136 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -1144,70 +1144,85 @@ func ForgotPasswdPost(ctx *context.Context) { ctx.HTML(200, tplForgotPassword) } +// Keep consistent behavior between phases of password reset +func commonResetPassword(ctx *context.Context) *models.User { + code := ctx.Query("code") + + ctx.Data["Title"] = ctx.Tr("auth.reset_password") + ctx.Data["Code"] = code + + if nil != ctx.User { + ctx.Data["user_signed_in"] = true + } + + if len(code) == 0 { + ctx.Flash.Error(ctx.Tr("auth.invalid_code")) + return nil + } + + // Fail early, don't frustrate the user + u := models.VerifyUserActiveCode(code) + if u == nil { + ctx.Flash.Error(ctx.Tr("auth.invalid_code")) + return nil + } + + // Show the user that they are affecting the account that they intended to + ctx.Data["user_email"] = u.Email + + if nil != ctx.User && u.ID != ctx.User.ID { + ctx.Flash.Error(ctx.Tr("auth.reset_password_wrong_user", ctx.User.Email, u.Email)) + return nil + } + + return u +} + // ResetPasswd render the reset password page func ResetPasswd(ctx *context.Context) { - ctx.Data["Title"] = ctx.Tr("auth.reset_password") - - // TODO for security and convenience, show the username / email here - - code := ctx.Query("code") - if len(code) == 0 { - ctx.Error(404) - return - } - ctx.Data["Code"] = code ctx.Data["IsResetForm"] = true + + _ = commonResetPassword(ctx) + ctx.HTML(200, tplResetPassword) } // ResetPasswdPost response from reset password request func ResetPasswdPost(ctx *context.Context) { - ctx.Data["Title"] = ctx.Tr("auth.reset_password") - - code := ctx.Query("code") - if len(code) == 0 { - ctx.Error(404) - return - } - ctx.Data["Code"] = code - - if u := models.VerifyUserActiveCode(code); u != nil { - // Validate password length. - passwd := ctx.Query("password") - if len(passwd) < setting.MinPasswordLength { - ctx.Data["IsResetForm"] = true - ctx.Data["Err_Password"] = true - ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplResetPassword, nil) - return - } - - var err error - if u.Rands, err = models.GetUserSalt(); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - if u.Salt, err = models.GetUserSalt(); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - - // Just in case the user is signed in to another account - handleSignOut(ctx) - - u.HashPassword(passwd) - if err := models.UpdateUserCols(u, "passwd", "rands", "salt"); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - - log.Trace("User password reset: %s", u.Name) - - // TODO change the former form to have password retype and remember me, - // then sign in here instead of redirecting - ctx.Redirect(setting.AppSubURL + "/user/login") + u := commonResetPassword(ctx) + if nil == u { + // Flash Error has been set + ctx.HTML(200, tplResetPassword) return } - ctx.Data["IsResetFailed"] = true - ctx.HTML(200, tplResetPassword) + // Validate password length. + passwd := ctx.Query("password") + if len(passwd) < setting.MinPasswordLength { + ctx.Data["IsResetForm"] = true + ctx.Data["Err_Password"] = true + ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplResetPassword, nil) + return + } + + var err error + if u.Rands, err = models.GetUserSalt(); err != nil { + ctx.ServerError("UpdateUser", err) + return + } + if u.Salt, err = models.GetUserSalt(); err != nil { + ctx.ServerError("UpdateUser", err) + return + } + + u.HashPassword(passwd) + if err := models.UpdateUserCols(u, "passwd", "rands", "salt"); err != nil { + ctx.ServerError("UpdateUser", err) + return + } + + log.Trace("User password reset: %s", u.Name) + + remember := len(ctx.Query("remember")) != 0 + handleSignInFull(ctx, u, remember, true) } diff --git a/templates/user/auth/reset_passwd.tmpl b/templates/user/auth/reset_passwd.tmpl index 19a7c8eea..e7d939294 100644 --- a/templates/user/auth/reset_passwd.tmpl +++ b/templates/user/auth/reset_passwd.tmpl @@ -10,11 +10,26 @@
{{template "base/alert" .}} + {{if .user_email }} +
+ + +
+ {{end}} {{if .IsResetForm}}
+ {{if not .user_signed_in}} +
+ +
+ + +
+
+ {{end}}