Skip to content

Commit

Permalink
Refactor: uncouple datasets and dataset_sharing modules - part 2-4 (#…
Browse files Browse the repository at this point in the history
…1214)

### Feature or Bugfix
- Refactoring
⚠️ MERGE AFTER #1213

### Detail
This is needed as explained in full PR [AFTER 2.4] Refactor: uncouple
datasets and dataset_sharing modules #1179
- [X] Use interface to resolve dataset roles related to datasets shared
and implement logic in the dataset_sharing module
- [X] Extend and clean-up stewards share permissions through interface

### Relates
- #1179 

### 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.
  • Loading branch information
dlpzx authored May 3, 2024
1 parent 6d3f2d4 commit 42a5f6b
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,11 @@ def get_share_by_uri(session, uri):
return share

@staticmethod
def get_share_by_dataset_attributes(session, dataset_uri, dataset_owner):
def get_share_by_dataset_attributes(session, dataset_uri, dataset_owner, groups=[]):
share: ShareObject = (
session.query(ShareObject)
.filter(ShareObject.datasetUri == dataset_uri)
.filter(ShareObject.owner == dataset_owner)
.filter(or_(ShareObject.owner == dataset_owner, ShareObject.principalId.in_(groups)))
.first()
)
return share
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
from dataall.base.context import get_context
from dataall.base.db import exceptions
from dataall.base.aws.sts import SessionHelper
from dataall.modules.dataset_sharing.db.share_object_models import ShareObject
from dataall.modules.dataset_sharing.db.share_object_repositories import (
ShareObjectRepository,
ShareItemSM,
)
from dataall.modules.dataset_sharing.services.share_permissions import SHARE_OBJECT_APPROVER
from dataall.modules.dataset_sharing.services.share_item_service import ShareItemService
from dataall.modules.datasets_base.db.dataset_repositories import DatasetRepository
from dataall.modules.datasets.services.dataset_permissions import (
Expand All @@ -20,6 +22,7 @@
)

from dataall.modules.datasets_base.db.dataset_models import Dataset
from dataall.modules.datasets_base.services.datasets_base_enums import DatasetRole
from dataall.modules.datasets.services.dataset_service import DatasetServiceInterface


Expand All @@ -29,6 +32,14 @@


class DatasetSharingService(DatasetServiceInterface):
@staticmethod
def resolve_additional_dataset_user_role(session, uri, username, groups):
"""Implemented as part of the DatasetServiceInterface"""
share = ShareObjectRepository.get_share_by_dataset_attributes(session, uri, username, groups)
if share is not None:
return DatasetRole.Shared.value
return None

@staticmethod
def check_before_delete(session, uri, **kwargs):
"""Implemented as part of the DatasetServiceInterface"""
Expand Down Expand Up @@ -70,6 +81,39 @@ def append_to_list_user_datasets(session, username, groups):
"""Implemented as part of the DatasetServiceInterface"""
return ShareObjectRepository.query_user_shared_datasets(session, username, groups)

@staticmethod
def extend_attach_steward_permissions(session, dataset, new_stewards, **kwargs):
"""Implemented as part of the DatasetServiceInterface"""
dataset_shares = ShareObjectRepository.find_dataset_shares(session, dataset.datasetUri)
if dataset_shares:
for share in dataset_shares:
ResourcePolicyService.attach_resource_policy(
session=session,
group=new_stewards,
permissions=SHARE_OBJECT_APPROVER,
resource_uri=share.shareUri,
resource_type=ShareObject.__name__,
)
if dataset.stewards != dataset.SamlAdminGroupName:
ResourcePolicyService.delete_resource_policy(
session=session,
group=dataset.stewards,
resource_uri=share.shareUri,
)

@staticmethod
def extend_delete_steward_permissions(session, dataset, **kwargs):
"""Implemented as part of the DatasetServiceInterface"""
dataset_shares = ShareObjectRepository.find_dataset_shares(session, dataset.datasetUri)
if dataset_shares:
for share in dataset_shares:
if dataset.stewards != dataset.SamlAdminGroupName:
ResourcePolicyService.delete_resource_policy(
session=session,
group=dataset.stewards,
resource_uri=share.shareUri,
)

