From c901b15f6c32b91cfceaf048aee07c9b7d711f52 Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Mon, 8 Jan 2024 13:53:34 -0800
Subject: [PATCH] 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
---
 ironic/api/controllers/v1/allocation.py             | 13 ++++++++++++-
 .../unit/api/controllers/v1/test_allocation.py      | 11 +++++++++++
 ...location-exception-on-list-c04e93fb9cace218.yaml |  6 ++++++
 3 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/fix-allocation-exception-on-list-c04e93fb9cace218.yaml

diff --git a/ironic/api/controllers/v1/allocation.py b/ironic/api/controllers/v1/allocation.py
index 7884df1fa1..0896cff868 100644
--- a/ironic/api/controllers/v1/allocation.py
+++ b/ironic/api/controllers/v1/allocation.py
@@ -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,
diff --git a/ironic/tests/unit/api/controllers/v1/test_allocation.py b/ironic/tests/unit/api/controllers/v1/test_allocation.py
index 0bc739bf2e..bbac4bd5e7 100644
--- a/ironic/tests/unit/api/controllers/v1/test_allocation.py
+++ b/ironic/tests/unit/api/controllers/v1/test_allocation.py
@@ -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:
diff --git a/releasenotes/notes/fix-allocation-exception-on-list-c04e93fb9cace218.yaml b/releasenotes/notes/fix-allocation-exception-on-list-c04e93fb9cace218.yaml
new file mode 100644
index 0000000000..1bef2ec67f
--- /dev/null
+++ b/releasenotes/notes/fix-allocation-exception-on-list-c04e93fb9cace218.yaml
@@ -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.