diff --git a/models/user/email_address.go b/models/user/email_address.go index ab1702ec0c695..7cb574de54f77 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -332,21 +332,40 @@ 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) +func makeEmailPrimary(ctx context.Context, user *User, email *EmailAddress) error { + ctx, committer, err := db.TxContext(ctx) if err != nil { return err - } else if !has { - return ErrEmailAddressNotExist{Email: email.Email} } + defer committer.Close() + sess := db.GetEngine(ctx) - if !email.IsActivated { - return ErrEmailNotActivated + // 1. Update user table + user.Email = email.Email + if _, err = sess.ID(user.ID).Cols("email").Update(user); err != nil { + return err } + // 2. Update old primary email + if _, err = sess.Where("uid=? AND is_primary=?", email.UID, true).Cols("is_primary").Update(&EmailAddress{ + IsPrimary: false, + }); err != nil { + return err + } + + // 3. update new primary email + email.IsPrimary = true + if _, err = sess.ID(email.ID).Cols("is_primary").Update(email); err != nil { + return err + } + + 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) + has, err := db.GetEngine(ctx).ID(email.UID).Get(user) if err != nil { return err } else if !has { @@ -357,33 +376,41 @@ func MakeEmailPrimary(ctx context.Context, email *EmailAddress) error { } } - ctx, committer, err := db.TxContext(ctx) + err = AddEmailAddress(ctx, email) if err != nil { return err } - defer committer.Close() - sess := db.GetEngine(ctx) - // 1. Update user table - user.Email = email.Email - if _, err = sess.ID(user.ID).Cols("email").Update(user); err != nil { + err = makeEmailPrimary(ctx, user, email) + if err != nil { return err } - // 2. Update old primary email - if _, err = sess.Where("uid=? AND is_primary=?", email.UID, true).Cols("is_primary").Update(&EmailAddress{ - IsPrimary: false, - }); err != nil { + 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} } - // 3. update new primary email - email.IsPrimary = true - if _, err = sess.ID(email.ID).Cols("is_primary").Update(email); err != nil { + 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 committer.Commit() + return makeEmailPrimary(ctx, user, email) } // VerifyActiveEmailCode verifies active email code when active account diff --git a/models/user/email_address_test.go b/models/user/email_address_test.go index be1ccea5449a2..0105aec0aa246 100644 --- a/models/user/email_address_test.go +++ b/models/user/email_address_test.go @@ -77,6 +77,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/routers/web/auth/auth.go b/routers/web/auth/auth.go index fdd92d86d7255..04f38a714a33c 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -653,13 +653,22 @@ func Activate(ctx *context.Context) { } // Resend confirmation email. if setting.Service.RegisterEmailConfirm { - if ctx.Cache.IsExist("MailResendLimit_" + ctx.Doer.LowerName) { + var cacheKey string + if ctx.Cache.IsExist("MailChangedJustNow_" + ctx.Doer.LowerName) { + cacheKey = "MailChangedLimit_" + if err := ctx.Cache.Delete("MailChangedJustNow_" + ctx.Doer.LowerName); err != nil { + log.Error("Delete cache(MailChangedJustNow) fail: %v", err) + } + } else { + cacheKey = "MailResendLimit_" + } + if ctx.Cache.IsExist(cacheKey + ctx.Doer.LowerName) { ctx.Data["ResendLimited"] = true } else { ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale) mailer.SendActivateAccountMail(ctx.Locale, ctx.Doer) - if err := ctx.Cache.Put("MailResendLimit_"+ctx.Doer.LowerName, ctx.Doer.LowerName, 180); err != nil { + if err := ctx.Cache.Put(cacheKey+ctx.Doer.LowerName, ctx.Doer.LowerName, 180); err != nil { log.Error("Set cache(MailResendLimit) fail: %v", err) } } @@ -693,6 +702,43 @@ 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 ctx.Cache.IsExist("MailChangeLimit_" + 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 + } + if err := ctx.Cache.Put("MailChangeLimit_"+ctx.Doer.LowerName, ctx.Doer.LowerName, 180); err != nil { + log.Error("Set cache(MailChangeLimit) fail: %v", err) + } + if err := ctx.Cache.Put("MailChangedJustNow_"+ctx.Doer.LowerName, ctx.Doer.LowerName, 180); err != nil { + log.Error("Set cache(MailChangedJustNow) fail: %v", err) + } + + // 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 1b0671975310e..d8a737e996f61 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 f983f98ad8bc7..8b0013419fe49 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,78 @@ 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)() + + // 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) + + // Try to change the email again + req = NewRequestWithValues(t, "POST", "/user/activate", map[string]string{ + "email": "wrong-again@example.com", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + // Verify that the email was NOT 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)() + + // 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) +}