Fix policy check in metadata APIs

When a user doesn't have access to a non-public
resource, the appropriate response is HTTP 404,
not HTTP 403.

Change-Id: I62afec521c5cdfdd67ab83da40e69e6a2688c737
Closes-Bug: #2004230
Signed-off-by: Goutham Pacha Ravi <gouthampravi@gmail.com>

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)

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'}}

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.