Skip to content
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

Metadata form 7: Access control and deletion behaviour #1540

Merged
merged 29 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
719d86a
when entity deleted, all attached MFs are deleted
Sep 11, 2024
9c83eda
when env/org is deleted, all MFs with visibility inside this env/org …
Sep 11, 2024
232cf67
check resource permissions when create/attach/delete MF
Sep 11, 2024
1ec4b91
migration to backfill MF resource permissions
Sep 11, 2024
3d5242d
migrations fix
Sep 12, 2024
1cdaa06
only owners of the orgs/envs have permissions for MF
Sep 12, 2024
e267e5c
add resource policies for MFs to ENV, ORG and DATASET policy lists
Sep 12, 2024
84250f8
frontend canEdit depends on resource policies
Sep 12, 2024
84a8067
linting
Sep 12, 2024
96828ff
PR comments
Sep 13, 2024
dabc41c
instead of connecting core and modules via logic -- DB triggers
Sep 13, 2024
9645643
PR changes
Sep 13, 2024
f63d539
revert get_resource_policy_permissions change
Sep 13, 2024
52d4dde
Merge branch 'mda-main' into metadata-form-7
Sep 16, 2024
cb91ae8
share improvements and bugfixes
Sep 16, 2024
3c6926c
Merge branch 'share-ram-fix' into metadata-form-7
Sep 16, 2024
d04d005
or replace
Sep 19, 2024
ea578df
another trigger
Sep 20, 2024
82404de
backfill MF permission downgrade
Sep 20, 2024
0642d69
remove unused
Sep 20, 2024
0fbccd9
SamlAdminGroupName
Sep 20, 2024
9b1153f
check ATTACH_METADATA_FORM via decorator
Sep 23, 2024
1c6ef9c
no more arrays set to None
Sep 23, 2024
7a04e7d
ruff
Sep 23, 2024
a2f4cee
problem queries
Sep 24, 2024
5719483
fix downgrade
Sep 24, 2024
7b6023d
fix downgrade for redshift
Sep 24, 2024
0e23814
permision => resource_pol_permission.permission
Sep 26, 2024
cf5b538
Merge branch 'mda-main' into metadata-form-7
Sep 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
noah-paige marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ def query_user_metadata_forms(session, is_da_admin, groups, env_uris, org_uris,
:param filter:
"""

env_uris = env_uris or []
org_uris = org_uris or []

query = session.query(MetadataForm)

if not is_da_admin:
Expand Down Expand Up @@ -140,6 +143,8 @@ def query_entity_metadata_forms(

entity_orgs_uris = entity_orgs_uris or []
entity_envs_uris = entity_envs_uris or []
user_org_uris = user_org_uris or []
user_env_uris = user_env_uris or []
noah-paige marked this conversation as resolved.
Show resolved Hide resolved

orgs = list(set(user_org_uris).intersection(set(entity_orgs_uris)))
envs = list(set(user_env_uris).intersection(set(entity_envs_uris)))
Expand Down Expand Up @@ -253,13 +258,3 @@ def query_all_attached_metadata_forms_for_entity(session, entityUri, entityType)
return session.query(AttachedMetadataForm).filter(
and_(AttachedMetadataForm.entityType == entityType, AttachedMetadataForm.entityUri == entityUri)
)

@staticmethod
def delete_attached_entity_metadata_forms(session, entityUri, entityType):
MetadataFormRepository.query_all_attached_metadata_forms_for_entity(session, entityUri, entityType).delete()

@staticmethod
def delete_all_home_metadata_forms(session, homeEntityUri, visibility):
session.query(MetadataForm).filter(
and_(MetadataForm.homeEntity == homeEntityUri, MetadataForm.visibility == visibility)
).delete()
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ def validate_enrich_fields_params(mf_fields, data):

class AttachedMetadataFormService:
@staticmethod
def _get_entity_uri(data):
return data.get('entityUri')

@staticmethod
@ResourcePolicyService.has_resource_permission(
ATTACH_METADATA_FORM, parent_resource=_get_entity_uri, param_name='data'
)
def create_attached_metadata_form(uri, data):
AttachedMetadataFormValidationService.validate_filled_form_params(uri, data)
context = get_context()
Expand All @@ -41,13 +48,6 @@ def create_attached_metadata_form(uri, data):
raise exceptions.ObjectNotFound('MetadataForm', uri)
mf_fields = MetadataFormRepository.get_metadata_form_fields(session, uri)
AttachedMetadataFormValidationService.validate_enrich_fields_params(mf_fields, data)
ResourcePolicyService.check_user_resource_permission(
session=session,
username=context.username,
groups=context.groups,
resource_uri=data.get('entityUri'),
permission_name=ATTACH_METADATA_FORM,
)
amf = MetadataFormRepository.create_attached_metadata_form(session, uri, data)
for f in data.get('fields'):
MetadataFormRepository.create_attached_metadata_form_field(
Expand Down Expand Up @@ -80,15 +80,11 @@ def list_attached_forms(filter=None):
).to_dict()

@staticmethod
@ResourcePolicyService.has_resource_permission(
ATTACH_METADATA_FORM, parent_resource=_get_entity_uri, param_name='data'
)
def delete_attached_metadata_form(uri):
mf = AttachedMetadataFormService.get_attached_metadata_form(uri)
context = get_context()
with context.db_engine.scoped_session() as session:
ResourcePolicyService.check_user_resource_permission(
session=session,
username=context.username,
groups=context.groups,
resource_uri=mf.entityUri,
permission_name=ATTACH_METADATA_FORM, # attach and delete are the same for now
)
return session.delete(mf)
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@

logger = logging.getLogger(__name__)
ACCESS_POINT_CREATION_TIME = 30
ACCESS_POINT_CREATION_RETRIES = 5
ACCESS_POINT_CREATION_RETRIES = 10
ACCESS_POINT_BACKOFF_COEFFICIENT = 1.1 # every time increase retry delay by 10%


class S3AccessPointShareManager:
Expand Down Expand Up @@ -447,12 +448,14 @@ def manage_access_point_and_policy(self):
access_point_arn = s3_client.create_bucket_access_point(self.bucket_name, self.access_point_name)
# Access point creation is slow
retries = 1
sleep_coeff = 1
while (
not s3_client.get_bucket_access_point_arn(self.access_point_name)
and retries < ACCESS_POINT_CREATION_RETRIES
):
logger.info('Waiting 30s for access point creation to complete..')
time.sleep(ACCESS_POINT_CREATION_TIME)
time.sleep(ACCESS_POINT_CREATION_TIME * sleep_coeff)
sleep_coeff = sleep_coeff * ACCESS_POINT_BACKOFF_COEFFICIENT
retries += 1
existing_policy = s3_client.get_access_point_policy(self.access_point_name)
# requester will use this role to access resources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def process_approved_shares(self) -> bool:
manager.grant_principals_permissions_to_source_table(table, share_item, share_item_filter)
if manager.cross_account:
retries = 0
retry_share_table = False
retry_share_table = True
while retry_share_table and retries < 1:
(
retry_share_table,
Expand Down
29 changes: 29 additions & 0 deletions backend/migrations/versions/075d344ae2cc_mf_triggers.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,25 @@ def upgrade():
FOR EACH ROW
EXECUTE FUNCTION dataset_delete_trigger_function();
""")

