RBAC: Fix allocation check

The tl;dr, is when the allocation owner support was added,
it was done so to try and use the same exception
being raised for two distinct cases. An invalid request,
and a mismatch. The reality is, we should be raising them
separately because there are two different cases we need
to guard against.

This was discovered when changing Ironic's default RBAC
policy enforcement so that the legacy policy is no longer
enabled, which meant the default path on the owner logic was
thus triggered, resulting in the failure and need to fix it.

Closes-Bug: #2048698
Change-Id: I0feefc273a2d18e7812139f59df3f43aba7d7936
This commit is contained in:
Julia Kreger 2024-01-08 13:53:34 -08:00
parent c3074524da
commit c901b15f6c
3 changed files with 29 additions and 1 deletions

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.NotAcceptable()
if requestor is not None:
owner = requestor
return self._get_allocations_collection(node, resource_class, state,
owner, marker, limit,

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.NOT_ACCEPTABLE, 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:

View File

@ -0,0 +1,6 @@
---
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.