diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 64cc041b15..a6df583cfe 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -722,24 +722,31 @@ function create_ironic_cache_dir { # create_ironic_accounts() - Set up common required ironic accounts -# Tenant User Roles +# Project User Roles # ------------------------------------------------------------------ -# service ironic admin # if enabled +# service ironic admin +# service nova baremetal_admin +# demo demo baremetal_observer function create_ironic_accounts { - - # Ironic - if [[ "$ENABLED_SERVICES" =~ "ir-api" ]]; then - # Get ironic user if exists - - # NOTE(Shrews): This user MUST have admin level privileges! - create_service_user "ironic" "admin" - + if [[ "$ENABLED_SERVICES" =~ "ir-api" && "$ENABLED_SERVICES" =~ "key" ]]; then + # Define service and endpoints in Keystone get_or_create_service "ironic" "baremetal" "Ironic baremetal provisioning service" get_or_create_endpoint "baremetal" \ "$REGION_NAME" \ "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" \ "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" \ "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" + + # Create ironic service user + # TODO(deva): make this work with the 'service' role + # https://bugs.launchpad.net/ironic/+bug/1605398 + create_service_user "ironic" "admin" + + # Create additional bare metal tenant and roles + get_or_create_role baremetal_admin + get_or_create_role baremetal_observer + get_or_add_user_project_role baremetal_admin nova $SERVICE_PROJECT_NAME + get_or_add_user_project_role baremetal_observer demo demo fi } diff --git a/etc/ironic/policy.json b/etc/ironic/policy.json index f7726778ed..1ae73ec930 100644 --- a/etc/ironic/policy.json +++ b/etc/ironic/policy.json @@ -1,5 +1,5 @@ +# Beginning with the Newton release, you may leave this file empty +# to use default policy defined in code. { - "admin_api": "role:admin or role:administrator", - "show_password": "!", - "default": "rule:admin_api" + } diff --git a/etc/ironic/policy.json.sample b/etc/ironic/policy.json.sample new file mode 100644 index 0000000000..888c0f0c0c --- /dev/null +++ b/etc/ironic/policy.json.sample @@ -0,0 +1,72 @@ +# Legacy rule for cloud admin access +"admin_api": "role:admin or role:administrator" +# Internal flag for public API routes +"public_api": "is_public_api:True" +# Show or mask passwords in API responses +"show_password": "!" +# May be used to restrict access to specific tenants +"is_member": "tenant:demo or tenant:baremetal" +# Read-only API access +"is_observer": "rule:is_member and (role:observer or role:baremetal_observer)" +# Full read/write API access +"is_admin": "rule:admin_api or (rule:is_member and role:baremetal_admin)" +# Retrieve Node records +"baremetal:node:get": "rule:is_admin or rule:is_observer" +# Retrieve Node boot device metadata +"baremetal:node:get_boot_device": "rule:is_admin or rule:is_observer" +# View Node power and provision state +"baremetal:node:get_states": "rule:is_admin or rule:is_observer" +# Create Node records +"baremetal:node:create": "rule:is_admin" +# Delete Node records +"baremetal:node:delete": "rule:is_admin" +# Update Node records +"baremetal:node:update": "rule:is_admin" +# Request active validation of Nodes +"baremetal:node:validate": "rule:is_admin" +# Set maintenance flag, taking a Node out of service +"baremetal:node:set_maintenance": "rule:is_admin" +# Clear maintenance flag, placing the Node into service again +"baremetal:node:clear_maintenance": "role:is_admin" +# Change Node boot device +"baremetal:node:set_boot_device": "rule:is_admin" +# Change Node power status +"baremetal:node:set_power_state": "rule:is_admin" +# Change Node provision status +"baremetal:node:set_provision_state": "rule:is_admin" +# Change Node RAID status +"baremetal:node:set_raid_state": "rule:is_admin" +# Get Node console connection information +"baremetal:node:get_console": "rule:is_admin" +# Change Node console status +"baremetal:node:set_console_state": "rule:is_admin" +# Retrieve Port records +"baremetal:port:get": "rule:is_admin or rule:is_observer" +# Create Port records +"baremetal:port:create": "rule:is_admin" +# Delete Port records +"baremetal:port:delete": "rule:is_admin" +# Update Port records +"baremetal:port:update": "rule:is_admin" +# Retrieve Chassis records +"baremetal:chassis:get": "rule:is_admin or rule:is_observer" +# Create Chassis records +"baremetal:chassis:create": "rule:is_admin" +# Delete Chassis records +"baremetal:chassis:delete": "rule:is_admin" +# Update Chassis records +"baremetal:chassis:update": "rule:is_admin" +# View list of available drivers +"baremetal:driver:get": "rule:is_admin or rule:is_observer" +# View driver-specific properties +"baremetal:driver:get_properties": "rule:is_admin or rule:is_observer" +# View driver-specific RAID metadata +"baremetal:driver:get_raid_logical_disk_properties": "rule:is_admin or rule:is_observer" +# Access vendor-specific Node functions +"baremetal:node:vendor_passthru": "rule:is_admin" +# Access vendor-specific Driver functions +"baremetal:driver:vendor_passthru": "rule:is_admin" +# Send heartbeats from IPA ramdisk +"baremetal:node:ipa_heartbeat": "rule:public_api" +# Access IPA ramdisk functions +"baremetal:driver:ipa_lookup": "rule:public_api" diff --git a/ironic/api/acl.py b/ironic/api/acl.py deleted file mode 100644 index ef5532b0ca..0000000000 --- a/ironic/api/acl.py +++ /dev/null @@ -1,34 +0,0 @@ -# -*- encoding: utf-8 -*- -# -# Copyright © 2012 New Dream Network, LLC (DreamHost) -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -"""Access Control Lists (ACL's) control access the API server.""" - -from ironic.api.middleware import auth_token - - -def install(app, conf, public_routes): - """Install ACL check on application. - - :param app: A WSGI application. - :param conf: Settings. Dict'ified and passed to keystonemiddleware - :param public_routes: The list of the routes which will be allowed to - access without authentication. - :return: The same WSGI application with ACL installed. - - """ - return auth_token.AuthTokenMiddleware(app, - conf=dict(conf), - public_api_routes=public_routes) diff --git a/ironic/api/app.py b/ironic/api/app.py index 5621d97595..7c2c401b7d 100644 --- a/ironic/api/app.py +++ b/ironic/api/app.py @@ -21,11 +21,11 @@ from oslo_config import cfg import oslo_middleware.cors as cors_middleware import pecan -from ironic.api import acl from ironic.api import config from ironic.api.controllers.base import Version from ironic.api import hooks from ironic.api import middleware +from ironic.api.middleware import auth_token from ironic.common import exception from ironic.conf import CONF @@ -49,9 +49,6 @@ def setup_app(pecan_config=None, extra_hooks=None): if not pecan_config: pecan_config = get_pecan_config() - if pecan_config.app.enable_acl: - app_hooks.append(hooks.TrustedCallHook()) - pecan.configuration.set_config(dict(pecan_config), overwrite=True) app = pecan.make_app( @@ -76,8 +73,10 @@ def setup_app(pecan_config=None, extra_hooks=None): reason=e ) - if pecan_config.app.enable_acl: - app = acl.install(app, cfg.CONF, pecan_config.app.acl_public_routes) + if CONF.auth_strategy == "keystone": + app = auth_token.AuthTokenMiddleware( + app, dict(cfg.CONF), + public_api_routes=pecan_config.app.acl_public_routes) # Create a CORS wrapper, and attach ironic-specific defaults that must be # included in all CORS responses. @@ -94,7 +93,6 @@ def setup_app(pecan_config=None, extra_hooks=None): class VersionSelectorApplication(object): def __init__(self): pc = get_pecan_config() - pc.app.enable_acl = (CONF.auth_strategy == 'keystone') self.v1 = setup_app(pecan_config=pc) def __call__(self, environ, start_response): diff --git a/ironic/api/config.py b/ironic/api/config.py index 9a1ff1129e..f707f5b4a2 100644 --- a/ironic/api/config.py +++ b/ironic/api/config.py @@ -26,7 +26,6 @@ app = { 'modules': ['ironic.api'], 'static_root': '%(confdir)s/public', 'debug': False, - 'enable_acl': True, 'acl_public_routes': [ '/', '/v1', diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py index e43841b5f1..f5ebab2d3c 100644 --- a/ironic/api/controllers/v1/chassis.py +++ b/ironic/api/controllers/v1/chassis.py @@ -31,6 +31,7 @@ from ironic.api.controllers.v1 import utils as api_utils from ironic.api import expose from ironic.common import exception from ironic.common.i18n import _ +from ironic.common import policy from ironic import objects METRICS = metrics_utils.get_metrics_logger(__name__) @@ -207,6 +208,9 @@ class ChassisController(rest.RestController): :param fields: Optional, a list with a specified set of fields of the resource to be returned. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:chassis:get', cdict, cdict) + api_utils.check_allow_specify_fields(fields) if fields is None: fields = _DEFAULT_RETURN_FIELDS @@ -224,6 +228,9 @@ class ChassisController(rest.RestController): :param sort_key: column to sort results by. Default: id. :param sort_dir: direction to sort. "asc" or "desc". Default: asc. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:chassis:get', cdict, cdict) + # /detail should only work against collections parent = pecan.request.path.split('/')[:-1][-1] if parent != "chassis": @@ -242,6 +249,9 @@ class ChassisController(rest.RestController): :param fields: Optional, a list with a specified set of fields of the resource to be returned. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:chassis:get', cdict, cdict) + api_utils.check_allow_specify_fields(fields) rpc_chassis = objects.Chassis.get_by_uuid(pecan.request.context, chassis_uuid) @@ -254,6 +264,9 @@ class ChassisController(rest.RestController): :param chassis: a chassis within the request body. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:chassis:create', cdict, cdict) + new_chassis = objects.Chassis(pecan.request.context, **chassis.as_dict()) new_chassis.create() @@ -270,6 +283,9 @@ class ChassisController(rest.RestController): :param chassis_uuid: UUID of a chassis. :param patch: a json PATCH document to apply to this chassis. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:chassis:update', cdict, cdict) + rpc_chassis = objects.Chassis.get_by_uuid(pecan.request.context, chassis_uuid) try: @@ -301,6 +317,9 @@ class ChassisController(rest.RestController): :param chassis_uuid: UUID of a chassis. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:chassis:delete', cdict, cdict) + rpc_chassis = objects.Chassis.get_by_uuid(pecan.request.context, chassis_uuid) rpc_chassis.destroy() diff --git a/ironic/api/controllers/v1/driver.py b/ironic/api/controllers/v1/driver.py index 4d327e9b21..fed3f9593f 100644 --- a/ironic/api/controllers/v1/driver.py +++ b/ironic/api/controllers/v1/driver.py @@ -26,6 +26,7 @@ from ironic.api.controllers.v1 import types from ironic.api.controllers.v1 import utils as api_utils from ironic.api import expose from ironic.common import exception +from ironic.common import policy METRICS = metrics_utils.get_metrics_logger(__name__) @@ -153,6 +154,9 @@ class DriverPassthruController(rest.RestController): :raises: DriverNotFound if the driver name is invalid or the driver cannot be loaded. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:driver:vendor_passthru', cdict, cdict) + if driver_name not in _VENDOR_METHODS: topic = pecan.request.rpcapi.get_topic_for_driver(driver_name) ret = pecan.request.rpcapi.get_driver_vendor_passthru_methods( @@ -172,6 +176,12 @@ class DriverPassthruController(rest.RestController): implementation. :param data: body of data to supply to the specified method. """ + cdict = pecan.request.context.to_dict() + if method == "lookup": + policy.authorize('baremetal:driver:ipa_lookup', cdict, cdict) + else: + policy.authorize('baremetal:driver:vendor_passthru', cdict, cdict) + topic = pecan.request.rpcapi.get_topic_for_driver(driver_name) return api_utils.vendor_passthru(driver_name, method, topic, data=data, driver_passthru=True) @@ -198,6 +208,10 @@ class DriverRaidController(rest.RestController): :raises: DriverNotFound, if driver is not loaded on any of the conductors. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:driver:get_raid_logical_disk_properties', + cdict, cdict) + if not api_utils.allow_raid_config(): raise exception.NotAcceptable() @@ -236,6 +250,9 @@ class DriversController(rest.RestController): # will break from a single-line doc string. # This is a result of a bug in sphinxcontrib-pecanwsme # https://github.com/dreamhost/sphinxcontrib-pecanwsme/issues/8 + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:driver:get', cdict, cdict) + driver_list = pecan.request.dbapi.get_active_driver_dict() return DriverList.convert_with_links(driver_list) @@ -247,6 +264,8 @@ class DriversController(rest.RestController): # retrieving a list of drivers using the current sqlalchemy schema, but # this path must be exposed for Pecan to route any paths we might # choose to expose below it. + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:driver:get', cdict, cdict) driver_dict = pecan.request.dbapi.get_active_driver_dict() for name, hosts in driver_dict.items(): @@ -266,6 +285,9 @@ class DriversController(rest.RestController): :raises: DriverNotFound (HTTP 404) if the driver name is invalid or the driver cannot be loaded. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:driver:get_properties', cdict, cdict) + if driver_name not in _DRIVER_PROPERTIES: topic = pecan.request.rpcapi.get_topic_for_driver(driver_name) properties = pecan.request.rpcapi.get_driver_properties( diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index d714ae8399..6792340090 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -38,6 +38,7 @@ from ironic.api.controllers.v1 import versions from ironic.api import expose from ironic.common import exception from ironic.common.i18n import _ +from ironic.common import policy from ironic.common import states as ir_states from ironic.conductor import utils as conductor_utils from ironic import objects @@ -198,6 +199,9 @@ class BootDeviceController(rest.RestController): Default: False. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:set_boot_device', cdict, cdict) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) pecan.request.rpcapi.set_boot_device(pecan.request.context, @@ -220,6 +224,9 @@ class BootDeviceController(rest.RestController): future boots or not, None if it is unknown. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:get_boot_device', cdict, cdict) + return self._get_boot_device(node_ident) @METRICS.timer('BootDeviceController.supported') @@ -232,6 +239,9 @@ class BootDeviceController(rest.RestController): devices. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:get_boot_device', cdict, cdict) + boot_devices = self._get_boot_device(node_ident, supported=True) return {'supported_boot_devices': boot_devices} @@ -267,6 +277,9 @@ class NodeConsoleController(rest.RestController): :param node_ident: UUID or logical name of a node. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:get_console', cdict, cdict) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) try: @@ -289,6 +302,9 @@ class NodeConsoleController(rest.RestController): :param enabled: Boolean value; whether to enable or disable the console. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:set_console_state', cdict, cdict) + rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) pecan.request.rpcapi.set_console_mode(pecan.request.context, @@ -377,6 +393,9 @@ class NodeStatesController(rest.RestController): :param node_ident: the UUID or logical_name of a node. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:get_states', cdict, cdict) + # NOTE(lucasagomes): All these state values come from the # DB. Ironic counts with a periodic task that verify the current # power states of the nodes and update the DB accordingly. @@ -398,6 +417,9 @@ class NodeStatesController(rest.RestController): :raises: NotAcceptable, if requested version of the API is less than 1.12. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:set_raid_state', cdict, cdict) + if not api_utils.allow_raid_config(): raise exception.NotAcceptable() rpc_node = api_utils.get_rpc_node(node_ident) @@ -426,6 +448,9 @@ class NodeStatesController(rest.RestController): state is not valid or if the node is in CLEANING state. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:set_power_state', cdict, cdict) + # TODO(lucasagomes): Test if it's able to transition to the # target state from the current one rpc_node = api_utils.get_rpc_node(node_ident) @@ -503,6 +528,9 @@ class NodeStatesController(rest.RestController): :raises: NotAcceptable (HTTP 406) if the API version specified does not allow the requested state transition. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:set_provision_state', cdict, cdict) + api_utils.check_allow_management_verbs(target) rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) @@ -903,6 +931,9 @@ class NodeVendorPassthruController(rest.RestController): entries. :raises: NodeNotFound if the node is not found. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:vendor_passthru', cdict, cdict) + # Raise an exception if node is not found rpc_node = api_utils.get_rpc_node(node_ident) @@ -924,6 +955,12 @@ class NodeVendorPassthruController(rest.RestController): :param method: name of the method in vendor driver. :param data: body of data to supply to the specified method. """ + cdict = pecan.request.context.to_dict() + if method == 'heartbeat': + policy.authorize('baremetal:node:ipa_heartbeat', cdict, cdict) + else: + policy.authorize('baremetal:node:vendor_passthru', cdict, cdict) + # Raise an exception if node is not found rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) @@ -956,6 +993,9 @@ class NodeMaintenanceController(rest.RestController): :param reason: Optional, the reason why it's in maintenance. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:set_maintenance', cdict, cdict) + self._set_maintenance(node_ident, True, reason=reason) @METRICS.timer('NodeMaintenanceController.delete') @@ -966,6 +1006,9 @@ class NodeMaintenanceController(rest.RestController): :param node_ident: the UUID or logical name of a node. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:clear_maintenance', cdict, cdict) + self._set_maintenance(node_ident, False) @@ -1169,6 +1212,9 @@ class NodesController(rest.RestController): :param fields: Optional, a list with a specified set of fields of the resource to be returned. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:get', cdict, cdict) + api_utils.check_allow_specify_fields(fields) api_utils.check_allowed_fields(fields) api_utils.check_for_invalid_state_and_allow_filter(provision_state) @@ -1215,6 +1261,9 @@ class NodesController(rest.RestController): :param resource_class: Optional string value to get only nodes with that resource_class. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:get', cdict, cdict) + api_utils.check_for_invalid_state_and_allow_filter(provision_state) api_utils.check_allow_specify_driver(driver) api_utils.check_allow_specify_resource_class(resource_class) @@ -1243,6 +1292,9 @@ class NodesController(rest.RestController): :param node: UUID or name of a node. :param node_uuid: UUID of a node. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:validate', cdict, cdict) + if node is not None: # We're invoking this interface using positional notation, or # explicitly using 'node'. Try and determine which one. @@ -1265,6 +1317,9 @@ class NodesController(rest.RestController): :param fields: Optional, a list with a specified set of fields of the resource to be returned. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:get', cdict, cdict) + if self.from_chassis: raise exception.OperationNotPermitted() @@ -1281,6 +1336,9 @@ class NodesController(rest.RestController): :param node: a node within the request body. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:create', cdict, cdict) + if self.from_chassis: raise exception.OperationNotPermitted() @@ -1345,6 +1403,9 @@ class NodesController(rest.RestController): :param node_ident: UUID or logical name of a node. :param patch: a json PATCH document to apply to this node. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:update', cdict, cdict) + if self.from_chassis: raise exception.OperationNotPermitted() @@ -1426,6 +1487,9 @@ class NodesController(rest.RestController): :param node_ident: UUID or logical name of a node. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:node:delete', cdict, cdict) + if self.from_chassis: raise exception.OperationNotPermitted() diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 90d549cdc2..e2899815d0 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -31,6 +31,7 @@ from ironic.api.controllers.v1 import utils as api_utils from ironic.api import expose from ironic.common import exception from ironic.common.i18n import _ +from ironic.common import policy from ironic import objects METRICS = metrics_utils.get_metrics_logger(__name__) @@ -308,6 +309,9 @@ class PortsController(rest.RestController): of the resource to be returned. :raises: NotAcceptable """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:port:get', cdict, cdict) + api_utils.check_allow_specify_fields(fields) if (fields and not api_utils.allow_port_advanced_net_fields() and set(fields).intersection(self.advanced_net_fields)): @@ -351,6 +355,9 @@ class PortsController(rest.RestController): :param sort_dir: direction to sort. "asc" or "desc". Default: asc. :raises: NotAcceptable, HTTPNotFound """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:port:get', cdict, cdict) + if not node_uuid and node: # We're invoking this interface using positional notation, or # explicitly using 'node'. Try and determine which one. @@ -379,6 +386,9 @@ class PortsController(rest.RestController): of the resource to be returned. :raises: NotAcceptable """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:port:get', cdict, cdict) + if self.from_nodes: raise exception.OperationNotPermitted() @@ -395,6 +405,9 @@ class PortsController(rest.RestController): :param port: a port within the request body. :raises: NotAcceptable """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:port:create', cdict, cdict) + if self.from_nodes: raise exception.OperationNotPermitted() @@ -421,6 +434,9 @@ class PortsController(rest.RestController): :param patch: a json PATCH document to apply to this port. :raises: NotAcceptable """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:port:update', cdict, cdict) + if self.from_nodes: raise exception.OperationNotPermitted() if not api_utils.allow_port_advanced_net_fields(): @@ -470,6 +486,9 @@ class PortsController(rest.RestController): :param port_uuid: UUID of a port. """ + cdict = pecan.request.context.to_dict() + policy.authorize('baremetal:port:delete', cdict, cdict) + if self.from_nodes: raise exception.OperationNotPermitted() rpc_port = objects.Port.get_by_uuid(pecan.request.context, diff --git a/ironic/api/hooks.py b/ironic/api/hooks.py index 764b0a434f..f25a05be92 100644 --- a/ironic/api/hooks.py +++ b/ironic/api/hooks.py @@ -17,7 +17,6 @@ from oslo_config import cfg from pecan import hooks from six.moves import http_client -from webob import exc from ironic.common import context from ironic.common import policy @@ -69,6 +68,7 @@ class ContextHook(hooks.PecanHook): # Do not pass any token with context for noauth mode auth_token = (None if cfg.CONF.auth_strategy == 'noauth' else headers.get('X-Auth-Token')) + is_public_api = state.request.environ.get('is_public_api', False) creds = { 'user': headers.get('X-User') or headers.get('X-User-Id'), @@ -77,16 +77,17 @@ class ContextHook(hooks.PecanHook): 'domain_name': headers.get('X-User-Domain-Name'), 'auth_token': auth_token, 'roles': headers.get('X-Roles', '').split(','), + 'is_public_api': is_public_api, } - is_admin = policy.enforce('admin_api', creds, creds) - is_public_api = state.request.environ.get('is_public_api', False) - show_password = policy.enforce('show_password', creds, creds) + # TODO(deva): refactor this so enforce is called directly at relevant + # places in code, not globally and for every request + show_password = policy.check('show_password', creds, creds) + is_admin = policy.check('is_admin', creds, creds) state.request.context = context.RequestContext( - is_admin=is_admin, - is_public_api=is_public_api, show_password=show_password, + is_admin=is_admin, **creds) def after(self, state): @@ -106,22 +107,6 @@ class RPCHook(hooks.PecanHook): state.request.rpcapi = rpcapi.ConductorAPI() -class TrustedCallHook(hooks.PecanHook): - """Verify that the user has admin rights. - - Checks whether the API call is performed against a public - resource or the user has admin privileges in the appropriate - tenant, domain or other administrative unit. - - """ - def before(self, state): - ctx = state.request.context - if ctx.is_public_api: - return - policy.enforce('admin_api', ctx.to_dict(), ctx.to_dict(), - do_raise=True, exc=exc.HTTPForbidden) - - class NoExceptionTracebackHook(hooks.PecanHook): """Workaround rpc.common: deserialize_remote_exception. diff --git a/ironic/common/exception.py b/ironic/common/exception.py index ee82c32942..bdb308daa4 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -430,8 +430,8 @@ class CommunicationError(IronicException): _msg_fmt = _("Unable to communicate with the server.") -class HTTPForbidden(Forbidden): - pass +class HTTPForbidden(NotAuthorized): + _msg_fmt = _("Access was denied to the following resource: %(resource)s") class Unauthorized(IronicException): diff --git a/ironic/common/policy.py b/ironic/common/policy.py index a9a79d9bcd..237b78f859 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -17,10 +17,165 @@ from oslo_concurrency import lockutils from oslo_config import cfg +from oslo_log import log from oslo_policy import policy +from ironic.common import exception +from ironic.common.i18n import _LW + _ENFORCER = None CONF = cfg.CONF +LOG = log.getLogger(__name__) + +default_policies = [ + # Legacy setting, don't remove. Likely to be overridden by operators who + # forget to update their policy.json configuration file. + # This gets rolled into the new "is_admin" rule below. + policy.RuleDefault('admin_api', + 'role:admin or role:administrator', + description='Legacy rule for cloud admin access'), + # is_public_api is set in the environment from AuthTokenMiddleware + policy.RuleDefault('public_api', + 'is_public_api:True', + description='Internal flag for public API routes'), + # Generic default to hide passwords + policy.RuleDefault('show_password', + '!', + description='Show or mask passwords in API responses'), + # Roles likely to be overriden by operator + policy.RuleDefault('is_member', + 'tenant:demo or tenant:baremetal', + description='May be used to restrict access to specific tenants'), # noqa + policy.RuleDefault('is_observer', + 'rule:is_member and (role:observer or role:baremetal_observer)', # noqa + description='Read-only API access'), + policy.RuleDefault('is_admin', + 'rule:admin_api or (rule:is_member and role:baremetal_admin)', # noqa + description='Full read/write API access'), +] + +# NOTE(deva): to follow policy-in-code spec, we define defaults for +# the granular policies in code, rather than in policy.json. +# All of these may be overridden by configuration, but we can +# depend on their existence throughout the code. + +node_policies = [ + policy.RuleDefault('baremetal:node:get', + 'rule:is_admin or rule:is_observer', + description='Retrieve Node records'), + policy.RuleDefault('baremetal:node:get_boot_device', + 'rule:is_admin or rule:is_observer', + description='Retrieve Node boot device metadata'), + policy.RuleDefault('baremetal:node:get_states', + 'rule:is_admin or rule:is_observer', + description='View Node power and provision state'), + policy.RuleDefault('baremetal:node:create', + 'rule:is_admin', + description='Create Node records'), + policy.RuleDefault('baremetal:node:delete', + 'rule:is_admin', + description='Delete Node records'), + policy.RuleDefault('baremetal:node:update', + 'rule:is_admin', + description='Update Node records'), + policy.RuleDefault('baremetal:node:validate', + 'rule:is_admin', + description='Request active validation of Nodes'), + policy.RuleDefault('baremetal:node:set_maintenance', + 'rule:is_admin', + description='Set maintenance flag, taking a Node ' + 'out of service'), + policy.RuleDefault('baremetal:node:clear_maintenance', + 'rule:is_admin', + description='Clear maintenance flag, placing the Node ' + 'into service again'), + policy.RuleDefault('baremetal:node:set_boot_device', + 'rule:is_admin', + description='Change Node boot device'), + policy.RuleDefault('baremetal:node:set_power_state', + 'rule:is_admin', + description='Change Node power status'), + policy.RuleDefault('baremetal:node:set_provision_state', + 'rule:is_admin', + description='Change Node provision status'), + policy.RuleDefault('baremetal:node:set_raid_state', + 'rule:is_admin', + description='Change Node RAID status'), + policy.RuleDefault('baremetal:node:get_console', + 'rule:is_admin', + description='Get Node console connection information'), + policy.RuleDefault('baremetal:node:set_console_state', + 'rule:is_admin', + description='Change Node console status'), +] + +port_policies = [ + policy.RuleDefault('baremetal:port:get', + 'rule:is_admin or rule:is_observer', + description='Retrieve Port records'), + policy.RuleDefault('baremetal:port:create', + 'rule:is_admin', + description='Create Port records'), + policy.RuleDefault('baremetal:port:delete', + 'rule:is_admin', + description='Delete Port records'), + policy.RuleDefault('baremetal:port:update', + 'rule:is_admin', + description='Update Port records'), +] + +chassis_policies = [ + policy.RuleDefault('baremetal:chassis:get', + 'rule:is_admin or rule:is_observer', + description='Retrieve Chassis records'), + policy.RuleDefault('baremetal:chassis:create', + 'rule:is_admin', + description='Create Chassis records'), + policy.RuleDefault('baremetal:chassis:delete', + 'rule:is_admin', + description='Delete Chassis records'), + policy.RuleDefault('baremetal:chassis:update', + 'rule:is_admin', + description='Update Chassis records'), +] + +driver_policies = [ + policy.RuleDefault('baremetal:driver:get', + 'rule:is_admin or rule:is_observer', + description='View list of available drivers'), + policy.RuleDefault('baremetal:driver:get_properties', + 'rule:is_admin or rule:is_observer', + description='View driver-specific properties'), + policy.RuleDefault('baremetal:driver:get_raid_logical_disk_properties', + 'rule:is_admin or rule:is_observer', + description='View driver-specific RAID metadata'), + +] + +extra_policies = [ + policy.RuleDefault('baremetal:node:vendor_passthru', + 'rule:is_admin', + description='Access vendor-specific Node functions'), + policy.RuleDefault('baremetal:driver:vendor_passthru', + 'rule:is_admin', + description='Access vendor-specific Driver functions'), + policy.RuleDefault('baremetal:node:ipa_heartbeat', + 'rule:public_api', + description='Send heartbeats from IPA ramdisk'), + policy.RuleDefault('baremetal:driver:ipa_lookup', + 'rule:public_api', + description='Access IPA ramdisk functions'), +] + + +def list_policies(): + policies = (default_policies + + node_policies + + port_policies + + chassis_policies + + driver_policies + + extra_policies) + return policies @lockutils.synchronized('policy_enforcer', 'ironic-') @@ -29,10 +184,11 @@ def init_enforcer(policy_file=None, rules=None, """Synchronously initializes the policy enforcer :param policy_file: Custom policy file to use, if none is specified, - `CONF.policy_file` will be used. + `CONF.oslo_policy.policy_file` will be used. :param rules: Default dictionary / Rules to use. It will be considered just in the first instantiation. - :param default_rule: Default rule to use, CONF.default_rule will + :param default_rule: Default rule to use, + CONF.oslo_policy.policy_default_rule will be used if none is specified. :param use_conf: Whether to load rules from config file. @@ -42,10 +198,15 @@ def init_enforcer(policy_file=None, rules=None, if _ENFORCER: return + # NOTE(deva): Register defaults for policy-in-code here so that they are + # loaded exactly once - when this module-global is initialized. + # Defining these in the relevant API modules won't work + # because API classes lack singletons and don't use globals. _ENFORCER = policy.Enforcer(CONF, policy_file=policy_file, rules=rules, default_rule=default_rule, use_conf=use_conf) + _ENFORCER.register_defaults(list_policies()) def get_enforcer(): @@ -57,12 +218,57 @@ def get_enforcer(): return _ENFORCER +# NOTE(deva): We can't call these methods from within decorators because the +# 'target' and 'creds' parameter must be fetched from the call time +# context-local pecan.request magic variable, but decorators are compiled +# at module-load time. + + +def authorize(rule, target, creds, *args, **kwargs): + """A shortcut for policy.Enforcer.authorize() + + Checks authorization of a rule against the target and credentials, and + raises an exception if the rule is not defined. + Always returns true if CONF.auth_strategy == noauth. + + Beginning with the Newton cycle, this should be used in place of 'enforce'. + """ + if CONF.auth_strategy == 'noauth': + return True + enforcer = get_enforcer() + try: + return enforcer.authorize(rule, target, creds, do_raise=True, + *args, **kwargs) + except policy.PolicyNotAuthorized: + raise exception.HTTPForbidden(resource=rule) + + +def check(rule, target, creds, *args, **kwargs): + """A shortcut for policy.Enforcer.enforce() + + Checks authorization of a rule against the target and credentials + and returns True or False. + """ + enforcer = get_enforcer() + return enforcer.enforce(rule, target, creds, *args, **kwargs) + + def enforce(rule, target, creds, do_raise=False, exc=None, *args, **kwargs): """A shortcut for policy.Enforcer.enforce() Checks authorization of a rule against the target and credentials. + Always returns true if CONF.auth_strategy == noauth. """ + # NOTE(deva): this method is obsoleted by authorize(), but retained for + # backwards compatibility in case it has been used downstream. + # It may be removed in the 'P' cycle. + LOG.warning(_LW( + "Deprecation warning: calls to ironic.common.policy.enforce() " + "should be replaced with authorize(). This method may be removed " + "in a future release.")) + if CONF.auth_strategy == 'noauth': + return True enforcer = get_enforcer() return enforcer.enforce(rule, target, creds, do_raise=do_raise, exc=exc, *args, **kwargs) diff --git a/ironic/tests/unit/api/base.py b/ironic/tests/unit/api/base.py index b81b2326b6..4d74524863 100644 --- a/ironic/tests/unit/api/base.py +++ b/ironic/tests/unit/api/base.py @@ -45,10 +45,11 @@ class BaseApiTest(base.DbTestCase): def setUp(self): super(BaseApiTest, self).setUp() - cfg.CONF.set_override("auth_version", "v2.0", + cfg.CONF.set_override("auth_version", "v3", group='keystone_authtoken') cfg.CONF.set_override("admin_user", "admin", group='keystone_authtoken') + cfg.CONF.set_override("auth_strategy", "noauth") self.app = self._make_app() def reset_pecan(): @@ -60,7 +61,7 @@ class BaseApiTest(base.DbTestCase): self._check_version = p.start() self.addCleanup(p.stop) - def _make_app(self, enable_acl=False): + def _make_app(self): # Determine where we are so we can set up paths in the config root_dir = self.path_get() @@ -70,11 +71,9 @@ class BaseApiTest(base.DbTestCase): 'modules': ['ironic.api'], 'static_root': '%s/public' % root_dir, 'template_path': '%s/api/templates' % root_dir, - 'enable_acl': enable_acl, 'acl_public_routes': ['/', '/v1'], }, } - return pecan.testing.load_test_app(self.config) def _request_json(self, path, params, expect_errors=False, headers=None, diff --git a/ironic/tests/unit/api/test_acl.py b/ironic/tests/unit/api/test_acl.py index 65479c1b00..fb11250099 100644 --- a/ironic/tests/unit/api/test_acl.py +++ b/ironic/tests/unit/api/test_acl.py @@ -50,7 +50,8 @@ class TestACL(base.BaseApiTest): def _make_app(self): cfg.CONF.set_override('cache', 'fake.cache', group='keystone_authtoken') - return super(TestACL, self)._make_app(enable_acl=True) + cfg.CONF.set_override('auth_strategy', 'keystone') + return super(TestACL, self)._make_app() def test_non_authenticated(self): response = self.get_json(self.node_path, expect_errors=True) diff --git a/ironic/tests/unit/api/test_audit.py b/ironic/tests/unit/api/test_audit.py index 6e53fbfb18..bd1565c1cb 100644 --- a/ironic/tests/unit/api/test_audit.py +++ b/ironic/tests/unit/api/test_audit.py @@ -38,7 +38,7 @@ class TestAuditMiddleware(base.BaseApiTest): @mock.patch.object(audit, 'AuditMiddleware') def test_enable_audit_request(self, mock_audit): CONF.audit.enabled = True - self._make_app(enable_acl=True) + self._make_app() mock_audit.assert_called_once_with( mock.ANY, audit_map_file=CONF.audit.audit_map_file, @@ -50,10 +50,10 @@ class TestAuditMiddleware(base.BaseApiTest): mock_audit.side_effect = IOError("file access error") self.assertRaises(exception.InputFileError, - self._make_app, enable_acl=True) + self._make_app) @mock.patch.object(audit, 'AuditMiddleware') def test_disable_audit_request(self, mock_audit): CONF.audit.enabled = False - self._make_app(enable_acl=True) + self._make_app() self.assertFalse(mock_audit.called) diff --git a/ironic/tests/unit/api/test_hooks.py b/ironic/tests/unit/api/test_hooks.py index e29ea298f8..7c9a589383 100644 --- a/ironic/tests/unit/api/test_hooks.py +++ b/ironic/tests/unit/api/test_hooks.py @@ -21,13 +21,11 @@ from oslo_config import cfg import oslo_messaging as messaging import six from six.moves import http_client -from webob import exc as webob_exc from ironic.api.controllers import root from ironic.api import hooks from ironic.common import context from ironic.tests.unit.api import base -from ironic.tests.unit import policy_fixture class FakeRequest(object): @@ -217,6 +215,7 @@ class TestNoExceptionTracebackHook(base.BaseApiTest): class TestContextHook(base.BaseApiTest): @mock.patch.object(context, 'RequestContext') def test_context_hook_not_admin(self, mock_ctx): + cfg.CONF.set_override('auth_strategy', 'keystone') headers = fake_headers(admin=False) reqstate = FakeRequestState(headers=headers) context_hook = hooks.ContextHook(None) @@ -234,6 +233,7 @@ class TestContextHook(base.BaseApiTest): @mock.patch.object(context, 'RequestContext') def test_context_hook_admin(self, mock_ctx): + cfg.CONF.set_override('auth_strategy', 'keystone') headers = fake_headers(admin=True) reqstate = FakeRequestState(headers=headers) context_hook = hooks.ContextHook(None) @@ -251,6 +251,7 @@ class TestContextHook(base.BaseApiTest): @mock.patch.object(context, 'RequestContext') def test_context_hook_public_api(self, mock_ctx): + cfg.CONF.set_override('auth_strategy', 'keystone') headers = fake_headers(admin=True) env = {'is_public_api': True} reqstate = FakeRequestState(headers=headers, environ=env) @@ -306,41 +307,6 @@ class TestContextHook(base.BaseApiTest): response.headers) -class TestTrustedCallHook(base.BaseApiTest): - def test_trusted_call_hook_not_admin(self): - headers = fake_headers(admin=False) - reqstate = FakeRequestState(headers=headers) - reqstate.set_context() - trusted_call_hook = hooks.TrustedCallHook() - self.assertRaises(webob_exc.HTTPForbidden, - trusted_call_hook.before, reqstate) - - def test_trusted_call_hook_admin(self): - headers = fake_headers(admin=True) - reqstate = FakeRequestState(headers=headers) - reqstate.set_context() - trusted_call_hook = hooks.TrustedCallHook() - trusted_call_hook.before(reqstate) - - def test_trusted_call_hook_public_api(self): - headers = fake_headers(admin=False) - env = {'is_public_api': True} - reqstate = FakeRequestState(headers=headers, environ=env) - reqstate.set_context() - trusted_call_hook = hooks.TrustedCallHook() - trusted_call_hook.before(reqstate) - - -class TestTrustedCallHookCompatJuno(TestTrustedCallHook): - def setUp(self): - super(TestTrustedCallHookCompatJuno, self).setUp() - self.policy = self.useFixture( - policy_fixture.PolicyFixture(compat='juno')) - - def test_trusted_call_hook_public_api(self): - self.skipTest('no public_api trusted call policy in juno') - - class TestPublicUrlHook(base.BaseApiTest): def test_before_host_url(self): diff --git a/ironic/tests/unit/common/test_policy.py b/ironic/tests/unit/common/test_policy.py index 2315414688..97d3375580 100644 --- a/ironic/tests/unit/common/test_policy.py +++ b/ironic/tests/unit/common/test_policy.py @@ -15,60 +15,107 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_policy import policy as oslo_policy + +from ironic.common import exception from ironic.common import policy from ironic.tests import base -class PolicyTestCase(base.TestCase): +class PolicyInCodeTestCase(base.TestCase): """Tests whether the configuration of the policy engine is corect.""" def test_admin_api(self): - creds = ({'roles': [u'admin']}, + creds = ({'roles': ['admin']}, {'roles': ['administrator']}, {'roles': ['admin', 'administrator']}) for c in creds: - self.assertTrue(policy.enforce('admin_api', c, c)) + self.assertTrue(policy.check('admin_api', c, c)) def test_public_api(self): creds = {'is_public_api': 'True'} - self.assertTrue(policy.enforce('public_api', creds, creds)) - - def test_trusted_call(self): - creds = ({'roles': ['admin']}, - {'is_public_api': 'True'}, - {'roles': ['admin'], 'is_public_api': 'True'}, - {'roles': ['Member'], 'is_public_api': 'True'}) - - for c in creds: - self.assertTrue(policy.enforce('trusted_call', c, c)) + self.assertTrue(policy.check('public_api', creds, creds)) def test_show_password(self): creds = {'roles': [u'admin'], 'tenant': 'admin'} - self.assertTrue(policy.enforce('show_password', creds, creds)) + self.assertTrue(policy.check('show_password', creds, creds)) + + def test_node_get(self): + creds = {'roles': ['baremetal_observer'], 'tenant': 'demo'} + self.assertTrue(policy.check('baremetal:node:get', creds, creds)) + + def test_node_create(self): + creds = {'roles': ['baremetal_admin'], 'tenant': 'demo'} + self.assertTrue(policy.check('baremetal:node:create', creds, creds)) -class PolicyTestCaseNegative(base.TestCase): +class PolicyInCodeTestCaseNegative(base.TestCase): """Tests whether the configuration of the policy engine is corect.""" def test_admin_api(self): creds = {'roles': ['Member']} - self.assertFalse(policy.enforce('admin_api', creds, creds)) + self.assertFalse(policy.check('admin_api', creds, creds)) def test_public_api(self): creds = ({'is_public_api': 'False'}, {}) for c in creds: - self.assertFalse(policy.enforce('public_api', c, c)) - - def test_trusted_call(self): - creds = ({'roles': ['Member']}, - {'is_public_api': 'False'}, - {'roles': ['Member'], 'is_public_api': 'False'}) - - for c in creds: - self.assertFalse(policy.enforce('trusted_call', c, c)) + self.assertFalse(policy.check('public_api', c, c)) def test_show_password(self): creds = {'roles': [u'admin'], 'tenant': 'demo'} - self.assertFalse(policy.enforce('show_password', creds, creds)) + self.assertFalse(policy.check('show_password', creds, creds)) + + def test_node_get(self): + creds = {'roles': ['generic_user'], 'tenant': 'demo'} + self.assertFalse(policy.check('baremetal:node:get', creds, creds)) + + def test_node_create(self): + creds = {'roles': ['baremetal_observer'], 'tenant': 'demo'} + self.assertFalse(policy.check('baremetal:node:create', creds, creds)) + + +class PolicyTestCase(base.TestCase): + """Tests whether ironic.common.policy behaves as expected.""" + + def setUp(self): + super(PolicyTestCase, self).setUp() + rule = oslo_policy.RuleDefault('has_foo_role', "role:foo") + enforcer = policy.get_enforcer() + enforcer.register_default(rule) + + def test_authorize_passes(self): + creds = {'roles': ['foo']} + policy.authorize('has_foo_role', creds, creds) + + def test_authorize_access_forbidden(self): + creds = {'roles': ['bar']} + self.assertRaises( + exception.HTTPForbidden, + policy.authorize, 'has_foo_role', creds, creds) + + def test_authorize_policy_not_registered(self): + creds = {'roles': ['foo']} + self.assertRaises( + oslo_policy.PolicyNotRegistered, + policy.authorize, 'has_bar_role', creds, creds) + + def test_enforce_existing_rule_passes(self): + creds = {'roles': ['foo']} + self.assertTrue(policy.enforce('has_foo_role', creds, creds)) + + def test_enforce_missing_rule_fails(self): + creds = {'roles': ['foo']} + self.assertFalse(policy.enforce('has_bar_role', creds, creds)) + + def test_enforce_existing_rule_fails(self): + creds = {'roles': ['bar']} + self.assertFalse(policy.enforce('has_foo_role', creds, creds)) + + def test_enforce_existing_rule_raises(self): + creds = {'roles': ['bar']} + self.assertRaises( + exception.IronicException, + policy.enforce, 'has_foo_role', creds, creds, True, + exception.IronicException) diff --git a/ironic/tests/unit/fake_policy.py b/ironic/tests/unit/fake_policy.py deleted file mode 100644 index 66f600845f..0000000000 --- a/ironic/tests/unit/fake_policy.py +++ /dev/null @@ -1,42 +0,0 @@ -# Copyright (c) 2012 OpenStack Foundation -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - - -policy_data = """ -{ - "admin_api": "role:admin or role:administrator", - "public_api": "is_public_api:True", - "trusted_call": "rule:admin_api or rule:public_api", - "default": "rule:trusted_call", - "show_password": "tenant:admin" -} -""" - - -policy_data_compat_juno = """ -{ - "admin": "role:admin or role:administrator", - "admin_api": "is_admin:True", - "default": "rule:admin_api" -} -""" - - -def get_policy_data(compat): - if not compat: - return policy_data - elif compat == 'juno': - return policy_data_compat_juno - else: - raise Exception('Policy data for %s not available' % compat) diff --git a/ironic/tests/unit/policy_fixture.py b/ironic/tests/unit/policy_fixture.py index 7f3f48ac96..4fe3b5faab 100644 --- a/ironic/tests/unit/policy_fixture.py +++ b/ironic/tests/unit/policy_fixture.py @@ -19,22 +19,27 @@ from oslo_config import cfg from oslo_policy import opts as policy_opts from ironic.common import policy as ironic_policy -from ironic.tests.unit import fake_policy CONF = cfg.CONF +# NOTE(deva): We ship a default that always masks passwords, but for testing +# we need to override that default to ensure passwords can be +# made visible by operators that choose to do so. +policy_data = """ +{ + "show_password": "tenant:admin" +} +""" + class PolicyFixture(fixtures.Fixture): - def __init__(self, compat=None): - self.compat = compat - def setUp(self): super(PolicyFixture, self).setUp() self.policy_dir = self.useFixture(fixtures.TempDir()) self.policy_file_name = os.path.join(self.policy_dir.path, 'policy.json') with open(self.policy_file_name, 'w') as policy_file: - policy_file.write(fake_policy.get_policy_data(self.compat)) + policy_file.write(policy_data) policy_opts.set_defaults(CONF) CONF.set_override('policy_file', self.policy_file_name, 'oslo_policy') ironic_policy._ENFORCER = None diff --git a/releasenotes/notes/implement-policy-in-code-cbb0216ef5f8224f.yaml b/releasenotes/notes/implement-policy-in-code-cbb0216ef5f8224f.yaml new file mode 100644 index 0000000000..6755307f9f --- /dev/null +++ b/releasenotes/notes/implement-policy-in-code-cbb0216ef5f8224f.yaml @@ -0,0 +1,22 @@ +--- +features: + - | + RESTful access to every API resource may now be controlled by adjusting + policy settings. Defaults are set in code, and remain backwards compatible + with the previously-included policy.json file. Two new roles are checked + by default, "baremetal_admin" and "baremetal_observer", though these may be + replaced or overridden by configuration. The "baremetal_observer" role + grants read-only access to Ironic's API. +security: + - | + Previously, access to Ironic's REST API was "all or nothing". With this + release, it is now possible to restrict read and write access to API + resources to specific cloud roles. +upgrade: + - | + During an upgrade, it is recommended that all deployers re-evaluate the + settings in their /etc/ironic/policy.json file. This file should now be + used only to override default configuration, such as by limiting access to + the Bare Metal service to specific tenants or restricting access to + specific API endpoints. A policy.json.sample file is provided that lists + all supported policies. diff --git a/setup.cfg b/setup.cfg index defced9964..576df9a67a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -25,6 +25,9 @@ packages = oslo.config.opts = ironic = ironic.conf.opts:list_opts +oslo.policy.policies = + ironic.api = ironic.common.policy:list_policies + console_scripts = ironic-api = ironic.cmd.api:main ironic-dbsync = ironic.cmd.dbsync:main diff --git a/tox.ini b/tox.ini index ef60d136e9..89b0bf2bbe 100644 --- a/tox.ini +++ b/tox.ini @@ -58,6 +58,12 @@ envdir = {toxworkdir}/venv commands = oslo-config-generator --config-file=tools/config/ironic-config-generator.conf +[testenv:genpolicy] +sitepackages = False +envdir = {toxworkdir}/venv +commands = + oslopolicy-sample-generator --namespace=ironic.api --output-file=etc/ironic/policy.json.sample + [testenv:debug] commands = oslo_debug_helper -t ironic/tests/unit {posargs}