op.execute("""
CREATE OR REPLACE FUNCTION metadata_form_delete_trigger_function()
RETURNS TRIGGER AS $$
BEGIN
-- Delete from resource_permission_policy
DELETE FROM resource_policy_permission
WHERE "sid" in (SELECT sid from resource_policy where "resourceUri"=OLD.uri and "resourceType"='MetadataForm');
DELETE FROM resource_policy where "resourceUri"=OLD.uri;
RETURN OLD;
END;
$$ LANGUAGE plpgsql;

-- Create the trigger for dataset table
CREATE TRIGGER metadata_form_delete_trigger
BEFORE DELETE ON metadata_form
FOR EACH ROW
EXECUTE FUNCTION metadata_form_delete_trigger_function();
""")
noah-paige marked this conversation as resolved.
Show resolved Hide resolved
# ### end Alembic commands ###


Expand Down Expand Up @@ -121,4 +140,14 @@ def downgrade():
DROP FUNCTION IF EXISTS dataset_delete_trigger_function;
"""
)

op.execute(
"""
-- Drop the dataset_delete_trigger
DROP TRIGGER IF EXISTS metadata_form_delete_trigger ON metadata_form;

-- Drop the dataset_delete_trigger_function
DROP FUNCTION IF EXISTS metadata_form_delete_trigger_function;
"""
)
# ### end Alembic commands ###
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,54 @@ def upgrade():
for dataset in datasets:
ResourcePolicyService.attach_resource_policy(
session=session,
group=dataset.SamlGroupName,
group=dataset.SamlAdminGroupName,
resource_uri=dataset.datasetUri,
permissions=[ATTACH_METADATA_FORM],
resource_type=DatasetBase.__name__,
)


def downgrade():
print('no downgrade supported')
bind = op.get_bind()
session = orm.Session(bind=bind)
all_environments = session.query(Environment).all()
for env in all_environments:
policies = ResourcePolicyService.find_resource_policies(
session=session,
group=env.SamlGroupName,
resource_uri=env.environmentUri,
resource_type=Environment.__name__,
permissions=[ATTACH_METADATA_FORM, CREATE_METADATA_FORM],
)
for policy in policies:
for permission in policy.permissions:
session.delete(permission)
session.commit()
noah-paige marked this conversation as resolved.
Show resolved Hide resolved

all_organizations = session.query(Organization).all()
for org in all_organizations:
policies = ResourcePolicyService.find_resource_policies(
session=session,
group=org.SamlGroupName,
resource_uri=org.organizationUri,
permissions=[ATTACH_METADATA_FORM, CREATE_METADATA_FORM],
resource_type=Organization.__name__,
)
for policy in policies:
for permission in policy.permissions:
session.delete(permission)
session.commit()

datasets = session.query(DatasetBase).all()
for dataset in datasets:
policies = ResourcePolicyService.find_resource_policies(
session=session,
group=dataset.SamlAdminGroupName,
resource_uri=dataset.datasetUri,
permissions=[ATTACH_METADATA_FORM],
resource_type=DatasetBase.__name__,
)
for policy in policies:
for permission in policy.permissions:
session.delete(permission)
session.commit()
Loading