From d146064cb52399deaa254f42b810792357793bda Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Wed, 18 Nov 2020 21:52:08 +0000 Subject: [PATCH] Implement system scoped RBAC for the allocation APIs This commit updates the policies for baremetal allocation policies to understand scope checking and account for a read-only role. This is part of a broader series of changes across OpenStack to provide a consistent RBAC experience and improve security. Change-Id: I1cb3a7e885710c19f20df63b83beaa787ffa3bc3 --- ironic/common/policy.py | 136 ++++++++++++++---- ironic/tests/unit/api/test_rbac_legacy.yaml | 21 +++ .../unit/api/test_rbac_system_scoped.yaml | 35 +---- ...coped-authentication-28e3651de250bea8.yaml | 2 +- 4 files changed, 135 insertions(+), 59 deletions(-) diff --git a/ironic/common/policy.py b/ironic/common/policy.py index 01e59b84c7..d8099fb34c 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -1176,44 +1176,120 @@ conductor_policies = [ ), ] + +deprecated_allocation_get = policy.DeprecatedRule( + name='baremetal:allocation:get', + check_str='rule:is_admin or rule:is_observer' +) +deprecated_allocation_list = policy.DeprecatedRule( + name='baremetal:allocation:list', + check_str='rule:baremetal:allocation:get' +) +deprecated_allocation_list_all = policy.DeprecatedRule( + name='baremetal:allocation:list_all', + check_str='rule:baremetal:allocation:get' +) +deprecated_allocation_create = policy.DeprecatedRule( + name='baremetal:allocation:create', + check_str='rule:is_admin' +) +deprecated_allocation_create_restricted = policy.DeprecatedRule( + name='baremetal:allocation:create_restricted', + check_str='rule:baremetal:allocation:create' +) +deprecated_allocation_delete = policy.DeprecatedRule( + name='baremetal:allocation:delete', + check_str='rule:is_admin' +) +deprecated_allocation_update = policy.DeprecatedRule( + name='baremetal:allocation:update', + check_str='rule:is_admin' +) +deprecated_allocation_reason = """ +The baremetal allocation API is now aware of system scope and default +roles. +""" + allocation_policies = [ policy.DocumentedRuleDefault( - 'baremetal:allocation:get', - 'rule:is_admin or rule:is_observer', - 'Retrieve Allocation records', - [{'path': '/allocations/{allocation_id}', 'method': 'GET'}, - {'path': '/nodes/{node_ident}/allocation', 'method': 'GET'}]), + name='baremetal:allocation:get', + check_str=SYSTEM_READER, + scope_types=['system'], + description='Retrieve Allocation records', + operations=[ + {'path': '/allocations/{allocation_id}', 'method': 'GET'}, + {'path': '/nodes/{node_ident}/allocation', 'method': 'GET'} + ], + deprecated_rule=deprecated_allocation_get, + deprecated_reason=deprecated_allocation_reason, + deprecated_since=versionutils.deprecated.WALLABY + ), policy.DocumentedRuleDefault( - 'baremetal:allocation:list', - 'rule:baremetal:allocation:get', - 'Retrieve multiple Allocation records, filtered by owner', - [{'path': '/allocations', 'method': 'GET'}]), + name='baremetal:allocation:list', + check_str=SYSTEM_READER, + scope_types=['system'], + description='Retrieve multiple Allocation records, filtered by owner', + operations=[{'path': '/allocations', 'method': 'GET'}], + deprecated_rule=deprecated_allocation_list, + deprecated_reason=deprecated_allocation_reason, + deprecated_since=versionutils.deprecated.WALLABY + ), policy.DocumentedRuleDefault( - 'baremetal:allocation:list_all', - 'rule:baremetal:allocation:get', - 'Retrieve multiple Allocation records', - [{'path': '/allocations', 'method': 'GET'}]), + name='baremetal:allocation:list_all', + check_str=SYSTEM_READER, + scope_types=['system'], + description='Retrieve multiple Allocation records', + operations=[{'path': '/allocations', 'method': 'GET'}], + deprecated_rule=deprecated_allocation_list_all, + deprecated_reason=deprecated_allocation_reason, + deprecated_since=versionutils.deprecated.WALLABY + ), policy.DocumentedRuleDefault( - 'baremetal:allocation:create', - 'rule:is_admin', - 'Create Allocation records', - [{'path': '/allocations', 'method': 'POST'}]), + name='baremetal:allocation:create', + check_str=SYSTEM_MEMBER, + scope_types=['system'], + description='Create Allocation records', + operations=[{'path': '/allocations', 'method': 'POST'}], + deprecated_rule=deprecated_allocation_create, + deprecated_reason=deprecated_allocation_reason, + deprecated_since=versionutils.deprecated.WALLABY + ), policy.DocumentedRuleDefault( - 'baremetal:allocation:create_restricted', - 'rule:baremetal:allocation:create', - 'Create Allocation records that are restricted to an owner', - [{'path': '/allocations', 'method': 'POST'}]), + name='baremetal:allocation:create_restricted', + check_str=SYSTEM_MEMBER, + scope_types=['system'], + description=( + 'Create Allocation records that are restricted to an owner' + ), + operations=[{'path': '/allocations', 'method': 'POST'}], + deprecated_rule=deprecated_allocation_create_restricted, + deprecated_reason=deprecated_allocation_reason, + deprecated_since=versionutils.deprecated.WALLABY + ), policy.DocumentedRuleDefault( - 'baremetal:allocation:delete', - 'rule:is_admin', - 'Delete Allocation records', - [{'path': '/allocations/{allocation_id}', 'method': 'DELETE'}, - {'path': '/nodes/{node_ident}/allocation', 'method': 'DELETE'}]), + name='baremetal:allocation:delete', + check_str=SYSTEM_MEMBER, + scope_types=['system'], + description='Delete Allocation records', + operations=[ + {'path': '/allocations/{allocation_id}', 'method': 'DELETE'}, + {'path': '/nodes/{node_ident}/allocation', 'method': 'DELETE'}], + deprecated_rule=deprecated_allocation_delete, + deprecated_reason=deprecated_allocation_reason, + deprecated_since=versionutils.deprecated.WALLABY + ), policy.DocumentedRuleDefault( - 'baremetal:allocation:update', - 'rule:is_admin', - 'Change name and extra fields of an allocation', - [{'path': '/allocations/{allocation_id}', 'method': 'PATCH'}]), + name='baremetal:allocation:update', + check_str=SYSTEM_MEMBER, + scope_types=['system'], + description='Change name and extra fields of an allocation', + operations=[ + {'path': '/allocations/{allocation_id}', 'method': 'PATCH'}, + ], + deprecated_rule=deprecated_allocation_update, + deprecated_reason=deprecated_allocation_reason, + deprecated_since=versionutils.deprecated.WALLABY + ), ] event_policies = [ diff --git a/ironic/tests/unit/api/test_rbac_legacy.yaml b/ironic/tests/unit/api/test_rbac_legacy.yaml index dfdf276b6d..2ec4942105 100644 --- a/ironic/tests/unit/api/test_rbac_legacy.yaml +++ b/ironic/tests/unit/api/test_rbac_legacy.yaml @@ -1888,6 +1888,7 @@ allocations_post_admin: body: &allocation_body resource_class: CUSTOM_TEST assert_status: 503 + deprecated: true allocations_post_member: path: '/v1/allocations' @@ -1895,6 +1896,7 @@ allocations_post_member: headers: *member_headers body: *allocation_body assert_status: 403 + deprecated: true allocations_post_observer: path: '/v1/allocations' @@ -1902,42 +1904,49 @@ allocations_post_observer: headers: *observer_headers body: *allocation_body assert_status: 403 + deprecated: true allocations_get_admin: path: '/v1/allocations' method: get headers: *admin_headers assert_status: 200 + deprecated: true allocations_get_member: path: '/v1/allocations' method: get headers: *member_headers assert_status: 403 + deprecated: true allocations_get_observer: path: '/v1/allocations' method: get headers: *observer_headers assert_status: 200 + deprecated: true allocations_allocation_id_get_admin: path: '/v1/allocations/{allocation_ident}' method: get headers: *admin_headers assert_status: 200 + deprecated: true allocations_allocation_id_get_member: path: '/v1/allocations/{allocation_ident}' method: get headers: *member_headers assert_status: 403 + deprecated: true allocations_allocation_id_get_observer: path: '/v1/allocations/{allocation_ident}' method: get headers: *observer_headers assert_status: 200 + deprecated: true allocations_allocation_id_patch_admin: path: '/v1/allocations/{allocation_ident}' @@ -1948,6 +1957,7 @@ allocations_allocation_id_patch_admin: path: /extra value: {'test': 'testing'} assert_status: 200 + deprecated: true allocations_allocation_id_patch_member: path: '/v1/allocations/{allocation_ident}' @@ -1955,6 +1965,7 @@ allocations_allocation_id_patch_member: headers: *member_headers body: *allocation_patch assert_status: 403 + deprecated: true allocations_allocation_id_patch_observer: path: '/v1/allocations/{allocation_ident}' @@ -1962,24 +1973,28 @@ allocations_allocation_id_patch_observer: headers: *observer_headers body: *allocation_patch assert_status: 403 + deprecated: true allocations_allocation_id_delete_admin: path: '/v1/allocations/{allocation_ident}' method: delete headers: *admin_headers assert_status: 503 + deprecated: true allocations_allocation_id_delete_member: path: '/v1/allocations/{allocation_ident}' method: delete headers: *member_headers assert_status: 403 + deprecated: true allocations_allocation_id_delete_observer: path: '/v1/allocations/{allocation_ident}' method: delete headers: *observer_headers assert_status: 403 + deprecated: true # Allocations ( Node level) - https://docs.openstack.org/api-ref/baremetal/#node-allocation-allocations-nodes nodes_allocation_get_admin: @@ -1987,36 +2002,42 @@ nodes_allocation_get_admin: method: get headers: *admin_headers assert_status: 200 + deprecated: true nodes_allocation_get_member: path: '/v1/nodes/{allocated_node_ident}/allocation' method: get headers: *member_headers assert_status: 403 + deprecated: true nodes_allocation_get_observer: path: '/v1/nodes/{allocated_node_ident}/allocation' method: get headers: *observer_headers assert_status: 200 + deprecated: true nodes_allocation_delete_admin: path: '/v1/nodes/{allocated_node_ident}/allocation' method: delete headers: *admin_headers assert_status: 503 + deprecated: true nodes_allocation_delete_member: path: '/v1/nodes/{allocated_node_ident}/allocation' method: delete headers: *member_headers assert_status: 403 + deprecated: true nodes_allocation_delete_observer: path: '/v1/nodes/{allocated_node_ident}/allocation' method: delete headers: *observer_headers assert_status: 403 + deprecated: true # Deploy Templates - https://docs.openstack.org/api-ref/baremetal/#deploy-templates-deploy-templates diff --git a/ironic/tests/unit/api/test_rbac_system_scoped.yaml b/ironic/tests/unit/api/test_rbac_system_scoped.yaml index 594df2916a..e03902ec17 100644 --- a/ironic/tests/unit/api/test_rbac_system_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_system_scoped.yaml @@ -1644,15 +1644,13 @@ allocations_post_admin: body: &allocation_body resource_class: CUSTOM_TEST assert_status: 503 - skip_reason: not updated for scope testing allocations_post_member: path: '/v1/allocations' method: post headers: *scoped_member_headers body: *allocation_body - assert_status: 403 - skip_reason: not updated for scope testing + assert_status: 503 allocations_post_observer: path: '/v1/allocations' @@ -1660,49 +1658,42 @@ allocations_post_observer: headers: *observer_headers body: *allocation_body assert_status: 403 - skip_reason: not updated for scope testing allocations_get_admin: path: '/v1/allocations' method: get headers: *admin_headers assert_status: 200 - skip_reason: not updated for scope testing allocations_get_member: path: '/v1/allocations' method: get headers: *scoped_member_headers - assert_status: 403 - skip_reason: not updated for scope testing + assert_status: 200 allocations_get_observer: path: '/v1/allocations' method: get headers: *observer_headers assert_status: 200 - skip_reason: not updated for scope testing allocations_allocation_id_get_admin: path: '/v1/allocations/{allocation_ident}' method: get headers: *admin_headers assert_status: 200 - skip_reason: not updated for scope testing allocations_allocation_id_get_member: path: '/v1/allocations/{allocation_ident}' method: get headers: *scoped_member_headers - assert_status: 403 - skip_reason: not updated for scope testing + assert_status: 200 allocations_allocation_id_get_observer: path: '/v1/allocations/{allocation_ident}' method: get headers: *observer_headers assert_status: 200 - skip_reason: not updated for scope testing allocations_allocation_id_patch_admin: path: '/v1/allocations/{allocation_ident}' @@ -1713,15 +1704,13 @@ allocations_allocation_id_patch_admin: path: /extra value: {'test': 'testing'} assert_status: 200 - skip_reason: not updated for scope testing allocations_allocation_id_patch_member: path: '/v1/allocations/{allocation_ident}' method: patch headers: *scoped_member_headers body: *allocation_patch - assert_status: 403 - skip_reason: not updated for scope testing + assert_status: 200 allocations_allocation_id_patch_observer: path: '/v1/allocations/{allocation_ident}' @@ -1729,7 +1718,6 @@ allocations_allocation_id_patch_observer: headers: *observer_headers body: *allocation_patch assert_status: 403 - skip_reason: not updated for scope testing # NOTE(TheJulia): Currently returns an HTTP 503 # Resource temporarily unavailable, please retry. @@ -1739,21 +1727,18 @@ allocations_allocation_id_delete_admin: method: delete headers: *admin_headers assert_status: 503 - skip_reason: not updated for scope testing allocations_allocation_id_delete_member: path: '/v1/allocations/{allocation_ident}' method: delete headers: *scoped_member_headers - assert_status: 403 - skip_reason: not updated for scope testing + assert_status: 503 allocations_allocation_id_delete_observer: path: '/v1/allocations/{allocation_ident}' method: delete headers: *observer_headers assert_status: 403 - skip_reason: not updated for scope testing # Allocations ( Node level) - https://docs.openstack.org/api-ref/baremetal/#node-allocation-allocations-nodes nodes_allocation_get_admin: @@ -1761,21 +1746,18 @@ nodes_allocation_get_admin: method: get headers: *admin_headers assert_status: 200 - skip_reason: not updated for scope testing nodes_allocation_get_member: path: '/v1/nodes/{allocated_node_ident}/allocation' method: get headers: *scoped_member_headers - assert_status: 403 - skip_reason: not updated for scope testing + assert_status: 200 nodes_allocation_get_observer: path: '/v1/nodes/{allocated_node_ident}/allocation' method: get headers: *observer_headers assert_status: 200 - skip_reason: not updated for scope testing # NOTE(TheJulia): Currently returns an HTTP 503 # Resource temporarily unavailable, please retry. @@ -1785,21 +1767,18 @@ nodes_allocation_delete_admin: method: delete headers: *admin_headers assert_status: 503 - skip_reason: not updated for scope testing nodes_allocation_delete_member: path: '/v1/nodes/{allocated_node_ident}/allocation' method: delete headers: *scoped_member_headers - assert_status: 403 - skip_reason: not updated for scope testing + assert_status: 503 nodes_allocation_delete_observer: path: '/v1/nodes/{allocated_node_ident}/allocation' method: delete headers: *observer_headers assert_status: 403 - skip_reason: not updated for scope testing # Deploy Templates - https://docs.openstack.org/api-ref/baremetal/#deploy-templates-deploy-templates diff --git a/releasenotes/notes/system-scoped-authentication-28e3651de250bea8.yaml b/releasenotes/notes/system-scoped-authentication-28e3651de250bea8.yaml index 06b257007d..38afdec50a 100644 --- a/releasenotes/notes/system-scoped-authentication-28e3651de250bea8.yaml +++ b/releasenotes/notes/system-scoped-authentication-28e3651de250bea8.yaml @@ -4,7 +4,7 @@ features: The Baremetal API, provided by the ironic-api process, now supports use of ``system`` scoped ``keystone`` authentication for the following endpoints: nodes, ports, portgroups, chassis, drivers, driver vendor passthru, - volume targets, volume connectors, conductors + volume targets, volume connectors, conductors, allocations upgrade: - | Deprecated policy rules are not expressed via a default policy file