handle reset password edge cases properly and consistently

go-gitea · Oct 7, 2018 · f8cf890 · f8cf890
1 parent d3a4d76
commit f8cf890
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 47 deletions.
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
@@ -219,7 +219,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
@@ -1139,73 +1139,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
+
+	// 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
@@ -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>