From db29a05b451538eb825efbd2c968bea0e07fbdf9 Mon Sep 17 00:00:00 2001 From: JakobDev Date: Thu, 10 Oct 2024 18:56:01 +0200 Subject: [PATCH] fix: Allow Organisations to remove the EMail Address --- models/user/email_address.go | 32 ++++++++++ models/user/email_address_test.go | 18 ++++++ routers/api/v1/org/org.go | 17 +++++- routers/web/org/setting.go | 8 ++- templates/swagger/v1_json.tmpl | 3 + tests/integration/api_org_test.go | 40 +++++++++++++ tests/integration/org_settings_test.go | 83 ++++++++++++++++++++++++++ 7 files changed, 198 insertions(+), 3 deletions(-) create mode 100644 tests/integration/org_settings_test.go diff --git a/models/user/email_address.go b/models/user/email_address.go index 8c6f24e57b..e8c81a730c 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -202,6 +202,38 @@ func GetPrimaryEmailAddressOfUser(ctx context.Context, uid int64) (*EmailAddress return ea, nil } +// Deletes the primary email address of the user +// This is only alllowed if the user is a organization +func DeletePrimaryEmailAddressOfUser(ctx context.Context, uid int64) error { + user, err := GetUserByID(ctx, uid) + if err != nil { + return err + } + + if user.Type != UserTypeOrganization { + return fmt.Errorf("%s is not a organization", user.Name) + } + + ctx, committer, err := db.TxContext(ctx) + if err != nil { + return err + } + defer committer.Close() + + _, err = db.GetEngine(ctx).Exec("DELETE FROM email_address WHERE uid = ? AND is_primary = true", uid) + if err != nil { + return err + } + + user.Email = "" + err = UpdateUserCols(ctx, user, "email") + if err != nil { + return err + } + + return committer.Commit() +} + // GetEmailAddresses returns all email addresses belongs to given user. func GetEmailAddresses(ctx context.Context, uid int64) ([]*EmailAddress, error) { emails := make([]*EmailAddress, 0, 5) diff --git a/models/user/email_address_test.go b/models/user/email_address_test.go index b918f21018..886e1715c3 100644 --- a/models/user/email_address_test.go +++ b/models/user/email_address_test.go @@ -220,3 +220,21 @@ func TestGetActivatedEmailAddresses(t *testing.T) { }) } } + +func TestDeletePrimaryEmailAddressOfUser(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + user, err := user_model.GetUserByName(db.DefaultContext, "org3") + require.NoError(t, err) + assert.Equal(t, "org3@example.com", user.Email) + + require.NoError(t, user_model.DeletePrimaryEmailAddressOfUser(db.DefaultContext, user.ID)) + + user, err = user_model.GetUserByName(db.DefaultContext, "org3") + require.NoError(t, err) + assert.Empty(t, user.Email) + + email, err := user_model.GetPrimaryEmailAddressOfUser(db.DefaultContext, user.ID) + assert.True(t, user_model.IsErrEmailAddressNotExist(err)) + assert.Nil(t, email) +} diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index 9f9483d4ff..5f4fc8f4b6 100644 --- a/routers/api/v1/org/org.go +++ b/routers/api/v1/org/org.go @@ -340,12 +340,25 @@ func Edit(ctx *context.APIContext) { // "$ref": "#/responses/Organization" // "404": // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/error" form := web.GetForm(ctx).(*api.EditOrgOption) - if form.Email != "" { + if form.Email == "" { + err := user_model.DeletePrimaryEmailAddressOfUser(ctx, ctx.Org.Organization.ID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "DeletePrimaryEmailAddressOfUser", err) + return + } + ctx.Org.Organization.Email = "" + } else { if err := user_service.ReplacePrimaryEmailAddress(ctx, ctx.Org.Organization.AsUser(), form.Email); err != nil { - ctx.Error(http.StatusInternalServerError, "ReplacePrimaryEmailAddress", err) + if user_model.IsErrEmailInvalid(err) || user_model.IsErrEmailCharIsNotSupported(err) { + ctx.Error(http.StatusUnprocessableEntity, "ReplacePrimaryEmailAddress", err) + } else { + ctx.Error(http.StatusInternalServerError, "ReplacePrimaryEmailAddress", err) + } return } } diff --git a/routers/web/org/setting.go b/routers/web/org/setting.go index 0be734abaf..8bd8ae6126 100644 --- a/routers/web/org/setting.go +++ b/routers/web/org/setting.go @@ -93,7 +93,13 @@ func SettingsPost(ctx *context.Context) { ctx.Org.OrgLink = setting.AppSubURL + "/org/" + url.PathEscape(org.Name) } - if form.Email != "" { + if form.Email == "" { + err := user_model.DeletePrimaryEmailAddressOfUser(ctx, org.ID) + if err != nil { + ctx.ServerError("DeletePrimaryEmailAddressOfUser", err) + return + } + } else { if err := user_service.ReplacePrimaryEmailAddress(ctx, org.AsUser(), form.Email); err != nil { ctx.Data["Err_Email"] = true ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSettingsOptions, &form) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 8c4e7eaf8b..5761fab2e3 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -2263,6 +2263,9 @@ }, "404": { "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/error" } } } diff --git a/tests/integration/api_org_test.go b/tests/integration/api_org_test.go index 70d3a446f7..4aff75cbd9 100644 --- a/tests/integration/api_org_test.go +++ b/tests/integration/api_org_test.go @@ -226,3 +226,43 @@ func TestAPIOrgSearchEmptyTeam(t *testing.T) { } }) } + +func TestAPIOrgChangeEmail(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user1") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization) + + t.Run("Invalid", func(t *testing.T) { + settings := api.EditOrgOption{Email: "invalid"} + + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", "/api/v1/orgs/org3", &settings).AddTokenAuth(token), http.StatusUnprocessableEntity) + + var org *api.Organization + DecodeJSON(t, resp, &org) + + assert.Empty(t, org.Email) + }) + + t.Run("Valid", func(t *testing.T) { + settings := api.EditOrgOption{Email: "example@example.com"} + + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", "/api/v1/orgs/org3", &settings).AddTokenAuth(token), http.StatusOK) + + var org *api.Organization + DecodeJSON(t, resp, &org) + + assert.Equal(t, "example@example.com", org.Email) + }) + + t.Run("Empty", func(t *testing.T) { + settings := api.EditOrgOption{Email: ""} + + resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", "/api/v1/orgs/org3", &settings).AddTokenAuth(token), http.StatusOK) + + var org *api.Organization + DecodeJSON(t, resp, &org) + + assert.Empty(t, org.Email) + }) +} diff --git a/tests/integration/org_settings_test.go b/tests/integration/org_settings_test.go new file mode 100644 index 0000000000..cd08a83552 --- /dev/null +++ b/tests/integration/org_settings_test.go @@ -0,0 +1,83 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "fmt" + "net/http" + "testing" + + auth_model "code.gitea.io/gitea/models/auth" + api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" +) + +func getOrgSettingsFormData(t *testing.T, session *TestSession, orgName string) map[string]string { + return map[string]string{ + "_csrf": GetCSRF(t, session, fmt.Sprintf("/org/%s/settings", orgName)), + "name": orgName, + "full_name": "", + "email": "", + "description": "", + "website": "", + "location": "", + "visibility": "0", + "repo_admin_change_team_access": "on", + "max_repo_creation": "-1", + } +} + +func getOrgSettings(t *testing.T, token, orgName string) *api.Organization { + t.Helper() + + req := NewRequestf(t, "GET", "/api/v1/orgs/%s", orgName).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + + var org *api.Organization + DecodeJSON(t, resp, &org) + + return org +} + +func TestOrgSettingsChangeEmail(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + const orgName = "org3" + settingsURL := fmt.Sprintf("/org/%s/settings", orgName) + + session := loginUser(t, "user1") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadOrganization) + + t.Run("Invalid", func(t *testing.T) { + settings := getOrgSettingsFormData(t, session, orgName) + + settings["email"] = "invalid" + session.MakeRequest(t, NewRequestWithValues(t, "POST", settingsURL, settings), http.StatusOK) + + org := getOrgSettings(t, token, orgName) + assert.Equal(t, "org3@example.com", org.Email) + }) + + t.Run("Valid", func(t *testing.T) { + settings := getOrgSettingsFormData(t, session, orgName) + + settings["email"] = "example@example.com" + session.MakeRequest(t, NewRequestWithValues(t, "POST", settingsURL, settings), http.StatusSeeOther) + + org := getOrgSettings(t, token, orgName) + assert.Equal(t, "example@example.com", org.Email) + }) + + t.Run("Empty", func(t *testing.T) { + settings := getOrgSettingsFormData(t, session, orgName) + + settings["email"] = "" + session.MakeRequest(t, NewRequestWithValues(t, "POST", settingsURL, settings), http.StatusSeeOther) + + org := getOrgSettings(t, token, orgName) + assert.Empty(t, org.Email) + }) +}