diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 0dde80f1fe..f85a40761a 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1206,11 +1206,32 @@ class NodeStatesController(rest.RestController): api_utils.check_allow_management_verbs(target) + # Secondary RBAC policy checking... + # done first because RBAC policies must be asserted in order + # of visibility to can I make a change upfront, before diving + # into if the change is valid to make. + if not runbook: + if clean_steps: + api_utils.check_owner_policy( + 'node', + 'baremetal:node:set_provision_state:clean_steps', + rpc_node['owner'], rpc_node['lessee'], + conceal_node=False) + if service_steps: + api_utils.check_owner_policy( + 'node', + 'baremetal:node:set_provision_state:service_steps', + rpc_node['owner'], rpc_node['lessee'], + conceal_node=False) + if (target in (ir_states.ACTIVE, ir_states.REBUILD) and rpc_node.maintenance): raise exception.NodeInMaintenance(op=_('provisioning'), node=rpc_node.uuid) + # This code does upfront state sanity checking, in that we're past + # basic RBAC policy checking, and we're shifting gears to content + # validation. m = ir_states.machine.copy() m.initialize(rpc_node.provision_state) if not m.is_actionable_event(ir_states.VERBS.get(target, target)): @@ -1236,13 +1257,6 @@ class NodeStatesController(rest.RestController): clean_steps, service_steps, disable_ramdisk = self._handle_runbook( rpc_node, target, runbook, clean_steps, service_steps ) - else: - if clean_steps: - api_utils.check_policy( - 'baremetal:node:set_provision_state:clean_steps') - if service_steps: - api_utils.check_policy( - 'baremetal:node:set_provision_state:service_steps') if clean_steps and target != ir_states.VERBS['clean']: msg = (_('"clean_steps" is only valid when setting target ' diff --git a/ironic/tests/unit/api/test_rbac_project_scoped.yaml b/ironic/tests/unit/api/test_rbac_project_scoped.yaml index 9dd9dad1df..32782b1cf5 100644 --- a/ironic/tests/unit/api/test_rbac_project_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_project_scoped.yaml @@ -1369,6 +1369,80 @@ service_cannot_change_provision_state: body: *provision_body assert_status: 404 +# The following tests fail on lock checking, which is fine, +# as it demonstrates we're past API RBAC policy checking... +# And these checks center around secondary policies, so the +# what we send is more important. +owner_member_can_set_provision_state_clean: + path: '/v1/nodes/{owner_node_ident}/states/provision' + method: put + headers: *owner_member_headers + body: &provision_body_clean_steps + target: clean + clean_steps: + - interface: deploy + step: update_firmware + args: + foo: bar + priority: 99 + assert_status: 409 + +owner_reader_cannot_set_provision_state_clean: + path: '/v1/nodes/{owner_node_ident}/states/provision' + method: put + headers: *owner_reader_headers + body: *provision_body_clean_steps + assert_status: 403 + +lessee_admin_can_set_provision_state_clean: + path: '/v1/nodes/{lessee_node_ident}/states/provision' + method: put + headers: *lessee_admin_headers + body: *provision_body_clean_steps + assert_status: 409 + +lessee_member_cannot_set_provision_state_clean: + path: '/v1/nodes/{lessee_node_ident}/states/provision' + method: put + headers: *lessee_member_headers + body: *provision_body_clean_steps + assert_status: 403 + +owner_member_can_set_provision_state_service: + path: '/v1/nodes/{owner_node_ident}/states/provision' + method: put + headers: *owner_member_headers + body: &provision_body_service_steps + target: service + clean_steps: + - interface: deploy + step: update_firmware + args: + foo: bar + priority: 99 + assert_status: 409 + +owner_reader_cannot_set_provision_state_service: + path: '/v1/nodes/{owner_node_ident}/states/provision' + method: put + headers: *owner_reader_headers + body: *provision_body_service_steps + assert_status: 403 + +lessee_admin_can_set_provision_state_service: + path: '/v1/nodes/{lessee_node_ident}/states/provision' + method: put + headers: *lessee_admin_headers + body: *provision_body_service_steps + assert_status: 409 + +lessee_member_cannot_set_provision_state_service: + path: '/v1/nodes/{lessee_node_ident}/states/provision' + method: put + headers: *lessee_member_headers + body: *provision_body_service_steps + assert_status: 403 + # Raid configuration owner_admin_can_set_raid_config: diff --git a/releasenotes/notes/fix-set-provision-state-subpolicy-13ae3ef7497d20c1.yaml b/releasenotes/notes/fix-set-provision-state-subpolicy-13ae3ef7497d20c1.yaml new file mode 100644 index 0000000000..18cda7501a --- /dev/null +++ b/releasenotes/notes/fix-set-provision-state-subpolicy-13ae3ef7497d20c1.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + Fixes newly added policy rules, + ``baremetal:node:set_provision_state:clean_steps`` and + ``baremetal:node:set_provision_state:service_steps``which impacted + ``project scoped`` users utilizing the ``2024.2`` release of Ironic + where they were attempting to invoke ``service`` or ``clean`` + provision state commands. + This was due to a misunderstanding of the correct policy checker to + invoke, and additional testing has been added around these functions + to ensure they work as expected moving forward.