From 23aca083ebc0ecad204900c119556fd061b7136a Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Fri, 1 Apr 2016 16:57:08 -0400 Subject: [PATCH 1/3] Added and updated several credential creation and listing API endpoints Should addres #1379 --- awx/api/serializers.py | 17 +--- awx/api/urls.py | 1 + awx/api/views.py | 127 ++++++++++++++++++++------ awx/main/access.py | 18 +--- awx/main/models/mixins.py | 4 + awx/main/tests/functional/conftest.py | 7 ++ 6 files changed, 116 insertions(+), 58 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index b170199bdb..d652e61095 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1505,7 +1505,7 @@ class CredentialSerializer(BaseSerializer): class Meta: model = Credential - fields = ('*', 'deprecated_user', 'deprecated_team', 'kind', 'cloud', 'host', 'username', + fields = ('*', 'kind', 'cloud', 'host', 'username', 'password', 'security_token', 'project', 'domain', 'ssh_key_data', 'ssh_key_unlock', 'become_method', 'become_username', 'become_password', @@ -1519,21 +1519,6 @@ class CredentialSerializer(BaseSerializer): field_kwargs['default'] = '' return field_class, field_kwargs - def to_representation(self, obj): - ret = super(CredentialSerializer, self).to_representation(obj) - if obj is not None and 'deprecated_user' in ret and not obj.deprecated_user: - ret['deprecated_user'] = None - if obj is not None and 'deprecated_team' in ret and not obj.deprecated_team: - ret['deprecated_team'] = None - return ret - - def validate(self, attrs): - # Ensure old style assignment for user/team is always None - attrs['deprecated_user'] = None - attrs['deprecated_team'] = None - - return super(CredentialSerializer, self).validate(attrs) - def get_related(self, obj): res = super(CredentialSerializer, self).get_related(obj) res.update(dict( diff --git a/awx/api/urls.py b/awx/api/urls.py index f3b24c147a..1cb8b4ad4e 100644 --- a/awx/api/urls.py +++ b/awx/api/urls.py @@ -19,6 +19,7 @@ organization_urls = patterns('awx.api.views', url(r'^(?P[0-9]+)/inventories/$', 'organization_inventories_list'), url(r'^(?P[0-9]+)/projects/$', 'organization_projects_list'), url(r'^(?P[0-9]+)/teams/$', 'organization_teams_list'), + url(r'^(?P[0-9]+)/credentials/$', 'organization_credential_list'), url(r'^(?P[0-9]+)/activity_stream/$', 'organization_activity_stream_list'), url(r'^(?P[0-9]+)/notifiers/$', 'organization_notifiers_list'), url(r'^(?P[0-9]+)/notifiers_any/$', 'organization_notifiers_any_list'), diff --git a/awx/api/views.py b/awx/api/views.py index 0c406dc610..a2e5037e38 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -824,21 +824,6 @@ class TeamProjectsList(SubListAPIView): return team_qs & user_qs -class TeamCredentialsList(SubListAPIView): - - model = Credential - serializer_class = CredentialSerializer - parent_model = Team - - def get_queryset(self): - team = self.get_parent_object() - self.check_parent_access(team) - - visible_creds = Credential.accessible_objects(self.request.user, {'read': True}) - team_creds = Credential.objects.filter(owner_role__parents=team.member_role) - return team_creds & visible_creds - - class TeamActivityStreamList(SubListAPIView): model = ActivityStream @@ -1109,20 +1094,6 @@ class UserProjectsList(SubListAPIView): user_qs = Project.accessible_objects(parent, {'read': True}) return my_qs & user_qs -class UserCredentialsList(SubListAPIView): - - model = Credential - serializer_class = CredentialSerializer - parent_model = User - - def get_queryset(self): - user = self.get_parent_object() - self.check_parent_access(user) - - visible_creds = Credential.accessible_objects(self.request.user, {'read': True}) - user_creds = Credential.accessible_objects(user, {'read': True}) - return user_creds & visible_creds - class UserOrganizationsList(SubListAPIView): model = Organization @@ -1216,6 +1187,104 @@ class CredentialList(ListCreateAPIView): model = Credential serializer_class = CredentialSerializer + def post(self, request, *args, **kwargs): + if not any([x in request.data for x in ['user', 'team', 'organization']]): + return Response({'detail': 'Missing user, team, or organization'}, status=status.HTTP_400_BAD_REQUEST) + + if sum([1 if x in request.data else 0 for x in ['user', 'team', 'organization']]) != 1: + return Response({'detail': 'Expecting exactly one of user, team, or organization'}, status=status.HTTP_400_BAD_REQUEST) + + if 'user' in request.data: + user = User.objects.get(pk=request.data['user']) + obj = user + if 'team' in request.data: + team = Team.objects.get(pk=request.data['team']) + obj = team + if 'organization' in request.data: + organization = Organization.objects.get(pk=request.data['organization']) + obj = organization + + if not obj.accessible_by(self.request.user, {'write': True}): + raise PermissionDenied() + + ret = super(CredentialList, self).post(request, *args, **kwargs) + credential = Credential.objects.get(id=ret.data['id']) + + if 'user' in request.data: + credential.owner_role.members.add(user) + if 'team' in request.data: + credential.owner_role.parents.add(team.member_role) + if 'organization' in request.data: + credential.owner_role.parents.add(organization.admin_role) + + return ret + +class UserCredentialsList(CredentialList): + + model = Credential + serializer_class = CredentialSerializer + + def get_queryset(self): + user = User.objects.get(pk=self.kwargs['pk']) + if not self.request.user.can_access(User, 'read', user): + raise PermissionDenied() + + visible_creds = Credential.accessible_objects(self.request.user, {'read': True}) + user_creds = Credential.accessible_objects(user, {'read': True}) + return user_creds & visible_creds + + def post(self, request, *args, **kwargs): + user = User.objects.get(pk=self.kwargs['pk']) + request.data['user'] = user.id + # The following post takes care of ensuring the current user can add a cred to this user + return super(UserCredentialsList, self).post(request, args, kwargs) + +class TeamCredentialsList(CredentialList): + + model = Credential + serializer_class = CredentialSerializer + + def get_queryset(self): + team = Team.objects.get(pk=self.kwargs['pk']) + if not self.request.user.can_access(Team, 'read', team): + raise PermissionDenied() + + visible_creds = Credential.accessible_objects(self.request.user, {'read': True}) + team_creds = Credential.objects.filter(owner_role__parents=team.member_role) + return team_creds & visible_creds + + def post(self, request, *args, **kwargs): + team = Team.objects.get(pk=self.kwargs['pk']) + request.data['team'] = team.id + # The following post takes care of ensuring the current user can add a cred to this user + return super(TeamCredentialsList, self).post(request, args, kwargs) + +class OrganizationCredentialList(CredentialList): + + model = Credential + serializer_class = CredentialSerializer + + def get_queryset(self): + organization = Organization.objects.get(pk=self.kwargs['pk']) + if not self.request.user.can_access(Organization, 'read', organization): + raise PermissionDenied() + + user_visible = Credential.accessible_objects(self.request.user, {'read': True}).all() + org_set = Credential.accessible_objects(organization.admin_role, {'read': True}).all() + + if self.request.user.is_superuser: + return org_set + + return org_set & user_visible + + def post(self, request, *args, **kwargs): + organization = Organization.objects.get(pk=self.kwargs['pk']) + request.data['organization'] = organization.id + # The following post takes care of ensuring the current user can add a cred to this user + return super(OrganizationCredentialList, self).post(request, args, kwargs) + + + class CredentialDetail(RetrieveUpdateDestroyAPIView): model = Credential diff --git a/awx/main/access.py b/awx/main/access.py index fc89a3487f..726b77252b 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -542,20 +542,12 @@ class CredentialAccess(BaseAccess): qs = self.model.accessible_objects(self.user, {'read':True}) return qs.select_related('created_by', 'modified_by').all() + def can_read(self, obj): + return obj.accessible_by(self.user, {'read': True}) + def can_add(self, data): - if self.user.is_superuser: - return True - - if 'user' in data: - pk = get_pk_from_dict(data, 'user') - user = get_object_or_400(User, pk=pk) - return user.accessible_by(self.user, {'write': True}) - elif 'organization' in data: - pk = get_pk_from_dict(data, 'organization') - org = get_object_or_400(Organization, pk=pk) - return org.accessible_by(self.user, {'write': True}) - - return False + # Access enforced in our view where we have context enough to make a decision + return True def can_change(self, obj, data): if self.user.is_superuser: diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index 0160ca9be5..bcf95fa4ac 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -43,6 +43,10 @@ class ResourceMixin(models.Model): qs = cls.objects.filter( role_permissions__role__ancestors__members=accessor ) + elif type(accessor) == Role: + qs = cls.objects.filter( + role_permissions__role__ancestors=accessor + ) else: accessor_type = ContentType.objects.get_for_model(accessor) roles = Role.objects.filter(content_type__pk=accessor_type.id, diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 405f3fa0e8..c9cfe00e02 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -92,6 +92,13 @@ def deploy_jobtemplate(project, inventory, credential): def team(organization): return organization.teams.create(name='test-team') +@pytest.fixture +def team_member(user, team): + ret = user('team-member', False) + team.member_role.members.add(ret) + return ret + + @pytest.fixture @mock.patch.object(Project, "update", lambda self, **kwargs: None) def project(instance, organization): From eb2550bc7c6a5349a3dfb34247fc817530b86157 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Mon, 4 Apr 2016 09:56:36 -0400 Subject: [PATCH 2/3] rbac active removal test cases fixes --- awx/main/tests/unit/api/test_serializers.py | 23 ++++++++++----------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/awx/main/tests/unit/api/test_serializers.py b/awx/main/tests/unit/api/test_serializers.py index 3cac6a34d8..50e6322d1d 100644 --- a/awx/main/tests/unit/api/test_serializers.py +++ b/awx/main/tests/unit/api/test_serializers.py @@ -76,14 +76,14 @@ class TestJobTemplateSerializerGetRelated(GetRelatedMixin): class TestJobTemplateSerializerGetSummaryFields(GetSummaryFieldsMixin): def test__recent_jobs(self, mocker, job_template, jobs): - job_template.jobs.filter = mocker.MagicMock(**{'order_by.return_value': jobs}) - job_template.jobs.filter.return_value = job_template.jobs.filter + job_template.jobs.all = mocker.MagicMock(**{'order_by.return_value': jobs}) + job_template.jobs.all.return_value = job_template.jobs.all serializer = JobTemplateSerializer() recent_jobs = serializer._recent_jobs(job_template) - job_template.jobs.filter.assert_called_with(active=True) - job_template.jobs.filter.order_by.assert_called_with('-created') + job_template.jobs.all.assert_called_once_with() + job_template.jobs.all.order_by.assert_called_once_with('-created') assert len(recent_jobs) == 10 for x in jobs[:10]: assert recent_jobs == [{'id': x.id, 'status': x.status, 'finished': x.finished} for x in jobs[:10]] @@ -126,18 +126,17 @@ class TestJobSerializerGetRelated(GetRelatedMixin): def test_get_related(self, mocker, job, related_resource_name): self._test_get_related(JobSerializer, job, 'jobs', related_resource_name) - def test_job_template_present(self, job): - job.job_template.active = True - serializer = JobSerializer() - related = serializer.get_related(job) - assert 'job_template' in related - - def test_job_template_absent(self, job): - job.job_template.active = False + def test_job_template_absent(self, mocker, job): + job.job_template = None serializer = JobSerializer() related = serializer.get_related(job) assert 'job_template' not in related + def test_job_template_present(self, job): + related = self._mock_and_run(JobSerializer, job) + assert 'job_template' in related + assert related['job_template'] == '/api/v1/%s/%d/' % ('job_templates', job.job_template.pk) + @mock.patch('awx.api.serializers.BaseSerializer.get_summary_fields', lambda x,y: {}) class TestJobOptionsSerializerGetSummaryFields(GetSummaryFieldsMixin): def test__summary_field_labels_10_max(self, mocker, job_template, labels): From da8cd505cf63295612a46dfb095ac25e8a3de7c1 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Mon, 4 Apr 2016 11:05:34 -0400 Subject: [PATCH 3/3] Updated and ported select inventory credential tests to new test system Also added all the test cases I wrote for the credential api but forgot to add before the last checking.. --- .../tests/functional/api/test_credential.py | 223 ++++++++++++++++++ awx/main/tests/old/inventory.py | 31 --- 2 files changed, 223 insertions(+), 31 deletions(-) create mode 100644 awx/main/tests/functional/api/test_credential.py diff --git a/awx/main/tests/functional/api/test_credential.py b/awx/main/tests/functional/api/test_credential.py new file mode 100644 index 0000000000..726742bd31 --- /dev/null +++ b/awx/main/tests/functional/api/test_credential.py @@ -0,0 +1,223 @@ +import mock # noqa +import pytest + +from django.core.urlresolvers import reverse + + +# +# user credential creation +# + +@pytest.mark.django_db +def test_create_user_credential_via_credentials_list(post, get, alice): + response = post(reverse('api:credential_list'), { + 'user': alice.id, + 'name': 'Some name', + 'username': 'someusername' + }, alice) + assert response.status_code == 201 + + response = get(reverse('api:credential_list'), alice) + assert response.status_code == 200 + assert response.data['count'] == 1 + +@pytest.mark.django_db +def test_create_user_credential_via_user_credentials_list(post, get, alice): + response = post(reverse('api:user_credentials_list', args=(alice.pk,)), { + 'name': 'Some name', + 'username': 'someusername', + }, alice) + assert response.status_code == 201 + + response = get(reverse('api:user_credentials_list', args=(alice.pk,)), alice) + assert response.status_code == 200 + assert response.data['count'] == 1 + +@pytest.mark.django_db +def test_create_user_credential_via_credentials_list_xfail(post, alice, bob): + response = post(reverse('api:credential_list'), { + 'user': bob.id, + 'name': 'Some name', + 'username': 'someusername' + }, alice) + assert response.status_code == 403 + +@pytest.mark.django_db +def test_create_user_credential_via_user_credentials_list_xfail(post, alice, bob): + response = post(reverse('api:user_credentials_list', args=(bob.pk,)), { + 'name': 'Some name', + 'username': 'someusername' + }, alice) + assert response.status_code == 403 + + +# +# team credential creation +# + +@pytest.mark.django_db +def test_create_team_credential(post, get, team, org_admin, team_member): + response = post(reverse('api:credential_list'), { + 'team': team.id, + 'name': 'Some name', + 'username': 'someusername' + }, org_admin) + assert response.status_code == 201 + + response = get(reverse('api:team_credentials_list', args=(team.pk,)), team_member) + assert response.status_code == 200 + assert response.data['count'] == 1 + +@pytest.mark.django_db +def test_create_team_credential_via_team_credentials_list(post, get, team, org_admin, team_member): + response = post(reverse('api:team_credentials_list', args=(team.pk,)), { + 'name': 'Some name', + 'username': 'someusername', + }, org_admin) + assert response.status_code == 201 + + response = get(reverse('api:team_credentials_list', args=(team.pk,)), team_member) + assert response.status_code == 200 + assert response.data['count'] == 1 + +@pytest.mark.django_db +def test_create_team_credential_by_urelated_user_xfail(post, team, alice, team_member): + response = post(reverse('api:credential_list'), { + 'team': team.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): + # Members can't add credentials, only org admins.. for now? + response = post(reverse('api:credential_list'), { + 'team': team.id, + 'name': 'Some name', + 'username': 'someusername' + }, team_member) + assert response.status_code == 403 + + + +# +# organization credentials +# + +@pytest.mark.django_db +def test_create_org_credential_as_not_admin(post, organization, org_member): + response = post(reverse('api:credential_list'), { + 'name': 'Some name', + 'username': 'someusername', + 'organization': organization.id, + }, org_member) + assert response.status_code == 403 + +@pytest.mark.django_db +def test_create_org_credential_as_admin(post, organization, org_admin): + response = post(reverse('api:credential_list'), { + 'name': 'Some name', + 'username': 'someusername', + 'organization': organization.id, + }, org_admin) + assert response.status_code == 201 + +@pytest.mark.django_db +def test_list_created_org_credentials(post, get, organization, org_admin, org_member): + response = post(reverse('api:credential_list'), { + 'name': 'Some name', + 'username': 'someusername', + 'organization': organization.id, + }, org_admin) + assert response.status_code == 201 + + response = get(reverse('api:credential_list'), org_admin) + assert response.status_code == 200 + assert response.data['count'] == 1 + + response = get(reverse('api:credential_list'), org_member) + assert response.status_code == 200 + assert response.data['count'] == 0 + + response = get(reverse('api:organization_credential_list', args=(organization.pk,)), org_admin) + assert response.status_code == 200 + assert response.data['count'] == 1 + + response = get(reverse('api:organization_credential_list', args=(organization.pk,)), org_member) + assert response.status_code == 200 + assert response.data['count'] == 0 + + + +# +# Openstack Credentials +# + +@pytest.mark.django_db +def test_openstack_create_ok(post, organization, admin): + data = { + 'kind': 'openstack', + 'name': 'Best credential ever', + 'username': 'some_user', + 'password': 'some_password', + 'project': 'some_project', + 'host': 'some_host', + 'organization': organization.id, + } + response = post(reverse('api:credential_list'), data, admin) + assert response.status_code == 201 + +@pytest.mark.django_db +def test_openstack_create_fail_required_fields(post, organization, admin): + data = { + 'kind': 'openstack', + 'name': 'Best credential ever', + 'organization': organization.id, + } + response = post(reverse('api:credential_list'), data, admin) + assert response.status_code == 400 + assert 'username' in response.data + assert 'password' in response.data + assert 'host' in response.data + assert 'project' in response.data + + +# +# misc xfail conditions +# + +@pytest.mark.django_db +def test_create_credential_xfails(post, organization, team, admin): + # Must specify one of user, team, or organization + response = post(reverse('api:credential_list'), { + 'name': 'Some name', + 'username': 'someusername', + }, admin) + assert response.status_code == 400 + # 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 + response = post(reverse('api:credential_list'), { + 'name': 'Some name', + 'username': 'someusername', + 'organization': organization.id, + 'team': team.id, + }, admin) + assert response.status_code == 400 + response = post(reverse('api:credential_list'), { + 'name': 'Some name', + 'username': 'someusername', + 'user': admin.id, + 'team': team.id, + }, admin) + assert response.status_code == 400 + + + + diff --git a/awx/main/tests/old/inventory.py b/awx/main/tests/old/inventory.py index 73e1bd5eb5..6d167ad10a 100644 --- a/awx/main/tests/old/inventory.py +++ b/awx/main/tests/old/inventory.py @@ -2003,34 +2003,3 @@ class InventoryUpdatesTest(BaseTransactionTest): self.check_inventory_source(inventory_source) self.assertFalse(self.group.all_hosts.filter(instance_id='').exists()) - -class InventoryCredentialTest(BaseTest): - def setUp(self): - super(InventoryCredentialTest, self).setUp() - #self.start_redis() - self.setup_instances() - self.setup_users() - - self.url = reverse('api:credential_list') - - def test_openstack_create_ok(self): - data = { - 'kind': 'openstack', - 'name': 'Best credential ever', - 'username': 'some_user', - 'password': 'some_password', - 'project': 'some_project', - 'host': 'some_host', - } - self.post(self.url, data=data, expect=201, auth=self.get_super_credentials()) - - def test_openstack_create_fail_required_fields(self): - data = { - 'kind': 'openstack', - 'name': 'Best credential ever', - } - response = self.post(self.url, data=data, expect=400, auth=self.get_super_credentials()) - self.assertIn('username', response) - self.assertIn('password', response) - self.assertIn('host', response) - self.assertIn('project', response)