diff --git a/etc/ironic/policy.json b/etc/ironic/policy.json index a889e0bb0f..94ac3a5b80 100644 --- a/etc/ironic/policy.json +++ b/etc/ironic/policy.json @@ -1,6 +1,5 @@ { + "admin": "role:admin or role:administrator", "admin_api": "is_admin:True", - "admin_or_owner": "is_admin:True or project_id:%(project_id)s", - "is_admin": "role:admin or role:administrator", - "default": "rule:admin_or_owner", + "default": "rule:admin_api" } diff --git a/ironic/api/acl.py b/ironic/api/acl.py index 85fa5325c9..453b67b465 100644 --- a/ironic/api/acl.py +++ b/ironic/api/acl.py @@ -20,11 +20,8 @@ from keystoneclient.middleware import auth_token as keystone_auth_token from oslo.config import cfg -from pecan import hooks -from webob import exc from ironic.api.middleware import auth_token -from ironic.common import policy OPT_GROUP_NAME = 'keystone_authtoken' @@ -56,17 +53,3 @@ def install(app, conf, public_routes): return auth_token.AuthTokenMiddleware(app, conf=keystone_config, public_api_routes=public_routes) - - -class AdminAuthHook(hooks.PecanHook): - """Verify that the user has admin rights. - - Checks whether the request context is an admin context and - rejects the request otherwise. - - """ - def before(self, state): - ctx = state.request.context - - if not policy.check_is_admin(ctx) and not ctx.is_public_api: - raise exc.HTTPForbidden() diff --git a/ironic/api/app.py b/ironic/api/app.py index 8bb865baa6..ec42b6c54e 100644 --- a/ironic/api/app.py +++ b/ironic/api/app.py @@ -23,6 +23,7 @@ from ironic.api import acl from ironic.api import config from ironic.api import hooks from ironic.api import middleware +from ironic.common import policy auth_opts = [ cfg.StrOpt('auth_strategy', @@ -41,6 +42,8 @@ def get_pecan_config(): def setup_app(pecan_config=None, extra_hooks=None): + policy.init() + app_hooks = [hooks.ConfigHook(), hooks.DBHook(), hooks.ContextHook(pecan_config.app.acl_public_routes), @@ -52,7 +55,7 @@ def setup_app(pecan_config=None, extra_hooks=None): pecan_config = get_pecan_config() if pecan_config.app.enable_acl: - app_hooks.append(acl.AdminAuthHook()) + app_hooks.append(hooks.AdminAuthHook()) pecan.configuration.set_config(dict(pecan_config), overwrite=True) diff --git a/ironic/api/hooks.py b/ironic/api/hooks.py index 5162eba7fa..20739a9486 100644 --- a/ironic/api/hooks.py +++ b/ironic/api/hooks.py @@ -18,6 +18,7 @@ from oslo.config import cfg from pecan import hooks +from webob import exc from ironic.common import context from ironic.common import utils @@ -74,7 +75,7 @@ class ContextHook(hooks.PecanHook): auth_token = state.request.headers.get('X-Auth-Token', None) creds = {'roles': state.request.headers.get('X-Roles', '').split(',')} - is_admin = policy.check('is_admin', state.request.headers, creds) + is_admin = policy.check('admin', state.request.headers, creds) path = utils.safe_rstrip(state.request.path, '/') is_public_api = path in self.public_api_routes @@ -94,3 +95,18 @@ class RPCHook(hooks.PecanHook): def before(self, state): state.request.rpcapi = rpcapi.ConductorAPI() + + +class AdminAuthHook(hooks.PecanHook): + """Verify that the user has admin rights. + + Checks whether the request context is an admin context and + rejects the request otherwise. + + """ + def before(self, state): + ctx = state.request.context + is_admin_api = policy.check('admin_api', {}, ctx.to_dict()) + + if not is_admin_api and not ctx.is_public_api: + raise exc.HTTPForbidden() diff --git a/ironic/common/policy.py b/ironic/common/policy.py index 1c764dacaa..4326556c83 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -66,66 +66,3 @@ def init(): def _set_rules(data): default_rule = CONF.policy_default_rule policy.set_rules(policy.Rules.load_json(data, default_rule)) - - -def enforce(context, action, target, do_raise=True): - """Verifies that the action is valid on the target in this context. - - :param context: ironic context - :param action: string representing the action to be checked - this should be colon separated for clarity. - i.e. ``compute:create_instance``, - ``compute:attach_volume``, - ``volume:attach_volume`` - :param target: dictionary representing the object of the action - for object creation this should be a dictionary representing the - location of the object e.g. ``{'project_id': context.project_id}`` - :param do_raise: if True (the default), raises PolicyNotAuthorized; - if False, returns False - - :raises ironic.exception.PolicyNotAuthorized: if verification fails - and do_raise is True. - - :return: returns a non-False value (not necessarily "True") if - authorized, and the exact value False if not authorized and - do_raise is False. - """ - init() - - credentials = context.to_dict() - - # Add the exception arguments if asked to do a raise - extra = {} - if do_raise: - extra.update(exc=exception.PolicyNotAuthorized, action=action) - - return policy.check(action, target, credentials, **extra) - - -def check_is_admin(context): - """Whether or not role contains 'admin' role according to policy setting. - - """ - init() - - credentials = context.to_dict() - target = credentials - - return policy.check('context_is_admin', target, credentials) - - -@policy.register('context_is_admin') -class IsAdminCheck(policy.Check): - """An explicit check for is_admin.""" - - def __init__(self, kind, match): - """Initialize the check.""" - - self.expected = (match.lower() == 'true') - - super(IsAdminCheck, self).__init__(kind, str(self.expected)) - - def __call__(self, target, creds): - """Determine whether is_admin matches the requested value.""" - - return creds['is_admin'] == self.expected diff --git a/ironic/tests/api/base.py b/ironic/tests/api/base.py index f79c4412cd..df227084ce 100644 --- a/ironic/tests/api/base.py +++ b/ironic/tests/api/base.py @@ -42,8 +42,6 @@ class FunctionalTest(base.DbTestCase): def setUp(self): super(FunctionalTest, self).setUp() cfg.CONF.set_override("auth_version", "v2.0", group=acl.OPT_GROUP_NAME) - cfg.CONF.set_override("policy_file", - self.path_get('tests/policy.json')) self.app = self._make_app() self.dbapi = dbapi.get_instance() diff --git a/ironic/tests/fake_policy.py b/ironic/tests/fake_policy.py index 15e19db95d..3851966b19 100644 --- a/ironic/tests/fake_policy.py +++ b/ironic/tests/fake_policy.py @@ -17,9 +17,8 @@ policy_data = """ { - "admin_api": "role:admin", - "admin_or_owner": "is_admin:True or project_id:%(project_id)s", - "is_admin": "role:admin or role:administrator", - "default": "rule:admin_or_owner" + "admin": "role:admin or role:administrator", + "admin_api": "is_admin:True", + "default": "rule:admin_api" } """ diff --git a/ironic/tests/policy.json b/ironic/tests/policy.json deleted file mode 100644 index a889e0bb0f..0000000000 --- a/ironic/tests/policy.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "admin_api": "is_admin:True", - "admin_or_owner": "is_admin:True or project_id:%(project_id)s", - "is_admin": "role:admin or role:administrator", - "default": "rule:admin_or_owner", -}