Allow node owners to administer nodes

Introduce is_node_owner to policy, giving Ironic admins
the option of modifying the policy file to allow users
specified by a node's owner field to perform API actions
on that node.

Change-Id: If08586f3e9705dd38ff83e4b500d9ee3cd45bce3
Story: #2006506
Task: #37214

ironic/api/controllers/v1/node.py

@@ -185,10 +185,10 @@ class BootDeviceController(rest.RestController):
         'supported': ['GET'],
     }
 
-    def _get_boot_device(self, node_ident, supported=False):
+    def _get_boot_device(self, rpc_node, supported=False):
         """Get the current boot device or a list of supported devices.
 
-        :param node_ident: the UUID or logical name of a node.
+        :param rpc_node: RPC Node object.
         :param supported: Boolean value. If true return a list of
                           supported boot devices, if false return the
                           current boot device. Default: False.
@@ -196,7 +196,6 @@ class BootDeviceController(rest.RestController):
                   boot devices.
 
         """
-        rpc_node = api_utils.get_rpc_node(node_ident)
         topic = api.request.rpcapi.get_topic_for(rpc_node)
         if supported:
             return api.request.rpcapi.get_supported_boot_devices(
@@ -221,10 +220,9 @@ class BootDeviceController(rest.RestController):
                            Default: False.
 
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:set_boot_device', cdict, cdict)
+            'baremetal:node:set_boot_device', node_ident)
 
-        rpc_node = api_utils.get_rpc_node(node_ident)
         topic = api.request.rpcapi.get_topic_for(rpc_node)
         api.request.rpcapi.set_boot_device(api.request.context,
                                            rpc_node.uuid,
@@ -246,10 +244,10 @@ class BootDeviceController(rest.RestController):
                 future boots or not, None if it is unknown.
 
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:get_boot_device', cdict, cdict)
+            'baremetal:node:get_boot_device', node_ident)
 
-        return self._get_boot_device(node_ident)
+        return self._get_boot_device(rpc_node)
 
     @METRICS.timer('BootDeviceController.supported')
     @expose.expose(wtypes.text, types.uuid_or_name)
@@ -261,10 +259,10 @@ class BootDeviceController(rest.RestController):
                   devices.
 
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:get_boot_device', cdict, cdict)
+            'baremetal:node:get_boot_device', node_ident)
 
-        boot_devices = self._get_boot_device(node_ident, supported=True)
+        boot_devices = self._get_boot_device(rpc_node, supported=True)
         return {'supported_boot_devices': boot_devices}
 
 
@@ -293,10 +291,9 @@ class InjectNmiController(rest.RestController):
         if not api_utils.allow_inject_nmi():
             raise exception.NotFound()
 
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:inject_nmi', cdict, cdict)
+            'baremetal:node:inject_nmi', node_ident)
 
-        rpc_node = api_utils.get_rpc_node(node_ident)
         topic = api.request.rpcapi.get_topic_for(rpc_node)
         api.request.rpcapi.inject_nmi(api.request.context,
                                       rpc_node.uuid,
@@ -337,10 +334,9 @@ class NodeConsoleController(rest.RestController):
 
         :param node_ident: UUID or logical name of a node.
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:get_console', cdict, cdict)
+            'baremetal:node:get_console', node_ident)
 
-        rpc_node = api_utils.get_rpc_node(node_ident)
         topic = api.request.rpcapi.get_topic_for(rpc_node)
         try:
             console = api.request.rpcapi.get_console_information(
@@ -362,10 +358,9 @@ class NodeConsoleController(rest.RestController):
         :param enabled: Boolean value; whether to enable or disable the
                 console.
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:set_console_state', cdict, cdict)
+            'baremetal:node:set_console_state', node_ident)
 
-        rpc_node = api_utils.get_rpc_node(node_ident)
         topic = api.request.rpcapi.get_topic_for(rpc_node)
         api.request.rpcapi.set_console_mode(api.request.context,
                                             rpc_node.uuid, enabled, topic)
@@ -453,13 +448,12 @@ class NodeStatesController(rest.RestController):
 
         :param node_ident: the UUID or logical_name of a node.
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:get_states', cdict, cdict)
+            'baremetal:node:get_states', node_ident)
 
         # NOTE(lucasagomes): All these state values come from the
         # DB. Ironic counts with a periodic task that verify the current
         # power states of the nodes and update the DB accordingly.
-        rpc_node = api_utils.get_rpc_node(node_ident)
         return NodeStates.convert(rpc_node)
 
     @METRICS.timer('NodeStatesController.raid')
