Merge pull request #4347 from AlanCoding/no_read_role

Kill off all model can_read access methods

Reviewed-by: Jake McDermott <yo@jakemcdermott.me>
             https://github.com/jakemcdermott

awx/main/access.py

@@ -834,10 +834,6 @@ class InventoryAccess(BaseAccess):
     def filtered_queryset(self, allowed=None, ad_hoc=None):
         return self.model.accessible_objects(self.user, 'read_role')
 
-    @check_superuser
-    def can_read(self, obj):
-        return self.user in obj.read_role
-
     @check_superuser
     def can_use(self, obj):
         return self.user in obj.use_role
@@ -907,9 +903,6 @@ class HostAccess(BaseAccess):
     def filtered_queryset(self):
         return self.model.objects.filter(inventory__in=Inventory.accessible_pk_qs(self.user, 'read_role'))
 
-    def can_read(self, obj):
-        return obj and self.user in obj.inventory.read_role
-
     def can_add(self, data):
         if not data:  # So the browseable API will work
             return Inventory.accessible_objects(self.user, 'admin_role').exists()
@@ -971,9 +964,6 @@ class GroupAccess(BaseAccess):
     def filtered_queryset(self):
         return Group.objects.filter(inventory__in=Inventory.accessible_pk_qs(self.user, 'read_role'))
 
-    def can_read(self, obj):
-        return obj and self.user in obj.inventory.read_role
-
     def can_add(self, data):
         if not data or 'inventory' not in data:
             return False
@@ -1017,12 +1007,6 @@ class InventorySourceAccess(NotificationAttachMixin, BaseAccess):
     def filtered_queryset(self):
         return self.model.objects.filter(inventory__in=Inventory.accessible_pk_qs(self.user, 'read_role'))
 
-    def can_read(self, obj):
-        if obj and obj.inventory:
-            return self.user.can_access(Inventory, 'read', obj.inventory)
-        else:
-            return False
-
     def can_add(self, data):
         if not data or 'inventory' not in data:
             return Organization.accessible_objects(self.user, 'admin_role').exists()
@@ -1115,9 +1099,6 @@ class CredentialTypeAccess(BaseAccess):
     model = CredentialType
     prefetch_related = ('created_by', 'modified_by',)
 
-    def can_read(self, obj):
-        return True
-
     def can_use(self, obj):
         return True
 
@@ -1159,10 +1140,6 @@ class CredentialAccess(BaseAccess):
     def filtered_queryset(self):
         return self.model.accessible_objects(self.user, 'read_role')
 
-    @check_superuser
-    def can_read(self, obj):
-        return self.user in obj.read_role
-
     @check_superuser
     def can_add(self, data):
         if not data:  # So the browseable API will work
@@ -1225,10 +1202,6 @@ class CredentialInputSourceAccess(BaseAccess):
         return CredentialInputSource.objects.filter(
             target_credential__in=Credential.accessible_pk_qs(self.user, 'read_role'))
 
-    @check_superuser
-    def can_read(self, obj):
-        return self.user in obj.target_credential.read_role
-
     @check_superuser
     def can_add(self, data):
         return (
@@ -1977,10 +1950,6 @@ class WorkflowJobTemplateAccess(NotificationAttachMixin, BaseAccess):
     def filtered_queryset(self):
         return self.model.accessible_objects(self.user, 'read_role')
 
-    @check_superuser
-    def can_read(self, obj):
-        return self.user in obj.read_role
-
     @check_superuser
     def can_add(self, data):
         '''
@@ -2501,14 +2470,6 @@ class NotificationTemplateAccess(BaseAccess):
             Q(organization__in=self.user.auditor_of_organizations)
         ).distinct()
 
-    def can_read(self, obj):
-        if self.user.is_superuser or self.user.is_system_auditor:
-            return True
-        if obj.organization is not None:
-            if self.user in obj.organization.notification_admin_role or self.user in obj.organization.auditor_role:
-                return True
-        return False
-
     @check_superuser
     def can_add(self, data):
         if not data:
@@ -2548,9 +2509,6 @@ class NotificationAccess(BaseAccess):
             Q(notification_template__organization__in=self.user.auditor_of_organizations)
         ).distinct()
 
-    def can_read(self, obj):
-        return self.user.can_access(NotificationTemplate, 'read', obj.notification_template)
-
     def can_delete(self, obj):
         return self.user.can_access(NotificationTemplate, 'delete', obj.notification_template)
 
@@ -2565,10 +2523,6 @@ class LabelAccess(BaseAccess):
     def filtered_queryset(self):
         return self.model.objects.all()
 
-    @check_superuser
-    def can_read(self, obj):
-        return self.user in obj.organization.read_role
-
     @check_superuser
     def can_add(self, data):
         if not data:  # So the browseable API will work
@@ -2726,15 +2680,6 @@ class RoleAccess(BaseAccess):
             result = result | super_qs
         return result
 
-    def can_read(self, obj):
-        if not obj:
-            return False
-        if self.user.is_superuser or self.user.is_system_auditor:
-            return True
-
-        return Role.filter_visible_roles(
-            self.user, Role.objects.filter(pk=obj.id)).exists()
-
     def can_add(self, obj, data):
         # Unsupported for now
         return False

awx/main/tests/functional/test_rbac_label.py

@@ -22,7 +22,7 @@ def test_label_get_queryset_su(label, user):
 @pytest.mark.django_db
 def test_label_access(label, user):
     access = LabelAccess(user('user', False))
-    assert not access.can_read(label)
+    assert access.can_read(label)
 
 
 @pytest.mark.django_db

awx/main/tests/functional/test_rbac_notifications.py

@@ -87,7 +87,7 @@ def test_notification_template_access_admin(role, organization_factory, notifica
     assert access.can_change(notification_template, {'organization': present_org.id})
     assert access.can_delete(notification_template)
 
-    nf = notification_template_factory("test-orphaned")
+    nf = notification_template_factory("test-orphaned").notification_template
     assert not access.can_read(nf)
     assert not access.can_change(nf, None)
     assert not access.can_delete(nf)