Skip to content

Commit

Permalink
Merge pull request #3369 from uktrade/feat/update-iam-role-team-membe…
Browse files Browse the repository at this point in the history
…rship

fix: bug when email address change users not able to access to new team folders
  • Loading branch information
michalc authored Dec 3, 2024
2 parents 38051d6 + 4467c9a commit 9cb31cb
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 103 deletions.
4 changes: 2 additions & 2 deletions .envs/sample.env
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ METRICS_SERVICE_DISCOVERY_BASIC_AUTH_USER=user
METRICS_SERVICE_DISCOVERY_BASIC_AUTH_PASSWORD=password
REDIS_URL=redis://data-workspace-redis:6379
SECRET_KEY=something-secret
S3_ASSUME_ROLE_POLICY_DOCUMENT_BASE64=InsnVmVyc2lvbic6ICcyMDEyLTEwLTE3JyxcbiAnU3RhdGVtZW50JzogW3snU2lkJzogJycsXG4gICAnRWZmZWN0JzogJ0FsbG93JyxcbiAgICdBY3Rpb24nOiAnc3RzOkFzc3VtZVJvbGUnLFxuICAgJ1ByaW5jaXBhbCc6IHsnQVdTJzogJ2Fybjphd3M6aWFtOjowMDAwMDAwMDAwMDA6cm9sZS9sb2NhbC1kZXYtYWRtaW4tdGFzaycsXG4gICAgJ1NlcnZpY2UnOiAnZWNzLXRhc2tzLmFtYXpvbmF3cy5jb20nfX0sXG4gIHsnU2lkJzogJycsXG4gICAnRWZmZWN0JzogJ0FsbG93JyxcbiAgICdBY3Rpb24nOiAnc3RzOkFzc3VtZVJvbGUnLFxuICAgJ1ByaW5jaXBhbCc6IHsnQVdTJzogJ2Fybjphd3M6aWFtOjowMDAwMDAwMDAwMDA6dXNlci9sb2NhbCd9fV19XG4i
S3_ASSUME_ROLE_POLICY_DOCUMENT_BASE64=eyJWZXJzaW9uIjogIjIwMTItMTAtMTciLAogIlN0YXRlbWVudCI6IFt7IlNpZCI6ICIiLAogICAiRWZmZWN0IjogIkFsbG93IiwKICAgIkFjdGlvbiI6ICJzdHM6QXNzdW1lUm9sZSIsCiAgICJQcmluY2lwYWwiOiB7IkFXUyI6ICJhcm46YXdzOmlhbTo6MDAwMDAwMDAwMDAwOnJvbGUvbG9jYWwtZGV2LWFkbWluLXRhc2siLAogICAgIlNlcnZpY2UiOiAiZWNzLXRhc2tzLmFtYXpvbmF3cy5jb20ifX0sCiAgeyJTaWQiOiAiIiwKICAgIkVmZmVjdCI6ICJBbGxvdyIsCiAgICJBY3Rpb24iOiAic3RzOkFzc3VtZVJvbGUiLAogICAiUHJpbmNpcGFsIjogeyJBV1MiOiAiYXJuOmF3czppYW06OjAwMDAwMDAwMDAwMDp1c2VyL2xvY2FsIn19XX0=
S3_POLICY_NAME=my-policy
S3_POLICY_DOCUMENT_TEMPLATE_BASE64=ewogICJWZXJzaW9uIjogIjIwMTItMTAtMTciLAogICJTdGF0ZW1lbnQiOiBbCiAgICB7CiAgICAgICJTaWQiOiAiIiwKICAgICAgIkVmZmVjdCI6ICJBbGxvdyIsCiAgICAgICJBY3Rpb24iOiAiczM6TGlzdEJ1Y2tldCIsCiAgICAgICJSZXNvdXJjZSI6ICJhcm46YXdzOnMzOjo6bm90ZWJvb2tzLmFuYWx5c2lzd29ya3NwYWNlLmRldi51a3RyYWRlLmlvIiwKICAgICAgIkNvbmRpdGlvbiI6IHsKICAgICAgICAiU3RyaW5nTGlrZSI6IHsKICAgICAgICAgICJzMzpwcmVmaXgiOiAiX19TM19QUkVGSVhfXyoiCiAgICAgICAgfQogICAgICB9CiAgICB9LAogICAgewogICAgICAiU2lkIjogIiIsCiAgICAgICJFZmZlY3QiOiAiQWxsb3ciLAogICAgICAiQWN0aW9uIjogWwogICAgICAgICJzMzpQdXRPYmplY3QiLAogICAgICAgICJzMzpHZXRPYmplY3QiLAogICAgICAgICJzMzpEZWxldGVPYmplY3QiCiAgICAgIF0sCiAgICAgICJSZXNvdXJjZSI6ICJhcm46YXdzOnMzOjo6bm90ZWJvb2tzLmFuYWx5c2lzd29ya3NwYWNlLmRldi51a3RyYWRlLmlvL19fUzNfUFJFRklYX18qIgogICAgfSwKICAgIHsKICAgICAgIlNpZCI6ICIiLAogICAgICAiRWZmZWN0IjogIkFsbG93IiwKICAgICAgIkFjdGlvbiI6IFsKICAgICAgICAiZWxhc3RpY2ZpbGVzeXN0ZW06Q2xpZW50V3JpdGUiLAogICAgICAgICJlbGFzdGljZmlsZXN5c3RlbTpDbGllbnRNb3VudCIKICAgICAgXSwKICAgICAgIlJlc291cmNlIjogImFybjphd3M6ZWxhc3RpY2ZpbGVzeXN0ZW06ZXUtd2VzdC0yOjE2NTU2MjEwNzI3MDpmaWxlLXN5c3RlbS9mcy03ZGIwZmY4YyIsCiAgICAgICJDb25kaXRpb24iOiB7CiAgICAgICAgIlN0cmluZ0VxdWFscyI6IHsKICAgICAgICAgICJlbGFzdGljZmlsZXN5c3RlbTpBY2Nlc3NQb2ludEFybiI6ICJhcm46YXdzOmVsYXN0aWNmaWxlc3lzdGVtOmV1LXdlc3QtMjoxNjU1NjIxMDcyNzA6YWNjZXNzLXBvaW50L19fQUNDRVNTX1BPSU5UX0lEX18iCiAgICAgICAgfQogICAgICB9CiAgICB9CiAgXQp9
S3_POLICY_DOCUMENT_TEMPLATE_BASE64=ewogICJWZXJzaW9uIjogIjIwMTItMTAtMTciLAogICJTdGF0ZW1lbnQiOiBbCiAgICB7CiAgICAgICJFZmZlY3QiOiAiQWxsb3ciLAogICAgICAiQWN0aW9uIjogInMzOkxpc3RCdWNrZXQiLAogICAgICAiUmVzb3VyY2UiOiAiYXJuOmF3czpzMzo6Om5vdGVib29rcy5kYXRhd29ya3NwYWNlLmxvY2FsIiwKICAgICAgIkNvbmRpdGlvbiI6IHsKICAgICAgICAiRm9yQW55VmFsdWU6U3RyaW5nTGlrZSI6IHsKICAgICAgICAgICJzMzpwcmVmaXgiOiAiX19TM19QUkVGSVhFU19fIgogICAgICAgIH0KICAgICAgfQogICAgfSwKICAgIHsKICAgICAgIkVmZmVjdCI6ICJBbGxvdyIsCiAgICAgICJBY3Rpb24iOiBbCiAgICAgICAgInMzOlB1dE9iamVjdCIsCiAgICAgICAgInMzOkdldE9iamVjdCIsCiAgICAgICAgInMzOkRlbGV0ZU9iamVjdCIKICAgICAgXSwKICAgICAgIlJlc291cmNlIjogIl9fUzNfQlVDS0VUX0FSTlNfXyIKICAgIH0sCiAgICB7CiAgICAgICJFZmZlY3QiOiAiQWxsb3ciLAogICAgICAiQWN0aW9uIjogInMzOkxpc3RCdWNrZXQiLAogICAgICAiUmVzb3VyY2UiOiAiYXJuOmF3czpzMzo6Om5vdGVib29rcy5kYXRhd29ya3NwYWNlLmxvY2FsIgogICAgfSwKICAgIHsKICAgICAgIkVmZmVjdCI6ICJBbGxvdyIsCiAgICAgICJBY3Rpb24iOiBbCiAgICAgICAgImVsYXN0aWNmaWxlc3lzdGVtOkNsaWVudFdyaXRlIiwKICAgICAgICAiZWxhc3RpY2ZpbGVzeXN0ZW06Q2xpZW50TW91bnQiCiAgICAgIF0sCiAgICAgICJSZXNvdXJjZSI6ICJhcm46YXdzOmVsYXN0aWNmaWxlc3lzdGVtOmV1LXdlc3QtMjowMDAwMDAwMDAwMDA6ZmlsZS1zeXN0ZW0vZnMtNDNiMGZmYjIiLAogICAgICAiQ29uZGl0aW9uIjogewogICAgICAgICJTdHJpbmdFcXVhbHMiOiB7CiAgICAgICAgICAiZWxhc3RpY2ZpbGVzeXN0ZW06QWNjZXNzUG9pbnRBcm4iOiAiYXJuOmF3czplbGFzdGljZmlsZXN5c3RlbTpldS13ZXN0LTI6MDAwMDAwMDAwMDAwOmFjY2Vzcy1wb2ludC9fX0FDQ0VTU19QT0lOVF9JRF9fIgogICAgICAgIH0KICAgICAgfQogICAgfQogIF0KfQ==
S3_PERMISSIONS_BOUNDARY_ARN=arn:aws:iam::123456789123:policy/local-notebook-task-boundary
S3_ROLE_PREFIX=my-prefix
S3_NOTEBOOKS_BUCKET_ARN=aws:arn:from:terraform
Expand Down
13 changes: 9 additions & 4 deletions .envs/test.env
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ DATA_DB__test_external_db2__PORT=5432
APPSTREAM_URL=https://url.to.appstream
NOTEBOOKS_URL=https://url.to.notebooks/

