-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Privilege Grants to Roles and Groups in Materialization #626
base: main
Are you sure you want to change the base?
Changes from 47 commits
a1ec09c
a0ca37d
ca046c9
2bb40b8
f464d44
7b89a16
72a2410
0f6442e
bfc08f6
53cc135
b546431
87c491d
574193c
0aa8fb1
c0235ba
bdc6177
ead3a2e
0d51ab7
f691812
de7da53
3cee6e2
2d65c09
8ec3461
016e62c
fad2ef8
343565f
aeadcac
1da43db
cee0637
e3bb550
9fd05c1
767749b
a738720
082c590
66782c4
96fc734
4a28c99
1f2f9e0
53956b4
5901cf2
f37d91d
220c810
c56c61d
00231be
30043a7
12d339d
3aeff0d
c8e06f5
eb07cfb
655cb53
5247bab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
kind: Features | ||
body: 'Add support for specifying groups and roles in grant statements' | ||
time: 2023-10-04T10:39:38.680813-07:00 | ||
custom: | ||
Author: soksamnanglim | ||
Issue: "415" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -187,3 +187,93 @@ def generate_python_submission_response(self, submission_result: Any) -> Adapter | |
def debug_query(self): | ||
"""Override for DebugTask method""" | ||
self.execute("select 1 as id") | ||
|
||
# grant-related methods | ||
@available | ||
def standardize_grants_dict(self, grants_table): | ||
""" | ||
Override for standardize_grants_dict | ||
""" | ||
grants_dict = {} # Dict[str, Dict[str, List[str]]] | ||
for row in grants_table: | ||
grantee_type = row["grantee_type"] | ||
grantee = row["grantee"] | ||
privilege = row["privilege_type"] | ||
if privilege not in grants_dict: | ||
grants_dict[privilege] = {} | ||
|
||
if grantee_type not in grants_dict[privilege]: | ||
grants_dict[privilege][grantee_type] = [] | ||
|
||
grants_dict[privilege][grantee_type].append(grantee) | ||
|
||
return grants_dict | ||
|
||
@available | ||
def diff_of_two_nested_dicts(self, dict_a, dict_b): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of |
||
""" | ||
Given two dictionaries of type Dict[str, Dict[str, List[str]]]: | ||
dict_a = {'key_x': {'key_a': 'VALUE_1'}, 'KEY_Y': {'key_b': value_2'}} | ||
dict_b = {'key_x': {'key_a': 'VALUE_1'}, 'KEY_Y': {'key_b': value_2'}} | ||
Return the same dictionary representation of dict_a MINUS dict_b, | ||
performing a case-insensitive comparison between the strings in each. | ||
All keys returned will be in the original case of dict_a. | ||
returns {'key_x': ['VALUE_2'], 'KEY_Y': ['value_3']} | ||
""" | ||
|
||
dict_diff = {} | ||
|
||
for k, v_a in dict_a.items(): | ||
if k.casefold() in dict_b: | ||
v_b = dict_b[k.casefold()] | ||
|
||
for sub_key, values_a in v_a.items(): | ||
if sub_key in v_b: | ||
values_b = v_b[sub_key] | ||
diff_values = [v for v in values_a if v.casefold() not in values_b] | ||
if diff_values: | ||
if k in dict_diff: | ||
dict_diff[k][sub_key] = diff_values | ||
else: | ||
dict_diff[k] = {sub_key: diff_values} | ||
else: | ||
if k in dict_diff: | ||
if values_a: | ||
dict_diff[k][sub_key] = values_a | ||
else: | ||
if values_a: | ||
dict_diff[k] = {sub_key: values_a} | ||
else: | ||
dict_diff[k] = v_a | ||
|
||
return dict_diff | ||
|
||
@available | ||
def process_grant_dicts(self, unknown_dict): | ||
""" | ||
Given a dictionary where the type can either be of type: | ||
- Dict[str, List[str]] | ||
- Dict[str, List[Dict[str, List[str]] | ||
Return a processed dictionary of the type Dict[str, Dict[str, List[str]] | ||
""" | ||
first_value = next(iter(unknown_dict.values())) | ||
if first_value: | ||
is_dict = isinstance(first_value[0], dict) | ||
else: | ||
is_dict = False | ||
|
||
temp = {} | ||
if not is_dict: | ||
for privilege, grantees in unknown_dict.items(): | ||
if grantees: | ||
temp[privilege] = {"user": grantees} | ||
else: | ||
for privilege, grantee_map in unknown_dict.items(): | ||
grantees_map_temp = {} | ||
for grantee_type, grantees in grantee_map[0].items(): | ||
if grantees: | ||
grantees_map_temp[grantee_type] = grantees | ||
if grantees_map_temp: | ||
temp[privilege] = grantees_map_temp | ||
|
||
return temp |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,42 @@ | ||||||||||
{% macro redshift__get_show_grant_sql(relation) %} | ||||||||||
{# ------- DCL STATEMENT TEMPLATES ------- #} | ||||||||||
|
||||||||||
{%- macro redshift__get_grant_sql(relation, privilege, grantee_dict) -%} | ||||||||||
{#-- generates a multiple-grantees grant privilege statement --#} | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overriding |
||||||||||
grant {{privilege}} on {{relation}} to | ||||||||||
{%- for grantee_type, grantees in grantee_dict.items() -%} | ||||||||||
{%- if grantee_type == 'user' and grantees -%} | ||||||||||
{{ " " + (grantees | join(', ')) }} | ||||||||||
{%- elif grantee_type == 'group' and grantees -%} | ||||||||||
{{ " " + ("group " + grantees | join(', group ')) }} | ||||||||||
{%- elif grantee_type == 'role' and grantees -%} | ||||||||||
{{ " " + ("role " + grantees | join(', role ')) }} | ||||||||||
{%- endif -%} | ||||||||||
{%- if not loop.last -%} | ||||||||||
, | ||||||||||
{%- endif -%} | ||||||||||
{%- endfor -%} | ||||||||||
{%- endmacro -%} | ||||||||||
|
||||||||||
{%- macro redshift__get_revoke_sql(relation, privilege, revokee_dict) -%} | ||||||||||
{#-- generates a multiple-grantees revoke privilege statement --#} | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overriding |
||||||||||
revoke {{privilege}} on {{relation}} from | ||||||||||
{%- for revokee_type, revokees in revokee_dict.items() -%} | ||||||||||
{%- if revokee_type == 'user' and revokees -%} | ||||||||||
{{ " " + (revokees | join(', ')) }} | ||||||||||
{%- elif revokee_type == 'group' and revokees -%} | ||||||||||
{{ " " + ("group " + revokees | join(', group ')) }} | ||||||||||
{%- elif revokee_type == 'role' and revokees -%} | ||||||||||
{{ " " + ("role " + revokees | join(', role ')) }} | ||||||||||
{%- endif -%} | ||||||||||
{%- if not loop.last -%} | ||||||||||
, | ||||||||||
{%- endif -%} | ||||||||||
{%- endfor -%} | ||||||||||
{%- endmacro -%} | ||||||||||
|
||||||||||
|
||||||||||
{% macro redshift__get_show_grant_sql(relation) %} | ||||||||||
{#-- shows the privilege grants on a table for users, groups, and roles --#} | ||||||||||
with privileges as ( | ||||||||||
|
||||||||||
-- valid options per https://docs.aws.amazon.com/redshift/latest/dg/r_HAS_TABLE_PRIVILEGE.html | ||||||||||
|
@@ -16,6 +53,7 @@ with privileges as ( | |||||||||
) | ||||||||||
|
||||||||||
select | ||||||||||
'user' as grantee_type, | ||||||||||
u.usename as grantee, | ||||||||||
p.privilege_type | ||||||||||
from pg_user u | ||||||||||
|
@@ -24,4 +62,72 @@ where has_table_privilege(u.usename, '{{ relation }}', privilege_type) | |||||||||
and u.usename != current_user | ||||||||||
and not u.usesuper | ||||||||||
|
||||||||||
union all | ||||||||||
-- check that group has table privilege | ||||||||||
select | ||||||||||
'group' as grantee_type, | ||||||||||
g.groname as grantee, | ||||||||||
p.privilege_type | ||||||||||
from pg_group g | ||||||||||
cross join privileges p | ||||||||||
where exists( | ||||||||||
select * | ||||||||||
from information_schema.table_privileges tp | ||||||||||
where tp.grantee=g.groname | ||||||||||
and tp.table_schema=replace(split_part('{{ relation }}', '.', 2), '"', '') | ||||||||||
and tp.table_name=replace(split_part('{{ relation }}', '.', 3), '"', '') | ||||||||||
and LOWER(tp.privilege_type)=p.privilege_type | ||||||||||
Comment on lines
+77
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use
Suggested change
|
||||||||||
) | ||||||||||
|
||||||||||
union all | ||||||||||
-- check that role has table privilege | ||||||||||
select | ||||||||||
'role' as grantee_type, | ||||||||||
r.role_name as grantee, | ||||||||||
p.privilege_type | ||||||||||
from svv_roles r | ||||||||||
cross join privileges p | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From pairing with @dbeatty10 - in order to query
References: |
||||||||||
where exists( | ||||||||||
select * | ||||||||||
from svv_relation_privileges rp | ||||||||||
where rp.identity_name=r.role_name | ||||||||||
and rp.namespace_name=replace(split_part('{{ relation }}', '.', 2), '"', '') | ||||||||||
and rp.relation_name=replace(split_part('{{ relation }}', '.', 3), '"', '') | ||||||||||
and LOWER(rp.privilege_type)=p.privilege_type | ||||||||||
Comment on lines
+94
to
+95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use
Suggested change
|
||||||||||
) | ||||||||||
|
||||||||||
{% endmacro %} | ||||||||||
|
||||||||||
{% macro redshift__apply_grants(relation, grant_config, should_revoke=True) %} | ||||||||||
{#-- Override for apply grants --#} | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overriding |
||||||||||
{#-- If grant_config is {} or None, this is a no-op --#} | ||||||||||
{% if grant_config %} | ||||||||||
{#-- change grant_config to Dict[str, Dict[str, List[str]] format --#} | ||||||||||
{% set grant_config = adapter.process_grant_dicts(grant_config) %} | ||||||||||
|
||||||||||
{% if should_revoke %} | ||||||||||
{#-- We think that there is a chance that grants are carried over. --#} | ||||||||||
{#-- Show the current grants for users, roles, and groups and calculate the diffs. --#} | ||||||||||
{% set current_grants_table = run_query(get_show_grant_sql(relation)) %} | ||||||||||
{% set current_grants_dict = adapter.standardize_grants_dict(current_grants_table) %} | ||||||||||
{% set needs_granting = adapter.diff_of_two_nested_dicts(grant_config, current_grants_dict) %} | ||||||||||
{% set needs_revoking = adapter.diff_of_two_nested_dicts(current_grants_dict, grant_config) %} | ||||||||||
{% if not (needs_granting or needs_revoking) %} | ||||||||||
{{ log('On ' ~ relation ~': All grants are in place, no revocation or granting needed.')}} | ||||||||||
{% endif %} | ||||||||||
{% else %} | ||||||||||
{#-- We don't think there's any chance of previous grants having carried over. --#} | ||||||||||
{#-- Jump straight to granting what the user has configured. --#} | ||||||||||
{% set needs_revoking = {} %} | ||||||||||
{% set needs_granting = grant_config %} | ||||||||||
{% endif %} | ||||||||||
{% if needs_granting or needs_revoking %} | ||||||||||
{% set revoke_statement_list = get_dcl_statement_list(relation, needs_revoking, get_revoke_sql) %} | ||||||||||
{% set grant_statement_list = get_dcl_statement_list(relation, needs_granting, get_grant_sql) %} | ||||||||||
{% set dcl_statement_list = revoke_statement_list + grant_statement_list %} | ||||||||||
{% if dcl_statement_list %} | ||||||||||
{{ call_dcl_statements(dcl_statement_list) }} | ||||||||||
{% endif %} | ||||||||||
{% endif %} | ||||||||||
{% endif %} | ||||||||||
{% endmacro %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
import pytest | ||
import os | ||
|
||
from dbt.tests.util import ( | ||
relation_from_name, | ||
get_connection, | ||
) | ||
|
||
TEST_USER_ENV_VARS = ["DBT_TEST_USER_1", "DBT_TEST_USER_2", "DBT_TEST_USER_3"] | ||
TEST_GROUP_ENV_VARS = ["DBT_TEST_GROUP_1", "DBT_TEST_GROUP_2", "DBT_TEST_GROUP_3"] | ||
TEST_ROLE_ENV_VARS = ["DBT_TEST_ROLE_1", "DBT_TEST_ROLE_2", "DBT_TEST_ROLE_3"] | ||
|
||
|
||
def replace_all(text, dic): | ||
for i, j in dic.items(): | ||
text = text.replace(i, j) | ||
return text | ||
|
||
|
||
class BaseGrantsRedshift: | ||
def privilege_grantee_name_overrides(self): | ||
# these privilege and grantee names are valid on most databases, but not all! | ||
# looking at you, BigQuery | ||
# optionally use this to map from "select" --> "other_select_name", "insert" --> ... | ||
return { | ||
"select": "select", | ||
"insert": "insert", | ||
"fake_privilege": "fake_privilege", | ||
"invalid_user": "invalid_user", | ||
} | ||
|
||
def interpolate_name_overrides(self, yaml_text): | ||
return replace_all(yaml_text, self.privilege_grantee_name_overrides()) | ||
|
||
@pytest.fixture(scope="class", autouse=True) | ||
def get_test_groups(self, project): | ||
test_groups = [] | ||
for env_var in TEST_GROUP_ENV_VARS: | ||
group_name = os.getenv(env_var) | ||
if group_name: | ||
test_groups.append(group_name) | ||
return test_groups | ||
|
||
@pytest.fixture(scope="class", autouse=True) | ||
def get_test_roles(self, project): | ||
test_roles = [] | ||
for env_var in TEST_ROLE_ENV_VARS: | ||
role_name = os.getenv(env_var) | ||
if role_name: | ||
test_roles.append(role_name) | ||
return test_roles | ||
|
||
@pytest.fixture(scope="class", autouse=True) | ||
def get_test_users(self, project): | ||
test_users = [] | ||
for env_var in TEST_USER_ENV_VARS: | ||
user_name = os.getenv(env_var) | ||
if user_name: | ||
test_users.append(user_name) | ||
return test_users | ||
|
||
def get_grants_on_relation(self, project, relation_name): | ||
relation = relation_from_name(project.adapter, relation_name) | ||
adapter = project.adapter | ||
with get_connection(adapter): | ||
kwargs = {"relation": relation} | ||
show_grant_sql = adapter.execute_macro("get_show_grant_sql", kwargs=kwargs) | ||
_, grant_table = adapter.execute(show_grant_sql, fetch=True) | ||
actual_grants = adapter.standardize_grants_dict(grant_table) | ||
return actual_grants | ||
|
||
# This is an override of the BaseGrants class | ||
def assert_expected_grants_match_actual(self, project, actual_grants, expected_grants): | ||
adapter = project.adapter | ||
# need a case-insensitive comparison | ||
# so just a simple "assert expected == actual_grants" won't work | ||
diff_a = adapter.diff_of_two_nested_dicts(actual_grants, expected_grants) | ||
diff_b = adapter.diff_of_two_nested_dicts(expected_grants, actual_grants) | ||
assert diff_a == diff_b == {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overriding
standardize_grants_dict
.