From 2cd132be04152a2eecf72966ccfa96f92b2b0487 Mon Sep 17 00:00:00 2001 From: Erica Portnoy Date: Wed, 14 Feb 2018 15:21:52 -0800 Subject: [PATCH] update certbot and tests to use new_account_and_tos method --- acme/acme/client.py | 9 ++++----- certbot/client.py | 16 +++++----------- certbot/main.py | 7 +++++-- certbot/tests/client_test.py | 20 ++++++++------------ 4 files changed, 22 insertions(+), 30 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index dfbe70d89..eceb078b7 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -579,12 +579,11 @@ class BackwardsCompatibleClientV2(object): else: raise AttributeError - def new_account_and_tos(self, regr=None, tos_cb=None): + def new_account_and_tos(self, regr=None, check_tos_cb=None): + # check_tos_cb should raise an error if we want to error def assess_tos(tos): - if tos_cb is not None and not tos_cb(tos): - raise errors.Error( - "Registration cannot proceed without accepting " - "Terms of Service.") + if check_tos_cb is not None: + check_tos_cb(tos) if self.acme_version == 1: regr = self.client.register(regr) if regr.terms_of_service is not None: diff --git a/certbot/client.py b/certbot/client.py index 3bf7c5aa3..67ee8f7fa 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -162,14 +162,7 @@ def register(config, account_storage, tos_cb=None): backend=default_backend()))) acme = acme_from_config_key(config, key) # TODO: add phone? - regr = perform_registration(acme, config) - - if regr.terms_of_service is not None: - if tos_cb is not None and not tos_cb(regr.terms_of_service): - raise errors.Error( - "Registration cannot proceed without accepting " - "Terms of Service.") - regr = acme.agree_to_tos(regr) + regr = perform_registration(acme, config, tos_cb) acc = account.Account(regr, key) account.report_new_account(config) @@ -180,7 +173,7 @@ def register(config, account_storage, tos_cb=None): return acc, acme -def perform_registration(acme, config): +def perform_registration(acme, config, tos_cb): """ Actually register new account, trying repeatedly if there are email problems @@ -192,7 +185,8 @@ def perform_registration(acme, config): :rtype: `acme.messages.RegistrationResource` """ try: - return acme.register(messages.NewRegistration.from_data(email=config.email)) + return acme.new_account_and_tos(messages.NewRegistration.from_data(email=config.email), + tos_cb) except messages.Error as e: if e.code == "invalidEmail" or e.code == "invalidContact": if config.noninteractive_mode: @@ -202,7 +196,7 @@ def perform_registration(acme, config): raise errors.Error(msg) else: config.email = display_ops.get_email(invalid=True) - return perform_registration(acme, config) + return perform_registration(acme, config, tos_cb) else: raise diff --git a/certbot/main.py b/certbot/main.py index 6e80580c8..1bb501ca6 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -503,9 +503,12 @@ def _determine_account(config): "server at {1}".format( terms_of_service, config.server)) obj = zope.component.getUtility(interfaces.IDisplay) - return obj.yesno(msg, "Agree", "Cancel", + result = obj.yesno(msg, "Agree", "Cancel", cli_flag="--agree-tos", force_interactive=True) - + if not result: + raise errors.Error( + "Registration cannot proceed without accepting " + "Terms of Service.") try: acc, acme = client.register( config, account_storage, tos_cb=_tos_cb) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index bbcc3be27..a9a87b80b 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -30,29 +30,25 @@ class RegisterTest(test_util.ConfigTestCase): self.config.register_unsafely_without_email = False self.config.email = "alias@example.com" self.account_storage = account.AccountMemoryStorage() - self.tos_cb = mock.MagicMock() def _call(self): from certbot.client import register - return register(self.config, self.account_storage, self.tos_cb) + tos_cb = mock.MagicMock() + return register(self.config, self.account_storage, tos_cb) def test_no_tos(self): with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client: - mock_client.register().terms_of_service = "http://tos" + mock_client.new_account_and_tos().terms_of_service = "http://tos" with mock.patch("certbot.eff.handle_subscription") as mock_handle: with mock.patch("certbot.account.report_new_account"): - self.tos_cb.return_value = False + mock_client().new_account_and_tos.side_effect = errors.Error self.assertRaises(errors.Error, self._call) self.assertFalse(mock_handle.called) - self.tos_cb.return_value = True + mock_client().new_account_and_tos.side_effect = None self._call() self.assertTrue(mock_handle.called) - self.tos_cb = None - self._call() - self.assertEqual(mock_handle.call_count, 2) - def test_it(self): with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2"): with mock.patch("certbot.account.report_new_account"): @@ -68,7 +64,7 @@ class RegisterTest(test_util.ConfigTestCase): mx_err = messages.Error.with_code('invalidContact', detail=msg) with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client: with mock.patch("certbot.eff.handle_subscription") as mock_handle: - mock_client().register.side_effect = [mx_err, mock.MagicMock()] + mock_client().new_account_and_tos.side_effect = [mx_err, mock.MagicMock()] self._call() self.assertEqual(mock_get_email.call_count, 1) self.assertTrue(mock_handle.called) @@ -81,7 +77,7 @@ class RegisterTest(test_util.ConfigTestCase): mx_err = messages.Error.with_code('invalidContact', detail=msg) with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client: with mock.patch("certbot.eff.handle_subscription"): - mock_client().register.side_effect = [mx_err, mock.MagicMock()] + mock_client().new_account_and_tos.side_effect = [mx_err, mock.MagicMock()] self.assertRaises(errors.Error, self._call) def test_needs_email(self): @@ -106,7 +102,7 @@ class RegisterTest(test_util.ConfigTestCase): mx_err = messages.Error(detail=msg, typ="malformed", title="title") with mock.patch("certbot.client.acme_client.BackwardsCompatibleClientV2") as mock_client: with mock.patch("certbot.eff.handle_subscription") as mock_handle: - mock_client().register.side_effect = [mx_err, mock.MagicMock()] + mock_client().new_account_and_tos.side_effect = [mx_err, mock.MagicMock()] self.assertRaises(messages.Error, self._call) self.assertFalse(mock_handle.called)