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
This commit is contained in:
parent
6da00730ab
commit
2ae14cc9ad
|
@ -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)
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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:
|
||||
|
|
Loading…
Reference in New Issue