From e927a86586afc138959b462d4040d1547f406f71 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 14 Feb 2026 04:06:59 -0800 Subject: [PATCH] Fix a bug user could change another user's primary email (#36586) (#36607) backport #36586 --- models/user/email_address.go | 19 ++++++++++------ models/user/email_address_test.go | 8 +++---- options/locale/locale_en-US.ini | 1 + routers/web/user/setting/account.go | 7 +++++- tests/integration/user_settings_test.go | 30 +++++++++++++++++++++++++ 5 files changed, 53 insertions(+), 12 deletions(-) diff --git a/models/user/email_address.go b/models/user/email_address.go index 67aa1bdd822..2b58edaeb5b 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -276,17 +276,22 @@ func updateActivation(ctx context.Context, email *EmailAddress, activate bool) e return UpdateUserCols(ctx, user, "rands") } -func MakeActiveEmailPrimary(ctx context.Context, emailID int64) error { - return makeEmailPrimaryInternal(ctx, emailID, true) +func MakeActiveEmailPrimary(ctx context.Context, ownerID, emailID int64) error { + return makeEmailPrimaryInternal(ctx, ownerID, emailID, true) } -func MakeInactiveEmailPrimary(ctx context.Context, emailID int64) error { - return makeEmailPrimaryInternal(ctx, emailID, false) +func MakeInactiveEmailPrimary(ctx context.Context, ownerID, emailID int64) error { + return makeEmailPrimaryInternal(ctx, ownerID, emailID, false) } -func makeEmailPrimaryInternal(ctx context.Context, emailID int64, isActive bool) error { +func makeEmailPrimaryInternal(ctx context.Context, ownerID, 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 { + if has, err := db.GetEngine(ctx).ID(emailID). + Where(builder.Eq{ + "uid": ownerID, + "is_activated": isActive, + }). + Get(email); err != nil { return err } else if !has { return ErrEmailAddressNotExist{} @@ -336,7 +341,7 @@ func ChangeInactivePrimaryEmail(ctx context.Context, uid int64, oldEmailAddr, ne if err != nil { return err } - return MakeInactiveEmailPrimary(ctx, newEmail.ID) + return MakeInactiveEmailPrimary(ctx, uid, newEmail.ID) }) } diff --git a/models/user/email_address_test.go b/models/user/email_address_test.go index 6ef18fb0f64..4167aaac0df 100644 --- a/models/user/email_address_test.go +++ b/models/user/email_address_test.go @@ -46,22 +46,22 @@ func TestIsEmailUsed(t *testing.T) { func TestMakeEmailPrimary(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - err := user_model.MakeActiveEmailPrimary(t.Context(), 9999999) + err := user_model.MakeActiveEmailPrimary(t.Context(), 1, 9999999) assert.Error(t, err) assert.ErrorIs(t, err, user_model.ErrEmailAddressNotExist{}) email := unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{Email: "user11@example.com"}) - err = user_model.MakeActiveEmailPrimary(t.Context(), email.ID) + err = user_model.MakeActiveEmailPrimary(t.Context(), email.UID, email.ID) assert.Error(t, err) assert.ErrorIs(t, err, user_model.ErrEmailAddressNotExist{}) // inactive email is considered as not exist for "MakeActiveEmailPrimary" email = unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{Email: "user9999999@example.com"}) - err = user_model.MakeActiveEmailPrimary(t.Context(), email.ID) + err = user_model.MakeActiveEmailPrimary(t.Context(), email.UID, email.ID) assert.Error(t, err) assert.True(t, user_model.IsErrUserNotExist(err)) email = unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{Email: "user101@example.com"}) - err = user_model.MakeActiveEmailPrimary(t.Context(), email.ID) + err = user_model.MakeActiveEmailPrimary(t.Context(), email.UID, email.ID) assert.NoError(t, err) user, _ := user_model.GetUserByID(t.Context(), int64(10)) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 08114f0d5fa..6f13d2c9c11 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -834,6 +834,7 @@ add_new_openid = Add New OpenID URI add_email = Add Email Address add_openid = Add OpenID URI add_email_confirmation_sent = A confirmation email has been sent to "%s". Please check your inbox within the next %s to confirm your email address. +email_primary_not_found = The selected email address could not be found. add_email_success = The new email address has been added. email_preference_set_success = Email preference has been set successfully. add_openid_success = The new OpenID address has been added. diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index 6b17da50e5b..a38a2cc4a37 100644 --- a/routers/web/user/setting/account.go +++ b/routers/web/user/setting/account.go @@ -113,7 +113,12 @@ func EmailPost(ctx *context.Context) { // Make email address primary. if ctx.FormString("_method") == "PRIMARY" { - if err := user_model.MakeActiveEmailPrimary(ctx, ctx.FormInt64("id")); err != nil { + if err := user_model.MakeActiveEmailPrimary(ctx, ctx.Doer.ID, ctx.FormInt64("id")); err != nil { + if user_model.IsErrEmailAddressNotExist(err) { + ctx.Flash.Error(ctx.Tr("settings.email_primary_not_found")) + ctx.Redirect(setting.AppSubURL + "/user/settings/account") + return + } ctx.ServerError("MakeEmailPrimary", err) return } diff --git a/tests/integration/user_settings_test.go b/tests/integration/user_settings_test.go index eab1a72ed5b..27ada9add33 100644 --- a/tests/integration/user_settings_test.go +++ b/tests/integration/user_settings_test.go @@ -180,6 +180,36 @@ func TestUserSettingsUpdateEmail(t *testing.T) { }) session.MakeRequest(t, req, http.StatusNotFound) }) + + t.Run("primary email not found", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + req := NewRequestWithValues(t, "POST", "/user/settings/account/email", map[string]string{ + "_method": "PRIMARY", + "id": "9999", + "_csrf": GetUserCSRFToken(t, session), + }) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + assert.Equal(t, "/user/settings/account", resp.Header().Get("Location")) + flashMsg := session.GetCookieFlashMessage() + assert.Equal(t, "The selected email address could not be found.", flashMsg.ErrorMsg) + }) + + t.Run("primary email not owned by user", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + req := NewRequestWithValues(t, "POST", "/user/settings/account/email", map[string]string{ + "_method": "PRIMARY", + "id": "6", + "_csrf": GetUserCSRFToken(t, session), + }) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + assert.Equal(t, "/user/settings/account", resp.Header().Get("Location")) + flashMsg := session.GetCookieFlashMessage() + assert.Equal(t, "The selected email address could not be found.", flashMsg.ErrorMsg) + }) } func TestUserSettingsDeleteEmail(t *testing.T) {