From 094565bb2d5b1c2e8ee6aa0534804fa21a2f64c7 Mon Sep 17 00:00:00 2001 From: Adam Gandelman Date: Thu, 8 Jan 2015 16:37:03 -0800 Subject: [PATCH] Provided backward compat for enforcing admin policy The ContextHook API hook was changed in a way that it cannot construct an admin-enabled context with the default juno policy.json, preventing admin users from using the API. This updates it to enforce the previous 'admin' rule in addtion to the new 'admin_api' rule. Change-Id: I47985e8989837ce8a9e44bbf5a068ddfcab179f6 Closes-bug: #1408808 --- ironic/api/hooks.py | 6 +++++- ironic/tests/api/test_hooks.py | 18 ++++++++++++++++++ ironic/tests/fake_policy.py | 18 ++++++++++++++++++ ironic/tests/policy_fixture.py | 4 +++- 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/ironic/api/hooks.py b/ironic/api/hooks.py index 8f787b34ec..4af78b5e8e 100644 --- a/ironic/api/hooks.py +++ b/ironic/api/hooks.py @@ -76,7 +76,11 @@ class ContextHook(hooks.PecanHook): 'roles': headers.get('X-Roles', '').split(','), } - is_admin = policy.enforce('admin_api', creds, creds) + # NOTE(adam_g): We also check the previous 'admin' rule to ensure + # compat with default juno policy.json. This double check may be + # removed in L. + is_admin = (policy.enforce('admin_api', creds, creds) or + policy.enforce('admin', creds, creds)) is_public_api = state.request.environ.get('is_public_api', False) state.request.context = context.RequestContext( diff --git a/ironic/tests/api/test_hooks.py b/ironic/tests/api/test_hooks.py index f139bb8e5b..370fa65972 100644 --- a/ironic/tests/api/test_hooks.py +++ b/ironic/tests/api/test_hooks.py @@ -25,6 +25,7 @@ from ironic.api.controllers import root from ironic.api import hooks from ironic.common import context from ironic.tests.api import base +from ironic.tests import policy_fixture class FakeRequest(object): @@ -216,6 +217,13 @@ class TestContextHook(base.FunctionalTest): roles=headers['X-Roles'].split(',')) +class TestContextHookCompatJuno(TestContextHook): + def setUp(self): + super(TestContextHookCompatJuno, self).setUp() + self.policy = self.useFixture( + policy_fixture.PolicyFixture(compat='juno')) + + class TestTrustedCallHook(base.FunctionalTest): def test_trusted_call_hook_not_admin(self): headers = fake_headers(admin=False) @@ -239,3 +247,13 @@ class TestTrustedCallHook(base.FunctionalTest): 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') diff --git a/ironic/tests/fake_policy.py b/ironic/tests/fake_policy.py index dbd686492f..eb723ff710 100644 --- a/ironic/tests/fake_policy.py +++ b/ironic/tests/fake_policy.py @@ -21,3 +21,21 @@ policy_data = """ "default": "rule:trusted_call" } """ + + +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/policy_fixture.py b/ironic/tests/policy_fixture.py index 59e7b60173..f02758dfe3 100644 --- a/ironic/tests/policy_fixture.py +++ b/ironic/tests/policy_fixture.py @@ -24,6 +24,8 @@ CONF = cfg.CONF class PolicyFixture(fixtures.Fixture): + def __init__(self, compat=None): + self.compat = compat def setUp(self): super(PolicyFixture, self).setUp() @@ -31,7 +33,7 @@ class PolicyFixture(fixtures.Fixture): 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.policy_data) + policy_file.write(fake_policy.get_policy_data(self.compat)) CONF.set_override('policy_file', self.policy_file_name) ironic_policy._ENFORCER = None self.addCleanup(ironic_policy.get_enforcer().clear)