From d193bf7f4ef793780d08922be2de5dd4d56b426b Mon Sep 17 00:00:00 2001 From: Ekaterina Chernova Date: Wed, 16 Sep 2015 19:14:10 +0300 Subject: [PATCH] Show public packages for non-admin users But only if modification of public packages are allowed in policy file. Turn off managing public packagef for non-admin users by default. Currently non-admin users are able to see only their own packages despite of the policy setting. Corresponding tests were updated. Change-Id: I5510f6b8b82d7d28358e1b7dcbffe275079512ee Closes-Bug: #1496418 --- murano/api/v1/catalog.py | 7 +++++- murano/db/catalog/api.py | 19 ++++++++++------ murano/tests/unit/api/v1/test_catalog.py | 29 ++++++++++++++++-------- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/murano/api/v1/catalog.py b/murano/api/v1/catalog.py index d8dd17c4..1a46683a 100644 --- a/murano/api/v1/catalog.py +++ b/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] diff --git a/murano/db/catalog/api.py b/murano/db/catalog/api.py index aed4cf51..f130e1a6 100644 --- a/murano/db/catalog/api.py +++ b/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. - Also all packages should be enabled. + and all other packages marked is_public in catalog mode. + 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,13 +270,16 @@ 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_( - pkg.owner_id == context.tenant, pkg.is_public) - ) + query = query.filter(or_(pkg.owner_id == context.tenant, + pkg.is_public)) else: # Show packages one can edit. if not context.is_admin: - query = query.filter(pkg.owner_id == context.tenant) + 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. if not filters.get('include_disabled', '').lower() == 'true': diff --git a/murano/tests/unit/api/v1/test_catalog.py b/murano/tests/unit/api/v1/test_catalog.py index daa38bca..7f744bc2 100644 --- a/murano/tests/unit/api/v1/test_catalog.py +++ b/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'}))