From fa54f7ad66419f8dfbd5ac8baa88c12aa17feb87 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Tue, 26 Mar 2013 18:18:05 -0400 Subject: [PATCH] This allows for creation of subresources when posting new objects to a subcollection. --- lib/main/base_views.py | 56 +++++++++++++++++++++++++++++++++++-- lib/main/models/__init__.py | 4 +++ lib/main/tests/inventory.py | 29 +++++++++++-------- lib/main/views.py | 26 +++++++++++++++-- lib/settings/defaults.py | 3 +- lib/urls.py | 2 ++ 6 files changed, 101 insertions(+), 19 deletions(-) diff --git a/lib/main/base_views.py b/lib/main/base_views.py index 9d19d20b15..f8f71965ed 100644 --- a/lib/main/base_views.py +++ b/lib/main/base_views.py @@ -29,6 +29,7 @@ from rest_framework.response import Response from rest_framework import status import exceptions import datetime +import json as python_json # FIXME: machinery for auto-adding audit trail logs to all CREATE/EDITS @@ -40,7 +41,7 @@ class BaseList(generics.ListCreateAPIView): return True if request.method == 'POST': if self.__class__.model in [ User ]: - ok = self.request.user.is_superuser or (self.request.user.admin_of_organizations.count() > 0) + ok = request.user.is_superuser or (request.user.admin_of_organizations.count() > 0) if not ok: raise PermissionDenied() return True @@ -83,9 +84,58 @@ class BaseSubList(BaseList): return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED) parent_id = kwargs['pk'] - sub_id = request.DATA.get('id') + sub_id = request.DATA.get('id', None) main = self.__class__.parent_model.objects.get(pk=parent_id) - subs = self.__class__.model.objects.filter(pk=sub_id) + + subs = None + + if sub_id: + subs = self.__class__.model.objects.filter(pk=sub_id) + else: + if 'disassociate' in request.DATA: + raise PermissionDenied() # ID is required to disassociate + else: + + # this is a little tricky and a little manual + # the object ID was not specified, so it probably doesn't exist in the DB yet. + # we want to see if we can create it. The URL may choose to inject it's primary key into the object + # because we are posting to a subcollection. Use all the normal access control mechanisms. + + inject_primary_key = getattr(self.__class__, 'inject_primary_key_on_post_as', None) + + if inject_primary_key is not None: + + # add the key to the post data using the pk from the URL + request.DATA[inject_primary_key] = kwargs['pk'] + + # attempt to deserialize the object + ser = self.__class__.serializer_class(data=request.DATA) + if not ser.is_valid(): + return Response(status=status.HTTP_400_BAD_REQUEST, data=python_json.dumps(dict(msg='invalid post data'))) + + # ask the usual access control settings + if not self.__class__.model.can_user_add(request.user, ser.init_data): + raise PermissionDenied() + + # save the object through the serializer, reload and returned the saved object deserialized + obj = ser.save() + ser = self.__class__.serializer_class(obj) + + # now make sure we could have already attached the two together. If we could not have, raise an exception + # such that the transaction does not commit. + if not self.__class__.parent_model.can_user_attach(request.user, main, obj, self.__class__.relationship): + raise PermissionDenied() + + return Response(status=status.HTTP_201_CREATED, data=python_json.dumps(ser.data)) + + else: + + # view didn't specify a way to get the pk from the URL, so not even trying + return Response(status=status.HTTP_400_BAD_REQUEST, data=python_json.dumps(dict(msg='object cannot be created'))) + + # we didn't have to create the object, so this is just associating the two objects together now... + # (or disassociating them) + if len(subs) != 1: return Response(status=status.HTTP_400_BAD_REQUEST) sub = subs[0] diff --git a/lib/main/models/__init__.py b/lib/main/models/__init__.py index fc137cdf6b..b7c3805599 100644 --- a/lib/main/models/__init__.py +++ b/lib/main/models/__init__.py @@ -371,6 +371,10 @@ class Host(CommonModelNameNotUnique): def __unicode__(self): return self.name + + @classmethod + def can_user_read(cls, user, obj): + return Inventory.can_user_read(user, obj.inventory) @classmethod def can_user_add(cls, user, data): diff --git a/lib/main/tests/inventory.py b/lib/main/tests/inventory.py index f029b376d7..cf848b1682 100644 --- a/lib/main/tests/inventory.py +++ b/lib/main/tests/inventory.py @@ -130,14 +130,14 @@ class InventoryTest(BaseTest): new_host_c = dict(name='asdf2.example.com', inventory=inv.pk) new_host_d = dict(name='asdf3.example.com', inventory=inv.pk) new_host_e = dict(name='asdf4.example.com', inventory=inv.pk) - data0 = self.post(hosts, data=invalid, expect=400, auth=self.get_super_credentials()) - data0 = self.post(hosts, data=new_host_a, expect=201, auth=self.get_super_credentials()) + host_data0 = self.post(hosts, data=invalid, expect=400, auth=self.get_super_credentials()) + host_data0 = self.post(hosts, data=new_host_a, expect=201, auth=self.get_super_credentials()) # an org admin can add hosts - data1 = self.post(hosts, data=new_host_e, expect=201, auth=self.get_normal_credentials()) + host_data1 = self.post(hosts, data=new_host_e, expect=201, auth=self.get_normal_credentials()) # a normal user cannot add hosts - data2 = self.post(hosts, data=new_host_b, expect=403, auth=self.get_nobody_credentials()) + host_data2 = self.post(hosts, data=new_host_b, expect=403, auth=self.get_nobody_credentials()) # a normal user with inventory edit permissions (on any inventory) can create hosts edit_perm = Permission.objects.create( @@ -145,10 +145,10 @@ class InventoryTest(BaseTest): inventory = Inventory.objects.get(pk=inv.pk), permission_type = PERM_INVENTORY_WRITE ) - data3 = self.post(hosts, data=new_host_c, expect=201, auth=self.get_other_credentials()) + host_data3 = self.post(hosts, data=new_host_c, expect=201, auth=self.get_other_credentials()) # hostnames must be unique inside an organization - data4 = self.post(hosts, data=new_host_c, expect=400, auth=self.get_other_credentials()) + host_data4 = self.post(hosts, data=new_host_c, expect=400, auth=self.get_other_credentials()) ########################################### # GROUPS @@ -163,10 +163,10 @@ class InventoryTest(BaseTest): data0 = self.post(groups, data=new_group_a, expect=201, auth=self.get_super_credentials()) # an org admin can add hosts - data1 = self.post(groups, data=new_group_e, expect=201, auth=self.get_normal_credentials()) + group_data1 = self.post(groups, data=new_group_e, expect=201, auth=self.get_normal_credentials()) # a normal user cannot add hosts - data2 = self.post(groups, data=new_group_b, expect=403, auth=self.get_nobody_credentials()) + group_data2 = self.post(groups, data=new_group_b, expect=403, auth=self.get_nobody_credentials()) # a normal user with inventory edit permissions (on any inventory) can create hosts # already done! @@ -175,13 +175,18 @@ class InventoryTest(BaseTest): # inventory = Inventory.objects.get(pk=inv.pk), # permission_type = PERM_INVENTORY_WRITE #) - data3 = self.post(groups, data=new_group_c, expect=201, auth=self.get_other_credentials()) + group_data3 = self.post(groups, data=new_group_c, expect=201, auth=self.get_other_credentials()) # hostnames must be unique inside an organization - data4 = self.post(groups, data=new_group_c, expect=400, auth=self.get_other_credentials()) + group_data4 = self.post(groups, data=new_group_c, expect=400, auth=self.get_other_credentials()) ################################################# - # HOSTS->inventories + # HOSTS->inventories POST via subcollection + + url = '/api/v1/inventories/1/hosts/' + + new_host = dict(name='web100.example.com') + added_by_collection = self.post(url, data=new_host, expect=201, auth=self.get_super_credentials()) # a super user can associate hosts with inventories @@ -192,7 +197,7 @@ class InventoryTest(BaseTest): # a normal user with edit permission on the inventory can associate hosts with inventories ################################################## - # GROUPS->inventories + # GROUPS->inventories POST via subcollection # a super user can associate groups with inventories diff --git a/lib/main/views.py b/lib/main/views.py index 2f6f5516f9..f93d9a1e1a 100644 --- a/lib/main/views.py +++ b/lib/main/views.py @@ -260,9 +260,7 @@ class InventoryList(BaseList): serializer_class = InventorySerializer permission_classes = (CustomRbac,) - def _get_queryset(self): - ''' I can see inventory when I'm a superuser, an org admin of the inventory, or I have permissions on it ''' - base = Inventory.objects + def _filter_queryset(self, base): if self.request.user.is_superuser: return base.all() admin_of = base.filter(organization__admins__in = [ self.request.user ]).distinct() @@ -276,6 +274,11 @@ class InventoryList(BaseList): ).distinct() return admin_of | has_user_perms | has_team_perms + def _get_queryset(self): + ''' I can see inventory when I'm a superuser, an org admin of the inventory, or I have permissions on it ''' + base = Inventory.objects + return self._filter_queryset(base) + class InventoryDetail(BaseDetail): model = Inventory @@ -315,6 +318,23 @@ class HostsDetail(BaseDetail): serializer_class = HostSerializer permission_classes = (CustomRbac,) +class InventoryHostsList(BaseSubList): + + model = Host + serializer_class = HostSerializer + permission_classes = (CustomRbac,) + # to allow the sub-aspect listing + parent_model = Inventory + relationship = 'hosts' + # to allow posting to this resource to create resources + postable = True + # FIXME: go back and add these to other SubLists + inject_primary_key_on_post_as = 'inventory' + + def _get_queryset(self): + # FIXME: more DRY methods like this + return Inventory._filter_queryset(Inventory.objects.get(pk=self.kwargs['pk']).hosts) + class GroupsList(BaseList): model = Group diff --git a/lib/settings/defaults.py b/lib/settings/defaults.py index f0ccfe252d..d3292439be 100644 --- a/lib/settings/defaults.py +++ b/lib/settings/defaults.py @@ -122,7 +122,8 @@ TEMPLATE_CONTEXT_PROCESSORS += ( MIDDLEWARE_CLASSES += ( 'django.contrib.auth.middleware.AuthenticationMiddleware', 'lib.middleware.exceptions.ExceptionMiddleware', - #'django.middleware.transaction.TransactionMiddleware', + 'django.middleware.transaction.TransactionMiddleware', + # middleware loaded after this point will be subject to transactions ) TEMPLATE_DIRS = ( diff --git a/lib/urls.py b/lib/urls.py index cb6dc58a56..12761b70a9 100644 --- a/lib/urls.py +++ b/lib/urls.py @@ -46,6 +46,7 @@ views_ProjectsDetail = views.OrganizationsDetail.as_view() # inventory service views_InventoryList = views.InventoryList.as_view() views_InventoryDetail = views.InventoryDetail.as_view() +views_InventoryHostsList = views.InventoryHostsList.as_view() # group service views_GroupsList = views.GroupsList.as_view() @@ -95,6 +96,7 @@ urlpatterns = patterns('', # inventory service url(r'^api/v1/inventories/$', views_InventoryList), url(r'^api/v1/inventories/(?P[0-9]+)/$', views_InventoryDetail), + url(r'^api/v1/inventories/(?P[0-9]+)/hosts/$', views_InventoryHostsList), # host service url(r'^api/v1/hosts/$', views_HostsList),