From df5f08843feb3befb32762e5eae5b2e8fd2a8593 Mon Sep 17 00:00:00 2001 From: Craig Smith Date: Wed, 30 Nov 2016 09:00:37 +0930 Subject: [PATCH] Output success message for revoke command (#3823) * Output status for `revoke` operation. Fixes #2819. - Added method to `certbot.display.ops` to output confirmation of `revoke`. - Wrapped call to `acme.client.Client.revoke` in a try to statement to handle possible error. - Added test for `main.revoke`. * Added test for failure of certificate revocation. Moved creation of mocks into RevokeTest setup function. Stopped mocks in RevokeTest teardown function. * Fixed lint errors. * Do not call `unittest.TestCase.assertRaises` as a context manager (to work with py26). * Fixed spelling error in successful revocation notification. Added test for the notification. --- certbot/display/ops.py | 13 +++++++ certbot/main.py | 8 +++- certbot/tests/display/ops_test.py | 15 ++++++++ certbot/tests/main_test.py | 62 +++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 1 deletion(-) diff --git a/certbot/display/ops.py b/certbot/display/ops.py index 97f050318..ee7e750f6 100644 --- a/certbot/display/ops.py +++ b/certbot/display/ops.py @@ -237,6 +237,19 @@ def success_renewal(domains): os.linesep.join(_gen_ssl_lab_urls(domains))), pause=False) +def success_revocation(cert_path): + """Display a box confirming a certificate has been revoked. + + :param list cert_path: path to certificate which was revoked. + + """ + z_util(interfaces.IDisplay).notification( + "Congratulations! You have successfully revoked the certificate " + "that was located at {0}{1}{1}".format( + cert_path, + os.linesep), + pause=False) + def _gen_ssl_lab_urls(domains): """Returns a list of urls. diff --git a/certbot/main.py b/certbot/main.py index a84a77dc4..1f3379fd7 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -12,6 +12,7 @@ import zope.component from acme import jose from acme import messages +from acme import errors as acme_errors import certbot @@ -503,7 +504,12 @@ def revoke(config, unused_plugins): # TODO: coop with renewal config key = acc.key acme = client.acme_from_config_key(config, key) cert = crypto_util.pyopenssl_load_certificate(config.cert_path[1])[0] - acme.revoke(jose.ComparableX509(cert)) + try: + acme.revoke(jose.ComparableX509(cert)) + except acme_errors.ClientError as e: + return e.message + + display_ops.success_revocation(config.cert_path[0]) def run(config, plugins): # pylint: disable=too-many-branches,too-many-locals diff --git a/certbot/tests/display/ops_test.py b/certbot/tests/display/ops_test.py index ebb695024..44efcf703 100644 --- a/certbot/tests/display/ops_test.py +++ b/certbot/tests/display/ops_test.py @@ -336,6 +336,21 @@ class SuccessRenewalTest(unittest.TestCase): for name in names: self.assertTrue(name in arg) +class SuccessRevocationTest(unittest.TestCase): + # pylint: disable=too-few-public-methods + """Test the success revocation message.""" + @classmethod + def _call(cls, path): + from certbot.display.ops import success_revocation + success_revocation(path) + + @mock.patch("certbot.display.ops.z_util") + def test_success_revocation(self, mock_util): + mock_util().notification.return_value = None + path = "/path/to/cert.pem" + self._call(path) + mock_util().notification.assert_called_once() + self.assertTrue(path in mock_util().notification.call_args[0][0]) if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 133606a19..ea744b570 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -3,6 +3,8 @@ import os import shutil import tempfile import unittest +import datetime +import pytz import mock @@ -13,6 +15,11 @@ from certbot import configuration from certbot import errors from certbot.plugins import disco as plugins_disco +from certbot.tests import test_util +from acme import jose + +CERT_PATH = test_util.vector_path('cert.pem') +KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key_2.pem")) class MainTest(unittest.TestCase): def setUp(self): @@ -110,6 +117,61 @@ class ObtainCertTest(unittest.TestCase): # pylint: disable=unused-argument self.assertFalse(pause) +class RevokeTest(unittest.TestCase): + """Tests for certbot.main.revoke.""" + + def setUp(self): + self.tempdir_path = tempfile.mkdtemp() + shutil.copy(CERT_PATH, self.tempdir_path) + self.tmp_cert_path = os.path.abspath(os.path.join(self.tempdir_path, + 'cert.pem')) + + self.patches = [ + mock.patch('acme.client.Client'), + mock.patch('certbot.client.Client'), + mock.patch('certbot.main._determine_account'), + mock.patch('certbot.main.display_ops.success_revocation') + ] + self.mock_acme_client = self.patches[0].start() + self.patches[1].start() + self.mock_determine_account = self.patches[2].start() + self.mock_success_revoke = self.patches[3].start() + + from certbot.account import Account + + self.regr = mock.MagicMock() + self.meta = Account.Meta( + creation_host="test.certbot.org", + creation_dt=datetime.datetime( + 2015, 7, 4, 14, 4, 10, tzinfo=pytz.UTC)) + self.acc = Account(self.regr, KEY, self.meta) + + self.mock_determine_account.return_value = (self.acc, None) + + + def tearDown(self): + shutil.rmtree(self.tempdir_path) + for patch in self.patches: + patch.stop() + + def _call(self): + args = 'revoke --cert-path={0}'.format(self.tmp_cert_path).split() + plugins = plugins_disco.PluginsRegistry.find_all() + config = configuration.NamespaceConfig( + cli.prepare_and_parse_args(plugins, args)) + + from certbot.main import revoke + revoke(config, plugins) + + def test_revocation_success(self): + self._call() + self.mock_success_revoke.assert_called_once_with(self.tmp_cert_path) + + def test_revocation_error(self): + from acme import errors as acme_errors + self.mock_acme_client.side_effect = acme_errors.ClientError() + self.assertRaises(acme_errors.ClientError, self._call) + self.mock_success_revoke.assert_not_called() class SetupLogFileHandlerTest(unittest.TestCase): """Tests for certbot.main.setup_log_file_handler."""