diff options
-rw-r--r-- | certbot/cli.py | 12 | ||||
-rw-r--r-- | certbot/constants.py | 1 | ||||
-rw-r--r-- | certbot/main.py | 8 | ||||
-rw-r--r-- | certbot/tests/cli_test.py | 14 | ||||
-rw-r--r-- | certbot/tests/main_test.py | 46 | ||||
-rwxr-xr-x | tests/boulder-integration.sh | 9 |
6 files changed, 75 insertions, 15 deletions
diff --git a/certbot/cli.py b/certbot/cli.py index 622462278..f0fa7eb7e 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -1220,6 +1220,18 @@ def _create_subparsers(helpful): key=constants.REVOCATION_REASONS.get)), action=_EncodeReasonAction, default=flag_default("reason"), help="Specify reason for revoking certificate. (default: unspecified)") + helpful.add("revoke", + "--delete-after-revoke", action="store_true", + default=flag_default("delete_after_revoke"), + help="Delete certificates after revoking them.") + helpful.add("revoke", + "--no-delete-after-revoke", action="store_false", + dest="delete_after_revoke", + default=flag_default("delete_after_revoke"), + help="Do not delete certificates after revoking them. This " + "option should be used with caution because the 'renew' " + "subcommand will attempt to renew undeleted revoked " + "certificates.") helpful.add("rollback", "--checkpoints", type=int, metavar="N", default=flag_default("rollback_checkpoints"), diff --git a/certbot/constants.py b/certbot/constants.py index 0ac82dafe..a6878824b 100644 --- a/certbot/constants.py +++ b/certbot/constants.py @@ -71,6 +71,7 @@ CLI_DEFAULTS = dict( user_agent_comment=None, csr=None, reason=0, + delete_after_revoke=None, rollback_checkpoints=1, init=False, prepare=False, diff --git a/certbot/main.py b/certbot/main.py index 1c6432fd9..e25e030aa 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -536,9 +536,11 @@ def _delete_if_appropriate(config): # pylint: disable=too-many-locals,too-many-b display = zope.component.getUtility(interfaces.IDisplay) reporter_util = zope.component.getUtility(interfaces.IReporter) - msg = ("Would you like to delete the cert(s) you just revoked?") - attempt_deletion = display.yesno(msg, yes_label="Yes (recommended)", no_label="No", - force_interactive=True, default=True) + attempt_deletion = config.delete_after_revoke + if attempt_deletion is None: + msg = ("Would you like to delete the cert(s) you just revoked?") + attempt_deletion = display.yesno(msg, yes_label="Yes (recommended)", no_label="No", + force_interactive=True, default=True) if not attempt_deletion: reporter_util.add_message("Not deleting revoked certs.", reporter_util.LOW_PRIORITY) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 2fce412e2..c5935d722 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -164,6 +164,8 @@ class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue("--cert-path" in out) self.assertTrue("--key-path" in out) self.assertTrue("--reason" in out) + self.assertTrue("--delete-after-revoke" in out) + self.assertTrue("--no-delete-after-revoke" in out) out = self._help_output(['-h', 'config_changes']) self.assertTrue("--cert-path" not in out) @@ -412,6 +414,18 @@ class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods def test_no_directory_hooks_unset(self): self.assertTrue(self.parse([]).directory_hooks) + def test_delete_after_revoke(self): + namespace = self.parse(["--delete-after-revoke"]) + self.assertTrue(namespace.delete_after_revoke) + + def test_delete_after_revoke_default(self): + namespace = self.parse([]) + self.assertEqual(namespace.delete_after_revoke, None) + + def test_no_delete_after_revoke(self): + namespace = self.parse(["--no-delete-after-revoke"]) + self.assertFalse(namespace.delete_after_revoke) + class DefaultTest(unittest.TestCase): """Tests for certbot.cli._Default.""" diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 04b71dcc7..b1d58542f 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -298,25 +298,29 @@ class RevokeTest(test_util.TempDirTestCase): self._call() self.assertFalse(mock_delete.called) -class DeleteIfAppropriateTest(unittest.TestCase): +class DeleteIfAppropriateTest(test_util.ConfigTestCase): """Tests for certbot.main._delete_if_appropriate """ - def setUp(self): - self.config = mock.Mock() - self.config.namespace = mock.Mock() - self.config.namespace.noninteractive_mode = False - def _call(self, mock_config): from certbot.main import _delete_if_appropriate _delete_if_appropriate(mock_config) - @mock.patch('certbot.cert_manager.delete') + def _test_delete_opt_out_common(self, mock_get_utility): + with mock.patch('certbot.cert_manager.delete') as mock_delete: + self._call(self.config) + mock_delete.assert_not_called() + self.assertTrue(mock_get_utility().add_message.called) + + @test_util.patch_get_utility() + def test_delete_flag_opt_out(self, mock_get_utility): + self.config.delete_after_revoke = False + self._test_delete_opt_out_common(mock_get_utility) + @test_util.patch_get_utility() - def test_delete_opt_out(self, mock_get_utility, mock_delete): + def test_delete_prompt_opt_out(self, mock_get_utility): util_mock = mock_get_utility() util_mock.yesno.return_value = False - self._call(self.config) - mock_delete.assert_not_called() + self._test_delete_opt_out_common(mock_get_utility) # pylint: disable=too-many-arguments @mock.patch('certbot.storage.renewal_file_for_certname') @@ -401,6 +405,28 @@ class DeleteIfAppropriateTest(unittest.TestCase): @mock.patch('certbot.storage.renewal_file_for_certname') @mock.patch('certbot.cert_manager.match_and_check_overlaps') @mock.patch('certbot.storage.full_archive_path') + @mock.patch('certbot.cert_manager.cert_path_to_lineage') + @mock.patch('certbot.cert_manager.delete') + @test_util.patch_get_utility() + def test_opt_in_deletion(self, mock_get_utility, mock_delete, + mock_cert_path_to_lineage, mock_full_archive_dir, + mock_match_and_check_overlaps, mock_renewal_file_for_certname): + # pylint: disable = unused-argument + config = self.config + config.namespace.delete_after_revoke = True + config.cert_path = "/some/reasonable/path" + config.certname = "" + mock_cert_path_to_lineage.return_value = "example.com" + mock_full_archive_dir.return_value = "" + mock_match_and_check_overlaps.return_value = "" + self._call(config) + self.assertEqual(mock_delete.call_count, 1) + self.assertFalse(mock_get_utility().yesno.called) + + # pylint: disable=too-many-arguments + @mock.patch('certbot.storage.renewal_file_for_certname') + @mock.patch('certbot.cert_manager.match_and_check_overlaps') + @mock.patch('certbot.storage.full_archive_path') @mock.patch('certbot.cert_manager.delete') @mock.patch('certbot.cert_manager.cert_path_to_lineage') @test_util.patch_get_utility() diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 1e0b7754b..e1aad4336 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -345,9 +345,14 @@ common auth --must-staple --domains "must-staple.le.wtf" openssl x509 -in "${root}/conf/live/must-staple.le.wtf/cert.pem" -text | grep '1.3.6.1.5.5.7.1.24' # revoke by account key -common revoke --cert-path "$root/conf/live/le.wtf/cert.pem" +common revoke --cert-path "$root/conf/live/le.wtf/cert.pem" --delete-after-revoke # revoke renewed -common revoke --cert-path "$root/conf/live/le1.wtf/cert.pem" +common revoke --cert-path "$root/conf/live/le1.wtf/cert.pem" --no-delete-after-revoke +if [ ! -d "$root/conf/live/le1.wtf" ]; then + echo "cert deleted when --no-delete-after-revoke was used!" + exit 1 +fi +common delete --cert-name le1.wtf # revoke by cert key common revoke --cert-path "$root/conf/live/le2.wtf/cert.pem" \ --key-path "$root/conf/live/le2.wtf/privkey.pem" |