diff --git a/mistral/api/access_control.py b/mistral/api/access_control.py index 6fa0eea3..7a8d2de1 100644 --- a/mistral/api/access_control.py +++ b/mistral/api/access_control.py @@ -27,11 +27,15 @@ _ENFORCER = None def setup(app): - if cfg.CONF.pecan.auth_enable: + if cfg.CONF.pecan.auth_enable and cfg.CONF.auth_type == 'keystone': conf = dict(cfg.CONF.keystone_authtoken) # Change auth decisions of requests to the app itself. conf.update({'delay_auth_decision': True}) + + # NOTE(rakhmerov): Policy enforcement works only if Keystone + # authentication is enabled. No support for other authentication + # types at this point. _ensure_enforcer_initialization() return auth_token.AuthProtocol(app, conf) @@ -58,6 +62,12 @@ def enforce(action, context, target=None, do_raise=True, :return: returns True if authorized and False if not authorized and do_raise is False. """ + + if cfg.CONF.auth_type != 'keystone': + # Policy enforcement is supported now only with Keystone + # authentication. + return + target_obj = { 'project_id': context.project_id, 'user_id': context.user_id, diff --git a/mistral/config.py b/mistral/config.py index f29c8117..1d45b96d 100644 --- a/mistral/config.py +++ b/mistral/config.py @@ -196,6 +196,11 @@ keycloak_oidc_opts = [ cfg.StrOpt( 'auth_url', help='Keycloak base url (e.g. https://my.keycloak:8443/auth)' + ), + cfg.StrOpt( + 'insecure', + default=False, + help='If True, SSL/TLS certificate verification is disabled' ) ] diff --git a/mistral/context.py b/mistral/context.py index 8c642ee8..6c92999b 100644 --- a/mistral/context.py +++ b/mistral/context.py @@ -15,17 +15,23 @@ import eventlet from keystoneclient.v3 import client as keystone_client +import logging from oslo_config import cfg import oslo_messaging as messaging from oslo_serialization import jsonutils from osprofiler import profiler import pecan from pecan import hooks +import pprint +import requests from mistral import exceptions as exc from mistral import utils +LOG = logging.getLogger(__name__) + + CONF = cfg.CONF _CTX_THREAD_LOCAL_NAME = "MISTRAL_APP_CTX_THREAD_LOCAL" @@ -209,24 +215,16 @@ class AuthHook(hooks.PecanHook): if state.request.path in ALLOWED_WITHOUT_AUTH: return - if CONF.pecan.auth_enable: - # Note(nmakhotkin): Since we have deferred authentication, - # need to check for auth manually (check for corresponding - # headers according to keystonemiddleware docs. - identity_status = state.request.headers.get('X-Identity-Status') - service_identity_status = state.request.headers.get( - 'X-Service-Identity-Status' - ) + if not CONF.pecan.auth_enable: + return - if (identity_status == 'Confirmed' - or service_identity_status == 'Confirmed'): - return - - if state.request.headers.get('X-Auth-Token'): - msg = ("Auth token is invalid: %s" - % state.request.headers['X-Auth-Token']) - else: - msg = 'Authentication required' + try: + if CONF.auth_type == 'keystone': + authenticate_with_keystone(state.request) + elif CONF.auth_type == 'keycloak-oidc': + authenticate_with_keycloak(state.request) + except Exception as e: + msg = "Failed to validate access token: %s" % str(e) pecan.abort( status_code=401, @@ -235,6 +233,54 @@ class AuthHook(hooks.PecanHook): ) +def authenticate_with_keystone(req): + # Note(nmakhotkin): Since we have deferred authentication, + # need to check for auth manually (check for corresponding + # headers according to keystonemiddleware docs. + identity_status = req.headers.get('X-Identity-Status') + service_identity_status = req.headers.get('X-Service-Identity-Status') + + if (identity_status == 'Confirmed' or + service_identity_status == 'Confirmed'): + return + + if req.headers.get('X-Auth-Token'): + msg = 'Auth token is invalid: %s' % req.headers['X-Auth-Token'] + else: + msg = 'Authentication required' + + raise exc.UnauthorizedException(msg) + + +def authenticate_with_keycloak(req): + realm_name = req.headers.get('X-Project-Id') + + # NOTE(rakhmerov): There's a special endpoint for introspecting + # access tokens described in OpenID Connect specification but it's + # available in KeyCloak starting only with version 1.8.Final so we have + # to use user info endpoint which also takes exactly one parameter + # (access token) and replies with error if token is invalid. + user_info_endpoint = ( + "%s/realms/%s/protocol/openid-connect/userinfo" % + (CONF.keycloak_oidc.auth_url, realm_name) + ) + + access_token = req.headers.get('X-Auth-Token') + + resp = requests.get( + user_info_endpoint, + headers={"Authorization": "Bearer %s" % access_token}, + verify=not CONF.keycloak_oidc.insecure + ) + + resp.raise_for_status() + + LOG.debug( + "HTTP response from OIDC provider: %s" % + pprint.pformat(resp.json()) + ) + + class ContextHook(hooks.PecanHook): def before(self, state): set_ctx(context_from_headers(state.request.headers)) diff --git a/mistral/exceptions.py b/mistral/exceptions.py index 4b35252e..744ae78e 100644 --- a/mistral/exceptions.py +++ b/mistral/exceptions.py @@ -183,3 +183,8 @@ class CoordinationException(MistralException): class NotAllowedException(MistralException): http_code = 403 message = "Operation not allowed" + + +class UnauthorizedException(MistralException): + http_code = 401 + message = "Unauthorized" diff --git a/mistral/tests/unit/api/base.py b/mistral/tests/unit/api/base.py index 4f61db3d..04c703f1 100644 --- a/mistral/tests/unit/api/base.py +++ b/mistral/tests/unit/api/base.py @@ -24,7 +24,7 @@ from mistral.services import periodic from mistral.tests.unit import base from mistral.tests.unit.mstrlfixtures import policy_fixtures -# Disable authentication for functional tests. +# Disable authentication for API tests. cfg.CONF.set_default('auth_enable', False, group='pecan') diff --git a/mistral/tests/unit/api/test_access_control.py b/mistral/tests/unit/api/test_access_control.py index 13732ca5..e2f4d7ff 100644 --- a/mistral/tests/unit/api/test_access_control.py +++ b/mistral/tests/unit/api/test_access_control.py @@ -22,7 +22,9 @@ class PolicyTestCase(base.BaseTest): """Tests whether the configuration of the policy engine is corect.""" def setUp(self): super(PolicyTestCase, self).setUp() + self.policy = self.useFixture(policy_fixtures.PolicyFixture()) + rules = { "admin_only": "is_admin:True", "admin_or_owner": "is_admin:True or project_id:%(project_id)s", @@ -30,16 +32,19 @@ class PolicyTestCase(base.BaseTest): "example:admin": "rule:admin_only", "example:admin_or_owner": "rule:admin_or_owner" } + self.policy.set_rules(rules) def test_admin_api_allowed(self): auth_ctx = base.get_context(default=True, admin=True) + self.assertTrue( acl.enforce('example:admin', auth_ctx, auth_ctx.to_dict()) ) def test_admin_api_disallowed(self): auth_ctx = base.get_context(default=True) + self.assertRaises( exc.NotAllowedException, acl.enforce, @@ -50,6 +55,7 @@ class PolicyTestCase(base.BaseTest): def test_admin_or_owner_api_allowed(self): auth_ctx = base.get_context(default=True) + self.assertTrue( acl.enforce('example:admin_or_owner', auth_ctx, auth_ctx.to_dict()) ) @@ -57,6 +63,7 @@ class PolicyTestCase(base.BaseTest): def test_admin_or_owner_api_disallowed(self): auth_ctx = base.get_context(default=True) target = {'project_id': 'another'} + self.assertRaises( exc.NotAllowedException, acl.enforce, diff --git a/mistral/tests/unit/api/v2/test_keycloak_auth.py b/mistral/tests/unit/api/v2/test_keycloak_auth.py new file mode 100644 index 00000000..e099f3a6 --- /dev/null +++ b/mistral/tests/unit/api/v2/test_keycloak_auth.py @@ -0,0 +1,167 @@ +# -*- coding: utf-8 -*- +# +# Copyright 2013 - Mirantis, Inc. +# +# 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. + +import datetime +import mock +from oslo_config import cfg +import pecan +import pecan.testing +import requests_mock + +from mistral.db.v2 import api as db_api +from mistral.db.v2.sqlalchemy import models +from mistral.services import periodic +from mistral.tests.unit import base +from mistral.tests.unit.mstrlfixtures import policy_fixtures + + +WF_DEFINITION = """ +--- +version: '2.0' + +flow: + type: direct + input: + - param1 + + tasks: + task1: + action: std.echo output="Hi" +""" + +WF_DB = models.WorkflowDefinition( + id='123e4567-e89b-12d3-a456-426655440000', + name='flow', + definition=WF_DEFINITION, + created_at=datetime.datetime(1970, 1, 1), + updated_at=datetime.datetime(1970, 1, 1), + spec={'input': ['param1']} +) + +WF = { + 'id': '123e4567-e89b-12d3-a456-426655440000', + 'name': 'flow', + 'definition': WF_DEFINITION, + 'created_at': '1970-01-01 00:00:00', + 'updated_at': '1970-01-01 00:00:00', + 'input': 'param1' +} + + +MOCK_WF = mock.MagicMock(return_value=WF_DB) + +# Set up config options. +AUTH_URL = 'https://my.keycloak.com:8443/auth' +REALM_NAME = 'my_realm' + +USER_INFO_ENDPOINT = ( + "%s/realms/%s/protocol/openid-connect/userinfo" % (AUTH_URL, REALM_NAME) +) + +USER_CLAIMS = { + "sub": "248289761001", + "name": "Jane Doe", + "given_name": "Jane", + "family_name": "Doe", + "preferred_username": "j.doe", + "email": "janedoe@example.com", + "picture": "http://example.com/janedoe/me.jpg" +} + + +class TestKeyCloakOIDCAuth(base.DbTestCase): + def setUp(self): + super(TestKeyCloakOIDCAuth, self).setUp() + + cfg.CONF.set_default('auth_enable', True, group='pecan') + cfg.CONF.set_default('auth_type', 'keycloak-oidc') + cfg.CONF.set_default('auth_url', AUTH_URL, group='keycloak_oidc') + + pecan_opts = cfg.CONF.pecan + + self.app = pecan.testing.load_test_app({ + 'app': { + 'root': pecan_opts.root, + 'modules': pecan_opts.modules, + 'debug': pecan_opts.debug, + 'auth_enable': True, + 'disable_cron_trigger_thread': True + } + }) + + self.addCleanup(pecan.set_config, {}, overwrite=True) + self.addCleanup( + cfg.CONF.set_default, + 'auth_enable', + False, + group='pecan' + ) + self.addCleanup(cfg.CONF.set_default, 'auth_type', 'keystone') + + # Adding cron trigger thread clean up explicitly in case if + # new tests will provide an alternative configuration for pecan + # application. + self.addCleanup(periodic.stop_all_periodic_tasks) + + # Make sure the api get the correct context. + self.patch_ctx = mock.patch('mistral.context.context_from_headers') + self.mock_ctx = self.patch_ctx.start() + self.mock_ctx.return_value = self.ctx + self.addCleanup(self.patch_ctx.stop) + + self.policy = self.useFixture(policy_fixtures.PolicyFixture()) + + @requests_mock.Mocker() + @mock.patch.object(db_api, 'get_workflow_definition', MOCK_WF) + def test_get_workflow_success_auth(self, req_mock): + # Imitate successful response from KeyCloak with user claims. + req_mock.get(USER_INFO_ENDPOINT, json=USER_CLAIMS) + + headers = { + 'X-Auth-Token': 'cvbcvbasrtqlwkjasdfasdf', + 'X-Project-Id': REALM_NAME + } + + resp = self.app.get('/v2/workflows/123', headers=headers) + + self.assertEqual(200, resp.status_code) + self.assertDictEqual(WF, resp.json) + + @requests_mock.Mocker() + @mock.patch.object(db_api, 'get_workflow_definition', MOCK_WF) + def test_get_workflow_failed_auth(self, req_mock): + # Imitate failure response from KeyCloak. + req_mock.get( + USER_INFO_ENDPOINT, + status_code=401, + reason='Access token is invalid' + ) + + headers = { + 'X-Auth-Token': 'cvbcvbasrtqlwkjasdfasdf', + 'X-Project-Id': REALM_NAME + } + + resp = self.app.get( + '/v2/workflows/123', + headers=headers, + expect_errors=True + ) + + self.assertEqual(401, resp.status_code) + self.assertEqual('401 Unauthorized', resp.status) + self.assertIn('Failed to validate access token', resp.text) + self.assertIn('Access token is invalid', resp.text) diff --git a/mistral/tests/unit/mstrlfixtures/policy_fixtures.py b/mistral/tests/unit/mstrlfixtures/policy_fixtures.py index 9ad8f243..f3630152 100644 --- a/mistral/tests/unit/mstrlfixtures/policy_fixtures.py +++ b/mistral/tests/unit/mstrlfixtures/policy_fixtures.py @@ -28,23 +28,30 @@ class PolicyFixture(fixtures.Fixture): 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.policy_data) + policy_opts.set_defaults(cfg.CONF) + cfg.CONF.set_override( 'policy_file', self.policy_file_name, 'oslo_policy' ) + acl._ENFORCER = oslo_policy.Enforcer(cfg.CONF) acl._ENFORCER.load_rules() + self.addCleanup(acl._ENFORCER.clear) def set_rules(self, rules): policy = acl._ENFORCER + policy.set_rules(oslo_policy.Rules.from_dict(rules)) diff --git a/test-requirements.txt b/test-requirements.txt index 41536f85..48f2fba0 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -9,6 +9,7 @@ oslosphinx!=3.4.0,>=2.5.0 # Apache-2.0 oslotest>=1.10.0 # Apache-2.0 pyflakes==0.8.1 # MIT pylint==1.4.5 # GPLv2 +requests-mock>=1.0 # Apache-2.0 sphinx!=1.3b1,<1.3,>=1.2.1 # BSD sphinxcontrib-httpdomain # BSD sphinxcontrib-pecanwsme>=0.8 # Apache-2.0