From 7669ff86ed8d24fc5771eea6028ca6b8f0fb9cce Mon Sep 17 00:00:00 2001 From: Meano Date: Thu, 8 Jul 2021 15:19:21 +0800 Subject: [PATCH] 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. Signed-off-by: Meano --- models/user_mail.go | 27 +++++++++-------- routers/web/admin/emails.go | 4 +-- routers/web/user/auth.go | 4 +++ routers/web/user/setting/account.go | 43 ++++++++++++++-------------- templates/user/settings/account.tmpl | 2 +- 5 files changed, 42 insertions(+), 38 deletions(-) diff --git a/models/user_mail.go b/models/user_mail.go index 89cbdf12a488..e8cb3ae6c3c6 100644 --- a/models/user_mail.go +++ b/models/user_mail.go @@ -366,8 +366,8 @@ func SearchEmails(opts *SearchEmailOptions) ([]*SearchEmailResult, int64, error) } // ActivateUserEmail will change the activated state of an email address, -// either primary (in the user table) or secondary (in the email_address table) -func ActivateUserEmail(userID int64, email string, primary, activate bool) (err error) { +// either primary or secondary (all in the email_address table) +func ActivateUserEmail(userID int64, email string, activate bool) (err error) { sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { @@ -397,24 +397,23 @@ func ActivateUserEmail(userID int64, email string, primary, activate bool) (err return fmt.Errorf("updateActivation(): %v", err) } - if primary { - // Activate/deactivate a user's primary email address + // Activate/deactivate a user's primary email address and account + if addr.IsPrimary { user := User{ID: userID, Email: email} if has, err := sess.Get(&user); err != nil { return err } else if !has { return fmt.Errorf("no such user: %d (%s)", userID, email) } - if user.IsActive == activate { - // Already in the desired state; no action - return nil - } - user.IsActive = activate - if user.Rands, err = GetUserSalt(); err != nil { - return fmt.Errorf("generate salt: %v", err) - } - if err = updateUserCols(sess, &user, "is_active", "rands"); err != nil { - return fmt.Errorf("updateUserCols(): %v", err) + // The user's activation state should synchronized with the primary email + if user.IsActive != activate { + user.IsActive = activate + if user.Rands, err = GetUserSalt(); err != nil { + return fmt.Errorf("generate salt: %v", err) + } + if err = updateUserCols(sess, &user, "is_active", "rands"); err != nil { + return fmt.Errorf("updateUserCols(): %v", err) + } } } diff --git a/routers/web/admin/emails.go b/routers/web/admin/emails.go index f7e8c97fb6b9..704cb88c640d 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 9458bf5c95a9..636d8492ac81 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -1429,6 +1429,10 @@ func handleAccountActivation(ctx *context.Context, user *models.User) { return } + if err := models.ActivateUserEmail(user.ID, user.Email, true); err != nil { + log.Error(fmt.Sprintf("Error active user mail: %v", err)) + } + log.Trace("User activated: %s", user.Name) if err := ctx.Session.Set("uid", user.ID); err != nil { diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index 48ab37d9369e..e0159f6e63f8 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 { + + 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 + } + if email.IsPrimary { + if ctx.User.IsActive && !setting.Service.RegisterEmailConfirm { log.Error("Send activation: email not set for activation") 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) diff --git a/templates/user/settings/account.tmpl b/templates/user/settings/account.tmpl index 1a74c64d74ba..2e1976aaa957 100644 --- a/templates/user/settings/account.tmpl +++ b/templates/user/settings/account.tmpl @@ -92,7 +92,7 @@
{{$.CsrfTokenHtml}} - + {{if $.ActivationsPending}} {{else}}