diff options
-rw-r--r-- | certbot/CHANGELOG.md | 3 | ||||
-rw-r--r-- | certbot/certbot/_internal/main.py | 9 | ||||
-rw-r--r-- | certbot/tests/main_test.py | 40 |
3 files changed, 50 insertions, 2 deletions
diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 3dd1a9e26..4258ac580 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -22,6 +22,9 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). agree with a non-existent Terms of Service ("None"). This bug is now fixed, so that if an ACME server does not provide any Terms of Service to agree with, the user is not asked to agree to a non-existent Terms of Service any longer. +* If account registration fails, Certbot did not relay the error from the ACME server + back to the user. This is now fixed: the error message from the ACME server is now + presented to the user when account registration fails. More details about these changes can be found on our GitHub repo. diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 18d819a5c..12021c89c 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -21,6 +21,7 @@ import zope.interface from acme import client as acme_client from acme import errors as acme_errors +from acme import messages as acme_messages import certbot from certbot import configuration from certbot import crypto_util @@ -726,10 +727,14 @@ def _determine_account(config: configuration.NamespaceConfig display_util.notify("Account registered.") except errors.MissingCommandlineFlag: raise - except errors.Error: + except (errors.Error, acme_messages.Error) as err: logger.debug("", exc_info=True) + if acme_messages.is_acme_error(err): + err_msg = f"Error returned by the ACME server: {str(err)}" + else: + err_msg = str(err) raise errors.Error( - "Unable to register an account with ACME server") + f"Unable to register an account with ACME server. {err_msg}") config.account = acc.id return acc, acme diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index c29f4d758..6b09f1ba2 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -16,6 +16,7 @@ import unittest import josepy as jose import pytz +from acme.messages import Error as acme_error from certbot import crypto_util, configuration from certbot import errors from certbot import interfaces @@ -593,6 +594,17 @@ class DetermineAccountTest(test_util.ConfigTestCase): mock_storage.return_value = self.account_storage return _determine_account(self.config) + @mock.patch('certbot._internal.client.register') + @mock.patch('certbot._internal.client.display_ops.get_email') + def _register_error_common(self, err_msg, exception, mock_get_email, mock_register): + mock_get_email.return_value = 'foo@bar.baz' + mock_register.side_effect = exception + try: + self._call() + except errors.Error as err: + self.assertEqual(f"Unable to register an account with ACME server. {err_msg}", + str(err)) + def test_args_account_set(self): self.account_storage.save(self.accs[1], self.mock_client) self.config.account = self.accs[1].id @@ -617,6 +629,16 @@ class DetermineAccountTest(test_util.ConfigTestCase): self.assertEqual(self.accs[1].id, self.config.account) self.assertIsNone(self.config.email) + @mock.patch('certbot._internal.client.display_ops.choose_account') + def test_multiple_accounts_canceled(self, mock_choose_accounts): + for acc in self.accs: + self.account_storage.save(acc, self.mock_client) + mock_choose_accounts.return_value = None + try: + self._call() + except errors.Error as err: + self.assertIn("No account has been chosen", str(err)) + @mock.patch('certbot._internal.client.display_ops.get_email') @mock.patch('certbot._internal.main.display_util.notify') def test_no_accounts_no_email(self, mock_notify, mock_get_email): @@ -641,6 +663,24 @@ class DetermineAccountTest(test_util.ConfigTestCase): self.assertEqual(self.accs[1].id, self.config.account) self.assertEqual('other email', self.config.email) + def test_register_error_certbot(self): + err_msg = "Some error message raised by Certbot" + self._register_error_common(err_msg, errors.Error(err_msg)) + + def test_register_error_acme_type_and_detail(self): + err_msg = ("Error returned by the ACME server: urn:ietf:params:acme:" + "error:malformed :: The request message was malformed :: " + "must agree to terms of service") + exception = acme_error(typ = "urn:ietf:params:acme:error:malformed", + detail = "must agree to terms of service") + self._register_error_common(err_msg, exception) + + def test_register_error_acme_type_only(self): + err_msg = ("Error returned by the ACME server: urn:ietf:params:acme:" + "error:serverInternal :: The server experienced an internal error") + exception = acme_error(typ = "urn:ietf:params:acme:error:serverInternal") + self._register_error_common(err_msg, exception) + class MainTest(test_util.ConfigTestCase): """Tests for different commands.""" |