diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index c6f6c95a3..88a4ab190 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -17,7 +17,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Changed -* +* Stop asking interactively if the user would like to add a redirect. ### Fixed diff --git a/certbot/certbot/_internal/cli/__init__.py b/certbot/certbot/_internal/cli/__init__.py index 96dfb163e..2f83edcc8 100644 --- a/certbot/certbot/_internal/cli/__init__.py +++ b/certbot/certbot/_internal/cli/__init__.py @@ -321,12 +321,14 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): "--redirect", action="store_true", dest="redirect", default=flag_default("redirect"), help="Automatically redirect all HTTP traffic to HTTPS for the newly " - "authenticated vhost. (default: Ask)") + "authenticated vhost. (default: redirect enabled for install and run, " + "disabled for enhance)") helpful.add( "security", "--no-redirect", action="store_false", dest="redirect", default=flag_default("redirect"), help="Do not automatically redirect all HTTP traffic to HTTPS for the newly " - "authenticated vhost. (default: Ask)") + "authenticated vhost. (default: redirect enabled for install and run, " + "disabled for enhance)") helpful.add( ["security", "enhance"], "--hsts", action="store_true", dest="hsts", default=flag_default("hsts"), diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py index 526f4200f..a9bf946cc 100644 --- a/certbot/certbot/_internal/client.py +++ b/certbot/certbot/_internal/client.py @@ -28,7 +28,6 @@ from certbot._internal import constants from certbot._internal import eff from certbot._internal import error_handler from certbot._internal import storage -from certbot._internal.display import enhancements from certbot._internal.plugins import selection as plugin_selection from certbot.compat import os from certbot.display import ops as display_ops @@ -521,12 +520,13 @@ class Client(object): # sites may have been enabled / final cleanup self.installer.restart() - def enhance_config(self, domains, chain_path, ask_redirect=True): + def enhance_config(self, domains, chain_path, redirect_default=True): """Enhance the configuration. :param list domains: list of domains to configure :param chain_path: chain file path :type chain_path: `str` or `None` + :param redirect_default: boolean value that the "redirect" flag should default to :raises .errors.Error: if no installer is specified in the client. @@ -548,14 +548,8 @@ class Client(object): for config_name, enhancement_name, option in enhancement_info: config_value = getattr(self.config, config_name) if enhancement_name in supported: - if ask_redirect: - if config_name == "redirect" and config_value is None: - config_value = enhancements.ask(enhancement_name) - if not config_value: - logger.warning("Future versions of Certbot will automatically " - "configure the webserver so that all requests redirect to secure " - "HTTPS access. You can control this behavior and disable this " - "warning with the --redirect and --no-redirect flags.") + if config_name == "redirect" and config_value is None: + config_value = redirect_default if config_value: self.apply_enhancement(domains, enhancement_name, option) enhanced = True diff --git a/certbot/certbot/_internal/display/enhancements.py b/certbot/certbot/_internal/display/enhancements.py deleted file mode 100644 index ce6470708..000000000 --- a/certbot/certbot/_internal/display/enhancements.py +++ /dev/null @@ -1,63 +0,0 @@ -"""Certbot Enhancement Display""" -import logging - -import zope.component - -from certbot import errors -from certbot import interfaces -from certbot.display import util as display_util - -logger = logging.getLogger(__name__) - -# Define a helper function to avoid verbose code -util = zope.component.getUtility - - -def ask(enhancement): - """Display the enhancement to the user. - - :param str enhancement: One of the - :const:`~certbot.plugins.enhancements.ENHANCEMENTS` enhancements - - :returns: True if feature is desired, False otherwise - :rtype: bool - - :raises .errors.Error: if the enhancement provided is not supported - - """ - try: - # Call the appropriate function based on the enhancement - return DISPATCH[enhancement]() - except KeyError: - logger.error("Unsupported enhancement given to ask(): %s", enhancement) - raise errors.Error("Unsupported Enhancement") - - -def redirect_by_default(): - """Determines whether the user would like to redirect to HTTPS. - - :returns: True if redirect is desired, False otherwise - :rtype: bool - - """ - choices = [ - ("No redirect", "Make no further changes to the webserver configuration."), - ("Redirect", "Make all requests redirect to secure HTTPS access. " - "Choose this for new sites, or if you're confident your site works on HTTPS. " - "You can undo this change by editing your web server's configuration."), - ] - - code, selection = util(interfaces.IDisplay).menu( - "Please choose whether or not to redirect HTTP traffic to HTTPS, removing HTTP access.", - choices, default=1, - cli_flag="--redirect / --no-redirect", force_interactive=True) - - if code != display_util.OK: - return False - - return selection == 1 - - -DISPATCH = { - "redirect": redirect_by_default -} diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 4a57dd78d..94ccb8b5e 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -927,7 +927,7 @@ def enhance(config, plugins): config.chain_path = lineage.chain_path if oldstyle_enh: le_client = _init_le_client(config, authenticator=None, installer=installer) - le_client.enhance_config(domains, config.chain_path, ask_redirect=False) + le_client.enhance_config(domains, config.chain_path, redirect_default=False) if enhancements.are_requested(config): enhancements.enable(lineage, domains, installer, config) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 7232ed84b..3e0e5b212 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -577,8 +577,7 @@ class EnhanceConfigTest(ClientTestCommon): self.assertRaises( errors.Error, self.client.enhance_config, [self.domain], None) - @mock.patch("certbot._internal.client.enhancements") - def test_unsupported(self, mock_enhancements): + def test_unsupported(self): self.client.installer = mock.MagicMock() self.client.installer.supported_enhancements.return_value = [] @@ -588,7 +587,6 @@ class EnhanceConfigTest(ClientTestCommon): self.client.enhance_config([self.domain], None) self.assertEqual(mock_logger.warning.call_count, 1) self.client.installer.enhance.assert_not_called() - mock_enhancements.ask.assert_not_called() @mock.patch("certbot._internal.client.logger") def test_already_exists_header(self, mock_log): @@ -612,14 +610,11 @@ class EnhanceConfigTest(ClientTestCommon): self._test_with_already_existing() self.assertFalse(mock_log.warning.called) - @mock.patch("certbot._internal.client.enhancements.ask") @mock.patch("certbot._internal.client.logger") - def test_warn_redirect(self, mock_log, mock_ask): + def test_no_warn_redirect(self, mock_log): self.config.redirect = None - mock_ask.return_value = False - self._test_with_already_existing() - self.assertTrue(mock_log.warning.called) - self.assertTrue("disable" in mock_log.warning.call_args[0][0]) + self._test_with_all_supported() + self.assertFalse(mock_log.warning.called) def test_no_ask_hsts(self): self.config.hsts = True @@ -670,12 +665,6 @@ class EnhanceConfigTest(ClientTestCommon): self.client.installer = installer self._test_error_with_rollback() - @mock.patch("certbot._internal.client.enhancements.ask") - def test_ask(self, mock_ask): - self.config.redirect = None - mock_ask.return_value = True - self._test_with_all_supported() - def _test_error_with_rollback(self): self._test_error() self.assertTrue(self.client.installer.restart.called) diff --git a/certbot/tests/display/enhancements_test.py b/certbot/tests/display/enhancements_test.py deleted file mode 100644 index edace29b1..000000000 --- a/certbot/tests/display/enhancements_test.py +++ /dev/null @@ -1,57 +0,0 @@ -"""Module for enhancement UI.""" -import logging -import unittest - -import mock - -from certbot import errors -from certbot.display import util as display_util - - -class AskTest(unittest.TestCase): - """Test the ask method.""" - def setUp(self): - logging.disable(logging.CRITICAL) - - def tearDown(self): - logging.disable(logging.NOTSET) - - @classmethod - def _call(cls, enhancement): - from certbot._internal.display.enhancements import ask - return ask(enhancement) - - @mock.patch("certbot._internal.display.enhancements.util") - def test_redirect(self, mock_util): - mock_util().menu.return_value = (display_util.OK, 1) - self.assertTrue(self._call("redirect")) - - def test_key_error(self): - self.assertRaises(errors.Error, self._call, "unknown_enhancement") - - -class RedirectTest(unittest.TestCase): - """Test the redirect_by_default method.""" - @classmethod - def _call(cls): - from certbot._internal.display.enhancements import redirect_by_default - return redirect_by_default() - - @mock.patch("certbot._internal.display.enhancements.util") - def test_secure(self, mock_util): - mock_util().menu.return_value = (display_util.OK, 1) - self.assertTrue(self._call()) - - @mock.patch("certbot._internal.display.enhancements.util") - def test_cancel(self, mock_util): - mock_util().menu.return_value = (display_util.CANCEL, 1) - self.assertFalse(self._call()) - - @mock.patch("certbot._internal.display.enhancements.util") - def test_easy(self, mock_util): - mock_util().menu.return_value = (display_util.OK, 0) - self.assertFalse(self._call()) - - -if __name__ == "__main__": - unittest.main() # pragma: no cover