diff --git a/backend/dataall/base/feature_toggle_checker.py b/backend/dataall/base/feature_toggle_checker.py index b6d681646..3fa3f70f3 100644 --- a/backend/dataall/base/feature_toggle_checker.py +++ b/backend/dataall/base/feature_toggle_checker.py @@ -2,6 +2,8 @@ Contains decorators that check if a feature has been enabled or not """ +from typing import List, Any, Optional, Callable + from dataall.base.config import config from dataall.base.utils.decorator_utls import process_func @@ -19,3 +21,35 @@ def decorated(*args, **kwargs): return fn_decorator(decorated) return decorator + + +def is_feature_enabled_for_allowed_values( + allowed_values: List[Any], + enabled_values: List[Any], + default_value: Any, + resolve_property: Optional[Callable] = None, + config_property: Optional[str] = None, +): + def decorator(f): + fn, fn_decorator = process_func(f) + + def decorated(*args, **kwargs): + config_property_value = None + if config_property is None and resolve_property is None: + raise Exception('Config property not provided') + if resolve_property: + config_property_value = resolve_property(*args, **kwargs) + if config_property: + config_property_value = config_property + value = config.get_property(config_property_value, default_value) + if value not in allowed_values: + raise Exception( + f'Disabled since incorrect values in config {config_property_value}. Correct config values {allowed_values}' + ) + if value not in enabled_values: + raise Exception(f'Disabled by config: {value}. Enable config value(s): {", ".join(enabled_values)}') + return fn(*args, **kwargs) + + return fn_decorator(decorated) + + return decorator diff --git a/backend/dataall/core/stacks/api/resolvers.py b/backend/dataall/core/stacks/api/resolvers.py index 71d66f03f..22f1b7fa5 100644 --- a/backend/dataall/core/stacks/api/resolvers.py +++ b/backend/dataall/core/stacks/api/resolvers.py @@ -6,7 +6,7 @@ from dataall.base.api.context import Context from dataall.core.environment.services.environment_service import EnvironmentService from dataall.core.stacks.services.keyvaluetag_service import KeyValueTagService -from dataall.core.stacks.services.stack_service import StackService +from dataall.core.stacks.services.stack_service import StackService, map_target_type_to_log_config_path from dataall.core.stacks.db.stack_models import Stack from dataall.core.stacks.aws.cloudwatch import CloudWatch from dataall.base.utils import Parameter @@ -57,6 +57,16 @@ def resolve_events(context, source: Stack, **kwargs): return json.dumps(source.events or {}) +def resolve_stack_visibility(context, source: Stack, **kwargs): + if not source: + return False + try: + return StackService.check_if_user_allowed_view_logs(target_type=source.stack, target_uri=source.targetUri) + except Exception as e: + log.error(f'Failed to check if the user is allowed to view stack logs due to: {e}') + return False + + def resolve_task_id(context, source: Stack, **kwargs): if not source: return None diff --git a/backend/dataall/core/stacks/api/types.py b/backend/dataall/core/stacks/api/types.py index 1ddd96184..4cd624de4 100644 --- a/backend/dataall/core/stacks/api/types.py +++ b/backend/dataall/core/stacks/api/types.py @@ -6,6 +6,7 @@ resolve_events, resolve_task_id, resolve_error, + resolve_stack_visibility, ) Stack = gql.ObjectType( @@ -27,6 +28,7 @@ gql.Field(name='events', type=gql.String, resolver=resolve_events), gql.Field(name='EcsTaskArn', type=gql.String), gql.Field(name='EcsTaskId', type=gql.String, resolver=resolve_task_id), + gql.Field(name='canViewLogs', type=gql.Boolean, resolver=resolve_stack_visibility), ], ) diff --git a/backend/dataall/core/stacks/services/stack_service.py b/backend/dataall/core/stacks/services/stack_service.py index 5da1c4ba6..c409a640d 100644 --- a/backend/dataall/core/stacks/services/stack_service.py +++ b/backend/dataall/core/stacks/services/stack_service.py @@ -3,6 +3,8 @@ import requests import logging +from dataall.base.db import exceptions +from dataall.base.feature_toggle_checker import is_feature_enabled_for_allowed_values from dataall.core.permissions.services.resource_policy_service import ResourcePolicyService from dataall.core.stacks.aws.cloudformation import CloudFormation from dataall.core.stacks.services.keyvaluetag_service import KeyValueTagService @@ -44,6 +46,22 @@ def verify_target_type_and_uri(target_type, target_uri): raise RequiredParameter('targetType') +def map_target_type_to_log_config_path(**kwargs): + target_type = kwargs.get('target_type') + if target_type == 'environment': + return 'core.features.show_stack_logs' + elif target_type == 'dataset': + return 'modules.s3_datasets.features.show_stack_logs' + elif target_type == 'mlstudio': + return 'modules.mlstudio.features.show_stack_logs' + elif target_type == 'notebooks': + return 'modules.notebooks.features.show_stack_logs' + elif target_type == 'datapipelines': + return 'modules.datapipelines.features.show_stack_logs' + else: + return 'Invalid Config' + + class StackService: @staticmethod def resolve_parent_obj_stack(targetUri: str, environmentUri: str): @@ -188,6 +206,7 @@ def update_stack_tags(input): @staticmethod def get_stack_logs(target_uri, target_type): context = get_context() + StackService.check_if_user_allowed_view_logs(target_type=target_type, target_uri=target_uri) StackRequestVerifier.verify_target_type_and_uri(target_uri, target_type) with context.db_engine.scoped_session() as session: @@ -213,3 +232,21 @@ def get_stack_logs(target_uri, target_type): | filter @logStream like "{stack.EcsTaskArn.split('/')[-1]}" """ return query + + @staticmethod + @is_feature_enabled_for_allowed_values( + allowed_values=['admin-only', 'enabled', 'disabled'], + enabled_values=['admin-only', 'enabled'], + default_value='enabled', + resolve_property=map_target_type_to_log_config_path, + ) + def check_if_user_allowed_view_logs(target_type: str, target_uri: str): + context = get_context() + config_value = config.get_property(map_target_type_to_log_config_path(target_type=target_type), 'enabled') + if config_value == 'admin-only' and 'DAAdministrators' not in context.groups: + raise exceptions.ResourceUnauthorized( + username=context.username, + action='View Stack logs', + resource_uri=f'{target_uri} ( Resource type: {target_type} )', + ) + return True diff --git a/backend/dataall/modules/shares_base/api/resolvers.py b/backend/dataall/modules/shares_base/api/resolvers.py index 6a8558b72..93524b3d4 100644 --- a/backend/dataall/modules/shares_base/api/resolvers.py +++ b/backend/dataall/modules/shares_base/api/resolvers.py @@ -5,7 +5,6 @@ from dataall.base.db.exceptions import RequiredParameter from dataall.core.environment.db.environment_models import Environment from dataall.core.environment.services.environment_service import EnvironmentService -from dataall.core.organizations.db.organization_repositories import OrganizationRepository from dataall.modules.datasets_base.db.dataset_models import DatasetBase from dataall.modules.datasets_base.db.dataset_repositories import DatasetBaseRepository from dataall.modules.shares_base.services.shares_enums import ShareObjectPermission, PrincipalType @@ -214,7 +213,11 @@ def resolve_user_role(context: Context, source: ShareObject, **kwargs): def resolve_can_view_logs(context: Context, source: ShareObject): - return ShareLogsService.check_view_log_permissions(context.username, context.groups, source.shareUri) + try: + return ShareLogsService.check_view_logs_permissions(source.shareUri) + except Exception as e: + log.error(f'Failed to check if user is allowed to view share logs due to: {e}') + return False def resolve_dataset(context: Context, source: ShareObject, **kwargs): diff --git a/backend/dataall/modules/shares_base/services/share_logs_service.py b/backend/dataall/modules/shares_base/services/share_logs_service.py index d79f165d8..d67e45784 100644 --- a/backend/dataall/modules/shares_base/services/share_logs_service.py +++ b/backend/dataall/modules/shares_base/services/share_logs_service.py @@ -2,6 +2,7 @@ import logging from dataall.base.context import get_context +from dataall.base.feature_toggle_checker import is_feature_enabled_for_allowed_values from dataall.base.utils import Parameter from dataall.base.db import exceptions from dataall.core.stacks.aws.cloudwatch import CloudWatch @@ -15,11 +16,33 @@ class ShareLogsService: @staticmethod - def check_view_log_permissions(username, groups, shareUri): - with get_context().db_engine.scoped_session() as session: + @is_feature_enabled_for_allowed_values( + allowed_values=['admin-only', 'enabled', 'disabled'], + enabled_values=['admin-only', 'enabled'], + default_value='enabled', + config_property='modules.shares_base.features.show_share_logs', + ) + def check_view_logs_permissions(shareUri): + context = get_context() + log_config = config.get_property('modules.shares_base.features.show_share_logs', 'enabled') + if log_config == 'admin-only' and 'DAAdministrators' not in context.groups: + raise exceptions.ResourceUnauthorized( + username=context.username, + action='View Share Logs', + resource_uri=shareUri, + ) + with context.db_engine.scoped_session() as session: share = ShareObjectRepository.get_share_by_uri(session, shareUri) ds = DatasetBaseRepository.get_dataset_by_uri(session, share.datasetUri) - return ds.stewards in groups or ds.SamlAdminGroupName in groups or username == ds.owner + if not ( + ds.stewards in context.groups or ds.SamlAdminGroupName in context.groups or context.username == ds.owner + ): + raise exceptions.ResourceUnauthorized( + username=context.username, + action='View Share Logs', + resource_uri=shareUri, + ) + return True @staticmethod def get_share_logs_name_query(shareUri): @@ -43,13 +66,7 @@ def get_share_logs_query(log_stream_name): @staticmethod def get_share_logs(shareUri): context = get_context() - if not ShareLogsService.check_view_log_permissions(context.username, context.groups, shareUri): - raise exceptions.ResourceUnauthorized( - username=context.username, - action='View Share Logs', - resource_uri=shareUri, - ) - + ShareLogsService.check_view_logs_permissions(shareUri) envname = os.getenv('envname', 'local') log_query_period_days = config.get_property('core.log_query_period_days', 1) log.info(f'log_query_period_days: {log_query_period_days}') diff --git a/config.json b/config.json index 0ff775965..e0e193c59 100644 --- a/config.json +++ b/config.json @@ -1,13 +1,22 @@ { "modules": { "mlstudio": { - "active": true + "active": true, + "features": { + "show_stack_logs": "enabled" + } }, "notebooks": { - "active": true + "active": true, + "features": { + "show_stack_logs": "enabled" + } }, "datapipelines": { - "active": true + "active": true, + "features": { + "show_stack_logs": "enabled" + } }, "omics": { "active": false @@ -47,7 +56,14 @@ "file_actions": true, "aws_actions": true, "preview_data": true, - "glue_crawler": true + "glue_crawler": true, + "show_stack_logs": "enabled" + } + }, + "shares_base": { + "active": true, + "features": { + "show_share_logs": "enabled" } }, "s3_datasets_shares": { @@ -73,7 +89,8 @@ "features": { "env_aws_actions": true, "cdk_pivot_role_multiple_environments_same_account": false, - "enable_quicksight_monitoring": false + "enable_quicksight_monitoring": false, + "show_stack_logs": "enabled" }, "log_query_period_days": 1 } diff --git a/frontend/src/modules/Shared/Stack/Stack.js b/frontend/src/modules/Shared/Stack/Stack.js index 56baeff50..abd20da55 100644 --- a/frontend/src/modules/Shared/Stack/Stack.js +++ b/frontend/src/modules/Shared/Stack/Stack.js @@ -35,6 +35,8 @@ export const Stack = (props) => { const [resources, setResources] = useState([]); const [stackName, setStackName] = useState(null); const [openLogsModal, setOpenLogsModal] = useState(null); + const [isStackLogsVisible, setIsStackLogsVisible] = useState(false); + const handleOpenLogsModal = () => { setOpenLogsModal(true); }; @@ -54,6 +56,7 @@ export const Stack = (props) => { setStack({ ...response.data.getStack }); setStackName(`${response.data.getStack.name}`); setResources(JSON.parse(response.data.getStack.resources).resources); + setIsStackLogsVisible(response.data.getStack.canViewLogs); } else { dispatch({ type: SET_ERROR, error: response.errors[0].message }); } @@ -120,15 +123,17 @@ export const Stack = (props) => { > Refresh - + {isStackLogsVisible === true && ( + + )} { )} )} - + {isStackLogsVisible === true && ( + + )} )} diff --git a/frontend/src/services/graphql/Stack/getStack.js b/frontend/src/services/graphql/Stack/getStack.js index 9add63401..960bf524d 100644 --- a/frontend/src/services/graphql/Stack/getStack.js +++ b/frontend/src/services/graphql/Stack/getStack.js @@ -32,6 +32,7 @@ export const getStack = (environmentUri, stackUri, targetUri, targetType) => ({ error events name + canViewLogs } } `