register: remove report_new_account, use logger (certbot#8393)

certbot/certbot/_internal/account.py

@@ -11,7 +11,6 @@
 import pyrfc3339
 import pytz
 import six
-import zope.component
 
 from acme import fields as acme_fields
 from acme import messages
@@ -94,21 +93,6 @@ def __eq__(self, other):
                 self.meta == other.meta)
 
 
-def report_new_account(config):
-    """Informs the user about their new ACME account."""
-    reporter = zope.component.queryUtility(interfaces.IReporter)
-    if reporter is None:
-        return
-    reporter.add_message(
-        "Your account credentials have been saved in your Certbot "
-        "configuration directory at {0}. You should make a secure backup "
-        "of this folder now. This configuration directory will also "
-        "contain certificates and private keys obtained by Certbot "
-        "so making regular backups of this folder is ideal.".format(
-            config.config_dir),
-        reporter.MEDIUM_PRIORITY)
-
-
 class AccountMemoryStorage(interfaces.AccountStorage):
     """In-memory account storage."""
 

certbot/certbot/_internal/client.py

@@ -158,7 +158,7 @@ def register(config, account_storage, tos_cb=None):
             logger.warning(msg)
             raise errors.Error(msg)
         if not config.dry_run:
-            logger.info("Registering without email!")
+            logger.debug("Registering without email!")
 
     # If --dry-run is used, and there is no staging account, create one with no email.
     if config.dry_run:
@@ -175,7 +175,6 @@ def register(config, account_storage, tos_cb=None):
     regr = perform_registration(acme, config, tos_cb)
 
     acc = account.Account(regr, key)
-    account.report_new_account(config)
     account_storage.save(acc, acme)
 
     eff.prepare_subscription(config, acc)

certbot/certbot/_internal/main.py

@@ -490,11 +490,9 @@ def _tos_cb(terms_of_service):
             return True
         msg = ("Please read the Terms of Service at {0}. You "
                "must agree in order to register with the ACME "
-               "server at {1}".format(
-                   terms_of_service, config.server))
+               "server. Do you agree?".format(terms_of_service))
         obj = zope.component.getUtility(interfaces.IDisplay)
