Revert "Revert "RBAC: Fix allocation check"" to use Unauthorized
In the backports to fix the policy of the original change, Dmitry
noted that it was actually wrong, because we should have instead
raised NotAuthorized. Dmitry was absolutely correct, because in hind
sight I made the change trying to keep exactly the same behavior,
but the reality is this is a case where we should be explicit,
and tell the user they have done something forbidden.
This revert of the revert fixes that change.
Original Change: https://review.opendev.org/c/openstack/ironic/+/905038
Dmitry's Review Feedback: https://review.opendev.org/c/openstack/ironic/+/905088
Change-Id: I5727df00b8c4ae9495ed14b5cea1c0734b5f688d
(cherry picked from commit 4398c11a5f
)
This commit is contained in:
parent
7bfc9e3f80
commit
ab5a5f6a19
@ -271,11 +271,22 @@ class AllocationsController(pecan.rest.RestController):
|
||||
:fields: fields
|
||||
:owner: r_owner
|
||||
"""
|
||||
owner = api_utils.check_list_policy('allocation', owner)
|
||||
requestor = api_utils.check_list_policy('allocation', owner)
|
||||
|
||||
self._check_allowed_allocation_fields(fields)
|
||||
if owner is not None and not api_utils.allow_allocation_owner():
|
||||
# Requestor has asked for an owner field/column match, but
|
||||
# their client version does not support it.
|
||||
raise exception.NotAcceptable()
|
||||
if (owner is not None
|
||||
and requestor is not None
|
||||
and owner != requestor):
|
||||
# The requestor is asking about other owner's records.
|
||||
# Naughty!
|
||||
raise exception.NotAuthorized()
|
||||
|
||||
if requestor is not None:
|
||||
owner = requestor
|
||||
|
||||
return self._get_allocations_collection(node, resource_class, state,
|
||||
owner, marker, limit,
|
||||
|
@ -28,6 +28,7 @@ from oslo_utils import uuidutils
|
||||
from ironic.api.controllers import base as api_base
|
||||
from ironic.api.controllers import v1 as api_v1
|
||||
from ironic.api.controllers.v1 import notification_utils
|
||||
from ironic.api.controllers.v1 import utils as v1_api_utils
|
||||
from ironic.common import exception
|
||||
from ironic.common import policy
|
||||
from ironic.conductor import rpcapi
|
||||
@ -420,6 +421,16 @@ class TestListAllocations(test_api_base.BaseApiTest):
|
||||
self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code)
|
||||
self.assertTrue(response.json['error_message'])
|
||||
|
||||
@mock.patch.object(v1_api_utils, 'check_list_policy', autospec=True)
|
||||
def test_get_all_by_owner_not_allowed_mismatch(self, mock_check):
|
||||
mock_check.return_value = '54321'
|
||||
response = self.get_json("/allocations?owner=12345",
|
||||
headers={api_base.Version.string: '1.60'},
|
||||
expect_errors=True)
|
||||
self.assertEqual('application/json', response.content_type)
|
||||
self.assertEqual(http_client.FORBIDDEN, response.status_code)
|
||||
self.assertTrue(response.json['error_message'])
|
||||
|
||||
def test_get_all_by_node_name(self):
|
||||
for i in range(5):
|
||||
if i < 3:
|
||||
|
@ -0,0 +1,8 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
Fixes an issue when listing allocations as a project scoped user when
|
||||
the legacy RBAC policies have been disabled which forced an HTTP 406
|
||||
error being erroneously raised. Users attempting to list allocations
|
||||
with a specific owner, different from their own, will now receive
|
||||
an HTTP 403 error.
|
Loading…
Reference in New Issue
Block a user