Various fixes and small enhancements

cfssl/cfssl.py

@@ -3,10 +3,14 @@
 # License MIT (https://opensource.org/licenses/MIT).
 
 import requests
+import logging
 
 from .exceptions import CFSSLException, CFSSLRemoteException
 
 from .models.config_key import ConfigKey
+from .utils import to_api
+
+log = logging.getLogger(__name__)
 
 
 class CFSSL(object):
@@ -45,7 +49,7 @@ def auth_sign(self, token, request, datetime=None, remote_address=None):
         """
         data = self._clean_mapping({
             'token': token,
-            'request': request.to_api(),
+            'request': to_api(request),
             'datetime': datetime,
             'remote_address': remote_address,
         })
@@ -183,13 +187,13 @@ def init_ca(self, certificate_request, ca=None):
                 * private key (str): a PEM-encoded CA private key.
                 * certificate (str): a PEM-encoded self-signed CA certificate.
         """
-        csr_api = certificate_request.to_api()
+        csr_api = to_api(certificate_request)
         data = self._clean_mapping({
             'hosts': csr_api['hosts'],
             'names': csr_api['names'],
             'CN': csr_api['CN'],
             'key': csr_api['key'],
-            'ca': ca and ca.to_api() or None,
+            'ca': ca and to_api(ca) or None,
         })
         return self.call('init_ca', 'POST', data=data)
 
