From 52902d4eceee4a3ee78a282128f2afc56bc42c22 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Fri, 5 Dec 2025 02:25:52 +0800 Subject: [PATCH] Fix edit user email bug in API (#36068) (#36081) Backport #36068 by @lunny Follow #36058 for API edit user bug when editing email. - The Admin Edit User API includes a breaking change. Previously, when updating a user with an email from an unallowed domain, the request would succeed but return a warning in the response headers. Now, the request will fail and return an error in the response body instead. - Removed `AdminAddOrSetPrimaryEmailAddress` because it will not be used any where. Fix https://github.com/go-gitea/gitea/pull/36058#issuecomment-3600005186 Co-authored-by: Lunny Xiao --- routers/api/v1/admin/user.go | 9 +++-- services/user/email.go | 54 ----------------------------- services/user/email_test.go | 51 --------------------------- tests/integration/api_admin_test.go | 8 +++-- 4 files changed, 9 insertions(+), 113 deletions(-) diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 494bace585..c99fda6cbb 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -216,9 +216,12 @@ func EditUser(ctx *context.APIContext) { } if form.Email != nil { - if err := user_service.AdminAddOrSetPrimaryEmailAddress(ctx, ctx.ContextUser, *form.Email); err != nil { + if err := user_service.ReplacePrimaryEmailAddress(ctx, ctx.ContextUser, *form.Email); err != nil { switch { case user_model.IsErrEmailCharIsNotSupported(err), user_model.IsErrEmailInvalid(err): + if !user_model.IsEmailDomainAllowed(*form.Email) { + err = fmt.Errorf("the domain of user email %s conflicts with EMAIL_DOMAIN_ALLOWLIST or EMAIL_DOMAIN_BLOCKLIST", *form.Email) + } ctx.APIError(http.StatusBadRequest, err) case user_model.IsErrEmailAlreadyUsed(err): ctx.APIError(http.StatusBadRequest, err) @@ -227,10 +230,6 @@ func EditUser(ctx *context.APIContext) { } return } - - if !user_model.IsEmailDomainAllowed(*form.Email) { - ctx.Resp.Header().Add("X-Gitea-Warning", fmt.Sprintf("the domain of user email %s conflicts with EMAIL_DOMAIN_ALLOWLIST or EMAIL_DOMAIN_BLOCKLIST", *form.Email)) - } } opts := &user_service.UpdateOptions{ diff --git a/services/user/email.go b/services/user/email.go index de1e024bd1..c45b3b3ec9 100644 --- a/services/user/email.go +++ b/services/user/email.go @@ -14,60 +14,6 @@ import ( "code.gitea.io/gitea/modules/util" ) -// AdminAddOrSetPrimaryEmailAddress is used by admins to add or set a user's primary email address -func AdminAddOrSetPrimaryEmailAddress(ctx context.Context, u *user_model.User, emailStr string) error { - if strings.EqualFold(u.Email, emailStr) { - return nil - } - - if err := user_model.ValidateEmailForAdmin(emailStr); err != nil { - return err - } - - // Check if address exists already - email, err := user_model.GetEmailAddressByEmail(ctx, emailStr) - if err != nil && !errors.Is(err, util.ErrNotExist) { - return err - } - if email != nil && email.UID != u.ID { - return user_model.ErrEmailAlreadyUsed{Email: emailStr} - } - - // Update old primary address - primary, err := user_model.GetPrimaryEmailAddressOfUser(ctx, u.ID) - if err != nil { - return err - } - - primary.IsPrimary = false - if err := user_model.UpdateEmailAddress(ctx, primary); err != nil { - return err - } - - // Insert new or update existing address - if email != nil { - email.IsPrimary = true - email.IsActivated = true - if err := user_model.UpdateEmailAddress(ctx, email); err != nil { - return err - } - } else { - email = &user_model.EmailAddress{ - UID: u.ID, - Email: emailStr, - IsActivated: true, - IsPrimary: true, - } - if _, err := user_model.InsertEmailAddress(ctx, email); err != nil { - return err - } - } - - u.Email = emailStr - - return user_model.UpdateUserCols(ctx, u, "email") -} - func ReplacePrimaryEmailAddress(ctx context.Context, u *user_model.User, emailStr string) error { if strings.EqualFold(u.Email, emailStr) { return nil diff --git a/services/user/email_test.go b/services/user/email_test.go index 76770a9230..a031b12cad 100644 --- a/services/user/email_test.go +++ b/services/user/email_test.go @@ -9,61 +9,10 @@ import ( organization_model "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/glob" - "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" ) -func TestAdminAddOrSetPrimaryEmailAddress(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 27}) - - emails, err := user_model.GetEmailAddresses(t.Context(), user.ID) - assert.NoError(t, err) - assert.Len(t, emails, 1) - - primary, err := user_model.GetPrimaryEmailAddressOfUser(t.Context(), user.ID) - assert.NoError(t, err) - assert.NotEqual(t, "new-primary@example.com", primary.Email) - assert.Equal(t, user.Email, primary.Email) - - assert.NoError(t, AdminAddOrSetPrimaryEmailAddress(t.Context(), user, "new-primary@example.com")) - - primary, err = user_model.GetPrimaryEmailAddressOfUser(t.Context(), user.ID) - assert.NoError(t, err) - assert.Equal(t, "new-primary@example.com", primary.Email) - assert.Equal(t, user.Email, primary.Email) - - emails, err = user_model.GetEmailAddresses(t.Context(), user.ID) - assert.NoError(t, err) - assert.Len(t, emails, 2) - - setting.Service.EmailDomainAllowList = []glob.Glob{glob.MustCompile("example.org")} - defer func() { - setting.Service.EmailDomainAllowList = []glob.Glob{} - }() - - assert.NoError(t, AdminAddOrSetPrimaryEmailAddress(t.Context(), user, "new-primary2@example2.com")) - - primary, err = user_model.GetPrimaryEmailAddressOfUser(t.Context(), user.ID) - assert.NoError(t, err) - assert.Equal(t, "new-primary2@example2.com", primary.Email) - assert.Equal(t, user.Email, primary.Email) - - assert.NoError(t, AdminAddOrSetPrimaryEmailAddress(t.Context(), user, "user27@example.com")) - - primary, err = user_model.GetPrimaryEmailAddressOfUser(t.Context(), user.ID) - assert.NoError(t, err) - assert.Equal(t, "user27@example.com", primary.Email) - assert.Equal(t, user.Email, primary.Email) - - emails, err = user_model.GetEmailAddresses(t.Context(), user.ID) - assert.NoError(t, err) - assert.Len(t, emails, 3) -} - func TestReplacePrimaryEmailAddress(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index dbd62c4078..763d4d526b 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -382,10 +382,12 @@ func TestAPIEditUser_NotAllowedEmailDomain(t *testing.T) { SourceID: 0, Email: &newEmail, }).AddTokenAuth(token) - resp := MakeRequest(t, req, http.StatusOK) - assert.Equal(t, "the domain of user email user2@example1.com conflicts with EMAIL_DOMAIN_ALLOWLIST or EMAIL_DOMAIN_BLOCKLIST", resp.Header().Get("X-Gitea-Warning")) + resp := MakeRequest(t, req, http.StatusBadRequest) + errMap := make(map[string]string) + assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), &errMap)) + assert.Equal(t, "the domain of user email user2@example1.com conflicts with EMAIL_DOMAIN_ALLOWLIST or EMAIL_DOMAIN_BLOCKLIST", errMap["message"]) - originalEmail := "user2@example.com" + originalEmail := "user2@example.org" req = NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ LoginName: "user2", SourceID: 0,