fix overaggressive 403->404 conversion

When a user is not authorized to see a given resource, we need to
convert HTTP 403s into HTTP 404s to avoid giving away information
that the resource exists. However, the previous code was being
overaggressive and doing this conversion even in some cases where
the user is allowed to see the resource and really needs to know
that what they were trying to do is forbidden, not be told that the
resource doesn't exist. This fixes that logic to only do the 403
to 404 conversion when truly appropriate.

Change-Id: I7a5b0a9e89c8a71490dd74497794a52489f46cd2
Closes-Bug: 1682621

neutron/api/v2/base.py

@@ -575,7 +575,13 @@ class Controller(object):
                            pluralized=self._collection)
         except oslo_policy.PolicyNotAuthorized:
             # To avoid giving away information, pretend that it
-            # doesn't exist
+            # doesn't exist if policy does not authorize SHOW
+            with excutils.save_and_reraise_exception() as ctxt:
+                if not policy.check(request.context,
+                                    self._plugin_handlers[self.SHOW],
+                                    obj,
+                                    pluralized=self._collection):
+                    ctxt.reraise = False
             msg = _('The resource could not be found.')
             raise webob.exc.HTTPNotFound(msg)
 
@@ -640,13 +646,13 @@ class Controller(object):
                            orig_obj,
                            pluralized=self._collection)
         except oslo_policy.PolicyNotAuthorized:
+            # To avoid giving away information, pretend that it
+            # doesn't exist if policy does not authorize SHOW
             with excutils.save_and_reraise_exception() as ctxt:
-                # If a tenant is modifying its own object, it's safe to return
-                # a 403. Otherwise, pretend that it doesn't exist to avoid
-                # giving away information.
-                orig_obj_tenant_id = orig_obj.get("tenant_id")
-                if (request.context.tenant_id != orig_obj_tenant_id or
-                    orig_obj_tenant_id is None):
+                if not policy.check(request.context,
+                                    self._plugin_handlers[self.SHOW],
+                                    orig_obj,
+                                    pluralized=self._collection):
                     ctxt.reraise = False
             msg = _('The resource could not be found.')
             raise webob.exc.HTTPNotFound(msg)

neutron/tests/tempest/api/test_qos.py

@@ -473,7 +473,7 @@ class QosBandwidthLimitRuleTestJSON(base.BaseAdminNetworkTest):
                                                     max_kbps=1,
                                                     max_burst_kbps=1)
         self.assertRaises(
-            exceptions.NotFound,
+            exceptions.Forbidden,
             self.client.update_bandwidth_limit_rule,
             policy['id'], rule['id'], max_kbps=2, max_burst_kbps=4)
 

neutron/tests/unit/db/test_db_base_plugin_v2.py

@@ -2608,8 +2608,7 @@ class TestNetworksV2(NeutronDbPluginV2TestCase):
                                           network['network']['id'])
             req.environ['neutron.context'] = context.Context('', 'somebody')
             res = req.get_response(self.api)
-            # The API layer always returns 404 on updates in place of 403
-            self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int)
+            self.assertEqual(403, res.status_int)
 
     def test_update_network_set_shared(self):
         with self.network(shared=False) as network: