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):