ACME exception fixes (#217)

* Fix wrong usages of ACMEProtocolException.

* Add changelog fragment.

* Fix error handling when content could not be decoded.

* Make sure that content_json is a dict or None.

* Improve acme_inspect's ACMEProtocolException handling.

* Improve error handling.

* Add tests.

* Fix challenge error.

* Add challenges tests.

* Provide content if available.

* Add some order tests.

* Linting.

plugins/module_utils/acme/acme.py

@@ -286,12 +286,14 @@ class ACMEClient(object):
                 content = info.pop('body', None)
 
         # Process result
+        parsed_json_result = False
         if parse_json_result:
             result = {}
             if content:
                 if info['content-type'].startswith('application/json'):
                     try:
                         result = self.module.from_json(content.decode('utf8'))
+                        parsed_json_result = True
                     except ValueError:
                         raise NetworkException("Failed to parse the ACME response: {0} {1}".format(uri, content))
                 else:
@@ -301,7 +303,7 @@ class ACMEClient(object):
 
         if fail_on_error and _is_failed(info, expected_status_codes=expected_status_codes):
             raise ACMEProtocolException(
-                self.module, msg=error_msg, info=info, content=content, content_json=result if parse_json_result else None)
+                self.module, msg=error_msg, info=info, content=content, content_json=result if parsed_json_result else None)
         return result, info
 
 

plugins/module_utils/acme/challenges.py

@@ -182,7 +182,7 @@ class Authorization(object):
             new_authz["resource"] = "new-authz"
         else:
             if 'newAuthz' not in client.directory.directory:
