From d216f60a6e312feda200b57b896d9268d0184899 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Thu, 22 Aug 2024 14:45:59 -0400 Subject: [PATCH] Sync to 2 different DAB team member roles (#2241) Log and ignore removing users that can not be removed Issue: AAH-3358 --- CHANGES/3358.misc | 1 + galaxy_ng/app/settings.py | 2 + galaxy_ng/app/signals/handlers.py | 67 ++++++++++++------- .../integration/dab/test_dab_rbac_contract.py | 6 +- 4 files changed, 52 insertions(+), 24 deletions(-) create mode 100644 CHANGES/3358.misc diff --git a/CHANGES/3358.misc b/CHANGES/3358.misc new file mode 100644 index 0000000000..a903be2120 --- /dev/null +++ b/CHANGES/3358.misc @@ -0,0 +1 @@ +sync RBAC assignments from JWT, specific Team Member role, to the pulp group users relationship diff --git a/galaxy_ng/app/settings.py b/galaxy_ng/app/settings.py index c6d2e1bc09..427108df9b 100644 --- a/galaxy_ng/app/settings.py +++ b/galaxy_ng/app/settings.py @@ -422,6 +422,8 @@ ANSIBLE_BASE_ALLOW_SINGLETON_TEAM_ROLES = True # Pass ignore_conflicts=False for bulk_create calls for role evaluations ANSIBLE_BASE_EVALUATIONS_IGNORE_CONFLICTS = False +# The JWT roles should never be assigned to users through the API +ALLOW_LOCAL_ASSIGNING_JWT_ROLES = False # Set up managed role definitions ANSIBLE_BASE_MANAGED_ROLE_REGISTRY = { 'platform_auditor': {'name': 'Platform Auditor', 'shortname': 'sys_auditor'}, diff --git a/galaxy_ng/app/signals/handlers.py b/galaxy_ng/app/signals/handlers.py index 26faa1937d..b0bc312bb9 100644 --- a/galaxy_ng/app/signals/handlers.py +++ b/galaxy_ng/app/signals/handlers.py @@ -15,6 +15,7 @@ from django.db.models import CharField, Value from django.db.models.functions import Concat from django.contrib.contenttypes.models import ContentType +from django.contrib.auth.models import Group from django.conf import settings from rest_framework.exceptions import ValidationError from django.apps import apps @@ -121,6 +122,7 @@ def _update_metadata(): # ___ DAB RBAC ___ TEAM_MEMBER_ROLE = 'Galaxy Team Member' +SHARED_TEAM_ROLE = 'Team Member' def create_managed_roles(*args, **kwargs) -> None: @@ -495,7 +497,7 @@ def copy_dab_user_role_assignment(sender, instance, created, **kwargs): if rbac_signal_in_progress(): return with dab_rbac_signals(): - if instance.role_definition.name == TEAM_MEMBER_ROLE and \ + if instance.role_definition.name in (TEAM_MEMBER_ROLE, SHARED_TEAM_ROLE) and \ isinstance(instance, RoleUserAssignment): instance.content_object.group.user_set.add(instance.user) return @@ -508,12 +510,23 @@ def delete_dab_user_role_assignment(sender, instance, **kwargs): if rbac_signal_in_progress(): return with dab_rbac_signals(): - if instance.role_definition.name == TEAM_MEMBER_ROLE and \ + if instance.role_definition.name in (TEAM_MEMBER_ROLE, SHARED_TEAM_ROLE) and \ isinstance(instance, RoleUserAssignment): + if instance.role_definition.name == TEAM_MEMBER_ROLE: + dup_name = SHARED_TEAM_ROLE + else: + dup_name = TEAM_MEMBER_ROLE # If the assignment does not have a content_object then it may be a global group role # this type of role is not compatible with DAB RBAC and what we do is still TBD if instance.content_object: - instance.content_object.group.user_set.remove(instance.user) + # Only remove assignment if other role does not grant this, otherwise leave it + other_assignment_qs = RoleUserAssignment.objects.filter( + role_definition__name=dup_name, + user=instance.user, + object_id=instance.object_id + ) + if not other_assignment_qs.exists(): + instance.content_object.group.user_set.remove(instance.user) return _unapply_dab_assignment(instance) @@ -545,17 +558,40 @@ def copy_dab_group_to_role(instance, action, model, pk_set, reverse, **kwargs): return member_rd = RoleDefinition.objects.get(name=TEAM_MEMBER_ROLE) + shared_member_rd = RoleDefinition.objects.get(name=SHARED_TEAM_ROLE) if reverse: - # NOTE: for we might prefer to use pk_set - # but it appears to be incorrectly empty on the reverse signal, - # the models itself on post_ actions seems good so we use that - team = Team.objects.get(group_id=instance.pk) + groups = [instance] + else: + if action == 'post_clear': + qs = RoleUserAssignment.objects.filter(role_definition=member_rd, user=instance) + groups = [assignment.content_object.group for assignment in qs] + else: + groups = Group.objects.filter(pk__in=pk_set) + + # For every group affected by the change, assure that the DAB role assignments + # are changed to match the users in the pulp group + for group in groups: + team = Team.objects.get(group_id=group.pk) current_dab_members = set( assignment.user for assignment in RoleUserAssignment.objects.filter( role_definition=member_rd, object_id=team.pk ) ) - desired_members = set(instance.user_set.all()) + current_dab_shared_members = set( + assignment.user for assignment in RoleUserAssignment.objects.filter( + role_definition=shared_member_rd, object_id=team.pk + ) + ) + current_pulp_members = set(group.user_set.all()) + not_allowed = current_dab_shared_members - current_pulp_members + if not_allowed: + usernames = ", ".join([u.username for u in not_allowed]) + logger.info( + f'Can not remove users {usernames} from team {team.name}, ' + 'because they are managed by the resource provider' + ) + # The local team member role should track all users not tracked in the shared member role + desired_members = current_pulp_members - current_dab_shared_members users_to_add = desired_members - current_dab_members users_to_remove = current_dab_members - desired_members with dab_rbac_signals(): @@ -563,21 +599,6 @@ def copy_dab_group_to_role(instance, action, model, pk_set, reverse, **kwargs): member_rd.give_permission(user, team) for user in users_to_remove: member_rd.remove_permission(user, team) - return - - with dab_rbac_signals(): - if action == 'post_add': - for group_id in pk_set: - team = Team.objects.get(group_id=group_id) - member_rd.give_permission(instance, team) - elif action == 'post_remove': - for group_id in pk_set: - team = Team.objects.get(group_id=group_id) - member_rd.remove_permission(instance, team) - elif action == 'post_clear': - qs = RoleUserAssignment.objects.filter(role_definition=member_rd, user=instance) - for assignment in qs: - member_rd.remove_permission(instance, assignment.content_object) m2m_changed.connect(copy_dab_group_to_role, sender=User.groups.through) diff --git a/galaxy_ng/tests/integration/dab/test_dab_rbac_contract.py b/galaxy_ng/tests/integration/dab/test_dab_rbac_contract.py index 6923f4f7ef..548640a953 100644 --- a/galaxy_ng/tests/integration/dab/test_dab_rbac_contract.py +++ b/galaxy_ng/tests/integration/dab/test_dab_rbac_contract.py @@ -601,7 +601,11 @@ def _rf(user_id, group_id, expected=True): assignment_r = gc.get( "_ui/v2/role_user_assignments/", - params={"user": user['id'], "content_type__model": "team"}, + params={ + "user": user['id'], + "content_type__model": "team", + "role_definition__name": "Galaxy Team Member" + }, ) group_ids = [assignment["object_id"] for assignment in assignment_r["results"]] if expected: