From 22661266a283c4b0f775911f93786d6438e6dab5 Mon Sep 17 00:00:00 2001 From: dlpzx <71252798+dlpzx@users.noreply.github.com> Date: Thu, 6 Jun 2024 15:02:51 +0200 Subject: [PATCH] Generic shares_base module and specific s3_datasets_shares module - part 5 (move exceptions and notifications to shares_base) (#1312) ### Feature or Bugfix - Refactoring ### Detail As explained in the design for #1123 and #1283 we are trying to implement generic `datasets_base` and `shares_base` modules that can be used by any type of datasets and by any type of shareable object in a generic way. In this PR: - Move share_exceptions to shares_base - Move share_notification_service to shares_base ### Relates - #1283 - #1123 - #955 ### 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. --- .../modules/s3_datasets_shares/services/share_item_service.py | 4 ++-- .../services/share_managers/s3_access_point_share_manager.py | 2 +- .../services/share_managers/s3_bucket_share_manager.py | 2 +- .../s3_datasets_shares/services/share_object_service.py | 4 ++-- .../services/share_processors/glue_table_share_processor.py | 2 +- .../share_processors/s3_access_point_share_processor.py | 2 +- .../services/share_processors/s3_bucket_share_processor.py | 2 +- .../s3_datasets_shares/tasks/dataset_subscription_task.py | 2 +- .../services/share_exceptions.py | 0 .../services/share_notification_service.py | 4 ++-- .../dataall/modules/shares_base/services/sharing_service.py | 4 ++-- 11 files changed, 14 insertions(+), 14 deletions(-) rename backend/dataall/modules/{s3_datasets_shares => shares_base}/services/share_exceptions.py (100%) rename backend/dataall/modules/{s3_datasets_shares => shares_base}/services/share_notification_service.py (98%) diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_item_service.py b/backend/dataall/modules/s3_datasets_shares/services/share_item_service.py index 2033fca30..49827ea78 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_item_service.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_item_service.py @@ -21,8 +21,8 @@ ShareObjectSM, ShareItemSM, ) -from dataall.modules.s3_datasets_shares.services.share_exceptions import ShareItemsFound -from dataall.modules.s3_datasets_shares.services.share_notification_service import ShareNotificationService +from dataall.modules.shares_base.services.share_exceptions import ShareItemsFound +from dataall.modules.shares_base.services.share_notification_service import ShareNotificationService from dataall.modules.shares_base.services.share_permissions import ( GET_SHARE_OBJECT, ADD_ITEM, diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_access_point_share_manager.py b/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_access_point_share_manager.py index 93cad1b08..d7e885c97 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_access_point_share_manager.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_access_point_share_manager.py @@ -20,7 +20,7 @@ from dataall.base.aws.iam import IAM from dataall.modules.s3_datasets_shares.services.dataset_sharing_alarm_service import DatasetSharingAlarmService from dataall.modules.s3_datasets_shares.db.share_object_repositories import ShareObjectRepository -from dataall.modules.s3_datasets_shares.services.share_exceptions import PrincipalRoleNotFound +from dataall.modules.shares_base.services.share_exceptions import PrincipalRoleNotFound from dataall.modules.s3_datasets_shares.services.share_managers.share_manager_utils import ShareErrorFormatter from dataall.modules.s3_datasets_shares.services.managed_share_policy_service import ( SharePolicyService, diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_bucket_share_manager.py b/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_bucket_share_manager.py index 9d255e4c9..e8154901b 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_bucket_share_manager.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_bucket_share_manager.py @@ -13,7 +13,7 @@ ) from dataall.modules.s3_datasets_shares.aws.s3_client import S3ControlClient, S3Client, DATAALL_READ_ONLY_SID from dataall.modules.shares_base.db.share_object_models import ShareObject -from dataall.modules.s3_datasets_shares.services.share_exceptions import PrincipalRoleNotFound +from dataall.modules.shares_base.services.share_exceptions import PrincipalRoleNotFound from dataall.modules.s3_datasets_shares.services.share_managers.share_manager_utils import ShareErrorFormatter from dataall.modules.s3_datasets_shares.services.dataset_sharing_alarm_service import DatasetSharingAlarmService from dataall.modules.s3_datasets_shares.services.managed_share_policy_service import ( diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_object_service.py b/backend/dataall/modules/s3_datasets_shares/services/share_object_service.py index 89ac4c0a9..be4626e69 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_object_service.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_object_service.py @@ -26,9 +26,9 @@ ShareObjectSM, ShareItemSM, ) -from dataall.modules.s3_datasets_shares.services.share_exceptions import ShareItemsFound, PrincipalRoleNotFound +from dataall.modules.shares_base.services.share_exceptions import ShareItemsFound, PrincipalRoleNotFound from dataall.modules.s3_datasets_shares.services.share_item_service import ShareItemService -from dataall.modules.s3_datasets_shares.services.share_notification_service import ShareNotificationService +from dataall.modules.shares_base.services.share_notification_service import ShareNotificationService from dataall.modules.s3_datasets_shares.services.managed_share_policy_service import SharePolicyService from dataall.modules.shares_base.services.share_permissions import ( REJECT_SHARE_OBJECT, diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_processors/glue_table_share_processor.py b/backend/dataall/modules/s3_datasets_shares/services/share_processors/glue_table_share_processor.py index 024813934..a4879d98f 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_processors/glue_table_share_processor.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_processors/glue_table_share_processor.py @@ -10,7 +10,7 @@ ShareableType, ) from dataall.modules.s3_datasets.db.dataset_models import DatasetTable -from dataall.modules.s3_datasets_shares.services.share_exceptions import PrincipalRoleNotFound +from dataall.modules.shares_base.services.share_exceptions import PrincipalRoleNotFound from dataall.modules.s3_datasets_shares.services.share_managers import LFShareManager from dataall.modules.s3_datasets_shares.aws.ram_client import RamClient from dataall.modules.s3_datasets_shares.services.share_object_service import ShareObjectService diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_access_point_share_processor.py b/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_access_point_share_processor.py index 8021873a7..76cab06f4 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_access_point_share_processor.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_access_point_share_processor.py @@ -2,7 +2,7 @@ from datetime import datetime from typing import List -from dataall.modules.s3_datasets_shares.services.share_exceptions import PrincipalRoleNotFound +from dataall.modules.shares_base.services.share_exceptions import PrincipalRoleNotFound from dataall.modules.s3_datasets_shares.services.share_managers import S3AccessPointShareManager from dataall.modules.s3_datasets_shares.services.share_object_service import ShareObjectService from dataall.modules.s3_datasets_shares.services.share_item_service import ShareItemService diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_bucket_share_processor.py b/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_bucket_share_processor.py index 021ba9ccc..df5f292fa 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_bucket_share_processor.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_bucket_share_processor.py @@ -2,7 +2,7 @@ from datetime import datetime from typing import List -from dataall.modules.s3_datasets_shares.services.share_exceptions import PrincipalRoleNotFound +from dataall.modules.shares_base.services.share_exceptions import PrincipalRoleNotFound from dataall.modules.s3_datasets_shares.services.share_managers import S3BucketShareManager from dataall.modules.s3_datasets_shares.services.share_object_service import ShareObjectService from dataall.modules.shares_base.services.shares_enums import ( diff --git a/backend/dataall/modules/s3_datasets_shares/tasks/dataset_subscription_task.py b/backend/dataall/modules/s3_datasets_shares/tasks/dataset_subscription_task.py index b0a3f127b..d7b0ddd4b 100644 --- a/backend/dataall/modules/s3_datasets_shares/tasks/dataset_subscription_task.py +++ b/backend/dataall/modules/s3_datasets_shares/tasks/dataset_subscription_task.py @@ -11,7 +11,7 @@ from dataall.base.db import get_engine from dataall.modules.shares_base.db.share_object_models import ShareObjectItem from dataall.modules.s3_datasets_shares.db.share_object_repositories import ShareObjectRepository -from dataall.modules.s3_datasets_shares.services.share_notification_service import ShareNotificationService +from dataall.modules.shares_base.services.share_notification_service import ShareNotificationService from dataall.modules.s3_datasets.aws.sns_dataset_client import SnsDatasetClient from dataall.modules.s3_datasets.db.dataset_location_repositories import DatasetLocationRepository from dataall.modules.s3_datasets.db.dataset_table_repositories import DatasetTableRepository diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_exceptions.py b/backend/dataall/modules/shares_base/services/share_exceptions.py similarity index 100% rename from backend/dataall/modules/s3_datasets_shares/services/share_exceptions.py rename to backend/dataall/modules/shares_base/services/share_exceptions.py diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_notification_service.py b/backend/dataall/modules/shares_base/services/share_notification_service.py similarity index 98% rename from backend/dataall/modules/s3_datasets_shares/services/share_notification_service.py rename to backend/dataall/modules/shares_base/services/share_notification_service.py index d97dcc308..bf86d4878 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_notification_service.py +++ b/backend/dataall/modules/shares_base/services/share_notification_service.py @@ -6,10 +6,10 @@ from dataall.core.tasks.db.task_models import Task from dataall.core.tasks.service_handlers import Worker from dataall.modules.shares_base.db.share_object_models import ShareObject -from dataall.modules.s3_datasets.db.dataset_models import S3Dataset from dataall.base.context import get_context from dataall.modules.shares_base.services.shares_enums import ShareObjectStatus from dataall.modules.notifications.db.notification_repositories import NotificationRepository +from dataall.modules.datasets_base.db.dataset_models import DatasetBase log = logging.getLogger(__name__) @@ -35,7 +35,7 @@ class ShareNotificationService: - share.owner (person that opened the request) OR share.groupUri (if group_notifications=true) """ - def __init__(self, session, dataset: S3Dataset, share: ShareObject): + def __init__(self, session, dataset: DatasetBase, share: ShareObject): self.dataset = dataset self.share = share self.session = session diff --git a/backend/dataall/modules/shares_base/services/sharing_service.py b/backend/dataall/modules/shares_base/services/sharing_service.py index c412f9845..f47a2e277 100644 --- a/backend/dataall/modules/shares_base/services/sharing_service.py +++ b/backend/dataall/modules/shares_base/services/sharing_service.py @@ -22,10 +22,10 @@ from dataall.modules.s3_datasets_shares.services.share_object_service import ( ShareObjectService, ) # TODO move to shares_base in following PR -from dataall.modules.s3_datasets_shares.services.share_exceptions import ( +from dataall.modules.shares_base.services.share_exceptions import ( PrincipalRoleNotFound, DatasetLockTimeout, -) # TODO move to shares_base in following PR +) from dataall.modules.datasets_base.db.dataset_models import DatasetLock log = logging.getLogger(__name__)