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
This commit is contained in:
Ekaterina Chernova 2015-09-16 19:14:10 +03:00
parent a8e91b16e7
commit d193bf7f4e
3 changed files with 38 additions and 17 deletions

View File

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

View File

@ -246,15 +246,17 @@ def package_update(pkg_id_or_name, changes, context):
return pkg 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 """Search packages with different filters
Catalog param controls the base query creation. Catalog queries Catalog param controls the base query creation. Catalog queries
only search packages a user can deploy. Non-catalog queries searches only search packages a user can deploy. Non-catalog queries searches
packages a user can edit. packages a user can edit.
* Admin is allowed to browse all the packages * Admin is allowed to browse all the packages
* Regular user is allowed to browse all packages belongs to user tenant * 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: * Use marker (inside filters param) and limit for pagination:
The typical pattern of limit and marker is to make an initial limited 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 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: if catalog:
# Only show packages one can deploy, i.e. own + public # 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: else:
# Show packages one can edit. # Show packages one can edit.
if not context.is_admin: 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. # No else here admin can edit everything.
if not filters.get('include_disabled', '').lower() == 'true': if not filters.get('include_disabled', '').lower() == 'true':

View File

@ -52,10 +52,12 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
def test_packages_filtering_admin(self): def test_packages_filtering_admin(self):
self.is_admin = True self.is_admin = True
self._set_policy_rules( self._set_policy_rules(
{'get_package': ''} {'get_package': '',
'manage_public_package': ''}
) )
for dummy in range(7): for dummy in range(7):
self.expect_policy_check('get_package') self.expect_policy_check('get_package')
self.expect_policy_check('manage_public_package')
pkg = self._add_pkg('test_tenant', type='Library') pkg = self._add_pkg('test_tenant', type='Library')
self._add_pkg('test_tenant') self._add_pkg('test_tenant')
@ -104,10 +106,12 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
def test_packages_filtering_non_admin(self): def test_packages_filtering_non_admin(self):
self.is_admin = False self.is_admin = False
self._set_policy_rules( 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('get_package')
self.expect_policy_check('manage_public_package')
pkg = self._add_pkg('test_tenant', type='Library') pkg = self._add_pkg('test_tenant', type='Library')
self._add_pkg('test_tenant') self._add_pkg('test_tenant')
@ -117,7 +121,7 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
result = self.controller.search(self._get( result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'False', '/v1/catalog/packages/', params={'catalog': 'False',
'owned': 'False'})) 'owned': 'False'}))
self.assertEqual(len(result['packages']), 2) self.assertEqual(len(result['packages']), 3)
result = self.controller.search(self._get( result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'False', '/v1/catalog/packages/', params={'catalog': 'False',
'owned': 'True'})) 'owned': 'True'}))
@ -150,15 +154,22 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
result = self.controller.search(self._get( result = self.controller.search(self._get(
'/v1/catalog/packages/', params={ '/v1/catalog/packages/', params={
'type': 'Library'})) '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): def test_packages(self):
self._set_policy_rules( self._set_policy_rules(
{'get_package': ''} {'get_package': '',
'manage_public_package': ''}
) )
for dummy in range(9): for dummy in range(9):
self.expect_policy_check('get_package') self.expect_policy_check('get_package')
self.expect_policy_check('manage_public_package')
result = self.controller.search(self._get('/v1/catalog/packages/')) result = self.controller.search(self._get('/v1/catalog/packages/'))
self.assertEqual(len(result['packages']), 0) 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('test_tenant', public=True)
self._add_pkg('other_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 self.is_admin = False
result = self.controller.search(self._get('/v1/catalog/packages/')) 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 # can deploy mine + other public
result = self.controller.search(self._get( result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'True'})) '/v1/catalog/packages/', params={'catalog': 'True'}))