@@ -214,14 +218,14 @@ def new_key(self, hosts, names, common_name=None, key=None, ca=None):
         """
         data = self._clean_mapping({
             'hosts': [
-                host.to_api() for host in hosts
+                to_api(host) for host in hosts
             ],
             'names': [
-                name.to_api() for name in names
+                to_api(name) for name in names
             ],
             'CN': common_name,
-            'key': key and key.to_api() or ConfigKey().to_api(),
-            'ca': ca and ca.to_api() or None,
+            'key': key and to_api(key) or ConfigKey().to_api(),
+            'ca': ca and to_api(ca) or None,
         })
         return self.call('newkey', 'POST', data=data)
 
@@ -248,7 +252,7 @@ def new_cert(self, request, label=None, profile=None, bundle=None):
                     if the bundle parameter was set).
         """
         data = self._clean_mapping({
-            'request': request.to_api(),
+            'request': to_api(request),
             'label': label,
             'profile': profile,
             'bundle': bundle,
@@ -300,7 +304,7 @@ def scan(self, host, ip=None, timeout=None, family=None, scanner=None):
             * output: (dict) Arbitrary data retrieved during the scan.
         """
         data = self._clean_mapping({
-            'host': host.to_api(),
+            'host': to_api(host),
             'ip': ip,
             'timeout': timeout,
             'family': family,
@@ -342,14 +346,14 @@ def sign(self, certificate_request, hosts=None, subject=None,
                 server.
         """
         data = self._clean_mapping({
-            'certificate_request': certificate_request.to_api(),
+            'certificate_request': to_api(certificate_request),
             'hosts': [
-                host.to_api() for host in hosts
+                to_api(host) for host in hosts
             ],
             'subject': subject,
             'serial_sequence': serial_sequence,
             'label': label,
-            'profile': profile.to_api(),
+            'profile': to_api(profile),
         })
         result = self.call('sign', 'POST', data=data)
         return result['certificate']
@@ -375,21 +379,35 @@ def call(self, endpoint, method='GET', params=None, data=None):
             method=method,
             url=endpoint,
             params=params,
-            data=data,
+            json=data,
             verify=self.verify,
         )
         response = response.json()
         if not response['success']:
             raise CFSSLRemoteException(
                 '\n'.join([
                     'Errors:',
-                    '\n'.join(response.get('errors', [])),
+                    '\n'.join(map(CFSSL._format_response_message, response.get('errors', []))),
                     'Messages:'
-                    '\n'.join(response.get('messages', [])),
+                    '\n'.join(map(CFSSL._format_response_message, response.get('messages', []))),
                 ])
             )
+        if 'messages' in response:
+            for message in response['messages']:
+                log.warning(CFSSL._format_response_message(message))
         return response['result']
 
+    @staticmethod
+    def _format_response_message(error):
+        message = ''
+        if isinstance(error, dict):
+            message += error['message'] if 'message' in error else '<undefined message>'
+            if 'code' in error:
+                message += ' (%s)' % error['code']
+        if not message:
+            message = str(error)
+        return message
+
     def _clean_mapping(self, mapping):
         """ It removes false entries from mapping """
-        return {k:v for k, v in mapping.iteritems() if v}
+        return {k:v for k, v in mapping.items() if v}

cfssl/models/certificate_request.py

@@ -5,16 +5,17 @@
 from .host import Host
 from .config_key import ConfigKey
 from .subject_info import SubjectInfo
+from ..utils import to_api
 
 
 class CertificateRequest(object):
     """ It provides a Certificate Request compatible with CFSSL. """
 
-    def __init__(self, common_name, names=None, hosts=None, key=None):
+    def __init__(self, common_name=None, names=None, hosts=None, key=None):
         """ Initialize a new CertificateRequest.
 
         Args:
-            common_name (str): The fully qualified domain name for the
+            common_name (str, optional): The fully qualified domain name for the
                 server. This must be an exact match.
             names (tuple of SubjectInfo, optional):
                 Subject Information to be added to the request.
@@ -26,17 +27,20 @@ def __init__(self, common_name, names=None, hosts=None, key=None):
         self.common_name = common_name
         self.names = names or []
         self.hosts = hosts or []
-        self.key = key or KeyConfig()
+        self.key = key
 
     def to_api(self):
         """ It returns an object compatible with the API. """
-        return {
-            'CN': self.common_name,
+        api = {
             'names': [
-                name.to_api() for name in self.names
+                to_api(name) for name in self.names
             ],
             'hosts': [
-                host.to_api() for host in self.hosts
-            ],
-            'key': self.key.to_api(),
+                to_api(host) for host in self.hosts
+            ]
         }
+        if self.common_name:
+            api['CN'] = self.common_name
+        if self.key:
+            api['key'] = to_api(self.key)
+        return api

cfssl/models/config_client.py

@@ -3,6 +3,7 @@
 # License MIT (https://opensource.org/licenses/MIT).
 
 from .config_mixer import ConfigMixer
+from ..utils import to_api
 
 
 class ConfigClient(ConfigMixer):
@@ -31,6 +32,6 @@ def to_api(self):
         """ It returns an object compatible with the API. """
         res = super(ConfigClient, self).to_api()
         res['remotes'] = {
-            r.name: r.to_api() for r in self.remotes
+            r.name: to_api(r) for r in self.remotes
         }
         return res

cfssl/models/config_mixer.py

@@ -2,6 +2,8 @@
 # Copyright 2016 LasLabs Inc.
 # License MIT (https://opensource.org/licenses/MIT).
 
+from ..utils import to_api
+
 
 class ConfigMixer(object):
     """ It provides a mixer for the Client and Server Configs """
@@ -25,12 +27,12 @@ def to_api(self):
         """ It returns an object compatible with the API. """
         return {
             'signing': {
-                'default': self.sign_policy.to_api(),
+                'default': to_api(self.sign_policy),
                 'profiles': {
-                    p.name: p.to_api() for p in self.sign_policies
+                    p.name: to_api(p) for p in self.sign_policies
                 },
             },
             'auth_keys': {
-                k.name: k.to_api() for k in self.auth_policies
+                k.name: to_api(k) for k in self.auth_policies
             },
         }

cfssl/models/policy_sign.py

@@ -3,6 +3,7 @@
 # License MIT (https://opensource.org/licenses/MIT).
 
 from ..defaults import DEFAULT_EXPIRE_DELTA
+from ..utils import to_api
 
 
 class PolicySign(object):
@@ -31,5 +32,5 @@ def to_api(self):
         return {
             'auth_key': self.auth_policy.name,
             'expiry': '%ds' % self.expire_delta.total_seconds(),
-            'usages': [u.to_api() for u in self.usage_policies],
+            'usages': [to_api(u) for u in self.usage_policies],
         }

cfssl/models/subject_info.py

@@ -15,6 +15,8 @@ def __init__(self, org_name, org_unit, city, state, country):
             org_unit (str): Section of the organization.
             city (str): The city where the organization is legally
                 located.
+            state (str): The state or province where your organization
+                is legally located. Can not be abbreviated.
             country (str): The two letter ISO abbreviation for the
                 country.
         """

cfssl/tests/test_certificate_request.py

@@ -20,6 +20,12 @@ def setUp(self):
         }
         self.model = CertificateRequest(**self.vals)
 
