add tests & correct auditor permissions

awx/main/access.py

@@ -604,17 +604,14 @@ class OAuth2ApplicationAccess(BaseAccess):
             return False
     
     def can_delete(self, obj):
-        if obj.organization in self.user.admin_of_organizations or self.user.is_superuser:
-            return True
-        else: 
-            return False
+        return obj.organization in self.user.admin_of_organizations or self.user.is_superuser
 
     def can_add(self, data):
         if self.user.is_superuser:
             return True    
         if not data:
             return Organization.accessible_objects(self.user, 'admin_role').exists()
-        return self.check_related('organization', Organization, data, role_field='admin_role')
+        return self.check_related('organization', Organization, data, role_field='admin_role', mandatory=True)
 
 
 class OAuth2TokenAccess(BaseAccess):
@@ -625,9 +622,9 @@ class OAuth2TokenAccess(BaseAccess):
      - I am the user of the token.
     I can create an OAuth2 app token when:
      - I have the read permission of the related application.
-     
     I can read, change or delete a personal token when: 
-     - 
+     - I am the user of the token
+     - I am the superuser
     I can create an OAuth2 Personal Access Token when:
      - I am a user.  But I can only create a PAT for myself.  
     '''
@@ -641,31 +638,20 @@ class OAuth2TokenAccess(BaseAccess):
             Q(admin_role__members=self.user) | Q(auditor_role__members=self.user))
         return self.model.objects.filter(application__organization__in=org_access_qs)  | self.model.objects.filter(user__id=self.user.pk)
         
-    def can_change(self, obj, data):
-        print 'obj user:', obj.user, '\nself.user:', self.user
-        if (self.user.is_superuser) | (obj.user == self.user):
-            return True
-        elif self.user.is_system_auditor:
-            return False
-        elif not obj.application:
-            return False
-        return self.user in obj.application.organization.admin_role
-
     def can_delete(self, obj):
         if (self.user.is_superuser) | (obj.user == self.user):
             return True
-        elif self.user.is_system_auditor:
-            return False
         elif not obj.application:
             return False
         return self.user in obj.application.organization.admin_role
+        
+    def can_change(self, obj, data):
+        return self.can_delete(obj)
 
     def can_add(self, data):
         if 'application' in data:
             app = get_object_from_data('application', OAuth2Application, data)
-            if self.user.is_system_auditor:
-                return False
-            elif app is None:
+            if app is None:
                 return True
             return OAuth2ApplicationAccess(self.user).can_read(app)
         return True

awx/main/tests/functional/test_rbac_oauth.py

@@ -40,7 +40,7 @@ class TestOAuth2Application:
             (2, [False, False]),
             (3, [False, False]),
         ])
-        def test_can_edit_delete(
+        def test_can_edit_delete_app(
             self, admin, org_admin, org_member, alice, user_for_access, can_access_list, organization
         ):
             organization.admin_role.members.add(org_admin)
@@ -101,6 +101,54 @@ class TestOAuth2Token:
             assert access.can_read(token) is can_access
             assert access.can_change(token, {}) is can_access
             assert access.can_delete(token) is can_access
+
+
+    def test_auditor_can_read(
+        self, post, admin, org_admin, org_member, alice, system_auditor, organization
+    ):
+        user_list = [admin, org_admin, org_member]
+        can_access_list = [True, True, True]
+        cannot_access_list = [False, False, False]
+        app = Application.objects.create(
+            name='test app for {}'.format(admin.username), user=admin,
+            client_type='confidential', authorization_grant_type='password',
+            organization=organization
+        )
+        for user, can_access, cannot_access in zip(user_list, can_access_list, cannot_access_list):
+            response = post(
+                reverse('api:o_auth2_application_token_list', kwargs={'pk': app.pk}),
+                {'scope': 'read'}, user, expect=201
+            )
+            token = AccessToken.objects.get(token=response.data['token'])
+            access = OAuth2TokenAccess(system_auditor)
+            assert access.can_read(token) is can_access
+            assert access.can_change(token, {}) is cannot_access
+            assert access.can_delete(token) is cannot_access
+            
+    def test_user_auditor_can_change(
+        self, post, org_member, org_admin, system_auditor, organization
+    ):    
+        app = Application.objects.create(
+            name='test app for {}'.format(org_admin.username), user=org_admin,
+            client_type='confidential', authorization_grant_type='password',
+            organization=organization
+        )
+        response = post(
+            reverse('api:o_auth2_application_token_list', kwargs={'pk': app.pk}),
+            {'scope': 'read'}, org_member, expect=201
+        )
+        token = AccessToken.objects.get(token=response.data['token'])
+        access = OAuth2TokenAccess(system_auditor)
+        assert access.can_read(token) is True
+        assert access.can_change(token, {}) is False
+        assert access.can_delete(token) is False
+        dual_user = system_auditor
+        organization.admin_role.members.add(dual_user)
+        access = OAuth2TokenAccess(dual_user)
+        assert access.can_read(token) is True
+        assert access.can_change(token, {}) is True
+        assert access.can_delete(token) is True
+        
             
             
     def test_can_read_change_delete_personal_token_org_member(
@@ -131,7 +179,7 @@ class TestOAuth2Token:
         for user, can_access in zip(user_list, can_access_list):
             response = post(
                 reverse('api:o_auth2_personal_token_list', kwargs={'pk': user.pk}),
-                {'scope': 'read', 'organization':None}, user, expect=201
+                {'scope': 'read', 'application':None}, user, expect=201
             )
             token = AccessToken.objects.get(token=response.data['token'])
             access = OAuth2TokenAccess(user)
@@ -159,4 +207,4 @@ class TestOAuth2Token:
                 reverse('api:o_auth2_application_token_list', kwargs={'pk': app.pk}),
                 {'scope': 'read'}, user_list[user_for_access], expect=201 if can_access else 403
             )
-            
+