diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 530d75a92..a0d5e505f 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -881,6 +881,20 @@ class MultipleVhostsTest(util.ApacheTest): errors.PluginError, self.config.enhance, "certbot.demo", "unknown_enhancement") + def test_enhance_no_ssl_vhost(self): + with mock.patch("certbot_apache.configurator.logger.warning") as mock_log: + self.config.enhance("certbot.demo", "redirect") + # Check that correct logger.warning was printed + self.assertTrue("not able to find" in mock_log.call_args[0][0]) + self.assertTrue("\"redirect\"" in mock_log.call_args[0][0]) + + mock_log.reset_mock() + + self.config.enhance("certbot.demo", "ensure-http-header", "Test") + # Check that correct logger.warning was printed + self.assertTrue("not able to find" in mock_log.call_args[0][0]) + self.assertTrue("Test" in mock_log.call_args[0][0]) + @mock.patch("certbot.util.exe_exists") def test_ocsp_stapling(self, mock_exe): self.config.parser.update_runtime_variables = mock.Mock() @@ -890,6 +904,7 @@ class MultipleVhostsTest(util.ApacheTest): mock_exe.return_value = True # This will create an ssl vhost for certbot.demo + self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "staple-ocsp") # Get the ssl vhost for certbot.demo @@ -916,6 +931,7 @@ class MultipleVhostsTest(util.ApacheTest): mock_exe.return_value = True # Checking the case with already enabled ocsp stapling configuration + self.config.choose_vhost("ocspvhost.com") self.config.enhance("ocspvhost.com", "staple-ocsp") # Get the ssl vhost for letsencrypt.demo @@ -940,6 +956,7 @@ class MultipleVhostsTest(util.ApacheTest): self.config.parser.modules.add("mod_ssl.c") self.config.parser.modules.add("socache_shmcb_module") self.config.get_version = mock.Mock(return_value=(2, 2, 0)) + self.config.choose_vhost("certbot.demo") self.assertRaises(errors.PluginError, self.config.enhance, "certbot.demo", "staple-ocsp") @@ -965,6 +982,7 @@ class MultipleVhostsTest(util.ApacheTest): mock_exe.return_value = True # This will create an ssl vhost for certbot.demo + self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "ensure-http-header", "Strict-Transport-Security") @@ -984,7 +1002,8 @@ class MultipleVhostsTest(util.ApacheTest): # skip the enable mod self.config.parser.modules.add("headers_module") - # This will create an ssl vhost for certbot.demo + # This will create an ssl vhost for encryption-example.demo + self.config.choose_vhost("encryption-example.demo") self.config.enhance("encryption-example.demo", "ensure-http-header", "Strict-Transport-Security") @@ -1003,6 +1022,7 @@ class MultipleVhostsTest(util.ApacheTest): mock_exe.return_value = True # This will create an ssl vhost for certbot.demo + self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "ensure-http-header", "Upgrade-Insecure-Requests") @@ -1024,7 +1044,8 @@ class MultipleVhostsTest(util.ApacheTest): # skip the enable mod self.config.parser.modules.add("headers_module") - # This will create an ssl vhost for certbot.demo + # This will create an ssl vhost for encryption-example.demo + self.config.choose_vhost("encryption-example.demo") self.config.enhance("encryption-example.demo", "ensure-http-header", "Upgrade-Insecure-Requests") @@ -1042,6 +1063,7 @@ class MultipleVhostsTest(util.ApacheTest): self.config.get_version = mock.Mock(return_value=(2, 2)) # This will create an ssl vhost for certbot.demo + self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "redirect") # These are not immediately available in find_dir even with save() and @@ -1092,6 +1114,7 @@ class MultipleVhostsTest(util.ApacheTest): self.config.save() # This will create an ssl vhost for certbot.demo + self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "redirect") # These are not immediately available in find_dir even with save() and @@ -1158,6 +1181,9 @@ class MultipleVhostsTest(util.ApacheTest): self.config.parser.modules.add("rewrite_module") self.config.get_version = mock.Mock(return_value=(2, 3, 9)) + # Creates ssl vhost for the domain + self.config.choose_vhost("red.blue.purple.com") + self.config.enhance("red.blue.purple.com", "redirect") verify_no_redirect = ("certbot_apache.configurator." "ApacheConfigurator._verify_no_certbot_redirect") @@ -1169,7 +1195,7 @@ class MultipleVhostsTest(util.ApacheTest): # Skip the enable mod self.config.parser.modules.add("rewrite_module") self.config.get_version = mock.Mock(return_value=(2, 3, 9)) - + self.config.choose_vhost("red.blue.purple.com") self.config.enhance("red.blue.purple.com", "redirect") # Clear state about enabling redirect on this run # pylint: disable=protected-access diff --git a/certbot-apache/certbot_apache/tests/debian_test.py b/certbot-apache/certbot_apache/tests/debian_test.py index a648101e9..fde8d4c35 100644 --- a/certbot-apache/certbot_apache/tests/debian_test.py +++ b/certbot-apache/certbot_apache/tests/debian_test.py @@ -161,6 +161,8 @@ class MultipleVhostsTestDebian(util.ApacheTest): self.config.parser.modules.add("mod_ssl.c") self.config.get_version = mock.Mock(return_value=(2, 4, 7)) mock_exe.return_value = True + # This will create an ssl vhost for certbot.demo + self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "staple-ocsp") self.assertTrue("socache_shmcb_module" in self.config.parser.modules) @@ -172,6 +174,7 @@ class MultipleVhostsTestDebian(util.ApacheTest): mock_exe.return_value = True # This will create an ssl vhost for certbot.demo + self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "ensure-http-header", "Strict-Transport-Security") self.assertTrue("headers_module" in self.config.parser.modules) @@ -183,6 +186,7 @@ class MultipleVhostsTestDebian(util.ApacheTest): mock_exe.return_value = True self.config.get_version = mock.Mock(return_value=(2, 2)) # This will create an ssl vhost for certbot.demo + self.config.choose_vhost("certbot.demo") self.config.enhance("certbot.demo", "redirect") self.assertTrue("rewrite_module" in self.config.parser.modules) diff --git a/certbot/cli.py b/certbot/cli.py index ea73a7662..d353376d7 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -886,7 +886,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "flag to 0 disables log rotation entirely, causing " "Certbot to always append to the same log file.") helpful.add( - [None, "automation", "run", "certonly", "enhance"], "-n", "--non-interactive", "--noninteractive", + [None, "automation", "run", "certonly", "enhance"], + "-n", "--non-interactive", "--noninteractive", dest="noninteractive_mode", action="store_true", default=flag_default("noninteractive_mode"), help="Run without ever asking for user input. This may require " diff --git a/certbot/main.py b/certbot/main.py index 567225ef8..0234d502d 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -1,4 +1,5 @@ """Certbot main entry point.""" +# pylint: disable=too-many-lines from __future__ import print_function import functools import logging.handlers @@ -837,17 +838,22 @@ def enhance(config, plugins): :rtype: None """ - # XXX: staple-ocsp should instruct user to rerun certbot to enable - # must-staple extension + supported_enhancements = ["hsts", "redirect", "staple", "uir"] + # Check that at least one enhancement was requested on command line + if not any([getattr(config, enh) for enh in supported_enhancements]): + msg = ("Please specify one or more enhancement types to configure. To list " + "the available enhancement types, run:\n\n%s --help enhance\n") + logger.warning(msg, sys.argv[0]) + raise errors.MisconfigurationError("No enhancements requested, exiting.") + try: installer, _ = plug_sel.choose_configurator_plugins(config, plugins, "enhance") except errors.PluginSelectionError as e: return str(e) le_client = _init_le_client(config, authenticator=None, installer=installer) - domains, _ = _find_domains_or_certname(config, installer, - "Which domain names would you like to " - "enable the selected enhancements for?") - + domain_question = ("Which domain names would you like to enable the selected " + "enhancements for") + domains, _ = _find_domains_or_certname(config, installer, domain_question) le_client.enhance_config(domains, None, ask_redirect=False) diff --git a/certbot/plugins/selection.py b/certbot/plugins/selection.py index 50ff4bf04..ddbde063c 100644 --- a/certbot/plugins/selection.py +++ b/certbot/plugins/selection.py @@ -147,6 +147,7 @@ def record_chosen_plugins(config, plugins, auth, inst): def choose_configurator_plugins(config, plugins, verb): + # pylint: disable=too-many-branches """ Figure out which configurator we're going to use, modifies config.authenticator and config.installer strings to reflect that choice if @@ -160,8 +161,8 @@ def choose_configurator_plugins(config, plugins, verb): req_auth, req_inst = cli_plugin_requests(config) - installer_question= ("How would you like to authenticate and install " - "certificates?") + installer_question = ("How would you like to authenticate and install " + "certificates?") # Which plugins do we need? if verb == "run": diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index b1d58542f..82310fe9b 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1451,5 +1451,67 @@ class MakeOrVerifyNeededDirs(test_util.ConfigTestCase): strict=self.config.strict_permissions) +class EnhanceTest(unittest.TestCase): + """Tests for certbot.main.enhance.""" + + def setUp(self): + self.get_utility_patch = test_util.patch_get_utility() + self.mock_get_utility = self.get_utility_patch.start() + + def tearDown(self): + self.get_utility_patch.stop() + + def _call(self, args): + plugins = disco.PluginsRegistry.find_all() + config = configuration.NamespaceConfig( + cli.prepare_and_parse_args(plugins, args)) + + with mock.patch('certbot.main._init_le_client') as mock_init: + mock_client = mock.MagicMock() + mock_client.config = config + mock_init.return_value = mock_client + main.enhance(config, plugins) + return mock_client # returns the client + + @mock.patch("certbot.main.plug_sel.record_chosen_plugins") + @mock.patch("certbot.main._find_domains_or_certname") + def test_selection_question(self, mock_find, _rec): + mock_find.return_value = (None, None) + with mock.patch('certbot.main.plug_sel.pick_installer') as mock_pick: + self._call(['enhance', '--redirect']) + self.assertTrue(mock_pick.called) + # Check that the message includes "enhancements" + self.assertTrue("enhancements" in mock_pick.call_args[0][3]) + + @mock.patch("certbot.main.plug_sel.record_chosen_plugins") + @mock.patch("certbot.main._find_domains_or_certname") + def test_selection_auth_warning(self, mock_find, _rec): + mock_find.return_value = (None, None) + with mock.patch('certbot.main.plug_sel.pick_installer'): + with mock.patch('certbot.main.plug_sel.logger.warning') as mock_log: + mock_client = self._call(['enhance', '-a', 'webroot', '--redirect']) + self.assertTrue(mock_log.called) + self.assertTrue("make sense" in mock_log.call_args[0][0]) + self.assertTrue(mock_client.enhance_config.called) + + @mock.patch("certbot.main.plug_sel.record_chosen_plugins") + def test_enhance_config_call(self, _rec): + # mock_find.return_value = (['domain.tld'], None) + with mock.patch('certbot.main.plug_sel.pick_installer'): + mock_client = self._call(['enhance', '-d', 'domain.tld', + '-d', 'another.tld', '--redirect', + '--hsts']) + req_enh = ["redirect", "hsts"] + not_req_enh = ["staple", "uir"] + self.assertTrue(mock_client.enhance_config.called) + self.assertTrue( + all([getattr(mock_client.config, e) for e in req_enh])) + self.assertFalse( + any([getattr(mock_client.config, e) for e in not_req_enh])) + self.assertTrue( + "domain.tld" in mock_client.enhance_config.call_args[0][0]) + self.assertTrue( + "another.tld" in mock_client.enhance_config.call_args[0][0]) + if __name__ == '__main__': unittest.main() # pragma: no cover