]> source.dussan.org Git - gitea.git/commitdiff
Allow to change primary email before account activation (#29412)
authorwxiaoguang <wxiaoguang@gmail.com>
Tue, 27 Feb 2024 10:55:13 +0000 (18:55 +0800)
committerGitHub <noreply@github.com>
Tue, 27 Feb 2024 10:55:13 +0000 (10:55 +0000)
models/user/email_address.go
models/user/email_address_test.go
options/locale/locale_en-US.ini
routers/web/auth/auth.go
routers/web/user/setting/account.go
templates/user/auth/activate.tmpl
tests/integration/signup_test.go

index 216840916d05707033ba0574fd20bbfc6ca9cd8d..9ddd1838d5808eaef2cab3cbcfb77dc6e48ff45c 100644 (file)
@@ -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
index 140443f82f9e05219703b631314fc3f554bae4d4..dc3073f98bd652132b663c6f2668b2b3d9a4ebfd 100644 (file)
@@ -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))
index a0ad09f77644061dbf4e22521481c9afce4db9bb..6d4e109e1dc66a0243e0a66dc1a0f456790dd85e 100644 (file)
@@ -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 <b>%s</b>. 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 <b>%s</b>. 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 <b>%s</b>. 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 (<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_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
index fee6a89a99962b769d2f9cd9186ab7e2f94f63d3..7704a110a67b5f8cb846ede79f2ee51dfb7b3453 100644 (file)
@@ -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
        }
index 23d3ca31618be3d2a6e1b16d05be78af3ba46f7a..abb5873e98ada23e515c01eea1fd051721649211 100644 (file)
@@ -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
                }
index 51dc1eb6a6bfabe7c9c2b707bf097b172ff438ea..e32a5d8707356d83149f8072438e01dca9c61baf 100644 (file)
                                                <input name="code" type="hidden" value="{{.ActivationCode}}">
                                        {{else}}
                                                <p>{{ctx.Locale.Tr "auth.has_unconfirmed_mail" .SignedUser.Name .SignedUser.Email}}</p>
+                                               <details>
+                                                       <summary>{{ctx.Locale.Tr "auth.change_unconfirmed_mail_address"}}</summary>
+                                                       <div class="tw-py-2">
+                                                               <label for="change-email">{{ctx.Locale.Tr "email"}}</label>
+                                                               <input id="change-email" name="change_email" type="email" value="{{.SignedUser.Email}}">
+                                                       </div>
+                                               </details>
                                                <div class="divider"></div>
                                                <div class="text right">
                                                        <button class="ui primary button">{{ctx.Locale.Tr "auth.resend_mail"}}</button>
index fbf586f6964f5f0ddc754cb51cc1958f3b2c3c98..e9a05201eefbd2d1483d28afdbc6a5a01819956b 100644 (file)
@@ -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 <b>email-1@example.com</b>.`)
 
-       // 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(), `<input id="change-email" name="change_email" `)
+
+       // post to "user/activate" with a new email
+       session.MakeRequest(t, NewRequestWithValues(t, "POST", "/user/activate", map[string]string{"change_email": "email-changed@example.com"}), http.StatusSeeOther)
        user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"})
+       assert.Equal(t, "email-changed@example.com", user.Email)
+       email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{Email: "email-changed@example.com"})
+       assert.False(t, email.IsActivated)
+       assert.True(t, email.IsPrimary)
+
+       // access "user/activate" with a valid activation code, then get the "verify password" page
+       user = unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "test-user-1"})
        activationCode := user.GenerateEmailActivateCode(user.Email)
        resp = session.MakeRequest(t, NewRequest(t, "GET", "/user/activate?code="+activationCode), http.StatusOK)
        assert.Contains(t, resp.Body.String(), `<input id="verify-password"`)