From 7e9a4e8a48cba6f674873563f44d79cb9cbbeef5 Mon Sep 17 00:00:00 2001 From: Winson Chan Date: Wed, 2 Mar 2016 02:51:15 +0000 Subject: [PATCH] Fix cacert and insecure options on HTTP requests On running mistral API with SSL, the client SSL options such as cacert and insecure are only passed to the authentication request and are not passed to the mistral client and mistral API subsequently. Change-Id: I35d013c468977fc3df578afcacc7f7a5cc3173c6 Closes-Bug: #1552030 --- mistralclient/api/httpclient.py | 50 +++- mistralclient/api/v2/client.py | 4 +- mistralclient/tests/unit/test_client.py | 161 +++++++++++- mistralclient/tests/unit/test_httpclient.py | 256 ++++++++++++++++++++ 4 files changed, 450 insertions(+), 21 deletions(-) create mode 100644 mistralclient/tests/unit/test_httpclient.py diff --git a/mistralclient/api/httpclient.py b/mistralclient/api/httpclient.py index 4fcfa1ff..6dd59565 100644 --- a/mistralclient/api/httpclient.py +++ b/mistralclient/api/httpclient.py @@ -1,4 +1,5 @@ # Copyright 2013 - Mirantis, Inc. +# Copyright 2016 - StackStorm, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -12,6 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy +import os + import requests import logging @@ -30,39 +34,61 @@ def log_request(func): class HTTPClient(object): - def __init__(self, base_url, token=None, project_id=None, user_id=None): + def __init__(self, base_url, token=None, project_id=None, user_id=None, + cacert=None, insecure=False): self.base_url = base_url self.token = token self.project_id = project_id self.user_id = user_id + self.ssl_options = {} + + if self.base_url.startswith('https'): + if cacert and not os.path.exists(cacert): + raise ValueError('Unable to locate cacert file ' + 'at %s.' % cacert) + + if cacert and insecure: + LOG.warning('Client is set to not verify even though ' + 'cacert is provided.') + + self.ssl_options['verify'] = not insecure + self.ssl_options['cert'] = cacert @log_request def get(self, url, headers=None): - headers = self._update_headers(headers) + options = self._get_request_options('get', headers) - return requests.get(self.base_url + url, headers=headers) + return requests.get(self.base_url + url, **options) @log_request def post(self, url, body, headers=None): - headers = self._update_headers(headers) - content_type = headers.get('content-type', 'application/json') - headers['content-type'] = content_type + options = self._get_request_options('post', headers) - return requests.post(self.base_url + url, body, headers=headers) + return requests.post(self.base_url + url, body, **options) @log_request def put(self, url, body, headers=None): - headers = self._update_headers(headers) - content_type = headers.get('content-type', 'application/json') - headers['content-type'] = content_type + options = self._get_request_options('put', headers) - return requests.put(self.base_url + url, body, headers=headers) + return requests.put(self.base_url + url, body, **options) @log_request def delete(self, url, headers=None): + options = self._get_request_options('delete', headers) + + return requests.delete(self.base_url + url, **options) + + def _get_request_options(self, method, headers): headers = self._update_headers(headers) - return requests.delete(self.base_url + url, headers=headers) + if method in ['post', 'put']: + content_type = headers.get('content-type', 'application/json') + headers['content-type'] = content_type + + options = copy.deepcopy(self.ssl_options) + options['headers'] = headers + + return options def _update_headers(self, headers): if not headers: diff --git a/mistralclient/api/v2/client.py b/mistralclient/api/v2/client.py index fff6e866..018876e8 100644 --- a/mistralclient/api/v2/client.py +++ b/mistralclient/api/v2/client.py @@ -62,7 +62,9 @@ class Client(object): mistral_url, auth_token, project_id, - user_id + user_id, + cacert=cacert, + insecure=insecure ) # Create all resource managers. diff --git a/mistralclient/tests/unit/test_client.py b/mistralclient/tests/unit/test_client.py index 0d8e2166..c3a90434 100644 --- a/mistralclient/tests/unit/test_client.py +++ b/mistralclient/tests/unit/test_client.py @@ -1,4 +1,5 @@ -# Copyright 2015 Huawei Technologies Co., Ltd. +# Copyright 2015 - Huawei Technologies Co., Ltd. +# Copyright 2016 - StackStorm, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -12,21 +13,165 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os +import tempfile +import uuid + import mock import testtools from mistralclient.api import client +AUTH_HTTP_URL = 'http://localhost:35357/v3' +AUTH_HTTPS_URL = AUTH_HTTP_URL.replace('http', 'https') +MISTRAL_HTTP_URL = 'http://localhost:8989/v2' +MISTRAL_HTTPS_URL = MISTRAL_HTTP_URL.replace('http', 'https') + class BaseClientTests(testtools.TestCase): @mock.patch('keystoneclient.v3.client.Client') @mock.patch('mistralclient.api.httpclient.HTTPClient') - def test_mistral_url_defult(self, mock, keystone_client_mock): - client.client(username='mistral', - project_name='misteal', - auth_url="http://localhost:35357/v3") + def test_mistral_url_default(self, mock, keystone_client_mock): + keystone_client_instance = keystone_client_mock.return_value + keystone_client_instance.auth_token = str(uuid.uuid4()) + keystone_client_instance.project_id = str(uuid.uuid4()) + keystone_client_instance.user_id = str(uuid.uuid4()) + + expected_args = ( + MISTRAL_HTTP_URL, + keystone_client_instance.auth_token, + keystone_client_instance.project_id, + keystone_client_instance.user_id + ) + + expected_kwargs = { + 'cacert': None, + 'insecure': False + } + + client.client( + username='mistral', + project_name='mistral', + auth_url=AUTH_HTTP_URL + ) + self.assertTrue(mock.called) - params = mock.call_args - self.assertEqual('http://localhost:8989/v2', - params[0][0]) + self.assertEqual(mock.call_args[0], expected_args) + self.assertDictEqual(mock.call_args[1], expected_kwargs) + + @mock.patch('keystoneclient.v3.client.Client') + @mock.patch('mistralclient.api.httpclient.HTTPClient') + def test_mistral_url_https_insecure(self, mock, keystone_client_mock): + keystone_client_instance = keystone_client_mock.return_value + keystone_client_instance.auth_token = str(uuid.uuid4()) + keystone_client_instance.project_id = str(uuid.uuid4()) + keystone_client_instance.user_id = str(uuid.uuid4()) + + expected_args = ( + MISTRAL_HTTPS_URL, + keystone_client_instance.auth_token, + keystone_client_instance.project_id, + keystone_client_instance.user_id + ) + + expected_kwargs = { + 'cacert': None, + 'insecure': True + } + + client.client( + mistral_url=MISTRAL_HTTPS_URL, + username='mistral', + project_name='mistral', + auth_url=AUTH_HTTP_URL, + cacert=None, + insecure=True + ) + + self.assertTrue(mock.called) + self.assertEqual(mock.call_args[0], expected_args) + self.assertDictEqual(mock.call_args[1], expected_kwargs) + + @mock.patch('keystoneclient.v3.client.Client') + @mock.patch('mistralclient.api.httpclient.HTTPClient') + def test_mistral_url_https_secure(self, mock, keystone_client_mock): + fd, path = tempfile.mkstemp(suffix='.pem') + + keystone_client_instance = keystone_client_mock.return_value + keystone_client_instance.auth_token = str(uuid.uuid4()) + keystone_client_instance.project_id = str(uuid.uuid4()) + keystone_client_instance.user_id = str(uuid.uuid4()) + + expected_args = ( + MISTRAL_HTTPS_URL, + keystone_client_instance.auth_token, + keystone_client_instance.project_id, + keystone_client_instance.user_id + ) + + expected_kwargs = { + 'cacert': path, + 'insecure': False + } + + try: + client.client( + mistral_url=MISTRAL_HTTPS_URL, + username='mistral', + project_name='mistral', + auth_url=AUTH_HTTP_URL, + cacert=path, + insecure=False + ) + finally: + os.close(fd) + os.unlink(path) + + self.assertTrue(mock.called) + self.assertEqual(mock.call_args[0], expected_args) + self.assertDictEqual(mock.call_args[1], expected_kwargs) + + @mock.patch('keystoneclient.v3.client.Client') + def test_mistral_url_https_bad_cacert(self, keystone_client_mock): + keystone_client_instance = keystone_client_mock.return_value + keystone_client_instance.auth_token = str(uuid.uuid4()) + keystone_client_instance.project_id = str(uuid.uuid4()) + keystone_client_instance.user_id = str(uuid.uuid4()) + + self.assertRaises( + ValueError, + client.client, + mistral_url=MISTRAL_HTTPS_URL, + username='mistral', + project_name='mistral', + auth_url=AUTH_HTTP_URL, + cacert='/path/to/foobar', + insecure=False + ) + + @mock.patch('logging.Logger.warning') + @mock.patch('keystoneclient.v3.client.Client') + def test_mistral_url_https_bad_insecure(self, keystone_client_mock, + log_warning_mock): + fd, path = tempfile.mkstemp(suffix='.pem') + + keystone_client_instance = keystone_client_mock.return_value + keystone_client_instance.auth_token = str(uuid.uuid4()) + keystone_client_instance.project_id = str(uuid.uuid4()) + keystone_client_instance.user_id = str(uuid.uuid4()) + + try: + client.client( + mistral_url=MISTRAL_HTTPS_URL, + username='mistral', + project_name='mistral', + auth_url=AUTH_HTTP_URL, + cacert=path, + insecure=True + ) + finally: + os.close(fd) + os.unlink(path) + + self.assertTrue(log_warning_mock.called) diff --git a/mistralclient/tests/unit/test_httpclient.py b/mistralclient/tests/unit/test_httpclient.py new file mode 100644 index 00000000..a3341e41 --- /dev/null +++ b/mistralclient/tests/unit/test_httpclient.py @@ -0,0 +1,256 @@ +# Copyright 2016 - StackStorm, 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 copy +import uuid + +import mock +import requests +import testtools + +from mistralclient.api import httpclient + +API_BASE_URL = 'http://localhost:8989/v2' +API_URL = '/executions' + +EXPECTED_URL = API_BASE_URL + API_URL + +AUTH_TOKEN = str(uuid.uuid4()) +PROJECT_ID = str(uuid.uuid4()) +USER_ID = str(uuid.uuid4()) + +EXPECTED_AUTH_HEADERS = { + 'x-auth-token': AUTH_TOKEN, + 'X-Project-Id': PROJECT_ID, + 'X-User-Id': USER_ID +} + +EXPECTED_REQ_OPTIONS = { + 'headers': EXPECTED_AUTH_HEADERS +} + +EXPECTED_BODY = { + 'k1': 'abc', + 'k2': 123, + 'k3': True +} + + +class FakeRequest(object): + + def __init__(self, method): + self.method = method + + +class FakeResponse(object): + + def __init__(self, method, url, status_code): + self.request = FakeRequest(method) + self.url = url + self.status_code = status_code + + +class HTTPClientTest(testtools.TestCase): + + def setUp(self): + super(HTTPClientTest, self).setUp() + self.client = httpclient.HTTPClient( + API_BASE_URL, + AUTH_TOKEN, + PROJECT_ID, + USER_ID + ) + + @mock.patch.object( + requests, + 'get', + mock.MagicMock(return_value=FakeResponse('get', EXPECTED_URL, 200)) + ) + def test_get_request_options(self): + self.client.get(API_URL) + + requests.get.assert_called_with( + EXPECTED_URL, + **EXPECTED_REQ_OPTIONS + ) + + @mock.patch.object( + requests, + 'get', + mock.MagicMock(return_value=FakeResponse('get', EXPECTED_URL, 200)) + ) + def test_get_request_options_with_headers_for_get(self): + headers = {'foo': 'bar'} + + self.client.get(API_URL, headers=headers) + + expected_options = copy.deepcopy(EXPECTED_REQ_OPTIONS) + expected_options['headers'].update(headers) + + requests.get.assert_called_with( + EXPECTED_URL, + **expected_options + ) + + @mock.patch.object( + requests, + 'post', + mock.MagicMock(return_value=FakeResponse('post', EXPECTED_URL, 201)) + ) + def test_get_request_options_with_headers_for_post(self): + headers = {'foo': 'bar'} + + self.client.post(API_URL, EXPECTED_BODY, headers=headers) + + expected_options = copy.deepcopy(EXPECTED_REQ_OPTIONS) + expected_options['headers'].update(headers) + expected_options['headers']['content-type'] = 'application/json' + + requests.post.assert_called_with( + EXPECTED_URL, + EXPECTED_BODY, + **expected_options + ) + + @mock.patch.object( + requests, + 'put', + mock.MagicMock(return_value=FakeResponse('put', EXPECTED_URL, 200)) + ) + def test_get_request_options_with_headers_for_put(self): + headers = {'foo': 'bar'} + + self.client.put(API_URL, EXPECTED_BODY, headers=headers) + + expected_options = copy.deepcopy(EXPECTED_REQ_OPTIONS) + expected_options['headers'].update(headers) + expected_options['headers']['content-type'] = 'application/json' + + requests.put.assert_called_with( + EXPECTED_URL, + EXPECTED_BODY, + **expected_options + ) + + @mock.patch.object( + requests, + 'delete', + mock.MagicMock(return_value=FakeResponse('delete', EXPECTED_URL, 200)) + ) + def test_get_request_options_with_headers_for_delete(self): + headers = {'foo': 'bar'} + + self.client.delete(API_URL, headers=headers) + + expected_options = copy.deepcopy(EXPECTED_REQ_OPTIONS) + expected_options['headers'].update(headers) + + requests.delete.assert_called_with( + EXPECTED_URL, + **expected_options + ) + + @mock.patch.object( + httpclient.HTTPClient, + '_get_request_options', + mock.MagicMock(return_value=copy.deepcopy(EXPECTED_REQ_OPTIONS)) + ) + @mock.patch.object( + requests, + 'get', + mock.MagicMock(return_value=FakeResponse('get', EXPECTED_URL, 200)) + ) + def test_http_get(self): + self.client.get(API_URL) + + httpclient.HTTPClient._get_request_options.assert_called_with( + 'get', + None + ) + + requests.get.assert_called_with( + EXPECTED_URL, + **EXPECTED_REQ_OPTIONS + ) + + @mock.patch.object( + httpclient.HTTPClient, + '_get_request_options', + mock.MagicMock(return_value=copy.deepcopy(EXPECTED_REQ_OPTIONS)) + ) + @mock.patch.object( + requests, + 'post', + mock.MagicMock(return_value=FakeResponse('post', EXPECTED_URL, 201)) + ) + def test_http_post(self): + self.client.post(API_URL, EXPECTED_BODY) + + httpclient.HTTPClient._get_request_options.assert_called_with( + 'post', + None + ) + + requests.post.assert_called_with( + EXPECTED_URL, + EXPECTED_BODY, + **EXPECTED_REQ_OPTIONS + ) + + @mock.patch.object( + httpclient.HTTPClient, + '_get_request_options', + mock.MagicMock(return_value=copy.deepcopy(EXPECTED_REQ_OPTIONS)) + ) + @mock.patch.object( + requests, + 'put', + mock.MagicMock(return_value=FakeResponse('put', EXPECTED_URL, 200)) + ) + def test_http_put(self): + self.client.put(API_URL, EXPECTED_BODY) + + httpclient.HTTPClient._get_request_options.assert_called_with( + 'put', + None + ) + + requests.put.assert_called_with( + EXPECTED_URL, + EXPECTED_BODY, + **EXPECTED_REQ_OPTIONS + ) + + @mock.patch.object( + httpclient.HTTPClient, + '_get_request_options', + mock.MagicMock(return_value=copy.deepcopy(EXPECTED_REQ_OPTIONS)) + ) + @mock.patch.object( + requests, + 'delete', + mock.MagicMock(return_value=FakeResponse('delete', EXPECTED_URL, 200)) + ) + def test_http_delete(self): + self.client.delete(API_URL) + + httpclient.HTTPClient._get_request_options.assert_called_with( + 'delete', + None + ) + + requests.delete.assert_called_with( + EXPECTED_URL, + **EXPECTED_REQ_OPTIONS + )