diff --git a/models/user_mail.go b/models/user_mail.go index 89cbdf12a..f8b084a00 100644 --- a/models/user_mail.go +++ b/models/user_mail.go @@ -74,14 +74,15 @@ func GetEmailAddressByID(uid, id int64) (*EmailAddress, error) { return email, nil } -func isEmailActive(e Engine, email string, userID, emailID int64) (bool, error) { +// isEmailActive check if email is activated with a different emailID +func isEmailActive(e Engine, email string, excludeEmailID int64) (bool, error) { if len(email) == 0 { return true, nil } // Can't filter by boolean field unless it's explicit cond := builder.NewCond() - cond = cond.And(builder.Eq{"lower_email": strings.ToLower(email)}, builder.Neq{"id": emailID}) + cond = cond.And(builder.Eq{"lower_email": strings.ToLower(email)}, builder.Neq{"id": excludeEmailID}) if setting.Service.RegisterEmailConfirm { // Inactive (unvalidated) addresses don't count as active if email validation is required cond = cond.And(builder.Eq{"is_activated": true}) @@ -90,7 +91,7 @@ func isEmailActive(e Engine, email string, userID, emailID int64) (bool, error) var em EmailAddress if has, err := e.Where(cond).Get(&em); has || err != nil { if has { - log.Info("isEmailActive('%s',%d,%d) found duplicate in email ID %d", email, userID, emailID, em.ID) + log.Info("isEmailActive(%q, %d) found duplicate in email ID %d", email, excludeEmailID, em.ID) } return has, err } @@ -366,8 +367,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 { @@ -387,34 +388,33 @@ func ActivateUserEmail(userID int64, email string, primary, activate bool) (err return nil } if activate { - if used, err := isEmailActive(sess, email, 0, addr.ID); err != nil { - return fmt.Errorf("isEmailActive(): %v", err) + if used, err := isEmailActive(sess, email, addr.ID); err != nil { + return fmt.Errorf("unable to check isEmailActive() for %s: %v", email, err) } else if used { return ErrEmailAlreadyUsed{Email: email} } } if err = addr.updateActivation(sess, activate); err != nil { - return fmt.Errorf("updateActivation(): %v", err) + return fmt.Errorf("unable to updateActivation() for %d:%s: %w", addr.ID, addr.Email, 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) + return fmt.Errorf("no user with ID: %d and Email: %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 be synchronized with the primary email + if user.IsActive != activate { + user.IsActive = activate + if user.Rands, err = GetUserSalt(); err != nil { + return fmt.Errorf("unable to generate salt: %v", err) + } + if err = updateUserCols(sess, &user, "is_active", "rands"); err != nil { + return fmt.Errorf("unable to updateUserCols() for user ID: %d: %v", userID, err) + } } } diff --git a/routers/web/admin/emails.go b/routers/web/admin/emails.go index f7e8c97fb..704cb88c6 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 9458bf5c9..4095d2956 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 48ab37d93..b805db620 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") - ctx.Redirect(setting.AppSubURL + "/user/settings/account") - return - } - 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 + + 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) + } else { + mailer.SendActivateEmailMail(ctx.User, 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 1a74c64d7..2e1976aaa 100644 --- a/templates/user/settings/account.tmpl +++ b/templates/user/settings/account.tmpl @@ -92,7 +92,7 @@
{{$.CsrfTokenHtml}} - + {{if $.ActivationsPending}} {{else}}