@@ -477,12 +471,11 @@ class NodeStatesController(rest.RestController):
         :raises: NotAcceptable, if requested version of the API is less than
             1.12.
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:set_raid_state', cdict, cdict)
+            'baremetal:node:set_raid_state', node_ident)
 
         if not api_utils.allow_raid_config():
             raise exception.NotAcceptable()
-        rpc_node = api_utils.get_rpc_node(node_ident)
         topic = api.request.rpcapi.get_topic_for(rpc_node)
         try:
             api.request.rpcapi.set_target_raid_config(
@@ -514,12 +507,11 @@ class NodeStatesController(rest.RestController):
         :raises: Invalid (HTTP 400) if timeout value is less than 1.
 
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:set_power_state', cdict, cdict)
+            'baremetal:node:set_power_state', node_ident)
 
         # TODO(lucasagomes): Test if it's able to transition to the
         #                    target state from the current one
-        rpc_node = api_utils.get_rpc_node(node_ident)
         topic = api.request.rpcapi.get_topic_for(rpc_node)
 
         if ((target in [ir_states.SOFT_REBOOT, ir_states.SOFT_POWER_OFF]
@@ -653,11 +645,10 @@ class NodeStatesController(rest.RestController):
         :raises: NotAcceptable (HTTP 406) if the API version specified does
                  not allow the requested state transition.
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:set_provision_state', cdict, cdict)
+            'baremetal:node:set_provision_state', node_ident)
 
         api_utils.check_allow_management_verbs(target)
-        rpc_node = api_utils.get_rpc_node(node_ident)
 
         if (target in (ir_states.ACTIVE, ir_states.REBUILD)
                 and rpc_node.maintenance):
@@ -777,9 +768,8 @@ class NodeTraitsController(rest.RestController):
     @expose.expose(Traits)
     def get_all(self):
         """List node traits."""
-        cdict = api.request.context.to_policy_values()
+        node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:traits:list', cdict, cdict)
+            'baremetal:node:traits:list', self.node_ident)
-        node = api_utils.get_rpc_node(self.node_ident)
         traits = objects.TraitList.get_by_node_id(api.request.context,
                                                   node.id)
         return Traits(traits=traits.get_trait_names())
@@ -797,9 +787,8 @@ class NodeTraitsController(rest.RestController):
             traits with this list.
         """
         context = api.request.context
-        cdict = context.to_policy_values()
+        node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:traits:set', cdict, cdict)
+            'baremetal:node:traits:set', self.node_ident)
-        node = api_utils.get_rpc_node(self.node_ident)
 
         if (trait and traits is not None) or not (trait or traits is not None):
             msg = _("A single node trait may be added via PUT "
@@ -854,9 +843,8 @@ class NodeTraitsController(rest.RestController):
                       None, all traits are removed.
         """
         context = api.request.context
-        cdict = context.to_policy_values()
+        node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:traits:delete', cdict, cdict)
+            'baremetal:node:traits:delete', self.node_ident)
-        node = api_utils.get_rpc_node(self.node_ident)
 
         if trait:
             traits = [trait]
@@ -1385,12 +1373,10 @@ class NodeVendorPassthruController(rest.RestController):
                   entries.
         :raises: NodeNotFound if the node is not found.
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:vendor_passthru', cdict, cdict)
+            'baremetal:node:vendor_passthru', node_ident)
 
         # Raise an exception if node is not found
-        rpc_node = api_utils.get_rpc_node(node_ident)
-
         if rpc_node.driver not in _VENDOR_METHODS:
             topic = api.request.rpcapi.get_topic_for(rpc_node)
             ret = api.request.rpcapi.get_node_vendor_passthru_methods(
@@ -1409,11 +1395,10 @@ class NodeVendorPassthruController(rest.RestController):
         :param method: name of the method in vendor driver.
         :param data: body of data to supply to the specified method.
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:vendor_passthru', cdict, cdict)
+            'baremetal:node:vendor_passthru', node_ident)
 
         # Raise an exception if node is not found
-        rpc_node = api_utils.get_rpc_node(node_ident)
         topic = api.request.rpcapi.get_topic_for(rpc_node)
         return api_utils.vendor_passthru(rpc_node.uuid, method, topic,
                                          data=data)
@@ -1421,9 +1406,8 @@ class NodeVendorPassthruController(rest.RestController):
 
 class NodeMaintenanceController(rest.RestController):
 
