Fixed org auditor visibility of team credentials

And by fix, I mean prevent us from getting into the situation that was
causing the asymetric visiblity by brining us into alignment with the
original intention and spec for how credentials were supposed behave.

 #3081

awx/api/serializers.py

@@ -1716,21 +1716,21 @@ class CredentialSerializerCreate(CredentialSerializer):
                     attrs.pop(field)
         if not owner_fields:
             raise serializers.ValidationError({"detail": "Missing 'user', 'team', or 'organization'."})
-        elif len(owner_fields) > 1:
-            raise serializers.ValidationError({"detail": "Expecting exactly one of 'user', 'team', or 'organization'."})
-
         return super(CredentialSerializerCreate, self).validate(attrs)
 
     def create(self, validated_data):
         user = validated_data.pop('user', None)
         team = validated_data.pop('team', None)
+        if team:
+            validated_data['organization'] = team.organization
         credential = super(CredentialSerializerCreate, self).create(validated_data)
         if user:
             credential.admin_role.members.add(user)
         if team:
+            if not credential.organization or team.organization.id != credential.organization.id:
+                raise serializers.ValidationError({"detail": "Credential organization must be set and match before assigning to a team"})
             credential.admin_role.parents.add(team.admin_role)
             credential.use_role.parents.add(team.member_role)
-
         return credential
 
 

awx/api/views.py

@@ -879,11 +879,18 @@ class TeamRolesList(SubListCreateAttachDetachAPIView):
             return Response(data, status=status.HTTP_400_BAD_REQUEST)
 
         role = get_object_or_400(Role, pk=sub_id)
-        content_type = ContentType.objects.get_for_model(Organization)
-        if role.content_type == content_type:
+        org_content_type = ContentType.objects.get_for_model(Organization)
+        if role.content_type == org_content_type:
             data = dict(msg="You cannot assign an Organization role as a child role for a Team.")
             return Response(data, status=status.HTTP_400_BAD_REQUEST)
 
+        team = get_object_or_404(Team, pk=self.kwargs['pk'])
+        credential_content_type = ContentType.objects.get_for_model(Credential)
+        if role.content_type == credential_content_type:
+            if not role.content_object.organization or role.content_object.organization.id != team.organization.id:
+                data = dict(msg="You cannot grant credential access to a team when the Organization field isn't set, or belongs to a different organization")
+                return Response(data, status=status.HTTP_400_BAD_REQUEST)
+
         return super(TeamRolesList, self).post(request, *args, **kwargs)
 
 class TeamObjectRolesList(SubListAPIView):
@@ -1209,11 +1216,23 @@ class UserRolesList(SubListCreateAttachDetachAPIView):
         if sub_id == self.request.user.admin_role.pk:
             raise PermissionDenied('You may not perform any action with your own admin_role.')
 
+        user = get_object_or_400(User, pk=self.kwargs['pk'])
         role = get_object_or_400(Role, pk=sub_id)
         user_content_type = ContentType.objects.get_for_model(User)
         if role.content_type == user_content_type:
             raise PermissionDenied('You may not change the membership of a users admin_role')
 
+        credential_content_type = ContentType.objects.get_for_model(Credential)
+        if role.content_type == credential_content_type:
+            if role.content_object.organization and user not in role.content_object.organization.member_role:
+                data = dict(msg="You cannot grant credential access to a user not in the credentials' organization")
+                return Response(data, status=status.HTTP_400_BAD_REQUEST)
+
+            if not role.content_object.organization and not request.user.is_superuser:
+                data = dict(msg="You cannot grant private credential access to another user")
+                return Response(data, status=status.HTTP_400_BAD_REQUEST)
+
+
         return super(UserRolesList, self).post(request, *args, **kwargs)
 
     def check_parent_access(self, parent=None):
@@ -3656,6 +3675,7 @@ class RoleUsersList(SubListCreateAttachDetachAPIView):
             data = dict(msg="User 'id' field is missing.")
             return Response(data, status=status.HTTP_400_BAD_REQUEST)
 
