Merge pull request #1412 from chrismeyersfsu/labels-rbac_fix

fix rbac label assignment across orgs

awx/main/access.py

@@ -1204,6 +1204,7 @@ class LabelAccess(BaseAccess):
     def can_add(self, data):
         if self.user.is_superuser:
             return True
+
         if not data or '_method' in data:  # So the browseable API will work?
             return True
 
@@ -1214,6 +1215,10 @@ class LabelAccess(BaseAccess):
     def can_change(self, obj, data):
         if self.user.is_superuser:
             return True
+
+        if self.can_add(data) is False:
+            return False
+
         return obj.organization and obj.organization.accessible_by(self.user, ALL_PERMISSIONS)
 
     def can_delete(self, obj):

awx/main/tests/functional/conftest.py

@@ -150,6 +150,18 @@ def instance(settings):
 def organization(instance):
     return Organization.objects.create(name="test-org", description="test-org-desc")
 
+@pytest.fixture
+def organization_factory(instance):
+    def factory(name):
+        try:
+            org = Organization.objects.get(name=name)
+        except Organization.DoesNotExist:
+            org = Organization.objects.create(name=name,
+                                              description="description for " + name,
+                                              )
+        return org
+    return factory
+
 @pytest.fixture
 def credential():
     return Credential.objects.create(kind='aws', name='test-cred')

awx/main/tests/functional/test_rbac_label.py

@@ -4,6 +4,8 @@ from awx.main.access import (
     LabelAccess,
 )
 
+from rest_framework.exceptions import ParseError
+
 @pytest.mark.django_db
 def test_label_get_queryset_user(label, user):
     access = LabelAccess(user('user', False))
@@ -29,14 +31,20 @@ def test_label_access_superuser(label, user):
     assert access.can_delete(label)
 
 @pytest.mark.django_db
-def test_label_access_admin(label, user):
+def test_label_access_admin(label, user, organization_factory):
     '''can_change because I am an admin of that org'''
     a = user('admin', False)
+    org_no_members = organization_factory("no_members")
+    org_members = organization_factory("has_members")
+
     label.organization.admin_role.members.add(a)
+    org_members.admin_role.members.add(a)
 
     access = LabelAccess(user('admin', False))
+    assert not access.can_change(label, {'organization': org_no_members.id})
     assert access.can_read(label)
     assert access.can_change(label, None)
+    assert access.can_change(label, {'organization': org_members.id})
     assert access.can_delete(label)
 
 @pytest.mark.django_db
@@ -44,7 +52,11 @@ def test_label_access_user(label, user):
     access = LabelAccess(user('user', False))
     label.organization.member_role.members.add(user('user', False))
 
-    assert access.can_read(label)
+    with pytest.raises(ParseError):
+        access.can_add({'organization': None})
     assert not access.can_change(label, None)
     assert not access.can_delete(label)
 
+    assert access.can_read(label)
+    assert access.can_add({'organization': label.organization.id})
+