Merge pull request #1524 from letsencrypt/email

Better UI when asking for email

letsencrypt/cli.py

@@ -140,10 +140,8 @@ def _determine_account(args, config):
         elif len(accounts) == 1:
             acc = accounts[0]
         else:  # no account registered yet
-            if args.email is None:
+            if args.email is None and not args.register_unsafely_without_email:
                 args.email = display_ops.get_email()
-            if not args.email:  # get_email might return ""
-                args.email = None
 
             def _tos_cb(regr):
                 if args.tos:
@@ -847,6 +845,16 @@ def prepare_and_parse_args(plugins, args):
     helpful.add(
         None, "-t", "--text", dest="text_mode", action="store_true",
         help="Use the text output instead of the curses UI.")
+    helpful.add(
+        None, "--register-unsafely-without-email", action="store_true",
+        help="Specifying this flag enables registering an account with no "
+             "email address. This is strongly discouraged, because in the "
+             "event of key loss or account compromise you will irrevocably "
+             "lose access to your account. You will also be unable to receive "
+             "notice about impending expiration of revocation of your "
+             "certificates. Updates to the Subscriber Agreement will still "
+             "affect you, and will be effective N days after posting an "
+             "update to the web site.")
     helpful.add(None, "-m", "--email", help=config_help("email"))
     # positional arg shadows --domains, instead of appending, and
     # --domains is useful, because it can be stored in config

letsencrypt/client.py

@@ -100,6 +100,11 @@ def register(config, account_storage, tos_cb=None):
     if account_storage.find_all():
         logger.info("There are already existing accounts for %s", config.server)
     if config.email is None:
+        if not config.register_unsafely_without_email:
+            msg = ("No email was provided and "
+                   "--register-unsafely-without-email was not present.")
+            logger.warn(msg)
+            raise errors.Error(msg)
         logger.warn("Registering without email!")
 
     # Each new registration shall use a fresh new key
@@ -110,7 +115,7 @@ def register(config, account_storage, tos_cb=None):
             backend=default_backend())))
     acme = acme_from_config_key(config, key)
     # TODO: add phone?
-    regr = acme.register(messages.NewRegistration.from_data(email=config.email))
+    regr = perform_registration(acme, config)
 
     if regr.terms_of_service is not None:
         if tos_cb is not None and not tos_cb(regr):
@@ -126,6 +131,30 @@ def register(config, account_storage, tos_cb=None):
     return acc, acme
 
 
+def perform_registration(acme, config):
+    """
+    Actually register new account, trying repeatedly if there are email
+    problems
+
+    :param .IConfig config: Client configuration.
+    :param acme.client.Client client: ACME client object.
+
+    :returns: Registration Resource.
+    :rtype: `acme.messages.RegistrationResource`
+
+    :raises .UnexpectedUpdate:
+    """
+    try:
+        return acme.register(messages.NewRegistration.from_data(email=config.email))
+    except messages.Error, e:
+        err = repr(e)
+        if "MX record" in err or "Validation of contact mailto" in err:
+            config.namespace.email = display_ops.get_email(more=True, invalid=True)
+            return perform_registration(acme, config)
+        else:
+            raise
+
+
 class Client(object):
     """ACME protocol client.
 

letsencrypt/display/ops.py

@@ -122,23 +122,36 @@ def pick_configurator(
         config, default, plugins, question,
         (interfaces.IAuthenticator, interfaces.IInstaller))
 
-
-def get_email():
+def get_email(more=False, invalid=False):
     """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
+
     :returns: Email or ``None`` if cancelled by user.
     :rtype: str
 
     """
-    while True:
-        code, email = zope.component.getUtility(interfaces.IDisplay).input(
-            "Enter email address (used for urgent notices and lost key recovery)")
+    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')
+    code, email = zope.component.getUtility(interfaces.IDisplay).input(msg)
 
-        if code == display_util.OK:
-            if le_util.safe_email(email):
-                return email
+    if code == display_util.OK:
+        if le_util.safe_email(email):
+            return email
         else:
-            return None
+            # 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 != ""))
+    else:
+        return None
 
 
 def choose_account(accounts):

letsencrypt/display/util.py

@@ -104,6 +104,7 @@ class NcursesDisplay(object):
 
             return code, int(tag) - 1
 
+
     def input(self, message):
         """Display an input box to the user.
 
@@ -114,7 +115,12 @@ class NcursesDisplay(object):
             `string` - input entered by the user
 
         """
-        return self.dialog.inputbox(message, width=self.width)
+        sections = message.split("\n")
+        # each section takes at least one line, plus extras if it's longer than self.width
+        wordlines = [1 + (len(section)/self.width) for section in sections]
+        height = 6 + sum(wordlines) + len(sections)
+        return self.dialog.inputbox(message, width=self.width, height=height)
+
 
     def yesno(self, message, yes_label="Yes", no_label="No"):
         """Display a Yes/No dialog box.