S3_LOCAL_ENDPOINT_URL=http://data-workspace-localstack:4566
STS_LOCAL_ENDPOINT_URL=http://data-workspace-localstack:4566
IAM_LOCAL_ENDPOINT_URL=http://data-workspace-localstack:4566

APPLICATION_ROOT_DOMAIN=dataworkspace.test:8000
APPLICATION_TEMPLATES__1__HOST_BASENAME=testapplication
APPLICATION_TEMPLATES__1__NICE_NAME='Test Application'
Expand All @@ -59,12 +63,13 @@ ZENDESK_SUBDOMAIN=subdomain
ZENDESK_TOKEN=abcd
ZENDESK_SERVICE_FIELD_ID=654321
ZENDESK_SERVICE_FIELD_VALUE=field_value
S3_ASSUME_ROLE_POLICY_DOCUMENT_BASE64=e30=
S3_ASSUME_ROLE_POLICY_DOCUMENT_BASE64=eyJWZXJzaW9uIjogIjIwMTItMTAtMTciLAogIlN0YXRlbWVudCI6IFt7IlNpZCI6ICIiLAogICAiRWZmZWN0IjogIkFsbG93IiwKICAgIkFjdGlvbiI6ICJzdHM6QXNzdW1lUm9sZSIsCiAgICJQcmluY2lwYWwiOiB7IkFXUyI6ICJhcm46YXdzOmlhbTo6MDAwMDAwMDAwMDAwOnJvbGUvbG9jYWwtZGV2LWFkbWluLXRhc2siLAogICAgIlNlcnZpY2UiOiAiZWNzLXRhc2tzLmFtYXpvbmF3cy5jb20ifX0sCiAgeyJTaWQiOiAiIiwKICAgIkVmZmVjdCI6ICJBbGxvdyIsCiAgICJBY3Rpb24iOiAic3RzOkFzc3VtZVJvbGUiLAogICAiUHJpbmNpcGFsIjogeyJBV1MiOiAiYXJuOmF3czppYW06OjAwMDAwMDAwMDAwMDp1c2VyL2xvY2FsIn19XX0=
S3_POLICY_NAME=my-policy
S3_POLICY_DOCUMENT_TEMPLATE_BASE64=e30=
S3_PERMISSIONS_BOUNDARY_ARN=my-arn
S3_POLICY_DOCUMENT_TEMPLATE_BASE64=ewogICJWZXJzaW9uIjogIjIwMTItMTAtMTciLAogICJTdGF0ZW1lbnQiOiBbCiAgICB7CiAgICAgICJFZmZlY3QiOiAiQWxsb3ciLAogICAgICAiQWN0aW9uIjogInMzOkxpc3RCdWNrZXQiLAogICAgICAiUmVzb3VyY2UiOiAiYXJuOmF3czpzMzo6Om5vdGVib29rcy5kYXRhd29ya3NwYWNlLmxvY2FsIiwKICAgICAgIkNvbmRpdGlvbiI6IHsKICAgICAgICAiRm9yQW55VmFsdWU6U3RyaW5nTGlrZSI6IHsKICAgICAgICAgICJzMzpwcmVmaXgiOiAiX19TM19QUkVGSVhFU19fIgogICAgICAgIH0KICAgICAgfQogICAgfSwKICAgIHsKICAgICAgIkVmZmVjdCI6ICJBbGxvdyIsCiAgICAgICJBY3Rpb24iOiBbCiAgICAgICAgInMzOlB1dE9iamVjdCIsCiAgICAgICAgInMzOkdldE9iamVjdCIsCiAgICAgICAgInMzOkRlbGV0ZU9iamVjdCIKICAgICAgXSwKICAgICAgIlJlc291cmNlIjogIl9fUzNfQlVDS0VUX0FSTlNfXyIKICAgIH0sCiAgICB7CiAgICAgICJFZmZlY3QiOiAiQWxsb3ciLAogICAgICAiQWN0aW9uIjogInMzOkxpc3RCdWNrZXQiLAogICAgICAiUmVzb3VyY2UiOiAiYXJuOmF3czpzMzo6Om5vdGVib29rcy5kYXRhd29ya3NwYWNlLmxvY2FsIgogICAgfSwKICAgIHsKICAgICAgIkVmZmVjdCI6ICJBbGxvdyIsCiAgICAgICJBY3Rpb24iOiBbCiAgICAgICAgImVsYXN0aWNmaWxlc3lzdGVtOkNsaWVudFdyaXRlIiwKICAgICAgICAiZWxhc3RpY2ZpbGVzeXN0ZW06Q2xpZW50TW91bnQiCiAgICAgIF0sCiAgICAgICJSZXNvdXJjZSI6ICJhcm46YXdzOmVsYXN0aWNmaWxlc3lzdGVtOmV1LXdlc3QtMjowMDAwMDAwMDAwMDA6ZmlsZS1zeXN0ZW0vZnMtNDNiMGZmYjIiLAogICAgICAiQ29uZGl0aW9uIjogewogICAgICAgICJTdHJpbmdFcXVhbHMiOiB7CiAgICAgICAgICAiZWxhc3RpY2ZpbGVzeXN0ZW06QWNjZXNzUG9pbnRBcm4iOiAiYXJuOmF3czplbGFzdGljZmlsZXN5c3RlbTpldS13ZXN0LTI6MDAwMDAwMDAwMDAwOmFjY2Vzcy1wb2ludC9fX0FDQ0VTU19QT0lOVF9JRF9fIgogICAgICAgIH0KICAgICAgfQogICAgfQogIF0KfQ==
S3_PERMISSIONS_BOUNDARY_ARN=arn:aws:iam::123456789123:policy/local-notebook-task-boundary
S3_ROLE_PREFIX=my-prefix
S3_NOTEBOOKS_BUCKET_ARN=my-arn
S3_NOTEBOOKS_BUCKET_ARN=arn:aws:s3:::notebooks.dataworkspace.local
NOTEBOOKS_BUCKET=notebooks.dataworkspace.local

