diff --git a/manila/api/v2/metadata.py b/manila/api/v2/metadata.py index 40afd306dc..e3ab4253cf 100644 --- a/manila/api/v2/metadata.py +++ b/manila/api/v2/metadata.py @@ -74,7 +74,7 @@ class MetadataController(object): for_modification=False, parent_id=None): if self.resource_name in ['share', 'share_network_subnet']: # we would allow retrieving some "public" resources - # across project namespaces excpet share snaphots, + # across project namespaces except share snapshots, # project_only=True is hard coded kwargs = {} else: @@ -87,10 +87,22 @@ class MetadataController(object): res = get_res_method(context, resource_id, **kwargs) get_policy = self.resource_policy_get[self.resource_name] - if res.get('is_public') is False or for_modification: - policy.check_policy(context, self.resource_name, - get_policy, res) - + if res.get('is_public') is False: + authorized = policy.check_policy(context, + self.resource_name, + get_policy, + res, + do_raise=False) + if not authorized: + # Raising NotFound to prevent existence detection + raise exception.NotFound() + elif for_modification: + # a public resource's metadata can be viewed, but not + # modified by non owners + policy.check_policy(context, + self.resource_name, + get_policy, + res) except exception.NotFound: msg = _('%s not found.' % self.resource_name.capitalize()) raise exc.HTTPNotFound(explanation=msg) diff --git a/manila/tests/api/v2/test_metadata.py b/manila/tests/api/v2/test_metadata.py index e775c7727c..d443f0cda2 100644 --- a/manila/tests/api/v2/test_metadata.py +++ b/manila/tests/api/v2/test_metadata.py @@ -27,7 +27,7 @@ from manila.tests import db_utils @ddt.ddt class MetadataAPITest(test.TestCase): - def _get_request(self, version="2.65", use_admin_context=True): + def _get_request(self, version="2.65", use_admin_context=False): req = fakes.HTTPRequest.blank( '/v2/shares/{resource_id}/metadata', version=version, use_admin_context=use_admin_context) @@ -38,11 +38,49 @@ class MetadataAPITest(test.TestCase): self.controller = ( metadata.MetadataController()) self.controller.resource_name = 'share' - self.admin_context = context.RequestContext('admin', 'fake', True) self.mock_policy_check = self.mock_object( policy, 'check_policy', mock.Mock(return_value=True)) self.resource = db_utils.create_share(size=1) + def test__get_resource_policy_not_authorized_pubic_resource(self): + fake_context = context.RequestContext('fake', 'fake', True) + policy_exception = exception.PolicyNotAuthorized(action='share:get') + mock_policy_check = self.mock_object( + policy, 'check_policy', mock.Mock(side_effect=policy_exception)) + share_obj = db_utils.create_share(size=1, is_public=True) + + self.assertRaises( + exception.PolicyNotAuthorized, + self.controller._get_resource, + fake_context, + share_obj['id'], + for_modification=True, + ) + + mock_policy_check.assert_called_once_with( + fake_context, 'share', 'get', mock.ANY) + policy_resource_obj = mock_policy_check.call_args[0][3] + self.assertEqual(share_obj['id'], policy_resource_obj['id']) + + @ddt.data(True, False) + def test__get_resource_policy_not_authorized_private_resource(self, formd): + fake_context = context.RequestContext('fake', 'fake', True) + mock_policy_check = self.mock_object( + policy, 'check_policy', mock.Mock(return_value=False)) + share_obj = db_utils.create_share(size=1, is_public=False) + + self.assertRaises( + webob.exc.HTTPNotFound, + self.controller._get_resource, + fake_context, + share_obj['id'], + for_modification=formd, + ) + mock_policy_check.assert_called_once_with( + fake_context, 'share', 'get', mock.ANY, do_raise=False) + policy_resource_obj = mock_policy_check.call_args[0][3] + self.assertEqual(share_obj['id'], policy_resource_obj['id']) + def test_create_index_metadata(self): url = self._get_request() body = {'metadata': {'test_key1': 'test_v1', 'test_key2': 'test_v2'}} diff --git a/releasenotes/notes/bug-2004230-fix-cross-project-rbac-328134c64c96c200.yaml b/releasenotes/notes/bug-2004230-fix-cross-project-rbac-328134c64c96c200.yaml new file mode 100644 index 0000000000..4ce0fa0136 --- /dev/null +++ b/releasenotes/notes/bug-2004230-fix-cross-project-rbac-328134c64c96c200.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Metadata APIs have been fixed to respond with HTTP 404 / Not Found when + the requester does not have access to a resource that the metadata + pertains to.