From 8d2b5e7c44ee2da7d4a8207860db0367ef51fb3e Mon Sep 17 00:00:00 2001
From: Kirill Zaitsev <kzaitsev@mirantis.com>
Date: Thu, 9 Apr 2015 15:36:16 +0300
Subject: [PATCH] Forbid setting is_public via querystring

All package parameters are specified via json body.
Before there was an undocumented ability to set 'is_public' via
querystring. This patch removes this exception and updates respective
tests

Change-Id: Idb284ca1af2902abcc19da112da9024bbbd28fc6
Closes-Bug: #1441125
---
 murano/api/v1/catalog.py                 |  4 --
 murano/tests/unit/api/base.py            |  3 +-
 murano/tests/unit/api/v1/test_catalog.py | 49 ++++++++++++++++--------
 3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/murano/api/v1/catalog.py b/murano/api/v1/catalog.py
index 738e9725..74d9e0dd 100644
--- a/murano/api/v1/catalog.py
+++ b/murano/api/v1/catalog.py
@@ -237,10 +237,6 @@ class Controller(object):
             if hasattr(pkg_to_upload, k):
                 package_meta[v] = getattr(pkg_to_upload, k)
 
-        if req.params.get('is_public', '').lower() == 'true':
-            policy.check('publicize_package', req.context)
-            package_meta['is_public'] = True
-
         try:
             package = db_api.package_upload(package_meta, req.context.tenant)
         except db_exc.DBDuplicateEntry:
diff --git a/murano/tests/unit/api/base.py b/murano/tests/unit/api/base.py
index 3b95c2a4..30642ce0 100644
--- a/murano/tests/unit/api/base.py
+++ b/murano/tests/unit/api/base.py
@@ -181,7 +181,8 @@ class ControllerTest(object):
         environ['REQUEST_METHOD'] = method
 
         req = wsgi.Request(environ)
-        req.context = utils.dummy_context(user, tenant)
+        req.context = utils.dummy_context(user, tenant,
+                                          is_admin=self.is_admin)
         self.context = req.context
         req.content_type = content_type
         req.body = data
diff --git a/murano/tests/unit/api/v1/test_catalog.py b/murano/tests/unit/api/v1/test_catalog.py
index e110c200..a7dd0c8d 100644
--- a/murano/tests/unit/api/v1/test_catalog.py
+++ b/murano/tests/unit/api/v1/test_catalog.py
@@ -60,7 +60,7 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
             'ui_definition': pkg.raw_ui,
             'class_definitions': pkg.classes,
             'archive': pkg.blob,
-            'categories': []
+            'categories': [],
         }
         return pkg, package
 
@@ -90,9 +90,12 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
     def test_add_public_unauthorized(self):
         policy.set_rules({
             'upload_package': '@',
-            'publicize_package': 'role:is_admin or is_admin:True'
+            'publicize_package': 'is_admin:True',
+            'delete_package': 'is_admin:True',
         })
 
+        self.expect_policy_check('upload_package')
+        self.expect_policy_check('delete_package', mock.ANY)
         self.expect_policy_check('upload_package')
         self.expect_policy_check('publicize_package')
         self.expect_policy_check('upload_package')
@@ -106,37 +109,51 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
         body = '''\
 
 --BOUNDARY
-Content-Disposition: form-data; name="ziparchive"
-Content-Type: text/plain:
+Content-Disposition: form-data; name="__metadata__"
+
+{0}
+--BOUNDARY
+Content-Disposition: form-data; name="ziparchive"; filename="file.zip"
 
 This is a fake zip archive
---BOUNDARY
-Content-Disposition: form-data; name="metadata"; filename="test.json"
-Content-Type: application/json
-
-%s
---BOUNDARY--''' % package_metadata
+--BOUNDARY--'''
 
         with mock.patch('murano.packages.load_utils.load_from_file') as lff:
             lff.return_value = package_from_dir
+
+            # Uploading a non-public package
             req = self._post(
                 '/catalog/packages',
-                body,
+                body.format(json.dumps({'is_public': False})),
                 content_type='multipart/form-data; ; boundary=BOUNDARY',
-                params={"is_public": "true"})
+            )
+            res = req.get_response(self.api)
+            self.assertEqual(200, res.status_code)
+
+            self.is_admin = True
+            app_id = json.loads(res.body)['id']
+            req = self._delete('/catalog/packages/{0}'.format(app_id))
             res = req.get_response(self.api)
 
-            # Nobody has access to upload public images
+            self.is_admin = False
+            # Uploading a public package fails
+            req = self._post(
+                '/catalog/packages',
+                body.format(json.dumps({'is_public': True})),
+                content_type='multipart/form-data; ; boundary=BOUNDARY',
+            )
+            res = req.get_response(self.api)
             self.assertEqual(403, res.status_code)
 
+            # Uploading a public package passes for admin
             self.is_admin = True
             req = self._post(
                 '/catalog/packages',
-                body,
+                body.format(json.dumps({'is_public': True})),
                 content_type='multipart/form-data; ; boundary=BOUNDARY',
-                params={"is_public": "true"})
+            )
             res = req.get_response(self.api)
-            self.assertEqual(403, res.status_code)
+            self.assertEqual(200, res.status_code)
 
     def test_add_category(self):
         """Check that category added successfully