From 4a28bb1af7dfa113578c4081f91914d9e420ee93 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 12:37:45 -0700 Subject: [PATCH 1/7] clarify invalid email error in non-interactive --- certbot/display/ops.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/certbot/display/ops.py b/certbot/display/ops.py index 4db6d71e2..901e7cd04 100644 --- a/certbot/display/ops.py +++ b/certbot/display/ops.py @@ -48,7 +48,10 @@ def get_email(invalid=False, optional=True): 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") + "or provide --agree-tos and --email flags. " + "If you have specified an email with the --email flag, " + "please make sure that you entered it correctly and the " + "domain is valid.") raise errors.MissingCommandlineFlag(msg) if code != display_util.OK: From 7767975204e82c4764964dd5a84af448b4eaed08 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 16:13:31 -0700 Subject: [PATCH 2/7] move error outside of get_email --- certbot/client.py | 10 ++++++++-- certbot/display/ops.py | 5 +---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 119fb0947..cb8fc623c 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -150,8 +150,14 @@ 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(invalid=True) - return perform_registration(acme, config) + if config.noninteractive_mode: + msg = ("The email you specified was unable to be verified " + "by acme. Please ensure it is a valid email and " + "attempt registration again.") + raise erros.MissingCommandlineFlag(msg) + else: + 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 901e7cd04..e8520fe96 100644 --- a/certbot/display/ops.py +++ b/certbot/display/ops.py @@ -48,10 +48,7 @@ def get_email(invalid=False, optional=True): 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. " - "If you have specified an email with the --email flag, " - "please make sure that you entered it correctly and the " - "domain is valid.") + "or provide --agree-tos and --email flags.") raise errors.MissingCommandlineFlag(msg) if code != display_util.OK: From 6e550f70b0eb6f5661b740b871dd6a6e9c7330d8 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 16:17:51 -0700 Subject: [PATCH 3/7] fix typo --- certbot/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/client.py b/certbot/client.py index cb8fc623c..03525cc0d 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -154,7 +154,7 @@ def perform_registration(acme, config): msg = ("The email you specified was unable to be verified " "by acme. Please ensure it is a valid email and " "attempt registration again.") - raise erros.MissingCommandlineFlag(msg) + raise errors.MissingCommandlineFlag(msg) else: config.namespace.email = display_ops.get_email(invalid=True) return perform_registration(acme, config) From 0bf78242148b53420bd50456a7adb07a462c2d91 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 17:28:12 -0700 Subject: [PATCH 4/7] fix test --- certbot/tests/client_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 4a8a8bdee..29718c263 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -67,8 +67,7 @@ class RegisterTest(unittest.TestCase): mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") with mock.patch("certbot.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) + self.assertRaises(errors.MissingCommandlineFlag, self._call) def test_needs_email(self): self.config.email = None From 7d0b71928c771dd48c8a3795d966fe2fa8882808 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 17:33:17 -0700 Subject: [PATCH 5/7] incorp peter's feedback --- certbot/client.py | 8 ++++---- certbot/tests/client_test.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/certbot/client.py b/certbot/client.py index 03525cc0d..ef59c6ce3 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -151,10 +151,10 @@ def perform_registration(acme, config): except messages.Error as e: if e.typ == "urn:acme:error:invalidEmail": if config.noninteractive_mode: - msg = ("The email you specified was unable to be verified " - "by acme. Please ensure it is a valid email and " - "attempt registration again.") - raise errors.MissingCommandlineFlag(msg) + msg = ("The ACME server believes %s is an invalid email address. " + "Please ensure it is a valid email and attempt " + "registration again." % config.email) + raise errors.Error(msg) else: config.namespace.email = display_ops.get_email(invalid=True) return perform_registration(acme, config) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 29718c263..1ed63f466 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -61,13 +61,13 @@ class RegisterTest(unittest.TestCase): @mock.patch("certbot.account.report_new_account") @mock.patch("certbot.client.display_ops.get_email") - def test_email_retry(self, _rep, mock_get_email): + def test_email_invalid_noninteractive(self, _rep, mock_get_email): from acme import messages msg = "DNS problem: NXDOMAIN looking up MX for example.com" mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") with mock.patch("certbot.client.acme_client.Client") as mock_client: mock_client().register.side_effect = [mx_err, mock.MagicMock()] - self.assertRaises(errors.MissingCommandlineFlag, self._call) + self.assertRaises(errors.Error, self._call) def test_needs_email(self): self.config.email = None From ceb5207d56758feb72edfd02c7e56c9c2692ca17 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 16 Aug 2016 18:06:21 -0700 Subject: [PATCH 6/7] lint --- certbot/tests/client_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 1ed63f466..7ff46be05 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -60,8 +60,7 @@ class RegisterTest(unittest.TestCase): self._call() @mock.patch("certbot.account.report_new_account") - @mock.patch("certbot.client.display_ops.get_email") - def test_email_invalid_noninteractive(self, _rep, mock_get_email): + def test_email_invalid_noninteractive(self, _rep): from acme import messages msg = "DNS problem: NXDOMAIN looking up MX for example.com" mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") From 93047d6579c4869cd4896f15b4d9ddbf47ddcbe5 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 17 Aug 2016 15:49:19 -0700 Subject: [PATCH 7/7] add back in email test --- certbot/tests/client_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 7ff46be05..98d853716 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -59,6 +59,18 @@ class RegisterTest(unittest.TestCase): with mock.patch("certbot.account.report_new_account"): self._call() + @mock.patch("certbot.account.report_new_account") + @mock.patch("certbot.client.display_ops.get_email") + def test_email_retry(self, _rep, mock_get_email): + from acme import messages + self.config.noninteractive_mode = False + msg = "DNS problem: NXDOMAIN looking up MX for example.com" + mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") + with mock.patch("certbot.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) + @mock.patch("certbot.account.report_new_account") def test_email_invalid_noninteractive(self, _rep): from acme import messages