From 6d2a007c8606487825f495521ee44238ed2838e5 Mon Sep 17 00:00:00 2001 From: Diederik van der Boor Date: Thu, 26 Sep 2024 14:21:37 +0200 Subject: [PATCH] Make sure "schema permissions apply" also updates sequence access. This should fix pg_dump on our databases, which insists on reading the sequences too. --- src/schematools/permissions/db.py | 84 +++++++++++++++++++++++++++---- tests/test_permissions.py | 40 +++++++++++++++ 2 files changed, 115 insertions(+), 9 deletions(-) diff --git a/src/schematools/permissions/db.py b/src/schematools/permissions/db.py index 11f053c4..3404f838 100644 --- a/src/schematools/permissions/db.py +++ b/src/schematools/permissions/db.py @@ -19,7 +19,8 @@ # configure the logger, if needed. logger = logging.getLogger(__name__) -existing_roles = set() +existing_roles = set() # note: used as global cache! +existing_sequences = {} # def is_remote(table_name: str) -> bool: @@ -192,6 +193,22 @@ def set_dataset_write_permissions( echo=echo, dry_run=dry_run, ) + if table.is_autoincrement: + # Get PostgreSQL generated sequence for 'id' column that Django added. + sequence_name = _get_sequence_name(session, table_name, "id") + _execute_grant( + session, + grant( + ["USAGE"], + PgObjectType.SEQUENCE, + sequence_name, + grantee, + grant_option=False, + schema=pg_schema, + ), + echo=echo, + dry_run=dry_run, + ) @dataclass @@ -207,7 +224,9 @@ class GrantParam: grantees: list[str] -def get_all_dataset_scopes(ams_schema: DatasetSchema, role: str, scope: str) -> list[GrantParam]: +def get_all_dataset_scopes( + session, ams_schema: DatasetSchema, role: str, scope: str +) -> list[GrantParam]: """Returns all scopes that should be applied to the tables of a dataset. Args: @@ -283,25 +302,49 @@ def _fetch_grantees(scopes: frozenset[str]) -> list[str]: continue column_name = field.db_name + grantees = _fetch_grantees(column_scopes.get(column_name, table_scopes)) all_scopes.append( GrantParam( # NB. space after SELECT is significant! privileges=[f"SELECT ({column_name})"], target_type=PgObjectType.TABLE, target=table_name, - grantees=_fetch_grantees(column_scopes.get(column_name, table_scopes)), + grantees=grantees, ) ) + if field.is_primary and table.is_autoincrement: + # Get PostgreSQL generated sequence for 'id' column that Django added. + sequence_name = _get_sequence_name(session, table_name, "id") + all_scopes.append( + GrantParam( + privileges=["SELECT"], + target_type=PgObjectType.SEQUENCE, + target=sequence_name, + grantees=grantees, + ) + ) else: if table_name not in all_scopes: + grantees = _fetch_grantees(table_scopes) all_scopes.append( GrantParam( privileges=["SELECT"], target_type=PgObjectType.TABLE, target=table_name, - grantees=_fetch_grantees(table_scopes), + grantees=grantees, ) ) + if table.is_autoincrement: + sequence_name = _get_sequence_name(session, table_name, "id") + all_scopes.append( + GrantParam( + privileges=["SELECT"], + target_type=PgObjectType.SEQUENCE, + target=sequence_name, + grantees=grantees, + ) + ) + return all_scopes @@ -347,7 +390,7 @@ def set_dataset_read_permissions( if create_roles and grantee: _create_role_if_not_exists(session, grantee, dry_run=dry_run) - all_grants = get_all_dataset_scopes(ams_schema, role, scope) + all_grants = get_all_dataset_scopes(session, ams_schema, role, scope) for grant_param in all_grants: # For global and specific columns: for _grantee in grant_param.grantees: @@ -538,11 +581,19 @@ def _revoke_all_privileges_from_read_and_write_roles( revoke_statements.append( f"REVOKE ALL PRIVILEGES ON {pg_schema}.{table.db_name} FROM {rolname[0]}" ) - revoke_statement = ";".join(revoke_statements) + if table.is_autoincrement: + sequence_name = _get_sequence_name(session, table.db_name, "id") + revoke_statements.append( + f"REVOKE ALL PRIVILEGES ON SEQUENCE {pg_schema}.{sequence_name}" + f" FROM {rolname[0]}" + ) else: - revoke_statement = ( - f"REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA {pg_schema} FROM {rolname[0]}" - ) + revoke_statements = [ + f"REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA {pg_schema} FROM {rolname[0]}", + f"REVOKE ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA {pg_schema} FROM {rolname[0]}", + ] + + revoke_statement = ";".join(revoke_statements) revoke_block_statement = f""" DO $$ @@ -612,6 +663,21 @@ def _create_role_if_not_exists( existing_roles.add(role) +def _get_sequence_name(session: Session, table_name: str, column: str) -> str | None: + key = (table_name, column) + try: + # Can't use lru_cache() as it caches 'session' too. + return existing_sequences[key] + except KeyError: + row = session.execute( + text("SELECT pg_get_serial_sequence(:table, :column)"), + {"table": table_name, "column": column}, + ).first() + value = row[0].replace("public.", "").replace('"', "") if row is not None else None + existing_sequences[key] = value + return value + + def scope_to_role(scope: str) -> str: """Return rolename for the postgres database.""" return f"scope_{scope.lower().replace('/', '_')}" diff --git a/tests/test_permissions.py b/tests/test_permissions.py index a3579340..46f724fd 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -97,6 +97,7 @@ def test_nm_relations_permissions( "GRANT SELECT (soort_cultuur_onbebouwd_omschrijving) ON TABLE public.brk_kadastraleobjecten TO brk_ro", "GRANT SELECT (soort_grootte) ON TABLE public.brk_kadastraleobjecten TO brk_rsn", "GRANT SELECT (volgnummer) ON TABLE public.brk_kadastraleobjecten TO brk_rsn", + "GRANT SELECT ON SEQUENCE public.brk_kadastraleobjecten_is_ontstaan_uit_kadastraalobject_id_seq TO brk_rsn", "GRANT SELECT ON TABLE public.brk_kadastraleobjecten_is_ontstaan_uit_kadastraalobject TO brk_rsn", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.brk_kadastraleobjecten TO write_brk", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.brk_kadastraleobjecten TO write_brk", @@ -104,6 +105,9 @@ def test_nm_relations_permissions( "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.brk_kadastraleobjecten_is_ontstaan_uit_kadastraalobject TO write_brk", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.brk_kadastraleobjecten_is_ontstaan_uit_kadastraalobject TO write_brk", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.brk_kadastraleobjecten_is_ontstaan_uit_kadastraalobject TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_kadastraleobjecten_is_ontstaan_uit_kadastraalobject_id_seq TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_kadastraleobjecten_is_ontstaan_uit_kadastraalobject_id_seq TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_kadastraleobjecten_is_ontstaan_uit_kadastraalobject_id_seq TO write_brk", ] # table denied @@ -160,6 +164,14 @@ def test_brk_permissions( grants = _filter_grant_statements(caplog) assert grants == [ + "GRANT SELECT ON SEQUENCE public.brk_aantekeningenkadastraleobjecten_heeft_betrokken_pers_id_seq TO scope_brk_rsn", + "GRANT SELECT ON SEQUENCE public.brk_aantekeningenrechten_heeft_betrokken_persoon_id_seq TO scope_brk_rsn", + "GRANT SELECT ON SEQUENCE public.brk_aantekeningenrechten_is_gbsd_op_sdl_id_seq TO scope_brk_rsn", + "GRANT SELECT ON SEQUENCE public.brk_kadastraleobjecten_hft_rel_mt_vot_id_seq TO scope_brk_rsn", + "GRANT SELECT ON SEQUENCE public.brk_kadastraleobjecten_soort_cultuur_bebouwd_id_seq TO scope_brk_rsn", + "GRANT SELECT ON SEQUENCE public.brk_stukdelen_is_bron_voor_aantekening_kadastraal_object_id_seq TO scope_brk_rsn", + "GRANT SELECT ON SEQUENCE public.brk_stukdelen_is_bron_voor_aantekening_recht_id_seq TO scope_brk_rsn", + "GRANT SELECT ON SEQUENCE public.brk_stukdelen_is_bron_voor_zakelijk_recht_id_seq TO scope_brk_rsn", "GRANT SELECT ON TABLE public.brk_aantekeningenkadastraleobjecten TO scope_brk_rsn", "GRANT SELECT ON TABLE public.brk_aantekeningenkadastraleobjecten_heeft_betrokken_persoon TO scope_brk_rsn", "GRANT SELECT ON TABLE public.brk_aantekeningenrechten TO scope_brk_rsn", @@ -202,6 +214,14 @@ def test_brk_permissions( "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.brk_stukdelen_is_bron_voor_zakelijk_recht TO write_brk", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.brk_tenaamstellingen TO write_brk", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.brk_zakelijkerechten TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_aantekeningenkadastraleobjecten_heeft_betrokken_pers_id_seq TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_aantekeningenrechten_heeft_betrokken_persoon_id_seq TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_aantekeningenrechten_is_gbsd_op_sdl_id_seq TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_kadastraleobjecten_hft_rel_mt_vot_id_seq TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_kadastraleobjecten_soort_cultuur_bebouwd_id_seq TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_stukdelen_is_bron_voor_aantekening_kadastraal_object_id_seq TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_stukdelen_is_bron_voor_aantekening_recht_id_seq TO write_brk", + "GRANT USAGE ON SEQUENCE public.brk_stukdelen_is_bron_voor_zakelijk_recht_id_seq TO write_brk", ] def test_openbaar_permissions(self, here, engine, afval_schema, dbsession, caplog): @@ -381,6 +401,7 @@ def test_auth_list_permissions( ) grants = _filter_grant_statements(caplog) assert grants == [ + "GRANT SELECT ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO level_a1", "GRANT SELECT ON TABLE public.gebieden_buurten TO level_a1", "GRANT SELECT ON TABLE public.gebieden_buurten_ligt_in_wijk TO level_a1", "GRANT SELECT ON TABLE public.gebieden_wijken TO level_a1", @@ -388,6 +409,7 @@ def test_auth_list_permissions( "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten_ligt_in_wijk TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_wijken TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO write_gebieden", ] apply_schema_and_profile_permissions( @@ -402,6 +424,7 @@ def test_auth_list_permissions( "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten_ligt_in_wijk TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_wijken TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO write_gebieden", ] apply_schema_and_profile_permissions( @@ -414,6 +437,7 @@ def test_auth_list_permissions( "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten_ligt_in_wijk TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_wijken TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO write_gebieden", ] apply_schema_and_profile_permissions( @@ -421,6 +445,7 @@ def test_auth_list_permissions( ) grants = _filter_grant_statements(caplog) assert grants == [ + "GRANT SELECT ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO level_a2", "GRANT SELECT ON TABLE public.gebieden_buurten TO level_a2", "GRANT SELECT ON TABLE public.gebieden_buurten_ligt_in_wijk TO level_a2", "GRANT SELECT ON TABLE public.gebieden_wijken TO level_a2", @@ -428,6 +453,7 @@ def test_auth_list_permissions( "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten_ligt_in_wijk TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_wijken TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO write_gebieden", ] apply_schema_and_profile_permissions( @@ -442,6 +468,7 @@ def test_auth_list_permissions( "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten_ligt_in_wijk TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_wijken TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO write_gebieden", ] apply_schema_and_profile_permissions( @@ -454,6 +481,7 @@ def test_auth_list_permissions( "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_buurten_ligt_in_wijk TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_wijken TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO write_gebieden", ] # Check if the read priviliges are correct @@ -555,6 +583,10 @@ def test_auto_create_roles(self, here, engine, gebieden_schema_auth, dbsession, "GRANT SELECT (ligt_in_buurt_loose_id) ON TABLE public.gebieden_bouwblokken TO scope_level_d", "GRANT SELECT (ligt_in_buurt_volgnummer) ON TABLE public.gebieden_bouwblokken TO scope_level_d", "GRANT SELECT (volgnummer) ON TABLE public.gebieden_ggwgebieden TO scope_level_a", + "GRANT SELECT ON SEQUENCE public.gebieden_bouwblokken_ligt_in_buurt_id_seq TO scope_level_d", + "GRANT SELECT ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO scope_level_a", + "GRANT SELECT ON SEQUENCE public.gebieden_ggwgebieden_bestaat_uit_buurten_id_seq TO scope_level_e", + "GRANT SELECT ON SEQUENCE public.gebieden_ggwgebieden_gebieds_grenzen_id_seq TO scope_level_f", "GRANT SELECT ON TABLE public.gebieden_bouwblokken_ligt_in_buurt TO scope_level_d", "GRANT SELECT ON TABLE public.gebieden_buurten TO scope_level_a", "GRANT SELECT ON TABLE public.gebieden_buurten_ligt_in_wijk TO scope_level_a", @@ -569,6 +601,10 @@ def test_auto_create_roles(self, here, engine, gebieden_schema_auth, dbsession, "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_ggwgebieden_bestaat_uit_buurten TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_ggwgebieden_gebieds_grenzen TO write_gebieden", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_wijken TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_bouwblokken_ligt_in_buurt_id_seq TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_buurten_ligt_in_wijk_id_seq TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_ggwgebieden_bestaat_uit_buurten_id_seq TO write_gebieden", + "GRANT USAGE ON SEQUENCE public.gebieden_ggwgebieden_gebieds_grenzen_id_seq TO write_gebieden", ] # Check if roles exist and the read priviliges are correct @@ -917,6 +953,8 @@ def test_setting_additional_grants(self, here, engine, meetbouten_schema, dbsess grants = _filter_grant_statements(caplog) assert grants == [ + "GRANT SELECT ON SEQUENCE public.meetbouten_meetbouten_ligt_in_buurt_id_seq TO scope_openbaar", + "GRANT SELECT ON SEQUENCE public.meetbouten_metingen_refereertaanreferentiepunten_id_seq TO scope_openbaar", "GRANT SELECT ON TABLE public.datasets_dataset TO scope_openbaar", "GRANT SELECT ON TABLE public.meetbouten_meetbouten TO scope_openbaar", "GRANT SELECT ON TABLE public.meetbouten_meetbouten_ligt_in_buurt TO scope_openbaar", @@ -928,6 +966,8 @@ def test_setting_additional_grants(self, here, engine, meetbouten_schema, dbsess "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.meetbouten_metingen TO write_meetbouten", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.meetbouten_metingen_refereertaanreferentiepunten TO write_meetbouten", "GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.meetbouten_referentiepunten TO write_meetbouten", + "GRANT USAGE ON SEQUENCE public.meetbouten_meetbouten_ligt_in_buurt_id_seq TO write_meetbouten", + "GRANT USAGE ON SEQUENCE public.meetbouten_metingen_refereertaanreferentiepunten_id_seq TO write_meetbouten", ] # Check perms on the datasets_dataset table