From 4e0b4516b9371e1de4098cc8061149ae8a87fbfd Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Sun, 7 Oct 2018 05:36:05 +0000 Subject: [PATCH 1/6] allow current user to reset their own password --- routers/routes/routes.go | 6 ++++-- routers/user/auth.go | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 5c6d36befaed9..3996df1b8f796 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -253,6 +253,10 @@ func RegisterRoutes(m *macaron.Macaron) { m.Get("/^:type(issues|pulls)$", reqSignIn, user.Issues) // ***** START: User ***** + m.Group("/user", func() { + m.Get("/reset_password", user.ResetPasswd) + m.Post("/reset_password", user.ResetPasswdPost) + }) m.Group("/user", func() { m.Get("/login", user.SignIn) m.Post("/login", bindIgnErr(auth.SignInForm{}), user.SignInPost) @@ -273,8 +277,6 @@ func RegisterRoutes(m *macaron.Macaron) { }, openIDSignInEnabled) m.Get("/sign_up", user.SignUp) m.Post("/sign_up", bindIgnErr(auth.RegisterForm{}), user.SignUpPost) - m.Get("/reset_password", user.ResetPasswd) - m.Post("/reset_password", user.ResetPasswdPost) m.Group("/oauth2", func() { m.Get("/:provider", user.SignInOAuth) m.Get("/:provider/callback", user.SignInOAuthCallback) diff --git a/routers/user/auth.go b/routers/user/auth.go index 7e8ac94e4b5d3..8b85429a73fae 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -893,8 +893,7 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au ctx.Redirect(setting.AppSubURL + "/user/login") } -// SignOut sign out from login status -func SignOut(ctx *context.Context) { +func handleSignOut(ctx *context.Context) { ctx.Session.Delete("uid") ctx.Session.Delete("uname") ctx.Session.Delete("socialId") @@ -904,6 +903,11 @@ func SignOut(ctx *context.Context) { ctx.SetCookie(setting.CookieRememberName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) ctx.SetCookie("lang", "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) // Setting the lang cookie will trigger the middleware to reset the language ot previous state. +} + +// SignOut sign out from login status +func SignOut(ctx *context.Context) { + handleSignOut(ctx) ctx.Redirect(setting.AppSubURL + "/") } @@ -1178,6 +1182,8 @@ func ForgotPasswdPost(ctx *context.Context) { 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) @@ -1222,6 +1228,10 @@ func ResetPasswdPost(ctx *context.Context) { ctx.ServerError("UpdateUser", err) return } + + // Just in case the user is signed in to another account + handleSignOut(ctx) + u.HashPassword(passwd) u.MustChangePassword = false if err := models.UpdateUserCols(u, "must_change_password", "passwd", "rands", "salt"); err != nil { @@ -1230,6 +1240,9 @@ func ResetPasswdPost(ctx *context.Context) { } 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") return } From a93b4efb48049d62dc35b835a6bf178199118c7d Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Sun, 7 Oct 2018 20:18:21 +0000 Subject: [PATCH 2/6] handle reset password edge cases properly and consistently --- options/locale/locale_en-US.ini | 3 +- routers/user/auth.go | 108 ++++++++++++++------------ templates/user/auth/reset_passwd.tmpl | 15 ++++ 3 files changed, 77 insertions(+), 49 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 23d1949203267..41b4c997f1ae2 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -218,7 +218,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 8b85429a73fae..8a7cef008262e 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -1178,77 +1178,89 @@ func ForgotPasswdPost(ctx *context.Context) { ctx.HTML(200, tplForgotPassword) } -// ResetPasswd render the reset password page -func ResetPasswd(ctx *context.Context) { +func commonResetPassword(ctx *context.Context) *models.User { + code := ctx.Query("code") + ctx.Data["Title"] = ctx.Tr("auth.reset_password") + ctx.Data["Code"] = code - // TODO for security and convenience, show the username / email here + if nil != ctx.User { + ctx.Data["user_signed_in"] = true + } - code := ctx.Query("code") if len(code) == 0 { - ctx.Error(404) - return + ctx.Flash.Error(ctx.Tr("auth.invalid_code")) + return nil } - ctx.Data["Code"] = code - if u := models.VerifyUserActiveCode(code); u != nil { - ctx.Data["IsResetForm"] = true + // 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["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") + u := commonResetPassword(ctx) - code := ctx.Query("code") - if len(code) == 0 { - ctx.Error(404) + if u == nil { + // Flash error has been set + ctx.HTML(200, tplResetPassword) 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) - u.MustChangePassword = false - if err := models.UpdateUserCols(u, "must_change_password", "passwd", "rands", "salt"); err != nil { - ctx.ServerError("UpdateUser", err) - return - } + // 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 + } - log.Trace("User password reset: %s", u.Name) + 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 + } - // 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.HashPassword(passwd) + u.MustChangePassword = false + if err := models.UpdateUserCols(u, "must_change_password", "passwd", "rands", "salt"); err != nil { + ctx.ServerError("UpdateUser", err) return } + log.Trace("User password reset: %s", u.Name) + ctx.Data["IsResetFailed"] = true - ctx.HTML(200, tplResetPassword) + remember := len(ctx.Query("remember")) != 0 + handleSignInFull(ctx, u, remember, true) } // MustChangePassword renders the page to change a user's password diff --git a/templates/user/auth/reset_passwd.tmpl b/templates/user/auth/reset_passwd.tmpl index 19a7c8eea8a1b..e7d939294e3f0 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}}
From 44f943e5eae836c276161a29eb823fc386d87364 Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Thu, 11 Oct 2018 02:28:23 +0000 Subject: [PATCH 3/6] remove dangling assignment --- routers/user/auth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/user/auth.go b/routers/user/auth.go index 8a7cef008262e..2768f49ca6093 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -1215,7 +1215,7 @@ func commonResetPassword(ctx *context.Context) *models.User { func ResetPasswd(ctx *context.Context) { ctx.Data["IsResetForm"] = true - _ = commonResetPassword(ctx) + commonResetPassword(ctx) ctx.HTML(200, tplResetPassword) } From 87314f2f552e12ae193fc64c0fcb2c458b351b99 Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Mon, 15 Oct 2018 18:49:22 +0000 Subject: [PATCH 4/6] properly label account recovery instead of reset password --- models/mail.go | 2 +- options/locale/locale_en-US.ini | 17 +++++++++-------- routers/routes/routes.go | 6 ++---- routers/user/auth.go | 4 ++-- templates/mail/auth/reset_passwd.tmpl | 2 +- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/models/mail.go b/models/mail.go index 4f2bd548f79ec..c8f67bd345b7f 100644 --- a/models/mail.go +++ b/models/mail.go @@ -73,7 +73,7 @@ func SendActivateAccountMail(c *macaron.Context, u *User) { // SendResetPasswordMail sends a password reset mail to the user func SendResetPasswordMail(c *macaron.Context, u *User) { - SendUserMail(c, u, mailAuthResetPassword, u.GenerateActivateCode(), c.Tr("mail.reset_password"), "reset password") + SendUserMail(c, u, mailAuthResetPassword, u.GenerateActivateCode(), c.Tr("mail.reset_password"), "recover account") } // SendActivateEmailMail sends confirmation email to confirm new email address diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 41b4c997f1ae2..6920ef8c684ff 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -206,7 +206,7 @@ sign_up_successful = Account was successfully created. confirmation_mail_sent_prompt = A new confirmation email has been sent to %s. Please check your inbox within the next %s to complete the registration process. must_change_password = Update your password allow_password_change = Require user to change password (recommended) -reset_password_mail_sent_prompt = A confirmation email has been sent to %s. Please check your inbox within the next %s to complete the password reset process. +reset_password_mail_sent_prompt = A confirmation email has been sent to %s. Please check your inbox within the next %s to complete the account recovery process. active_your_account = Activate Your Account account_activated = Account has been activated prohibit_login = Sign In Prohibited @@ -215,11 +215,11 @@ resent_limit_prompt = You have already requested an activation email recently. P has_unconfirmed_mail = Hi %s, you have an unconfirmed email address (%s). If you haven't received a confirmation email or need to resend a new one, please click on the button below. resend_mail = Click here to resend your activation email 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 +send_reset_mail = Click here to resend your account recovery email +reset_password = Account Recovery invalid_code = Your confirmation code is invalid or has expired. -reset_password_helper = Reset Password -reset_password_wrong_user = You are signed in as %s, but the password reset link is for %s +reset_password_helper = Recover Account +reset_password_wrong_user = You are signed in as %s, but the account recovery 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 @@ -242,7 +242,7 @@ openid_connect_desc = The chosen OpenID URI is unknown. Associate it with a new openid_register_title = Create new account openid_register_desc = The chosen OpenID URI is unknown. Associate it with a new account here. openid_signin_desc = Enter your OpenID URI. For example: https://anne.me, bob.openid.org.cn or gnusocial.net/carry. -disable_forgot_password_mail = Password reset is disabled. Please contact your site administrator. +disable_forgot_password_mail = Account recovery is disabled. Please contact your site administrator. email_domain_blacklisted = You cannot register with your email address. authorize_application = Authorize Application authroize_redirect_notice = You will be redirected to %s if you authorize this application. @@ -251,11 +251,12 @@ 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've tried to authorize. +disable_forgot_password_mail = Account recovery is disabled. Please contact your site administrator. [mail] activate_account = Please activate your account activate_email = Verify your email address -reset_password = Reset your password +reset_password = Recover your account register_success = Registration successful register_notify = Welcome to Gitea @@ -1682,7 +1683,7 @@ config.mail_notify = Enable Email Notifications config.disable_key_size_check = Disable Minimum Key Size Check config.enable_captcha = Enable CAPTCHA config.active_code_lives = Active Code Lives -config.reset_password_code_lives = Reset Password Code Expiry Time +config.reset_password_code_lives = Recover Account Code Expiry Time config.default_keep_email_private = Hide Email Addresses by Default config.default_allow_create_organization = Allow Creation of Organizations by Default config.enable_timetracking = Enable Time Tracking diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 3996df1b8f796..eb4a7a8b8f672 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -253,10 +253,6 @@ func RegisterRoutes(m *macaron.Macaron) { m.Get("/^:type(issues|pulls)$", reqSignIn, user.Issues) // ***** START: User ***** - m.Group("/user", func() { - m.Get("/reset_password", user.ResetPasswd) - m.Post("/reset_password", user.ResetPasswdPost) - }) m.Group("/user", func() { m.Get("/login", user.SignIn) m.Post("/login", bindIgnErr(auth.SignInForm{}), user.SignInPost) @@ -383,6 +379,8 @@ func RegisterRoutes(m *macaron.Macaron) { m.Any("/activate", user.Activate, reqSignIn) m.Any("/activate_email", user.ActivateEmail) m.Get("/email2user", user.Email2User) + m.Get("/recover_account", user.ResetPasswd) + m.Post("/recover_account", user.ResetPasswdPost) m.Get("/forgot_password", user.ForgotPasswd) m.Post("/forgot_password", user.ForgotPasswdPost) m.Get("/logout", user.SignOut) diff --git a/routers/user/auth.go b/routers/user/auth.go index 2768f49ca6093..433a4a87dce2f 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -1211,7 +1211,7 @@ func commonResetPassword(ctx *context.Context) *models.User { return u } -// ResetPasswd render the reset password page +// ResetPasswd render the account recovery page func ResetPasswd(ctx *context.Context) { ctx.Data["IsResetForm"] = true @@ -1220,7 +1220,7 @@ func ResetPasswd(ctx *context.Context) { ctx.HTML(200, tplResetPassword) } -// ResetPasswdPost response from reset password request +// ResetPasswdPost response from account recovery request func ResetPasswdPost(ctx *context.Context) { u := commonResetPassword(ctx) diff --git a/templates/mail/auth/reset_passwd.tmpl b/templates/mail/auth/reset_passwd.tmpl index 5fb98c7819fbc..65e8ad980a42e 100644 --- a/templates/mail/auth/reset_passwd.tmpl +++ b/templates/mail/auth/reset_passwd.tmpl @@ -8,7 +8,7 @@

Hi {{.DisplayName}},

Please click the following link to reset your password within {{.ResetPwdCodeLives}}:

-

{{AppUrl}}user/reset_password?code={{.Code}}

+

{{AppUrl}}user/recover_account?code={{.Code}}

Not working? Try copying and pasting it to your browser.

© {{AppName}}

From e7a01b6cead4763527f0d4f2f7486b7b1966a9b1 Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Mon, 15 Oct 2018 19:16:27 +0000 Subject: [PATCH 5/6] remove 'Click here' from button --- options/locale/locale_en-US.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 6920ef8c684ff..3aba48944aff0 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -215,7 +215,7 @@ resent_limit_prompt = You have already requested an activation email recently. P has_unconfirmed_mail = Hi %s, you have an unconfirmed email address (%s). If you haven't received a confirmation email or need to resend a new one, please click on the button below. resend_mail = Click here to resend your activation email email_not_associate = The email address is not associated with any account. -send_reset_mail = Click here to resend your account recovery email +send_reset_mail = Send Account Recovery Email reset_password = Account Recovery invalid_code = Your confirmation code is invalid or has expired. reset_password_helper = Recover Account From 383998494c960f9c6acbea6cd49d7260f65782bd Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Mon, 15 Oct 2018 19:36:35 +0000 Subject: [PATCH 6/6] update English-only account-recovery templates --- templates/mail/auth/register_notify.tmpl | 2 +- templates/mail/auth/reset_passwd.tmpl | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/templates/mail/auth/register_notify.tmpl b/templates/mail/auth/register_notify.tmpl index 4ee25329c47b7..ea1857030abaf 100644 --- a/templates/mail/auth/register_notify.tmpl +++ b/templates/mail/auth/register_notify.tmpl @@ -9,7 +9,7 @@

Hi {{.DisplayName}}, this is your registration confirmation email for {{AppName}}!

You can now login via username: {{.Username}}.

{{AppUrl}}user/login

-

If this account has been created for you, please reset your password first.

+

If this account has been created for you, please set your password first.

© {{AppName}}

diff --git a/templates/mail/auth/reset_passwd.tmpl b/templates/mail/auth/reset_passwd.tmpl index 65e8ad980a42e..e01d57cea25e6 100644 --- a/templates/mail/auth/reset_passwd.tmpl +++ b/templates/mail/auth/reset_passwd.tmpl @@ -2,12 +2,13 @@ - {{.DisplayName}}, you have requested to reset your password + {{.DisplayName}}, you have requested to recover your account

Hi {{.DisplayName}},

-

Please click the following link to reset your password within {{.ResetPwdCodeLives}}:

+

Please click the following link to recover your account within {{.ResetPwdCodeLives}}:

+

{{AppUrl}}user/recover_account?code={{.Code}}

Not working? Try copying and pasting it to your browser.

© {{AppName}}