diff --git a/server/supabase/migrations/20260929000006_drop_legacy_admin_write_policies.sql b/server/supabase/migrations/20260929000006_drop_legacy_admin_write_policies.sql new file mode 100644 index 0000000..a5f8d2d --- /dev/null +++ b/server/supabase/migrations/20260929000006_drop_legacy_admin_write_policies.sql @@ -0,0 +1,32 @@ +-- Back-office writes go only through the audited service-role RPCs. +-- +-- 20260413000005_add_manager_role.sql created three JWT-role write policies: +-- * subscriptions.admin_write_subscriptions FOR ALL (admin, super_admin) +-- * subscriptions.manager_update_subscriptions FOR UPDATE (manager) +-- * profiles.admin_update_profiles FOR UPDATE (admin, super_admin) +-- They key only on auth.jwt() -> app_metadata.role and restrict no columns. +-- 20260821000023_atomic_admin_management.sql moved every back-office mutation to +-- service-role RPCs (admin_mutate_subscription_v1 and friends) that validate +-- input, use an idempotency ledger and write audit_log. The admin web app and +-- the admin-* edge functions only use the service-role client, so these +-- policies serve no legitimate path. +-- +-- Left in place they let any staff access token PATCH /rest/v1/subscriptions +-- directly: set tier='pro_plus' or overage_credits to an arbitrary value, or +-- copy another customer's payple_payer_id onto a row so payple-renew charges +-- the wrong card. None of that writes an audit_log row or goes through the RPC +-- whitelist and bounds checks. +-- +-- The read policies (admin_read_all_*, *_read_own) and profiles_update_own are +-- kept. All subscription writers (handle_new_user, quota, payment-provider and +-- admin RPCs) are SECURITY DEFINER or run as service_role, so revoking the +-- write privileges from the client roles does not affect them. + +DROP POLICY IF EXISTS admin_write_subscriptions ON public.subscriptions; +DROP POLICY IF EXISTS manager_update_subscriptions ON public.subscriptions; +DROP POLICY IF EXISTS admin_update_profiles ON public.profiles; + +-- Defense in depth: without any write policy RLS already denies these rows, but +-- the client roles should not hold table-level write privileges at all. TRUNCATE +-- is included because it is not subject to RLS. +REVOKE INSERT, UPDATE, DELETE, TRUNCATE ON TABLE public.subscriptions FROM anon, authenticated; diff --git a/server/supabase/tests/drop-legacy-admin-write-policies.integration.sql b/server/supabase/tests/drop-legacy-admin-write-policies.integration.sql new file mode 100644 index 0000000..7d8fa4d --- /dev/null +++ b/server/supabase/tests/drop-legacy-admin-write-policies.integration.sql @@ -0,0 +1,261 @@ +\set ON_ERROR_STOP on + +-- Regression: staff access tokens must not write subscriptions or other users' +-- profiles directly through PostgREST. Back-office mutations go only through the +-- audited service-role RPCs (admin_mutate_subscription_v1 and friends). +-- Before 20260929000006 the JWT-role policies admin_write_subscriptions, +-- manager_update_subscriptions and admin_update_profiles let a manager copy +-- another customer's payple_payer_id onto a row (so payple-renew charged the +-- wrong card) or raise tier/overage_credits with no audit_log row. + +BEGIN; + +INSERT INTO auth.users ( + id, aud, role, email, encrypted_password, email_confirmed_at, + raw_app_meta_data, raw_user_meta_data, created_at, updated_at +) VALUES + ( + '42600000-0000-4000-8000-000000000001', 'authenticated', 'authenticated', + 'legacy-policy-manager@example.invalid', crypt('fixture-password', gen_salt('bf')), now(), + '{"provider":"email","providers":["email"],"role":"manager"}'::jsonb, '{}'::jsonb, now(), now() + ), + ( + '42600000-0000-4000-8000-000000000002', 'authenticated', 'authenticated', + 'legacy-policy-admin@example.invalid', crypt('fixture-password', gen_salt('bf')), now(), + '{"provider":"email","providers":["email"],"role":"admin"}'::jsonb, '{}'::jsonb, now(), now() + ), + ( + '42600000-0000-4000-8000-000000000003', 'authenticated', 'authenticated', + 'legacy-policy-payer@example.invalid', crypt('fixture-password', gen_salt('bf')), now(), + '{"provider":"email","providers":["email"]}'::jsonb, '{}'::jsonb, now(), now() + ), + ( + '42600000-0000-4000-8000-000000000004', 'authenticated', 'authenticated', + 'legacy-policy-target@example.invalid', crypt('fixture-password', gen_salt('bf')), now(), + '{"provider":"email","providers":["email"]}'::jsonb, '{}'::jsonb, now(), now() + ), + ( + '42600000-0000-4000-8000-000000000005', 'authenticated', 'authenticated', + 'legacy-policy-unsubscribed@example.invalid', crypt('fixture-password', gen_salt('bf')), now(), + '{"provider":"email","providers":["email"]}'::jsonb, '{}'::jsonb, now(), now() + ); + +UPDATE public.profiles SET role = 'manager' WHERE id = '42600000-0000-4000-8000-000000000001'; +UPDATE public.profiles SET role = 'admin' WHERE id = '42600000-0000-4000-8000-000000000002'; +UPDATE public.profiles SET name = 'Original Target' WHERE id = '42600000-0000-4000-8000-000000000004'; + +INSERT INTO public.subscriptions (user_id, tier, status, provider, payment_provider) +VALUES + ('42600000-0000-4000-8000-000000000003', 'free', 'active', 'none', 'none'), + ('42600000-0000-4000-8000-000000000004', 'free', 'active', 'none', 'none') +ON CONFLICT (user_id) DO NOTHING; + +UPDATE public.subscriptions + SET payple_payer_id = 'legacy-policy-victim-payer', + overage_credits = 0 + WHERE user_id = '42600000-0000-4000-8000-000000000003'; + +UPDATE public.subscriptions + SET tier = 'free', status = 'active', payple_payer_id = NULL, overage_credits = 0 + WHERE user_id = '42600000-0000-4000-8000-000000000004'; + +DELETE FROM public.subscriptions WHERE user_id = '42600000-0000-4000-8000-000000000005'; + +-- Catalog: only read policies remain on subscriptions, and the client roles hold +-- no table-level write privilege on it. +DO $$ +DECLARE + v_policy text; +BEGIN + SELECT string_agg(policyname || ':' || cmd, ', ') INTO v_policy + FROM pg_policies + WHERE schemaname = 'public' AND tablename = 'subscriptions' AND cmd <> 'SELECT'; + IF v_policy IS NOT NULL THEN + RAISE EXCEPTION 'assertion_failed: write policies remain on subscriptions: %', v_policy; + END IF; + + IF EXISTS ( + SELECT 1 FROM pg_policies + WHERE schemaname = 'public' AND tablename = 'profiles' AND policyname = 'admin_update_profiles' + ) THEN + RAISE EXCEPTION 'assertion_failed: admin_update_profiles still exists'; + END IF; + + IF has_table_privilege('authenticated', 'public.subscriptions', 'INSERT') + OR has_table_privilege('authenticated', 'public.subscriptions', 'UPDATE') + OR has_table_privilege('authenticated', 'public.subscriptions', 'DELETE') + OR has_table_privilege('authenticated', 'public.subscriptions', 'TRUNCATE') + OR has_table_privilege('anon', 'public.subscriptions', 'UPDATE') THEN + RAISE EXCEPTION 'assertion_failed: client roles still hold write privileges on subscriptions'; + END IF; +END; +$$; + +-- Manager token: may read every subscription, may not write any. +SET LOCAL ROLE authenticated; +SELECT set_config('request.jwt.claim.sub', '42600000-0000-4000-8000-000000000001', true); +SELECT set_config( + 'request.jwt.claims', + '{"sub":"42600000-0000-4000-8000-000000000001","role":"authenticated","app_metadata":{"role":"manager"}}', + true +); + +DO $$ +DECLARE + v_rows integer; + v_payer text; +BEGIN + SELECT payple_payer_id INTO v_payer + FROM public.subscriptions WHERE user_id = '42600000-0000-4000-8000-000000000003'; + IF v_payer IS DISTINCT FROM 'legacy-policy-victim-payer' THEN + RAISE EXCEPTION 'assertion_failed: manager lost read access to subscriptions (got %)', v_payer; + END IF; + + BEGIN + UPDATE public.subscriptions + SET payple_payer_id = v_payer, tier = 'pro_plus', overage_credits = 1000000 + WHERE user_id = '42600000-0000-4000-8000-000000000004'; + GET DIAGNOSTICS v_rows = ROW_COUNT; + IF v_rows <> 0 THEN + RAISE EXCEPTION 'assertion_failed: manager direct UPDATE changed % subscription row(s)', v_rows; + END IF; + EXCEPTION WHEN insufficient_privilege THEN + NULL; + END; +END; +$$; + +-- Admin token: no direct UPDATE, INSERT or DELETE on subscriptions, and no +-- UPDATE of another user's profile. +SELECT set_config('request.jwt.claim.sub', '42600000-0000-4000-8000-000000000002', true); +SELECT set_config( + 'request.jwt.claims', + '{"sub":"42600000-0000-4000-8000-000000000002","role":"authenticated","app_metadata":{"role":"admin"}}', + true +); + +DO $$ +DECLARE + v_rows integer; +BEGIN + BEGIN + UPDATE public.subscriptions + SET tier = 'pro_plus', overage_credits = 1000000 + WHERE user_id = '42600000-0000-4000-8000-000000000004'; + GET DIAGNOSTICS v_rows = ROW_COUNT; + IF v_rows <> 0 THEN + RAISE EXCEPTION 'assertion_failed: admin direct UPDATE changed % subscription row(s)', v_rows; + END IF; + EXCEPTION WHEN insufficient_privilege THEN + NULL; + END; + + BEGIN + DELETE FROM public.subscriptions WHERE user_id = '42600000-0000-4000-8000-000000000004'; + GET DIAGNOSTICS v_rows = ROW_COUNT; + IF v_rows <> 0 THEN + RAISE EXCEPTION 'assertion_failed: admin direct DELETE removed % subscription row(s)', v_rows; + END IF; + EXCEPTION WHEN insufficient_privilege THEN + NULL; + END; + + BEGIN + INSERT INTO public.subscriptions (user_id, tier, status, provider, payment_provider) + VALUES ('42600000-0000-4000-8000-000000000005', 'pro_plus', 'active', 'none', 'none'); + RAISE EXCEPTION 'assertion_failed: admin direct INSERT created a subscription row'; + EXCEPTION WHEN insufficient_privilege THEN + NULL; + END; + + BEGIN + UPDATE public.profiles + SET name = 'Renamed By Admin' + WHERE id = '42600000-0000-4000-8000-000000000004'; + GET DIAGNOSTICS v_rows = ROW_COUNT; + IF v_rows <> 0 THEN + RAISE EXCEPTION 'assertion_failed: admin direct UPDATE changed % other profile row(s)', v_rows; + END IF; + EXCEPTION WHEN insufficient_privilege THEN + NULL; + END; +END; +$$; + +-- Ordinary user token: own-profile edits (profiles_update_own) still work. +SELECT set_config('request.jwt.claim.sub', '42600000-0000-4000-8000-000000000004', true); +SELECT set_config( + 'request.jwt.claims', + '{"sub":"42600000-0000-4000-8000-000000000004","role":"authenticated","app_metadata":{}}', + true +); + +DO $$ +DECLARE + v_rows integer; +BEGIN + UPDATE public.profiles SET name = 'Self Renamed' WHERE id = '42600000-0000-4000-8000-000000000004'; + GET DIAGNOSTICS v_rows = ROW_COUNT; + IF v_rows <> 1 THEN + RAISE EXCEPTION 'assertion_failed: user could not update own profile (% rows)', v_rows; + END IF; + + IF NOT EXISTS ( + SELECT 1 FROM public.subscriptions WHERE user_id = '42600000-0000-4000-8000-000000000004' + ) THEN + RAISE EXCEPTION 'assertion_failed: user lost read access to own subscription'; + END IF; +END; +$$; + +RESET ROLE; + +-- The service-role client (edge functions, payple-renew) still writes directly. +SET LOCAL ROLE service_role; + +DO $$ +DECLARE + v_rows integer; +BEGIN + UPDATE public.subscriptions + SET admin_note = 'service-role write' + WHERE user_id = '42600000-0000-4000-8000-000000000004'; + GET DIAGNOSTICS v_rows = ROW_COUNT; + IF v_rows <> 1 THEN + RAISE EXCEPTION 'assertion_failed: service_role direct UPDATE changed % rows', v_rows; + END IF; +END; +$$; + +RESET ROLE; + +-- Nothing the staff tokens attempted reached the rows. +DO $$ +DECLARE + v_row public.subscriptions%ROWTYPE; +BEGIN + SELECT * INTO v_row FROM public.subscriptions WHERE user_id = '42600000-0000-4000-8000-000000000004'; + IF v_row.user_id IS NULL THEN + RAISE EXCEPTION 'assertion_failed: target subscription was deleted'; + END IF; + IF v_row.tier <> 'free' + OR v_row.overage_credits <> 0 + OR v_row.payple_payer_id IS NOT NULL THEN + RAISE EXCEPTION 'assertion_failed: target subscription changed: %/%/%', + v_row.tier, v_row.overage_credits, v_row.payple_payer_id; + END IF; + + IF EXISTS ( + SELECT 1 FROM public.subscriptions WHERE user_id = '42600000-0000-4000-8000-000000000005' + ) THEN + RAISE EXCEPTION 'assertion_failed: admin INSERT created a subscription'; + END IF; + + IF (SELECT name FROM public.profiles WHERE id = '42600000-0000-4000-8000-000000000004') + IS DISTINCT FROM 'Self Renamed' THEN + RAISE EXCEPTION 'assertion_failed: target profile name unexpected'; + END IF; +END; +$$; + +ROLLBACK;