-                raise ACMEProtocolException('ACME endpoint does not support pre-authorization')
+                raise ACMEProtocolException(client.module, 'ACME endpoint does not support pre-authorization')
             url = client.directory['newAuthz']
 
         result, info = client.send_signed_request(
@@ -214,7 +214,7 @@ class Authorization(object):
                 data[challenge.type] = validation_data
         return data
 
-    def raise_error(self, error_msg):
+    def raise_error(self, error_msg, module=None):
         '''
         Aborts with a specific error for a challenge.
         '''
@@ -227,17 +227,20 @@ class Authorization(object):
                 if 'error' in challenge.data:
                     msg = '{msg}: {problem}'.format(
                         msg=msg,
-                        problem=format_error_problem(challenge.data['error'], subproblem_prefix='{0}.'.format(type)),
+                        problem=format_error_problem(challenge.data['error'], subproblem_prefix='{0}.'.format(challenge.type)),
                     )
                 error_details.append(msg)
         raise ACMEProtocolException(
+            module,
             'Failed to validate challenge for {identifier}: {error}. {details}'.format(
                 identifier=self.combined_identifier,
                 error=error_msg,
                 details='; '.join(error_details),
             ),
-            identifier=self.combined_identifier,
-            authorization=self.data,
+            extras=dict(
+                identifier=self.combined_identifier,
+                authorization=self.data,
+            ),
         )
 
     def find_challenge(self, challenge_type):
@@ -254,7 +257,7 @@ class Authorization(object):
             time.sleep(2)
 
         if self.status == 'invalid':
-            self.raise_error('Status is "invalid"')
+            self.raise_error('Status is "invalid"', module=client.module)
 
         return self.status == 'valid'
 

plugins/module_utils/acme/errors.py

@@ -7,6 +7,9 @@
 from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
+from ansible.module_utils.six import binary_type
+from ansible.module_utils._text import to_text
+
 
 def format_error_problem(problem, subproblem_prefix=''):
     if 'title' in problem:
@@ -23,7 +26,7 @@ def format_error_problem(problem, subproblem_prefix=''):
         msg = '{msg} Subproblems:'.format(msg=msg)
         for index, problem in enumerate(subproblems):
             index_str = '{prefix}{index}'.format(prefix=subproblem_prefix, index=index)
-            msg = '{msg}\n({index}) {problem}.'.format(
+            msg = '{msg}\n({index}) {problem}'.format(
                 msg=msg,
                 index=index_str,
                 problem=format_error_problem(problem, subproblem_prefix='{0}.'.format(index_str)),
@@ -45,7 +48,7 @@ class ModuleFailException(Exception):
 
 
 class ACMEProtocolException(ModuleFailException):
-    def __init__(self, module, msg=None, info=None, response=None, content=None, content_json=None):
+    def __init__(self, module, msg=None, info=None, response=None, content=None, content_json=None, extras=None):
         # Try to get hold of content, if response is given and content is not provided
         if content is None and content_json is None and response is not None:
             try:
@@ -53,50 +56,61 @@ class ACMEProtocolException(ModuleFailException):
             except AttributeError:
                 content = info.pop('body', None)
 
+        # Make sure that content_json is None or a dictionary
+        if content_json is not None and not isinstance(content_json, dict):
+            if content is None and isinstance(content_json, binary_type):
+                content = content_json
+            content_json = None
+
         # Try to get hold of JSON decoded content, when content is given and JSON not provided
-        if content_json is None and content is not None:
+        if content_json is None and content is not None and module is not None:
             try:
-                content_json = module.from_json(content.decode('utf8'))
-            except Exception:
+                content_json = module.from_json(to_text(content))
+            except Exception as e:
                 pass
 
-        extras = dict()
-        url = info['url'] if info else None
-        code = info['status'] if info else None
-        extras['http_url'] = url
-        extras['http_status'] = code
+        extras = extras or dict()
 
         if msg is None:
             msg = 'ACME request failed'
         add_msg = ''
 
-        if code >= 400 and content_json is not None and 'type' in content_json:
-            if 'status' in content_json and content_json['status'] != code:
-                code = 'status {problem_code} (HTTP status: {http_code})'.format(http_code=code, problem_code=content_json['status'])
+        if info is not None:
+            url = info['url']
+            code = info['status']
+            extras['http_url'] = url
+            extras['http_status'] = code
+            if code is not None and code >= 400 and content_json is not None and 'type' in content_json:
+                if 'status' in content_json and content_json['status'] != code:
+                    code = 'status {problem_code} (HTTP status: {http_code})'.format(http_code=code, problem_code=content_json['status'])
+                else:
+                    code = 'status {problem_code}'.format(problem_code=code)
+                subproblems = content_json.pop('subproblems', None)
+                add_msg = ' {problem}.'.format(problem=format_error_problem(content_json))
+                extras['problem'] = content_json
+                extras['subproblems'] = subproblems or []
+                if subproblems is not None:
+                    add_msg = '{add_msg} Subproblems:'.format(add_msg=add_msg)
+                    for index, problem in enumerate(subproblems):
+                        add_msg = '{add_msg}\n({index}) {problem}.'.format(
+                            add_msg=add_msg,
+                            index=index,
+                            problem=format_error_problem(problem, subproblem_prefix='{0}.'.format(index)),
+                        )
             else:
-                code = 'status {problem_code}'.format(problem_code=code)
-            add_msg = ' {problem}.'.format(problem=format_error_problem(content_json))
-
-            subproblems = content_json.pop('subproblems', None)
-            extras['problem'] = content_json
-            extras['subproblems'] = subproblems or []
-            if subproblems is not None:
-                add_msg = '{add_msg} Subproblems:'.format(add_msg=add_msg)
-                for index, problem in enumerate(subproblems):
-                    add_msg = '{add_msg}\n({index}) {problem}.'.format(
-                        add_msg=add_msg,
-                        index=index,
-                        problem=format_error_problem(problem, subproblem_prefix='{0}.'.format(index)),
-                    )
-        else:
-            code = 'HTTP status {code}'.format(code=code)
-            if content_json is not None:
-                add_msg = ' The JSON error result: {content}'.format(content=content_json)
-            elif content is not None:
-                add_msg = ' The raw error result: {content}'.format(content=content.decode('utf-8'))
+                code = 'HTTP status {code}'.format(code=code)
+                if content_json is not None:
+                    add_msg = ' The JSON error result: {content}'.format(content=content_json)
+                elif content is not None:
+                    add_msg = ' The raw error result: {content}'.format(content=to_text(content))
+            msg = '{msg} for {url} with {code}'.format(msg=msg, url=url, code=code)
+        elif content_json is not None:
+            add_msg = ' The JSON result: {content}'.format(content=content_json)
+        elif content is not None:
+            add_msg = ' The raw result: {content}'.format(content=to_text(content))
 
         super(ACMEProtocolException, self).__init__(
-            '{msg} for {url} with {code}.{add_msg}'.format(msg=msg, url=url, code=code, add_msg=add_msg),
+            '{msg}.{add_msg}'.format(msg=msg, add_msg=add_msg),
             **extras
         )
         self.problem = {}

plugins/module_utils/acme/orders.py

@@ -99,7 +99,9 @@ class Order(object):
 
         if self.status != 'valid':
             raise ACMEProtocolException(
-                'Failed to wait for order to complete; got status "{status}"'.format(status=self.status), content_json=self.data)
+                client.module,
+                'Failed to wait for order to complete; got status "{status}"'.format(status=self.status),
+                content_json=self.data)
 
     def finalize(self, client, csr_der, wait=True):
         '''
@@ -121,5 +123,7 @@ class Order(object):
             self.refresh(client)
             if self.status not in ['procesing', 'valid', 'invalid']:
                 raise ACMEProtocolException(
-                    'Failed to finalize order; got status "{status}"'.format(
-                        status=self.status), info=info, content_json=result)
+                    client.module,
+                    'Failed to finalize order; got status "{status}"'.format(status=self.status),
+                    info=info,
+                    content_json=result)

plugins/modules/acme_certificate.py

@@ -739,7 +739,7 @@ class ACMECertificateClient(object):
                 raise ModuleFailException('Found no authorization information for "{identifier}"!'.format(
                     identifier=combine_identifier(identifier_type, identifier)))
             if authz.status != 'valid':
-                authz.raise_error('Status is "{status}" and not "valid"'.format(status=authz.status))
+                authz.raise_error('Status is "{status}" and not "valid"'.format(status=authz.status), module=self.module)
 
         if self.version == 1:
             cert = retrieve_acme_v1_certificate(self.client, pem_to_der(self.csr, self.csr_content))

plugins/modules/acme_certificate_revoke.py

@@ -229,7 +229,7 @@ def main():
             # but successfully terminate while indicating no change
             if already_revoked:
                 module.exit_json(changed=False)
-            raise ACMEProtocolException('Failed to revoke certificate', info=info, content_json=result)
+            raise ACMEProtocolException(module, 'Failed to revoke certificate', info=info, content_json=result)
         module.exit_json(changed=True)
     except ModuleFailException as e:
         e.do_fail(module)

plugins/modules/acme_inspect.py

@@ -307,7 +307,7 @@ def main():
                 pass
             # Fail if error was returned
             if fail_on_acme_error and info['status'] >= 400:
-                raise ACMEProtocolException(info=info, content_json=result)
+                raise ACMEProtocolException(module, info=info, content=data)
         # Done!
         module.exit_json(changed=changed, **result)
     except ModuleFailException as e: