Refactor "user/active" related logic (#29390)

And add more tests. Remove a lot of fragile "if" blocks.

The old logic is kept as-is.

1 files changed, 66 insertions, 59 deletions

diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go
index 3de1f3373d..a30ee0ce54 100644
--- a/routers/web/auth/auth.go
+++ b/routers/web/auth/auth.go
@@ -7,6 +7,7 @@ package auth
 import (
 	"errors"
 	"fmt"
+	"html/template"
 	"net/http"
 	"strings"
 
@@ -37,12 +38,10 @@ import (
 )
 
 const (
-	// tplSignIn template for sign in page
-	tplSignIn base.TplName = "user/auth/signin"
-	// tplSignUp template path for sign up page
-	tplSignUp base.TplName = "user/auth/signup"
-	// TplActivate template path for activate user
-	TplActivate base.TplName = "user/auth/activate"
+	tplSignIn         base.TplName = "user/auth/signin"          // for sign in page
+	tplSignUp         base.TplName = "user/auth/signup"          // for sign up page
+	TplActivate       base.TplName = "user/auth/activate"        // for activate user
+	TplActivatePrompt base.TplName = "user/auth/activate_prompt" // for showing a message for user activation
 )
 
 // autoSignIn reads cookie and try to auto-login.
@@ -613,72 +612,83 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth.
 		}
 	}
 
-	// Send confirmation email
-	if !u.IsActive && u.ID > 1 {
-		if setting.Service.RegisterManualConfirm {
-			ctx.Data["ManualActivationOnly"] = true
-			ctx.HTML(http.StatusOK, TplActivate)
-			return false
-		}
+	// for active user or the first (admin) user, we don't need to send confirmation email
+	if u.IsActive || u.ID == 1 {
+		return true
+	}
 
-		mailer.SendActivateAccountMail(ctx.Locale, u)
+	if setting.Service.RegisterManualConfirm {
+		renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.manual_activation_only"))
+		return false
+	}
 
-		ctx.Data["IsSendRegisterMail"] = true
-		ctx.Data["Email"] = u.Email
-		ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale)
-		ctx.HTML(http.StatusOK, TplActivate)
+	sendActivateEmail(ctx, u)
+	return false
+}
 
