Implement system scoped RBAC for port groups

This commit updates the policies for baremetal port groups 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. Co-Authored-By: Julia Kreger <juliaashleykreger@gmail.com> Change-Id: Ida3b341285e3f889311a44f615e0665a0736f6af
2020-11-18 20:56:50 +00:00 · 2020-11-18 20:56:50 +00:00 · 4fe64c9702
parent 34edc6444e
commit 4fe64c9702
5 changed files with 115 additions and 70 deletions
--- a/ironic/common/policy.py
+++ b/ironic/common/policy.py
@ -757,31 +757,77 @@ port_policies = [
    ),
 ]

+deprecated_portgroup_get = policy.DeprecatedRule(
+    name='baremetal:portgroup:get',
+    check_str='rule:is_admin or rule:is_observer'
+)
+deprecated_portgroup_create = policy.DeprecatedRule(
+    name='baremetal:portgroup:create',
+    check_str='rule:is_admin'
+)
+deprecated_portgroup_delete = policy.DeprecatedRule(
+    name='baremetal:portgroup:delete',
+    check_str='rule:is_admin'
+)
+deprecated_portgroup_update = policy.DeprecatedRule(
+    name='baremetal:portgroup:update',
+    check_str='rule:is_admin'
+)
+deprecated_portgroup_reason = """
+The baremetal port groups API is now aware of system scope and default roles.
+"""
+
 portgroup_policies = [
    policy.DocumentedRuleDefault(
-        'baremetal:portgroup:get',
-        'rule:is_admin or rule:is_observer',
-        'Retrieve Portgroup records',
-        [{'path': '/portgroups', 'method': 'GET'},
-         {'path': '/portgroups/detail', 'method': 'GET'},
-         {'path': '/portgroups/{portgroup_ident}', 'method': 'GET'},
-         {'path': '/nodes/{node_ident}/portgroups', 'method': 'GET'},
-         {'path': '/nodes/{node_ident}/portgroups/detail', 'method': 'GET'}]),
+        name='baremetal:portgroup:get',
+        check_str=SYSTEM_READER,
+        scope_types=['system'],
+        description='Retrieve Portgroup records',
+        operations=[
+            {'path': '/portgroups', 'method': 'GET'},
+            {'path': '/portgroups/detail', 'method': 'GET'},
+            {'path': '/portgroups/{portgroup_ident}', 'method': 'GET'},
+            {'path': '/nodes/{node_ident}/portgroups', 'method': 'GET'},
+            {'path': '/nodes/{node_ident}/portgroups/detail', 'method': 'GET'},
+        ],
+        deprecated_rule=deprecated_portgroup_get,
+        deprecated_reason=deprecated_portgroup_reason,
+        deprecated_since=versionutils.deprecated.WALLABY
+    ),
    policy.DocumentedRuleDefault(
-        'baremetal:portgroup:create',
-        'rule:is_admin',
-        'Create Portgroup records',
-        [{'path': '/portgroups', 'method': 'POST'}]),
+        name='baremetal:portgroup:create',
+        check_str=SYSTEM_ADMIN,
+        scope_types=['system'],
+        description='Create Portgroup records',
+        operations=[{'path': '/portgroups', 'method': 'POST'}],
+        deprecated_rule=deprecated_portgroup_create,
+        deprecated_reason=deprecated_portgroup_reason,
+        deprecated_since=versionutils.deprecated.WALLABY
+    ),
    policy.DocumentedRuleDefault(
-        'baremetal:portgroup:delete',
-        'rule:is_admin',
-        'Delete Portgroup records',
-        [{'path': '/portgroups/{portgroup_ident}', 'method': 'DELETE'}]),
+        name='baremetal:portgroup:delete',
+        check_str=SYSTEM_ADMIN,
+        scope_types=['system'],
+        description='Delete Portgroup records',
+        operations=[
+            {'path': '/portgroups/{portgroup_ident}', 'method': 'DELETE'}
+        ],
+        deprecated_rule=deprecated_portgroup_delete,
+        deprecated_reason=deprecated_portgroup_reason,
+        deprecated_since=versionutils.deprecated.WALLABY
+    ),
    policy.DocumentedRuleDefault(
-        'baremetal:portgroup:update',
-        'rule:is_admin',
-        'Update Portgroup records',
-        [{'path': '/portgroups/{portgroup_ident}', 'method': 'PATCH'}]),
+        name='baremetal:portgroup:update',
+        check_str=SYSTEM_MEMBER,
+        scope_types=['system'],
+        description='Update Portgroup records',
+        operations=[
+            {'path': '/portgroups/{portgroup_ident}', 'method': 'PATCH'}
+        ],
+        deprecated_rule=deprecated_portgroup_update,
+        deprecated_reason=deprecated_portgroup_reason,
+        deprecated_since=versionutils.deprecated.WALLABY
+    ),
 ]

 chassis_policies = [
--- a/ironic/tests/unit/api/test_acl.py
+++ b/ironic/tests/unit/api/test_acl.py
@ -237,6 +237,11 @@ class TestRBACModelBeforeScopesBase(TestACLBase):
        db_utils.create_test_node_trait(
            node_id=fake_db_node['id'])

+        # dedicated node for portgroup addition test to avoid
+        # false positives with test runners.
+        db_utils.create_test_node(
+            uuid='18a552fb-dcd2-43bf-9302-e4c93287be11')
+
        self.format_data.update({
            'node_ident': fake_db_node['uuid'],
            'allocated_node_ident': fake_db_node_alloced['uuid'],
--- a/ironic/tests/unit/api/test_rbac_legacy.yaml
+++ b/ironic/tests/unit/api/test_rbac_legacy.yaml
@ -107,7 +107,7 @@ nodes_get_admin:
  method: get
  headers: *admin_headers
  assert_list_length:
-    nodes: 2
+    nodes: 3
  assert_status: 200
  deprecated: true

@ -116,7 +116,7 @@ nodes_get_other_admin:
  method: get
  headers: *other_admin_headers
  assert_list_length:
-    nodes: 2
+    nodes: 3
  assert_status: 200
  deprecated: true

@ -837,18 +837,21 @@ portgroups_get_admin:
  method: get
  headers: *admin_headers
  assert_status: 200
+  deprecated: true

 portgroups_get_member:
  path: '/v1/portgroups'
  method: get
  headers: *member_headers
  assert_status: 403
+  deprecated: true

 portgroups_get_observer:
  path: '/v1/portgroups'
  method: get
  headers: *observer_headers
  assert_status: 200
+  deprecated: true

 # Returns 400 as the node uuid cannot be found, but
 # it is past the access controls.
@ -857,8 +860,9 @@ portgroups_post_admin:
  method: post
  headers: *admin_headers
  body: &portgroup_body
-    node_uuid: 68a552fb-dcd2-43bf-9302-e4c93287be16
-  assert_status: 400
+    node_uuid: 18a552fb-dcd2-43bf-9302-e4c93287be11
+  assert_status: 201
+  deprecated: true

 portgroups_post_member:
  path: '/v1/portgroups'
@ -866,6 +870,7 @@ portgroups_post_member:
  headers: *member_headers
  body: *portgroup_body
  assert_status: 403
+  deprecated: true

 portgroups_post_observer:
  path: '/v1/portgroups'
@ -873,42 +878,49 @@ portgroups_post_observer:
  headers: *observer_headers
  body: *portgroup_body
  assert_status: 403
+  deprecated: true

 portgroups_detail_get_admin:
  path: '/v1/portgroups/detail'
  method: get
  headers: *admin_headers
  assert_status: 200
+  deprecated: true

 portgroups_detail_get_member:
  path: '/v1/portgroups/detail'
  method: get
  headers: *member_headers
  assert_status: 403
+  deprecated: true

 portgroups_detail_get_observer:
  path: '/v1/portgroups/detail'
  method: get
  headers: *observer_headers
  assert_status: 200
+  deprecated: true

 portgroups_portgroup_ident_get_admin:
  path: '/v1/portgroups/{portgroup_ident}'
  method: get
  headers: *admin_headers
  assert_status: 200
+  deprecated: true

 portgroups_portgroup_ident_get_member:
  path: '/v1/portgroups/{portgroup_ident}'
  method: get
  headers: *member_headers
  assert_status: 403
+  deprecated: true

 portgroups_portgroup_ident_get_observer:
  path: '/v1/portgroups/{portgroup_ident}'
  method: get
  headers: *observer_headers
  assert_status: 200
+  deprecated: true

 portgroups_portgroup_ident_patch_admin:
  path: '/v1/portgroups/{portgroup_ident}'
@ -919,6 +931,7 @@ portgroups_portgroup_ident_patch_admin:
      path: /extra
      value: {'test': 'testing'}
  assert_status: 503
+  deprecated: true

 portgroups_portgroup_ident_patch_member:
  path: '/v1/portgroups/{portgroup_ident}'
@ -926,6 +939,7 @@ portgroups_portgroup_ident_patch_member:
  headers: *member_headers
  body: *portgroup_patch_body
  assert_status: 403
+  deprecated: true

 portgroups_portgroup_ident_patch_observer:
  path: '/v1/portgroups/{portgroup_ident}'
@ -933,24 +947,28 @@ portgroups_portgroup_ident_patch_observer:
  headers: *observer_headers
  body: *portgroup_patch_body
  assert_status: 403
+  deprecated: true

 portgroups_portgroup_ident_delete_admin:
  path: '/v1/portgroups/{portgroup_ident}'
  method: delete
  headers: *admin_headers
  assert_status: 503
+  deprecated: true

 portgroups_portgroup_ident_delete_member:
  path: '/v1/portgroups/{portgroup_ident}'
  method: delete
  headers: *member_headers
  assert_status: 403
+  deprecated: true

 portgroups_portgroup_ident_delete_observer:
  path: '/v1/portgroups/{portgroup_ident}'
  method: delete
  headers: *observer_headers
  assert_status: 403
+  deprecated: true

 # Portgroups by node - https://docs.openstack.org/api-ref/baremetal/#listing-portgroups-by-node-nodes-portgroups

@ -959,36 +977,42 @@ nodes_portgroups_get_admin:
  method: get
  headers: *admin_headers
  assert_status: 200
+  deprecated: true

 nodes_portgroups_get_member:
  path: '/v1/nodes/{node_ident}/portgroups'
  method: get
  headers: *member_headers
  assert_status: 403
+  deprecated: true

 nodes_portgroups_get_observer:
  path: '/v1/nodes/{node_ident}/portgroups'
  method: get
  headers: *observer_headers
  assert_status: 200
+  deprecated: true

 nodes_portgroups_detail_get_admin:
  path: '/v1/nodes/{node_ident}/portgroups/detail'
  method: get
  headers: *admin_headers
  assert_status: 200
+  deprecated: true

 nodes_portgroups_detail_get_member:
  path: '/v1/nodes/{node_ident}/portgroups/detail'
  method: get
  headers: *member_headers
  assert_status: 403
+  deprecated: true

 nodes_portgroups_detail_get_observer:
  path: '/v1/nodes/{node_ident}/portgroups/detail'
  method: get
  headers: *observer_headers
  assert_status: 200
+  deprecated: true

 # Ports - https://docs.openstack.org/api-ref/baremetal/#ports-ports

--- a/ironic/tests/unit/api/test_rbac_system_scoped.yaml
+++ b/ironic/tests/unit/api/test_rbac_system_scoped.yaml
@ -90,7 +90,7 @@ nodes_get_admin:
  method: get
  headers: *admin_headers
  assert_list_length:
-    nodes: 2
+    nodes: 3
  assert_status: 200

 nodes_get_other_admin:
@ -99,7 +99,7 @@ nodes_get_other_admin:
  skip_reason: 'Not implemented yet'
  headers: *other_admin_headers
  assert_list_length:
-    nodes: 2
+    nodes: 3
  assert_status: 200

 nodes_detail_get_admin:
@ -747,38 +747,33 @@ portgroups_get_admin:
  method: get
  headers: *admin_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 portgroups_get_member:
  path: '/v1/portgroups'
  method: get
  headers: *scoped_member_headers
-  assert_status: 403
-  skip_reason: not updated for scope testing
+  assert_status: 200

 portgroups_get_observer:
  path: '/v1/portgroups'
  method: get
  headers: *observer_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 portgroups_post_admin:
  path: '/v1/portgroups'
  method: post
  headers: *admin_headers
  body: &portgroup_body
-    node_uuid: 68a552fb-dcd2-43bf-9302-e4c93287be16
-  assert_status: 204
-  skip_reason: not updated for scope testing
+    node_uuid: 18a552fb-dcd2-43bf-9302-e4c93287be11
+  assert_status: 201

-portgroups_post_admin:
+portgroups_post_member:
  path: '/v1/portgroups'
  method: post
  headers: *scoped_member_headers
  body: *portgroup_body
  assert_status: 403
-  skip_reason: not updated for scope testing

 portgroups_post_observer:
  path: '/v1/portgroups'
@ -786,49 +781,42 @@ portgroups_post_observer:
  headers: *observer_headers
  body: *portgroup_body
  assert_status: 403
-  skip_reason: not updated for scope testing

 portgroups_detail_get_admin:
  path: '/v1/portgroups/detail'
  method: get
  headers: *admin_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

-portgroups_detail_get_admin:
+portgroups_detail_get_member:
  path: '/v1/portgroups/detail'
  method: get
  headers: *scoped_member_headers
-  assert_status: 403
-  skip_reason: not updated for scope testing
+  assert_status: 200

 portgroups_detail_get_observer:
  path: '/v1/portgroups/detail'
  method: get
  headers: *observer_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 portgroups_portgroup_ident_get_admin:
  path: '/v1/portgroups/{portgroup_ident}'
  method: get
  headers: *admin_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 portgroups_portgroup_ident_get_member:
  path: '/v1/portgroups/{portgroup_ident}'
  method: get
  headers: *scoped_member_headers
-  assert_status: 403
-  skip_reason: not updated for scope testing
+  assert_status: 200

 portgroups_portgroup_ident_get_observer:
  path: '/v1/portgroups/{portgroup_ident}'
  method: get
  headers: *observer_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 portgroups_portgroup_ident_patch_admin:
  path: '/v1/portgroups/{portgroup_ident}'
@ -839,15 +827,13 @@ portgroups_portgroup_ident_patch_admin:
      path: /extra
      value: {'test': 'testing'}
  assert_status: 503
-  skip_reason: not updated for scope testing

 portgroups_portgroup_ident_patch_member:
  path: '/v1/portgroups/{portgroup_ident}'
  method: patch
  headers: *scoped_member_headers
  body: *portgroup_patch_body
-  assert_status: 403
-  skip_reason: not updated for scope testing
+  assert_status: 503

 portgroups_portgroup_ident_patch_observer:
  path: '/v1/portgroups/{portgroup_ident}'
@ -855,7 +841,6 @@ portgroups_portgroup_ident_patch_observer:
  headers: *observer_headers
  body: *portgroup_patch_body
  assert_status: 403
-  skip_reason: not updated for scope testing

 # NOTE(TheJulia): Calls the conductor, so not 403 to validate
 # successful access control check.
@ -864,21 +849,18 @@ portgroups_portgroup_ident_delete_admin:
  method: delete
  headers: *admin_headers
  assert_status: 503
-  skip_reason: not updated for scope testing

 portgroups_portgroup_ident_delete_member:
  path: '/v1/portgroups/{portgroup_ident}'
  method: delete
  headers: *scoped_member_headers
  assert_status: 403
-  skip_reason: not updated for scope testing

 portgroups_portgroup_ident_delete_observer:
  path: '/v1/portgroups/{portgroup_ident}'
  method: delete
  headers: *observer_headers
  assert_status: 403
-  skip_reason: not updated for scope testing

 # Portgroups by node - https://docs.openstack.org/api-ref/baremetal/#listing-portgroups-by-node-nodes-portgroups

@ -887,42 +869,36 @@ nodes_portgroups_get_admin:
  method: get
  headers: *admin_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 nodes_portgroups_get_member:
  path: '/v1/nodes/{node_ident}/portgroups'
  method: get
  headers: *scoped_member_headers
-  assert_status: 403
-  skip_reason: not updated for scope testing
+  assert_status: 200

 nodes_portgroups_get_observer:
  path: '/v1/nodes/{node_ident}/portgroups'
  method: get
  headers: *observer_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 nodes_portgroups_detail_get_admin:
  path: '/v1/nodes/{node_ident}/portgroups/detail'
  method: get
  headers: *admin_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 nodes_portgroups_detail_get_member:
  path: '/v1/nodes/{node_ident}/portgroups/detail'
  method: get
  headers: *scoped_member_headers
-  assert_status: 403
-  skip_reason: not updated for scope testing
+  assert_status: 200

 nodes_portgroups_detail_get_observer:
  path: '/v1/nodes/{node_ident}/portgroups/detail'
  method: get
  headers: *observer_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 # Ports - https://docs.openstack.org/api-ref/baremetal/#ports-ports

@ -1097,42 +1073,36 @@ portgroups_ports_get_admin:
  method: get
  headers: *admin_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 portgroups_ports_get_member:
  path: '/v1/portgroups/{portgroup_ident}/ports'
  method: get
  headers: *scoped_member_headers
-  assert_status: 403
-  skip_reason: not updated for scope testing
+  assert_status: 200

 portgroups_ports_get_observer:
  path: '/v1/portgroups/{portgroup_ident}/ports'
  method: get
  headers: *observer_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 portgroups_ports_detail_get_admin:
  path: '/v1/portgroups/{portgroup_ident}/ports/detail'
  method: get
  headers: *admin_headers
  assert_status: 200
-  skip_reason: not updated for scope testing

 portgroups_ports_detail_get_member:
  path: '/v1/portgroups/{portgroup_ident}/ports/detail'
  method: get
  headers: *scoped_member_headers
-  assert_status: 403
-  skip_reason: not updated for scope testing
+  assert_status: 200

 portgroups_ports_detail_get_observer:
  path: '/v1/portgroups/{portgroup_ident}/ports/detail'
  method: get
  headers: *observer_headers
  assert_status: 200
-  skip_reason: not updated for scope testing


 # Volume(s) - https://docs.openstack.org/api-ref/baremetal/#volume-volume
--- a/releasenotes/notes/system-scoped-authentication-28e3651de250bea8.yaml
+++ b/releasenotes/notes/system-scoped-authentication-28e3651de250bea8.yaml
@ -2,8 +2,8 @@
 features:
  - |
    The Baremetal API, provided by the ironic-api process, now supports use of
-    ``system`` scoped ``keystone`` authentication for the nodes and ports
-    endpoints.
+    ``system`` scoped ``keystone`` authentication for the following endpoints:
+    nodes, ports, portgroups
 upgrade:
  - |
    Deprecated policy rules are not expressed via a default policy file