Merge "Show public packages for non-admin users"

murano/api/v1/catalog.py

@@ -176,6 +176,11 @@ class Controller(object):
 
     def search(self, req):
         policy.check("get_package", req.context)
+        manage_public = True
+        try:
+            policy.check("manage_public_package", req.context)
+        except exc.HTTPForbidden:
+            manage_public = False
 
         filters = _get_filters(req.GET.items())
 
@@ -188,7 +193,7 @@ class Controller(object):
 
         catalog = req.GET.pop('catalog', '').lower() == 'true'
         packages = db_api.package_search(
-            filters, req.context, limit, catalog=catalog)
+            filters, req.context, manage_public, limit, catalog=catalog)
         if len(packages) == limit:
             result['next_marker'] = packages[-1].id
         result['packages'] = [package.to_dict() for package in packages]

murano/db/catalog/api.py

@@ -246,15 +246,17 @@ def package_update(pkg_id_or_name, changes, context):
     return pkg
 
 
-def package_search(filters, context, limit=None, catalog=False):
+def package_search(filters, context, manage_public=False,
+                   limit=None, catalog=False):
     """Search packages with different filters
       Catalog param controls the base query creation. Catalog queries
       only search packages a user can deploy. Non-catalog queries searches
       packages a user can edit.
       * Admin is allowed to browse all the packages
       * Regular user is allowed to browse all packages belongs to user tenant
-        and all other packages marked is_public.
+        and all other packages marked is_public in catalog mode.
-        Also all packages should be enabled.
+        In edit-mode non-admins are able to get own packages and public
+        packages if corresponding policy is passed.
       * Use marker (inside filters param) and limit for pagination:
         The typical pattern of limit and marker is to make an initial limited
         request and then to use the ID of the last package from the response
@@ -268,12 +270,15 @@ def package_search(filters, context, limit=None, catalog=False):
 
     if catalog:
         # Only show packages one can deploy, i.e. own + public
-        query = query.filter(or_(
+        query = query.filter(or_(pkg.owner_id == context.tenant,
-            pkg.owner_id == context.tenant, pkg.is_public)
+                                 pkg.is_public))
-        )
     else:
         # Show packages one can edit.
         if not context.is_admin:
+            if manage_public:
+                query = query.filter(or_(pkg.owner_id == context.tenant,
+                                         pkg.is_public))
+            else:
                 query = query.filter(pkg.owner_id == context.tenant)
         # No else here admin can edit everything.
 

murano/tests/unit/api/v1/test_catalog.py

@@ -52,10 +52,12 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
     def test_packages_filtering_admin(self):
         self.is_admin = True
         self._set_policy_rules(
-            {'get_package': ''}
+            {'get_package': '',
+             'manage_public_package': ''}
         )
         for dummy in range(7):
             self.expect_policy_check('get_package')
+            self.expect_policy_check('manage_public_package')
 
         pkg = self._add_pkg('test_tenant', type='Library')
         self._add_pkg('test_tenant')
@@ -104,10 +106,12 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
     def test_packages_filtering_non_admin(self):
         self.is_admin = False
         self._set_policy_rules(
-            {'get_package': ''}
+            {'get_package': '',
+             'manage_public_package': ''}
         )
-        for dummy in range(7):
+        for dummy in range(8):
             self.expect_policy_check('get_package')
+            self.expect_policy_check('manage_public_package')
 
         pkg = self._add_pkg('test_tenant', type='Library')
         self._add_pkg('test_tenant')
@@ -117,7 +121,7 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
         result = self.controller.search(self._get(
             '/v1/catalog/packages/', params={'catalog': 'False',
                                              'owned': 'False'}))
-        self.assertEqual(len(result['packages']), 2)
+        self.assertEqual(len(result['packages']), 3)
         result = self.controller.search(self._get(
             '/v1/catalog/packages/', params={'catalog': 'False',
                                              'owned': 'True'}))
@@ -150,15 +154,22 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
         result = self.controller.search(self._get(
             '/v1/catalog/packages/', params={
                 'type': 'Library'}))
-        self.assertEqual(len(result['packages']), 1)
+        self.assertEqual(len(result['packages']), 2)
+
+        self._set_policy_rules({'get_package': '',
+                                'manage_public_package': '!'})
+        result = self.controller.search(self._get(
+            '/v1/catalog/packages/', params={'catalog': 'False'}))
+        self.assertEqual(len(result['packages']), 2)
 
     def test_packages(self):
         self._set_policy_rules(
-            {'get_package': ''}
+            {'get_package': '',
+             'manage_public_package': ''}
         )
         for dummy in range(9):
             self.expect_policy_check('get_package')
-
+            self.expect_policy_check('manage_public_package')
         result = self.controller.search(self._get('/v1/catalog/packages/'))
         self.assertEqual(len(result['packages']), 0)
 
@@ -188,10 +199,10 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
         self._add_pkg('test_tenant', public=True)
         self._add_pkg('other_tenant', public=True)
 
-        # non-admin can't edit other_tenants public pkgs
+        # non-admin are allowed to edit public packages by policy
         self.is_admin = False
         result = self.controller.search(self._get('/v1/catalog/packages/'))
-        self.assertEqual(len(result['packages']), 3)
+        self.assertEqual(len(result['packages']), 4)
         # can deploy mine + other public
         result = self.controller.search(self._get(
             '/v1/catalog/packages/', params={'catalog': 'True'}))