diff --git a/backend/dataall/core/environment/api/input_types.py b/backend/dataall/core/environment/api/input_types.py index 504ad2110..f0e180462 100644 --- a/backend/dataall/core/environment/api/input_types.py +++ b/backend/dataall/core/environment/api/input_types.py @@ -31,6 +31,7 @@ gql.Argument('vpcId', gql.String), gql.Argument('subnetIds', gql.ArrayType(gql.String)), gql.Argument('type', gql.String), + gql.Argument('PermissionsBoundaryPolicyArn', gql.String), ], ) @@ -45,6 +46,7 @@ gql.Argument('parameters', gql.ArrayType(ModifyEnvironmentParameterInput)), gql.Argument('vpcId', gql.String), gql.Argument('subnetIds', gql.ArrayType(gql.String)), + gql.Argument('PermissionsBoundaryPolicyArn', gql.String), ], ) diff --git a/backend/dataall/core/environment/api/types.py b/backend/dataall/core/environment/api/types.py index 45a97d892..cba341198 100644 --- a/backend/dataall/core/environment/api/types.py +++ b/backend/dataall/core/environment/api/types.py @@ -105,6 +105,7 @@ gql.Field('EnvironmentDefaultBucketName', type=gql.String), gql.Field('EnvironmentLogsBucketName', type=gql.String), gql.Field('EnvironmentDefaultAthenaWorkGroup', type=gql.String), + gql.Field('PermissionsBoundaryPolicyArn', type=gql.String), gql.Field( name='networks', type=gql.ArrayType(gql.Ref('Vpc')), diff --git a/backend/dataall/core/environment/cdk/environment_stack.py b/backend/dataall/core/environment/cdk/environment_stack.py index ac2794b57..7ad0961f7 100644 --- a/backend/dataall/core/environment/cdk/environment_stack.py +++ b/backend/dataall/core/environment/cdk/environment_stack.py @@ -11,6 +11,7 @@ aws_sns_subscriptions as sns_subs, aws_kms as kms, aws_athena, + Aspects, RemovalPolicy, CfnOutput, Stack, @@ -32,6 +33,7 @@ from dataall.base.aws.parameter_store import ParameterStoreManager from dataall.base.aws.sts import SessionHelper from dataall.base.utils.cdk_nag_utils import CDKNagUtil +from dataall.core.environment.cdk.permissions_boundary_aspect import PermissionsBoundaryAspect logger = logging.getLogger(__name__) @@ -404,6 +406,9 @@ def __init__(self, scope, id, target_uri: str = None, **kwargs): TagsUtil.add_tags(stack=self, model=Environment, target_type='environment') + if self._environment.PermissionsBoundaryPolicyArn: + Aspects.of(self).add(PermissionsBoundaryAspect(self._environment.PermissionsBoundaryPolicyArn)) + CDKNagUtil.check_rules(self) def create_or_import_environment_admin_group_role(self): diff --git a/backend/dataall/core/environment/cdk/permissions_boundary_aspect.py b/backend/dataall/core/environment/cdk/permissions_boundary_aspect.py new file mode 100644 index 000000000..62aea7dc0 --- /dev/null +++ b/backend/dataall/core/environment/cdk/permissions_boundary_aspect.py @@ -0,0 +1,18 @@ +import jsii +from aws_cdk import IAspect, CfnResource + + +@jsii.implements(IAspect) +class PermissionsBoundaryAspect: + """CDK Aspect that applies a permissions boundary to all IAM roles in a stack. + + This handles roles auto-created by CDK constructs (e.g. cr.Provider, BucketDeployment) + that cannot be configured directly. + """ + + def __init__(self, permissions_boundary_arn: str): + self._arn = permissions_boundary_arn + + def visit(self, node): + if isinstance(node, CfnResource) and node.cfn_resource_type == 'AWS::IAM::Role': + node.add_property_override('PermissionsBoundary', self._arn) diff --git a/backend/dataall/core/environment/db/environment_models.py b/backend/dataall/core/environment/db/environment_models.py index ea03c3cff..1d795893c 100644 --- a/backend/dataall/core/environment/db/environment_models.py +++ b/backend/dataall/core/environment/db/environment_models.py @@ -43,6 +43,8 @@ class Environment(Resource, Base): subscriptionsConsumersTopicName = Column(String) subscriptionsConsumersTopicImported = Column(Boolean, default=False) + PermissionsBoundaryPolicyArn = Column(String, nullable=True) + def uri(self): return self.environmentUri diff --git a/backend/dataall/core/environment/services/environment_service.py b/backend/dataall/core/environment/services/environment_service.py index 2883aaa6b..f257a1809 100644 --- a/backend/dataall/core/environment/services/environment_service.py +++ b/backend/dataall/core/environment/services/environment_service.py @@ -248,6 +248,7 @@ def create_environment(uri, data=None): EnvironmentDefaultIAMRoleArn=data.get('EnvironmentDefaultIAMRoleArn', 'unknown'), CDKRoleArn=f'arn:aws:iam::{data.get("AwsAccountId")}:role/{cdk_role_name}', resourcePrefix=data.get('resourcePrefix'), + PermissionsBoundaryPolicyArn=data.get('PermissionsBoundaryPolicyArn', ''), ) session.add(env) @@ -343,6 +344,8 @@ def update_environment(uri, data=None): environment.tags = data.get('tags') if data.get('resourcePrefix'): environment.resourcePrefix = data.get('resourcePrefix') + if 'PermissionsBoundaryPolicyArn' in data: + environment.PermissionsBoundaryPolicyArn = data.get('PermissionsBoundaryPolicyArn', '') EnvironmentService._update_env_parameters(session, environment, data) diff --git a/backend/dataall/modules/datapipelines/cdk/datapipelines_pipeline.py b/backend/dataall/modules/datapipelines/cdk/datapipelines_pipeline.py index c39c52ba4..81a1ddf28 100644 --- a/backend/dataall/modules/datapipelines/cdk/datapipelines_pipeline.py +++ b/backend/dataall/modules/datapipelines/cdk/datapipelines_pipeline.py @@ -4,7 +4,7 @@ import subprocess from typing import List -from aws_cdk import aws_codebuild as codebuild, Stack, RemovalPolicy, CfnOutput +from aws_cdk import aws_codebuild as codebuild, Aspects, Stack, RemovalPolicy, CfnOutput from aws_cdk import aws_codecommit as codecommit from aws_cdk import aws_codepipeline as codepipeline from aws_cdk import aws_codepipeline_actions as codepipeline_actions @@ -23,6 +23,7 @@ from dataall.modules.datapipelines.db.datapipelines_models import DataPipeline, DataPipelineEnvironment from dataall.modules.datapipelines.db.datapipelines_repositories import DatapipelinesRepository from dataall.base.utils.cdk_nag_utils import CDKNagUtil +from dataall.core.environment.cdk.permissions_boundary_aspect import PermissionsBoundaryAspect from dataall.base.utils.shell_utils import CommandSanitizer logger = logging.getLogger(__name__) @@ -397,6 +398,9 @@ def __init__(self, scope, id, target_uri: str = None, **kwargs): TagsUtil.add_tags(stack=self, model=DataPipeline, target_type='pipeline') + if pipeline_environment.PermissionsBoundaryPolicyArn: + Aspects.of(self).add(PermissionsBoundaryAspect(pipeline_environment.PermissionsBoundaryPolicyArn)) + CDKNagUtil.check_rules(self) PipelineStack.cleanup_zip_directory(code_dir_path) diff --git a/backend/dataall/modules/s3_datasets/cdk/dataset_stack.py b/backend/dataall/modules/s3_datasets/cdk/dataset_stack.py index 4a8a422bf..9991423ed 100644 --- a/backend/dataall/modules/s3_datasets/cdk/dataset_stack.py +++ b/backend/dataall/modules/s3_datasets/cdk/dataset_stack.py @@ -7,6 +7,7 @@ aws_iam as iam, aws_ssm as ssm, aws_glue as glue, + Aspects, Stack, Duration, CfnResource, @@ -27,6 +28,7 @@ from dataall.modules.s3_datasets.aws.lf_dataset_client import LakeFormationDatasetClient from dataall.modules.s3_datasets.db.dataset_models import S3Dataset from dataall.base.utils.cdk_nag_utils import CDKNagUtil +from dataall.core.environment.cdk.permissions_boundary_aspect import PermissionsBoundaryAspect from dataall.base.config import config @@ -598,4 +600,7 @@ def __init__(self, scope, id, target_uri: str = None, **kwargs): TagsUtil.add_tags(stack=self, model=S3Dataset, target_type='dataset') + if env.PermissionsBoundaryPolicyArn: + Aspects.of(self).add(PermissionsBoundaryAspect(env.PermissionsBoundaryPolicyArn)) + CDKNagUtil.check_rules(self) diff --git a/backend/migrations/versions/a4f8b2c1d3e5_add_permissions_boundary_to_environment.py b/backend/migrations/versions/a4f8b2c1d3e5_add_permissions_boundary_to_environment.py new file mode 100644 index 000000000..ea6d98110 --- /dev/null +++ b/backend/migrations/versions/a4f8b2c1d3e5_add_permissions_boundary_to_environment.py @@ -0,0 +1,24 @@ +"""add_permissions_boundary_to_environment + +Revision ID: a4f8b2c1d3e5 +Revises: 2258cd8d6e9f +Create Date: 2024-05-01 00:00:00.000000 + +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'a4f8b2c1d3e5' +down_revision = '2258cd8d6e9f' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column('environment', sa.Column('PermissionsBoundaryPolicyArn', sa.String(), nullable=True)) + + +def downgrade(): + op.drop_column('environment', 'PermissionsBoundaryPolicyArn') diff --git a/backend/migrations/versions/ba2da94739ab_backfill_mf_enforcement_permissions.py b/backend/migrations/versions/ba2da94739ab_backfill_mf_enforcement_permissions.py index 073ef7210..e6f7de02f 100644 --- a/backend/migrations/versions/ba2da94739ab_backfill_mf_enforcement_permissions.py +++ b/backend/migrations/versions/ba2da94739ab_backfill_mf_enforcement_permissions.py @@ -8,15 +8,13 @@ from alembic import op import sqlalchemy as sa -from sqlalchemy import orm +from sqlalchemy import orm, text -from dataall.core.environment.db.environment_models import Environment from dataall.core.organizations.db.organization_models import Organization from dataall.core.permissions.api.enums import PermissionType from dataall.core.permissions.services.permission_service import PermissionService from dataall.core.permissions.services.resource_policy_service import ResourcePolicyService from dataall.core.permissions.services.resources_permissions import RESOURCES_ALL_WITH_DESC -from dataall.modules.datasets_base.db.dataset_models import DatasetBase from dataall.modules.metadata_forms.services.metadata_form_permissions import ENFORCE_METADATA_FORM # revision identifiers, used by Alembic. @@ -57,24 +55,24 @@ def upgrade(): resource_type=Organization.__name__, ) print('Adding environment resource permissions...') - envs = session.query(Environment).all() + envs = session.execute(text('SELECT "environmentUri", "SamlGroupName" FROM environment')).fetchall() for env in envs: ResourcePolicyService.attach_resource_policy( session=session, group=env.SamlGroupName, resource_uri=env.environmentUri, permissions=[ENFORCE_METADATA_FORM], - resource_type=Environment.__name__, + resource_type='Environment', ) print('Adding dataset resource permissions...') - datasets = session.query(DatasetBase).all() + datasets = session.execute(text('SELECT "datasetUri", "SamlAdminGroupName" FROM dataset')).fetchall() for dataset in datasets: ResourcePolicyService.attach_resource_policy( session=session, group=dataset.SamlAdminGroupName, resource_uri=dataset.datasetUri, permissions=[ENFORCE_METADATA_FORM], - resource_type=DatasetBase.__name__, + resource_type='DatasetBase', ) @@ -83,13 +81,13 @@ def downgrade(): op.drop_column('metadata_form_enforcement_rule', 'homeEntity') bind = op.get_bind() session = orm.Session(bind=bind) - all_environments = session.query(Environment).all() + all_environments = session.execute(text('SELECT "environmentUri", "SamlGroupName" FROM environment')).fetchall() for env in all_environments: policies = ResourcePolicyService.find_resource_policies( session=session, group=env.SamlGroupName, resource_uri=env.environmentUri, - resource_type=Environment.__name__, + resource_type='Environment', permissions=[ENFORCE_METADATA_FORM], ) for policy in policies: @@ -113,14 +111,14 @@ def downgrade(): session.delete(resource_pol_permission) session.commit() - datasets = session.query(DatasetBase).all() + datasets = session.execute(text('SELECT "datasetUri", "SamlAdminGroupName" FROM dataset')).fetchall() for dataset in datasets: policies = ResourcePolicyService.find_resource_policies( session=session, group=dataset.SamlAdminGroupName, resource_uri=dataset.datasetUri, permissions=[ENFORCE_METADATA_FORM], - resource_type=DatasetBase.__name__, + resource_type='DatasetBase', ) for policy in policies: diff --git a/frontend/src/modules/Environments/components/EnvironmentConsoleAccess.js b/frontend/src/modules/Environments/components/EnvironmentConsoleAccess.js index c414c9429..02f5342b3 100644 --- a/frontend/src/modules/Environments/components/EnvironmentConsoleAccess.js +++ b/frontend/src/modules/Environments/components/EnvironmentConsoleAccess.js @@ -37,6 +37,16 @@ export const EnvironmentConsoleAccess = ({ environment }) => { {environment.EnvironmentDefaultIAMRoleArn} + {environment.PermissionsBoundaryPolicyArn && ( + + + IAM Permissions Boundary + + + {environment.PermissionsBoundaryPolicyArn} + + + )} ); }; diff --git a/frontend/src/modules/Environments/services/getEnvironment.js b/frontend/src/modules/Environments/services/getEnvironment.js index f743be2fc..509ec0520 100644 --- a/frontend/src/modules/Environments/services/getEnvironment.js +++ b/frontend/src/modules/Environments/services/getEnvironment.js @@ -23,6 +23,7 @@ export const getEnvironment = ({ environmentUri }) => ({ EnvironmentDefaultIAMRoleName EnvironmentDefaultIAMRoleImported resourcePrefix + PermissionsBoundaryPolicyArn subscriptionsEnabled subscriptionsProducersTopicImported subscriptionsConsumersTopicImported diff --git a/frontend/src/modules/Environments/views/EnvironmentCreateForm.js b/frontend/src/modules/Environments/views/EnvironmentCreateForm.js index de11aaac3..fe21692fb 100644 --- a/frontend/src/modules/Environments/views/EnvironmentCreateForm.js +++ b/frontend/src/modules/Environments/views/EnvironmentCreateForm.js @@ -173,6 +173,7 @@ const EnvironmentCreateForm = (props) => { description: values.description, region: values.region, EnvironmentDefaultIAMRoleArn: values.EnvironmentDefaultIAMRoleArn, + PermissionsBoundaryPolicyArn: values.PermissionsBoundaryPolicyArn, resourcePrefix: values.resourcePrefix, vpcId: values.vpcId, subnetIds: values.subnetIds, @@ -440,6 +441,7 @@ const EnvironmentCreateForm = (props) => { pipelinesEnabled: isModuleEnabled(ModuleNames.DATAPIPELINES), omicsEnabled: isModuleEnabled(ModuleNames.OMICS), EnvironmentDefaultIAMRoleArn: '', + PermissionsBoundaryPolicyArn: '', resourcePrefix: 'dataall', vpcId: '', subnetIds: [] @@ -1028,6 +1030,18 @@ EOF`; variant="outlined" /> + + + {values.mlStudiosEnabled && ( diff --git a/frontend/src/modules/Environments/views/EnvironmentEditForm.js b/frontend/src/modules/Environments/views/EnvironmentEditForm.js index c1945db37..6e16a6750 100644 --- a/frontend/src/modules/Environments/views/EnvironmentEditForm.js +++ b/frontend/src/modules/Environments/views/EnvironmentEditForm.js @@ -100,6 +100,7 @@ const EnvironmentEditForm = (props) => { tags: values.tags, description: values.description, resourcePrefix: values.resourcePrefix, + PermissionsBoundaryPolicyArn: values.PermissionsBoundaryPolicyArn, vpcId: values.vpcId, subnetIds: values.subnetIds, parameters: [ @@ -246,7 +247,9 @@ const EnvironmentEditForm = (props) => { dashboardsEnabled: env.parameters['dashboardsEnabled'] === 'true', omicsEnabled: env.parameters['omicsEnabled'] === 'true', - resourcePrefix: env.resourcePrefix + resourcePrefix: env.resourcePrefix, + PermissionsBoundaryPolicyArn: + env.PermissionsBoundaryPolicyArn || '' }} validationSchema={Yup.object().shape({ label: Yup.string() @@ -418,6 +421,18 @@ const EnvironmentEditForm = (props) => { variant="outlined" /> + + + {!previousEnvMLStudioEnabled && diff --git a/tests_new/integration_tests/core/environment/global_conftest.py b/tests_new/integration_tests/core/environment/global_conftest.py index 173645ba1..308a99e6e 100644 --- a/tests_new/integration_tests/core/environment/global_conftest.py +++ b/tests_new/integration_tests/core/environment/global_conftest.py @@ -26,7 +26,9 @@ @contextmanager -def create_env(client, env_name, group, org_uri, account_id, region, tags=[], retain=False): +def create_env( + client, env_name, group, org_uri, account_id, region, tags=[], retain=False, PermissionsBoundaryPolicyArn=None +): env = None errors = False try: @@ -38,6 +40,7 @@ def create_env(client, env_name, group, org_uri, account_id, region, tags=[], re awsAccountId=account_id, region=region, tags=tags, + PermissionsBoundaryPolicyArn=PermissionsBoundaryPolicyArn, ) check_stack_ready(client, env.environmentUri, env.stack.stackUri) env = get_environment(client, env.environmentUri) @@ -78,7 +81,14 @@ def delete_env(client, env): def session_env1(client1, group1, group5, org1, session_id, testdata): envdata = testdata.envs['session_env1'] with create_env( - client1, 'session_env1', group1, org1.organizationUri, envdata.accountId, envdata.region, tags=[session_id] + client1, + 'session_env1', + group1, + org1.organizationUri, + envdata.accountId, + envdata.region, + tags=[session_id], + PermissionsBoundaryPolicyArn='arn:aws:iam::aws:policy/AdministratorAccess', ) as env: invite_group_on_env(client1, env.environmentUri, group5, ['CREATE_DATASET', 'CREATE_SHARE_OBJECT']) yield env diff --git a/tests_new/integration_tests/core/environment/queries.py b/tests_new/integration_tests/core/environment/queries.py index cae063ae7..15d4ce04e 100644 --- a/tests_new/integration_tests/core/environment/queries.py +++ b/tests_new/integration_tests/core/environment/queries.py @@ -16,6 +16,7 @@ EnvironmentDefaultIAMRoleName EnvironmentDefaultIAMRoleImported resourcePrefix +PermissionsBoundaryPolicyArn subscriptionsEnabled subscriptionsProducersTopicImported subscriptionsConsumersTopicImported @@ -51,7 +52,9 @@ """ -def create_environment(client, name, group, organizationUri, awsAccountId, region, tags): +def create_environment( + client, name, group, organizationUri, awsAccountId, region, tags, PermissionsBoundaryPolicyArn=None +): query = { 'operationName': 'CreateEnvironment', 'variables': { @@ -65,6 +68,7 @@ def create_environment(client, name, group, organizationUri, awsAccountId, regio 'tags': tags, 'type': 'IntegrationTesting', 'parameters': [], + 'PermissionsBoundaryPolicyArn': PermissionsBoundaryPolicyArn, } }, 'query': f""" diff --git a/tests_new/integration_tests/core/environment/test_environment.py b/tests_new/integration_tests/core/environment/test_environment.py index 13fac2745..e26e4f0ff 100644 --- a/tests_new/integration_tests/core/environment/test_environment.py +++ b/tests_new/integration_tests/core/environment/test_environment.py @@ -3,6 +3,8 @@ from assertpy import assert_that +from integration_tests.aws_clients.iam import IAMClient +from integration_tests.aws_clients.sts import STSClient from integration_tests.core.environment.queries import ( get_environment, update_environment, @@ -121,3 +123,15 @@ def test_add_consumption_role_unauthorized(client2, session_env2, group1): def test_create_crossaccount_env(client5, session_cross_acc_env_1, group5): assert_that(session_cross_acc_env_1.stack.status).is_in('CREATE_COMPLETE', 'UPDATE_COMPLETE') + + +def test_env_permissions_boundary(client1, session_env1): + env = get_environment(client1, session_env1.environmentUri) + boundary_arn = 'arn:aws:iam::aws:policy/AdministratorAccess' + assert_that(env.PermissionsBoundaryPolicyArn).is_equal_to(boundary_arn) + + role_arn = f'arn:aws:iam::{env.AwsAccountId}:role/dataall-integration-tests-role-{env.region}' + session = STSClient(role_arn=role_arn, region=env.region).get_refreshable_session() + iam_client = IAMClient(session=session, region=env.region) + role = iam_client.get_role(env.EnvironmentDefaultIAMRoleName) + assert_that(role['Role']['PermissionsBoundary']['PermissionsBoundaryArn']).is_equal_to(boundary_arn)