From e9dfe5ddaad7324d8d89fef0661f41f18542028f Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Fri, 12 Feb 2021 16:36:38 -0800 Subject: [PATCH] Port/Portgroup project scoped access This patch implements the project scoped rbac policies for a system and project scoped deployment of ironic. Because of the nature of Ports and Portgroups, along with the subcontroller resources, this change was a little more invasive than was originally anticipated. In that process, along with some discussion in the #openstack-ironic IRC channel, that it would be most security concious to respond only with 404s if the user simply does not have access to the underlying node object. In essence, their view of the universe has been restricted as they have less acess rights, and we appropriately enforce that. Not expecting that, or not conciously being aware of that, can quickly lead to confusion though. Possibly a day or more of Julia's life as well, but it comes down to perceptions and awareness. Change-Id: I68c5f2bae76ca313ba77285747dc6b1bc8b623b9 --- doc/source/admin/secure-rbac.rst | 65 ++++++++- ironic/api/controllers/v1/node.py | 2 - ironic/api/controllers/v1/port.py | 45 +++++- ironic/api/controllers/v1/portgroup.py | 91 ++++++++---- ironic/api/controllers/v1/utils.py | 122 ++++++++++++++-- ironic/common/policy.py | 72 +++++++--- ironic/db/api.py | 11 +- ironic/db/sqlalchemy/api.py | 26 +++- ironic/objects/portgroup.py | 18 ++- .../unit/api/controllers/v1/test_node.py | 12 +- .../unit/api/controllers/v1/test_utils.py | 20 ++- ironic/tests/unit/api/test_acl.py | 50 +++++-- ironic/tests/unit/api/test_rbac_legacy.yaml | 24 ++-- .../unit/api/test_rbac_project_scoped.yaml | 136 +++++++----------- .../unit/api/test_rbac_system_scoped.yaml | 4 +- ironic/tests/unit/objects/test_portgroup.py | 2 +- 16 files changed, 498 insertions(+), 202 deletions(-) diff --git a/doc/source/admin/secure-rbac.rst b/doc/source/admin/secure-rbac.rst index 3e611cba21..245feef8f3 100644 --- a/doc/source/admin/secure-rbac.rst +++ b/doc/source/admin/secure-rbac.rst @@ -2,6 +2,33 @@ Secure RBAC =========== +Suggested Reading +================= + +It is likely an understatement to say that policy enforcement is a complex +subject. It requires operational context to craft custom policy to meet +general use needs. Part of this is why the Secure RBAC effort was started, +to provide consistency and a "good" starting place for most users who need +a higher level of granularity. + +That being said, it would likely help anyone working to implement +customization of these policies to consult some reference material +in hopes of understanding the context. + +* `Keystone Adminstrator Guide - Service API Protection `_ +* `Ironic Scoped Role Based Access Control Specification `_ + +Historical Context - How we reached our access model +---------------------------------------------------- + +Ironic has reached the access model through an evolution the API and the data +stored. Along with the data stored, the enforcement of policy based upon data +stored in these fields. + +* `Ownership Information Storage `_ +* `Allow Node owners to Administer `_ +* `Allow Leasable Nodes `_ + System Scoped ============= @@ -37,6 +64,10 @@ Supported Endpoints ------------------- * /nodes +* /nodes//ports +* /nodes//portgroups +* /ports +* /portgroups How Project Scoped Works ------------------------ @@ -51,8 +82,6 @@ of the node. Regardless of the use model that the fields and mechanics support, these fields are to support humans, and possibly services where applicable. -.. todo: Add link to owner spec. - The purpose of a lessee is more for a *tenant* in their *project* to be able to have access to perform basic actions with the API. In some cases that may be to reprovision or rebuild a node. Ultimately that is the lessee's @@ -60,9 +89,6 @@ progative, but by default there are actions and field updates that cannot be performed by default. This is also governed by access level within a project. -.. todo: Add link to lessee spec. - - These policies are applied in the way data is viewed and how data can be updated. Generally, an inability to view a node is an access permission issue in term of the project ID being correct for owner/lessee. @@ -72,7 +98,6 @@ reasonable, however operators may wish to override these policy settings. For details general policy setting details, please see :doc:`/configuration/policy`. - Field value visibility restrictions ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -117,3 +142,31 @@ change the owner. it is restricted to system scoped administrators. More information is available on these fields in :doc:`/configuration/policy`. + +Pratical differences +-------------------- + +Most users, upon implementing the use of ``system`` scoped authenticaiton, +should not notice a difference as long as their authentication token is +properly scoped to ``system`` and with the appropriate role for their +access level. For most users who used a ``baremetal`` project, +or other custom project via a custom policy file, along with a custom +role name such as ``baremetal_admin``, this will require changing +the user to be a ``system`` scoped user with ``admin`` privilges. + +The most noticable difference for API consumers is the HTTP 403 access +code is now mainly a HTTP 404 access code. The access concept has changed +from "Does the user user broadly has access to the API?" to +"Does user have access to the node, and then do they have access +to the specific resource?". + +How do I assign an owner? +------------------------- + +.. todo: need to add information on the owner assignment + and also cover what this generally means... maybe? + +How do I assign a lessee? +------------------------- + +.. todo: Need to cover how to assign a lessee. diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index dfdc9c02d6..be5f0106de 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -2232,7 +2232,6 @@ class NodesController(rest.RestController): policy_checks.append('baremetal:node:update:retired') else: generic_update = True - # always do at least one check if generic_update or not policy_checks: policy_checks.append('baremetal:node:update') @@ -2334,7 +2333,6 @@ class NodesController(rest.RestController): node_dict, NODE_PATCH_SCHEMA, NODE_PATCH_VALIDATOR) self._update_changed_fields(node_dict, rpc_node) - # NOTE(tenbrae): we calculate the rpc topic here in case node.driver # has changed, so that update is sent to the # new conductor, not the old one which may fail to diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index f4480ef7bd..4c58e0110a 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -383,7 +383,15 @@ class PortsController(rest.RestController): for that portgroup. :raises: NotAcceptable, HTTPNotFound """ - project = api_utils.check_port_list_policy() + project = api_utils.check_port_list_policy( + parent_node=self.parent_node_ident, + parent_portgroup=self.parent_portgroup_ident) + + if self.parent_node_ident: + node = self.parent_node_ident + + if self.parent_portgroup_ident: + portgroup = self.parent_portgroup_ident api_utils.check_allow_specify_fields(fields) self._check_allowed_port_fields(fields) @@ -439,7 +447,9 @@ class PortsController(rest.RestController): :param sort_dir: direction to sort. "asc" or "desc". Default: asc. :raises: NotAcceptable, HTTPNotFound """ - project = api_utils.check_port_list_policy() + project = api_utils.check_port_list_policy( + parent_node=self.parent_node_ident, + parent_portgroup=self.parent_portgroup_ident) self._check_allowed_port_fields([sort_key]) if portgroup and not api_utils.allow_portgroups_subcontrollers(): @@ -499,13 +509,36 @@ class PortsController(rest.RestController): if self.parent_node_ident or self.parent_portgroup_ident: raise exception.OperationNotPermitted() - context = api.request.context - api_utils.check_policy('baremetal:port:create') - # NOTE(lucasagomes): Create the node_id attribute on-the-fly # to satisfy the api -> rpc object # conversion. - node = api_utils.replace_node_uuid_with_id(port) + # NOTE(TheJulia): The get of the node *does* check if the node + # can be accessed. We need to be able to get the node regardless + # in order to perform the actual policy check. + raise_node_not_found = False + node = None + owner = None + lessee = None + node_uuid = port.get('node_uuid') + try: + node = api_utils.replace_node_uuid_with_id(port) + owner = node.owner + lessee = node.lessee + except exception.NotFound: + raise_node_not_found = True + + # While the rule is for the port, the base object that controls access + # is the node. + api_utils.check_owner_policy('node', 'baremetal:port:create', + owner, lessee=lessee, + conceal_node=False) + if raise_node_not_found: + # Delayed raise of NodeNotFound because we want to check + # the access policy first. + raise exception.NodeNotFound(node=node_uuid, + code=http_client.BAD_REQUEST) + + context = api.request.context self._check_allowed_port_fields(port) diff --git a/ironic/api/controllers/v1/portgroup.py b/ironic/api/controllers/v1/portgroup.py index 077e9ab71d..6c9cc9303f 100644 --- a/ironic/api/controllers/v1/portgroup.py +++ b/ironic/api/controllers/v1/portgroup.py @@ -170,7 +170,7 @@ class PortgroupsController(pecan.rest.RestController): def _get_portgroups_collection(self, node_ident, address, marker, limit, sort_key, sort_dir, resource_url=None, fields=None, - detail=None): + detail=None, project=None): """Return portgroups collection. :param node_ident: UUID or name of a node. @@ -182,6 +182,7 @@ class PortgroupsController(pecan.rest.RestController): :param resource_url: Optional, URL to the portgroup resource. :param fields: Optional, a list with a specified set of fields of the resource to be returned. + :param project: Optional, project ID to filter the request by. """ limit = api_utils.validate_limit(limit) sort_dir = api_utils.validate_sort_dir(sort_dir) @@ -206,13 +207,16 @@ class PortgroupsController(pecan.rest.RestController): node = api_utils.get_rpc_node(node_ident) portgroups = objects.Portgroup.list_by_node_id( api.request.context, node.id, limit, - marker_obj, sort_key=sort_key, sort_dir=sort_dir) + marker_obj, sort_key=sort_key, sort_dir=sort_dir, + project=project) elif address: - portgroups = self._get_portgroups_by_address(address) + portgroups = self._get_portgroups_by_address(address, + project=project) else: portgroups = objects.Portgroup.list(api.request.context, limit, marker_obj, sort_key=sort_key, - sort_dir=sort_dir) + sort_dir=sort_dir, + project=project) parameters = {} if detail is not None: parameters['detail'] = detail @@ -224,7 +228,7 @@ class PortgroupsController(pecan.rest.RestController): sort_dir=sort_dir, **parameters) - def _get_portgroups_by_address(self, address): + def _get_portgroups_by_address(self, address, project=None): """Retrieve a portgroup by its address. :param address: MAC address of a portgroup, to get the portgroup @@ -235,7 +239,8 @@ class PortgroupsController(pecan.rest.RestController): """ try: portgroup = objects.Portgroup.get_by_address(api.request.context, - address) + address, + project=project) return [portgroup] except exception.PortgroupNotFound: return [] @@ -268,7 +273,14 @@ class PortgroupsController(pecan.rest.RestController): if not api_utils.allow_portgroups(): raise exception.NotFound() - api_utils.check_policy('baremetal:portgroup:get') + if self.parent_node_ident: + # Override the node, since this is being called by another + # controller with a linked view. + node = self.parent_node_ident + + project = api_utils.check_port_list_policy( + portgroup=True, + parent_node=self.parent_node_ident) api_utils.check_allowed_portgroup_fields(fields) api_utils.check_allowed_portgroup_fields([sort_key]) @@ -280,7 +292,8 @@ class PortgroupsController(pecan.rest.RestController): marker, limit, sort_key, sort_dir, fields=fields, - detail=detail) + detail=detail, + project=project) @METRICS.timer('PortgroupsController.detail') @method.expose() @@ -306,7 +319,15 @@ class PortgroupsController(pecan.rest.RestController): if not api_utils.allow_portgroups(): raise exception.NotFound() - api_utils.check_policy('baremetal:portgroup:get') + if self.parent_node_ident: + # If we have a parent node, then we need to override this method's + # node filter. + node = self.parent_node_ident + + project = api_utils.check_port_list_policy( + portgroup=True, + parent_node=self.parent_node_ident) + api_utils.check_allowed_portgroup_fields([sort_key]) # NOTE: /detail should only work against collections @@ -317,7 +338,7 @@ class PortgroupsController(pecan.rest.RestController): resource_url = '/'.join(['portgroups', 'detail']) return self._get_portgroups_collection( node, address, marker, limit, sort_key, sort_dir, - resource_url=resource_url) + resource_url=resource_url, project=project) @METRICS.timer('PortgroupsController.get_one') @method.expose() @@ -332,7 +353,8 @@ class PortgroupsController(pecan.rest.RestController): if not api_utils.allow_portgroups(): raise exception.NotFound() - api_utils.check_policy('baremetal:portgroup:get') + rpc_portgroup, rpc_node = api_utils.check_port_policy_and_retrieve( + 'baremetal:portgroup:get', portgroup_ident, portgroup=True) if self.parent_node_ident: raise exception.OperationNotPermitted() @@ -355,8 +377,31 @@ class PortgroupsController(pecan.rest.RestController): if not api_utils.allow_portgroups(): raise exception.NotFound() + raise_node_not_found = False + node = None + owner = None + lessee = None + node_uuid = portgroup.get('node_uuid') + try: + # The replace_node_uuid_with_id also checks access to the node + # and will raise an exception if access is not permitted. + node = api_utils.replace_node_uuid_with_id(portgroup) + owner = node.owner + lessee = node.lessee + except exception.NotFound: + raise_node_not_found = True + + # While the rule is for the port, the base object that controls access + # is the node. + api_utils.check_owner_policy('node', 'baremetal:portgroup:create', + owner, lessee=lessee, + conceal_node=False) + if raise_node_not_found: + # Delayed raise of NodeNotFound because we want to check + # the access policy first. + raise exception.NodeNotFound(node=node_uuid, + code=http_client.BAD_REQUEST) context = api.request.context - api_utils.check_policy('baremetal:portgroup:create') if self.parent_node_ident: raise exception.OperationNotPermitted() @@ -378,8 +423,6 @@ class PortgroupsController(pecan.rest.RestController): if not portgroup.get('uuid'): portgroup['uuid'] = uuidutils.generate_uuid() - node = api_utils.replace_node_uuid_with_id(portgroup) - new_portgroup = objects.Portgroup(context, **portgroup) notify.emit_start_notification(context, new_portgroup, 'create', @@ -409,7 +452,9 @@ class PortgroupsController(pecan.rest.RestController): raise exception.NotFound() context = api.request.context - api_utils.check_policy('baremetal:portgroup:update') + + rpc_portgroup, rpc_node = api_utils.check_port_policy_and_retrieve( + 'baremetal:portgroup:update', portgroup_ident, portgroup=True) if self.parent_node_ident: raise exception.OperationNotPermitted() @@ -421,9 +466,6 @@ class PortgroupsController(pecan.rest.RestController): api_utils.patch_validate_allowed_fields(patch, PATCH_ALLOWED_FIELDS) - rpc_portgroup = api_utils.get_rpc_portgroup_with_suffix( - portgroup_ident) - names = api_utils.get_patch_values(patch, '/name') for name in names: if (name and not api_utils.is_valid_logical_name(name)): @@ -440,8 +482,8 @@ class PortgroupsController(pecan.rest.RestController): # 1) Remove node_id because it's an internal value and # not present in the API object # 2) Add node_uuid - rpc_node = api_utils.replace_node_id_with_uuid(portgroup_dict) - + portgroup_dict.pop('node_id') + portgroup_dict['node_uuid'] = rpc_node.uuid portgroup_dict = api_utils.apply_jsonpatch(portgroup_dict, patch) if 'mode' not in portgroup_dict: @@ -504,17 +546,14 @@ class PortgroupsController(pecan.rest.RestController): if not api_utils.allow_portgroups(): raise exception.NotFound() + rpc_portgroup, rpc_node = api_utils.check_port_policy_and_retrieve( + 'baremetal:portgroup:delete', portgroup_ident, portgroup=True) + context = api.request.context - api_utils.check_policy('baremetal:portgroup:delete') if self.parent_node_ident: raise exception.OperationNotPermitted() - rpc_portgroup = api_utils.get_rpc_portgroup_with_suffix( - portgroup_ident) - rpc_node = objects.Node.get_by_id(api.request.context, - rpc_portgroup.node_id) - notify.emit_start_notification(context, rpc_portgroup, 'delete', node_uuid=rpc_node.uuid) with notify.handle_error_notification(context, rpc_portgroup, 'delete', diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 4bee978665..b618b2f455 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -275,6 +275,13 @@ def replace_node_uuid_with_id(to_dict): node = objects.Node.get_by_uuid(api.request.context, to_dict.pop('node_uuid')) to_dict['node_id'] = node.id + # if they cannot get the node, then this will error + # helping guard access to all users of this method as + # users which may have rights at a minimum need to be able + # to see the node they are trying to do something with. + check_owner_policy('node', 'baremetal:node:get', node['owner'], + node['lessee'], conceal_node=node.uuid) + except exception.NodeNotFound as e: # Change error code because 404 (NotFound) is inappropriate # response for requests acting on non-nodes @@ -1646,52 +1653,137 @@ def check_list_policy(object_type, owner=None): return owner -def check_port_policy_and_retrieve(policy_name, port_uuid): +def check_port_policy_and_retrieve(policy_name, port_ident, portgroup=False): """Check if the specified policy authorizes this request on a port. :param: policy_name: Name of the policy to check. - :param: port_uuid: the UUID of a port. + :param: port_ident: The name, uuid, or other valid ID value to find + a port or portgroup by. :raises: HTTPForbidden if the policy forbids access. :raises: NodeNotFound if the node is not found. - :return: RPC port identified by port_uuid and associated node + :return: RPC port identified by port_ident associated node """ context = api.request.context cdict = context.to_policy_values() - + owner = None + lessee = None try: - rpc_port = objects.Port.get_by_uuid(context, port_uuid) - except exception.PortNotFound: + if not portgroup: + rpc_port = objects.Port.get(context, port_ident) + else: + rpc_port = objects.Portgroup.get(context, port_ident) + except (exception.PortNotFound, exception.PortgroupNotFound): # don't expose non-existence of port unless requester # has generic access to policy - policy.authorize(policy_name, cdict, context) raise - rpc_node = objects.Node.get_by_id(context, rpc_port.node_id) target_dict = dict(cdict) - target_dict['node.owner'] = rpc_node['owner'] - target_dict['node.lessee'] = rpc_node['lessee'] + try: + rpc_node = objects.Node.get_by_id(context, rpc_port.node_id) + owner = rpc_node['owner'] + lessee = rpc_node['lessee'] + except exception.NodeNotFound: + # There is no spoon, err, node. + rpc_node = None + pass + target_dict = dict(cdict) + target_dict['node.owner'] = owner + target_dict['node.lessee'] = lessee + try: + policy.authorize('baremetal:node:get', target_dict, context) + except exception.NotAuthorized: + if not portgroup: + raise exception.PortNotFound(port=port_ident) + else: + raise exception.PortgroupNotFound(portgroup=port_ident) + policy.authorize(policy_name, target_dict, context) return rpc_port, rpc_node -def check_port_list_policy(): +def check_port_list_policy(portgroup=False, parent_node=None, + parent_portgroup=None): """Check if the specified policy authorizes this request on a port. + :param portgroup: Boolean value, default false, indicating if the list + policy check is for a portgroup as the policy names + are different between ports and portgroups. + :param parent_node: The UUID of a node, if any, to apply a policy + check to as well before applying other policy + check operations. + :param parent_portgroup: The UUID of the parent portgroup if the list + of ports was retrieved via the + /v1/portgroups//ports. + :raises: HTTPForbidden if the policy forbids access. :return: owner that should be used for list query, if needed """ + cdict = api.request.context.to_policy_values() + + # No node is associated with this request, yet. + rpc_node = None + conceal_linked_node = None + + if parent_portgroup: + # lookup the portgroup via the db, and then set parent_node + rpc_portgroup = objects.Portgroup.get_by_uuid(api.request.context, + parent_portgroup) + rpc_node = objects.Node.get_by_id(api.request.context, + rpc_portgroup.node_id) + parent_node = rpc_node.uuid + + if parent_node and not rpc_node: + try: + rpc_node = objects.Node.get_by_uuid(api.request.context, + parent_node) + conceal_linked_node = rpc_node.uuid + except exception.NotFound: + # NOTE(TheJulia): This only covers portgroups since + # you can't go from ports to other items. + raise exception.PortgroupNotFound(portgroup=parent_portgroup) + + if parent_node: + try: + check_owner_policy( + 'node', 'baremetal:node:get', + rpc_node.owner, rpc_node.lessee, + conceal_node=conceal_linked_node) + except exception.NotAuthorized: + if parent_portgroup: + # If this call was invoked with a parent portgroup + # then we need to signal the parent portgroup was not + # found. + raise exception.PortgroupNotFound( + portgroup=parent_portgroup) + if parent_node: + # This should likely never be hit, because + # the existence of a parent node should + # trigger the node not found exception to be + # explicitly raised. + raise exception.NodeNotFound( + node=parent_node) + raise + try: - policy.authorize('baremetal:port:list_all', - cdict, api.request.context) + if not portgroup: + policy.authorize('baremetal:port:list_all', + cdict, api.request.context) + else: + policy.authorize('baremetal:portgroup:list_all', + cdict, api.request.context) except exception.HTTPForbidden: owner = cdict.get('project_id') if not owner: raise - policy.authorize('baremetal:port:list', - cdict, api.request.context) + if not portgroup: + policy.authorize('baremetal:port:list', + cdict, api.request.context) + else: + policy.authorize('baremetal:portgroup:list', + cdict, api.request.context) return owner diff --git a/ironic/common/policy.py b/ironic/common/policy.py index fa60a7edc7..10641bd4e1 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -96,6 +96,10 @@ SYSTEM_OR_OWNER_MEMBER_AND_LESSEE_ADMIN = ( '(' + SYSTEM_MEMBER + ') or (' + PROJECT_OWNER_MEMBER + ') or (' + PROJECT_LESSEE_ADMIN + ')' # noqa ) +SYSTEM_ADMIN_OR_OWNER_ADMIN = ( + '(' + SYSTEM_ADMIN + ') or (' + PROJECT_OWNER_ADMIN + ')' +) + SYSTEM_MEMBER_OR_OWNER_ADMIN = ( '(' + SYSTEM_MEMBER + ') or (' + PROJECT_OWNER_ADMIN + ')' ) @@ -906,8 +910,8 @@ The baremetal port API is now aware of system scope and default roles. port_policies = [ policy.DocumentedRuleDefault( name='baremetal:port:get', - check_str=SYSTEM_READER, - scope_types=['system'], + check_str=SYSTEM_OR_PROJECT_READER, + scope_types=['system', 'project'], description='Retrieve Port records', operations=[ {'path': '/ports/{port_id}', 'method': 'GET'}, @@ -923,8 +927,8 @@ port_policies = [ ), policy.DocumentedRuleDefault( name='baremetal:port:list', - check_str=SYSTEM_READER, - scope_types=['system'], + check_str=API_READER, + scope_types=['system', 'project'], description='Retrieve multiple Port records, filtered by owner', operations=[ {'path': '/ports', 'method': 'GET'}, @@ -937,7 +941,7 @@ port_policies = [ policy.DocumentedRuleDefault( name='baremetal:port:list_all', check_str=SYSTEM_READER, - scope_types=['system'], + scope_types=['system', 'project'], description='Retrieve multiple Port records', operations=[ {'path': '/ports', 'method': 'GET'}, @@ -949,8 +953,8 @@ port_policies = [ ), policy.DocumentedRuleDefault( name='baremetal:port:create', - check_str=SYSTEM_ADMIN, - scope_types=['system'], + check_str=SYSTEM_ADMIN_OR_OWNER_ADMIN, + scope_types=['system', 'project'], description='Create Port records', operations=[{'path': '/ports', 'method': 'POST'}], deprecated_rule=deprecated_port_create, @@ -959,8 +963,8 @@ port_policies = [ ), policy.DocumentedRuleDefault( name='baremetal:port:delete', - check_str=SYSTEM_ADMIN, - scope_types=['system'], + check_str=SYSTEM_ADMIN_OR_OWNER_ADMIN, + scope_types=['system', 'project'], description='Delete Port records', operations=[{'path': '/ports/{port_id}', 'method': 'DELETE'}], deprecated_rule=deprecated_port_delete, @@ -969,8 +973,8 @@ port_policies = [ ), policy.DocumentedRuleDefault( name='baremetal:port:update', - check_str=SYSTEM_MEMBER, - scope_types=['system'], + check_str=SYSTEM_MEMBER_OR_OWNER_ADMIN, + scope_types=['system', 'project'], description='Update Port records', operations=[{'path': '/ports/{port_id}', 'method': 'PATCH'}], deprecated_rule=deprecated_port_update, @@ -1002,8 +1006,8 @@ The baremetal port groups API is now aware of system scope and default roles. portgroup_policies = [ policy.DocumentedRuleDefault( name='baremetal:portgroup:get', - check_str=SYSTEM_READER, - scope_types=['system'], + check_str=SYSTEM_OR_PROJECT_READER, + scope_types=['system', 'project'], description='Retrieve Portgroup records', operations=[ {'path': '/portgroups', 'method': 'GET'}, @@ -1018,8 +1022,8 @@ portgroup_policies = [ ), policy.DocumentedRuleDefault( name='baremetal:portgroup:create', - check_str=SYSTEM_ADMIN, - scope_types=['system'], + check_str=SYSTEM_ADMIN_OR_OWNER_ADMIN, + scope_types=['system', 'project'], description='Create Portgroup records', operations=[{'path': '/portgroups', 'method': 'POST'}], deprecated_rule=deprecated_portgroup_create, @@ -1028,8 +1032,8 @@ portgroup_policies = [ ), policy.DocumentedRuleDefault( name='baremetal:portgroup:delete', - check_str=SYSTEM_ADMIN, - scope_types=['system'], + check_str=SYSTEM_ADMIN_OR_OWNER_ADMIN, + scope_types=['system', 'project'], description='Delete Portgroup records', operations=[ {'path': '/portgroups/{portgroup_ident}', 'method': 'DELETE'} @@ -1040,8 +1044,8 @@ portgroup_policies = [ ), policy.DocumentedRuleDefault( name='baremetal:portgroup:update', - check_str=SYSTEM_MEMBER, - scope_types=['system'], + check_str=SYSTEM_MEMBER_OR_OWNER_ADMIN, + scope_types=['system', 'project'], description='Update Portgroup records', operations=[ {'path': '/portgroups/{portgroup_ident}', 'method': 'PATCH'} @@ -1050,6 +1054,32 @@ portgroup_policies = [ deprecated_reason=deprecated_portgroup_reason, deprecated_since=versionutils.deprecated.WALLABY ), + policy.DocumentedRuleDefault( + name='baremetal:portgroup:list', + check_str=API_READER, + scope_types=['system', 'project'], + description='Retrieve multiple Port records, filtered by owner', + operations=[ + {'path': '/portgroups', 'method': 'GET'}, + {'path': '/portgroups/detail', 'method': 'GET'} + ], + deprecated_rule=deprecated_portgroup_get, + deprecated_reason=deprecated_portgroup_reason, + deprecated_since=versionutils.deprecated.WALLABY + ), + policy.DocumentedRuleDefault( + name='baremetal:portgroup:list_all', + check_str=SYSTEM_READER, + scope_types=['system', 'project'], + description='Retrieve multiple Port records', + operations=[ + {'path': '/portgroups', 'method': 'GET'}, + {'path': '/portgroups/detail', 'method': 'GET'} + ], + deprecated_rule=deprecated_portgroup_get, + deprecated_reason=deprecated_portgroup_reason, + deprecated_since=versionutils.deprecated.WALLABY + ), ] @@ -1714,7 +1744,9 @@ def authorize(rule, target, creds, *args, **kwargs): try: return enforcer.authorize(rule, target, creds, do_raise=True, *args, **kwargs) - except policy.PolicyNotAuthorized: + except policy.PolicyNotAuthorized as e: + LOG.error('Rejecting authorzation: %(error)s', + {'error': e}) raise exception.HTTPForbidden(resource=rule) diff --git a/ironic/db/api.py b/ironic/db/api.py index bee805f5ca..92fab5b60f 100644 --- a/ironic/db/api.py +++ b/ironic/db/api.py @@ -344,10 +344,11 @@ class Connection(object, metaclass=abc.ABCMeta): """ @abc.abstractmethod - def get_portgroup_by_address(self, address): + def get_portgroup_by_address(self, address, project=None): """Return a network portgroup representation. :param address: The MAC address of a portgroup. + :param project: A node owner or lessee to filter by. :returns: A portgroup. :raises: PortgroupNotFound """ @@ -363,7 +364,8 @@ class Connection(object, metaclass=abc.ABCMeta): @abc.abstractmethod def get_portgroup_list(self, limit=None, marker=None, - sort_key=None, sort_dir=None): + sort_key=None, sort_dir=None, + project=None): """Return a list of portgroups. :param limit: Maximum number of portgroups to return. @@ -372,12 +374,14 @@ class Connection(object, metaclass=abc.ABCMeta): :param sort_key: Attribute by which results should be sorted. :param sort_dir: Direction in which results should be sorted. (asc, desc) + :param project: A node owner or lessee to filter by. :returns: A list of portgroups. """ @abc.abstractmethod def get_portgroups_by_node_id(self, node_id, limit=None, marker=None, - sort_key=None, sort_dir=None): + sort_key=None, sort_dir=None, + project=None): """List all the portgroups for a given node. :param node_id: The integer node ID. @@ -387,6 +391,7 @@ class Connection(object, metaclass=abc.ABCMeta): :param sort_key: Attribute by which results should be sorted :param sort_dir: Direction in which results should be sorted (asc, desc) + :param project: A node owner or lessee to filter by. :returns: A list of portgroups. """ diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 716fd601fd..7b5f1731bf 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -162,6 +162,13 @@ def add_port_filter_by_node_project(query, value): | (models.Node.lessee == value)) +def add_portgroup_filter_by_node_project(query, value): + query = query.join(models.Node, + models.Portgroup.node_id == models.Node.id) + return query.filter((models.Node.owner == value) + | (models.Node.lessee == value)) + + def add_portgroup_filter(query, value): """Adds a portgroup-specific filter to a query. @@ -796,8 +803,10 @@ class Connection(api.Connection): if count == 0: raise exception.PortNotFound(port=port_id) - def get_portgroup_by_id(self, portgroup_id): + def get_portgroup_by_id(self, portgroup_id, project=None): query = model_query(models.Portgroup).filter_by(id=portgroup_id) + if project: + query = add_portgroup_filter_by_node_project(query, project) try: return query.one() except NoResultFound: @@ -810,8 +819,10 @@ class Connection(api.Connection): except NoResultFound: raise exception.PortgroupNotFound(portgroup=portgroup_uuid) - def get_portgroup_by_address(self, address): + def get_portgroup_by_address(self, address, project=None): query = model_query(models.Portgroup).filter_by(address=address) + if project: + query = add_portgroup_filter_by_node_project(query, project) try: return query.one() except NoResultFound: @@ -825,14 +836,19 @@ class Connection(api.Connection): raise exception.PortgroupNotFound(portgroup=name) def get_portgroup_list(self, limit=None, marker=None, - sort_key=None, sort_dir=None): + sort_key=None, sort_dir=None, project=None): + query = model_query(models.Portgroup) + if project: + query = add_portgroup_filter_by_node_project(query, project) return _paginate_query(models.Portgroup, limit, marker, - sort_key, sort_dir) + sort_key, sort_dir, query) def get_portgroups_by_node_id(self, node_id, limit=None, marker=None, - sort_key=None, sort_dir=None): + sort_key=None, sort_dir=None, project=None): query = model_query(models.Portgroup) query = query.filter_by(node_id=node_id) + if project: + query = add_portgroup_filter_by_node_project(query, project) return _paginate_query(models.Portgroup, limit, marker, sort_key, sort_dir, query) diff --git a/ironic/objects/portgroup.py b/ironic/objects/portgroup.py index 4c6a763a29..8628df731e 100644 --- a/ironic/objects/portgroup.py +++ b/ironic/objects/portgroup.py @@ -152,17 +152,19 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): # Implications of calling new remote procedures should be thought through. # @object_base.remotable_classmethod @classmethod - def get_by_address(cls, context, address): + def get_by_address(cls, context, address, project=None): """Find portgroup by address and return a :class:`Portgroup` object. :param cls: the :class:`Portgroup` :param context: Security context :param address: The MAC address of a portgroup. + :param project: a node owner or lessee to match against. :returns: A :class:`Portgroup` object. :raises: PortgroupNotFound """ - db_portgroup = cls.dbapi.get_portgroup_by_address(address) + db_portgroup = cls.dbapi.get_portgroup_by_address(address, + project=project) portgroup = cls._from_db_object(context, cls(), db_portgroup) return portgroup @@ -191,7 +193,7 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): # @object_base.remotable_classmethod @classmethod def list(cls, context, limit=None, marker=None, - sort_key=None, sort_dir=None): + sort_key=None, sort_dir=None, project=None): """Return a list of Portgroup objects. :param cls: the :class:`Portgroup` @@ -200,6 +202,7 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): :param marker: Pagination marker for large data sets. :param sort_key: Column to sort results by. :param sort_dir: Direction to sort. "asc" or "desc". + :param project: a node owner or lessee to match against. :returns: A list of :class:`Portgroup` object. :raises: InvalidParameterValue @@ -207,7 +210,8 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): db_portgroups = cls.dbapi.get_portgroup_list(limit=limit, marker=marker, sort_key=sort_key, - sort_dir=sort_dir) + sort_dir=sort_dir, + project=project) return cls._from_db_object_list(context, db_portgroups) # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable @@ -216,7 +220,7 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): # @object_base.remotable_classmethod @classmethod def list_by_node_id(cls, context, node_id, limit=None, marker=None, - sort_key=None, sort_dir=None): + sort_key=None, sort_dir=None, project=None): """Return a list of Portgroup objects associated with a given node ID. :param cls: the :class:`Portgroup` @@ -226,6 +230,7 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): :param marker: Pagination marker for large data sets. :param sort_key: Column to sort results by. :param sort_dir: Direction to sort. "asc" or "desc". + :param project: a node owner or lessee to match against. :returns: A list of :class:`Portgroup` object. :raises: InvalidParameterValue @@ -234,7 +239,8 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): limit=limit, marker=marker, sort_key=sort_key, - sort_dir=sort_dir) + sort_dir=sort_dir, + project=project) return cls._from_db_object_list(context, db_portgroups) # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 04289aa130..c983f6d86f 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -2383,6 +2383,16 @@ class TestPatch(test_api_base.BaseApiTest): self.node_no_name = obj_utils.create_test_node( self.context, uuid='deadbeef-0000-1111-2222-333333333333', chassis_id=self.chassis.id) + self.port = obj_utils.create_test_port( + self.context, + uuid='9bb50f13-0b8d-4ade-ad2d-d91fefdef9cc', + address='00:01:02:03:04:05', + node_id=self.node.id) + self.portgroup = obj_utils.create_test_portgroup( + self.context, + uuid='9bb50f13-0b8d-4ade-ad2d-d91fefdef9ff', + address='00:00:00:00:00:ff', + node_id=self.node.id) p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for', autospec=True) self.mock_gtf = p.start() @@ -2693,7 +2703,7 @@ class TestPatch(test_api_base.BaseApiTest): def test_patch_portgroups_subresource(self): response = self.patch_json( - '/nodes/%s/portgroups/9bb50f13-0b8d-4ade-ad2d-d91fefdef9cc' % + '/nodes/%s/portgroups/9bb50f13-0b8d-4ade-ad2d-d91fefdef9ff' % self.node.uuid, [{'path': '/extra/foo', 'value': 'bar', 'op': 'add'}], expect_errors=True, diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index 23e57e17b4..5826af89b8 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -891,14 +891,18 @@ class TestNodeIdent(base.TestCase): self.assertRaises(exception.NodeNotFound, utils.populate_node_uuid, port, d) + @mock.patch.object(utils, 'check_owner_policy', autospec=True) @mock.patch.object(objects.Node, 'get_by_uuid', autospec=True) - def test_replace_node_uuid_with_id(self, mock_gbu, mock_pr): + def test_replace_node_uuid_with_id(self, mock_gbu, mock_check, mock_pr): node = obj_utils.get_test_node(self.context, id=1) mock_gbu.return_value = node to_dict = {'node_uuid': self.valid_uuid} self.assertEqual(node, utils.replace_node_uuid_with_id(to_dict)) self.assertEqual({'node_id': 1}, to_dict) + mock_check.assert_called_once_with('node', 'baremetal:node:get', + None, None, + conceal_node=mock.ANY) @mock.patch.object(objects.Node, 'get_by_uuid', autospec=True) def test_replace_node_uuid_with_id_not_found(self, mock_gbu, mock_pr): @@ -1507,8 +1511,12 @@ class TestCheckPortPolicyAndRetrieve(base.TestCase): mock_pgbu.assert_called_once_with(mock_pr.context, self.valid_port_uuid) mock_ngbi.assert_called_once_with(mock_pr.context, 42) - mock_authorize.assert_called_once_with( - 'fake_policy', expected_target, fake_context) + expected = [ + mock.call('baremetal:node:get', expected_target, fake_context), + mock.call('fake_policy', expected_target, fake_context)] + + mock_authorize.assert_has_calls(expected) + self.assertEqual(self.port, rpc_port) self.assertEqual(self.node, rpc_node) @@ -1524,7 +1532,7 @@ class TestCheckPortPolicyAndRetrieve(base.TestCase): port=self.valid_port_uuid) self.assertRaises( - exception.HTTPForbidden, + exception.PortNotFound, utils.check_port_policy_and_retrieve, 'fake-policy', self.valid_port_uuid @@ -1551,7 +1559,7 @@ class TestCheckPortPolicyAndRetrieve(base.TestCase): @mock.patch.object(policy, 'authorize', spec=True) @mock.patch.object(objects.Port, 'get_by_uuid', autospec=True) @mock.patch.object(objects.Node, 'get_by_id', autospec=True) - def test_check_port_policy_and_retrieve_policy_forbidden( + def test_check_port_policy_and_retrieve_policy_notfound( self, mock_ngbi, mock_pgbu, mock_authorize, mock_pr ): mock_pr.version.minor = 50 @@ -1561,7 +1569,7 @@ class TestCheckPortPolicyAndRetrieve(base.TestCase): mock_ngbi.return_value = self.node self.assertRaises( - exception.HTTPForbidden, + exception.PortNotFound, utils.check_port_policy_and_retrieve, 'fake-policy', self.valid_port_uuid diff --git a/ironic/tests/unit/api/test_acl.py b/ironic/tests/unit/api/test_acl.py index 940583825d..4bbb1ff38e 100644 --- a/ironic/tests/unit/api/test_acl.py +++ b/ironic/tests/unit/api/test_acl.py @@ -160,14 +160,12 @@ class TestACLBase(base.BaseApiTest): or '403' in response.status) and cfg.CONF.oslo_policy.enforce_scope and cfg.CONF.oslo_policy.enforce_new_defaults): - # NOTE(TheJulia): Everything, once migrated, should - # return a 403. self.assertEqual(assert_status, response.status_int) else: self.assertTrue( - '404' in response.status - or '500' in response.status - or '403' in response.status) + ('404' in response.status + or '500' in response.status + or '403' in response.status)) # We can't check the contents of the response if there is no # response. return @@ -258,6 +256,7 @@ class TestRBACModelBeforeScopesBase(TestACLBase): node_id=fake_db_node['id'], internal_info={'tenant_vif_port_id': fake_vif_port_id}) fake_db_portgroup = db_utils.create_test_portgroup( + uuid="6eb02b44-18a3-4659-8c0b-8d2802581ae4", node_id=fake_db_node['id']) fake_db_chassis = db_utils.create_test_chassis( drivers=['fake-hardware', 'fake-driverz', 'fake-driver']) @@ -371,13 +370,29 @@ class TestRBACProjectScoped(TestACLBase): owner_project_id = '70e5e25a-2ca2-4cb1-8ae8-7d8739cee205' lessee_project_id = 'f11853c7-fa9c-4db3-a477-c9d8e0dbbf13' unowned_node = db_utils.create_test_node(chassis_id=None) + # owned node - since the tests use the same node for # owner/lesse checks - db_utils.create_test_node( + owned_node = db_utils.create_test_node( uuid=owner_node_ident, owner=owner_project_id, last_error='meow', reservation='lolcats') + owned_node_port = db_utils.create_test_port( + uuid='ebe30f19-358d-41e1-8d28-fd7357a0164c', + node_id=owned_node['id'], + address='00:00:00:00:00:01') + db_utils.create_test_port( + uuid='21a3c5a7-1e14-44dc-a9dd-0c84d5477a57', + node_id=owned_node['id'], + address='00:00:00:00:00:02') + owner_pgroup = db_utils.create_test_portgroup( + uuid='b16efcf3-2990-41a1-bc1d-5e2c16f3d5fc', + node_id=owned_node['id'], + name='magicfoo', + address='01:03:09:ff:01:01') + + # Leased nodes leased_node = db_utils.create_test_node( uuid=lessee_node_ident, owner=owner_project_id, @@ -395,6 +410,19 @@ class TestRBACProjectScoped(TestACLBase): fake_trait = 'CUSTOM_MEOW' fake_vif_port_id = "0e21d58f-5de2-4956-85ff-33935ea1ca01" + # Random objects that shouldn't be project visible + other_port = db_utils.create_test_port( + uuid='abfd8dbb-1732-449a-b760-2224035c6b99', + address='00:00:00:00:00:ff') + + other_node = db_utils.create_test_node( + uuid='573208e5-cd41-4e26-8f06-ef44022b3793') + other_pgroup = db_utils.create_test_portgroup( + uuid='5810f41c-6585-41fc-b9c9-a94f50d421b5', + node_id=other_node['id'], + name='corgis_rule_the_world', + address='ff:ff:ff:ff:ff:0f') + self.format_data.update({ 'node_ident': unowned_node['uuid'], 'owner_node_ident': owner_node_ident, @@ -402,12 +430,16 @@ class TestRBACProjectScoped(TestACLBase): 'allocated_node_ident': lessee_node_ident, 'volume_target_ident': fake_db_volume_target['uuid'], 'volume_connector_ident': fake_db_volume_connector['uuid'], - 'port_ident': fake_db_port['uuid'], - 'portgroup_ident': fake_db_portgroup['uuid'], + 'lessee_port_ident': fake_db_port['uuid'], + 'lessee_portgroup_ident': fake_db_portgroup['uuid'], 'trait': fake_trait, 'vif_ident': fake_vif_port_id, 'ind_component': 'component', - 'ind_ident': 'magic_light'}) + 'ind_ident': 'magic_light', + 'owner_port_ident': owned_node_port['uuid'], + 'other_port_ident': other_port['uuid'], + 'owner_portgroup_ident': owner_pgroup['uuid'], + 'other_portgroup_ident': other_pgroup['uuid']}) @ddt.file_data('test_rbac_project_scoped.yaml') @ddt.unpack diff --git a/ironic/tests/unit/api/test_rbac_legacy.yaml b/ironic/tests/unit/api/test_rbac_legacy.yaml index 5882510026..6a3c5127b1 100644 --- a/ironic/tests/unit/api/test_rbac_legacy.yaml +++ b/ironic/tests/unit/api/test_rbac_legacy.yaml @@ -927,7 +927,7 @@ portgroups_portgroup_ident_get_member: path: '/v1/portgroups/{portgroup_ident}' method: get headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true portgroups_portgroup_ident_get_observer: @@ -953,7 +953,7 @@ portgroups_portgroup_ident_patch_member: method: patch headers: *member_headers body: *portgroup_patch_body - assert_status: 403 + assert_status: 404 deprecated: true portgroups_portgroup_ident_patch_observer: @@ -975,7 +975,7 @@ portgroups_portgroup_ident_delete_member: path: '/v1/portgroups/{portgroup_ident}' method: delete headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true portgroups_portgroup_ident_delete_observer: @@ -998,7 +998,7 @@ nodes_portgroups_get_member: path: '/v1/nodes/{node_ident}/portgroups' method: get headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true nodes_portgroups_get_observer: @@ -1019,7 +1019,7 @@ nodes_portgroups_detail_get_member: path: '/v1/nodes/{node_ident}/portgroups/detail' method: get headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true nodes_portgroups_detail_get_observer: @@ -1113,7 +1113,7 @@ ports_port_id_get_member: path: '/v1/ports/{port_ident}' method: get headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true ports_port_id_get_observer: @@ -1138,7 +1138,7 @@ ports_port_id_patch_member: path: '/v1/ports/{port_ident}' method: patch headers: *member_headers - assert_status: 403 + assert_status: 404 body: *port_patch_body deprecated: true @@ -1161,7 +1161,7 @@ ports_port_id_delete_member: path: '/v1/ports/{port_ident}' method: delete headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true ports_port_id_delete_observer: @@ -1184,7 +1184,7 @@ nodes_ports_get_member: path: '/v1/nodes/{node_ident}/ports' method: get headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true nodes_ports_get_observer: @@ -1205,7 +1205,7 @@ nodes_ports_detail_get_member: path: '/v1/nodes/{node_ident}/ports/detail' method: get headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true nodes_ports_detail_get_observer: @@ -1228,7 +1228,7 @@ portgroups_ports_get_member: path: '/v1/portgroups/{portgroup_ident}/ports' method: get headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true portgroups_ports_get_observer: @@ -1249,7 +1249,7 @@ portgroups_ports_detail_get_member: path: '/v1/portgroups/{portgroup_ident}/ports/detail' method: get headers: *member_headers - assert_status: 403 + assert_status: 404 deprecated: true portgroups_ports_detail_get_observer: diff --git a/ironic/tests/unit/api/test_rbac_project_scoped.yaml b/ironic/tests/unit/api/test_rbac_project_scoped.yaml index dfe28ac63e..475e07e4f9 100644 --- a/ironic/tests/unit/api/test_rbac_project_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_project_scoped.yaml @@ -69,7 +69,7 @@ values: owner_project_id: &owner_project_id 70e5e25a-2ca2-4cb1-8ae8-7d8739cee205 lessee_project_id: &lessee_project_id f11853c7-fa9c-4db3-a477-c9d8e0dbbf13 owned_node_ident: &owned_node_ident f11853c7-fa9c-4db3-a477-c9d8e0dbbf13 - lessee_node_ident: &lessee_node_ident + lessee_node_ident: &lessee_node_ident 38d5abed-c585-4fce-a57e-a2ffc2a2ec6f # Nodes - https://docs.openstack.org/api-ref/baremetal/?expanded=#nodes-nodes @@ -1493,44 +1493,42 @@ owner_reader_can_list_portgroups: method: get headers: *owner_reader_headers assert_status: 200 - skip_reason: policy not implemented + assert_list_length: + portgroups: 2 lessee_reader_can_list_portgroups: path: '/v1/portgroups' method: get headers: *lessee_reader_headers assert_status: 200 - skip_reason: policy not implemented + assert_list_length: + portgroups: 1 third_party_admin_cannot_list_portgroups: path: '/v1/portgroups' method: get headers: *third_party_admin_headers - assert_status: 99 # TODO This should be 200 + assert_status: 200 assert_list_length: portgroups: 0 - skip_reason: policy not implemented owner_reader_can_read_portgroup: path: '/v1/portgroups/{owner_portgroup_ident}' method: get headers: *owner_reader_headers assert_status: 200 - skip_reason: policy not implemented lessee_reader_can_read_portgroup: path: '/v1/portgroups/{lessee_portgroup_ident}' method: get headers: *lessee_reader_headers assert_status: 200 - skip_reason: policy not implemented third_party_admin_cannot_read_portgroup: - path: '/v1/portgroups/{portgroup_ident}' + path: '/v1/portgroups/{owner_portgroup_ident}' method: get headers: *third_party_admin_headers - assert_status: 200 - skip_reason: policy not implemented + assert_status: 404 # NB: Ports have to be posted with a node UUID to associate to, # so that seems policy-check-able. @@ -1539,9 +1537,8 @@ owner_admin_can_add_portgroup: method: post headers: *owner_admin_headers body: &owner_portgroup_body - node_uuid: 18a552fb-dcd2-43bf-9302-e4c93287be11 + node_uuid: 1ab63b9e-66d7-4cd7-8618-dddd0f9f7881 assert_status: 201 - skip_reason: policy not implemented owner_member_cannot_add_portgroup: path: '/v1/portgroups' @@ -1549,16 +1546,14 @@ owner_member_cannot_add_portgroup: headers: *owner_member_headers body: *owner_portgroup_body assert_status: 403 - skip_reason: policy not implemented lessee_admin_cannot_add_portgroup: path: '/v1/portgroups' method: post headers: *lessee_admin_headers body: &lessee_portgroup_body - node_uuid: 18a552fb-dcd2-43bf-9302-e4c93287be11 + node_uuid: 38d5abed-c585-4fce-a57e-a2ffc2a2ec6f assert_status: 403 - skip_reason: policy not implemented # TODO, likely will need separate port/port groups established for the tests @@ -1568,7 +1563,6 @@ lessee_member_cannot_add_portgroup: headers: *lessee_member_headers body: *lessee_portgroup_body assert_status: 403 - skip_reason: policy not implemented third_party_admin_cannot_add_portgroup: path: '/v1/portgroups' @@ -1576,7 +1570,6 @@ third_party_admin_cannot_add_portgroup: headers: *third_party_admin_headers body: *lessee_portgroup_body assert_status: 403 - skip_reason: policy not implemented owner_admin_can_modify_portgroup: path: '/v1/portgroups/{owner_portgroup_ident}' @@ -1587,15 +1580,13 @@ owner_admin_can_modify_portgroup: path: /extra value: {'test': 'testing'} assert_status: 503 - skip_reason: policy not implemented -owner_member_can_modify_portgroup: +owner_member_cannot_modify_portgroup: path: '/v1/portgroups/{owner_portgroup_ident}' method: patch headers: *owner_member_headers body: *portgroup_patch_body - assert_status: 503 - skip_reason: policy not implemented + assert_status: 403 lessee_admin_cannot_modify_portgroup: path: '/v1/portgroups/{lessee_portgroup_ident}' @@ -1603,7 +1594,6 @@ lessee_admin_cannot_modify_portgroup: headers: *lessee_admin_headers body: *portgroup_patch_body assert_status: 403 - skip_reason: policy not implemented lessee_member_cannot_modify_portgroup: path: '/v1/portgroups/{lessee_portgroup_ident}' @@ -1611,50 +1601,43 @@ lessee_member_cannot_modify_portgroup: headers: *lessee_member_headers body: *portgroup_patch_body assert_status: 403 - skip_reason: policy not implemented third_party_admin_cannot_modify_portgroup: path: '/v1/portgroups/{lessee_portgroup_ident}' method: patch headers: *third_party_admin_headers body: *portgroup_patch_body - assert_status: 403 - skip_reason: policy not implemented + assert_status: 404 owner_admin_can_delete_portgroup: path: '/v1/portgroups/{owner_portgroup_ident}' method: delete headers: *owner_admin_headers assert_status: 503 - skip_reason: policy not implemented owner_member_cannot_delete_portgroup: path: '/v1/portgroups/{owner_portgroup_ident}' method: delete headers: *owner_member_headers assert_status: 403 - skip_reason: policy not implemented lessee_admin_cannot_delete_portgroup: path: '/v1/portgroups/{lessee_portgroup_ident}' method: delete headers: *lessee_admin_headers assert_status: 403 - skip_reason: policy not implemented lessee_member_cannot_delete_portgroup: path: '/v1/portgroups/{lessee_portgroup_ident}' method: delete headers: *lessee_member_headers assert_status: 403 - skip_reason: policy not implemented third_party_admin_cannot_delete_portgroup: path: '/v1/portgroups/{lessee_portgroup_ident}' method: delete headers: *third_party_admin_headers - assert_status: 403 - skip_reason: policy not implemented + assert_status: 404 # Portgroups by node - https://docs.openstack.org/api-ref/baremetal/#listing-portgroups-by-node-nodes-portgroups @@ -1662,22 +1645,19 @@ owner_reader_can_get_node_portgroups: path: '/v1/nodes/{owner_node_ident}/portgroups' method: get headers: *owner_reader_headers - assert_status: 403 - skip_reason: policy not implemented + assert_status: 200 lessee_reader_can_get_node_porgtroups: path: '/v1/nodes/{lessee_node_ident}/portgroups' method: get headers: *lessee_reader_headers - assert_status: 403 - skip_reason: policy not implemented + assert_status: 200 third_party_admin_cannot_get_portgroups: path: '/v1/nodes/{lessee_node_ident}/portgroups' method: get headers: *third_party_admin_headers - assert_status: 403 - skip_reason: policy not implemented + assert_status: 404 # Ports - https://docs.openstack.org/api-ref/baremetal/#ports-ports @@ -1688,43 +1668,43 @@ owner_reader_can_list_ports: method: get headers: *owner_reader_headers assert_status: 200 - skip_reason: policy not implemented + # Two ports owned, one on the leased node. 1 invisible. + assert_list_length: + ports: 3 lessee_reader_can_list_ports: path: '/v1/ports' method: get headers: *lessee_reader_headers assert_status: 200 - skip_reason: policy not implemented + assert_list_length: + ports: 1 third_party_admin_cannot_list_ports: path: '/v1/ports' method: get headers: *third_party_admin_headers - assert_status: 99 # TODO This should be 200 - # TODO(Assert list has zero members!) - skip_reason: policy not implemented + assert_status: 200 + assert_list_length: + ports: 0 owner_reader_can_read_port: path: '/v1/ports/{owner_port_ident}' method: get headers: *owner_reader_headers assert_status: 200 - skip_reason: policy not implemented lessee_reader_can_read_port: path: '/v1/ports/{lessee_port_ident}' method: get headers: *lessee_reader_headers assert_status: 200 - skip_reason: policy not implemented third_party_admin_cannot_read_port: - path: '/v1/ports/{port_ident}' + path: '/v1/ports/{other_port_ident}' method: get headers: *third_party_admin_headers - assert_status: 200 - skip_reason: policy not implemented + assert_status: 404 # NB: Ports have to be posted with a node UUID to associate to, # so that seems policy-check-able. @@ -1733,10 +1713,18 @@ owner_admin_can_add_ports: method: post headers: *owner_admin_headers body: &owner_port_body - node_uuid: 18a552fb-dcd2-43bf-9302-e4c93287be11 + node_uuid: 1ab63b9e-66d7-4cd7-8618-dddd0f9f7881 address: 00:01:02:03:04:05 - assert_status: 201 - skip_reason: policy not implemented + assert_status: 503 + +owner_admin_cannot_add_ports_to_other_nodes: + path: '/v1/ports' + method: post + headers: *owner_admin_headers + body: + node_uuid: 573208e5-cd41-4e26-8f06-ef44022b3793 + address: 09:01:02:03:04:09 + assert_status: 403 owner_member_cannot_add_port: path: '/v1/ports' @@ -1744,19 +1732,15 @@ owner_member_cannot_add_port: headers: *owner_member_headers body: *owner_port_body assert_status: 403 - skip_reason: policy not implemented lessee_admin_cannot_add_port: path: '/v1/ports' method: post headers: *lessee_admin_headers body: &lessee_port_body - node_uuid: 18a552fb-dcd2-43bf-9302-e4c93287be11 + node_uuid: 38d5abed-c585-4fce-a57e-a2ffc2a2ec6f address: 00:01:02:03:04:05 assert_status: 403 - skip_reason: policy not implemented - -# TODO, likely will need separate port/port groups established for the tests lessee_member_cannot_add_port: path: '/v1/ports' @@ -1764,7 +1748,6 @@ lessee_member_cannot_add_port: headers: *lessee_member_headers body: *lessee_port_body assert_status: 403 - skip_reason: policy not implemented third_party_admin_cannot_add_port: path: '/v1/ports' @@ -1772,7 +1755,6 @@ third_party_admin_cannot_add_port: headers: *third_party_admin_headers body: *lessee_port_body assert_status: 403 - skip_reason: policy not implemented owner_admin_can_modify_port: path: '/v1/ports/{owner_port_ident}' @@ -1783,15 +1765,13 @@ owner_admin_can_modify_port: path: /extra value: {'test': 'testing'} assert_status: 503 - skip_reason: policy not implemented -owner_member_can_modify_port: +owner_member_cannot_modify_port: path: '/v1/ports/{owner_port_ident}' method: patch headers: *owner_member_headers body: *port_patch_body - assert_status: 503 - skip_reason: policy not implemented + assert_status: 403 lessee_admin_cannot_modify_port: path: '/v1/ports/{lessee_port_ident}' @@ -1799,7 +1779,6 @@ lessee_admin_cannot_modify_port: headers: *lessee_admin_headers body: *port_patch_body assert_status: 403 - skip_reason: policy not implemented lessee_member_cannot_modify_port: path: '/v1/ports/{lessee_port_ident}' @@ -1807,50 +1786,43 @@ lessee_member_cannot_modify_port: headers: *lessee_member_headers body: *port_patch_body assert_status: 403 - skip_reason: policy not implemented third_party_admin_cannot_modify_port: path: '/v1/ports/{lessee_port_ident}' method: patch headers: *third_party_admin_headers body: *port_patch_body - assert_status: 403 - skip_reason: policy not implemented + assert_status: 404 owner_admin_can_delete_port: path: '/v1/ports/{owner_port_ident}' method: delete headers: *owner_admin_headers assert_status: 503 - skip_reason: policy not implemented owner_member_cannot_delete_port: path: '/v1/ports/{owner_port_ident}' method: delete headers: *owner_member_headers assert_status: 403 - skip_reason: policy not implemented lessee_admin_cannot_delete_port: path: '/v1/ports/{lessee_port_ident}' method: delete headers: *lessee_admin_headers assert_status: 403 - skip_reason: policy not implemented lessee_member_cannot_delete_port: path: '/v1/ports/{lessee_port_ident}' method: delete headers: *lessee_member_headers assert_status: 403 - skip_reason: policy not implemented third_party_admin_cannot_delete_port: path: '/v1/ports/{lessee_port_ident}' method: delete headers: *third_party_admin_headers - assert_status: 403 - skip_reason: policy not implemented + assert_status: 404 # Ports by node - https://docs.openstack.org/api-ref/baremetal/#listing-ports-by-node-nodes-ports @@ -1858,47 +1830,45 @@ owner_reader_can_get_node_ports: path: '/v1/nodes/{owner_node_ident}/ports' method: get headers: *owner_reader_headers - assert_status: 403 - skip_reason: policy not implemented + assert_status: 200 + assert_list_length: + ports: 2 -lessee_reader_can_get_node_porgtroups: +lessee_reader_can_get_node_port: path: '/v1/nodes/{lessee_node_ident}/ports' method: get headers: *lessee_reader_headers - assert_status: 403 - skip_reason: policy not implemented + assert_status: 200 + assert_list_length: + ports: 1 third_party_admin_cannot_get_ports: path: '/v1/nodes/{lessee_node_ident}/ports' method: get headers: *third_party_admin_headers - assert_status: 403 - skip_reason: policy not implemented + assert_status: 404 # Ports by portgroup - https://docs.openstack.org/api-ref/baremetal/#listing-ports-by-portgroup-portgroup-ports -# Based on potgroups_ports_get* tests +# Based on portgroups_ports_get* tests owner_reader_can_get_ports_by_portgroup: path: '/v1/portgroups/{owner_portgroup_ident}/ports' method: get headers: *owner_reader_headers assert_status: 200 - skip_reason: policy not implemented lessee_reader_can_get_ports_by_portgroup: path: '/v1/portgroups/{lessee_portgroup_ident}/ports' method: get headers: *lessee_reader_headers assert_status: 200 - skip_reason: policy not implemented third_party_admin_cannot_get_ports_by_portgroup: path: '/v1/portgroups/{other_portgroup_ident}/ports' method: get headers: *third_party_admin_headers - assert_status: 403 - skip_reason: policy not implemented + assert_status: 404 # Volume(s) - https://docs.openstack.org/api-ref/baremetal/#volume-volume # TODO(TheJulia): volumes will likely need some level of exhaustive testing. diff --git a/ironic/tests/unit/api/test_rbac_system_scoped.yaml b/ironic/tests/unit/api/test_rbac_system_scoped.yaml index 86ebfeefb2..340878c1ee 100644 --- a/ironic/tests/unit/api/test_rbac_system_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_system_scoped.yaml @@ -941,7 +941,9 @@ ports_post_member: method: post headers: *scoped_member_headers assert_status: 403 - body: *port_body + body: + node_uuid: 22e26c0b-03f2-4d2e-ae87-c02d7f33c000 + address: 03:04:05:06:07:08 ports_post_reader: path: '/v1/ports' diff --git a/ironic/tests/unit/objects/test_portgroup.py b/ironic/tests/unit/objects/test_portgroup.py index 29bab20d04..9c0dc788ce 100644 --- a/ironic/tests/unit/objects/test_portgroup.py +++ b/ironic/tests/unit/objects/test_portgroup.py @@ -58,7 +58,7 @@ class TestPortgroupObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn): portgroup = objects.Portgroup.get(self.context, address) - mock_get_portgroup.assert_called_once_with(address) + mock_get_portgroup.assert_called_once_with(address, project=None) self.assertEqual(self.context, portgroup._context) def test_get_by_name(self):