diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py index 8c8a34220..d66439456 100644 --- a/certbot/certbot/_internal/client.py +++ b/certbot/certbot/_internal/client.py @@ -9,7 +9,6 @@ from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives.asymmetric.rsa import generate_private_key # type: ignore import josepy as jose import OpenSSL -import zope.component from acme import client as acme_client from acme import crypto_util as acme_crypto_util @@ -18,7 +17,6 @@ from acme import messages import certbot from certbot import crypto_util from certbot import errors -from certbot import interfaces from certbot import util from certbot._internal import account from certbot._internal import auth_handler @@ -30,6 +28,7 @@ from certbot._internal import storage from certbot._internal.plugins import selection as plugin_selection from certbot.compat import os from certbot.display import ops as display_ops +from certbot.display import util as display_util logger = logging.getLogger(__name__) @@ -517,10 +516,11 @@ class Client: return abs_cert_path, abs_chain_path, abs_fullchain_path - def deploy_certificate(self, domains, privkey_path, + def deploy_certificate(self, cert_name, domains, privkey_path, cert_path, chain_path, fullchain_path): """Install certificate + :param str cert_name: name of the certificate lineage (optional) :param list domains: list of domains to install the certificate :param str privkey_path: path to certificate private key :param str cert_path: certificate file path (optional) @@ -534,7 +534,13 @@ class Client: chain_path = None if chain_path is None else os.path.abspath(chain_path) - msg = ("Unable to install the certificate") + display_util.notify("Deploying certificate") + + msg = f"Failed to install the certificate (installer: {self.config.installer})." + if cert_name: + msg += (" Try again by running:\n\n" + f" {cli.cli_constants.cli_command} install --cert-name {cert_name}\n") + with error_handler.ErrorHandler(self._recovery_routine_with_msg, msg): for dom in domains: self.installer.deploy_cert( @@ -625,7 +631,7 @@ class Client: logger.warning("Enhancement %s was already set.", enhancement) except errors.PluginError: - logger.warning("Unable to set enhancement %s for %s", + logger.error("Unable to set enhancement %s for %s", enhancement, dom) raise @@ -638,8 +644,7 @@ class Client: """ self.installer.recovery_routine() - reporter = zope.component.getUtility(interfaces.IReporter) - reporter.add_message(success_msg, reporter.HIGH_PRIORITY) + display_util.notify(success_msg) def _rollback_and_restart(self, success_msg): """Rollback the most recent checkpoint and restart the webserver @@ -648,19 +653,18 @@ class Client: """ logger.critical("Rolling back to previous server configuration...") - reporter = zope.component.getUtility(interfaces.IReporter) try: self.installer.rollback_checkpoints() self.installer.restart() except: - reporter.add_message( + logger.error( "An error occurred and we failed to restore your config and " "restart your server. Please post to " "https://community.letsencrypt.org/c/help " - "with details about your configuration and this error you received.", - reporter.HIGH_PRIORITY) + "with details about your configuration and this error you received." + ) raise - reporter.add_message(success_msg, reporter.HIGH_PRIORITY) + display_util.notify(success_msg) def validate_key_csr(privkey, csr=None): diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 3fd72af1e..ebfbfe1d0 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -838,7 +838,19 @@ def _install_cert(config, le_client, domains, lineage=None): path_provider = lineage if lineage else config assert path_provider.cert_path is not None - le_client.deploy_certificate(domains, path_provider.key_path, + cert_name: Optional[str] = None + if isinstance(path_provider, storage.RenewableCert): + cert_name = path_provider.lineagename + elif path_provider.certname: + cert_name = path_provider.certname + else: + # Check if the cert path happens to be part of an existing lineage + try: + cert_name = cert_manager.cert_path_to_lineage(config) + except errors.Error: + pass + + le_client.deploy_certificate(cert_name, domains, path_provider.key_path, path_provider.cert_path, path_provider.chain_path, path_provider.fullchain_path) le_client.enhance_config(domains, path_provider.chain_path) diff --git a/certbot/tests/cert_manager_test.py b/certbot/tests/cert_manager_test.py index 3e8fb0de7..5f2a91cb4 100644 --- a/certbot/tests/cert_manager_test.py +++ b/certbot/tests/cert_manager_test.py @@ -578,6 +578,10 @@ class CertPathToLineageTest(storage_test.BaseRenewableCertTest): self._archive_files(x, 'fullchain')] self.assertEqual('example.org', self._call(self.config)) + def test_only_path(self): + self.config.cert_path = self.fullchain + self.assertEqual('example.org', self._call(self.config)) + class MatchAndCheckOverlaps(storage_test.BaseRenewableCertTest): """Tests for certbot._internal.cert_manager.match_and_check_overlaps w/o overlapping diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index a89636a63..2cae225b6 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -515,15 +515,16 @@ class ClientTest(ClientTestCommon): shutil.rmtree(tmp_path) - def test_deploy_certificate_success(self): + @test_util.patch_get_utility() + def test_deploy_certificate_success(self, mock_util): self.assertRaises(errors.Error, self.client.deploy_certificate, - ["foo.bar"], "key", "cert", "chain", "fullchain") + "foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain") installer = mock.MagicMock() self.client.installer = installer self.client.deploy_certificate( - ["foo.bar"], "key", "cert", "chain", "fullchain") + "foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain") installer.deploy_cert.assert_called_once_with( cert_path=os.path.abspath("cert"), chain_path=os.path.abspath("chain"), @@ -533,46 +534,81 @@ class ClientTest(ClientTestCommon): self.assertEqual(installer.save.call_count, 2) installer.restart.assert_called_once_with() - def test_deploy_certificate_failure(self): + @mock.patch('certbot._internal.client.display_util.notify') + @test_util.patch_get_utility() + def test_deploy_certificate_failure(self, mock_util, mock_notify): installer = mock.MagicMock() self.client.installer = installer + self.config.installer = "foobar" installer.deploy_cert.side_effect = errors.PluginError self.assertRaises(errors.PluginError, self.client.deploy_certificate, - ["foo.bar"], "key", "cert", "chain", "fullchain") + "foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain") installer.recovery_routine.assert_called_once_with() - def test_deploy_certificate_save_failure(self): + mock_notify.assert_any_call('Deploying certificate') + mock_notify.assert_any_call( + 'Failed to install the certificate (installer: foobar). ' + 'Try again by running:\n\n certbot install --cert-name foo.bar\n' + ) + + @mock.patch('certbot._internal.client.display_util.notify') + @test_util.patch_get_utility() + def test_deploy_certificate_failure_no_certname(self, mock_util, mock_notify): + installer = mock.MagicMock() + self.client.installer = installer + self.config.installer = "foobar" + + installer.deploy_cert.side_effect = errors.PluginError + self.assertRaises(errors.PluginError, self.client.deploy_certificate, + None, ["foo.bar"], "key", "cert", "chain", "fullchain") + installer.recovery_routine.assert_called_once_with() + + mock_notify.assert_any_call('Deploying certificate') + mock_notify.assert_any_call( + 'Failed to install the certificate (installer: foobar).' + ) + + + @test_util.patch_get_utility() + def test_deploy_certificate_save_failure(self, mock_util): installer = mock.MagicMock() self.client.installer = installer installer.save.side_effect = errors.PluginError self.assertRaises(errors.PluginError, self.client.deploy_certificate, - ["foo.bar"], "key", "cert", "chain", "fullchain") + "foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain") installer.recovery_routine.assert_called_once_with() + @mock.patch('certbot._internal.client.display_util.notify') @test_util.patch_get_utility() - def test_deploy_certificate_restart_failure(self, mock_get_utility): + def test_deploy_certificate_restart_failure(self, mock_get_utility, mock_notify): installer = mock.MagicMock() installer.restart.side_effect = [errors.PluginError, None] self.client.installer = installer self.assertRaises(errors.PluginError, self.client.deploy_certificate, - ["foo.bar"], "key", "cert", "chain", "fullchain") - self.assertEqual(mock_get_utility().add_message.call_count, 1) + "foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain") + mock_notify.assert_called_with( + 'We were unable to install your certificate, however, we successfully restored ' + 'your server to its prior configuration.') installer.rollback_checkpoints.assert_called_once_with() self.assertEqual(installer.restart.call_count, 2) + @mock.patch('certbot._internal.client.logger') @test_util.patch_get_utility() - def test_deploy_certificate_restart_failure2(self, mock_get_utility): + def test_deploy_certificate_restart_failure2(self, mock_get_utility, mock_logger): installer = mock.MagicMock() installer.restart.side_effect = errors.PluginError installer.rollback_checkpoints.side_effect = errors.ReverterError self.client.installer = installer self.assertRaises(errors.PluginError, self.client.deploy_certificate, - ["foo.bar"], "key", "cert", "chain", "fullchain") - self.assertEqual(mock_get_utility().add_message.call_count, 1) + "foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain") + self.assertEqual(mock_logger.error.call_count, 1) + self.assertIn( + 'An error occurred and we failed to restore your config', + mock_logger.error.call_args[0][0]) installer.rollback_checkpoints.assert_called_once_with() self.assertEqual(installer.restart.call_count, 1) @@ -659,7 +695,7 @@ class EnhanceConfigTest(ClientTestCommon): def test_enhance_failure(self): self.client.installer = mock.MagicMock() self.client.installer.enhance.side_effect = errors.PluginError - self._test_error() + self._test_error(enhance_error=True) self.client.installer.recovery_routine.assert_called_once_with() def test_save_failure(self): @@ -685,12 +721,19 @@ class EnhanceConfigTest(ClientTestCommon): self._test_error() self.assertIs(self.client.installer.restart.called, True) - def _test_error(self): + def _test_error(self, enhance_error=False, restart_error=False): self.config.redirect = True - with test_util.patch_get_utility() as mock_gu: + with mock.patch('certbot._internal.client.logger') as mock_logger, \ + test_util.patch_get_utility() as mock_gu: self.assertRaises( errors.PluginError, self._test_with_all_supported) - self.assertEqual(mock_gu().add_message.call_count, 1) + + if enhance_error: + self.assertEqual(mock_logger.error.call_count, 1) + self.assertIn('Unable to set enhancement', mock_logger.error.call_args_list[0][0][0]) + if restart_error: + mock_logger.critical.assert_called_with( + 'Rolling back to previous server configuration...') def _test_with_all_supported(self): if self.client.installer is None: