crypt/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-01-26 07:41:33 +03:00
Code Activity

Remove interactive redirect ask (#7861)

Browse Source 
Fixes #7594.

Removes the code asking interactively if the user would like to add a redirect.

* Remove interactive redirect ask

* display.enhancements is no longer used, so remove it.

* update changelog

* remove references to removed display.enhancements

* add redirect_default flag to enhance_config to conditionally set default for redirect value

* Update default in help text.
This commit is contained in:
ohemorange
2020-03-31 12:12:14 -07:00
committed by GitHub
parent 6df90d17ae
commit dbda499b08
7 changed files with 14 additions and 149 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
certbot/CHANGELOG.md
View File

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

6
certbot/certbot/_internal/cli/__init__.py
View File

@@ -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"),

14
certbot/certbot/_internal/client.py
View File

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

63
certbot/certbot/_internal/display/enhancements.py
View File

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

2
certbot/certbot/_internal/main.py
View File

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

19
certbot/tests/client_test.py
View File

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

57
certbot/tests/display/enhancements_test.py
View File

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