From 30451f230bbfec190fd46ff53ba740bf4f427ee2 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Tue, 16 Aug 2016 14:00:45 -0400 Subject: [PATCH] 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 | 8 +- awx/api/views.py | 47 ++++- ...0032_v302_credential_permissions_update.py | 29 +++ awx/main/models/credential.py | 2 +- .../tests/functional/api/test_credential.py | 171 ++++++++++++++---- 5 files changed, 214 insertions(+), 43 deletions(-) create mode 100644 awx/main/migrations/0032_v302_credential_permissions_update.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index c907836f72..2eb4e3d7a1 100644 --- a/awx/api/serializers.py +++ b/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 diff --git a/awx/api/views.py b/awx/api/views.py index 3dfab42e26..0b20304892 100644 --- a/awx/api/views.py +++ b/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' diff --git a/awx/main/migrations/0032_v302_credential_permissions_update.py b/awx/main/migrations/0032_v302_credential_permissions_update.py new file mode 100644 index 0000000000..a961be6dcf --- /dev/null +++ b/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), + ] diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index 1bd11ec68e..3188e10083 100644 --- a/awx/main/models/credential.py +++ b/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', ] ) diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py index eb6391ba05..8031a493c5 100644 --- a/awx/main/tests/functional/api/test_credential.py +++ b/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