Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added permission check - is tenant to update SSM parameters API #1714

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Added permission check - is tenant to update SSM parameters API #1714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 2 additions & 16 deletions backend/dataall/core/permissions/api/resolvers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import logging
import os

from dataall.base.aws.sts import SessionHelper
from dataall.base.aws.parameter_store import ParameterStoreManager
from dataall.base.db.exceptions import RequiredParameter
from dataall.core.permissions.services.permission_service import PermissionService
from dataall.core.permissions.services.tenant_policy_service import TenantPolicyService
from dataall.core.permissions.services.tenant_policy_service import TenantPolicyService, TenantActionsService

log = logging.getLogger(__name__)

Expand All @@ -26,12 +20,4 @@ def list_tenant_groups(context, source, filter=None):


def update_ssm_parameter(context, source, name: str = None, value: str = None):
current_account = SessionHelper.get_account()
region = os.getenv('AWS_REGION', 'eu-west-1')
response = ParameterStoreManager.update_parameter(
AwsAccountId=current_account,
region=region,
parameter_name=f'/dataall/{os.getenv("envname", "local")}/quicksightmonitoring/{name}',
parameter_value=value,
)
return response
return TenantActionsService.update_monitoring_ssm_parameter(name, value)
22 changes: 22 additions & 0 deletions backend/dataall/core/permissions/services/tenant_policy_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from dataall.core.permissions.services.permission_service import PermissionService
from dataall.core.permissions.db.tenant.tenant_models import Tenant
from dataall.base.services.service_provider_factory import ServiceProviderFactory
from dataall.base.aws.sts import SessionHelper
from dataall.base.aws.parameter_store import ParameterStoreManager
import logging
import os
from functools import wraps
Expand Down Expand Up @@ -121,6 +123,26 @@ def validate_permissions(session, tenant_name, g_permissions, group):
return tenant_group_permissions


class TenantActionsService:
@staticmethod
def update_monitoring_ssm_parameter(name, value):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we keeping this API or removing this along with other Admin Dashboard ones?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you did in https://github.com/data-dot-all/dataall/pull/1712/files

# raises UnauthorizedOperation exception, if there is no admin access
context = get_context()
TenantPolicyValidationService.validate_admin_access(
context.username, context.groups, 'UPDATE_SSM_PARAMETER_MONITORING'
)

current_account = SessionHelper.get_account()
region = os.getenv('AWS_REGION', 'eu-west-1')
response = ParameterStoreManager.update_parameter(
AwsAccountId=current_account,
region=region,
parameter_name=f'/dataall/{os.getenv("envname", "local")}/quicksightmonitoring/{name}',
parameter_value=value,
)
Comment on lines +137 to +142
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is carry over from resolver code - but this does not work...

All of the following use ParameterStore and try to assume into pivotRole in infra account to get SSM (pivot role DNE in infra account)
createQuicksightDataSourceSet,
getMonitoringDashboardId,
getMonitoringVPCConnectionId,
updateSSMParameter

Will approve this PR but leave this comment in https://github.com/data-dot-all/dataall/pull/1712/files where I think more focus is on QuickSight Monitoring View

return response


class TenantPolicyService:
TENANT_NAME = 'dataall'

Expand Down
Loading