refactor get_email

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

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 <email_address> 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 <email_address> 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):

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)