Fix activation of primary email addresses (#16385)

* fix: primary email cannot be activated

* Primary email should be activated together with user account when
'RegisterEmailConfirm' is enabled.

* To fix the existing error state. When 'RegisterEmailConfirm' is enabled, the
admin should have permission to modify the activations status of user email.
And the user should be allowed to send activation to primary email.

* Only judge whether email is primary from email_address table.

* Improve logging and refactor isEmailActive

Co-authored-by: zeripath <art27@cantab.net>

3 files changed, 34 insertions, 27 deletions

diff --git a/routers/web/admin/emails.go b/routers/web/admin/emails.go
index f7e8c97fb6..704cb88c64 100644
--- a/routers/web/admin/emails.go
+++ b/routers/web/admin/emails.go
@@ -125,8 +125,8 @@ func ActivateEmail(ctx *context.Context) {
 
 	log.Info("Changing activation for User ID: %d, email: %s, primary: %v to %v", uid, email, primary, activate)
 
-	if err := models.ActivateUserEmail(uid, email, primary, activate); err != nil {
-		log.Error("ActivateUserEmail(%v,%v,%v,%v): %v", uid, email, primary, activate, err)
+	if err := models.ActivateUserEmail(uid, email, activate); err != nil {
+		log.Error("ActivateUserEmail(%v,%v,%v): %v", uid, email, activate, err)
 		if models.IsErrEmailAlreadyUsed(err) {
 			ctx.Flash.Error(ctx.Tr("admin.emails.duplicate_active"))
 		} else {
diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go
index 9458bf5c95..4095d2956e 100644
--- a/routers/web/user/auth.go
+++ b/routers/web/user/auth.go
@@ -1429,16 +1429,22 @@ func handleAccountActivation(ctx *context.Context, user *models.User) {
 		return
 	}
 
+	if err := models.ActivateUserEmail(user.ID, user.Email, true); err != nil {
+		log.Error("Unable to activate email for user: %-v with email: %s: %v", user, user.Email, err)
+		ctx.ServerError("ActivateUserEmail", err)
+		return
+	}
+
 	log.Trace("User activated: %s", user.Name)
 
 	if err := ctx.Session.Set("uid", user.ID); err != nil {
-		log.Error(fmt.Sprintf("Error setting uid in session: %v", err))
+		log.Error("Error setting uid in session[%s]: %v", ctx.Session.ID(), err)
 	}
 	if err := ctx.Session.Set("uname", user.Name); err != nil {
-		log.Error(fmt.Sprintf("Error setting uname in session: %v", err))
+		log.Error("Error setting uname in session[%s]: %v", ctx.Session.ID(), err)
 	}
 	if err := ctx.Session.Release(); err != nil {
-		log.Error("Error storing session: %v", err)
+		log.Error("Error storing session[%s]: %v", ctx.Session.ID(), err)
 	}
 
 	ctx.Flash.Success(ctx.Tr("auth.account_activated"))
diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go
index 48ab37d936..b805db6200 100644
--- a/routers/web/user/setting/account.go
+++ b/routers/web/user/setting/account.go
@@ -107,35 +107,36 @@ func EmailPost(ctx *context.Context) {
 			ctx.Redirect(setting.AppSubURL + "/user/settings/account")
 			return
 		}
-		if ctx.Query("id") == "PRIMARY" {
-			if ctx.User.IsActive {
-				log.Error("Send activation: email not set for activation")
+
+		id := ctx.QueryInt64("id")
+		email, err := models.GetEmailAddressByID(ctx.User.ID, id)
+		if err != nil {
+			log.Error("GetEmailAddressByID(%d,%d) error: %v", ctx.User.ID, id, err)
+			ctx.Redirect(setting.AppSubURL + "/user/settings/account")
+			return
+		}
+		if email == nil {
+			log.Warn("Send activation failed: EmailAddress[%d] not found for user: %-v", id, ctx.User)
+			ctx.Redirect(setting.AppSubURL + "/user/settings/account")
+			return
+		}
+		if email.IsActivated {
+			log.Debug("Send activation failed: email %s is already activated for user: %-v", email.Email, ctx.User)
+			ctx.Redirect(setting.AppSubURL + "/user/settings/account")
+			return
+		}
+		if email.IsPrimary {
+			if ctx.User.IsActive && !setting.Service.RegisterEmailConfirm {
+				log.Debug("Send activation failed: email %s is already activated for user: %-v", email.Email, ctx.User)
 				ctx.Redirect(setting.AppSubURL + "/user/settings/account")
 				return
 			}
+			// Only fired when the primary email is inactive (Wrong state)
 			mailer.SendActivateAccountMail(ctx.Locale, ctx.User)
-			address = ctx.User.Email
 		} else {
-			id := ctx.QueryInt64("id")
-			email, err := models.GetEmailAddressByID(ctx.User.ID, id)
-			if err != nil {
-				log.Error("GetEmailAddressByID(%d,%d) error: %v", ctx.User.ID, id, err)
-				ctx.Redirect(setting.AppSubURL + "/user/settings/account")
-				return
-			}
-			if email == nil {
-				log.Error("Send activation: EmailAddress not found; user:%d, id: %d", ctx.User.ID, id)
-				ctx.Redirect(setting.AppSubURL + "/user/settings/account")
-				return
-			}
-			if email.IsActivated {
-				log.Error("Send activation: email not set for activation")
-				ctx.Redirect(setting.AppSubURL + "/user/settings/account")
-				return
-			}
 			mailer.SendActivateEmailMail(ctx.User, email)
-			address = email.Email
 		}
+		address = email.Email
 
 		if err := ctx.Cache.Put("MailResendLimit_"+ctx.User.LowerName, ctx.User.LowerName, 180); err != nil {
 			log.Error("Set cache(MailResendLimit) fail: %v", err)