Merge pull request #4288 from AlanCoding/user_aud_31_fixes

Fix system auditor getter logic

awx/main/models/__init__.py

@@ -76,7 +76,12 @@ User.add_to_class('auditor_of_organizations', user_get_auditor_of_organizations)
 @property
 def user_is_system_auditor(user):
     if not hasattr(user, '_is_system_auditor'):
-        user._is_system_auditor = Role.objects.filter(role_field='system_auditor', id=user.id).exists()
+        if user.pk:
+            user._is_system_auditor = user.roles.filter(
+                singleton_name='system_auditor', role_field='system_auditor').exists()
+        else:
+            # Odd case where user is unsaved, this should never be relied on
+            return False
     return user._is_system_auditor
 
 

awx/main/tests/functional/api/test_user.py

@@ -7,65 +7,41 @@ from django.core.urlresolvers import reverse
 # user creation
 #
 
+EXAMPLE_USER_DATA = {
+    "username": "affable",
+    "first_name": "a",
+    "last_name": "a",
+    "email": "a@a.com",
+    "is_superuser": False,
+    "password": "r$TyKiOCb#ED"
+}
+
 
 @pytest.mark.django_db
 def test_user_create(post, admin):
-    response = post(reverse('api:user_list'), {
-        "username": "affable",
-        "first_name": "a",
-        "last_name": "a",
-        "email": "a@a.com",
-        "is_superuser": False,
-        "password": "fo0m4nchU"
-    }, admin)
+    response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin)
     assert response.status_code == 201
+    assert not response.data['is_superuser']
+    assert not response.data['is_system_auditor']
 
 
 @pytest.mark.django_db
 def test_fail_double_create_user(post, admin):
-    response = post(reverse('api:user_list'), {
-        "username": "affable",
-        "first_name": "a",
-        "last_name": "a",
-        "email": "a@a.com",
-        "is_superuser": False,
-        "password": "fo0m4nchU"
-    }, admin)
+    response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin)
     assert response.status_code == 201
 
-    response = post(reverse('api:user_list'), {
-        "username": "affable",
-        "first_name": "a",
-        "last_name": "a",
-        "email": "a@a.com",
-        "is_superuser": False,
-        "password": "fo0m4nchU"
-    }, admin)
+    response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin)
     assert response.status_code == 400
 
 
 @pytest.mark.django_db
 def test_create_delete_create_user(post, delete, admin):
-    response = post(reverse('api:user_list'), {
-        "username": "affable",
-        "first_name": "a",
-        "last_name": "a",
-        "email": "a@a.com",
-        "is_superuser": False,
-        "password": "fo0m4nchU"
-    }, admin)
+    response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin)
     assert response.status_code == 201
 
     response = delete(reverse('api:user_detail', args=(response.data['id'],)), admin)
     assert response.status_code == 204
 
-    response = post(reverse('api:user_list'), {
-        "username": "affable",
-        "first_name": "a",
-        "last_name": "a",
-        "email": "a@a.com",
-        "is_superuser": False,
-        "password": "fo0m4nchU"
-    }, admin)
+    response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin)
     print(response.data)
     assert response.status_code == 201

awx/main/tests/functional/conftest.py

@@ -41,7 +41,7 @@ from awx.main.models.organization import (
     Permission,
     Team,
 )
-
+from awx.main.models.rbac import Role
 from awx.main.models.notifications import (
     NotificationTemplate,
     Notification
@@ -262,6 +262,13 @@ def admin(user):
     return user('admin', True)
 
 
+@pytest.fixture
+def system_auditor(user):
+    u = user(False)
+    Role.singleton('system_auditor').members.add(u)
+    return u
+
+
 @pytest.fixture
 def alice(user):
     return user('alice', False)

awx/main/tests/functional/test_rbac_user.py

@@ -9,7 +9,7 @@ from awx.main.models import Role, User, Organization, Inventory
 
 
 @pytest.mark.django_db
-class TestSysAuditor(TransactionTestCase):
+class TestSysAuditorTransactional(TransactionTestCase):
     def rando(self):
         return User.objects.create(username='rando', password='rando', email='rando@com.com')
 
@@ -41,6 +41,10 @@ class TestSysAuditor(TransactionTestCase):
         assert not rando.is_system_auditor
 
 
+@pytest.mark.django_db
+def test_system_auditor_is_system_auditor(system_auditor):
+    assert system_auditor.is_system_auditor
+
 
 @pytest.mark.django_db
 def test_user_admin(user_project, project, user):