From 5ca1b49e2ee1a368b296e209baaa87f4d30a2b6a Mon Sep 17 00:00:00 2001 From: ynasser Date: Thu, 2 Nov 2017 06:23:38 +0000 Subject: [PATCH] revoke accepts --cert-name too now; from the livestram --- certbot/cli.py | 8 ++++---- certbot/main.py | 9 ++++++++- certbot/tests/main_test.py | 26 +++++++++++++++++++++----- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 880ffd543..47d8df3f4 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -90,7 +90,7 @@ obtain, install, and renew certificates: manage certificates: certificates Display information about certificates you have from Certbot - revoke Revoke a certificate (supply --cert-path) + revoke Revoke a certificate (supply --cert-path or --cert-name) delete Delete a certificate manage your account with Let's Encrypt: @@ -371,9 +371,9 @@ VERB_HELP = [ "usage": "\n\n certbot delete --cert-name CERTNAME\n\n" }), ("revoke", { - "short": "Revoke a certificate specified with --cert-path", + "short": "Revoke a certificate specified with --cert-path or --cert-name", "opts": "Options for revocation of certificates", - "usage": "\n\n certbot revoke --cert-path /path/to/fullchain.pem [options]\n\n" + "usage": "\n\n certbot revoke --cert-path /path/to/fullchain.pem --cert-name example.com [options]\n\n" }), ("register", { "short": "Register for account with Let's Encrypt / other ACME server", @@ -1262,7 +1262,7 @@ def _paths_parser(helpful): add(section, "--cert-path", type=os.path.abspath, default=flag_default("auth_cert_path"), help=cph) elif verb == "revoke": - add(section, "--cert-path", type=read_file, required=True, help=cph) + add(section, "--cert-path", type=read_file, required=False, help=cph) else: add(section, "--cert-path", type=os.path.abspath, help=cph, required=(verb == "install")) diff --git a/certbot/main.py b/certbot/main.py index 9e2850891..c2a6a7bb2 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -655,6 +655,14 @@ def revoke(config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" # For user-agent construction config.installer = config.authenticator = "None" + + if config.cert_path is None and config.certname: + config.cert_path = storage.cert_path_for_cert_name(config, config.certname) + elif not config.cert_path or (config.cert_path and config.certname): + # intentionally not supporting --cert-path & --cert-name together, + # to avoid dealing with mismatched values + raise errors.Error("Error! Exactly one of --cert-path or --cert-name must be specified!") + if config.key_path is not None: # revocation by cert key logger.debug("Revoking %s using cert key %s", config.cert_path[0], config.key_path[0]) @@ -667,7 +675,6 @@ def revoke(config, unused_plugins): # TODO: coop with renewal config acme = client.acme_from_config_key(config, key) cert = crypto_util.pyopenssl_load_certificate(config.cert_path[1])[0] logger.debug("Reason code for revocation: %s", config.reason) - try: acme.revoke(jose.ComparableX509(cert), config.reason) _delete_if_appropriate(config) diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 45e5db1df..5995f77ad 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -224,6 +224,8 @@ class RevokeTest(test_util.TempDirTestCase): shutil.copy(CERT_PATH, self.tempdir) self.tmp_cert_path = os.path.abspath(os.path.join(self.tempdir, 'cert_512.pem')) + with open(self.tmp_cert_path, 'r') as f: + self.tmp_cert = (self.tmp_cert_path, f.read()) self.patches = [ mock.patch('acme.client.Client', autospec=True), @@ -253,9 +255,10 @@ class RevokeTest(test_util.TempDirTestCase): for patch in self.patches: patch.stop() - def _call(self, extra_args=""): - args = 'revoke --cert-path={0} ' + extra_args - args = args.format(self.tmp_cert_path).split() + def _call(self, args=[]): + if not args: + args = 'revoke --cert-path={0} ' + args = args.format(self.tmp_cert_path).split() plugins = disco.PluginsRegistry.find_all() config = configuration.NamespaceConfig( cli.prepare_and_parse_args(plugins, args)) @@ -271,12 +274,25 @@ class RevokeTest(test_util.TempDirTestCase): mock_revoke = mock_acme_client.Client().revoke expected = [] for reason, code in constants.REVOCATION_REASONS.items(): - self._call("--reason " + reason) + args = 'revoke --cert-path={0} --reason {1}'.format(self.tmp_cert_path, reason).split() + self._call(args) expected.append(mock.call(mock.ANY, code)) - self._call("--reason " + reason.upper()) + args = 'revoke --cert-path={0} --reason {1}'.format(self.tmp_cert_path, + reason.upper()).split() + self._call(args) expected.append(mock.call(mock.ANY, code)) self.assertEqual(expected, mock_revoke.call_args_list) + @mock.patch('certbot.main._delete_if_appropriate') + @mock.patch('certbot.storage.cert_path_for_cert_name') + def test_revoke_by_certname(self, mock_cert_path_for_cert_name, + mock_delete_if_appropriate): + args = 'revoke --cert-name=example.com'.split() + mock_cert_path_for_cert_name.return_value = self.tmp_cert + mock_delete_if_appropriate.return_value = False + self._call(args) + self.mock_success_revoke.assert_called_once_with(self.tmp_cert_path) + @mock.patch('certbot.main._delete_if_appropriate') def test_revocation_success(self, mock_delete_if_appropriate): self._call()