diff --git a/cli/skills/organization-management/SKILL.md b/cli/skills/organization-management/SKILL.md index ebd0470895..ed8aed448e 100644 --- a/cli/skills/organization-management/SKILL.md +++ b/cli/skills/organization-management/SKILL.md @@ -52,9 +52,10 @@ Use this skill for account and organization administration commands. - `npx @capgo/cli@latest organization set ORG_ID --enforce-hashed-api-keys` - Notes: - Security settings require `super_admin` role. + - Management email updates require `super_admin` role and sync the billing customer email through the private organization email endpoint. - Key options: - `-n, --name ` - - `-e, --email ` + - `-e, --email ` (requires `super_admin`) - `--enforce-2fa`, `--no-enforce-2fa` - `--password-policy`, `--no-password-policy` - `--min-length ` diff --git a/cli/src/index.ts b/cli/src/index.ts index c2db4a1847..79859b461e 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -607,7 +607,7 @@ organization .alias('s') .description(`⚙️ Update organization settings including name, email, security policies, and enforcement options. -Security settings require super_admin role. +Security settings and management email updates require super_admin role. Example: npx @capgo/cli@latest organization set ORG_ID --name "New Name" Example: npx @capgo/cli@latest organization set ORG_ID --enforce-2fa @@ -616,7 +616,7 @@ Example: npx @capgo/cli@latest organization set ORG_ID --require-apikey-expirati Example: npx @capgo/cli@latest organization set ORG_ID --enforce-hashed-api-keys`) .action(setOrganization) .option('-n, --name ', `Organization name`) - .option('-e, --email ', `Management email for the organization`) + .option('-e, --email ', `Management email for the organization (requires super_admin)`) .option('--enforce-2fa', `Enable 2FA enforcement for all organization members`) .option('--no-enforce-2fa', `Disable 2FA enforcement for organization`) .option('--password-policy', `Enable password policy enforcement for organization`) @@ -698,7 +698,7 @@ organisation .description(`[DEPRECATED] Use "organization set" instead.`) .action(setOrganization) .option('-n, --name ', `Organization name`) - .option('-e, --email ', `Management email for the organization`) + .option('-e, --email ', `Management email for the organization (requires super_admin)`) .option('--enforce-2fa', `Enable 2FA enforcement for all organization members`) .option('--no-enforce-2fa', `Disable 2FA enforcement for organization`) .option('--password-policy', `Enable password policy enforcement for organization`) diff --git a/cli/src/organization/set.ts b/cli/src/organization/set.ts index b501512b38..1b6520dbc9 100644 --- a/cli/src/organization/set.ts +++ b/cli/src/organization/set.ts @@ -11,6 +11,25 @@ import { sendEvent, } from '../utils' +type SupabaseClient = Awaited> + +async function updateOrganizationManagementEmail( + supabase: SupabaseClient, + orgId: string, + email: string, + silent: boolean, +) { + const { error } = await supabase.functions.invoke('private/set_org_email', { + body: JSON.stringify({ org_id: orgId, email }), + }) + + if (error) { + if (!silent) + log.error(`Could not update organization management email: ${formatError(error)}`) + throw new Error(`Could not update organization management email: ${formatError(error)}`) + } +} + export async function setOrganizationInternal( orgId: string, options: OrganizationSetOptions, @@ -399,18 +418,21 @@ export async function setOrganizationInternal( if (!silent) log.info(`Updating organization "${orgId}"`) - const { error: dbError } = await supabase - .from('orgs') - .update({ - name, - management_email: email, - }) - .eq('id', orgId) + if (name !== orgData.name) { + const { error: dbError } = await supabase + .from('orgs') + .update({ name }) + .eq('id', orgId) - if (dbError) { - if (!silent) - log.error(`Could not update organization ${formatError(dbError)}`) - throw new Error(`Could not update organization: ${formatError(dbError)}`) + if (dbError) { + if (!silent) + log.error(`Could not update organization ${formatError(dbError)}`) + throw new Error(`Could not update organization: ${formatError(dbError)}`) + } + } + + if (email !== orgData.management_email) { + await updateOrganizationManagementEmail(supabase, orgId, email, silent) } await sendEvent(enrichedOptions.apikey, { diff --git a/cli/webdocs/organisation.mdx b/cli/webdocs/organisation.mdx index 8b9ff8de2a..e26a5f35c2 100644 --- a/cli/webdocs/organisation.mdx +++ b/cli/webdocs/organisation.mdx @@ -62,7 +62,7 @@ npx @capgo/cli@latest organisation set | Param | Type | Description | | -------------- | ------------- | -------------------- | | **-n** | string | Organization name | -| **-e** | string | Management email for the organization | +| **-e** | string | Management email for the organization (requires super_admin) | | **--enforce-2fa** | boolean | Enable 2FA enforcement for all organization members | | **--no-enforce-2fa** | boolean | Disable 2FA enforcement for organization | | **--password-policy** | boolean | Enable password policy enforcement for organization | diff --git a/cli/webdocs/organization.mdx b/cli/webdocs/organization.mdx index a3c7ace275..9f6263e158 100644 --- a/cli/webdocs/organization.mdx +++ b/cli/webdocs/organization.mdx @@ -96,7 +96,7 @@ npx @capgo/cli@latest organization set ``` ⚙️ Update organization settings including name, email, security policies, and enforcement options. -Security settings require super_admin role. +Security settings and management email updates require super_admin role. **Example:** @@ -109,7 +109,7 @@ npx @capgo/cli@latest organization set ORG_ID --name "New Name" | Param | Type | Description | | -------------- | ------------- | -------------------- | | **-n** | string | Organization name | -| **-e** | string | Management email for the organization | +| **-e** | string | Management email for the organization (requires super_admin) | | **--enforce-2fa** | boolean | Enable 2FA enforcement for all organization members | | **--no-enforce-2fa** | boolean | Disable 2FA enforcement for organization | | **--password-policy** | boolean | Enable password policy enforcement for organization | diff --git a/supabase/functions/_backend/private/set_org_email.ts b/supabase/functions/_backend/private/set_org_email.ts index a7f6acb007..f49479bb46 100644 --- a/supabase/functions/_backend/private/set_org_email.ts +++ b/supabase/functions/_backend/private/set_org_email.ts @@ -5,7 +5,7 @@ import { safeParseSchema } from '../utils/ark_validation.ts' import { BRES, parseBody, quickError, simpleError, useCors } from '../utils/hono.ts' import { middlewareV2 } from '../utils/hono_middleware.ts' import { updateCustomerEmail } from '../utils/stripe.ts' -import { supabaseWithAuth } from '../utils/supabase.ts' +import { supabaseAdmin, supabaseWithAuth } from '../utils/supabase.ts' const bodySchema = type({ email: 'string.email', @@ -66,7 +66,8 @@ app.post('/', middlewareV2(['all', 'write']), async (c) => { await updateCustomerEmail(c, organization.customer_id, safeBody.email) // Update supabase - const { error: updateOrgErr } = await supabase.from('orgs') + const { error: updateOrgErr } = await supabaseAdmin(c) + .from('orgs') .update({ management_email: safeBody.email }) .eq('id', safeBody.org_id) diff --git a/supabase/functions/_backend/public/organization/put.ts b/supabase/functions/_backend/public/organization/put.ts index a2dddce86e..3dd410325e 100644 --- a/supabase/functions/_backend/public/organization/put.ts +++ b/supabase/functions/_backend/public/organization/put.ts @@ -7,7 +7,7 @@ import { safeParseSchema } from '../../utils/ark_validation.ts' import { quickError, simpleError } from '../../utils/hono.ts' import { checkPermission } from '../../utils/rbac.ts' import { createSignedImageUrl, normalizeImagePath } from '../../utils/storage.ts' -import { getStripeCustomerName, isDeterministicStripeCustomerUpdateError, updateCustomerOrganizationName } from '../../utils/stripe.ts' +import { getStripeCustomerName, isDeterministicStripeCustomerUpdateError, updateCustomerEmail, updateCustomerOrganizationName } from '../../utils/stripe.ts' import { apikeyHasOrgRightWithPolicy, supabaseAdmin, supabaseApikey, supabaseClient } from '../../utils/supabase.ts' import { normalizeWebsiteUrl } from './website.ts' @@ -74,6 +74,28 @@ async function ensureOrgAccess( throw simpleError('cannot_access_organization', 'You can\'t access this organization', { orgId }) } +async function ensureManagementEmailAccess( + supabase: ReturnType, + orgId: string, + authUserId: string, +) { + const userRight = await supabase.rpc('check_min_rights', { + min_right: 'super_admin', + org_id: orgId, + user_id: authUserId, + channel_id: null as any, + app_id: null as any, + }) + + if (userRight.error) { + throw simpleError('internal_auth_error', 'Internal auth error', { userRight }) + } + + if (!userRight.data) { + throw quickError(403, 'not_authorized', 'Only organization super admins can update the management email', { orgId }) + } +} + function validateMaxExpirationDays(maxDays?: number | null) { if (maxDays === undefined || maxDays === null) { return @@ -257,6 +279,26 @@ function getErrorDetail(error: unknown) { return error } +async function rollbackStripeCustomerEmail( + c: Context, + currentOrg: OrgRow, + originalError: unknown, +) { + if (!currentOrg.customer_id) { + return + } + + try { + await updateCustomerEmail(c, currentOrg.customer_id, currentOrg.management_email) + } + catch (rollbackError) { + throw simpleError('cannot_update_org', 'Cannot update org', { + error: getErrorDetail(originalError), + rollbackError: getErrorDetail(rollbackError), + }) + } +} + export async function put( c: Context, bodyRaw: any, @@ -275,6 +317,10 @@ export async function put( // Auth context is already set by middlewareV2 await ensureOrgAccess(c, apikey, body.orgId, supabase) + const shouldSyncStripeEmail = body.management_email !== undefined + if (body.management_email !== undefined) { + await ensureManagementEmailAccess(supabase, body.orgId, authUserId) + } if (body.enforcing_2fa) { await enforceSelf2faRequirement(authUserId, c) @@ -287,13 +333,30 @@ export async function put( : undefined const updateFields = buildUpdateFields(body, sanitizedOrgName) const shouldSyncStripeName = body.name !== undefined - const currentOrg = shouldSyncStripeName + const currentOrg = shouldSyncStripeName || shouldSyncStripeEmail ? await getOrgForNameSync(supabase, body.orgId) : null - const dataOrg: Database['public']['Tables']['orgs']['Row'] = await updateOrg(supabase, body.orgId, updateFields, { - expectedCurrentName: shouldSyncStripeName ? currentOrg?.name : undefined, - }) + if (shouldSyncStripeEmail) { + if (!currentOrg?.customer_id) { + throw simpleError('org_does_not_have_customer', 'Organization does not have a customer id', { orgId: body.orgId }) + } + await updateCustomerEmail(c, currentOrg.customer_id, body.management_email!) + } + + const writeSupabase = shouldSyncStripeEmail ? supabaseAdmin(c) : supabase + let dataOrg: Database['public']['Tables']['orgs']['Row'] + try { + dataOrg = await updateOrg(writeSupabase, body.orgId, updateFields, { + expectedCurrentName: shouldSyncStripeName ? currentOrg?.name : undefined, + }) + } + catch (updateError) { + if (shouldSyncStripeEmail && currentOrg) { + await rollbackStripeCustomerEmail(c, currentOrg, updateError) + } + throw updateError + } const committedCustomerId = dataOrg.customer_id @@ -311,10 +374,13 @@ export async function put( const rollbackFields = buildRollbackFields(currentOrg, updateFields) try { - await updateOrg(supabase, body.orgId, rollbackFields, { + await updateOrg(writeSupabase, body.orgId, rollbackFields, { expectedCurrentName: dataOrg.name, expectedCurrentFields: buildExpectedCurrentFields(dataOrg, updateFields), }) + if (shouldSyncStripeEmail && currentOrg) { + await rollbackStripeCustomerEmail(c, currentOrg, stripeError) + } } catch (rollbackError) { throw simpleError('cannot_update_org', 'Cannot update org', { diff --git a/supabase/migrations/20260511022030_restrict_org_management_email_updates.sql b/supabase/migrations/20260511022030_restrict_org_management_email_updates.sql new file mode 100644 index 0000000000..43eee25749 --- /dev/null +++ b/supabase/migrations/20260511022030_restrict_org_management_email_updates.sql @@ -0,0 +1,36 @@ +DROP TRIGGER IF EXISTS "prevent_org_management_email_non_super_admin_update" ON "public"."orgs"; +DROP TRIGGER IF EXISTS "prevent_org_management_email_direct_update" ON "public"."orgs"; +DROP FUNCTION IF EXISTS "public"."prevent_org_management_email_non_super_admin_update"(); + +CREATE OR REPLACE FUNCTION "public"."prevent_org_management_email_direct_update"() +RETURNS trigger +LANGUAGE "plpgsql" +SECURITY DEFINER +SET "search_path" TO '' +AS $$ +BEGIN + IF NEW.management_email IS NOT DISTINCT FROM OLD.management_email THEN + RETURN NEW; + END IF; + + IF (SELECT auth.role()) = 'service_role' THEN + RETURN NEW; + END IF; + + RAISE EXCEPTION 'Management email updates must use the organization email sync endpoint' + USING ERRCODE = '42501'; + + RETURN NEW; +END; +$$; + +ALTER FUNCTION "public"."prevent_org_management_email_direct_update"() OWNER TO "postgres"; +REVOKE ALL ON FUNCTION "public"."prevent_org_management_email_direct_update"() FROM PUBLIC; +REVOKE ALL ON FUNCTION "public"."prevent_org_management_email_direct_update"() FROM "anon"; +REVOKE ALL ON FUNCTION "public"."prevent_org_management_email_direct_update"() FROM "authenticated"; +GRANT ALL ON FUNCTION "public"."prevent_org_management_email_direct_update"() TO "service_role"; + +CREATE TRIGGER "prevent_org_management_email_direct_update" +BEFORE UPDATE OF "management_email" ON "public"."orgs" +FOR EACH ROW +EXECUTE FUNCTION "public"."prevent_org_management_email_direct_update"(); diff --git a/tests/organization-api.test.ts b/tests/organization-api.test.ts index 633b118592..2e9b7aa159 100644 --- a/tests/organization-api.test.ts +++ b/tests/organization-api.test.ts @@ -7,6 +7,7 @@ import { parseSchema, safeParseSchema } from '../supabase/functions/_backend/uti import { BASE_URL, + getAuthHeaders, getAuthHeadersForCredentials, getSupabaseClient, headers, @@ -1095,6 +1096,163 @@ describe('[PUT] /organization', () => { } }) + it.concurrent('rejects management email updates from org admins', async () => { + const orgId = randomUUID() + const orgName = `Admin Email Boundary Organization ${new Date().toISOString()}` + const attemptedEmail = `admin-bypass-${randomUUID()}@example.com` + const { error: createError } = await getSupabaseClient().from('orgs').insert({ + id: orgId, + name: orgName, + management_email: TEST_EMAIL, + created_by: USER_ID_2, + use_new_rbac: false, + }) + if (createError) + throw createError + + const { error: orgUserError } = await getSupabaseClient().from('org_users').insert({ + org_id: orgId, + user_id: USER_ID, + user_right: 'admin', + }) + if (orgUserError) + throw orgUserError + + try { + const adminHeaders = await getAuthHeaders() + const response = await fetch(`${BASE_URL}/organization`, { + headers: adminHeaders, + method: 'PUT', + body: JSON.stringify({ orgId, management_email: attemptedEmail }), + }) + expect(response.status).toBe(403) + const responseData = await response.json() as { error: string } + expect(responseData.error).toBe('not_authorized') + + const { data, error } = await getSupabaseClient() + .from('orgs') + .select('management_email') + .eq('id', orgId) + .single() + expect(error).toBeNull() + expect(data?.management_email).toBe(TEST_EMAIL) + } + finally { + await getSupabaseClient().from('org_users').delete().eq('org_id', orgId) + await getSupabaseClient().from('orgs').delete().eq('id', orgId) + } + }) + + it.concurrent('rejects direct management email updates from org admins', async () => { + const orgId = randomUUID() + const orgName = `Direct Admin Email Boundary Organization ${new Date().toISOString()}` + const attemptedEmail = `direct-admin-bypass-${randomUUID()}@example.com` + const { error: createError } = await getSupabaseClient().from('orgs').insert({ + id: orgId, + name: orgName, + management_email: TEST_EMAIL, + created_by: USER_ID_2, + use_new_rbac: false, + }) + if (createError) + throw createError + + const { error: orgUserError } = await getSupabaseClient().from('org_users').insert({ + org_id: orgId, + user_id: USER_ID, + user_right: 'admin', + }) + if (orgUserError) + throw orgUserError + + try { + const adminHeaders = await getAuthHeaders() + const adminClient = createClient(normalizedSupabaseBaseUrl, SUPABASE_ANON_KEY, { + auth: { + persistSession: false, + }, + global: { + headers: adminHeaders, + }, + }) + + const { error: updateError } = await adminClient + .from('orgs') + .update({ management_email: attemptedEmail }) + .eq('id', orgId) + + expect(updateError).toBeTruthy() + expect(updateError?.message).toContain('Management email updates must use the organization email sync endpoint') + + const { data, error } = await getSupabaseClient() + .from('orgs') + .select('management_email') + .eq('id', orgId) + .single() + expect(error).toBeNull() + expect(data?.management_email).toBe(TEST_EMAIL) + } + finally { + await getSupabaseClient().from('org_users').delete().eq('org_id', orgId) + await getSupabaseClient().from('orgs').delete().eq('id', orgId) + } + }) + + it.concurrent('rejects direct management email updates from org super admins', async () => { + const orgId = randomUUID() + const orgName = `Direct Super Admin Email Boundary Organization ${new Date().toISOString()}` + const attemptedEmail = `direct-super-admin-bypass-${randomUUID()}@example.com` + const { error: createError } = await getSupabaseClient().from('orgs').insert({ + id: orgId, + name: orgName, + management_email: TEST_EMAIL, + created_by: USER_ID, + use_new_rbac: false, + }) + if (createError) + throw createError + + const { error: orgUserError } = await getSupabaseClient().from('org_users').insert({ + org_id: orgId, + user_id: USER_ID, + user_right: 'super_admin', + }) + if (orgUserError) + throw orgUserError + + try { + const superAdminHeaders = await getAuthHeaders() + const superAdminClient = createClient(normalizedSupabaseBaseUrl, SUPABASE_ANON_KEY, { + auth: { + persistSession: false, + }, + global: { + headers: superAdminHeaders, + }, + }) + + const { error: updateError } = await superAdminClient + .from('orgs') + .update({ management_email: attemptedEmail }) + .eq('id', orgId) + + expect(updateError).toBeTruthy() + expect(updateError?.message).toContain('Management email updates must use the organization email sync endpoint') + + const { data, error } = await getSupabaseClient() + .from('orgs') + .select('management_email') + .eq('id', orgId) + .single() + expect(error).toBeNull() + expect(data?.management_email).toBe(TEST_EMAIL) + } + finally { + await getSupabaseClient().from('org_users').delete().eq('org_id', orgId) + await getSupabaseClient().from('orgs').delete().eq('id', orgId) + } + }) + it('update organization with invalid body', async () => { const response = await fetch(`${BASE_URL}/organization`, { headers, diff --git a/tests/organization-put-stripe-sync.unit.test.ts b/tests/organization-put-stripe-sync.unit.test.ts index 2cbcb1c8f7..dffacab8b9 100644 --- a/tests/organization-put-stripe-sync.unit.test.ts +++ b/tests/organization-put-stripe-sync.unit.test.ts @@ -9,6 +9,7 @@ const { apikeyHasOrgRightWithPolicyMock, supabaseAdminMock, updateCustomerOrganizationNameMock, + updateCustomerEmailMock, getStripeCustomerNameMock, isDeterministicStripeCustomerUpdateErrorMock, } = vi.hoisted(() => ({ @@ -18,6 +19,7 @@ const { apikeyHasOrgRightWithPolicyMock: vi.fn(), supabaseAdminMock: vi.fn(), updateCustomerOrganizationNameMock: vi.fn(), + updateCustomerEmailMock: vi.fn(), getStripeCustomerNameMock: vi.fn(), isDeterministicStripeCustomerUpdateErrorMock: vi.fn(), })) @@ -27,6 +29,7 @@ vi.mock('../supabase/functions/_backend/utils/rbac.ts', () => ({ })) vi.mock('../supabase/functions/_backend/utils/stripe.ts', () => ({ + updateCustomerEmail: (...args: unknown[]) => updateCustomerEmailMock(...args), updateCustomerOrganizationName: (...args: unknown[]) => updateCustomerOrganizationNameMock(...args), getStripeCustomerName: (...args: unknown[]) => getStripeCustomerNameMock(...args), isDeterministicStripeCustomerUpdateError: (...args: unknown[]) => isDeterministicStripeCustomerUpdateErrorMock(...args), @@ -126,12 +129,47 @@ describe('organization put Stripe sync', () => { beforeEach(() => { vi.clearAllMocks() checkPermissionMock.mockResolvedValue(true) + updateCustomerEmailMock.mockResolvedValue(undefined) updateCustomerOrganizationNameMock.mockResolvedValue(undefined) getStripeCustomerNameMock.mockResolvedValue(undefined) isDeterministicStripeCustomerUpdateErrorMock.mockReturnValue(false) apikeyHasOrgRightWithPolicyMock.mockResolvedValue({ valid: true }) }) + it('writes management email changes with the service-role client after Stripe sync', async () => { + const currentOrg = createOrgRow({ + id: 'org-123', + name: 'Old Name', + customer_id: 'cus_123', + }) + const selectBuilder = createOrgSelectBuilder(currentOrg) + const updateBuilder = createOrgUpdateBuilder({ + ...currentOrg, + management_email: 'new-billing@capgo.app', + }) + const userFrom = vi.fn().mockReturnValueOnce(selectBuilder) + const adminFrom = vi.fn().mockReturnValueOnce(updateBuilder) + + supabaseClientMock.mockReturnValue({ + from: userFrom, + rpc: vi.fn().mockResolvedValue({ data: true, error: null }), + }) + supabaseAdminMock.mockReturnValue({ + from: adminFrom, + }) + + const response = await put(createContext(), { + orgId: 'org-123', + management_email: 'new-billing@capgo.app', + }, undefined) + + expect(response.status).toBe(200) + expect(updateCustomerEmailMock).toHaveBeenCalledWith(expect.anything(), 'cus_123', 'new-billing@capgo.app') + expect(adminFrom).toHaveBeenCalledWith('orgs') + expect(updateBuilder.update).toHaveBeenCalledWith({ management_email: 'new-billing@capgo.app' }) + expect(userFrom).toHaveBeenCalledWith('orgs') + }) + it('updates the org row before syncing Stripe customer name', async () => { const selectBuilder = createOrgSelectBuilder(createOrgRow({ id: 'org-123',