letsencrypt/tests/cli_test.py

@@ -499,7 +499,8 @@ class DetermineAccountTest(unittest.TestCase):
     """Tests for letsencrypt.cli._determine_account."""
 
     def setUp(self):
-        self.args = mock.MagicMock(account=None, email=None)
+        self.args = mock.MagicMock(account=None, email=None,
+            register_unsafely_without_email=False)
         self.config = configuration.NamespaceConfig(self.args)
         self.accs = [mock.MagicMock(id='x'), mock.MagicMock(id='y')]
         self.account_storage = account.AccountMemoryStorage()

letsencrypt/tests/client_test.py

@@ -24,7 +24,7 @@ class RegisterTest(unittest.TestCase):
     """Tests for letsencrypt.client.register."""
 
     def setUp(self):
-        self.config = mock.MagicMock(rsa_key_size=1024)
+        self.config = mock.MagicMock(rsa_key_size=1024, register_unsafely_without_email=False)
         self.account_storage = account.AccountMemoryStorage()
         self.tos_cb = mock.MagicMock()
 
@@ -47,10 +47,23 @@ class RegisterTest(unittest.TestCase):
 
     def test_it(self):
         with mock.patch("letsencrypt.client.acme_client.Client"):
-            with mock.patch("letsencrypt.account."
-                            "report_new_account"):
+            with mock.patch("letsencrypt.account.report_new_account"):
                 self._call()
 
+    @mock.patch("letsencrypt.account.report_new_account")
+    @mock.patch("letsencrypt.client.display_ops.get_email")
+    def test_email_retry(self, _rep, mock_get_email):
+        from acme import messages
+        msg = "Validation of contact mailto:sousaphone@improbablylongggstring.tld failed"
+        mx_err = messages.Error(detail=msg, typ="malformed", title="title")
+        with mock.patch("letsencrypt.client.acme_client.Client") as mock_client:
+            mock_client().register.side_effect = [mx_err, mock.MagicMock()]
+            self._call()
+            self.assertEqual(mock_get_email.call_count, 1)
+
+    def test_needs_email(self):
+        self.config.email = None
+        self.assertRaises(errors.Error, self._call)
 
 class ClientTest(unittest.TestCase):
     """Tests for letsencrypt.client.Client."""

letsencrypt/tests/display/ops_test.py

@@ -170,9 +170,9 @@ class GetEmailTest(unittest.TestCase):
         zope.component.provideUtility(mock_display, interfaces.IDisplay)
 
     @classmethod
-    def _call(cls):
+    def _call(cls, **kwargs):
         from letsencrypt.display.ops import get_email
-        return get_email()
+        return get_email(**kwargs)
 
     def test_cancel_none(self):
         self.input.return_value = (display_util.CANCEL, "foo@bar.baz")
@@ -180,18 +180,38 @@ class GetEmailTest(unittest.TestCase):
 
     def test_ok_safe(self):
         self.input.return_value = (display_util.OK, "foo@bar.baz")
-        with mock.patch("letsencrypt.display.ops.le_util"
-                        ".safe_email") as mock_safe_email:
+        with mock.patch("letsencrypt.display.ops.le_util.safe_email") as mock_safe_email:
             mock_safe_email.return_value = True
             self.assertTrue(self._call() is "foo@bar.baz")
 
     def test_ok_not_safe(self):
         self.input.return_value = (display_util.OK, "foo@bar.baz")
-        with mock.patch("letsencrypt.display.ops.le_util"
-                        ".safe_email") as mock_safe_email:
+        with mock.patch("letsencrypt.display.ops.le_util.safe_email") as mock_safe_email:
             mock_safe_email.side_effect = [False, True]
             self.assertTrue(self._call() is "foo@bar.baz")
 
+    def test_more_and_invalid_flags(self):
+        more_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")
+        with mock.patch("letsencrypt.display.ops.le_util.safe_email") as mock_safe_email:
+            mock_safe_email.return_value = True
+            self._call()
+            msg = self.input.call_args[0][0]
+            self.assertTrue(more_txt not in msg)
+            self.assertTrue(invalid_txt not in msg)
+            self.assertTrue(base_txt in msg)
+            self._call(more=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(invalid_txt in msg)
+            self.assertTrue(base_txt in msg)
+
 
 class ChooseAccountTest(unittest.TestCase):
     """Tests for letsencrypt.display.ops.choose_account."""

tests/integration/_common.sh

@@ -23,7 +23,7 @@ letsencrypt_test () {
         --no-redirect \
         --agree-dev-preview \
         --agree-tos \
-        --email "" \
+        --register-unsafely-without-email \
         --renew-by-default \
         --debug \
         -vvvvvvv \