improved redirect, testcases

letsencrypt-apache/letsencrypt_apache/configurator.py

@@ -1,5 +1,6 @@
 """Apache Configuration based off of Augeas Configurator."""
 # pylint: disable=too-many-lines
+import itertools
 import logging
 import os
 import re
@@ -60,6 +61,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         prone.
     .. todo:: Write a server protocol finder. Listen <port> <protocol> or
         Protocol <protocol>.  This can verify partial setups are correct
+    .. todo:: Add directives to sites-enabled... not sites-available.
+        sites-available doesn't allow immediate find_dir search even with save()
+        and load()
 
     :ivar config: Configuration.
     :type config: :class:`~letsencrypt.interfaces.IConfig`
@@ -672,11 +676,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         try:
             return self._enhance_func[enhancement](
                 self.choose_vhost(domain), options)
-        except ValueError:
+        except KeyError:
             raise errors.PluginError(
                 "Unsupported enhancement: {}".format(enhancement))
         except errors.PluginError:
             logger.warn("Failed %s for %s", enhancement, domain)
+            raise
 
     def _enable_redirect(self, ssl_vhost, unused_options):
         """Redirect all equivalent HTTP traffic to ssl_vhost.
@@ -703,9 +708,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
         if "rewrite_module" not in self.parser.modules:
             self.enable_mod("rewrite")
+        general_vh = self._get_http_vhost(ssl_vhost)
 
-        general_v = self._get_http_vhost(ssl_vhost)
-        if general_v is None:
+        if general_vh is None:
             # Add virtual_server with redirect
             logger.debug("Did not find http version of ssl virtual host "
                          "attempting to create")
@@ -717,20 +722,23 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                         "Unable to create one as intended addresses conflict; "
                         "Current configuration does not support automated "
                         "redirection")
-            self.create_redirect_vhost(redirect_addrs)
+            self._create_redirect_vhost(redirect_addrs)
         else:
             # Check if redirection already exists
-            self._verify_no_redirects(vhost)
+            self._verify_no_redirects(general_vh)
+
             # Add directives to server
-            self.parser.add_dir(general_v.path, "RewriteEngine", "on")
-            self.parser.add_dir(general_v.path, "RewriteRule",
+            # Note: These are not immediately searchable in sites-enabled
+            #     even with save() and load()
+            self.parser.add_dir(general_vh.path, "RewriteEngine", "on")
+            self.parser.add_dir(general_vh.path, "RewriteRule",
                                 constants.REWRITE_HTTPS_ARGS)
             self.save_notes += ("Redirecting host in %s to ssl vhost in %s\n" %
-                                (general_v.filep, ssl_vhost.filep))
+                                (general_vh.filep, ssl_vhost.filep))
             self.save()
 
             logger.info("Redirecting vhost in %s to ssl vhost in %s",
-                        general_v.filep, ssl_vhost.filep)
+                        general_vh.filep, ssl_vhost.filep)
 
     def _verify_no_redirects(self, vhost):
         """Checks to see if existing redirect is in place.
@@ -775,7 +783,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
         text = self._get_redirect_config_str(ssl_vhost)
 
-        self._write_out_redirect(ssl_vhost, text)
+        redirect_filepath = self._write_out_redirect(ssl_vhost, text)
 
         self.aug.load()
         # Make a new vhost data structure and add it to the lists
@@ -853,7 +861,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
     def _get_redirect_addrs(self, ssl_vhost):
         redirects = set()
-        for addr in vhost.addrs:
+        for addr in ssl_vhost.addrs:
             redirects.add(addr.get_addr_obj("80"))
 
         return redirects

letsencrypt-apache/letsencrypt_apache/display_ops.py

@@ -60,9 +60,9 @@ def _vhost_menu(domain, vhosts):
 
     choices = []
     for vhost in vhosts:
-        if len(vhost.names) == 1:
-            disp_name = next(iter(vhost.names))
-        elif len(vhost.names) == 0:
+        if len(vhost.get_names()) == 1:
+            disp_name = next(iter(vhost.get_names()))
+        elif len(vhost.get_names()) == 0:
             disp_name = ""
         else:
             disp_name = "Multiple Names"

letsencrypt-apache/letsencrypt_apache/obj.py

