From 6598bcb53eda7e16975e6129f3700bd1944270db Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 25 May 2016 21:53:20 -0700 Subject: [PATCH] refactor get_email --- certbot/client.py | 2 +- certbot/display/ops.py | 69 +++++++++++++++++++------------ certbot/tests/display/ops_test.py | 15 +++---- 3 files changed, 49 insertions(+), 37 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index ba31f8760..a885d0426 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -150,7 +150,7 @@ def perform_registration(acme, config): return acme.register(messages.NewRegistration.from_data(email=config.email)) except messages.Error as e: if e.typ == "urn:acme:error:invalidEmail": - config.namespace.email = display_ops.get_email(more=True, invalid=True) + config.namespace.email = display_ops.get_email(invalid=True) return perform_registration(acme, config) else: raise diff --git a/certbot/display/ops.py b/certbot/display/ops.py index 6752bf0c1..16c44a881 100644 --- a/certbot/display/ops.py +++ b/certbot/display/ops.py @@ -15,41 +15,56 @@ logger = logging.getLogger(__name__) z_util = zope.component.getUtility -def get_email(more=False, invalid=False): +def get_email(invalid=False, optional=True): """Prompt for valid email address. - :param bool more: explain why the email is strongly advisable, but how to - skip it - :param bool invalid: true if the user just typed something, but it wasn't - a valid-looking email + :param bool invalid: True if an invalid was provided by the user + :param bool optional: True if the user can use + --register-unsafely-without-email to avoid providing an e-mail - :returns: Email or ``None`` if cancelled by user. + :returns: e-mail address :rtype: str - """ - msg = "Enter email address (used for urgent notices and lost key recovery)" - if invalid: - msg = "There seem to be problems with that address. " + msg - if more: - msg += ('\n\nIf you really want to skip this, you can run the client with ' - '--register-unsafely-without-email but make sure you backup your ' - 'account key from /etc/letsencrypt/accounts\n\n') - try: - code, email = zope.component.getUtility(interfaces.IDisplay).input(msg) - except errors.MissingCommandlineFlag: - msg = ("You should register before running non-interactively, or provide --agree-tos" - " and --email flags") - raise errors.MissingCommandlineFlag(msg) + :raises errors.Error: if the user cancels - if code == display_util.OK: - if le_util.safe_email(email): - return email + """ + invalid_prefix = "There seem to be problems with that address. " + msg = "Enter email address (used for urgent notices and lost key recovery)" + unsafe_suggestion = ("\n\nIf you really want to skip this, you can run " + "the client with --register-unsafely-without-email " + "but make sure you backup your account key from " + "/etc/letsencrypt/accounts\n\n") + if optional: + if invalid: + msg += unsafe_suggestion else: - # TODO catch the server's ACME invalid email address error, and - # make a similar call when that happens - return get_email(more=True, invalid=(email != "")) + suggest_unsafe = True else: - return None + suggest_unsafe = False + + while True: + try: + code, email = z_util(interfaces.IDisplay).input( + invalid_prefix + msg if invalid else msg) + except errors.MissingCommandlineFlag: + msg = ("You should register before running non-interactively, " + "or provide --agree-tos and --email flags") + raise errors.MissingCommandlineFlag(msg) + + if code != display_util.OK: + if optional: + raise errors.Error( + "An e-mail address or " + "--register-unsafely-without-email must be provided.") + else: + raise errors.Error("An e-mail address must be provided.") + elif le_util.safe_email(email): + return email + elif suggest_unsafe: + msg += unsafe_suggestion + suggest_unsafe = False # add this message at most once + + invalid = bool(email) def choose_account(accounts): diff --git a/certbot/tests/display/ops_test.py b/certbot/tests/display/ops_test.py index 05cb6b12d..be496a42a 100644 --- a/certbot/tests/display/ops_test.py +++ b/certbot/tests/display/ops_test.py @@ -12,6 +12,7 @@ from acme import jose from acme import messages from certbot import account +from certbot import errors from certbot import interfaces from certbot.display import util as display_util @@ -37,7 +38,7 @@ class GetEmailTest(unittest.TestCase): def test_cancel_none(self): self.input.return_value = (display_util.CANCEL, "foo@bar.baz") - self.assertTrue(self._call() is None) + self.assertRaises(errors.Error, self._call) def test_ok_safe(self): self.input.return_value = (display_util.OK, "foo@bar.baz") @@ -52,7 +53,7 @@ class GetEmailTest(unittest.TestCase): self.assertTrue(self._call() is "foo@bar.baz") def test_more_and_invalid_flags(self): - more_txt = "--register-unsafely-without-email" + optional_txt = "--register-unsafely-without-email" invalid_txt = "There seem to be problems" base_txt = "Enter email" self.input.return_value = (display_util.OK, "foo@bar.baz") @@ -60,16 +61,12 @@ class GetEmailTest(unittest.TestCase): mock_safe_email.return_value = True self._call() msg = self.input.call_args[0][0] - self.assertTrue(more_txt not in msg) + self.assertTrue(optional_txt not in msg) self.assertTrue(invalid_txt not in msg) self.assertTrue(base_txt in msg) - self._call(more=True) + self._call(invalid=True) msg = self.input.call_args[0][0] - self.assertTrue(more_txt in msg) - self.assertTrue(invalid_txt not in msg) - self._call(more=True, invalid=True) - msg = self.input.call_args[0][0] - self.assertTrue(more_txt in msg) + self.assertTrue(optional_txt in msg) self.assertTrue(invalid_txt in msg) self.assertTrue(base_txt in msg)