-    def _set_maintenance(self, node_ident, maintenance_mode, reason=None):
+    def _set_maintenance(self, rpc_node, maintenance_mode, reason=None):
         context = api.request.context
-        rpc_node = api_utils.get_rpc_node(node_ident)
         rpc_node.maintenance = maintenance_mode
         rpc_node.maintenance_reason = reason
         notify.emit_start_notification(context, rpc_node, 'maintenance_set')
@@ -1449,10 +1433,10 @@ class NodeMaintenanceController(rest.RestController):
         :param reason: Optional, the reason why it's in maintenance.
 
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:set_maintenance', cdict, cdict)
+            'baremetal:node:set_maintenance', node_ident)
 
-        self._set_maintenance(node_ident, True, reason=reason)
+        self._set_maintenance(rpc_node, True, reason=reason)
 
     @METRICS.timer('NodeMaintenanceController.delete')
     @expose.expose(None, types.uuid_or_name, status_code=http_client.ACCEPTED)
@@ -1462,10 +1446,10 @@ class NodeMaintenanceController(rest.RestController):
         :param node_ident: the UUID or logical name of a node.
 
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node = api_utils.check_node_policy_and_retrieve(
-        policy.authorize('baremetal:node:clear_maintenance', cdict, cdict)
+            'baremetal:node:clear_maintenance', node_ident)
 
-        self._set_maintenance(node_ident, False)
+        self._set_maintenance(rpc_node, False)
 
 
 # NOTE(vsaienko) We don't support pagination with VIFs, so we don't use
@@ -1488,8 +1472,9 @@ class NodeVIFController(rest.RestController):
     def __init__(self, node_ident):
         self.node_ident = node_ident
 
-    def _get_node_and_topic(self):
+    def _get_node_and_topic(self, policy_name):
-        rpc_node = api_utils.get_rpc_node(self.node_ident)
+        rpc_node = api_utils.check_node_policy_and_retrieve(
+            policy_name, self.node_ident)
         try:
             return rpc_node, api.request.rpcapi.get_topic_for(rpc_node)
         except exception.NoValidHost as e:
@@ -1500,9 +1485,7 @@ class NodeVIFController(rest.RestController):
     @expose.expose(VifCollection)
     def get_all(self):
         """Get a list of attached VIFs"""
-        cdict = api.request.context.to_policy_values()
+        rpc_node, topic = self._get_node_and_topic('baremetal:node:vif:list')
-        policy.authorize('baremetal:node:vif:list', cdict, cdict)
-        rpc_node, topic = self._get_node_and_topic()
         vifs = api.request.rpcapi.vif_list(api.request.context,
                                            rpc_node.uuid, topic=topic)
         return VifCollection.collection_from_list(vifs)
@@ -1517,9 +1500,7 @@ class NodeVIFController(rest.RestController):
             It must have an 'id' key, whose value is a unique identifier
             for that VIF.
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node, topic = self._get_node_and_topic('baremetal:node:vif:attach')
-        policy.authorize('baremetal:node:vif:attach', cdict, cdict)
-        rpc_node, topic = self._get_node_and_topic()
         api.request.rpcapi.vif_attach(api.request.context, rpc_node.uuid,
                                       vif_info=vif, topic=topic)
 
@@ -1531,9 +1512,7 @@ class NodeVIFController(rest.RestController):
 
         :param vif_id: The ID of a VIF to detach
         """
-        cdict = api.request.context.to_policy_values()
+        rpc_node, topic = self._get_node_and_topic('baremetal:node:vif:detach')
-        policy.authorize('baremetal:node:vif:detach', cdict, cdict)
-        rpc_node, topic = self._get_node_and_topic()
         api.request.rpcapi.vif_detach(api.request.context, rpc_node.uuid,
                                       vif_id=vif_id, topic=topic)
 
@@ -1842,8 +1821,7 @@ class NodesController(rest.RestController):
                                      with description field contains matching
                                      value.
         """
-        cdict = api.request.context.to_policy_values()
+        owner = api_utils.check_node_list_policy(owner)
-        policy.authorize('baremetal:node:get', cdict, cdict)
 
         api_utils.check_allow_specify_fields(fields)
         api_utils.check_allowed_fields(fields)