ARANGO_DB__HOST=data-workspace-arango
ARANGO_DB__PASSWORD=arango
Expand Down
6 changes: 2 additions & 4 deletions dataworkspace/dataworkspace/apps/accounts/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@
AdminVisualisationUserPermission,
)
from dataworkspace.apps.applications.models import ApplicationInstance
from dataworkspace.apps.applications.utils import (
create_tools_access_iam_role_task,
sync_quicksight_permissions,
)
from dataworkspace.apps.applications.utils import sync_quicksight_permissions
from dataworkspace.apps.core.utils import create_tools_access_iam_role_task
from dataworkspace.apps.eventlog.models import EventLog
from dataworkspace.apps.eventlog.utils import log_permission_change
from dataworkspace.apps.explorer.schema import clear_schema_info_cache_for_user
Expand Down
28 changes: 1 addition & 27 deletions dataworkspace/dataworkspace/apps/applications/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
from dataworkspace.apps.core.utils import (
GLOBAL_LOCK_ID,
close_all_connections_if_not_in_atomic_block,
create_tools_access_iam_role,
create_tools_access_iam_role_task,
database_dsn,
stable_identification_suffix,
source_tables_for_app,
Expand Down Expand Up @@ -1128,32 +1128,6 @@ def create_user_from_sso(
return user


@celery_app.task(autoretry_for=(redis.exceptions.LockError,))
@close_all_connections_if_not_in_atomic_block
def create_tools_access_iam_role_task(user_id):
with cache.lock(
"create_tools_access_iam_role_task",
blocking_timeout=0,
timeout=360,
):
_do_create_tools_access_iam_role(user_id)


def _do_create_tools_access_iam_role(user_id):
User = get_user_model()
try:
user = User.objects.get(id=user_id)
except User.DoesNotExist:
logger.exception("User id %d does not exist", user_id)
else:
create_tools_access_iam_role(
user.id,
user.email,
user.profile.home_directory_efs_access_point_id,
)
gevent.sleep(1)


@celery_app.task(autoretry_for=(redis.exceptions.LockError,))
@close_all_connections_if_not_in_atomic_block
def sync_s3_sso_users():
Expand Down
147 changes: 86 additions & 61 deletions dataworkspace/dataworkspace/apps/core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -926,63 +926,6 @@ def get_s3_prefix(user_sso_id):
return "user/federated/" + stable_identification_suffix(user_sso_id, short=False) + "/"


def create_tools_access_iam_role(user_id, user_email_address, access_point_id):
user = get_user_model().objects.get(id=user_id)
s3_prefixes = get_user_s3_prefixes(user)
if user.profile.tools_access_role_arn:
return user.profile.tools_access_role_arn, s3_prefixes

iam_client = get_iam_client()

assume_role_policy_document = settings.S3_ASSUME_ROLE_POLICY_DOCUMENT
permissions_boundary_arn = settings.S3_PERMISSIONS_BOUNDARY_ARN
role_prefix = settings.S3_ROLE_PREFIX

role_name = role_prefix + user_email_address
max_attempts = 10

try:
iam_client.create_role(
RoleName=role_name,
Path="/",
AssumeRolePolicyDocument=assume_role_policy_document,
PermissionsBoundary=permissions_boundary_arn,
)
except iam_client.exceptions.EntityAlreadyExistsException:
# If the role already exists, we might need to update its assume role
# policy document
for i in range(0, max_attempts):
try:
iam_client.update_assume_role_policy(
RoleName=role_name, PolicyDocument=assume_role_policy_document
)
except iam_client.exceptions.NoSuchEntityException:
if i == max_attempts - 1:
raise
gevent.sleep(1)
else:
break

for i in range(0, max_attempts):
try:
role_arn = iam_client.get_role(RoleName=role_name)["Role"]["Arn"]
logger.info("User (%s) set up AWS role... done (%s)", user_email_address, role_arn)
except iam_client.exceptions.NoSuchEntityException:
if i == max_attempts - 1:
raise
gevent.sleep(1)
else:
break

update_user_tool_access_policy(user, access_point_id)

# Cache the role_arn so it can be retrieved in the future without calling AWS
user.profile.tools_access_role_arn = role_arn
user.save()

return role_arn, s3_prefixes


def without_duplicates_preserve_order(seq):
# https://stackoverflow.com/a/480227/1319998
seen = set()
Expand Down Expand Up @@ -1268,6 +1211,90 @@ def get_team_prefixes(user):
]


@celery_app.task(autoretry_for=(redis.exceptions.LockError,))
@close_all_connections_if_not_in_atomic_block
def create_tools_access_iam_role_task(user_id, force=False):
with cache.lock(
f"create_tools_access_iam_role_task_{user_id}",
blocking_timeout=0,
timeout=360,
):
_do_create_tools_access_iam_role(user_id, force=force)


def _do_create_tools_access_iam_role(user_id, force=False):
User = get_user_model()
try:
user = User.objects.get(id=user_id)
except User.DoesNotExist:
logger.exception("User id %d does not exist", user_id)
else:
create_tools_access_iam_role(
user.id,
user.email,
user.profile.home_directory_efs_access_point_id,
force=force,
)
gevent.sleep(1)


def create_tools_access_iam_role(user_id, user_email_address, access_point_id, force=False):
user = get_user_model().objects.get(id=user_id)
s3_prefixes = get_user_s3_prefixes(user)
if not force and user.profile.tools_access_role_arn:
return user.profile.tools_access_role_arn, s3_prefixes

iam_client = get_iam_client()

assume_role_policy_document = settings.S3_ASSUME_ROLE_POLICY_DOCUMENT
permissions_boundary_arn = settings.S3_PERMISSIONS_BOUNDARY_ARN
role_prefix = settings.S3_ROLE_PREFIX

role_name = role_prefix + user_email_address
max_attempts = 10

try:
iam_client.create_role(
RoleName=role_name,
Path="/",
AssumeRolePolicyDocument=assume_role_policy_document,
PermissionsBoundary=permissions_boundary_arn,
)
except iam_client.exceptions.EntityAlreadyExistsException:
# If the role already exists, we might need to update its assume role
# policy document
for i in range(0, max_attempts):
try:
iam_client.update_assume_role_policy(
RoleName=role_name, PolicyDocument=assume_role_policy_document
)
except iam_client.exceptions.NoSuchEntityException:
if i == max_attempts - 1:
raise
gevent.sleep(1)
else:
break

for i in range(0, max_attempts):
try:
role_arn = iam_client.get_role(RoleName=role_name)["Role"]["Arn"]
logger.info("User (%s) set up AWS role... done (%s)", user_email_address, role_arn)
except iam_client.exceptions.NoSuchEntityException:
if i == max_attempts - 1:
raise
gevent.sleep(1)
else:
break

update_user_tool_access_policy(user, access_point_id)

# Cache the role_arn so it can be retrieved in the future without calling AWS
user.profile.tools_access_role_arn = role_arn
user.profile.save(update_fields=["tools_access_role_arn"])

return role_arn, s3_prefixes


def update_user_tool_access_policy(user, access_point_id):
max_attempts = 10
iam_client = get_iam_client()
Expand Down Expand Up @@ -1318,8 +1345,7 @@ def team_membership_post_save(instance, **kwargs):
When a team member is added to a team, update their tools access
to include the team s3 prefix
"""
if kwargs["created"] and instance.user.profile.tools_access_role_arn:
update_tools_access_policy_task.delay(instance.user_id)
create_tools_access_iam_role_task.delay(instance.user_id, force=True)


@receiver(post_delete, sender=TeamMembership)
Expand All @@ -1328,8 +1354,7 @@ def team_membership_post_delete(instance, **_):
When a team member is removed from a team, update their tools access
to remove the team s3 prefix
"""
if instance.user.profile.tools_access_role_arn:
update_tools_access_policy_task.delay(instance.user_id)
create_tools_access_iam_role_task.delay(instance.user_id, force=True)


def get_postgres_datatype_choices():
Expand Down
1 change: 1 addition & 0 deletions dataworkspace/dataworkspace/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ def aws_fargate_private_ip():
AWS_UPLOADS_BUCKET = env["UPLOADS_BUCKET"]
S3_LOCAL_ENDPOINT_URL = env.get("S3_LOCAL_ENDPOINT_URL", "")
STS_LOCAL_ENDPOINT_URL = env.get("STS_LOCAL_ENDPOINT_URL", "")
IAM_LOCAL_ENDPOINT_URL = env.get("IAM_LOCAL_ENDPOINT_URL", "")

YOUR_FILES_CONNECT_SRC = [
APPLICATION_ROOT_DOMAIN,
Expand Down
Loading

0 comments on commit 9cb31cb

Please sign in to comment.