Skip to content

Commit

Permalink
Make sure "schema permissions apply" also updates sequence access.
Browse files Browse the repository at this point in the history
This should fix pg_dump on our databases, which insists on reading the sequences too.
  • Loading branch information
vdboor committed Oct 1, 2024
1 parent 5a52e26 commit ebf05ce
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 9 deletions.
84 changes: 75 additions & 9 deletions src/schematools/permissions/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
$$
Expand Down Expand Up @@ -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('/', '_')}"
40 changes: 40 additions & 0 deletions tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,17 @@ 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",
"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_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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -381,13 +401,15 @@ 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",
"GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_bouwblokken TO write_gebieden",
"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(
Expand All @@ -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(
Expand All @@ -414,20 +437,23 @@ 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(
engine, "public", ams_schema, profiles, "level_a2", "LEVEL/A2", verbose=1
)
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",
"GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES ON TABLE public.gebieden_bouwblokken TO write_gebieden",
"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(
Expand All @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand Down

0 comments on commit ebf05ce

Please sign in to comment.