diff --git a/db/migrations/20260924200000_rechtepruefung_einmal_je_abfrage.sql b/db/migrations/20260924200000_rechtepruefung_einmal_je_abfrage.sql new file mode 100644 index 0000000..a092a01 --- /dev/null +++ b/db/migrations/20260924200000_rechtepruefung_einmal_je_abfrage.sql @@ -0,0 +1,141 @@ +-- Die Rechteprüfung in den Policies einmal je Abfrage statt einmal je Zeile. +-- +-- ═══ Der Befund ════════════════════════════════════════════════════ +-- +-- Die Personalakte brauchte beim Öffnen rund anderthalb Sekunden. Gemessen +-- im Container, nicht geschätzt: +-- +-- [Akte 47313676] person 4ms, hauptabfrage 1554ms, nachschlag 11ms +-- +-- Die Hauptabfrage ist es also, und darin om_reporting_lines(): +-- +-- als postgres (RLS wird übergangen) 60 ms +-- als alpenwerk_app (RLS in Kraft) 1457 ms +-- +-- Derselbe Aufschlag liegt auf allem anderen: ein blosses count(*) kostet +-- unter RLS 44 bis 55 ms statt 7. +-- +-- ═══ Die Ursache ═══════════════════════════════════════════════════ +-- +-- Ein Policy-Ausdruck wird **je Zeile** ausgewertet. `is_hr_user()` ist zwar +-- `stable`, aber das erlaubt Postgres nur, den Wert innerhalb einer Anweisung +-- als unveränderlich anzusehen — nicht, ihn aus dem Zeilenfilter +-- herauszuziehen. Bei jeder Zeile laufen also zwei Unterabfragen auf +-- `profiles` und `app_passwoerter`. +-- +-- In einen Unterausdruck gefasst wird daraus ein InitPlan, den der Planer +-- **einmal** auswertet. Gemessen, ohne RLS, damit nur dieser Unterschied +-- sichtbar ist: +-- +-- select count(*) from om_positions where is_hr_user(); 14,8 ms +-- select count(*) from om_positions where (select is_hr_user()); 0,5 ms +-- select count(*) from employees where is_hr_user(); 4,1 ms +-- select count(*) from employees where (select is_hr_user()); 0,5 ms +-- +-- ═══ Was sich dabei **nicht** ändert ═══════════════════════════════ +-- +-- Die Zugriffsentscheidung. Es ist dieselbe Funktion mit demselben Ergebnis; +-- sie wird nur nicht mehr achthundertmal gefragt, sondern einmal. Keine +-- Tabelle verliert ihre Policy, keine Rolle bekommt Rechte dazu, und +-- `alpenwerk_app` bleibt ohne BYPASSRLS. +-- +-- Der naheliegende andere Weg — om_reporting_lines auf `security definer` +-- umstellen, damit sie die Tabellen am Zeilenschutz vorbei liest — wäre +-- genau das nicht: er verschöbe die Grenze, um schneller zu werden, und +-- hälfe nur dieser einen Funktion. Der Aufschlag liegt aber auf jeder +-- Abfrage der Anwendung. +-- +-- ═══ Warum erzeugt und nicht abgeschrieben ═════════════════════════ +-- +-- Es sind rund zwanzig Policies. Eine davon von Hand falsch abzuschreiben +-- hiesse, eine Tabelle zu öffnen oder zu schliessen, ohne dass es auffällt. +-- Deshalb werden die bestehenden Definitionen aus `pg_policies` gelesen und +-- unverändert wieder angelegt — ersetzt wird allein der Funktionsaufruf. + +-- Erst sammeln, dann umbauen: ein Cursor über pg_policies, der während des +-- Laufs Policies austauscht, liest aus einem Katalog, den er selbst ändert. +create temp table policy_umbau on commit drop as + select tablename, policyname, permissive, cmd, + array_to_string(roles, ', ') as rollen, qual, with_check + from pg_policies + where schemaname = 'public' + and (coalesce(qual, '') || coalesce(with_check, '')) like '%is_hr_user()%' + and (coalesce(qual, '') || coalesce(with_check, '')) not like '%SELECT is_hr_user()%'; + +do $$ +declare + r record; + v_qual text; + v_check text; + v_sql text; + v_anzahl int := 0; +begin + for r in select * from policy_umbau order by tablename, policyname loop + -- Nur der Aufruf wird gefasst. app_current_user_id() steht in den + -- Policies der Entwürfe und Berichte neben is_hr_user() und wird aus + -- demselben Grund je Zeile gerufen. + v_qual := replace(replace(r.qual, 'is_hr_user()', '(select is_hr_user())'), + 'app_current_user_id()', '(select app_current_user_id())'); + v_check := replace(replace(r.with_check, 'is_hr_user()', '(select is_hr_user())'), + 'app_current_user_id()', '(select app_current_user_id())'); + + execute format('drop policy %I on public.%I', r.policyname, r.tablename); + + v_sql := format('create policy %I on public.%I as %s for %s to %s', + r.policyname, r.tablename, r.permissive, r.cmd, r.rollen); + -- Eine INSERT-Policy hat kein `using`, eine SELECT- und DELETE-Policy + -- kein `with check`. Beides anzugeben wäre ein Syntaxfehler. + if v_qual is not null then v_sql := v_sql || format(' using (%s)', v_qual); end if; + if v_check is not null then v_sql := v_sql || format(' with check (%s)', v_check); end if; + + execute v_sql; + v_anzahl := v_anzahl + 1; + end loop; + + raise notice 'Policies umgeschrieben: %', v_anzahl; +end +$$; + + +-- Selbstprüfung. +do $$ +declare + v_offen int; + v_vorher int; + v_jetzt int; +begin + -- Keine Policy darf den Aufruf noch ungefasst führen. + select count(*) into v_offen + from pg_policies + where schemaname = 'public' + and (coalesce(qual, '') || coalesce(with_check, '')) like '%is_hr_user()%' + and (coalesce(qual, '') || coalesce(with_check, '')) not like '%SELECT is_hr_user()%'; + if v_offen > 0 then + raise exception '% Policies pruefen die Rechte weiterhin je Zeile.', v_offen; + end if; + + -- Und keine darf dabei verlorengegangen sein. + select count(*) into v_vorher from policy_umbau; + select count(*) into v_jetzt + from pg_policies + where schemaname = 'public' + and (coalesce(qual, '') || coalesce(with_check, '')) like '%SELECT is_hr_user()%'; + if v_jetzt < v_vorher then + raise exception 'Vorher % Policies mit Rechtepruefung, jetzt nur noch % — es fehlt eine.', v_vorher, v_jetzt; + end if; + + -- Jede Tabelle, die vorher geschützt war, ist es weiterhin: Zeilenschutz + -- eingeschaltet und mindestens eine Policy darauf. + if exists ( + select 1 from policy_umbau u + where not exists ( + select 1 from pg_policies p + where p.schemaname = 'public' and p.tablename = u.tablename + ) + ) then + raise exception 'Eine Tabelle hat ihre Policies verloren.'; + end if; + + raise notice 'Geprueft: % Policies umgestellt, keine offen.', v_vorher; +end +$$;