diff --git a/etc/ironic/policy.json b/etc/ironic/policy.json index b9458e8fe8..a889e0bb0f 100644 --- a/etc/ironic/policy.json +++ b/etc/ironic/policy.json @@ -1,6 +1,6 @@ { "admin_api": "is_admin:True", "admin_or_owner": "is_admin:True or project_id:%(project_id)s", - "context_is_admin": "role:admin", + "is_admin": "role:admin or role:administrator", "default": "rule:admin_or_owner", } diff --git a/ironic/api/acl.py b/ironic/api/acl.py index eab697e69c..cf68086b43 100644 --- a/ironic/api/acl.py +++ b/ironic/api/acl.py @@ -31,6 +31,8 @@ OPT_GROUP_NAME = 'keystone_authtoken' def register_opts(conf): """Register keystoneclient middleware options + + :param conf: Ironic settings. """ conf.register_opts(auth_token.opts, group=OPT_GROUP_NAME) @@ -41,16 +43,23 @@ register_opts(cfg.CONF) def install(app, conf): - """Install ACL check on application.""" - return auth_token.AuthProtocol(app, - conf=dict(conf.get(OPT_GROUP_NAME))) + """Install ACL check on application. + + :param app: A WSGI applicatin. + :param conf: Settings. Must include OPT_GROUP_NAME section. + :return: The same WSGI application with ACL installed. + """ + keystone_config = dict(conf.get(OPT_GROUP_NAME)) + return auth_token.AuthProtocol(app, conf=keystone_config) class AdminAuthHook(hooks.PecanHook): - """Verify that the user has admin rights - """ + """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): - headers = state.request.headers - if not policy.check_is_admin(headers.get('X-Roles', "").split(",")): - raise exc.HTTPUnauthorized() + if not policy.check_is_admin(state.request.context): + raise exc.HTTPForbidden() diff --git a/ironic/api/config.py b/ironic/api/config.py index ad00995b43..632a1dc9af 100644 --- a/ironic/api/config.py +++ b/ironic/api/config.py @@ -16,7 +16,7 @@ # Server Specific Configurations server = { - 'port': '6382', + 'port': '6385', 'host': '0.0.0.0' } @@ -27,12 +27,5 @@ app = { 'static_root': '%(confdir)s/public', 'template_path': '%(confdir)s/ironic/api/templates', 'debug': False, - 'enable_acl': False, + 'enable_acl': True, } - -# Custom Configurations must be in Python dictionary format:: -# -# foo = {'bar':'baz'} -# -# All configurations are accessible at:: -# pecan.conf diff --git a/ironic/api/hooks.py b/ironic/api/hooks.py index dacaf92f27..bf884cd2bc 100644 --- a/ironic/api/hooks.py +++ b/ironic/api/hooks.py @@ -22,6 +22,7 @@ from pecan import hooks from ironic.conductor import rpcapi from ironic.db import api as dbapi from ironic.openstack.common import context +from ironic.openstack.common import policy class ConfigHook(hooks.PecanHook): @@ -40,11 +41,40 @@ class DBHook(hooks.PecanHook): class ContextHook(hooks.PecanHook): + """Configures a request context and attaches it to the request. + The following HTTP request headers are used: + + X-User-Id or X-User: + Used for context.user_id. + + X-Tenant-Id or X-Tenant: + Used for context.tenant. + + X-Auth-Token: + Used for context.auth_token. + + X-Roles: + Used for setting context.is_admin flag to either True or False. + The flag is set to True, if X-Roles contains either an administrator + or admin substring. Otherwise it is set to False. + + """ def before(self, state): - # TODO(deva): Making all requests have admin context for early - # development. This needs to be fixed later! - state.request.context = context.get_admin_context() + user_id = state.request.headers.get('X-User-Id') + user_id = state.request.headers.get('X-User', user_id) + tenant = state.request.headers.get('X-Tenant-Id') + tenant = state.request.headers.get('X-Tenant', tenant) + auth_token = state.request.headers.get('X-Auth-Token') + creds = {'roles': state.request.headers.get('X-Roles').split(',')} + is_admin = policy.check('is_admin', state.request.headers, creds) + + state.request.context = context.RequestContext( + auth_token=auth_token, + user=user_id, + tenant=tenant, + request_id=context.generate_request_id(), + is_admin=is_admin) class RPCHook(hooks.PecanHook): diff --git a/ironic/common/policy.py b/ironic/common/policy.py index a99aaf3fd5..1c764dacaa 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -83,7 +83,7 @@ def enforce(context, action, target, do_raise=True): :param do_raise: if True (the default), raises PolicyNotAuthorized; if False, returns False - :raises nova.exception.PolicyNotAuthorized: if verification fails + :raises ironic.exception.PolicyNotAuthorized: if verification fails and do_raise is True. :return: returns a non-False value (not necessarily "True") if @@ -114,7 +114,7 @@ def check_is_admin(context): return policy.check('context_is_admin', target, credentials) -@policy.register('is_admin') +@policy.register('context_is_admin') class IsAdminCheck(policy.Check): """An explicit check for is_admin.""" diff --git a/ironic/tests/api/test_acl.py b/ironic/tests/api/test_acl.py new file mode 100644 index 0000000000..5d9d58c812 --- /dev/null +++ b/ironic/tests/api/test_acl.py @@ -0,0 +1,77 @@ +# -*- encoding: utf-8 -*- +# +# 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. +""" +Tests for ACL. Checks whether certain kinds of requests +are blocked or allowed to be processed. +""" + +from oslo.config import cfg + +from ironic.api import acl +from ironic.db import api as db_api +from ironic.tests.api import base +from ironic.tests.api import utils +from ironic.tests.db import utils as db_utils + + +class TestACL(base.FunctionalTest): + + def setUp(self): + super(TestACL, self).setUp() + + self.environ = {'fake.cache': utils.FakeMemcache()} + self.fake_node = db_utils.get_test_node() + self.dbapi = db_api.get_instance() + self.node_path = '/nodes/%s' % self.fake_node['uuid'] + + def get_json(self, path, expect_errors=False, headers=None, q=[], **param): + return super(TestACL, self).get_json(path, + expect_errors=expect_errors, + headers=headers, + q=q, + extra_environ=self.environ, + **param) + + def _make_app(self): + cfg.CONF.set_override('cache', 'fake.cache', group=acl.OPT_GROUP_NAME) + return super(TestACL, self)._make_app(enable_acl=True) + + def test_non_authenticated(self): + response = self.get_json(self.node_path, expect_errors=True) + self.assertEqual(response.status_int, 401) + + def test_authenticated(self): + self.mox.StubOutWithMock(self.dbapi, 'get_node') + self.dbapi.get_node(self.fake_node['uuid']).AndReturn(self.fake_node) + self.mox.ReplayAll() + + response = self.get_json(self.node_path, + headers={'X-Auth-Token': utils.ADMIN_TOKEN}) + + self.assertEquals(response['uuid'], self.fake_node['uuid']) + + def test_non_admin(self): + response = self.get_json(self.node_path, + headers={'X-Auth-Token': utils.MEMBER_TOKEN}, + expect_errors=True) + + self.assertEqual(response.status_int, 403) + + def test_non_admin_with_admin_header(self): + response = self.get_json(self.node_path, + headers={'X-Auth-Token': utils.MEMBER_TOKEN, + 'X-Roles': 'admin'}, + expect_errors=True) + + self.assertEqual(response.status_int, 403) diff --git a/ironic/tests/api/test_list_nodes.py b/ironic/tests/api/test_list_nodes.py index 46f960ff83..b1e4ee09de 100644 --- a/ironic/tests/api/test_list_nodes.py +++ b/ironic/tests/api/test_list_nodes.py @@ -20,5 +20,5 @@ from ironic.tests.api import base class TestListNodes(base.FunctionalTest): def test_empty(self): - data = self.get_json('/nodes') + data = self.get_json('/nodes', headers={'X-Roles': 'admin'}) self.assertEqual([], data) diff --git a/ironic/tests/api/utils.py b/ironic/tests/api/utils.py new file mode 100644 index 0000000000..9d7017ab3c --- /dev/null +++ b/ironic/tests/api/utils.py @@ -0,0 +1,60 @@ +# -*- encoding: utf-8 -*- +# +# 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 + +import datetime +import json + +ADMIN_TOKEN = '4562138218392831' +MEMBER_TOKEN = '4562138218392832' + + +class FakeMemcache(object): + """Fake cache that is used for keystone tokens lookup.""" + + _cache = { + 'tokens/%s' % ADMIN_TOKEN: { + 'access': { + 'token': {'id': ADMIN_TOKEN}, + 'user': {'id': 'user_id1', + 'name': 'user_name1', + 'tenantId': '123i2910', + 'tenantName': 'mytenant', + 'roles': [{'name': 'admin'}] + }, + } + }, + 'tokens/%s' % MEMBER_TOKEN: { + 'access': { + 'token': {'id': MEMBER_TOKEN}, + 'user': {'id': 'user_id2', + 'name': 'user-good', + 'tenantId': 'project-good', + 'tenantName': 'goodies', + 'roles': [{'name': 'Member'}] + } + } + } + } + + def __init__(self): + self.set_key = None + self.set_value = None + self.token_expiration = None + + def get(self, key): + dt = datetime.datetime.now() + datetime.timedelta(minutes=5) + return json.dumps((self._cache.get(key), dt.strftime('%s'))) + + def set(self, key, value, timeout=None): + self.set_value = value + self.set_key = key diff --git a/ironic/tests/fake_policy.py b/ironic/tests/fake_policy.py index ecd72eaf21..15e19db95d 100644 --- a/ironic/tests/fake_policy.py +++ b/ironic/tests/fake_policy.py @@ -19,7 +19,7 @@ policy_data = """ { "admin_api": "role:admin", "admin_or_owner": "is_admin:True or project_id:%(project_id)s", - "context_is_admin": "role:admin or role:administrator", + "is_admin": "role:admin or role:administrator", "default": "rule:admin_or_owner" } """ diff --git a/ironic/tests/policy.json b/ironic/tests/policy.json index b9458e8fe8..a889e0bb0f 100644 --- a/ironic/tests/policy.json +++ b/ironic/tests/policy.json @@ -1,6 +1,6 @@ { "admin_api": "is_admin:True", "admin_or_owner": "is_admin:True or project_id:%(project_id)s", - "context_is_admin": "role:admin", + "is_admin": "role:admin or role:administrator", "default": "rule:admin_or_owner", }