[RBAC] Return 404 if share is inaccessible

When a user is prevented from listing a non-public
share, the API service would return a 403 Forbidden.
This isn't consistent with the API SIG's guidance
on resources restricted by virtue of RBAC policy since
users with malicious intent may use the signal to
mean that the resource exists.

Depends-On: I27fdd7dfffeb15965b66dbb3f6b1568c11ff9ad4
Change-Id: I7e05dcb343c932cc7fec8d395919053d0a1801ce
Closes-Bug: #1901210
Signed-off-by: Goutham Pacha Ravi <gouthampravi@gmail.com>

manila/share/api.py

@@ -1952,7 +1952,10 @@ class API(base.Base):
     def get(self, context, share_id):
         rv = self.db.share_get(context, share_id)
         if not rv['is_public']:
-            policy.check_policy(context, 'share', 'get', rv)
+            authorized = policy.check_policy(
+                context, 'share', 'get', rv, do_raise=False)
+            if not authorized:
+                raise exception.NotFound()
         return rv
 
     def get_all(self, context, search_opts=None, sort_key='created_at',

manila/tests/api/v2/test_share_accesses.py

@@ -137,7 +137,7 @@ class ShareAccessesAPITest(test.TestCase):
             mock.call(req.environ['manila.context'],
                       'share', 'access_get'),
             mock.call(req.environ['manila.context'],
-                      'share', 'get', mock.ANY)])
+                      'share', 'get', mock.ANY, do_raise=False)])
         policy_check_call_args_list = policy.check_policy.call_args_list[2][0]
         share_being_checked = policy_check_call_args_list[3]
         self.assertEqual('c3c5ec1ccc4640d0af1914cbf11f05ad',

manila/tests/api/v2/test_share_instances.py

@@ -201,7 +201,7 @@ class ShareInstancesAPITest(test.TestCase):
         req = self._get_request('fake', version=version)
         req_context = req.environ['manila.context']
         share_policy_check_call = mock.call(
-            req_context, 'share', 'get', mock.ANY)
+            req_context, 'share', 'get', mock.ANY, do_raise=False)
         get_instances_policy_check_call = mock.call(
             req_context, 'share_instance', 'index')
 

manila/tests/share/test_api.py

@@ -2640,10 +2640,29 @@ class ShareAPITestCase(test.TestCase):
             result = self.api.get(self.context, 'fakeid')
             self.assertEqual(share, result)
             share_api.policy.check_policy.assert_called_once_with(
-                self.context, 'share', 'get', share)
+                self.context, 'share', 'get', share, do_raise=False)
             db_api.share_get.assert_called_once_with(
                 self.context, 'fakeid')
 
+    def test_get_not_authorized(self):
+        share = db_utils.create_share(
+            is_public=False,
+            project_id='5db325fc4de14fe1a860ff69f190c78c')
+        share_api.policy.check_policy.return_value = False
+        ctx = context.RequestContext('df6d65cc1f8946ba86be06b8140ec4b3',
+                                     'e8133457b853436591a7e4610e7ce679',
+                                     is_admin=False)
+        with mock.patch.object(db_api, 'share_get',
+                               mock.Mock(return_value=share)):
+
+            self.assertRaises(exception.NotFound,
+                              self.api.get,
+                              ctx,
+                              share['id'])
+            share_api.policy.check_policy.assert_called_once_with(
+                ctx, 'share', 'get', share, do_raise=False)
+            db_api.share_get.assert_called_once_with(ctx, share['id'])
+
     @mock.patch.object(db_api, 'share_snapshot_get_all_by_project',
                        mock.Mock())
     def test_get_all_snapshots_admin_not_all_tenants(self):

releasenotes/notes/bug-1901210-return-404-if-share-access-forbidden-02ca9a9552ad3e15.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    The GET /shares/{share_id} API now responds with HTTP 404 (Not Found)
+    for inaccessible resources. See `bug 1901210
+    <https://launchpad.net/bugs/1901210>`_ for further information.