From aaaece28e4c6a8980cef932e224e84933d7c9262 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Thu, 7 Dec 2023 12:57:16 +0100 Subject: [PATCH] Allow changing the email address before activation During registration, one may be required to give their email address, to be verified and activated later. However, if one makes a mistake, a typo, they may end up with an account that cannot be activated due to having a wrong email address. They can still log in, but not change the email address, thus, no way to activate it without help from an administrator. To remedy this issue, lets allow changing the email address for logged in, but not activated users. This fixes gitea#17785. Signed-off-by: Gergely Nagy --- models/user/email_address.go | 77 +++++++++++++++++++++---------- models/user/email_address_test.go | 22 +++++++++ options/locale/locale_en-US.ini | 5 +- routers/web/auth/auth.go | 30 ++++++++++++ templates/user/auth/activate.tmpl | 10 ++++ tests/integration/signup_test.go | 69 +++++++++++++++++++++++++++ 6 files changed, 187 insertions(+), 26 deletions(-) diff --git a/models/user/email_address.go b/models/user/email_address.go index b795a7e94b..986f12e900 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -375,31 +375,7 @@ 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 { - return err - } else if !has { - return ErrEmailAddressNotExist{Email: email.Email} - } - - if !email.IsActivated { - return ErrEmailNotActivated - } - - user := &User{} - has, err = db.GetEngine(ctx).ID(email.UID).Get(user) - if err != nil { - return err - } else if !has { - return ErrUserNotExist{ - UID: email.UID, - Name: "", - KeyID: 0, - } - } - +func makeEmailPrimary(ctx context.Context, user *User, email *EmailAddress) error { ctx, committer, err := db.TxContext(ctx) if err != nil { return err @@ -429,6 +405,57 @@ func MakeEmailPrimary(ctx context.Context, email *EmailAddress) error { return committer.Commit() } +// ReplaceInactivePrimaryEmail replaces the primary email of a given user, even if the primary is not yet activated. +func ReplaceInactivePrimaryEmail(ctx context.Context, oldEmail string, email *EmailAddress) error { + user := &User{} + has, err := db.GetEngine(ctx).ID(email.UID).Get(user) + if err != nil { + return err + } else if !has { + return ErrUserNotExist{ + UID: email.UID, + Name: "", + KeyID: 0, + } + } + + err = AddEmailAddress(ctx, email) + if err != nil { + return err + } + + err = makeEmailPrimary(ctx, user, email) + if err != nil { + return err + } + + return DeleteEmailAddress(ctx, &EmailAddress{UID: email.UID, Email: oldEmail}) +} + +// 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 { + return err + } else if !has { + return ErrEmailAddressNotExist{Email: email.Email} + } + + if !email.IsActivated { + return ErrEmailNotActivated + } + + user := &User{} + has, err = db.GetEngine(ctx).ID(email.UID).Get(user) + if err != nil { + return err + } else if !has { + return ErrUserNotExist{UID: email.UID} + } + + return makeEmailPrimary(ctx, user, email) +} + // 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 b20797d700..c2c0378061 100644 --- a/models/user/email_address_test.go +++ b/models/user/email_address_test.go @@ -167,6 +167,28 @@ func TestMakeEmailPrimary(t *testing.T) { assert.Equal(t, "user101@example.com", user.Email) } +func TestReplaceInactivePrimaryEmail(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + email := &user_model.EmailAddress{ + Email: "user9999999@example.com", + UID: 9999999, + } + err := user_model.ReplaceInactivePrimaryEmail(db.DefaultContext, "user10@example.com", email) + assert.Error(t, err) + assert.True(t, user_model.IsErrUserNotExist(err)) + + email = &user_model.EmailAddress{ + Email: "user201@example.com", + UID: 10, + } + err = user_model.ReplaceInactivePrimaryEmail(db.DefaultContext, "user10@example.com", email) + assert.NoError(t, err) + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10}) + assert.Equal(t, "user201@example.com", user.Email) +} + func TestActivate(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b5fc124d19..aebc600579 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -367,7 +367,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 = A new confirmation email has been sent to %s. Please check your inbox within the next %s to complete the registration process. If the email is incorrect, you can log in, and request another confirmation email to be sent to a different address. 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. @@ -377,6 +377,9 @@ 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_email_summary = Change the email address activation mail is sent to. +change_unconfirmed_email = If you have given the wrong email address during registration, you can change it below, and a confirmation will be sent to the new address instead. +change_unconfirmed_email_error = Unable to change the email address: %v 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 49197d418c..78a6fdd776 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -690,6 +690,36 @@ func Activate(ctx *context.Context) { func ActivatePost(ctx *context.Context) { code := ctx.FormString("code") if len(code) == 0 { + email := ctx.FormString("email") + if len(email) > 0 { + ctx.Data["IsActivatePage"] = true + if ctx.Doer == nil || ctx.Doer.IsActive { + ctx.NotFound("invalid user", nil) + return + } + // Change the primary email + if setting.Service.RegisterEmailConfirm { + if setting.CacheService.Enabled && ctx.Cache.IsExist("MailResendLimit_"+ctx.Doer.LowerName) { + ctx.Data["ResendLimited"] = true + } else { + ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale) + err := user_model.ReplaceInactivePrimaryEmail(ctx, ctx.Doer.Email, &user_model.EmailAddress{ + UID: ctx.Doer.ID, + Email: email, + }) + if err != nil { + ctx.Data["IsActivatePage"] = false + log.Error("Couldn't replace inactive primary email of user %d: %v", ctx.Doer.ID, err) + ctx.RenderWithErr(ctx.Tr("auth.change_unconfirmed_email_error", err), TplActivate, nil) + return + } + // Confirmation mail will be re-sent after the redirect to `/user/activate` below. + } + } else { + ctx.Data["ServiceNotEnabled"] = true + } + } + ctx.Redirect(setting.AppSubURL + "/user/activate") return } diff --git a/templates/user/auth/activate.tmpl b/templates/user/auth/activate.tmpl index 1b06719753..d8a737e996 100644 --- a/templates/user/auth/activate.tmpl +++ b/templates/user/auth/activate.tmpl @@ -39,6 +39,16 @@ {{else}}