-        result = obj.yesno(msg, "Agree", "Cancel",
-                         cli_flag="--agree-tos", force_interactive=True)
+        result = obj.yesno(msg, cli_flag="--agree-tos", force_interactive=True)
         if not result:
             raise errors.Error(
                 "Registration cannot proceed without accepting "
@@ -518,6 +516,7 @@ def _tos_cb(terms_of_service):
             try:
                 acc, acme = client.register(
                     config, account_storage, tos_cb=_tos_cb)
+                logger.info("Account registered.")
             except errors.MissingCommandlineFlag:
                 raise
             except errors.Error:

certbot/tests/account_test.py

@@ -83,24 +83,6 @@ def test_deserialize_full(self):
         self.assertIsNotNone(meta.creation_host)
         self.assertIsNotNone(meta.register_to_eff)
 
-class ReportNewAccountTest(test_util.ConfigTestCase):
-    """Tests for certbot._internal.account.report_new_account."""
-
-    def _call(self):
-        from certbot._internal.account import report_new_account
-        report_new_account(self.config)
-
-    @mock.patch("certbot._internal.account.zope.component.queryUtility")
-    def test_no_reporter(self, mock_zope):
-        mock_zope.return_value = None
-        self._call()
-
-    @mock.patch("certbot._internal.account.zope.component.queryUtility")
-    def test_it(self, mock_zope):
-        self._call()
-        call_list = mock_zope().add_message.call_args_list
-        self.assertTrue(self.config.config_dir in call_list[0][0][0])
-
 
 class AccountMemoryStorageTest(unittest.TestCase):
     """Tests for certbot._internal.account.AccountMemoryStorage."""

certbot/tests/client_test.py

@@ -93,25 +93,22 @@ def test_no_tos(self):
             mock_client.new_account_and_tos().terms_of_service = "http://tos"
             mock_client().external_account_required.side_effect = self._false_mock
             with mock.patch("certbot._internal.eff.prepare_subscription") as mock_prepare:
-                with mock.patch("certbot._internal.account.report_new_account"):
-                    mock_client().new_account_and_tos.side_effect = errors.Error
-                    self.assertRaises(errors.Error, self._call)
-                    self.assertFalse(mock_prepare.called)
+                mock_client().new_account_and_tos.side_effect = errors.Error
+                self.assertRaises(errors.Error, self._call)
+                self.assertFalse(mock_prepare.called)
 
-                    mock_client().new_account_and_tos.side_effect = None
-                    self._call()
-                    self.assertTrue(mock_prepare.called)
+                mock_client().new_account_and_tos.side_effect = None
+                self._call()
+                self.assertTrue(mock_prepare.called)
 
     def test_it(self):
         with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
             mock_client().external_account_required.side_effect = self._false_mock
-            with mock.patch("certbot._internal.account.report_new_account"):
-                with mock.patch("certbot._internal.eff.handle_subscription"):
-                    self._call()
+            with mock.patch("certbot._internal.eff.handle_subscription"):
+                self._call()
 
-    @mock.patch("certbot._internal.account.report_new_account")
     @mock.patch("certbot._internal.client.display_ops.get_email")
-    def test_email_retry(self, _rep, mock_get_email):
+    def test_email_retry(self, mock_get_email):
         from acme import messages
         self.config.noninteractive_mode = False
         msg = "DNS problem: NXDOMAIN looking up MX for example.com"
@@ -124,8 +121,7 @@ def test_email_retry(self, _rep, mock_get_email):
                 self.assertEqual(mock_get_email.call_count, 1)
                 self.assertTrue(mock_prepare.called)
 
-    @mock.patch("certbot._internal.account.report_new_account")
-    def test_email_invalid_noninteractive(self, _rep):
+    def test_email_invalid_noninteractive(self):
         from acme import messages
         self.config.noninteractive_mode = True
         msg = "DNS problem: NXDOMAIN looking up MX for example.com"
@@ -145,28 +141,25 @@ def test_without_email(self, mock_logger):
         with mock.patch("certbot._internal.eff.prepare_subscription") as mock_prepare:
             with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_clnt:
                 mock_clnt().external_account_required.side_effect = self._false_mock
-                with mock.patch("certbot._internal.account.report_new_account"):
-                    self.config.email = None
-                    self.config.register_unsafely_without_email = True
-                    self.config.dry_run = False
-                    self._call()
-                    mock_logger.info.assert_called_once_with(mock.ANY)
-                    self.assertTrue(mock_prepare.called)
+                self.config.email = None
+                self.config.register_unsafely_without_email = True
+                self.config.dry_run = False
+                self._call()
+                mock_logger.debug.assert_called_once_with(mock.ANY)
+                self.assertTrue(mock_prepare.called)
 
-    @mock.patch("certbot._internal.account.report_new_account")
     @mock.patch("certbot._internal.client.display_ops.get_email")
-    def test_dry_run_no_staging_account(self, _rep, mock_get_email):
+    def test_dry_run_no_staging_account(self, mock_get_email):
         """Tests dry-run for no staging account, expect account created with no email"""
         with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_client:
             mock_client().external_account_required.side_effect = self._false_mock
             with mock.patch("certbot._internal.eff.handle_subscription"):
-                with mock.patch("certbot._internal.account.report_new_account"):
-                    self.config.dry_run = True
-                    self._call()
-                    # check Certbot did not ask the user to provide an email
-                    self.assertFalse(mock_get_email.called)
-                    # check Certbot created an account with no email. Contact should return empty
-                    self.assertFalse(mock_client().new_account_and_tos.call_args[0][0].contact)
+                self.config.dry_run = True
+                self._call()
+                # check Certbot did not ask the user to provide an email
+                self.assertFalse(mock_get_email.called)
+                # check Certbot created an account with no email. Contact should return empty
+                self.assertFalse(mock_client().new_account_and_tos.call_args[0][0].contact)
 
     def test_with_eab_arguments(self):
         with mock.patch("certbot._internal.client.acme_client.BackwardsCompatibleClientV2") as mock_client:

certbot/tests/main_test.py

@@ -481,7 +481,8 @@ def test_multiple_accounts(self, mock_choose_accounts):
         self.assertTrue(self.config.email is None)
 
     @mock.patch('certbot._internal.client.display_ops.get_email')
-    def test_no_accounts_no_email(self, mock_get_email):
+    @mock.patch('certbot._internal.main.logger')
+    def test_no_accounts_no_email(self, mock_logger, mock_get_email):
         mock_get_email.return_value = '[email protected]'
 
         with mock.patch('certbot._internal.main.client') as client:
@@ -493,6 +494,7 @@ def test_no_accounts_no_email(self, mock_get_email):
 
         self.assertEqual(self.accs[0].id, self.config.account)
         self.assertEqual('[email protected]', self.config.email)
+        mock_logger.info.assert_called_once_with('Account registered.')
 
     def test_no_accounts_email(self):
         self.config.email = 'other email'