-		if err := ctx.Cache.Put("MailResendLimit_"+u.LowerName, u.LowerName, 180); err != nil {
-			log.Error("Set cache(MailResendLimit) fail: %v", err)
-		}
-		return false
+func renderActivationPromptMessage(ctx *context.Context, msg template.HTML) {
+	ctx.Data["ActivationPromptMessage"] = msg
+	ctx.HTML(http.StatusOK, TplActivatePrompt)
+}
+
+func sendActivateEmail(ctx *context.Context, u *user_model.User) {
+	if ctx.Cache.IsExist("MailResendLimit_" + u.LowerName) {
+		renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.resent_limit_prompt"))
+		return
 	}
 
-	return true
+	if err := ctx.Cache.Put("MailResendLimit_"+u.LowerName, u.LowerName, 180); err != nil {
+		log.Error("Set cache(MailResendLimit) fail: %v", err)
+		renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.resent_limit_prompt"))
+		return
+	}
+
+	mailer.SendActivateAccountMail(ctx.Locale, u)
+
+	activeCodeLives := timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale)
+	msgHTML := ctx.Locale.Tr("auth.confirmation_mail_sent_prompt", u.Email, activeCodeLives)
+	renderActivationPromptMessage(ctx, msgHTML)
+}
+
+func renderActivationVerifyPassword(ctx *context.Context, code string) {
+	ctx.Data["ActivationCode"] = code
+	ctx.Data["NeedVerifyLocalPassword"] = true
+	ctx.HTML(http.StatusOK, TplActivate)
 }
 
 // Activate render activate user page
 func Activate(ctx *context.Context) {
 	code := ctx.FormString("code")
 
-	if len(code) == 0 {
-		ctx.Data["IsActivatePage"] = true
-		if ctx.Doer == nil || ctx.Doer.IsActive {
-			ctx.NotFound("invalid user", nil)
+	if code == "" {
+		if ctx.Doer == nil {
+			ctx.Redirect(setting.AppSubURL + "/user/login")
+			return
+		} else if ctx.Doer.IsActive {
+			ctx.Redirect(setting.AppSubURL + "/")
 			return
 		}
-		// Resend confirmation email.
-		if setting.Service.RegisterEmailConfirm {
-			if ctx.Cache.IsExist("MailResendLimit_" + ctx.Doer.LowerName) {
-				ctx.Data["ResendLimited"] = true
-			} else {
-				ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale)
-				mailer.SendActivateAccountMail(ctx.Locale, ctx.Doer)
 
-				if err := ctx.Cache.Put("MailResendLimit_"+ctx.Doer.LowerName, ctx.Doer.LowerName, 180); err != nil {
-					log.Error("Set cache(MailResendLimit) fail: %v", err)
-				}
-			}
-		} else {
-			ctx.Data["ServiceNotEnabled"] = true
+		if setting.MailService == nil || !setting.Service.RegisterEmailConfirm {
+			renderActivationPromptMessage(ctx, ctx.Tr("auth.disable_register_mail"))
+			return
 		}
-		ctx.HTML(http.StatusOK, TplActivate)
+
+		// Resend confirmation email.
+		sendActivateEmail(ctx, ctx.Doer)
 		return
 	}
 
+	// TODO: ctx.Doer/ctx.Data["SignedUser"] could be nil or not the same user as the one being activated
 	user := user_model.VerifyUserActiveCode(ctx, code)
-	// if code is wrong
-	if user == nil {
-		ctx.Data["IsCodeInvalid"] = true
-		ctx.HTML(http.StatusOK, TplActivate)
+	if user == nil { // if code is wrong
+		renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.invalid_code"))
 		return
 	}
 
 	// if account is local account, verify password
 	if user.LoginSource == 0 {
-		ctx.Data["Code"] = code
-		ctx.Data["NeedsPassword"] = true
-		ctx.HTML(http.StatusOK, TplActivate)
+		renderActivationVerifyPassword(ctx, code)
 		return
 	}
 
@@ -688,31 +698,28 @@ func Activate(ctx *context.Context) {
 // ActivatePost handles account activation with password check
 func ActivatePost(ctx *context.Context) {
 	code := ctx.FormString("code")
-	if len(code) == 0 {
+	if code == "" || (ctx.Doer != nil && ctx.Doer.IsActive) {
 		ctx.Redirect(setting.AppSubURL + "/user/activate")
 		return
 	}
 
+	// TODO: ctx.Doer/ctx.Data["SignedUser"] could be nil or not the same user as the one being activated
 	user := user_model.VerifyUserActiveCode(ctx, code)
-	// if code is wrong
-	if user == nil {
-		ctx.Data["IsCodeInvalid"] = true
-		ctx.HTML(http.StatusOK, TplActivate)
+	if user == nil { // if code is wrong
+		renderActivationPromptMessage(ctx, ctx.Locale.Tr("auth.invalid_code"))
 		return
 	}
 
 	// if account is local account, verify password
 	if user.LoginSource == 0 {
 		password := ctx.FormString("password")
-		if len(password) == 0 {
-			ctx.Data["Code"] = code
-			ctx.Data["NeedsPassword"] = true
-			ctx.HTML(http.StatusOK, TplActivate)
+		if password == "" {
+			renderActivationVerifyPassword(ctx, code)
 			return
 		}
 		if !user.ValidatePassword(password) {
-			ctx.Data["IsPasswordInvalid"] = true
-			ctx.HTML(http.StatusOK, TplActivate)
+			ctx.Flash.Error(ctx.Locale.Tr("auth.invalid_password"), true)
+			renderActivationVerifyPassword(ctx, code)
 			return
 		}
 	}