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

@@ -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)
+        options = self._get_request_options('post', headers)
-        content_type = headers.get('content-type', 'application/json')
-        headers['content-type'] = content_type
 
-        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)
+        options = self._get_request_options('put', headers)
-        content_type = headers.get('content-type', 'application/json')
-        headers['content-type'] = content_type
 
-        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:

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.

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):
+    def test_mistral_url_default(self, mock, keystone_client_mock):
-        client.client(username='mistral',
+        keystone_client_instance = keystone_client_mock.return_value
-                      project_name='misteal',
+        keystone_client_instance.auth_token = str(uuid.uuid4())
-                      auth_url="http://localhost:35357/v3")
+        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(mock.call_args[0], expected_args)
-        self.assertEqual('http://localhost:8989/v2',
+        self.assertDictEqual(mock.call_args[1], expected_kwargs)
-                         params[0][0])
+
+    @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)

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
+        )