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.

Sign up for GitHub

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

Jump to bottom

Scope down dataset sharing requester IAM role managed IAM policy S3 permissions #1280

Merged
merged 9 commits into from
Jun 3, 2024
Merged

Scope down dataset sharing requester IAM role managed IAM policy S3 permissions #1280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ee2f3a0
Updated managed_share_policy_service.py to restrict s3 permissions to…
mourya-33 May 9, 2024
4912b7c
Updated managed_share_policy_service.py to fix ruff formatting
mourya-33 May 9, 2024
ea1edea
Merge branch 'data-dot-all:main' into ds-sharing-consumptionrole-s3-p…
mourya-33 May 16, 2024
d82382d
Update managed_share_policy_service.py to remove S3* and add getObject
mourya-33 May 16, 2024
5098846
Updated managed_share_policy_service.py to restrict missing resource …
mourya-33 May 18, 2024
f11029c
Updated managed_share_policy_service.py to restrict permissions in ad…
mourya-33 May 18, 2024
b63864f
Updated test_s3_access_point_share_manager.py with updates to restric…
mourya-33 May 18, 2024
56f12eb
Updated test_s3_bucket_share_manager.py for the restricted s3 permiss…
mourya-33 May 18, 2024
9812523
Merge remote-tracking branch 'origin/main' into ds-sharing-consumptio…
Jun 1, 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
9 changes: 7 additions & 2 deletions backend/dataall/modules/s3_datasets_shares/services/managed_share_policy_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,15 @@ def add_missing_resources_to_policy_statement(
index = self._get_statement_by_sid(policy_document, statement_sid)
if index is None:
log.info(f'{statement_sid} does NOT exists for Managed policy {policy_name} ' f'creating statement...')
policy_actions = (
[f'{resource_type}:List*', f'{resource_type}:Describe*', f'{resource_type}:GetObject']
if resource_type == 's3'
else [f'{resource_type}:*']
)
additional_policy = {
'Sid': statement_sid,
'Effect': 'Allow',
'Action': [f'{resource_type}:*'],
'Action': policy_actions,
'Resource': target_resources,
}
policy_document['Statement'].append(additional_policy)
Expand Down Expand Up @@ -209,7 +214,7 @@ def _update_policy_resources_from_inline_policy(self, policy, statement_sid, exi
additional_policy = {
'Sid': f'{statement_sid}S3',
'Effect': 'Allow',
'Action': ['s3:*', 's3:List*', 's3:Describe*'],
'Action': ['s3:List*', 's3:Describe*', 's3:GetObject'],
'Resource': existing_s3,
}
policy['Statement'].append(additional_policy)
Expand Down
2 changes: 1 addition & 1 deletion tests/modules/datasets/tasks/test_s3_access_point_share_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ def test_grant_target_role_access_policy_test_empty_policy(
{
'Sid': f'{IAM_S3_ACCESS_POINTS_STATEMENT_SID}S3',
'Effect': 'Allow',
'Action': ['s3:*'],
'Action': ['s3:List*', 's3:Describe*', 's3:GetObject'],
'Resource': [
f'arn:aws:s3:::{location1.S3BucketName}',
f'arn:aws:s3:::{location1.S3BucketName}/*',
Expand Down
8 changes: 6 additions & 2 deletions tests/modules/datasets/tasks/test_s3_bucket_share_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,9 @@ def test_grant_s3_iam_access_with_no_policy(mocker, dataset2, share2_manager):
assert len(iam_policy['Statement'][kms_index]['Resource']) == 1
assert (
f'arn:aws:s3:::{dataset2.S3BucketName}' in iam_policy['Statement'][s3_index]['Resource']
and 's3:*' in iam_policy['Statement'][s3_index]['Action']
and 's3:List*' in iam_policy['Statement'][s3_index]['Action']
and 's3:Describe*' in iam_policy['Statement'][s3_index]['Action']
and 's3:GetObject' in iam_policy['Statement'][s3_index]['Action']
)
assert (
f'arn:aws:kms:{dataset2.region}:{dataset2.AwsAccountId}:key/kms-key'
Expand Down Expand Up @@ -519,7 +521,9 @@ def test_grant_s3_iam_access_with_empty_policy(mocker, dataset2, share2_manager)
assert len(iam_policy['Statement'][kms_index]['Resource']) == 1
assert (
f'arn:aws:s3:::{dataset2.S3BucketName}' in iam_policy['Statement'][s3_index]['Resource']
and 's3:*' in iam_policy['Statement'][s3_index]['Action']
and 's3:List*' in iam_policy['Statement'][s3_index]['Action']
and 's3:Describe*' in iam_policy['Statement'][s3_index]['Action']
and 's3:GetObject' in iam_policy['Statement'][s3_index]['Action']
)
assert (
f'arn:aws:kms:{dataset2.region}:{dataset2.AwsAccountId}:key/kms-key'
Expand Down
Loading