openstack/ironic
Code Issues Proposed changes

Revert "Revert "RBAC: Fix allocation check"" to use Unauthorized

Browse Source 
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
This commit is contained in:
Julia Kreger 2024-01-16 09:53:28 -08:00
parent 4b31633862
commit 4398c11a5f
3 changed files with 31 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
ironic/api/controllers/v1/allocation.py
View File

@ -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,

11
ironic/tests/unit/api/controllers/v1/test_allocation.py
View File

@ -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:

8
releasenotes/notes/fix-allocation-exception-on-list-c04e93fb9cace218.yaml Normal file
View File

@ -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.