+        user = get_object_or_400(User, pk=sub_id)
         role = self.get_parent_object()
         if role == self.request.user.admin_role:
             raise PermissionDenied('You may not perform any action with your own admin_role.')
@@ -3664,6 +3684,16 @@ class RoleUsersList(SubListCreateAttachDetachAPIView):
         if role.content_type == user_content_type:
             raise PermissionDenied('You may not change the membership of a users admin_role')
 
+        credential_content_type = ContentType.objects.get_for_model(Credential)
+        if role.content_type == credential_content_type:
+            if role.content_object.organization and user not in role.content_object.organization.member_role:
+                data = dict(msg="You cannot grant credential access to a user not in the credentials' organization")
+                return Response(data, status=status.HTTP_400_BAD_REQUEST)
+
+            if not role.content_object.organization and not request.user.is_superuser:
+                data = dict(msg="You cannot grant private credential access to another user")
+                return Response(data, status=status.HTTP_400_BAD_REQUEST)
+
         return super(RoleUsersList, self).post(request, *args, **kwargs)
 
 
@@ -3688,13 +3718,20 @@ class RoleTeamsList(SubListAPIView):
             data = dict(msg="Team 'id' field is missing.")
             return Response(data, status=status.HTTP_400_BAD_REQUEST)
 
+        team = get_object_or_400(Team, pk=sub_id)
         role = Role.objects.get(pk=self.kwargs['pk'])
-        content_type = ContentType.objects.get_for_model(Organization)
-        if role.content_type == content_type:
+
+        organization_content_type = ContentType.objects.get_for_model(Organization)
+        if role.content_type == organization_content_type:
             data = dict(msg="You cannot assign an Organization role as a child role for a Team.")
             return Response(data, status=status.HTTP_400_BAD_REQUEST)
 
-        team = get_object_or_400(Team, pk=sub_id)
+        credential_content_type = ContentType.objects.get_for_model(Credential)
+        if role.content_type == credential_content_type:
+            if not role.content_object.organization or role.content_object.organization.id != team.organization.id:
+                data = dict(msg="You cannot grant credential access to a team when the Organization field isn't set, or belongs to a different organization")
+                return Response(data, status=status.HTTP_400_BAD_REQUEST)
+
         action = 'attach'
         if request.data.get('disassociate', None):
             action = 'unattach'

awx/main/migrations/0032_v302_credential_permissions_update.py

@@ -0,0 +1,29 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations
+from awx.main.migrations import _rbac as rbac
+from awx.main.migrations import _migration_utils as migration_utils
+import awx.main.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('main', '0031_v302_migrate_survey_passwords'),
+    ]
+
+    operations = [
+        migrations.RunPython(migration_utils.set_current_apps_for_migrations),
+        migrations.AlterField(
+            model_name='credential',
+            name='admin_role',
+            field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'singleton:system_administrator', b'organization.admin_role'], to='main.Role', null=b'True'),
+        ),
+        migrations.AlterField(
+            model_name='credential',
+            name='use_role',
+            field=awx.main.fields.ImplicitRoleField(related_name='+', parent_role=[b'admin_role'], to='main.Role', null=b'True'),
+        ),
+        migrations.RunPython(rbac.rebuild_role_hierarchy),
+    ]

awx/main/models/credential.py

@@ -215,11 +215,11 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin):
     admin_role = ImplicitRoleField(
         parent_role=[
             'singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR,
+            'organization.admin_role',
         ],
     )
     use_role = ImplicitRoleField(
         parent_role=[
-            'organization.admin_role',
             'admin_role',
         ]
     )

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

