From 53139b109e87ae4b06e0625c6a23f9f6a8fd5dd5 Mon Sep 17 00:00:00 2001
From: adamscmRH <chadams@redhat.com>
Date: Tue, 3 Apr 2018 16:05:29 -0400
Subject: [PATCH] clean up application logic

---
 awx/main/access.py                           | 14 +++-----
 awx/main/tests/functional/api/test_oauth.py  |  2 +-
 awx/main/tests/functional/test_rbac_oauth.py | 38 +++++++++++---------
 awx/settings/defaults.py                     |  3 +-
 4 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/awx/main/access.py b/awx/main/access.py
index 29d717791d..187aae3bc3 100644
--- a/awx/main/access.py
+++ b/awx/main/access.py
@@ -586,7 +586,7 @@ class OAuth2ApplicationAccess(BaseAccess):
      - I am a user in the organization of the application.
     I can create OAuth 2 applications when:
      - I am a superuser.
-     - I am the admin of the organization of the organization of the application.
+     - I am the admin of the organization of the application.
     '''
 
     model = OAuth2Application
@@ -596,15 +596,11 @@ class OAuth2ApplicationAccess(BaseAccess):
         return self.model.objects.filter(organization__in=self.user.organizations)
 
     def can_change(self, obj, data):
-        if obj.organization in self.user.admin_of_organizations or self.user.is_superuser:
-            if not self.check_related('organization', Organization, data, role_field='admin_role'):
-                return False
-            return True
-        else: 
-            return False
-    
+        return self.user.is_superuser or self.check_related('organization', Organization, data, obj=obj, 
+                                                            role_field='admin_role', mandatory=True)
+
     def can_delete(self, obj):
-        return obj.organization in self.user.admin_of_organizations or self.user.is_superuser
+        return self.user.is_superuser or obj.organization in self.user.admin_of_organizations
 
     def can_add(self, data):
         if self.user.is_superuser:
diff --git a/awx/main/tests/functional/api/test_oauth.py b/awx/main/tests/functional/api/test_oauth.py
index 6b462bbd58..a417eb539f 100644
--- a/awx/main/tests/functional/api/test_oauth.py
+++ b/awx/main/tests/functional/api/test_oauth.py
@@ -9,7 +9,7 @@ from oauth2_provider.models import RefreshToken
 
 
 @pytest.mark.django_db
-def test_personal_access_token_creation(oauth_application, post, alice):            # TODO: Update this test
+def test_personal_access_token_creation(oauth_application, post, alice):
     url = drf_reverse('api:oauth_authorization_root_view') + 'token/'
     resp = post(
         url,
diff --git a/awx/main/tests/functional/test_rbac_oauth.py b/awx/main/tests/functional/test_rbac_oauth.py
index 6a99a284d0..8f673cab80 100644
--- a/awx/main/tests/functional/test_rbac_oauth.py
+++ b/awx/main/tests/functional/test_rbac_oauth.py
@@ -34,30 +34,34 @@ class TestOAuth2Application:
                 assert access.can_read(app) is can_access    
     
     
-        @pytest.mark.parametrize("user_for_access, can_access_list", [
-            (0, [True, True]),
-            (1, [True, True]),
-            (2, [False, False]),
-            (3, [False, False]),
-        ])
-        def test_can_edit_delete_app(
-            self, admin, org_admin, org_member, alice, user_for_access, can_access_list, organization
+        def test_can_edit_delete_app_org_admin(
+            self, admin, org_admin, org_member, alice, organization
         ):
-            organization.admin_role.members.add(org_admin)
-            organization.member_role.members.add(org_member)
             user_list = [admin, org_admin, org_member, alice]
-            access = OAuth2ApplicationAccess(user_list[user_for_access])
-            app_creation_user_list = [admin, org_admin]
-            for user, can_access in zip(app_creation_user_list, can_access_list):
+            can_access_list = [True, True, False, False]
+            for user, can_access in zip(user_list, can_access_list):
                 app = Application.objects.create(
-                    name='test app for {}'.format(user.username), user=user,
+                    name='test app for {}'.format(org_admin.username), user=org_admin,
                     client_type='confidential', authorization_grant_type='password', organization=organization
                 )
+                access = OAuth2ApplicationAccess(user)
+                assert access.can_change(app, {}) is can_access
+                assert access.can_delete(app) is can_access
+                
+                
+        def test_can_edit_delete_app_admin(
+            self, admin, org_admin, org_member, alice, organization
+        ):
+            user_list = [admin, org_admin, org_member, alice]
+            can_access_list = [True, True, False, False]
+            for user, can_access in zip(user_list, can_access_list):
+                app = Application.objects.create(
+                    name='test app for {}'.format(admin.username), user=admin,
+                    client_type='confidential', authorization_grant_type='password', organization=organization
+                )
+                access = OAuth2ApplicationAccess(user)
                 assert access.can_change(app, {}) is can_access
                 assert access.can_delete(app) is can_access
-    
-    
-    
     
 
         def test_superuser_can_always_create(self, admin, org_admin, org_member, alice):
diff --git a/awx/settings/defaults.py b/awx/settings/defaults.py
index 013f831492..645947eb60 100644
--- a/awx/settings/defaults.py
+++ b/awx/settings/defaults.py
@@ -289,8 +289,9 @@ REST_FRAMEWORK = {
     'DEFAULT_PAGINATION_CLASS': 'awx.api.pagination.Pagination',
     'PAGE_SIZE': 25,
     'DEFAULT_AUTHENTICATION_CLASSES': (
-        'awx.api.authentication.LoggedOAuth2Authentication',
         'awx.api.authentication.SessionAuthentication',
+        'awx.api.authentication.LoggedOAuth2Authentication',
+        # 'awx.api.authentication.SessionAuthentication',
         'awx.api.authentication.LoggedBasicAuthentication',
     ),
     'DEFAULT_PERMISSION_CLASSES': (