From 814ab083bde51c17e3fe445d4f3cb68939b77f71 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 2 Jun 2015 11:16:24 -0700 Subject: [PATCH 1/4] Added account registration message and fixed double output --- letsencrypt/cli.py | 2 +- letsencrypt/client.py | 9 +++++++++ letsencrypt/reporter.py | 14 +++++++++++++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 169b55aff..798a816af 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -377,7 +377,7 @@ def main(args=sys.argv[1:]): # Reporter report = reporter.Reporter() zope.component.provideUtility(report) - atexit.register(report.print_messages) + atexit.register(report.atexit_print_messages) # Logging level = -args.verbose_count * 10 diff --git a/letsencrypt/client.py b/letsencrypt/client.py index c4b8ef40c..bb0a6d01f 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -99,6 +99,15 @@ class Client(object): raise errors.LetsEncryptClientError("Must agree to TOS") self.account.save() + reporter = zope.component.getUtility(interfaces.IReporter) + reporter.add_message( + "Your account credentials have been saved in your Let's Encrypt " + "configuration directory at {0}. You should make a secure backup " + "of this folder now. This configuration directory will also " + "contain certificates and private keys obtained by Let's Encrypt " + "so making regular backups of this folder is ideal.".format( + self.config.config_dir), + reporter.HIGH_PRIORITY, True) def obtain_certificate(self, domains, csr=None): """Obtains a certificate from the ACME server. diff --git a/letsencrypt/reporter.py b/letsencrypt/reporter.py index 7dfacaf14..947643d48 100644 --- a/letsencrypt/reporter.py +++ b/letsencrypt/reporter.py @@ -1,5 +1,7 @@ """Collects and displays information to the user.""" import collections +import logging +import os import Queue import sys import textwrap @@ -46,6 +48,16 @@ class Reporter(object): """ assert self.HIGH_PRIORITY <= priority <= self.LOW_PRIORITY self.messages.put(self._msg_type(priority, msg, on_crash)) + logging.info("Reporting to user: %s", msg) + + def atexit_print_messages(self, pid=os.getpid()): + """Function to be registered with atexit to print messages. + + :param int pid: Process ID + + """ + if pid == os.getpid(): + self.print_messages() def print_messages(self): """Prints messages to the user and clears the message queue. @@ -59,7 +71,7 @@ class Reporter(object): no_exception = sys.exc_info()[0] is None bold_on = sys.stdout.isatty() if bold_on: - sys.stdout.write(self._BOLD) + print self._BOLD print 'IMPORTANT NOTES:' wrapper = textwrap.TextWrapper(initial_indent=' - ', subsequent_indent=(' ' * 3)) From 34a66b1bff0a663b96c75ac9b41ef509dc2003aa Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 2 Jun 2015 17:11:00 -0700 Subject: [PATCH 2/4] Added key notifications to the client --- letsencrypt/client.py | 49 ++++++++++++++++++++++++++++++++++++++--- letsencrypt/reporter.py | 5 +++-- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index b755940e4..17bee6069 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -99,6 +99,10 @@ class Client(object): raise errors.LetsEncryptClientError("Must agree to TOS") self.account.save() + self._report_new_account() + + def _report_new_account(self): + """Informs the user about their new Let's Encrypt account.""" reporter = zope.component.getUtility(interfaces.IReporter) reporter.add_message( "Your account credentials have been saved in your Let's Encrypt " @@ -107,7 +111,17 @@ class Client(object): "contain certificates and private keys obtained by Let's Encrypt " "so making regular backups of this folder is ideal.".format( self.config.config_dir), - reporter.HIGH_PRIORITY, True) + reporter.MEDIUM_PRIORITY, True) + + assert self.account.recovery_token is not None + recovery_msg = ("If you lose your account credentials, you can recover " + "them using the token \"{0}\". You must write that down " + "and put it in a safe place.".format( + self.account.recovery_token)) + if self.account.email is not None: + recovery_msg += (" Another recovery method will be e-mails sent to " + "{0}.".format(self.account.email)) + reporter.add_message(recovery_msg, reporter.HIGH_PRIORITY, True) def obtain_certificate(self, domains, csr=None): """Obtains a certificate from the ACME server. @@ -204,9 +218,38 @@ class Client(object): params = vars(self.config.namespace) config = {"renewer_config_file": params["renewer_config_file"]} if "renewer_config_file" in params else None - return storage.RenewableCert.new_lineage(domains[0], cert, privkey, - chain, params, config) + renewable_cert = storage.RenewableCert.new_lineage(domains[0], cert, privkey, + chain, params, config) + self._report_renewal_status(renewable_cert) + return renewable_cert + def _report_renewal_status(self, cert): + # pylint: disable=no-self-use + """Informs the user about automatic renewal and deployment. + + :param cert: Newly issued certificate + :type cert: :class:`letsencrypt.storage.RenewableCert` + + """ + if ("autorenew" not in cert.configuration + or cert.configuration.as_bool("autorenew")): + if ("autodeploy" not in cert.configuration or + cert.configuration.as_bool("autodeploy")): + msg = "Automatic renewal and deployment has " + else: + msg = "Automatic renewal but not automatic deployment has " + else: + if ("autodeploy" not in cert.configuration or + cert.configuration.as_bool("autodeploy")): + msg = "Automatic deployment but not automatic renewal has " + else: + msg = "Automatic renewal and deployment has not " + + msg += ("been enabled for your certificate. These settings can be " + "configured in the directories under {0}.").format( + cert.configuration["renewal_configs_dir"]) + reporter = zope.component.getUtility(interfaces.IReporter) + reporter.add_message(msg, reporter.LOW_PRIORITY, True) def save_certificate(self, certr, cert_path, chain_path): # pylint: disable=no-self-use diff --git a/letsencrypt/reporter.py b/letsencrypt/reporter.py index 947643d48..045c1befa 100644 --- a/letsencrypt/reporter.py +++ b/letsencrypt/reporter.py @@ -56,6 +56,8 @@ class Reporter(object): :param int pid: Process ID """ + # This ensures that messages are only printed from the process that + # created the Reporter. if pid == os.getpid(): self.print_messages() @@ -64,8 +66,7 @@ class Reporter(object): If there is an unhandled exception, only messages for which ``on_crash`` is ``True`` are printed. - - """ +""" bold_on = False if not self.messages.empty(): no_exception = sys.exc_info()[0] is None From 9f37a46c92f672cff123fbe3734867fd4ffe5258 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Jun 2015 11:40:14 -0700 Subject: [PATCH 3/4] Added client tests --- letsencrypt/tests/client_test.py | 43 +++++++++++++++++++ letsencrypt/tests/proof_of_possession_test.py | 2 +- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 7687d5205..cfcee5b05 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -5,6 +5,7 @@ import pkg_resources import shutil import tempfile +import configobj import mock from letsencrypt import account @@ -35,6 +36,48 @@ class ClientTest(unittest.TestCase): self.network2.Network.assert_called_once_with( mock.ANY, mock.ANY, verify_ssl=True) + @mock.patch("letsencrypt.client.zope.component.getUtility") + def test_report_new_account(self, mock_zope): + self.config.config_dir = "/usr/bin/coffee" + self.account.recovery_token = "ECCENTRIC INVISIBILITY RHINOCEROS" + self.account.email = "rhino@jungle.io" + + self.client._report_new_account() + call_list = mock_zope().add_message.call_args_list + self.assertTrue(self.config.config_dir in call_list[0][0][0]) + self.assertTrue(self.account.recovery_token in call_list[1][0][0]) + self.assertTrue(self.account.email in call_list[1][0][0]) + + @mock.patch("letsencrypt.client.zope.component.getUtility") + def test_report_renewal_status(self, mock_zope): + cert = mock.MagicMock() + cert.configuration = configobj.ConfigObj() + cert.configuration["renewal_configs_dir"] = "/etc/letsencrypt/configs" + + cert.configuration["autorenew"] = "True" + cert.configuration["autodeploy"] = "True" + self.client._report_renewal_status(cert) + msg = mock_zope().add_message.call_args[0][0] + self.assertTrue("renewal and deployment has been" in msg) + self.assertTrue(cert.configuration["renewal_configs_dir"] in msg) + + cert.configuration["autorenew"] = "False" + self.client._report_renewal_status(cert) + msg = mock_zope().add_message.call_args[0][0] + self.assertTrue("deployment but not automatic renewal" in msg) + self.assertTrue(cert.configuration["renewal_configs_dir"] in msg) + + cert.configuration["autodeploy"] = "False" + self.client._report_renewal_status(cert) + msg = mock_zope().add_message.call_args[0][0] + self.assertTrue("renewal and deployment has not" in msg) + self.assertTrue(cert.configuration["renewal_configs_dir"] in msg) + + cert.configuration["autorenew"] = "True" + self.client._report_renewal_status(cert) + msg = mock_zope().add_message.call_args[0][0] + self.assertTrue("renewal but not automatic deployment" in msg) + self.assertTrue(cert.configuration["renewal_configs_dir"] in msg) class DetermineAccountTest(unittest.TestCase): """Tests for letsencrypt.client.determine_authenticator.""" diff --git a/letsencrypt/tests/proof_of_possession_test.py b/letsencrypt/tests/proof_of_possession_test.py index 3635b5ead..0a044810c 100644 --- a/letsencrypt/tests/proof_of_possession_test.py +++ b/letsencrypt/tests/proof_of_possession_test.py @@ -69,7 +69,7 @@ class ProofOfPossessionTest(unittest.TestCase): def test_perform_no_input(self): self.assertTrue(self.proof_of_pos.perform(self.achall).verify()) - @mock.patch("letsencrypt.recovery_token.zope.component.getUtility") + @mock.patch("letsencrypt.proof_of_possession.zope.component.getUtility") def test_perform_with_input(self, mock_input): # Remove the matching certificate self.installer.get_all_certs_keys.return_value.pop() From 2c40cc77fb5175f5753f943a6696b37a8ae1f6c4 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 3 Jun 2015 12:06:51 -0700 Subject: [PATCH 4/4] Added atexit_print_messages test --- letsencrypt/tests/client_test.py | 4 +++- letsencrypt/tests/reporter_test.py | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index cfcee5b05..1fb9c2a03 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -38,6 +38,7 @@ class ClientTest(unittest.TestCase): @mock.patch("letsencrypt.client.zope.component.getUtility") def test_report_new_account(self, mock_zope): + # pylint: disable=protected-access self.config.config_dir = "/usr/bin/coffee" self.account.recovery_token = "ECCENTRIC INVISIBILITY RHINOCEROS" self.account.email = "rhino@jungle.io" @@ -50,6 +51,7 @@ class ClientTest(unittest.TestCase): @mock.patch("letsencrypt.client.zope.component.getUtility") def test_report_renewal_status(self, mock_zope): + # pylint: disable=protected-access cert = mock.MagicMock() cert.configuration = configobj.ConfigObj() cert.configuration["renewal_configs_dir"] = "/etc/letsencrypt/configs" @@ -66,7 +68,7 @@ class ClientTest(unittest.TestCase): msg = mock_zope().add_message.call_args[0][0] self.assertTrue("deployment but not automatic renewal" in msg) self.assertTrue(cert.configuration["renewal_configs_dir"] in msg) - + cert.configuration["autodeploy"] = "False" self.client._report_renewal_status(cert) msg = mock_zope().add_message.call_args[0][0] diff --git a/letsencrypt/tests/reporter_test.py b/letsencrypt/tests/reporter_test.py index e6a8c9005..2248e7caf 100644 --- a/letsencrypt/tests/reporter_test.py +++ b/letsencrypt/tests/reporter_test.py @@ -30,6 +30,15 @@ class ReporterTest(unittest.TestCase): self.reporter.print_messages() self.assertEqual(sys.stdout.getvalue(), "") + def test_atexit_print_messages(self): + self._add_messages() + self.reporter.atexit_print_messages() + output = sys.stdout.getvalue() + self.assertTrue("IMPORTANT NOTES:" in output) + self.assertTrue("High" in output) + self.assertTrue("Med" in output) + self.assertTrue("Low" in output) + def test_tty_successful_exit(self): sys.stdout.isatty = lambda: True self._successful_exit_common()