@@ -1917,8 +1895,7 @@ class NodesController(rest.RestController):
                                      with description field contains matching
                                      value.
         """
-        cdict = api.request.context.to_policy_values()
+        owner = api_utils.check_node_list_policy(owner)
-        policy.authorize('baremetal:node:get', cdict, cdict)
 
         api_utils.check_for_invalid_state_and_allow_filter(provision_state)
         api_utils.check_allow_specify_driver(driver)
@@ -1960,9 +1937,6 @@ class NodesController(rest.RestController):
         :param node: UUID or name of a node.
         :param node_uuid: UUID of a node.
         """
-        cdict = api.request.context.to_policy_values()
-        policy.authorize('baremetal:node:validate', cdict, cdict)
-
         if node is not None:
             # We're invoking this interface using positional notation, or
             # explicitly using 'node'.  Try and determine which one.
@@ -1970,7 +1944,8 @@ class NodesController(rest.RestController):
                 and not uuidutils.is_uuid_like(node)):
                 raise exception.NotAcceptable()
 
-        rpc_node = api_utils.get_rpc_node(node_uuid or node)
+        rpc_node = api_utils.check_node_policy_and_retrieve(
+            'baremetal:node:validate', node_uuid or node)
 
         topic = api.request.rpcapi.get_topic_for(rpc_node)
         return api.request.rpcapi.validate_driver_interfaces(
@@ -1985,16 +1960,15 @@ class NodesController(rest.RestController):
         :param fields: Optional, a list with a specified set of fields
             of the resource to be returned.
         """
-        cdict = api.request.context.to_policy_values()
-        policy.authorize('baremetal:node:get', cdict, cdict)
-
         if self.from_chassis:
             raise exception.OperationNotPermitted()
 
+        rpc_node = api_utils.check_node_policy_and_retrieve(
+            'baremetal:node:get', node_ident, with_suffix=True)
+
         api_utils.check_allow_specify_fields(fields)
         api_utils.check_allowed_fields(fields)
 
-        rpc_node = api_utils.get_rpc_node_with_suffix(node_ident)
         return Node.convert_with_links(rpc_node, fields=fields)
 
     @METRICS.timer('NodesController.post')
@@ -2004,13 +1978,13 @@ class NodesController(rest.RestController):
 
         :param node: a node within the request body.
         """
+        if self.from_chassis:
+            raise exception.OperationNotPermitted()
+
         context = api.request.context
         cdict = context.to_policy_values()
         policy.authorize('baremetal:node:create', cdict, cdict)
 
-        if self.from_chassis:
-            raise exception.OperationNotPermitted()
-
         if node.conductor is not wtypes.Unset:
             msg = _("Cannot specify conductor on node creation.")
             raise exception.Invalid(msg)
@@ -2112,17 +2086,15 @@ class NodesController(rest.RestController):
             defaults. Only valid when updating the driver field.
         :param patch: a json PATCH document to apply to this node.
         """
-        context = api.request.context
-        cdict = context.to_policy_values()
-        policy.authorize('baremetal:node:update', cdict, cdict)
-
         if (reset_interfaces is not None and not
                 api_utils.allow_reset_interfaces()):
             raise exception.NotAcceptable()
 
         self._validate_patch(patch, reset_interfaces)
 
-        rpc_node = api_utils.get_rpc_node_with_suffix(node_ident)
+        context = api.request.context
+        rpc_node = api_utils.check_node_policy_and_retrieve(
+            'baremetal:node:update', node_ident, with_suffix=True)
 
         remove_inst_uuid_patch = [{'op': 'remove', 'path': '/instance_uuid'}]
         if rpc_node.maintenance and patch == remove_inst_uuid_patch:
@@ -2196,14 +2168,13 @@ class NodesController(rest.RestController):
 
         :param node_ident: UUID or logical name of a node.
         """
-        context = api.request.context
-        cdict = context.to_policy_values()
-        policy.authorize('baremetal:node:delete', cdict, cdict)
-
         if self.from_chassis:
             raise exception.OperationNotPermitted()
 
-        rpc_node = api_utils.get_rpc_node_with_suffix(node_ident)
+        context = api.request.context
+        rpc_node = api_utils.check_node_policy_and_retrieve(
+            'baremetal:node:delete', node_ident, with_suffix=True)
+
         chassis_uuid = _get_chassis_uuid(rpc_node)
         notify.emit_start_notification(context, rpc_node, 'delete',
                                        chassis_uuid=chassis_uuid)

ironic/api/controllers/v1/utils.py

@@ -1155,6 +1155,58 @@ def check_policy(policy_name):
     policy.authorize(policy_name, cdict, cdict)
 
 
+def check_node_policy_and_retrieve(policy_name, node_ident, with_suffix=False):
+    """Check if the specified policy authorizes this request on a node.
+
+    :param: policy_name: Name of the policy to check.
+    :param: node_ident: the UUID or logical name of a node.
+    :param: with_suffix: whether the RPC node should include the suffix
+
+    :raises: HTTPForbidden if the policy forbids access.
+    :raises: NodeNotFound if the node is not found.
+    :return: RPC node identified by node_ident
+    """
+    cdict = api.request.context.to_policy_values()
+
+    try:
+        if with_suffix:
+            rpc_node = get_rpc_node_with_suffix(node_ident)
+        else:
+            rpc_node = get_rpc_node(node_ident)
+    except exception.NodeNotFound:
+        # don't expose non-existence of node unless requester
+        # has generic access to policy
+        policy.authorize(policy_name, cdict, cdict)
+        raise
+
+    target_dict = dict(cdict)
+    target_dict['node.owner'] = rpc_node['owner']
+    policy.authorize(policy_name, target_dict, cdict)
+
+    return rpc_node
+
+
+def check_node_list_policy(owner=None):
+    """Check if the specified policy authorizes this request on a node.
+
+    :param: owner: owner filter for list query, if any
+
+    :raises: HTTPForbidden if the policy forbids access.
+    :raises: NodeNotFound if the node is not found.
+    :return: owner that should be used for list query, if needed
+    """
+    cdict = api.request.context.to_policy_values()
+    try:
+        policy.authorize('baremetal:node:list_all', cdict, cdict)
+    except exception.HTTPForbidden:
+        project_owner = cdict.get('project_id')
+        if (not project_owner or (owner and owner != project_owner)):
+            raise
+        policy.authorize('baremetal:node:list', cdict, cdict)
+        return project_owner
+    return owner
+
+
 def allow_build_configdrive():
     """Check if building configdrive is allowed.
 

ironic/common/policy.py

@@ -63,6 +63,9 @@ default_policies = [
     policy.RuleDefault('is_admin',
                        'rule:admin_api or (rule:is_member and role:baremetal_admin)',  # noqa
                        description='Full read/write API access'),
+    policy.RuleDefault('is_node_owner',
+                       'project_id:%(node.owner)s',
+                       description='Owner of node'),
 ]
 
 # NOTE(deva): to follow policy-in-code spec, we define defaults for
@@ -79,10 +82,20 @@ node_policies = [
     policy.DocumentedRuleDefault(
         'baremetal:node:get',
         'rule:is_admin or rule:is_observer',
-        'Retrieve Node records',
+        'Retrieve a single Node record',
+        [{'path': '/nodes/{node_ident}', 'method': 'GET'}]),
+    policy.DocumentedRuleDefault(
+        'baremetal:node:list',
+        'rule:baremetal:node:get',
+        'Retrieve multiple Node records, filtered by owner',
         [{'path': '/nodes', 'method': 'GET'},
-         {'path': '/nodes/detail', 'method': 'GET'},
+         {'path': '/nodes/detail', 'method': 'GET'}]),
-         {'path': '/nodes/{node_ident}', 'method': 'GET'}]),
+    policy.DocumentedRuleDefault(
+        'baremetal:node:list_all',
+        'rule:baremetal:node:get',
+        'Retrieve multiple Node records',
+        [{'path': '/nodes', 'method': 'GET'},
+         {'path': '/nodes/detail', 'method': 'GET'}]),
     policy.DocumentedRuleDefault(
         'baremetal:node:update',
         'rule:is_admin',

ironic/tests/unit/api/controllers/v1/test_expose.py

@@ -61,12 +61,14 @@ class TestExposedAPIMethodsCheckPolicy(test_base.TestCase):
 
         for func in self.exposed_methods:
             src = inspect.getsource(func)
-            self.assertIn('policy.authorize', src,
+            self.assertTrue(
-                          'policy.authorize call not found in exposed '
+                ('api_utils.check_node_policy_and_retrieve' in src) or
-                          'method %s' % func)
+                ('api_utils.check_node_list_policy' in src) or
-            self.assertIn('context.to_policy_values', src,
+                ('self._get_node_and_topic' in src) or
-                          'context.to_policy_values call not found in '
+                ('policy.authorize' in src and
-                          'exposed method %s' % func)
+                 'context.to_policy_values' in src),
+                'no policy check found in in exposed '
+                'method %s' % func)
 
     def test_chassis_api_policy(self):
         self._test('ironic.api.controllers.v1.chassis')

ironic/tests/unit/api/controllers/v1/test_node.py

@@ -36,6 +36,7 @@ from ironic.api.controllers.v1 import versions
 from ironic.common import boot_devices
 from ironic.common import driver_factory
 from ironic.common import exception
+from ironic.common import policy
 from ironic.common import states
 from ironic.conductor import rpcapi
 from ironic import objects
@@ -684,6 +685,75 @@ class TestListNodes(test_api_base.BaseApiTest):
                                  expect_errors=True)
         self.assertEqual(http_client.NOT_FOUND, response.status_int)
 
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_detail_forbidden(self, mock_authorize):
+        def mock_authorize_function(rule, target, creds):
+            raise exception.HTTPForbidden(resource='fake')
+        mock_authorize.side_effect = mock_authorize_function
+
+        response = self.get_json('/nodes/detail', expect_errors=True,
+                                 headers={
+                                     api_base.Version.string: '1.50',
+                                     'X-Project-Id': '12345'
+                                 })
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_detail_list_all_forbidden_no_project(self, mock_authorize):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:list_all':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+
+        response = self.get_json('/nodes/detail', expect_errors=True,
+                                 headers={
+                                     api_base.Version.string: '1.49',
+                                 })
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_detail_list_all_forbid_owner_proj_mismatch(self, mock_authorize):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:list_all':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+
+        response = self.get_json('/nodes/detail?owner=54321',
+                                 expect_errors=True,
+                                 headers={
+                                     api_base.Version.string: '1.50',
+                                     'X-Project-Id': '12345'
+                                 })
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_detail_list_all_forbidden(self, mock_authorize):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:list_all':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+
+        nodes = []
+        for id in range(5):
+            node = obj_utils.create_test_node(self.context,
+                                              uuid=uuidutils.generate_uuid(),
+                                              owner='12345')
+            nodes.append(node.uuid)
+        for id in range(2):
+            node = obj_utils.create_test_node(self.context,
+                                              uuid=uuidutils.generate_uuid())
+
+        data = self.get_json('/nodes/detail', headers={
+            api_base.Version.string: '1.50',
+            'X-Project-Id': '12345'})
+        self.assertEqual(len(nodes), len(data['nodes']))
+
+        uuids = [n['uuid'] for n in data['nodes']]
+        self.assertEqual(sorted(nodes), sorted(uuids))
+
     def test_mask_available_state(self):
         node = obj_utils.create_test_node(self.context,
                                           provision_state=states.AVAILABLE)
@@ -856,6 +926,75 @@ class TestListNodes(test_api_base.BaseApiTest):
         self.assertEqual(len(nodes), len(data['nodes']))
         self.assertEqual(sorted(node_names), sorted(names))
 
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_many_forbidden(self, mock_authorize):
+        def mock_authorize_function(rule, target, creds):
+            raise exception.HTTPForbidden(resource='fake')
+        mock_authorize.side_effect = mock_authorize_function
+
+        response = self.get_json('/nodes', expect_errors=True,
+                                 headers={
+                                     api_base.Version.string: '1.50',
+                                     'X-Project-Id': '12345'
+                                 })
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_many_list_all_forbidden_no_project(self, mock_authorize):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:list_all':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+
+        response = self.get_json('/nodes', expect_errors=True,
+                                 headers={
+                                     api_base.Version.string: '1.49',
+                                 })
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_many_list_all_forbid_owner_proj_mismatch(self, mock_authorize):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:list_all':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+
+        response = self.get_json('/nodes?owner=54321',
+                                 expect_errors=True,
+                                 headers={
+                                     api_base.Version.string: '1.50',
+                                     'X-Project-Id': '12345'
+                                 })
+        self.assertEqual(http_client.FORBIDDEN, response.status_int)
+
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_many_list_all_forbidden(self, mock_authorize):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:list_all':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+
+        nodes = []
+        for id in range(5):
+            node = obj_utils.create_test_node(self.context,
+                                              uuid=uuidutils.generate_uuid(),
+                                              owner='12345')
+            nodes.append(node.uuid)
+        for id in range(2):
+            node = obj_utils.create_test_node(self.context,
+                                              uuid=uuidutils.generate_uuid())
+
+        data = self.get_json('/nodes', headers={
+            api_base.Version.string: '1.50',
+            'X-Project-Id': '12345'})
+        self.assertEqual(len(nodes), len(data['nodes']))
+
+        uuids = [n['uuid'] for n in data['nodes']]
+        self.assertEqual(sorted(nodes), sorted(uuids))
+
     def _test_links(self, public_url=None):
         cfg.CONF.set_override('public_endpoint', public_url, 'api')
         uuid = uuidutils.generate_uuid()

ironic/tests/unit/api/controllers/v1/test_utils.py

@@ -770,3 +770,207 @@ class TestPortgroupIdent(base.TestCase):
         self.assertRaises(exception.InvalidUuidOrName,
                           utils.get_rpc_portgroup,
                           self.invalid_name)
+
+
+class TestCheckNodePolicyAndRetrieve(base.TestCase):
+    def setUp(self):
+        super(TestCheckNodePolicyAndRetrieve, self).setUp()
+        self.valid_node_uuid = uuidutils.generate_uuid()
+        self.node = test_api_utils.post_get_test_node()
+        self.node['owner'] = '12345'
+
+    @mock.patch.object(api, 'request', spec_set=["context", "version"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    @mock.patch.object(utils, 'get_rpc_node')
+    @mock.patch.object(utils, 'get_rpc_node_with_suffix')
+    def test_check_node_policy_and_retrieve(
+            self, mock_grnws, mock_grn, mock_authorize, mock_pr
+    ):
+        mock_pr.version.minor = 50
+        mock_pr.context.to_policy_values.return_value = {}
+        mock_grn.return_value = self.node
+
+        rpc_node = utils.check_node_policy_and_retrieve(
+            'fake_policy', self.valid_node_uuid
+        )
+        mock_grn.assert_called_once_with(self.valid_node_uuid)
+        mock_grnws.assert_not_called()
+        mock_authorize.assert_called_once_with(
+            'fake_policy', {'node.owner': '12345'}, {})
+        self.assertEqual(self.node, rpc_node)
+
+    @mock.patch.object(api, 'request', spec_set=["context", "version"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    @mock.patch.object(utils, 'get_rpc_node')
+    @mock.patch.object(utils, 'get_rpc_node_with_suffix')
+    def test_check_node_policy_and_retrieve_with_suffix(
+            self, mock_grnws, mock_grn, mock_authorize, mock_pr
+    ):
+        mock_pr.version.minor = 50
+        mock_pr.context.to_policy_values.return_value = {}
+        mock_grnws.return_value = self.node
+
+        rpc_node = utils.check_node_policy_and_retrieve(
+            'fake_policy', self.valid_node_uuid, True
+        )
+        mock_grn.assert_not_called()
+        mock_grnws.assert_called_once_with(self.valid_node_uuid)
+        mock_authorize.assert_called_once_with(
+            'fake_policy', {'node.owner': '12345'}, {})
+        self.assertEqual(self.node, rpc_node)
+
+    @mock.patch.object(api, 'request', spec_set=["context"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    @mock.patch.object(utils, 'get_rpc_node')
+    def test_check_node_policy_and_retrieve_no_node_policy_forbidden(
+            self, mock_grn, mock_authorize, mock_pr
+    ):
+        mock_pr.context.to_policy_values.return_value = {}
+        mock_authorize.side_effect = exception.HTTPForbidden(resource='fake')
+        mock_grn.side_effect = exception.NodeNotFound(
+            node=self.valid_node_uuid)
+
+        self.assertRaises(
+            exception.HTTPForbidden,
+            utils.check_node_policy_and_retrieve,
+            'fake-policy',
+            self.valid_node_uuid
+        )
+
+    @mock.patch.object(api, 'request', spec_set=["context"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    @mock.patch.object(utils, 'get_rpc_node')
+    def test_check_node_policy_and_retrieve_no_node(
+            self, mock_grn, mock_authorize, mock_pr
+    ):
+        mock_pr.context.to_policy_values.return_value = {}
+        mock_grn.side_effect = exception.NodeNotFound(
+            node=self.valid_node_uuid)
+
+        self.assertRaises(
+            exception.NodeNotFound,
+            utils.check_node_policy_and_retrieve,
+            'fake-policy',
+            self.valid_node_uuid
+        )
+
+    @mock.patch.object(api, 'request', spec_set=["context", "version"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    @mock.patch.object(utils, 'get_rpc_node')
+    def test_check_node_policy_and_retrieve_policy_forbidden(
+            self, mock_grn, mock_authorize, mock_pr
+    ):
+        mock_pr.version.minor = 50
+        mock_pr.context.to_policy_values.return_value = {}
+        mock_authorize.side_effect = exception.HTTPForbidden(resource='fake')
+        mock_grn.return_value = self.node
+
+        self.assertRaises(
+            exception.HTTPForbidden,
+            utils.check_node_policy_and_retrieve,
+            'fake-policy',
+            self.valid_node_uuid
+        )
+
+
+class TestCheckNodeListPolicy(base.TestCase):
+    @mock.patch.object(api, 'request', spec_set=["context", "version"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_check_node_list_policy(
+            self, mock_authorize, mock_pr
+    ):
+        mock_pr.context.to_policy_values.return_value = {
+            'project_id': '12345'
+        }
+        mock_pr.version.minor = 50
+
+        owner = utils.check_node_list_policy()
+        self.assertIsNone(owner)
+
+    @mock.patch.object(api, 'request', spec_set=["context", "version"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_check_node_list_policy_with_owner(
+            self, mock_authorize, mock_pr
+    ):
+        mock_pr.context.to_policy_values.return_value = {
+            'project_id': '12345'
+        }
+        mock_pr.version.minor = 50
+
+        owner = utils.check_node_list_policy('12345')
+        self.assertEqual(owner, '12345')
+
+    @mock.patch.object(api, 'request', spec_set=["context", "version"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_check_node_list_policy_forbidden(
+            self, mock_authorize, mock_pr
+    ):
+        def mock_authorize_function(rule, target, creds):
+            raise exception.HTTPForbidden(resource='fake')
+        mock_authorize.side_effect = mock_authorize_function
+        mock_pr.context.to_policy_values.return_value = {
+            'project_id': '12345'
+        }
+        mock_pr.version.minor = 50
+
+        self.assertRaises(
+            exception.HTTPForbidden,
+            utils.check_node_list_policy,
+        )
+
+    @mock.patch.object(api, 'request', spec_set=["context", "version"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_check_node_list_policy_forbidden_no_project(
+            self, mock_authorize, mock_pr
+    ):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:list_all':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+        mock_pr.context.to_policy_values.return_value = {}
+        mock_pr.version.minor = 50
+
+        self.assertRaises(
+            exception.HTTPForbidden,
+            utils.check_node_list_policy,
+        )
+
+    @mock.patch.object(api, 'request', spec_set=["context", "version"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_check_node_list_policy_non_admin(
+            self, mock_authorize, mock_pr
+    ):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:list_all':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+        mock_pr.context.to_policy_values.return_value = {
+            'project_id': '12345'
+        }
+        mock_pr.version.minor = 50
+
+        owner = utils.check_node_list_policy()
+        self.assertEqual(owner, '12345')
+
+    @mock.patch.object(api, 'request', spec_set=["context", "version"])
+    @mock.patch.object(policy, 'authorize', spec=True)
+    def test_check_node_list_policy_non_admin_owner_proj_mismatch(
+            self, mock_authorize, mock_pr
+    ):
+        def mock_authorize_function(rule, target, creds):
+            if rule == 'baremetal:node:list_all':
+                raise exception.HTTPForbidden(resource='fake')
+            return True
+        mock_authorize.side_effect = mock_authorize_function
+        mock_pr.context.to_policy_values.return_value = {
+            'project_id': '12345'
+        }
+        mock_pr.version.minor = 50
+
+        self.assertRaises(
+            exception.HTTPForbidden,
+            utils.check_node_list_policy,
+            '54321'
+        )

ironic/tests/unit/common/test_policy.py

@@ -56,6 +56,19 @@ class PolicyInCodeTestCase(base.TestCase):
         c = {'project_name': 'demo1', 'project_domain_id': 'default2'}
         self.assertFalse(policy.check('is_member', c, c))
 
+    def test_is_node_owner(self):
+        c1 = {'project_id': '1234',
+              'project_name': 'demo',
+              'project_domain_id': 'default'}
+        c2 = {'project_id': '5678',
+              'project_name': 'demo',
+              'project_domain_id': 'default'}
+        target = dict.copy(c1)
+        target['node.owner'] = '1234'
+
+        self.assertTrue(policy.check('is_node_owner', target, c1))
+        self.assertFalse(policy.check('is_node_owner', target, c2))
+
     def test_node_get(self):
         creds = {'roles': ['baremetal_observer'], 'project_name': 'demo',
                  'project_domain_id': 'default'}

releasenotes/notes/node-owner-policy-d7168976bba70566.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - Adds a ``is_node_owner`` policy rule. This rule can be used with node
+    policy rules in order to expose specific node APIs to a project ID
+    specified by a node's ``owner`` field. Default rules are unaffected,
+    so default behavior is unchanged.