@@ -68,9 +68,10 @@ def test_create_user_credential_via_user_credentials_list_xfail(post, alice, bob
 #
 
 @pytest.mark.django_db
-def test_create_team_credential(post, get, team, org_admin, team_member):
+def test_create_team_credential(post, get, team, organization, org_admin, team_member):
     response = post(reverse('api:credential_list'), {
         'team': team.id,
+        'organization': organization.id,
         'name': 'Some name',
         'username': 'someusername'
     }, org_admin)
@@ -94,25 +95,159 @@ def test_create_team_credential_via_team_credentials_list(post, get, team, org_a
     assert response.data['count'] == 1
 
 @pytest.mark.django_db
-def test_create_team_credential_by_urelated_user_xfail(post, team, alice, team_member):
+def test_create_team_credential_by_urelated_user_xfail(post, team, organization, alice, team_member):
     response = post(reverse('api:credential_list'), {
         'team': team.id,
+        'organization': organization.id,
         'name': 'Some name',
         'username': 'someusername'
     }, alice)
     assert response.status_code == 403
 
 @pytest.mark.django_db
-def test_create_team_credential_by_team_member_xfail(post, team, alice, team_member):
+def test_create_team_credential_by_team_member_xfail(post, team, organization, alice, team_member):
     # Members can't add credentials, only org admins.. for now?
     response = post(reverse('api:credential_list'), {
         'team': team.id,
+        'organization': organization.id,
         'name': 'Some name',
         'username': 'someusername'
     }, team_member)
     assert response.status_code == 403
 
 
+#
+# Permission granting
+#
+
+@pytest.mark.django_db
+def test_grant_org_credential_to_org_user_through_role_users(post, credential, organization, org_admin, org_member):
+    credential.organization = organization
+    credential.save()
+    response = post(reverse('api:role_users_list', args=(credential.use_role.id,)), {
+        'id': org_member.id
+    }, org_admin)
+    assert response.status_code == 204
+
+@pytest.mark.django_db
+def test_grant_org_credential_to_org_user_through_user_roles(post, credential, organization, org_admin, org_member):
+    credential.organization = organization
+    credential.save()
+    response = post(reverse('api:user_roles_list', args=(org_member.id,)), {
+        'id': credential.use_role.id
+    }, org_admin)
+    assert response.status_code == 204
+
+@pytest.mark.django_db
+def test_grant_org_credential_to_non_org_user_through_role_users(post, credential, organization, org_admin, alice):
+    credential.organization = organization
+    credential.save()
+    response = post(reverse('api:role_users_list', args=(credential.use_role.id,)), {
+        'id': alice.id
+    }, org_admin)
+    assert response.status_code == 400
+
+@pytest.mark.django_db
+def test_grant_org_credential_to_non_org_user_through_user_roles(post, credential, organization, org_admin, alice):
+    credential.organization = organization
+    credential.save()
+    response = post(reverse('api:user_roles_list', args=(alice.id,)), {
+        'id': credential.use_role.id
+    }, org_admin)
+    assert response.status_code == 400
+
+@pytest.mark.django_db
+def test_grant_private_credential_to_user_through_role_users(post, credential, alice, bob):
+    # normal users can't do this
+    credential.admin_role.members.add(alice)
+    response = post(reverse('api:role_users_list', args=(credential.use_role.id,)), {
+        'id': bob.id
+    }, alice)
+    assert response.status_code == 400
+
+@pytest.mark.django_db
+def test_grant_private_credential_to_org_user_through_role_users(post, credential, org_admin, org_member):
+    # org admins can't either
+    credential.admin_role.members.add(org_admin)
+    response = post(reverse('api:role_users_list', args=(credential.use_role.id,)), {
+        'id': org_member.id
+    }, org_admin)
+    assert response.status_code == 400
+
+@pytest.mark.django_db
+def test_sa_grant_private_credential_to_user_through_role_users(post, credential, admin, bob):
+    # but system admins can
+    response = post(reverse('api:role_users_list', args=(credential.use_role.id,)), {
+        'id': bob.id
+    }, admin)
+    assert response.status_code == 204
+
+@pytest.mark.django_db
+def test_grant_private_credential_to_user_through_user_roles(post, credential, alice, bob):
+    # normal users can't do this
+    credential.admin_role.members.add(alice)
+    response = post(reverse('api:user_roles_list', args=(bob.id,)), {
+        'id': credential.use_role.id
+    }, alice)
+    assert response.status_code == 400
+
+@pytest.mark.django_db
+def test_grant_private_credential_to_org_user_through_user_roles(post, credential, org_admin, org_member):
+    # org admins can't either
+    credential.admin_role.members.add(org_admin)
+    response = post(reverse('api:user_roles_list', args=(org_member.id,)), {
+        'id': credential.use_role.id
+    }, org_admin)
+    assert response.status_code == 400
+
+@pytest.mark.django_db
+def test_sa_grant_private_credential_to_user_through_user_roles(post, credential, admin, bob):
+    # but system admins can
+    response = post(reverse('api:user_roles_list', args=(bob.id,)), {
+        'id': credential.use_role.id
+    }, admin)
+    assert response.status_code == 204
+
+@pytest.mark.django_db
+def test_grant_org_credential_to_team_through_role_teams(post, credential, organization, org_admin, org_auditor, team):
+    assert org_auditor not in credential.read_role
+    credential.organization = organization
+    credential.save()
+    response = post(reverse('api:role_teams_list', args=(credential.use_role.id,)), {
+        'id': team.id
+    }, org_admin)
+    assert response.status_code == 204
+    assert org_auditor in credential.read_role
+
+@pytest.mark.django_db
+def test_grant_org_credential_to_team_through_team_roles(post, credential, organization, org_admin, org_auditor, team):
+    assert org_auditor not in credential.read_role
+    credential.organization = organization
+    credential.save()
+    response = post(reverse('api:team_roles_list', args=(team.id,)), {
+        'id': credential.use_role.id
+    }, org_admin)
+    assert response.status_code == 204
+    assert org_auditor in credential.read_role
+
+@pytest.mark.django_db
+def test_sa_grant_private_credential_to_team_through_role_teams(post, credential, admin, team):
+    # not even a system admin can grant a private cred to a team though
+    response = post(reverse('api:role_teams_list', args=(credential.use_role.id,)), {
+        'id': team.id
+    }, admin)
+    assert response.status_code == 400
+
+@pytest.mark.django_db
+def test_sa_grant_private_credential_to_team_through_team_roles(post, credential, admin, team):
+    # not even a system admin can grant a private cred to a team though
+    response = post(reverse('api:role_teams_list', args=(team.id,)), {
+        'id': credential.use_role.id
+    }, admin)
+    assert response.status_code == 400
+
+
+
 
 #
 # organization credentials
@@ -224,33 +359,3 @@ def test_create_credential_missing_user_team_org_xfail(post, admin):
     }, admin)
     assert response.status_code == 400
 
-@pytest.mark.django_db
-def test_create_credential_with_user_and_org_xfail(post, organization, admin):
-    # Can only specify one of user, team, or organization
-    response = post(reverse('api:credential_list'), {
-        'name': 'Some name',
-        'username': 'someusername',
-        'user': admin.id,
-        'organization': organization.id,
-    }, admin)
-    assert response.status_code == 400
-
-@pytest.mark.django_db
-def test_create_credential_with_team_and_org_xfail(post, organization, team, admin):
-    response = post(reverse('api:credential_list'), {
-        'name': 'Some name',
-        'username': 'someusername',
-        'organization': organization.id,
-        'team': team.id,
-    }, admin)
-    assert response.status_code == 400
-
-@pytest.mark.django_db
-def test_create_credential_with_user_and_team_xfail(post, team, admin):
-    response = post(reverse('api:credential_list'), {
-        'name': 'Some name',
-        'username': 'someusername',
-        'user': admin.id,
-        'team': team.id,
-    }, admin)
-    assert response.status_code == 400