Skip to content

Commit 823d642

Browse files
authored
Fix permissions on share workflows (#914)
### Feature or Bugfix - Bugfix ### Detail There has been a mismatch on the permissions-granting in the shares. There are 2 types of permissions: on the share_object and preview permissions for requesters in the shared_tabled In this PR: - delete share object permissions from RDS when the share object is deleted - attach table permissions to requesters for approved tables in `approve_share_object` - delete table permissions to requesters for revoked tables in `revoke_items_share_object` - delete table permissions to requesters for revoked tables in `remove_shared_item` if the removed item is a table and was in Share_Failed - remove the deletion of permissions on the dataset level (which should not have been there in the first place) ### Relates ### Security Please answer the questions below briefly where applicable, or write `N/A`. Based on [OWASP 10](https://owasp.org/Top10/en/). - Does this PR introduce or modify any input fields or queries - this includes fetching data from storage outside the application (e.g. a database, an S3 bucket)? - Is the input sanitized? - What precautions are you taking before deserializing the data you consume? - Is injection prevented by parametrizing queries? - Have you ensured no `eval` or similar functions are used? - Does this PR introduce any functionality or component that requires authorization? - How have you ensured it respects the existing AuthN/AuthZ mechanisms? - Are you logging failed auth attempts? - Are you using or adding any cryptographic features? - Do you use a standard proven implementations? - Are the used keys controlled by the customer? Where are they stored? - Are you introducing any new policies/roles/users? - Have you used the least-privilege principle? How? By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 2652202 commit 823d642

File tree

3 files changed

+56
-21
lines changed

3 files changed

+56
-21
lines changed

backend/dataall/modules/dataset_sharing/db/share_object_repositories.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -849,17 +849,20 @@ def _find_all_share_item(session, share, status, share_type_model, share_type_ur
849849
)
850850

851851
@staticmethod
852-
def find_all_share_items(session, share_uri, share_type):
853-
return (
852+
def find_all_share_items(session, share_uri, share_type, status=None):
853+
query = (
854854
session.query(ShareObjectItem).filter(
855855
(
856856
and_(
857857
ShareObjectItem.shareUri == share_uri,
858858
ShareObjectItem.itemType == share_type
859859
)
860860
)
861-
).all()
861+
)
862862
)
863+
if status :
864+
query = query.filter(ShareObjectItem.status.in_(status))
865+
return query.all()
863866

864867
@staticmethod
865868
def other_approved_share_item_table_exists(session, environment_uri, item_uri, share_item_uri):

backend/dataall/modules/dataset_sharing/services/share_item_service.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,16 @@ def revoke_items_share_object(uri, revoked_uris):
5757

5858
share_sm.update_state(session, share, new_share_state)
5959

60-
ResourcePolicy.delete_resource_policy(
61-
session=session,
62-
group=share.groupUri,
63-
resource_uri=dataset.datasetUri,
64-
)
60+
if share.groupUri != dataset.SamlAdminGroupName:
61+
revoke_table_items = ShareObjectRepository.find_all_share_items(
62+
session, uri, ShareableType.Table.value, [ShareItemStatus.Revoke_Approved.value]
63+
)
64+
for item in revoke_table_items:
65+
ResourcePolicy.delete_resource_policy(
66+
session=session,
67+
group=share.groupUri,
68+
resource_uri=item.itemUri,
69+
)
6570

6671
ShareNotificationService(
6772
session=session,
@@ -141,6 +146,13 @@ def add_shared_item(uri: str, data: dict = None):
141146
def remove_shared_item(uri: str):
142147
with get_context().db_engine.scoped_session() as session:
143148
share_item = ShareObjectRepository.get_share_item_by_uri(session, uri)
149+
if share_item.itemType == ShareableType.Table.value and share_item.status == ShareItemStatus.Share_Failed.value:
150+
share = ShareObjectRepository.get_share_by_uri(session, share_item.shareUri)
151+
ResourcePolicy.delete_resource_policy(
152+
session=session,
153+
group=share.groupUri,
154+
resource_uri=share_item.itemUri,
155+
)
144156

145157
item_sm = ShareItemSM(share_item.status)
146158
item_sm.run_transition(ShareItemActions.RemoveItem.value)

backend/dataall/modules/dataset_sharing/services/share_object_service.py

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -206,15 +206,18 @@ def approve_share_object(cls, uri: str):
206206
cls._run_transitions(session, share, states, ShareObjectActions.Approve)
207207

208208
# GET TABLES SHARED AND APPROVE SHARE FOR EACH TABLE
209-
share_table_items = ShareObjectRepository.find_all_share_items(session, uri, ShareableType.Table.value)
210-
for table in share_table_items:
211-
ResourcePolicy.attach_resource_policy(
212-
session=session,
213-
group=share.principalId,
214-
permissions=DATASET_TABLE_READ,
215-
resource_uri=table.itemUri,
216-
resource_type=DatasetTable.__name__,
209+
if share.groupUri != dataset.SamlAdminGroupName:
210+
share_table_items = ShareObjectRepository.find_all_share_items(
211+
session, uri, ShareableType.Table.value, [ShareItemStatus.Share_Approved.value]
217212
)
213+
for table in share_table_items:
214+
ResourcePolicy.attach_resource_policy(
215+
session=session,
216+
group=share.groupUri,
217+
permissions=DATASET_TABLE_READ,
218+
resource_uri=table.itemUri,
219+
resource_type=DatasetTable.__name__,
220+
)
218221

219222
share.rejectPurpose = ""
220223
session.commit()
@@ -261,11 +264,6 @@ def reject_share_object(cls, uri: str, reject_purpose: str):
261264
with context.db_engine.scoped_session() as session:
262265
share, dataset, states = cls._get_share_data(session, uri)
263266
cls._run_transitions(session, share, states, ShareObjectActions.Reject)
264-
ResourcePolicy.delete_resource_policy(
265-
session=session,
266-
group=share.groupUri,
267-
resource_uri=dataset.datasetUri,
268-
)
269267

270268
# Update Reject Purpose
271269
share.rejectPurpose = reject_purpose
@@ -295,6 +293,28 @@ def delete_share_object(cls, uri: str):
295293
)
296294

297295
if new_state == ShareObjectStatus.Deleted.value:
296+
# Delete share resource policy permissions
297+
# Deleting REQUESTER permissions
298+
ResourcePolicy.delete_resource_policy(
299+
session=session,
300+
group=share.groupUri,
301+
resource_uri=share.shareUri,
302+
)
303+
304+
# Deleting APPROVER permissions
305+
ResourcePolicy.delete_resource_policy(
306+
session=session,
307+
group=dataset.SamlAdminGroupName,
308+
resource_uri=share.shareUri,
309+
)
310+
if dataset.stewards != dataset.SamlAdminGroupName:
311+
ResourcePolicy.delete_resource_policy(
312+
session=session,
313+
group=dataset.stewards,
314+
resource_uri=share.shareUri,
315+
)
316+
317+
# Delete share
298318
session.delete(share)
299319

300320
return True

0 commit comments

Comments
 (0)