Merge pull request '[GITEA] Allow changing the email address before activation' (#1891) from algernon/forgejo:feature/unactivated-account-email-change into forgejo-dependency

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/1891
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

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, user *User, email *EmailAddress) error {
-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,
-		}
-	}
-
 	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

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())
 

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 <b>%s</b>. 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 <b>%s</b>. 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 <b>%s</b>. 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 (<b>%s</b>). 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

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
 	}

templates/user/auth/activate.tmpl

@@ -39,6 +39,16 @@
 						{{else}}
 							<p>{{ctx.Locale.Tr "auth.has_unconfirmed_mail" (.SignedUser.Name|Escape) (.SignedUser.Email|Escape) | Str2html}}</p>
 							<div class="divider"></div>
+							<details class="inline field">
+								<summary>{{ctx.Locale.Tr "auth.change_unconfirmed_email_summary"}}</summary>
+
+								<p>{{ctx.Locale.Tr "auth.change_unconfirmed_email"}}</p>
+								<div class="inline field">
+									<label for="email">{{ctx.Locale.Tr "email"}}</label>
+									<input id="email" name="email" type="email" autocomplete="on">
+								</div>
+							</details>
+
 							<div class="text right">
 								<button class="ui primary button">{{ctx.Locale.Tr "auth.resend_mail"}}</button>
 							</div>

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)
+}