From 02b41f297f5c96bcf85da840357da3810f5a0d89 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Sun, 9 Jul 2017 16:31:22 +0300 Subject: [PATCH] Improve keycloak auth module This patch adds new improvements to the keycloak auth module: * Token format is validated before sending it to keycloak server. * Realm name is taken from parsed token, 'X-PROJECT-ID' header is not required anymore. * Added support of user roles - now Mistral understands the difference between admin and regular user. * Added more detailed error explanations and new unit tests. Change-Id: I7ac4834f2ecb4cafb9d4fcd154a8cf41a71e6c4a --- mistral/auth/keycloak.py | 48 ++++- .../tests/unit/api/v2/test_keycloak_auth.py | 186 ++++++++++++++++-- requirements.txt | 1 + 3 files changed, 209 insertions(+), 26 deletions(-) diff --git a/mistral/auth/keycloak.py b/mistral/auth/keycloak.py index de07f0871..d3d82d9af 100644 --- a/mistral/auth/keycloak.py +++ b/mistral/auth/keycloak.py @@ -12,12 +12,15 @@ # See the License for the specific language governing permissions and # limitations under the License. +import jwt from oslo_config import cfg from oslo_log import log as logging import pprint import requests +from mistral._i18n import _ from mistral import auth +from mistral import exceptions as exc LOG = logging.getLogger(__name__) @@ -28,7 +31,28 @@ CONF = cfg.CONF class KeycloakAuthHandler(auth.AuthHandler): def authenticate(self, req): - realm_name = req.headers.get('X-Project-Id') + + if 'X-Auth-Token' not in req.headers: + msg = _("Auth token must be provided in 'X-Auth-Token' header.") + LOG.error(msg) + raise exc.UnauthorizedException(message=msg) + access_token = req.headers.get('X-Auth-Token') + + try: + decoded = jwt.decode(access_token, algorithms=['RS256'], + verify=False) + except Exception: + msg = _("Token can't be decoded because of wrong format.") + LOG.error(msg) + raise exc.UnauthorizedException(message=msg) + + # Get user realm from parsed token + # Format is "iss": "http://:/auth/realms/", + __, __, realm_name = decoded['iss'].strip().rpartition('/realms/') + + # Get roles from from parsed token + roles = ','.join(decoded['realm_access']['roles']) \ + if 'realm_access' in decoded else '' # NOTE(rakhmerov): There's a special endpoint for introspecting # access tokens described in OpenID Connect specification but it's @@ -40,13 +64,17 @@ class KeycloakAuthHandler(auth.AuthHandler): (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 - ) + try: + resp = requests.get( + user_info_endpoint, + headers={"Authorization": "Bearer %s" % access_token}, + verify=not CONF.keycloak_oidc.insecure + ) + except requests.ConnectionError: + msg = _("Can't connect to keycloak server with address '%s'." + ) % CONF.keycloak_oidc.auth_url + LOG.error(msg) + raise exc.MistralException(message=msg) resp.raise_for_status() @@ -54,3 +82,7 @@ class KeycloakAuthHandler(auth.AuthHandler): "HTTP response from OIDC provider: %s" % pprint.pformat(resp.json()) ) + + req.headers["X-Identity-Status"] = "Confirmed" + req.headers["X-Project-Id"] = realm_name + req.headers["X-Roles"] = roles diff --git a/mistral/tests/unit/api/v2/test_keycloak_auth.py b/mistral/tests/unit/api/v2/test_keycloak_auth.py index f94c8fe0f..8950352fb 100644 --- a/mistral/tests/unit/api/v2/test_keycloak_auth.py +++ b/mistral/tests/unit/api/v2/test_keycloak_auth.py @@ -1,4 +1,4 @@ -# Copyright 2013 - Mirantis, Inc. +# Copyright 2017 - Nokia Networks # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -17,10 +17,14 @@ import mock from oslo_config import cfg import pecan import pecan.testing +import requests import requests_mock +import webob +from mistral.auth import keycloak from mistral.db.v2 import api as db_api from mistral.db.v2.sqlalchemy import models +from mistral import exceptions as exc from mistral.services import periodic from mistral.tests.unit import base from mistral.tests.unit.mstrlfixtures import policy_fixtures @@ -29,12 +33,10 @@ 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" @@ -80,9 +82,112 @@ USER_CLAIMS = { } -class TestKeyCloakOIDCAuth(base.DbTestCase): +class TestKeyCloakOIDCAuth(base.BaseTest): + def setUp(self): super(TestKeyCloakOIDCAuth, self).setUp() + cfg.CONF.set_default('auth_url', AUTH_URL, group='keycloak_oidc') + self.auth_handler = keycloak.KeycloakAuthHandler() + + def _build_request(self, token): + req = webob.Request.blank("/") + req.headers["x-auth-token"] = token + req.get_response = lambda app: None + return req + + @requests_mock.Mocker() + def test_header_parsing(self, req_mock): + token = { + "iss": "http://localhost:8080/auth/realms/my_realm", + "realm_access": { + "roles": ["role1", "role2"] + } + } + # Imitate successful response from KeyCloak with user claims. + req_mock.get(USER_INFO_ENDPOINT, json=USER_CLAIMS) + + req = self._build_request(token) + with mock.patch("jwt.decode", return_value=token): + self.auth_handler.authenticate(req) + self.assertEqual("Confirmed", req.headers["X-Identity-Status"]) + self.assertEqual("my_realm", req.headers["X-Project-Id"]) + self.assertEqual("role1,role2", req.headers["X-Roles"]) + self.assertEqual(1, req_mock.call_count) + + def test_no_auth_token(self): + req = webob.Request.blank("/") + self.assertRaises( + exc.UnauthorizedException, + self.auth_handler.authenticate, + req + ) + + @requests_mock.Mocker() + def test_no_realm_roles(self, req_mock): + token = { + "iss": "http://localhost:8080/auth/realms/my_realm", + } + # Imitate successful response from KeyCloak with user claims. + req_mock.get(USER_INFO_ENDPOINT, json=USER_CLAIMS) + + req = self._build_request(token) + with mock.patch("jwt.decode", return_value=token): + self.auth_handler.authenticate(req) + self.assertEqual("Confirmed", req.headers["X-Identity-Status"]) + self.assertEqual("my_realm", req.headers["X-Project-Id"]) + self.assertEqual("", req.headers["X-Roles"]) + + def test_wrong_token_format(self): + req = self._build_request(token="WRONG_FORMAT_TOKEN") + self.assertRaises( + exc.UnauthorizedException, + self.auth_handler.authenticate, + req + ) + + @requests_mock.Mocker() + def test_server_unauthorized(self, req_mock): + token = { + "iss": "http://localhost:8080/auth/realms/my_realm", + } + # Imitate failure response from KeyCloak. + req_mock.get( + USER_INFO_ENDPOINT, + status_code=401, + reason='Access token is invalid' + ) + + req = self._build_request(token) + with mock.patch("jwt.decode", return_value=token): + try: + self.auth_handler.authenticate(req) + except requests.exceptions.HTTPError as e: + self.assertIn( + "401 Client Error: Access token is invalid for url", + str(e) + ) + else: + raise Exception("Test is broken") + + @requests_mock.Mocker() + def test_connection_error(self, req_mock): + token = { + "iss": "http://localhost:8080/auth/realms/my_realm", + } + req_mock.get(USER_INFO_ENDPOINT, exc=requests.ConnectionError) + + req = self._build_request(token) + with mock.patch("jwt.decode", return_value=token): + self.assertRaises( + exc.MistralException, + self.auth_handler.authenticate, + req + ) + + +class TestKeyCloakOIDCAuthScenarios(base.DbTestCase): + def setUp(self): + super(TestKeyCloakOIDCAuthScenarios, self).setUp() cfg.CONF.set_default('auth_enable', True, group='pecan') cfg.CONF.set_default('auth_type', 'keycloak-oidc') @@ -130,29 +235,38 @@ class TestKeyCloakOIDCAuth(base.DbTestCase): # 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 + token = { + "iss": "http://localhost:8080/auth/realms/%s" % REALM_NAME, + "realm_access": { + "roles": ["role1", "role2"] + } } - resp = self.app.get('/v2/workflows/123', headers=headers) + headers = { + 'X-Auth-Token': str(token) + } + + with mock.patch("jwt.decode", return_value=token): + 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("requests.get") @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' - ) + def test_get_workflow_invalid_token_format(self, req_mock): + # Imitate successful response from KeyCloak with user claims. + req_mock.get(USER_INFO_ENDPOINT, json=USER_CLAIMS) + + token = { + "iss": "http://localhost:8080/auth/realms/%s" % REALM_NAME, + "realm_access": { + "roles": ["role1", "role2"] + } + } headers = { - 'X-Auth-Token': 'cvbcvbasrtqlwkjasdfasdf', - 'X-Project-Id': REALM_NAME + 'X-Auth-Token': str(token) } resp = self.app.get( @@ -161,6 +275,42 @@ class TestKeyCloakOIDCAuth(base.DbTestCase): 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( + "Token can't be decoded because of wrong format.", + resp.text + ) + + @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' + ) + + token = { + "iss": "http://localhost:8080/auth/realms/%s" % REALM_NAME, + "realm_access": { + "roles": ["role1", "role2"] + } + } + + headers = { + 'X-Auth-Token': str(token) + } + + with mock.patch("jwt.decode", return_value=token): + 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) diff --git a/requirements.txt b/requirements.txt index 0e0ad6e99..d8a18261b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -49,6 +49,7 @@ python-troveclient>=2.2.0 # Apache-2.0 python-ironicclient>=1.11.0 # Apache-2.0 python-ironic-inspector-client>=1.5.0 # Apache-2.0 python-zaqarclient>=1.0.0 # Apache-2.0 +PyJWT>=1.0.1 # MIT PyYAML>=3.10.0 # MIT requests>=2.14.2 # Apache-2.0 tenacity>=3.2.1 # Apache-2.0