From 417c02ae8ae362713dc7c46740f1af7e2a9d55c2 Mon Sep 17 00:00:00 2001
From: NiallBunting <niall.bunting@hp.com>
Date: Thu, 19 Nov 2015 14:02:06 +0000
Subject: [PATCH] Disallow user modifing ACTIVE_IMMUTABLE of deactivated images

Currently the user can change the ACTIVE_IMMUTABLE properties whilst
the image is 'deactivated'. This should not be the case once an image
has become 'active'.

APIImpact
Change-Id: I744fbce90893008ef49568c3cba47bf0e26dec9d
Closes-Bug: 1517060
Closes-Bug: 1517963
(cherry picked from commit fbe964a0f20b9ab708b85634c3d707630d403dcd)
---
 glance/api/v1/images.py          |  6 ++-
 glance/tests/unit/v1/test_api.py | 66 +++++++++++++++++++++++++++++++-
 2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py
index 95e32949d9..78784fbdfb 100644
--- a/glance/api/v1/images.py
+++ b/glance/api/v1/images.py
@@ -906,9 +906,11 @@ class Controller(controller.BaseController):
             # Once an image is 'active' only an admin can
             # modify certain core metadata keys
             for key in ACTIVE_IMMUTABLE:
-                if (orig_status == 'active' and image_meta.get(key) is not None
+                if ((orig_status == 'active' or orig_status == 'deactivated')
+                        and image_meta.get(key) is not None
                         and image_meta.get(key) != orig_image_meta.get(key)):
-                    msg = _("Forbidden to modify '%s' of active image.") % key
+                    msg = _("Forbidden to modify '%(key)s' of %(status)s "
+                            "image.") % {'key': key, 'status': orig_status}
                     raise HTTPForbidden(explanation=msg,
                                         request=req,
                                         content_type="text/plain")
diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py
index fe949df920..313c36f49c 100644
--- a/glance/tests/unit/v1/test_api.py
+++ b/glance/tests/unit/v1/test_api.py
@@ -101,7 +101,7 @@ class TestGlanceAPI(base.IsolatedUnitTest):
              'updated_at': timeutils.utcnow(),
              'deleted_at': None,
              'deleted': False,
-             'checksum': None,
+             'checksum': '13',
              'size': 13,
              'locations': [{'url': "file:///%s/%s" % (self.test_dir, UUID1),
                             'metadata': {}, 'status': 'active'}],
@@ -2995,6 +2995,40 @@ class TestGlanceAPI(base.IsolatedUnitTest):
                 self.assertEqual(200, res.status_int)
                 self.assertEqual(orig_value, res.headers[k])
 
+    def test_deactivated_image_immutable_props_for_user(self):
+        """
+        Tests user cannot update immutable props of deactivated image
+        """
+        test_router_api = router.API(self.mapper)
+        self.api = test_utils.FakeAuthMiddleware(
+            test_router_api, is_admin=False)
+        fixture_header_list = [{'x-image-meta-checksum': '1234'},
+                               {'x-image-meta-size': '12345'}]
+        for fixture_header in fixture_header_list:
+            req = webob.Request.blank('/images/%s' % UUID3)
+            req.method = 'PUT'
+            for k, v in six.iteritems(fixture_header):
+                req = webob.Request.blank('/images/%s' % UUID3)
+                req.method = 'HEAD'
+                res = req.get_response(self.api)
+                self.assertEqual(200, res.status_int)
+                orig_value = res.headers[k]
+
+                req = webob.Request.blank('/images/%s' % UUID3)
+                req.headers[k] = v
+                req.method = 'PUT'
+                res = req.get_response(self.api)
+                self.assertEqual(403, res.status_int)
+                prop = k[len('x-image-meta-'):]
+                self.assertNotEqual(-1, res.body.find(
+                    "Forbidden to modify '%s' of deactivated image" % prop))
+
+                req = webob.Request.blank('/images/%s' % UUID3)
+                req.method = 'HEAD'
+                res = req.get_response(self.api)
+                self.assertEqual(200, res.status_int)
+                self.assertEqual(orig_value, res.headers[k])
+
     def test_props_of_active_image_mutable_for_admin(self):
         """
         Tests admin can update 'immutable' props of active image
@@ -3025,6 +3059,36 @@ class TestGlanceAPI(base.IsolatedUnitTest):
                 self.assertEqual(200, res.status_int)
                 self.assertEqual(v, res.headers[k])
 
+    def test_props_of_deactivated_image_mutable_for_admin(self):
+        """
+        Tests admin can update 'immutable' props of deactivated image
+        """
+        test_router_api = router.API(self.mapper)
+        self.api = test_utils.FakeAuthMiddleware(
+            test_router_api, is_admin=True)
+        fixture_header_list = [{'x-image-meta-checksum': '1234'},
+                               {'x-image-meta-size': '12345'}]
+        for fixture_header in fixture_header_list:
+            req = webob.Request.blank('/images/%s' % UUID3)
+            req.method = 'PUT'
+            for k, v in six.iteritems(fixture_header):
+                req = webob.Request.blank('/images/%s' % UUID3)
+                req.method = 'HEAD'
+                res = req.get_response(self.api)
+                self.assertEqual(200, res.status_int)
+
+                req = webob.Request.blank('/images/%s' % UUID3)
+                req.headers[k] = v
+                req.method = 'PUT'
+                res = req.get_response(self.api)
+                self.assertEqual(200, res.status_int)
+
+                req = webob.Request.blank('/images/%s' % UUID3)
+                req.method = 'HEAD'
+                res = req.get_response(self.api)
+                self.assertEqual(200, res.status_int)
+                self.assertEqual(v, res.headers[k])
+
     def test_replace_members_non_existing_image(self):
         """
         Tests replacing image members raises right exception