@staticmethod
@TenantPolicyService.has_tenant_permission(MANAGE_DATASETS)
@ResourcePolicyService.has_resource_permission(UPDATE_DATASET)
Expand Down
8 changes: 5 additions & 3 deletions backend/dataall/modules/datasets/api/dataset/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@ def resolve_user_role(context: Context, source: Dataset, **kwargs):
return DatasetRole.DataSteward.value
else:
with context.engine.scoped_session() as session:
share = session.query(ShareObject).filter(ShareObject.datasetUri == source.datasetUri).first()
if share and (share.owner == context.username or share.principalId in context.groups):
return DatasetRole.Shared.value
other_modules_user_role = DatasetService.get_other_modules_dataset_user_role(
session, source.datasetUri, context.username, context.groups
)
if other_modules_user_role is not None:
return other_modules_user_role
return DatasetRole.NoPermission.value


Expand Down
49 changes: 24 additions & 25 deletions backend/dataall/modules/datasets/services/dataset_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ class DatasetService:
def register(cls, interface: DatasetServiceInterface):
cls._interfaces.append(interface)

@classmethod
def get_other_modules_dataset_user_role(cls, session, uri, username, groups) -> str:
"""All other user role types that might come from other modules"""
for interface in cls._interfaces:
role = interface.resolve_additional_dataset_user_role(session, uri, username, groups)
if role is not None:
return role
return None

@classmethod
def check_before_delete(cls, session, uri, **kwargs) -> bool:
"""All actions from other modules that need to be executed before deletion"""
Expand All @@ -101,6 +110,18 @@ def _list_all_user_interface_datasets(cls, session, username, groups) -> List:
if query.first() is not None
]

@classmethod
def _attach_additional_steward_permissions(cls, session, dataset, new_stewards):
"""All permissions from other modules that need to be granted to stewards"""
for interface in cls._interfaces:
interface.extend_attach_steward_permissions(session, dataset, new_stewards)

@classmethod
def _delete_additional_steward__permissions(cls, session, dataset):
"""All permissions from other modules that need to be deleted to stewards"""
for interface in cls._interfaces:
interface.extend_delete_steward_permissions(session, dataset)

@staticmethod
def check_dataset_account(session, environment):
dashboards_enabled = EnvironmentService.get_boolean_env_param(session, environment, 'dashboardsEnabled')
Expand Down Expand Up @@ -515,15 +536,7 @@ def _transfer_stewardship_to_owners(session, dataset):
resource_uri=tableUri,
)

# Remove Steward Resource Policy on Dataset Share Objects
dataset_shares = ShareObjectRepository.find_dataset_shares(session, dataset.datasetUri)
if dataset_shares:
for share in dataset_shares:
ResourcePolicyService.delete_resource_policy(
session=session,
group=dataset.stewards,
resource_uri=share.shareUri,
)
DatasetService._delete_additional_steward__permissions(session, dataset)
return dataset

@staticmethod
Expand Down Expand Up @@ -559,22 +572,8 @@ def _transfer_stewardship_to_new_stewards(session, dataset, new_stewards):
resource_type=DatasetTable.__name__,
)

dataset_shares = ShareObjectRepository.find_dataset_shares(session, dataset.datasetUri)
if dataset_shares:
for share in dataset_shares:
ResourcePolicyService.attach_resource_policy(
session=session,
group=new_stewards,
permissions=SHARE_OBJECT_APPROVER,
resource_uri=share.shareUri,
resource_type=ShareObject.__name__,
)
if dataset.stewards != dataset.SamlAdminGroupName:
ResourcePolicyService.delete_resource_policy(
session=session,
group=dataset.stewards,
resource_uri=share.shareUri,
)
DatasetService._attach_additional_steward_permissions(session, dataset, new_stewards)

return dataset

@staticmethod
Expand Down

0 comments on commit 42a5f6b

Please sign in to comment.