From c7da7a69250fcffb6841d36710770608d603bb6a Mon Sep 17 00:00:00 2001 From: Devananda van der Veen Date: Fri, 3 Jun 2016 15:43:12 -0700 Subject: [PATCH] Add keystone policy support to Ironic Implements more fine-grained policy support within our API service, following the oslo policy-in-code spec, while maintaining compatibility with the previous default policy.json file. An empty policy.json file is included, along with a sample file listig all supported policy settings and their default values. A new tox target "genpolicy" has been added to ease automation of sample policy file generation. All calls to policy.enforce() have been replaced with with policy.authorize() to avoid silent failures when a rule is undefined, because enforce() does not raise() if the target rule does not exist. NOTE: policy.enforce() is not removed by this patch, but a deprecation warning will be logged if it this method is invoked. Updates unit test coverage for the new authorize() method, as well as more general unit test updates for some of the new rules. Partial-bug: #1526752 Change-Id: Ie4398f840601d027e2fe209c17d854421687c7b7 --- devstack/lib/ironic | 27 ++- etc/ironic/policy.json | 6 +- etc/ironic/policy.json.sample | 72 ++++++ ironic/api/acl.py | 34 --- ironic/api/app.py | 12 +- ironic/api/config.py | 1 - ironic/api/controllers/v1/chassis.py | 19 ++ ironic/api/controllers/v1/driver.py | 22 ++ ironic/api/controllers/v1/node.py | 64 ++++++ ironic/api/controllers/v1/port.py | 19 ++ ironic/api/hooks.py | 29 +-- ironic/common/exception.py | 4 +- ironic/common/policy.py | 210 +++++++++++++++++- ironic/tests/unit/api/base.py | 7 +- ironic/tests/unit/api/test_acl.py | 3 +- ironic/tests/unit/api/test_audit.py | 6 +- ironic/tests/unit/api/test_hooks.py | 40 +--- ironic/tests/unit/common/test_policy.py | 99 ++++++--- ironic/tests/unit/fake_policy.py | 42 ---- ironic/tests/unit/policy_fixture.py | 15 +- ...ement-policy-in-code-cbb0216ef5f8224f.yaml | 22 ++ setup.cfg | 3 + tox.ini | 6 + 23 files changed, 563 insertions(+), 199 deletions(-) create mode 100644 etc/ironic/policy.json.sample delete mode 100644 ironic/api/acl.py delete mode 100644 ironic/tests/unit/fake_policy.py create mode 100644 releasenotes/notes/implement-policy-in-code-cbb0216ef5f8224f.yaml 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}