Skip to content

Commit

Permalink
[Gh-1528] Configurable stack logs display (#1559)
Browse files Browse the repository at this point in the history
### Feature or Bugfix
- Feature


### Detail

As described in this issue -
#1528 , stack logs provide
some key info.

Creating a feature config called `show_stack_logs` for s3_datasets,
s3_dataset_shares, and core ( for environment stack logs ) and adding
logic in frontend to hide/show the logs button.
Using decorator in backend to restrict access based on config.


### Relates
- #1528

### 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)? N/A
  - 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? N/A
- 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? N/A
  - 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? N/A
  - 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.

---------

Co-authored-by: trajopadhye <tejas.rajopadhye@yahooinc.com>
  • Loading branch information
TejasRGitHub and trajopadhye authored Sep 25, 2024
1 parent 59f0b26 commit 075b43c
Show file tree
Hide file tree
Showing 9 changed files with 162 additions and 34 deletions.
34 changes: 34 additions & 0 deletions backend/dataall/base/feature_toggle_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
12 changes: 11 additions & 1 deletion backend/dataall/core/stacks/api/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions backend/dataall/core/stacks/api/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
resolve_events,
resolve_task_id,
resolve_error,
resolve_stack_visibility,
)

Stack = gql.ObjectType(
Expand All @@ -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),
],
)

Expand Down
37 changes: 37 additions & 0 deletions backend/dataall/core/stacks/services/stack_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand All @@ -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
7 changes: 5 additions & 2 deletions backend/dataall/modules/shares_base/api/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
37 changes: 27 additions & 10 deletions backend/dataall/modules/shares_base/services/share_logs_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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}')
Expand Down
27 changes: 22 additions & 5 deletions config.json
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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": {
Expand All @@ -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
}
Expand Down
39 changes: 23 additions & 16 deletions frontend/src/modules/Shared/Stack/Stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand All @@ -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 });
}
Expand Down Expand Up @@ -120,15 +123,17 @@ export const Stack = (props) => {
>
Refresh
</Button>
<Button
color="primary"
startIcon={<Article fontSize="small" />}
sx={{ m: 1 }}
variant="outlined"
onClick={handleOpenLogsModal}
>
Logs
</Button>
{isStackLogsVisible === true && (
<Button
color="primary"
startIcon={<Article fontSize="small" />}
sx={{ m: 1 }}
variant="outlined"
onClick={handleOpenLogsModal}
>
Logs
</Button>
)}
<LoadingButton
color="primary"
loading={updating}
Expand Down Expand Up @@ -256,13 +261,15 @@ export const Stack = (props) => {
)}
</Box>
)}
<StackLogs
environmentUri={environmentUri}
stack={stack}
targetType={targetType}
onClose={handleCloseOpenLogs}
open={openLogsModal}
/>
{isStackLogsVisible === true && (
<StackLogs
environmentUri={environmentUri}
stack={stack}
targetType={targetType}
onClose={handleCloseOpenLogs}
open={openLogsModal}
/>
)}
</Box>
)}
</Box>
Expand Down
1 change: 1 addition & 0 deletions frontend/src/services/graphql/Stack/getStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const getStack = (environmentUri, stackUri, targetUri, targetType) => ({
error
events
name
canViewLogs
}
}
`
Expand Down

0 comments on commit 075b43c

Please sign in to comment.