Merge "Reject rather than ignore forbidden updates"

glance/api/v2/images.py

@@ -65,12 +65,7 @@ class ImagesController(object):
 
     @utils.mutating
     def create(self, req, image):
-        if 'owner' not in image:
-            image['owner'] = req.context.owner
-        elif not req.context.is_admin:
-            raise webob.exc.HTTPForbidden()
-
-        #TODO(bcwaldon): this should eventually be settable through the API
+        image.setdefault('owner', req.context.owner)
         image['status'] = 'queued'
 
         tags = self._extract_tags(image)
@@ -175,14 +170,23 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer):
         if 'visibility' in image:
             image['is_public'] = image.pop('visibility') == 'public'
 
-        self._remove_readonly(image)
+        self._check_readonly(image)
+        self._check_adminonly(image, request.context)
         return {'image': image}
 
     @staticmethod
-    def _remove_readonly(image):
+    def _check_readonly(image):
         for key in ['created_at', 'updated_at', 'status']:
             if key in image:
-                del image[key]
+                msg = "Attribute \'%s\' is read-only." % key
+                raise webob.exc.HTTPForbidden(explanation=unicode(msg))
+
+    @staticmethod
+    def _check_adminonly(image, context):
+        for key in ['owner']:
+            if key in image and not context.is_admin:
+                msg = "Must be admin to set attribute \'%s\'." % key
+                raise webob.exc.HTTPForbidden(explanation=unicode(msg))
 
     def create(self, request):
         return self._parse_image(request)

glance/tests/unit/v2/test_images_resource.py

@@ -328,12 +328,6 @@ class TestImagesController(test_utils.BaseTestCase):
         }
         self.assertEqual(expected, output)
 
-    def test_create_with_owner_forbidden(self):
-        request = unit_test_utils.get_fake_request()
-        image = {'name': 'image-1', 'owner': utils.generate_uuid()}
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.create,
-                          request, image)
-
     def test_create_with_owner_as_admin(self):
         request = unit_test_utils.get_fake_request(is_admin=True)
         image = {'name': 'image-1', 'owner': utils.generate_uuid()}
@@ -411,9 +405,15 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
         expected = {'image': {'name': 'image-1', 'properties': {}}}
         self.assertEqual(expected, output)
 
-    def test_create_with_owner(self):
+    def test_create_with_owner_forbidden(self):
         request = unit_test_utils.get_fake_request()
         request.body = json.dumps({'owner': TENANT2})
+        self.assertRaises(webob.exc.HTTPForbidden,
+                          self.deserializer.create, request)
+
+    def test_create_with_owner_admin(self):
+        request = unit_test_utils.get_fake_request(is_admin=True)
+        request.body = json.dumps({'owner': TENANT2})
         output = self.deserializer.create(request)
         expected = {'image': {'owner': TENANT2, 'properties': {}}}
         self.assertEqual(expected, output)
@@ -432,20 +432,18 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
         expected = {'image': {'is_public': False, 'properties': {}}}
         self.assertEqual(expected, output)
 
-    def test_create_readonly_attributes_ignored(self):
+    def test_create_readonly_attributes_forbidden(self):
         for key in ['created_at', 'updated_at']:
             request = unit_test_utils.get_fake_request()
             request.body = json.dumps({key: ISOTIME})
-            output = self.deserializer.create(request)
-            expected = {'image': {'properties': {}}}
-            self.assertEqual(expected, output)
+            self.assertRaises(webob.exc.HTTPForbidden,
+                              self.deserializer.update, request)
 
-    def test_create_status_attribute_ignored(self):
+    def test_create_status_attribute_forbidden(self):
         request = unit_test_utils.get_fake_request()
         request.body = json.dumps({'status': 'saving'})
-        output = self.deserializer.create(request)
-        expected = {'image': {'properties': {}}}
-        self.assertEqual(expected, output)
+        self.assertRaises(webob.exc.HTTPForbidden,
+                          self.deserializer.update, request)
 
     def test_create_with_tags(self):
         request = unit_test_utils.get_fake_request()
@@ -467,20 +465,18 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
         }
         self.assertEqual(expected, output)
 
-    def test_update_readonly_attributes_ignored(self):
+    def test_update_readonly_attributes_forbidden(self):
         for key in ['created_at', 'updated_at']:
             request = unit_test_utils.get_fake_request()
             request.body = json.dumps({key: ISOTIME})
-            output = self.deserializer.update(request)
-            expected = {'image': {'properties': {}}}
-            self.assertEqual(expected, output)
+            self.assertRaises(webob.exc.HTTPForbidden,
+                              self.deserializer.update, request)
 
-    def test_update_status_attribute_ignored(self):
+    def test_update_status_attribute_forbidden(self):
         request = unit_test_utils.get_fake_request()
         request.body = json.dumps({'status': 'saving'})
-        output = self.deserializer.update(request)
-        expected = {'image': {'properties': {}}}
-        self.assertEqual(expected, output)
+        self.assertRaises(webob.exc.HTTPForbidden,
+                          self.deserializer.update, request)
 
     def test_index(self):
         marker = utils.generate_uuid()