diff --git a/models/user/email_address.go b/models/user/email_address.go index 216840916d057..9ddd1838d5808 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -21,9 +21,6 @@ import ( "xorm.io/builder" ) -// ErrEmailNotActivated e-mail address has not been activated error -var ErrEmailNotActivated = util.NewInvalidArgumentErrorf("e-mail address has not been activated") - // ErrEmailCharIsNotSupported e-mail address contains unsupported character type ErrEmailCharIsNotSupported struct { Email string @@ -313,27 +310,27 @@ func updateActivation(ctx context.Context, email *EmailAddress, activate bool) e return UpdateUserCols(ctx, user, "rands") } -// MakeEmailPrimary sets primary email address of given user. -func MakeEmailPrimary(ctx context.Context, email *EmailAddress) error { - has, err := db.GetEngine(ctx).Get(email) - if err != nil { +func MakeActiveEmailPrimary(ctx context.Context, emailID int64) error { + return makeEmailPrimaryInternal(ctx, emailID, true) +} + +func MakeInactiveEmailPrimary(ctx context.Context, emailID int64) error { + return makeEmailPrimaryInternal(ctx, emailID, false) +} + +func makeEmailPrimaryInternal(ctx context.Context, emailID int64, isActive bool) error { + email := &EmailAddress{} + if has, err := db.GetEngine(ctx).ID(emailID).Where(builder.Eq{"is_activated": isActive}).Get(email); err != nil { return err } else if !has { - return ErrEmailAddressNotExist{Email: email.Email} - } - - if !email.IsActivated { - return ErrEmailNotActivated + return ErrEmailAddressNotExist{} } user := &User{} - has, err = db.GetEngine(ctx).ID(email.UID).Get(user) - if err != nil { + if has, err := db.GetEngine(ctx).ID(email.UID).Get(user); err != nil { return err } else if !has { - return ErrUserNotExist{ - UID: email.UID, - } + return ErrUserNotExist{UID: email.UID} } ctx, committer, err := db.TxContext(ctx) @@ -365,6 +362,21 @@ func MakeEmailPrimary(ctx context.Context, email *EmailAddress) error { return committer.Commit() } +// ChangeInactivePrimaryEmail replaces the inactive primary email of a given user +func ChangeInactivePrimaryEmail(ctx context.Context, uid int64, oldEmailAddr, newEmailAddr string) error { + return db.WithTx(ctx, func(ctx context.Context) error { + _, err := db.GetEngine(ctx).Where(builder.Eq{"uid": uid, "lower_email": strings.ToLower(oldEmailAddr)}).Delete(&EmailAddress{}) + if err != nil { + return err + } + newEmail, err := InsertEmailAddress(ctx, &EmailAddress{UID: uid, Email: newEmailAddr}) + if err != nil { + return err + } + return MakeInactiveEmailPrimary(ctx, newEmail.ID) + }) +} + // VerifyActiveEmailCode verifies active email code when active account func VerifyActiveEmailCode(ctx context.Context, code, email string) *EmailAddress { minutes := setting.Service.ActiveCodeLives diff --git a/models/user/email_address_test.go b/models/user/email_address_test.go index 140443f82f9e0..dc3073f98bd65 100644 --- a/models/user/email_address_test.go +++ b/models/user/email_address_test.go @@ -45,31 +45,22 @@ func TestIsEmailUsed(t *testing.T) { func TestMakeEmailPrimary(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - email := &user_model.EmailAddress{ - Email: "user567890@example.com", - } - err := user_model.MakeEmailPrimary(db.DefaultContext, email) + err := user_model.MakeActiveEmailPrimary(db.DefaultContext, 9999999) assert.Error(t, err) - assert.EqualError(t, err, user_model.ErrEmailAddressNotExist{Email: email.Email}.Error()) + assert.ErrorIs(t, err, user_model.ErrEmailAddressNotExist{}) - email = &user_model.EmailAddress{ - Email: "user11@example.com", - } - err = user_model.MakeEmailPrimary(db.DefaultContext, email) + email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{Email: "user11@example.com"}) + err = user_model.MakeActiveEmailPrimary(db.DefaultContext, email.ID) assert.Error(t, err) - assert.EqualError(t, err, user_model.ErrEmailNotActivated.Error()) + assert.ErrorIs(t, err, user_model.ErrEmailAddressNotExist{}) // inactive email is considered as not exist for "MakeActiveEmailPrimary" - email = &user_model.EmailAddress{ - Email: "user9999999@example.com", - } - err = user_model.MakeEmailPrimary(db.DefaultContext, email) + email = unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{Email: "user9999999@example.com"}) + err = user_model.MakeActiveEmailPrimary(db.DefaultContext, email.ID) assert.Error(t, err) assert.True(t, user_model.IsErrUserNotExist(err)) - email = &user_model.EmailAddress{ - Email: "user101@example.com", - } - err = user_model.MakeEmailPrimary(db.DefaultContext, email) + email = unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{Email: "user101@example.com"}) + err = user_model.MakeActiveEmailPrimary(db.DefaultContext, email.ID) assert.NoError(t, err) user, _ := user_model.GetUserByID(db.DefaultContext, int64(10)) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index a0ad09f776440..6d4e109e1dc66 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -368,7 +368,7 @@ forgot_password_title= Forgot Password forgot_password = Forgot password? sign_up_now = Need an account? Register now. sign_up_successful = Account was successfully created. Welcome! -confirmation_mail_sent_prompt = A new confirmation email has been sent to %s. Please check your inbox within the next %s to complete the registration process. +confirmation_mail_sent_prompt_ex = A new confirmation email has been sent to %s. Please check your inbox within the next %s to complete the registration process. If your registration email address is incorrect, you can sign in again and change it. must_change_password = Update your password allow_password_change = Require user to change password (recommended) reset_password_mail_sent_prompt = A confirmation email has been sent to %s. Please check your inbox within the next %s to complete the account recovery process. @@ -378,6 +378,7 @@ prohibit_login = Sign In Prohibited prohibit_login_desc = Your account is prohibited from signing in, please contact your site administrator. resent_limit_prompt = You have already requested an activation email recently. Please wait 3 minutes and try again. has_unconfirmed_mail = Hi %s, you have an unconfirmed email address (%s). If you haven't received a confirmation email or need to resend a new one, please click on the button below. +change_unconfirmed_mail_address = If your registration email address is incorrect, you can change it here and resend a new confirmation email. resend_mail = Click here to resend your activation email email_not_associate = The email address is not associated with any account. send_reset_mail = Send Account Recovery Email diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index fee6a89a99962..7704a110a67b5 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -646,7 +646,7 @@ func sendActivateEmail(ctx *context.Context, u *user_model.User) { 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) + msgHTML := ctx.Locale.Tr("auth.confirmation_mail_sent_prompt_ex", u.Email, activeCodeLives) renderActivationPromptMessage(ctx, msgHTML) } @@ -656,6 +656,10 @@ func renderActivationVerifyPassword(ctx *context.Context, code string) { ctx.HTML(http.StatusOK, TplActivate) } +func renderActivationChangeEmail(ctx *context.Context) { + ctx.HTML(http.StatusOK, TplActivate) +} + // Activate render activate user page func Activate(ctx *context.Context) { code := ctx.FormString("code") @@ -674,7 +678,7 @@ func Activate(ctx *context.Context) { return } - // Resend confirmation email. + // Resend confirmation email. FIXME: ideally this should be in a POST request sendActivateEmail(ctx, ctx.Doer) return } @@ -698,7 +702,28 @@ func Activate(ctx *context.Context) { // ActivatePost handles account activation with password check func ActivatePost(ctx *context.Context) { code := ctx.FormString("code") - if code == "" || (ctx.Doer != nil && ctx.Doer.IsActive) { + if ctx.Doer != nil && ctx.Doer.IsActive { + ctx.Redirect(setting.AppSubURL + "/user/activate") // it will redirect again to the correct page + return + } + + if code == "" { + newEmail := strings.TrimSpace(ctx.FormString("change_email")) + if ctx.Doer != nil && newEmail != "" && !strings.EqualFold(ctx.Doer.Email, newEmail) { + if user_model.ValidateEmail(newEmail) != nil { + ctx.Flash.Error(ctx.Locale.Tr("form.email_invalid"), true) + renderActivationChangeEmail(ctx) + return + } + err := user_model.ChangeInactivePrimaryEmail(ctx, ctx.Doer.ID, ctx.Doer.Email, newEmail) + if err != nil { + ctx.Flash.Error(ctx.Locale.Tr("admin.emails.not_updated", newEmail), true) + renderActivationChangeEmail(ctx) + return + } + ctx.Doer.Email = newEmail + } + // FIXME: at the moment, GET request handles the "send confirmation email" action. But the old code does this redirect and then send a confirmation email. ctx.Redirect(setting.AppSubURL + "/user/activate") return } diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index 23d3ca31618be..abb5873e98ada 100644 --- a/routers/web/user/setting/account.go +++ b/routers/web/user/setting/account.go @@ -92,9 +92,9 @@ func EmailPost(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAccount"] = true - // Make emailaddress primary. + // Make email address primary. if ctx.FormString("_method") == "PRIMARY" { - if err := user_model.MakeEmailPrimary(ctx, &user_model.EmailAddress{ID: ctx.FormInt64("id")}); err != nil { + if err := user_model.MakeActiveEmailPrimary(ctx, ctx.FormInt64("id")); err != nil { ctx.ServerError("MakeEmailPrimary", err) return } diff --git a/templates/user/auth/activate.tmpl b/templates/user/auth/activate.tmpl index 51dc1eb6a6bfa..e32a5d8707356 100644 --- a/templates/user/auth/activate.tmpl +++ b/templates/user/auth/activate.tmpl @@ -21,6 +21,13 @@ {{else}}

{{ctx.Locale.Tr "auth.has_unconfirmed_mail" .SignedUser.Name .SignedUser.Email}}

+
+ {{ctx.Locale.Tr "auth.change_unconfirmed_mail_address"}} +
+ + +
+
diff --git a/tests/integration/signup_test.go b/tests/integration/signup_test.go index fbf586f6964f5..e9a05201eefbd 100644 --- a/tests/integration/signup_test.go +++ b/tests/integration/signup_test.go @@ -107,13 +107,25 @@ func TestSignupEmailActive(t *testing.T) { resp := MakeRequest(t, req, http.StatusOK) assert.Contains(t, resp.Body.String(), `A new confirmation email has been sent to email-1@example.com.`) - // access "user/active" means trying to re-send the activation email + // access "user/activate" means trying to re-send the activation email session := loginUserWithPassword(t, "test-user-1", "password1") resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/activate"), http.StatusOK) assert.Contains(t, resp.Body.String(), "You have already requested an activation email recently") - // access "user/active" with a valid activation code, then get the "verify password" page + // access anywhere else will see a "Activate Your Account" prompt, and there is a chance to change email + resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/issues"), http.StatusOK) + assert.Contains(t, resp.Body.String(), `