Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UX + Security current user password reset #5042

Merged
merged 7 commits into from
Apr 18, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion models/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 9 additions & 7 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ sign_up_successful = Account was successfully created.
confirmation_mail_sent_prompt = A new confirmation email has been sent to <b>%s</b>. 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 <b>%s</b>. 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 <b>%s</b>. 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
Expand All @@ -215,10 +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 (<b>%s</b>). 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 = Send Account Recovery Email
reset_password = Account Recovery
invalid_code = Your confirmation code is invalid or has expired.
reset_password_helper = Click here to reset your password
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
Expand All @@ -241,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.
Expand All @@ -250,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

Expand Down Expand Up @@ -1681,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
Expand Down
4 changes: 2 additions & 2 deletions routers/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to atleast redirect to the new route before this is finally removed in 1.10?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adelowo I don't see why as emails having old route won't be usable anyway as code is valid only for few hours

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha true... stupid me

m.Post("/reset_password", user.ResetPasswdPost)
m.Group("/oauth2", func() {
m.Get("/:provider", user.SignInOAuth)
m.Get("/:provider/callback", user.SignInOAuthCallback)
Expand Down Expand Up @@ -381,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)
Expand Down
111 changes: 68 additions & 43 deletions routers/user/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 + "/")
}

Expand Down Expand Up @@ -1174,68 +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

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 account recovery page
func ResetPasswd(ctx *context.Context) {
ctx.Data["IsResetForm"] = true

commonResetPassword(ctx)

ctx.HTML(200, tplResetPassword)
}

// ResetPasswdPost response from reset password request
// ResetPasswdPost response from account recovery 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
}
// 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)
u.MustChangePassword = false
if err := models.UpdateUserCols(u, "must_change_password", "passwd", "rands", "salt"); err != nil {
ctx.ServerError("UpdateUser", err)
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
}

log.Trace("User password reset: %s", u.Name)
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
Expand Down
2 changes: 1 addition & 1 deletion templates/mail/auth/register_notify.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<p>Hi <b>{{.DisplayName}}</b>, this is your registration confirmation email for {{AppName}}!</p>
<p>You can now login via username: {{.Username}}.</p>
<p><a href="{{AppUrl}}user/login">{{AppUrl}}user/login</a></p>
<p>If this account has been created for you, please <a href="{{AppUrl}}user/forgot_password">reset your password</a> first.</p>
<p>If this account has been created for you, please <a href="{{AppUrl}}user/forgot_password">set your password</a> first.</p>
<p>© <a target="_blank" rel="noopener noreferrer" href="{{AppUrl}}">{{AppName}}</a></p>
</body>
</html>
7 changes: 4 additions & 3 deletions templates/mail/auth/reset_passwd.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>{{.DisplayName}}, you have requested to reset your password</title>
<title>{{.DisplayName}}, you have requested to recover your account</title>
</head>

<body>
<p>Hi <b>{{.DisplayName}}</b>,</p>
<p>Please click the following link to reset your password within <b>{{.ResetPwdCodeLives}}</b>:</p>
<p><a href="{{AppUrl}}user/reset_password?code={{.Code}}">{{AppUrl}}user/reset_password?code={{.Code}}</a></p>
<p>Please click the following link to recover your account within <b>{{.ResetPwdCodeLives}}</b>:</p>

<p><a href="{{AppUrl}}user/recover_account?code={{.Code}}">{{AppUrl}}user/recover_account?code={{.Code}}</a></p>
<p>Not working? Try copying and pasting it to your browser.</p>
<p>© <a target="_blank" rel="noopener noreferrer" href="{{AppUrl}}">{{AppName}}</a></p>
</body>
Expand Down
15 changes: 15 additions & 0 deletions templates/user/auth/reset_passwd.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,26 @@
</h2>
<div class="ui attached segment">
{{template "base/alert" .}}
{{if .user_email }}
<div class="inline field">
<label for="user_name">{{.i18n.Tr "email"}}</label>
<input id="user_name" type="text" value="{{ .user_email }}" disabled>
</div>
{{end}}
{{if .IsResetForm}}
<div class="required inline field {{if .Err_Password}}error{{end}}">
<label for="password">{{.i18n.Tr "password"}}</label>
<input id="password" name="password" type="password" value="{{.password}}" autocomplete="off" autofocus required>
</div>
{{if not .user_signed_in}}
<div class="inline field">
<label></label>
<div class="ui checkbox">
<label>{{.i18n.Tr "auth.remember_me"}}</label>
<input name="remember" type="checkbox">
</div>
</div>
{{end}}
<div class="ui divider"></div>
<div class="inline field">
<label></label>
Expand Down