openstack/manila
Code Issues Proposed changes

Merge "Fix getting share networks and security services error"

Browse Source
This commit is contained in:
Zuul 2017-12-18 13:57:24 +00:00 committed by Gerrit Code Review
commit 81c4ee6c5f
9 changed files with 58 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
manila/api/v1/security_service.py
View File

@ -105,7 +105,7 @@ class SecurityServiceController(wsgi.Controller):
security_services = share_nw['security_services']
del search_opts['share_network_id']
else:
if 'all_tenants' in search_opts:
if 'all_tenants' in search_opts and context.is_admin:
policy.check_policy(context, RESOURCE_NAME,
'get_all_security_services')
security_services = db.security_service_get_all(context)

12
manila/api/v2/share_networks.py
View File

@ -112,20 +112,13 @@ class ShareNetworkController(wsgi.Controller):
search_opts = {}
search_opts.update(req.GET)
if ('all_tenants' in search_opts or
('project_id' in search_opts and
search_opts['project_id'] != context.project_id)):
policy.check_policy(context, RESOURCE_NAME,
'get_all_share_networks')
if 'security_service_id' in search_opts:
networks = db_api.share_network_get_all_by_security_service(
context, search_opts['security_service_id'])
elif ('project_id' in search_opts and
search_opts['project_id'] != context.project_id):
elif context.is_admin and 'project_id' in search_opts:
networks = db_api.share_network_get_all_by_project(
context, search_opts['project_id'])
elif 'all_tenants' in search_opts:
elif context.is_admin and 'all_tenants' in search_opts:
networks = db_api.share_network_get_all(context)
else:
networks = db_api.share_network_get_all_by_project(
@ -160,6 +153,7 @@ class ShareNetworkController(wsgi.Controller):
'limit',
'offset',
'security_service_id',
'project_id'
]
for opt in opts_to_remove:
search_opts.pop(opt, None)

16
manila/tests/api/v1/test_security_service.py
View File

@ -302,18 +302,16 @@ class ShareApiTest(test.TestCase):
db.security_service_get_all.assert_called_once_with(
req.environ['manila.context'])
@mock.patch.object(db, 'security_service_get_all', mock.Mock())
@mock.patch.object(db, 'security_service_get_all_by_project', mock.Mock())
def test_security_services_list_all_tenants_non_admin_context(self):
self.check_policy_patcher.stop()
db.security_service_get_all.return_value = [
self.ss_active_directory,
self.ss_ldap,
]
db.security_service_get_all_by_project.return_value = []
req = fakes.HTTPRequest.blank(
'/security-services?all_tenants=1')
self.assertRaises(exception.PolicyNotAuthorized, self.controller.index,
req)
self.assertFalse(db.security_service_get_all.called)
fake_context = req.environ['manila.context']
self.controller.index(req)
db.security_service_get_all_by_project.assert_called_once_with(
fake_context, fake_context.project_id
)
@mock.patch.object(db, 'security_service_get_all_by_project', mock.Mock())
def test_security_services_list_admin_context_invalid_opts(self):

24
manila/tests/api/v2/test_share_networks.py
View File

@ -384,13 +384,15 @@ class ShareNetworkAPITest(test.TestCase):
result[share_networks.RESOURCES_NAME][0],
fake_sn_with_ss_shortened)
@mock.patch.object(db_api, 'share_network_get_all', mock.Mock())
@mock.patch.object(db_api, 'share_network_get_all_by_project', mock.Mock())
def test_index_all_tenants_non_admin_context(self):
req = fakes.HTTPRequest.blank(
'/share_networks?all_tenants=1')
self.assertRaises(exception.PolicyNotAuthorized, self.controller.index,
req)
self.assertFalse(db_api.share_network_get_all.called)
fake_context = req.environ['manila.context']
db_api.share_network_get_all_by_project.return_value = []
self.controller.index(req)
db_api.share_network_get_all_by_project.assert_called_with(
fake_context, fake_context.project_id)
@mock.patch.object(db_api, 'share_network_get_all', mock.Mock())
def test_index_all_tenants_admin_context(self):
@ -410,15 +412,16 @@ class ShareNetworkAPITest(test.TestCase):
def test_index_filter_by_project_id_non_admin_context(self):
req = fakes.HTTPRequest.blank(
'/share_networks?project_id=fake project')
self.assertRaises(exception.PolicyNotAuthorized, self.controller.index,
req)
self.assertFalse(db_api.share_network_get_all_by_project.called)
fake_context = req.environ['manila.context']
db_api.share_network_get_all_by_project.return_value = []
self.controller.index(req)
db_api.share_network_get_all_by_project.assert_called_with(
fake_context, fake_context.project_id)
@mock.patch.object(db_api, 'share_network_get_all_by_project', mock.Mock())
def test_index_filter_by_project_id_admin_context(self):
db_api.share_network_get_all_by_project.return_value = [
fake_share_network,
fake_share_network_with_ss,
fake_share_network_with_ss
]
req = fakes.HTTPRequest.blank(
'/share_networks?project_id=fake',
@ -435,8 +438,7 @@ class ShareNetworkAPITest(test.TestCase):
mock.Mock())
def test_index_filter_by_ss_and_project_id_admin_context(self):
db_api.share_network_get_all_by_security_service.return_value = [
fake_share_network,
fake_share_network_with_ss,
fake_share_network_with_ss
]
req = fakes.HTTPRequest.blank(
'/share_networks?security_service_id=fake-ss-id&project_id=fake',

8
manila_tempest_tests/tests/api/test_security_services.py
View File

@ -200,3 +200,11 @@ class SecurityServicesTest(base.BaseSharesTest,
self.assertTrue(any(self.ss_ldap['id'] == ss['id'] for ss in listed))
self.assertTrue(any(self.ss_kerberos['id'] == ss['id']
for ss in listed))
@tc.attr(base.TAG_POSITIVE, base.TAG_API)
def test_try_list_security_services_all_tenants(self):
listed = self.shares_client.list_security_services(
params={'all_tenants': 1})
self.assertTrue(any(self.ss_ldap['id'] == ss['id'] for ss in listed))
self.assertTrue(any(self.ss_kerberos['id'] == ss['id']
for ss in listed))

6
manila_tempest_tests/tests/api/test_security_services_negative.py
View File

@ -120,9 +120,3 @@ class SecurityServicesNegativeTest(base.BaseSharesTest):
self.assertRaises(lib_exc.NotFound,
self.shares_client.get_security_service,
ss["id"])
@tc.attr(base.TAG_NEGATIVE, base.TAG_API)
def test_try_list_security_services_all_tenants(self):
self.assertRaises(lib_exc.Forbidden,
self.shares_client.list_security_services,
params={'all_tenants': 1})

20
manila_tempest_tests/tests/api/test_share_networks.py
View File

@ -35,6 +35,26 @@ class ShareNetworkListMixin(object):
keys = ["name", "id"]
[self.assertIn(key, sn.keys()) for sn in listed for key in keys]
@tc.attr(base.TAG_POSITIVE, base.TAG_API)
def test_try_list_share_networks_all_tenants(self):
listed = self.shares_client.list_share_networks_with_detail(
params={'all_tenants': 1})
any(self.sn_with_ldap_ss["id"] in sn["id"] for sn in listed)
# verify keys
keys = ["name", "id"]
[self.assertIn(key, sn.keys()) for sn in listed for key in keys]
@tc.attr(base.TAG_POSITIVE, base.TAG_API)
def test_try_list_share_networks_project_id(self):
listed = self.shares_client.list_share_networks_with_detail(
params={'project_id': 'some_project'})
any(self.sn_with_ldap_ss["id"] in sn["id"] for sn in listed)
# verify keys
keys = ["name", "id"]
[self.assertIn(key, sn.keys()) for sn in listed for key in keys]
@tc.attr(base.TAG_POSITIVE, base.TAG_API)
def test_list_share_networks_with_detail(self):
listed = self.shares_v2_client.list_share_networks_with_detail()

12
manila_tempest_tests/tests/api/test_share_networks_negative.py
View File

@ -81,18 +81,6 @@ class ShareNetworksNegativeTest(base.BaseSharesTest):
self.shares_client.get_security_service,
sn["id"])
@tc.attr(base.TAG_NEGATIVE, base.TAG_API)
def test_try_list_share_networks_all_tenants(self):
self.assertRaises(lib_exc.Forbidden,
self.shares_client.list_share_networks_with_detail,
params={'all_tenants': 1})
@tc.attr(base.TAG_NEGATIVE, base.TAG_API)
def test_try_list_share_networks_project_id(self):
self.assertRaises(lib_exc.Forbidden,
self.shares_client.list_share_networks_with_detail,
params={'project_id': 'some_project'})
@tc.attr(base.TAG_NEGATIVE, base.TAG_API)
def test_try_list_share_networks_wrong_created_since_value(self):
self.assertRaises(

6
releasenotes/notes/bug-1721787-fix-getting-share-networks-and-security-services-error-7e5e7981fcbf2b53.yaml Executable file
View File

@ -0,0 +1,6 @@
---
fixes:
- Non admin users may invoke GET /share-networks and GET /security-services
APIs with the 'all-tenants' flag in the query, however, the flag is
ignored, and only resources belonging to the project will be served. This
API change was made to fix bug 1721787 in the manila client project.