From c6a400504913412ee301822f31684ace20ef38cb Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Mon, 4 Mar 2024 08:47:43 -0800
Subject: [PATCH] Fix artifical rbac policy constraint that resulted in 500s

Some of the endpoints are *highly* restricted in ironic's newer
more stringently enforced RBAC world. Some of these endpoints would
emit 500s by default, when realistically it was the policy definition
saying "only system scope could be used" for the endpoint, but the
reality is that 403 is what should have been returned for a client to
properly understand what is going on.

Change-Id: If5e13764dad886ba3ee1a848f3ff9f3279f4d7f6
---
 ironic/common/policy.py                        | 10 +++++-----
 .../unit/api/test_rbac_project_scoped.yaml     | 18 +++++++++---------
 ...riginating-500-errors-4b54977631a015d9.yaml |  8 ++++++++
 3 files changed, 22 insertions(+), 14 deletions(-)
 create mode 100644 releasenotes/notes/address-rbac-originating-500-errors-4b54977631a015d9.yaml

diff --git a/ironic/common/policy.py b/ironic/common/policy.py
index b8af77e9f6..647bd72609 100644
--- a/ironic/common/policy.py
+++ b/ironic/common/policy.py
@@ -1638,7 +1638,7 @@ conductor_policies = [
     policy.DocumentedRuleDefault(
         name='baremetal:conductor:get',
         check_str=SYSTEM_READER,
-        scope_types=['system'],
+        scope_types=['system', 'project'],
         description='Retrieve Conductor records',
         operations=[
             {'path': '/conductors', 'method': 'GET'},
@@ -1839,7 +1839,7 @@ deploy_template_policies = [
     policy.DocumentedRuleDefault(
         name='baremetal:deploy_template:get',
         check_str=SYSTEM_READER,
-        scope_types=['system'],
+        scope_types=['system', 'project'],
         description='Retrieve Deploy Template records',
         operations=[
             {'path': '/deploy_templates', 'method': 'GET'},
@@ -1851,7 +1851,7 @@ deploy_template_policies = [
     policy.DocumentedRuleDefault(
         name='baremetal:deploy_template:create',
         check_str=SYSTEM_ADMIN,
-        scope_types=['system'],
+        scope_types=['system', 'project'],
         description='Create Deploy Template records',
         operations=[{'path': '/deploy_templates', 'method': 'POST'}],
         deprecated_rule=deprecated_deploy_template_create
@@ -1859,7 +1859,7 @@ deploy_template_policies = [
     policy.DocumentedRuleDefault(
         name='baremetal:deploy_template:delete',
         check_str=SYSTEM_ADMIN,
-        scope_types=['system'],
+        scope_types=['system', 'project'],
         description='Delete Deploy Template records',
         operations=[
             {'path': '/deploy_templates/{deploy_template_ident}',
@@ -1870,7 +1870,7 @@ deploy_template_policies = [
     policy.DocumentedRuleDefault(
         name='baremetal:deploy_template:update',
         check_str=SYSTEM_ADMIN,
-        scope_types=['system'],
+        scope_types=['system', 'project'],
         description='Update Deploy Template records',
         operations=[
             {'path': '/deploy_templates/{deploy_template_ident}',
diff --git a/ironic/tests/unit/api/test_rbac_project_scoped.yaml b/ironic/tests/unit/api/test_rbac_project_scoped.yaml
index e52fe6a19c..37937c8703 100644
--- a/ironic/tests/unit/api/test_rbac_project_scoped.yaml
+++ b/ironic/tests/unit/api/test_rbac_project_scoped.yaml
@@ -3383,19 +3383,19 @@ owner_reader_cannot_get_conductors:
   path: '/v1/conductors'
   method: get
   headers: *owner_reader_headers
-  assert_status: 500
+  assert_status: 403
 
 lessee_reader_cannot_get_conductors:
   path: '/v1/conductors'
   method: get
   headers: *lessee_reader_headers
-  assert_status: 500
+  assert_status: 403
 
 third_party_admin_cannot_get_conductors:
   path: '/v1/conductors'
   method: get
   headers: *third_party_admin_headers
-  assert_status: 500
+  assert_status: 403
 
 # Allocations - https://docs.openstack.org/api-ref/baremetal/#allocations-allocations
 
@@ -3631,19 +3631,19 @@ owner_reader_cannot_get_deploy_templates:
   path: '/v1/deploy_templates'
   method: get
   headers: *owner_reader_headers
-  assert_status: 500
+  assert_status: 403
 
 lessee_reader_cannot_get_deploy_templates:
   path: '/v1/deploy_templates'
   method: get
   headers: *lessee_reader_headers
-  assert_status: 500
+  assert_status: 403
 
 third_party_admin_cannot_get_deploy_templates:
   path: '/v1/deploy_templates'
   method: get
   headers: *third_party_admin_headers
-  assert_status: 500
+  assert_status: 403
 
 third_party_admin_cannot_post_deploy_template:
   path: '/v1/deploy_templates'
@@ -3656,20 +3656,20 @@ third_party_admin_cannot_post_deploy_template:
         args: {}
         priority: 0
   headers: *third_party_admin_headers
-  assert_status: 500
+  assert_status: 403
 
 service_cannot_get_deploy_templates:
   path: '/v1/deploy_templates'
   method: get
   headers: *service_headers
-  assert_status: 500
+  assert_status: 403
 
 service_cannot_post_deploy_template:
   path: '/v1/deploy_templates'
   method: post
   body: *deploy_template
   headers: *service_headers
-  assert_status: 500
+  assert_status: 403
 
 # Chassis endpoints - https://docs.openstack.org/api-ref/baremetal/#chassis-chassis
 
diff --git a/releasenotes/notes/address-rbac-originating-500-errors-4b54977631a015d9.yaml b/releasenotes/notes/address-rbac-originating-500-errors-4b54977631a015d9.yaml
new file mode 100644
index 0000000000..e28b968635
--- /dev/null
+++ b/releasenotes/notes/address-rbac-originating-500-errors-4b54977631a015d9.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Some of Ironic's API endpoints, when the new RBAC policy is being enforced,
+    were previously emitting *500* error codes when insufficent access rights were
+    being used, specifically because the policy required ``system`` scope. This
+    has been corrected, and the endpoints should now properly signal a *403* error
+    code if insufficient access rights are present for an authenticated requestor.