Tests and error messages

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

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)
 

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 "

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)
 
 

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":

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