{{ctx.Locale.Tr "auth.has_unconfirmed_mail" (.SignedUser.Name|Escape) (.SignedUser.Email|Escape) | Str2html}}

+
+ {{ctx.Locale.Tr "auth.change_unconfirmed_email_summary"}} + +

{{ctx.Locale.Tr "auth.change_unconfirmed_email"}}

+
+ + +
+
+
diff --git a/tests/integration/signup_test.go b/tests/integration/signup_test.go index f983f98ad8..eea4772b21 100644 --- a/tests/integration/signup_test.go +++ b/tests/integration/signup_test.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/tests" @@ -91,3 +92,71 @@ func TestSignupEmail(t *testing.T) { } } } + +func TestSignupEmailChangeForInactiveUser(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // Disable the captcha & enable email confirmation for registrations + defer test.MockVariableValue(&setting.Service.EnableCaptcha, false)() + defer test.MockVariableValue(&setting.Service.RegisterEmailConfirm, true)() + defer test.MockVariableValue(&setting.CacheService.Enabled, false)() + + // Create user + req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{ + "user_name": "exampleUserX", + "email": "wrong-email@example.com", + "password": "examplePassword!1", + "retype": "examplePassword!1", + }) + MakeRequest(t, req, http.StatusOK) + + session := loginUserWithPassword(t, "exampleUserX", "examplePassword!1") + + // Verify that the initial e-mail is the wrong one. + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "exampleUserX"}) + assert.Equal(t, "wrong-email@example.com", user.Email) + + // Change the email address + req = NewRequestWithValues(t, "POST", "/user/activate", map[string]string{ + "email": "fine-email@example.com", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + // Verify that the email was updated + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "exampleUserX"}) + assert.Equal(t, "fine-email@example.com", user.Email) +} + +func TestSignupEmailChangeForActiveUser(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // Disable the captcha & enable email confirmation for registrations + defer test.MockVariableValue(&setting.Service.EnableCaptcha, false)() + defer test.MockVariableValue(&setting.Service.RegisterEmailConfirm, false)() + defer test.MockVariableValue(&setting.CacheService.Enabled, false)() + + // Create user + req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{ + "user_name": "exampleUserY", + "email": "wrong-email-2@example.com", + "password": "examplePassword!1", + "retype": "examplePassword!1", + }) + MakeRequest(t, req, http.StatusSeeOther) + + session := loginUserWithPassword(t, "exampleUserY", "examplePassword!1") + + // Verify that the initial e-mail is the wrong one. + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "exampleUserY"}) + assert.Equal(t, "wrong-email-2@example.com", user.Email) + + // Changing the email for a validated address is not available + req = NewRequestWithValues(t, "POST", "/user/activate", map[string]string{ + "email": "fine-email-2@example.com", + }) + session.MakeRequest(t, req, http.StatusNotFound) + + // Verify that the email remained unchanged + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "exampleUserY"}) + assert.Equal(t, "wrong-email-2@example.com", user.Email) +}