@@ -78,6 +78,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
         self.aliases = serveralias
 
     def get_names(self):
+        """Return a set of all names."""
         all_names = set(self.aliases)
         # Strip out any scheme:// and <port> field from servername
         if self.name is not None:

letsencrypt-apache/letsencrypt_apache/parser.py

@@ -287,9 +287,11 @@ class ApacheParser(object):
             dir_ = self.aug.get(match).lower()
             if dir_ == "include" or dir_ == "includeoptional":
                 # start[6:] to strip off /files
+                #print self._get_include_path(self.get_arg(match +"/arg")), directive, arg
                 ordered_matches.extend(self.find_dir(
-                    directive, arg, self._get_include_path(
-                        self.get_arg(match + "/arg"))))
+                    directive, arg,
+                    self._get_include_path(self.get_arg(match + "/arg")),
+                    exclude))
             # This additionally allows Include
             if dir_ == directive.lower():
                 ordered_matches.extend(self.aug.match(match + arg_suffix))

letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py

@@ -254,6 +254,62 @@ class TwoVhost80Test(util.ApacheTest):
         mock_popen.side_effect = OSError("Can't find program")
         self.assertRaises(errors.PluginError, self.config.get_version)
 
+    # TEST ENHANCEMENTS
+    def test_enhance_unknown_enhancement(self):
+        self.assertRaises(
+            errors.PluginError,
+            self.config.enhance, "letsencrypt.demo", "unknown_enhancement")
+
+    @mock.patch("letsencrypt_apache.parser."
+                "ApacheParser.update_runtime_variables")
+    def test_redirect_well_formed_http(self, unused):
+        # This will create an ssl vhost for letsencrypt.demo
+        self.config.enhance("letsencrypt.demo", "redirect")
+
+        # These are not immediately available in find_dir even with save() and
+        # load(). They must be found in sites-available
+        rw_engine = self.config.parser.find_dir(
+            "RewriteEngine", "on", self.vh_truth[3].path)
+        rw_rule = self.config.parser.find_dir(
+            "RewriteRule", None, self.vh_truth[3].path)
+
+        self.assertEqual(len(rw_engine), 1)
+        # three args to rw_rule
+        self.assertEqual(len(rw_rule), 3)
+
+        self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path))
+        self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path))
+
+        self.assertTrue("rewrite_module" in self.config.parser.modules)
+
+    def test_redirect_twice(self):
+        # Skip the enable mod
+        self.config.parser.modules.add("rewrite_module")
+        self.config.enhance("encryption-example.demo", "redirect")
+        self.assertRaises(
+            errors.PluginError,
+            self.config.enhance, "encryption-example.demo", "redirect")
+
+    def test_unknown_rewrite(self):
+        # Skip the enable mod
+        self.config.parser.modules.add("rewrite_module")
+        self.config.parser.add_dir(
+            self.vh_truth[3].path, "RewriteRule", ["Unknown"])
+        self.config.save()
+        self.assertRaises(
+            errors.PluginError,
+            self.config.enhance, "letsencrypt.demo", "redirect")
+
+    def test_unknown_redirect(self):
+        # Skip the enable mod
+        self.config.parser.modules.add("rewrite_module")
+        self.config.parser.add_dir(
+            self.vh_truth[3].path, "Redirect", ["Unknown"])
+        self.config.save()
+        self.assertRaises(
+            errors.PluginError,
+            self.config.enhance, "letsencrypt.demo", "redirect")
+
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

letsencrypt-apache/letsencrypt_apache/tests/dvsni_test.py

@@ -109,9 +109,10 @@ class DvsniPerformTest(util.ApacheTest):
         self.assertEqual(len(vhs), 2)
         for vhost in vhs:
             self.assertEqual(vhost.addrs, set([obj.Addr.fromstring("*:443")]))
+            names = vhost.get_names()
             self.assertTrue(
-                vhost.names == set([self.achalls[0].nonce_domain]) or
-                vhost.names == set([self.achalls[1].nonce_domain]))
+                names == set([self.achalls[0].nonce_domain]) or
+                names == set([self.achalls[1].nonce_domain]))
 
 
 if __name__ == "__main__":

letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/rewrite.load

@@ -0,0 +1 @@
+LoadModule rewrite_module /usr/lib/apache2/modules/mod_rewrite.so