From 873f5c0ecc56e5ac0225a202e23824c4a8a2aec1 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Mon, 22 Sep 2025 14:19:08 -0400 Subject: [PATCH] Remove some attached methods from User model (#15325) Remove archaic monkey patches (#15338) Remove some attached methods from User model Test user-org sublist URLs we did not test before --- awx/api/permissions.py | 4 ++-- awx/api/views/__init__.py | 2 -- awx/main/access.py | 12 ++++++++---- awx/main/models/__init__.py | 18 ------------------ awx/main/tests/functional/api/test_user.py | 16 ++++++++++++++++ .../dab_rbac/test_translation_layer.py | 2 +- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/awx/api/permissions.py b/awx/api/permissions.py index dcf6028579..ce422742db 100644 --- a/awx/api/permissions.py +++ b/awx/api/permissions.py @@ -10,7 +10,7 @@ from rest_framework import permissions # AWX from awx.main.access import check_user_access -from awx.main.models import Inventory, UnifiedJob +from awx.main.models import Inventory, UnifiedJob, Organization from awx.main.utils import get_object_or_400 logger = logging.getLogger('awx.api.permissions') @@ -228,7 +228,7 @@ class InventoryInventorySourcesUpdatePermission(ModelAccessPermission): class UserPermission(ModelAccessPermission): def check_post_permissions(self, request, view, obj=None): if not request.data: - return request.user.admin_of_organizations.exists() + return Organization.access_qs(request.user, 'change').exists() elif request.user.is_superuser: return True raise PermissionDenied() diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index 64e4c87a5d..7fa64fdc5f 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -1152,7 +1152,6 @@ class UserOrganizationsList(OrganizationCountsMixin, SubListAPIView): model = models.Organization serializer_class = serializers.OrganizationSerializer parent_model = models.User - relationship = 'organizations' def get_queryset(self): parent = self.get_parent_object() @@ -1166,7 +1165,6 @@ class UserAdminOfOrganizationsList(OrganizationCountsMixin, SubListAPIView): model = models.Organization serializer_class = serializers.OrganizationSerializer parent_model = models.User - relationship = 'admin_of_organizations' def get_queryset(self): parent = self.get_parent_object() diff --git a/awx/main/access.py b/awx/main/access.py index 4526703a8e..a8eca06717 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -639,7 +639,9 @@ class UserAccess(BaseAccess): prefetch_related = ('resource',) def filtered_queryset(self): - if settings.ORG_ADMINS_CAN_SEE_ALL_USERS and (self.user.admin_of_organizations.exists() or self.user.auditor_of_organizations.exists()): + if settings.ORG_ADMINS_CAN_SEE_ALL_USERS and ( + Organization.access_qs(self.user, 'change').exists() or Organization.access_qs(self.user, 'audit').exists() + ): qs = User.objects.all() else: qs = ( @@ -1224,7 +1226,9 @@ class TeamAccess(BaseAccess): ) def filtered_queryset(self): - if settings.ORG_ADMINS_CAN_SEE_ALL_USERS and (self.user.admin_of_organizations.exists() or self.user.auditor_of_organizations.exists()): + if settings.ORG_ADMINS_CAN_SEE_ALL_USERS and ( + Organization.access_qs(self.user, 'change').exists() or Organization.access_qs(self.user, 'audit').exists() + ): return self.model.objects.all() return self.model.objects.filter( Q(organization__in=Organization.accessible_pk_qs(self.user, 'member_role')) | Q(pk__in=self.model.accessible_pk_qs(self.user, 'read_role')) @@ -2564,7 +2568,7 @@ class NotificationTemplateAccess(BaseAccess): if settings.ANSIBLE_BASE_ROLE_SYSTEM_ACTIVATED: return self.model.access_qs(self.user, 'view') return self.model.objects.filter( - Q(organization__in=Organization.access_qs(self.user, 'add_notificationtemplate')) | Q(organization__in=self.user.auditor_of_organizations) + Q(organization__in=Organization.access_qs(self.user, 'add_notificationtemplate')) | Q(organization__in=Organization.access_qs(self.user, 'audit')) ).distinct() @check_superuser @@ -2599,7 +2603,7 @@ class NotificationAccess(BaseAccess): def filtered_queryset(self): return self.model.objects.filter( Q(notification_template__organization__in=Organization.access_qs(self.user, 'add_notificationtemplate')) - | Q(notification_template__organization__in=self.user.auditor_of_organizations) + | Q(notification_template__organization__in=Organization.access_qs(self.user, 'audit')) ).distinct() def can_delete(self, obj): diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index 95aa1a0396..e4b4b3100c 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -172,29 +172,11 @@ def cleanup_created_modified_by(sender, **kwargs): pre_delete.connect(cleanup_created_modified_by, sender=User) -@property -def user_get_organizations(user): - return Organization.access_qs(user, 'member') - - -@property -def user_get_admin_of_organizations(user): - return Organization.access_qs(user, 'change') - - -@property -def user_get_auditor_of_organizations(user): - return Organization.access_qs(user, 'audit') - - @property def created(user): return user.date_joined -User.add_to_class('organizations', user_get_organizations) -User.add_to_class('admin_of_organizations', user_get_admin_of_organizations) -User.add_to_class('auditor_of_organizations', user_get_auditor_of_organizations) User.add_to_class('created', created) diff --git a/awx/main/tests/functional/api/test_user.py b/awx/main/tests/functional/api/test_user.py index 1efaa9b2b1..b61ec71615 100644 --- a/awx/main/tests/functional/api/test_user.py +++ b/awx/main/tests/functional/api/test_user.py @@ -258,3 +258,19 @@ def test_user_verify_attribute_created(admin, get): for op, count in (('gt', 1), ('lt', 0)): resp = get(reverse('api:user_list') + f'?created__{op}={past}', admin) assert resp.data['count'] == count + + +@pytest.mark.django_db +def test_org_not_shown_in_admin_user_sublists(admin_user, get, organization): + for view_name in ('user_admin_of_organizations_list', 'user_organizations_list'): + url = reverse(f'api:{view_name}', kwargs={'pk': admin_user.pk}) + r = get(url, user=admin_user, expect=200) + assert organization.pk not in [org['id'] for org in r.data['results']] + + +@pytest.mark.django_db +def test_admin_user_not_shown_in_org_users(admin_user, get, organization): + for view_name in ('organization_users_list', 'organization_admins_list'): + url = reverse(f'api:{view_name}', kwargs={'pk': organization.pk}) + r = get(url, user=admin_user, expect=200) + assert admin_user.pk not in [u['id'] for u in r.data['results']] diff --git a/awx/main/tests/functional/dab_rbac/test_translation_layer.py b/awx/main/tests/functional/dab_rbac/test_translation_layer.py index dbbc4926e9..98ee58f5b8 100644 --- a/awx/main/tests/functional/dab_rbac/test_translation_layer.py +++ b/awx/main/tests/functional/dab_rbac/test_translation_layer.py @@ -186,7 +186,7 @@ def test_user_auditor_rel(organization, rando, setup_managed_roles): assert rando not in organization.auditor_role audit_rd = RoleDefinition.objects.get(name='Organization Audit') audit_rd.give_permission(rando, organization) - assert list(rando.auditor_of_organizations) == [organization] + assert list(Organization.access_qs(rando, 'audit')) == [organization] @pytest.mark.django_db