Update ACME error namespace to match the new draft. (#3469)

* Update error namespace in acme package.

* Use new error namespace in certbot.

* fix lint and py26 errors.

* Update with_code docstring.

* @pde's suggestions

acme/acme/messages.py

@@ -7,6 +7,37 @@ from acme import fields
 from acme import jose
 from acme import util
 
+OLD_ERROR_PREFIX = "urn:acme:error:"
+ERROR_PREFIX = "urn:ietf:params:acme:error:"
+
+ERROR_CODES = {
+    'badCSR': 'The CSR is unacceptable (e.g., due to a short key)',
+    'badNonce': 'The client sent an unacceptable anti-replay nonce',
+    'connection': ('The server could not connect to the client to verify the'
+                   ' domain'),
+    'dnssec': 'The server could not validate a DNSSEC signed domain',
+    # deprecate invalidEmail
+    'invalidEmail': 'The provided email for a registration was invalid',
+    'invalidContact': 'The provided contact URI was invalid',
+    'malformed': 'The request message was malformed',
+    'rateLimited': 'There were too many requests of a given type',
+    'serverInternal': 'The server experienced an internal error',
+    'tls': 'The server experienced a TLS error during domain verification',
+    'unauthorized': 'The client lacks sufficient authorization',
+    'unknownHost': 'The server could not resolve a domain name',
+}
+
+ERROR_TYPE_DESCRIPTIONS = dict(
+    (ERROR_PREFIX + name, desc) for name, desc in ERROR_CODES.items())
+
+ERROR_TYPE_DESCRIPTIONS.update(dict(  # add errors with old prefix, deprecate me
+    (OLD_ERROR_PREFIX + name, desc) for name, desc in ERROR_CODES.items()))
+
+
+def is_acme_error(err):
+    """Check if argument is an ACME error."""
+    return (ERROR_PREFIX in str(err)) or (OLD_ERROR_PREFIX in str(err))
+
 
 class Error(jose.JSONObjectWithFields, errors.Error):
     """ACME error.
@@ -18,31 +49,24 @@ class Error(jose.JSONObjectWithFields, errors.Error):
     :ivar unicode detail:
 
     """
-    ERROR_TYPE_DESCRIPTIONS = dict(
-        ('urn:acme:error:' + name, description) for name, description in (
-            ('badCSR', 'The CSR is unacceptable (e.g., due to a short key)'),
-            ('badNonce', 'The client sent an unacceptable anti-replay nonce'),
-            ('connection', 'The server could not connect to the client to '
-             'verify the domain'),
-            ('dnssec', 'The server could not validate a DNSSEC signed domain'),
-            ('invalidEmail',
-             'The provided email for a registration was invalid'),
-            ('invalidContact',
-             'The provided contact URI was invalid'),
-            ('malformed', 'The request message was malformed'),
-            ('rateLimited', 'There were too many requests of a given type'),
-            ('serverInternal', 'The server experienced an internal error'),
-            ('tls', 'The server experienced a TLS error during domain '
-             'verification'),
-            ('unauthorized', 'The client lacks sufficient authorization'),
-            ('unknownHost', 'The server could not resolve a domain name'),
-        )
-    )
-
     typ = jose.Field('type', omitempty=True, default='about:blank')
     title = jose.Field('title', omitempty=True)
     detail = jose.Field('detail', omitempty=True)
 
+    @classmethod
+    def with_code(cls, code, **kwargs):
+        """Create an Error instance with an ACME Error code.
+
+        :unicode code: An ACME error code, like 'dnssec'.
+        :kwargs: kwargs to pass to Error.
+
+        """
+        if code not in ERROR_CODES:
+            raise ValueError("The supplied code: %s is not a known ACME error"
+                             " code" % code)
+        typ = ERROR_PREFIX + code
+        return cls(typ=typ, **kwargs)
+
     @property
     def description(self):
         """Hardcoded error description based on its type.
@@ -51,7 +75,21 @@ class Error(jose.JSONObjectWithFields, errors.Error):
         :rtype: unicode
 
         """
-        return self.ERROR_TYPE_DESCRIPTIONS.get(self.typ)
+        return ERROR_TYPE_DESCRIPTIONS.get(self.typ)
+
+    @property
+    def code(self):
+        """ACME error code.
+
+        Basically self.typ without the ERROR_PREFIX.
+
+        :returns: error code if standard ACME code or ``None``.
+        :rtype: unicode
+
+        """
+        code = str(self.typ).split(':')[-1]
+        if code in ERROR_CODES:
+            return code
 
     def __str__(self):
         return ' :: '.join(

acme/acme/messages_test.py

@@ -17,13 +17,13 @@ class ErrorTest(unittest.TestCase):
     """Tests for acme.messages.Error."""
 
     def setUp(self):
-        from acme.messages import Error
+        from acme.messages import Error, ERROR_PREFIX
         self.error = Error(
-            detail='foo', typ='urn:acme:error:malformed', title='title')
+            detail='foo', typ=ERROR_PREFIX + 'malformed', title='title')
         self.jobj = {
             'detail': 'foo',
             'title': 'some title',
-            'type': 'urn:acme:error:malformed',
+            'type': ERROR_PREFIX + 'malformed',
         }
         self.error_custom = Error(typ='custom', detail='bar')
         self.jobj_cusom = {'type': 'custom', 'detail': 'bar'}
@@ -47,10 +47,27 @@ class ErrorTest(unittest.TestCase):
 
     def test_str(self):
         self.assertEqual(
-            'urn:acme:error:malformed :: The request message was '
+            'urn:ietf:params:acme:error:malformed :: The request message was '
             'malformed :: foo :: title', str(self.error))
         self.assertEqual('custom :: bar', str(self.error_custom))
 
+    def test_code(self):
+        from acme.messages import Error
+        self.assertEqual('malformed', self.error.code)
+        self.assertEqual(None, self.error_custom.code)
+        self.assertEqual(None, Error().code)
+
+    def test_is_acme_error(self):
+        from acme.messages import is_acme_error
+        self.assertTrue(is_acme_error(self.error))
+        self.assertTrue(is_acme_error(str(self.error)))
+        self.assertFalse(is_acme_error(self.error_custom))
+
+    def test_with_code(self):
+        from acme.messages import Error, is_acme_error
+        self.assertTrue(is_acme_error(Error.with_code('badCSR')))
+        self.assertRaises(ValueError, Error.with_code, 'not an ACME error code')
+
 
 class ConstantTest(unittest.TestCase):
     """Tests for acme.messages._Constant."""
@@ -240,7 +257,7 @@ class ChallengeBodyTest(unittest.TestCase):
         from acme.messages import Error
         from acme.messages import STATUS_INVALID
         self.status = STATUS_INVALID
-        error = Error(typ='urn:acme:error:serverInternal',
+        error = Error(typ='urn:ietf:params:acme:error:serverInternal',
                       detail='Unable to communicate with DNS server')
         self.challb = ChallengeBody(
             uri='http://challb', chall=self.chall, status=self.status,
@@ -256,7 +273,7 @@ class ChallengeBodyTest(unittest.TestCase):
         self.jobj_from = self.jobj_to.copy()
         self.jobj_from['status'] = 'invalid'
         self.jobj_from['error'] = {
-            'type': 'urn:acme:error:serverInternal',
+            'type': 'urn:ietf:params:acme:error:serverInternal',
             'detail': 'Unable to communicate with DNS server',
         }
 

certbot/auth_handler.py

@@ -437,9 +437,6 @@ def _report_no_chall_path():
     raise errors.AuthorizationError(msg)
 
 
-_ACME_PREFIX = "urn:acme:error:"
-
-
 _ERROR_HELP_COMMON = (
     "To fix these errors, please make sure that your domain name was entered "
     "correctly and the DNS A record(s) for that domain contain(s) the "
@@ -501,9 +498,10 @@ def _generate_failed_chall_msg(failed_achalls):
     :rtype: str
 
     """
-    typ = failed_achalls[0].error.typ
+    error = failed_achalls[0].error
-    if typ.startswith(_ACME_PREFIX):
+    typ = error.typ
-        typ = typ[len(_ACME_PREFIX):]
+    if messages.is_acme_error(error):
+        typ = error.code
     msg = ["The following errors were reported by the server:"]
 
     for achall in failed_achalls:

certbot/client.py

@@ -149,7 +149,7 @@ def perform_registration(acme, config):
     try:
         return acme.register(messages.NewRegistration.from_data(email=config.email))
     except messages.Error as e:
-        if e.typ == "urn:acme:error:invalidEmail" or e.typ == "urn:acme:error:invalidContact":
+        if e.code == "invalidEmail" or e.code == "invalidContact":
             if config.noninteractive_mode:
                 msg = ("The ACME server believes %s is an invalid email address. "
                        "Please ensure it is a valid email and attempt "

certbot/main.py

@@ -12,6 +12,7 @@ import traceback
 import zope.component
 
 from acme import jose
+from acme import messages
 
 import certbot
 
@@ -691,11 +692,12 @@ def _handle_exception(exc_type, exc_value, trace, config):
             else:
                 err = traceback.format_exception_only(exc_type, exc_value)[0]
             # Typical error from the ACME module:
-            # acme.messages.Error: urn:acme:error:malformed :: The request message was
+            # acme.messages.Error: urn:ietf:params:acme:error:malformed :: The
-            # malformed :: Error creating new registration :: Validation of contact
+            # request message was malformed :: Error creating new registration
-            # mailto:none@longrandomstring.biz failed: Server failure at resolver
+            # :: Validation of contact mailto:none@longrandomstring.biz failed:
-            if (("urn:acme" in err and ":: " in err and
+            # Server failure at resolver
-                 config.verbose_count <= cli.flag_default("verbose_count"))):
+            if (messages.is_acme_error(err) and ":: " in err and
+                 config.verbose_count <= cli.flag_default("verbose_count")):
                 # prune ACME error code, we have a human description
                 _code, _sep, err = err.partition(":: ")
             msg = "An unexpected error occurred:\n" + err + "Please see the "

certbot/tests/auth_handler_test.py

@@ -386,7 +386,7 @@ class ReportFailedChallsTest(unittest.TestCase):
             "chall": acme_util.HTTP01,
             "uri": "uri",
             "status": messages.STATUS_INVALID,
-            "error": messages.Error(typ="urn:acme:error:tls", detail="detail"),
+            "error": messages.Error.with_code("tls", detail="detail"),
         }
 
         # Prevent future regressions if the error type changes

certbot/tests/cli_test.py

@@ -952,8 +952,8 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             mock_sys.exit.assert_any_call(''.join(
                 traceback.format_exception_only(errors.Error, error)))
 
-        exception = messages.Error(detail='alpha', typ='urn:acme:error:triffid',
+        bad_typ = messages.ERROR_PREFIX + 'triffid'
-                                   title='beta')
+        exception = messages.Error(detail='alpha', typ=bad_typ, title='beta')
         config = mock.MagicMock(debug=False, verbose_count=-3)
         main._handle_exception(
             messages.Error, exc_value=exception, trace=None, config=config)

certbot/tests/client_test.py

@@ -65,7 +65,7 @@ class RegisterTest(unittest.TestCase):
         from acme import messages
         self.config.noninteractive_mode = False
         msg = "DNS problem: NXDOMAIN looking up MX for example.com"
-        mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidContact")
+        mx_err = messages.Error.with_code('invalidContact', detail=msg)
         with mock.patch("certbot.client.acme_client.Client") as mock_client:
             mock_client().register.side_effect = [mx_err, mock.MagicMock()]
             self._call()
@@ -75,7 +75,7 @@ class RegisterTest(unittest.TestCase):
     def test_email_invalid_noninteractive(self, _rep):
         from acme import messages
         msg = "DNS problem: NXDOMAIN looking up MX for example.com"
-        mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidContact")
+        mx_err = messages.Error.with_code('invalidContact', detail=msg)
         with mock.patch("certbot.client.acme_client.Client") as mock_client:
             mock_client().register.side_effect = [mx_err, mock.MagicMock()]
             self.assertRaises(errors.Error, self._call)