diff --git a/manila/api/v1/security_service.py b/manila/api/v1/security_service.py index b577d27afb..6fe5805278 100644 --- a/manila/api/v1/security_service.py +++ b/manila/api/v1/security_service.py @@ -108,11 +108,15 @@ class SecurityServiceController(wsgi.Controller): security_services = share_nw['security_services'] del search_opts['share_network_id'] else: - if context.is_admin and utils.is_all_tenants(search_opts): - policy.check_policy(context, RESOURCE_NAME, - 'get_all_security_services') - security_services = db.security_service_get_all(context) - else: + # ignore all_tenants if not authorized to use it. + security_services = None + if utils.is_all_tenants(search_opts): + allowed_to_list_all_tenants = policy.check_policy( + context, RESOURCE_NAME, 'get_all_security_services', + do_raise=False) + if allowed_to_list_all_tenants: + security_services = db.security_service_get_all(context) + if security_services is None: security_services = db.security_service_get_all_by_project( context, context.project_id) search_opts.pop('all_tenants', None) diff --git a/manila/tests/api/v1/test_security_service.py b/manila/tests/api/v1/test_security_service.py index 7946ab5641..35cd1e7f64 100644 --- a/manila/tests/api/v1/test_security_service.py +++ b/manila/tests/api/v1/test_security_service.py @@ -23,6 +23,7 @@ from manila.api.v1 import security_service from manila.common import constants from manila import db from manila import exception +from manila import policy from manila import test from manila.tests.api import fakes @@ -339,31 +340,38 @@ class ShareApiTest(test.TestCase): req.environ['manila.context'], sn['id']) + @mock.patch.object(db, 'security_service_get_all_by_project', mock.Mock()) @mock.patch.object(db, 'security_service_get_all', mock.Mock()) - def test_security_services_list_all_tenants_admin_context(self): + def test_security_services_list_all_tenants_policy_authorized(self): self.check_policy_patcher.stop() db.security_service_get_all.return_value = [ self.ss_active_directory, self.ss_ldap, ] - req = fakes.HTTPRequest.blank( - '/security-services?all_tenants=1&name=fake-name', - use_admin_context=True) - res_dict = self.controller.index(req) - self.assertEqual(self.security_service_list_expected_resp, res_dict) - db.security_service_get_all.assert_called_once_with( - req.environ['manila.context']) - - @mock.patch.object(db, 'security_service_get_all_by_project', mock.Mock()) - def test_security_services_list_all_tenants_non_admin_context(self): - db.security_service_get_all_by_project.return_value = [] req = fakes.HTTPRequest.blank( '/security-services?all_tenants=1') + self.mock_object(policy, "check_policy", mock.Mock(return_value=True)) 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 - ) + db.security_service_get_all_by_project.assert_not_called() + db.security_service_get_all.assert_called_once_with(fake_context) + + @mock.patch.object(db, 'security_service_get_all_by_project', mock.Mock()) + @mock.patch.object(db, 'security_service_get_all', mock.Mock()) + def test_security_services_list_all_tenants_policy_not_authorized(self): + self.check_policy_patcher.stop() + db.security_service_get_all.return_value = [ + self.ss_active_directory, + self.ss_ldap, + ] + req = fakes.HTTPRequest.blank( + '/security-services?all_tenants=1') + self.mock_object(policy, + "check_policy", + mock.Mock(side_effect=exception.NotAuthorized())) + self.assertRaises(exception.NotAuthorized, self.controller.index, req) + db.security_service_get_all_by_project.assert_not_called() + db.security_service_get_all.assert_not_called() @mock.patch.object(db, 'security_service_get_all', mock.Mock()) def test_security_services_list_all_tenants_with_invalid_value(self): diff --git a/releasenotes/notes/bug-1916102-fix-security-service-policy-check-8e72254fa9fedc9e.yaml b/releasenotes/notes/bug-1916102-fix-security-service-policy-check-8e72254fa9fedc9e.yaml new file mode 100644 index 0000000000..b87424deab --- /dev/null +++ b/releasenotes/notes/bug-1916102-fix-security-service-policy-check-8e72254fa9fedc9e.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Decoupled the RBAC ``share:get_all_security_services`` from + ``context_is_admin``, potentially allowing the use of the + ``all_tenants`` query by non-administrators.