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

@@ -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.

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

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

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."""