+        self.partial_vals = {
+            'names': [mock.MagicMock()],
+            'hosts': [mock.MagicMock()],
+        }
+        self.model_partial = CertificateRequest(**self.partial_vals)
+
     def test_to_api(self):
         """ It should return the correctly compatible obj """
         res = self.model.to_api()
@@ -31,6 +37,15 @@ def test_to_api(self):
         }
         self.assertDictEqual(res, expect)
 
+    def test_to_api_partial(self):
+        """ It should return the correctly compatible obj when no CN and no key are defined """
+        res = self.model_partial.to_api()
+        expect = {
+            'names': [self.partial_vals['names'][0].to_api()],
+            'hosts': [self.partial_vals['hosts'][0].to_api()],
+        }
+        self.assertDictEqual(res, expect)
+
 
 if __name__ == '__main__':
     unittest.main()

cfssl/tests/test_cfssl.py

@@ -10,6 +10,7 @@
                      CFSSLRemoteException,
                      requests,
                      )
+from cfssl import cfssl
 
 _logger = logging.getLogger(__name__)
 
@@ -184,7 +185,7 @@ def test_call_request(self, requests):
             method='method',
             url='https://test:1/api/v1/cfssl/endpoint',
             params='params',
-            data='data',
+            json='data',
             verify=True,
         )
 
@@ -197,11 +198,29 @@ def test_call_error(self, requests):
 
     @mock.patch.object(requests, 'request')
     def test_call_success(self, requests):
-        """ It should reteurn result on success response """
+        """ It should return result on success response """
         requests().json.return_value = {'success': True,
                                         'result': 'result'}
         res = self.cfssl.call(None)
         self.assertEqual(res, 'result')
 
+    @mock.patch.object(cfssl, 'log')
+    @mock.patch.object(requests, 'request')
+    def test_log_messages(self, requests, log):
+        """ It should return result on success response """
+        requests().json.return_value = {'success': True,
+                                        'messages': [
+                                            {'message': 'some message', 'code': 5000},
+                                            {'code': 5001},
+                                            {'message': 'message only'},
+                                            'another message'],
+                                        'result': 'result'}
+        res = self.cfssl.call(None)
+        self.assertEqual(res, 'result')
+        log.warning.assert_any_call('some message (5000)')
+        log.warning.assert_any_call('<undefined message> (5001)')
+        log.warning.assert_any_call('message only')
+        log.warning.assert_any_call('another message')
+
 if __name__ == '__main__':
     unittest.main()

cfssl/tests/test_utils.py

@@ -0,0 +1,30 @@
+# -*- coding: utf-8 -*-
+# Copyright 2016 LasLabs Inc.
+# License MIT (https://opensource.org/licenses/MIT).
+
+import unittest
+from ..utils import to_api
+
+
+class TestUtils(unittest.TestCase):
+    def test_to_api_native_structure(self):
+        """ It should return the same object when it doesn't implement to_api()"""
+
+        expect = "fallback"
+        res = to_api(expect)
+        self.assertIs(res, expect)
+
+    def test_to_api_object(self):
+        """ It should delegate to to_api() method of a supported object"""
+
+        class SupportedObject(object):
+            def to_api(self):
+                return "supported"
+
+        expect = "supported"
+        res = to_api(SupportedObject())
+        self.assertEqual(res, expect)
+
+
+if __name__ == '__main__':
+    unittest.main()

cfssl/utils.py

@@ -0,0 +1,15 @@
+# -*- coding: utf-8 -*-
+# Copyright 2016 LasLabs Inc.
+# License MIT (https://opensource.org/licenses/MIT).
+
+
+def to_api(object):
+    """ Ensure an object is converted using it's to_api method if it exists.
+
+    Args:
+        object (any):
+    Returns:
+        str: A PEM-encoded certificate that has been signed by the
+            server.
+    """
+    return object.to_api() if hasattr(object, 'to_api') else object