Merge pull request #2758 from letsencrypt/interactive-webroot

Interactive webroot

letsencrypt/cli.py

@@ -2,7 +2,6 @@
 from __future__ import print_function
 import argparse
 import glob
-import json
 import logging
 import logging.handlers
 import os
@@ -315,12 +314,6 @@ class HelpfulArgumentParser(object):
 
         # Do any post-parsing homework here
 
-        # we get domains from -d, but also from the webroot map...
-        if parsed_args.webroot_map:
-            for domain in parsed_args.webroot_map.keys():
-                if domain not in parsed_args.domains:
-                    parsed_args.domains.append(domain)
-
         if parsed_args.staging or parsed_args.dry_run:
             if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
                 conflicts = ["--staging"] if parsed_args.staging else []
@@ -353,10 +346,7 @@ class HelpfulArgumentParser(object):
         return parsed_args
 
     def handle_csr(self, parsed_args):
-        """
-        Process a --csr flag. This needs to happen early enough that the
-        webroot plugin can know about the calls to process_domain
-        """
+        """Process a --csr flag."""
         if parsed_args.verb != "certonly":
             raise errors.Error("Currently, a CSR file may only be specified "
                                "when obtaining a new or replacement "
@@ -377,14 +367,11 @@ class HelpfulArgumentParser(object):
                 logger.debug("DER CSR parse error %s", e1)
                 logger.debug("PEM CSR parse error %s", traceback.format_exc())
                 raise errors.Error("Failed to parse CSR file: {0}".format(parsed_args.csr[0]))
-        for d in domains:
-            process_domain(parsed_args, d)
 
-        for d in domains:
-            sanitised = le_util.enforce_domain_sanity(d)
-            if d.lower() != sanitised:
-                raise errors.ConfigurationError(
-                    "CSR domain {0} needs to be sanitised to {1}.".format(d, sanitised))
+        # This is not necessary for webroot to work, however,
+        # obtain_certificate_from_csr requires parsed_args.domains to be set
+        for domain in domains:
+            add_domains(parsed_args, domain)
 
         if not domains:
             # TODO: add CN to domains instead:
@@ -611,7 +598,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
     #for subparser in parser_run, parser_auth, parser_install:
     #    subparser.add_argument("domains", nargs="*", metavar="domain")
     helpful.add(None, "-d", "--domains", "--domain", dest="domains",
-                metavar="DOMAIN", action=DomainFlagProcessor, default=[],
+                metavar="DOMAIN", action=_DomainsAction, default=[],
                 help="Domain names to apply. For multiple domains you can use "
                 "multiple -d flags or enter a comma separated list of domains "
                 "as a parameter.")
@@ -892,83 +879,35 @@ def _plugins_parsing(helpful, plugins):
 
     helpful.add_plugin_args(plugins)
 
-    # These would normally be a flag within the webroot plugin, but because
-    # they are parsed in conjunction with --domains, they live here for
-    # legibility. helpful.add_plugin_ags must be called first to add the
-    # "webroot" topic
-    helpful.add("webroot", "-w", "--webroot-path", default=[], action=WebrootPathProcessor,
-                help="public_html / webroot path. This can be specified multiple times to "
-                     "handle different domains; each domain will have the webroot path that"
-                     " preceded it.  For instance: `-w /var/www/example -d example.com -d "
-                     "www.example.com -w /var/www/thing -d thing.net -d m.thing.net`")
-    # --webroot-map still has some awkward properties, so it is undocumented
-    helpful.add("webroot", "--webroot-map", default={}, action=WebrootMapProcessor,
-                help="JSON dictionary mapping domains to webroot paths; this "
-                     "implies -d for each entry. You may need to escape this "
-                     "from your shell. E.g.: --webroot-map "
-                     """'{"eg1.is,m.eg1.is":"/www/eg1/", "eg2.is":"/www/eg2"}' """
-                     "This option is merged with, but takes precedence over, "
-                     "-w / -d entries. At present, if you put webroot-map in "
-                     "a config file, it needs to be on a single line, like: "
-                     'webroot-map = {"example.com":"/var/www"}.')
+
+class _DomainsAction(argparse.Action):
+    """Action class for parsing domains."""
+
+    def __call__(self, parser, namespace, domain, option_string=None):
+        """Just wrap add_domains in argparseese."""
+        add_domains(namespace, domain)
 
 
-class WebrootPathProcessor(argparse.Action):  # pylint: disable=missing-docstring
-    def __init__(self, *args, **kwargs):
-        self.domain_before_webroot = False
-        argparse.Action.__init__(self, *args, **kwargs)
+def add_domains(args_or_config, domains):
+    """Registers new domains to be used during the current client run.
 
-    def __call__(self, parser, args, webroot, option_string=None):
-        """
-        Keep a record of --webroot-path / -w flags during processing, so that
-        we know which apply to which -d flags
-        """
-        if not args.webroot_path:      # first -w flag encountered
-            # if any --domain flags preceded the first --webroot-path flag,
-            # apply that webroot path to those; subsequent entries in
-            # args.webroot_map are filled in by cli.DomainFlagProcessor
-            if args.domains:
-                self.domain_before_webroot = True
-                for d in args.domains:
-                    args.webroot_map.setdefault(d, webroot)
-        elif self.domain_before_webroot:
-            # FIXME if you set domains in a args file, you should get a different error
-            # here, pointing you to --webroot-map
-            raise errors.Error("If you specify multiple webroot paths, one of "
-                               "them must precede all domain flags")
-        args.webroot_path.append(webroot)
+    Domains are not added to the list of requested domains if they have
+    already been registered.
 
+    :param args_or_config: parsed command line arguments
+    :type args_or_config: argparse.Namespace or
+        configuration.NamespaceConfig
+    :param str domain: one or more comma separated domains
 
-def process_domain(args_or_config, domain_arg, webroot_path=None):
-    """
-    Process a new -d flag, helping the webroot plugin construct a map of
-    {domain : webrootpath} if -w / --webroot-path is in use
-
-    :param args_or_config: may be an argparse args object, or a NamespaceConfig object
-    :param str domain_arg: a string representing 1+ domains, eg: "eg.is, example.com"
-    :param str webroot_path: (optional) the webroot_path for these domains
+    :returns: domains after they have been normalized and validated
+    :rtype: `list` of `str`
 
     """
-    webroot_path = webroot_path if webroot_path else args_or_config.webroot_path
-
-    for domain in (d.strip() for d in domain_arg.split(",")):
-        domain = le_util.enforce_domain_sanity(domain)
+    validated_domains = []
+    for domain in domains.split(","):
+        domain = le_util.enforce_domain_sanity(domain.strip())
+        validated_domains.append(domain)
         if domain not in args_or_config.domains:
             args_or_config.domains.append(domain)
-            # Each domain has a webroot_path of the most recent -w flag
-            # unless it was explicitly included in webroot_map
-            if webroot_path:
-                args_or_config.webroot_map.setdefault(domain, webroot_path[-1])
 
-
-class WebrootMapProcessor(argparse.Action):  # pylint: disable=missing-docstring
-    def __call__(self, parser, args, webroot_map_arg, option_string=None):
-        webroot_map = json.loads(webroot_map_arg)
-        for domains, webroot_path in six.iteritems(webroot_map):
-            process_domain(args, domains, [webroot_path])
-
-
-class DomainFlagProcessor(argparse.Action):  # pylint: disable=missing-docstring
-    def __call__(self, parser, args, domain_arg, option_string=None):
-        """Just wrap process_domain in argparseese."""
-        process_domain(args, domain_arg)
+    return validated_domains

letsencrypt/main.py

@@ -255,14 +255,10 @@ def _find_duplicative_certs(config, domains):
 
 
 def _find_domains(config, installer):
-    if not config.domains:
-        domains = display_ops.choose_names(installer)
-        # record in config.domains (so that it can be serialised in renewal config files),
-        # and set webroot_map entries if applicable
-        for d in domains:
-            cli.process_domain(config, d)
-    else:
+    if config.domains:
         domains = config.domains
+    else:
+        domains = display_ops.choose_names(installer)
 
     if not domains:
         raise errors.Error("Please specify --domains, or --installer that "

letsencrypt/plugins/webroot.py

@@ -1,16 +1,21 @@
 """Webroot plugin."""
+import argparse
+import collections
 import errno
+import json
 import logging
 import os
-from collections import defaultdict
 
-import zope.interface
 import six
+import zope.component
+import zope.interface
 
 from acme import challenges
 
+from letsencrypt import cli
 from letsencrypt import errors
 from letsencrypt import interfaces
+from letsencrypt.display import util as display_util
 from letsencrypt.plugins import common
 
 
@@ -35,9 +40,21 @@ to serve all files under specified web root ({0})."""
 
     @classmethod
     def add_parser_arguments(cls, add):
-        # --webroot-path and --webroot-map are added in cli.py because they
-        # are parsed in conjunction with --domains
-        pass
+        add("path", "-w", default=[], action=_WebrootPathAction,
+            help="public_html / webroot path. This can be specified multiple "
+                 "times to handle different domains; each domain will have "
+                 "the webroot path that preceded it.  For instance: `-w "
+                 "/var/www/example -d example.com -d www.example.com -w "
+                 "/var/www/thing -d thing.net -d m.thing.net`")
+        add("map", default={}, action=_WebrootMapAction,
+            help="JSON dictionary mapping domains to webroot paths; this "
+                 "implies -d for each entry. You may need to escape this from "
+                 "your shell. E.g.: --webroot-map "
+                 '\'{"eg1.is,m.eg1.is":"/www/eg1/", "eg2.is":"/www/eg2"}\' '
+                 "This option is merged with, but takes precedence over, -w / "
+                 "-d entries. At present, if you put webroot-map in a config "
+                 "file, it needs to be on a single line, like: webroot-map = "
+                 '{"example.com":"/var/www"}.')
 
     def get_chall_pref(self, domain):  # pragma: no cover
         # pylint: disable=missing-docstring,no-self-use,unused-argument
@@ -46,19 +63,102 @@ to serve all files under specified web root ({0})."""
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
         self.full_roots = {}
-        self.performed = defaultdict(set)
+        self.performed = collections.defaultdict(set)
 
     def prepare(self):  # pylint: disable=missing-docstring
-        path_map = self.conf("map")
+        pass
 
+    def perform(self, achalls):  # pylint: disable=missing-docstring
+        self._set_webroots(achalls)
+
+        self._create_challenge_dirs()
+
+        return [self._perform_single(achall) for achall in achalls]
+
+    def _set_webroots(self, achalls):
+        if self.conf("path"):
+            webroot_path = self.conf("path")[-1]
+            logger.info("Using the webroot path %s for all unmatched domains.",
+                        webroot_path)
+            for achall in achalls:
+                self.conf("map").setdefault(achall.domain, webroot_path)
+        else:
+            known_webroots = list(set(six.itervalues(self.conf("map"))))
+            for achall in achalls:
+                if achall.domain not in self.conf("map"):
+                    new_webroot = self._prompt_for_webroot(achall.domain,
+                                                           known_webroots)
+                    # Put the most recently input
+                    # webroot first for easy selection
+                    try:
+                        known_webroots.remove(new_webroot)
+                    except ValueError:
+                        pass
+                    known_webroots.insert(0, new_webroot)
+                    self.conf("map")[achall.domain] = new_webroot
+
+    def _prompt_for_webroot(self, domain, known_webroots):
+        webroot = None
+
+        while webroot is None:
+            webroot = self._prompt_with_webroot_list(domain, known_webroots)
+
+            if webroot is None:
+                webroot = self._prompt_for_new_webroot(domain)
+
+        return webroot
+
+    def _prompt_with_webroot_list(self, domain, known_webroots):
+        display = zope.component.getUtility(interfaces.IDisplay)
+
+        while True:
+            code, index = display.menu(
+                "Select the webroot for {0}:".format(domain),
+                ["Enter a new webroot"] + known_webroots,
+                help_label="Help", cli_flag="--" + self.option_name("path"))
+            if code == display_util.CANCEL:
+                raise errors.PluginError(
+                    "Every requested domain must have a "
+                    "webroot when using the webroot plugin.")
+            elif code == display_util.HELP:
+                display.notification(
+                    "To use the webroot plugin, you need to have an "
+                    "HTTP server running on this system serving files "
+                    "for the requested domain. Additionally, this "
+                    "server should be serving all files contained in a "
+                    "public_html or webroot directory. The webroot "
+                    "plugin works by temporarily saving necessary "
+                    "resources in the HTTP server's webroot directory "
+                    "to pass domain validation challenges.")
+            else:  # code == display_util.OK
+                return None if index == 0 else known_webroots[index - 1]
+
+    def _prompt_for_new_webroot(self, domain):
+        display = zope.component.getUtility(interfaces.IDisplay)
+
+        while True:
+            code, webroot = display.directory_select(
+                "Input the webroot for {0}:".format(domain))
+            if code == display_util.HELP:
+                # Help can currently only be selected
+                # when using the ncurses interface
+                display.notification(display_util.DSELECT_HELP)
+            elif code == display_util.CANCEL:
+                return None
+            else:  # code == display_util.OK
+                try:
+                    return _validate_webroot(webroot)
+                except errors.PluginError as error:
+                    display.notification(str(error), pause=False)
+
+    def _create_challenge_dirs(self):
+        path_map = self.conf("map")
         if not path_map:
             raise errors.PluginError(
                 "Missing parts of webroot configuration; please set either "
                 "--webroot-path and --domains, or --webroot-map. Run with "
                 " --help webroot for examples.")
         for name, path in path_map.items():
-            if not os.path.isdir(path):
-                raise errors.PluginError(path + " does not exist or is not a directory")
             self.full_roots[name] = os.path.join(path, challenges.HTTP01.URI_ROOT_PATH)
 
             logger.debug("Creating root challenges validation dir at %s",
@@ -96,28 +196,13 @@ to serve all files under specified web root ({0})."""
             finally:
                 os.umask(old_umask)
 
-    def perform(self, achalls):  # pylint: disable=missing-docstring
-        assert self.full_roots, "Webroot plugin appears to be missing webroot map"
-        return [self._perform_single(achall) for achall in achalls]
-
-    def _get_root_path(self, achall):
-        try:
-            path = self.full_roots[achall.domain]
-        except KeyError:
-            raise errors.PluginError("Missing --webroot-path for domain: {0}"
-                                     .format(achall.domain))
-        if not os.path.exists(path):
-            raise errors.PluginError("Mysteriously missing path {0} for domain: {1}"
-                                     .format(path, achall.domain))
-        return path
-
     def _get_validation_path(self, root_path, achall):
         return os.path.join(root_path, achall.chall.encode("token"))
 
     def _perform_single(self, achall):
         response, validation = achall.response_and_validation()
 
-        root_path = self._get_root_path(achall)
+        root_path = self.full_roots[achall.domain]
         validation_path = self._get_validation_path(root_path, achall)
         logger.debug("Attempting to save validation to %s", validation_path)
 
@@ -136,11 +221,12 @@ to serve all files under specified web root ({0})."""
 
     def cleanup(self, achalls):  # pylint: disable=missing-docstring
         for achall in achalls:
-            root_path = self._get_root_path(achall)
-            validation_path = self._get_validation_path(root_path, achall)
-            logger.debug("Removing %s", validation_path)
-            os.remove(validation_path)
-            self.performed[root_path].remove(achall)
+            root_path = self.full_roots.get(achall.domain, None)
+            if root_path is not None:
+                validation_path = self._get_validation_path(root_path, achall)
+                logger.debug("Removing %s", validation_path)
+                os.remove(validation_path)
+                self.performed[root_path].remove(achall)
 
         for root_path, achalls in six.iteritems(self.performed):
             if not achalls:
@@ -157,3 +243,53 @@ to serve all files under specified web root ({0})."""
                                      root_path)
                     else:
                         raise
+
+
+class _WebrootMapAction(argparse.Action):
+    """Action class for parsing webroot_map."""
+
+    def __call__(self, parser, namespace, webroot_map, option_string=None):
+        for domains, webroot_path in six.iteritems(json.loads(webroot_map)):
+            webroot_path = _validate_webroot(webroot_path)
+            namespace.webroot_map.update(
+                (d, webroot_path) for d in cli.add_domains(namespace, domains))
+
+
+class _WebrootPathAction(argparse.Action):
+    """Action class for parsing webroot_path."""
+
+    def __init__(self, *args, **kwargs):
+        super(_WebrootPathAction, self).__init__(*args, **kwargs)
+        self._domain_before_webroot = False
+
+    def __call__(self, parser, namespace, webroot_path, option_string=None):
+        if self._domain_before_webroot:
+            raise errors.PluginError(
+                "If you specify multiple webroot paths, "
+                "one of them must precede all domain flags")
+
+        if namespace.webroot_path:
+            # Apply previous webroot to all matched
+            # domains before setting the new webroot path
+            prev_webroot = namespace.webroot_path[-1]
+            for domain in namespace.domains:
+                namespace.webroot_map.setdefault(domain, prev_webroot)
+        elif namespace.domains:
+            self._domain_before_webroot = True
+
+        namespace.webroot_path.append(_validate_webroot(webroot_path))
+
+
+def _validate_webroot(webroot_path):
+    """Validates and returns the absolute path of webroot_path.
+
+    :param str webroot_path: path to the webroot directory
+
+    :returns: absolute path of webroot_path
+    :rtype: str
+
+    """
+    if not os.path.isdir(webroot_path):
+        raise errors.PluginError(webroot_path + " does not exist or is not a directory")
+
+    return os.path.abspath(webroot_path)

letsencrypt/plugins/webroot_test.py

@@ -2,6 +2,7 @@
 
 from __future__ import print_function
 
+import argparse
 import errno
 import os
 import shutil
@@ -10,12 +11,14 @@ import tempfile
 import unittest
 
 import mock
+import six
 
 from acme import challenges
 from acme import jose
 
 from letsencrypt import achallenges
 from letsencrypt import errors
+from letsencrypt.display import util as display_util
 
 from letsencrypt.tests import acme_util
 from letsencrypt.tests import test_util
@@ -53,23 +56,74 @@ class AuthenticatorTest(unittest.TestCase):
     def test_add_parser_arguments(self):
         add = mock.MagicMock()
         self.auth.add_parser_arguments(add)
-        self.assertEqual(0, add.call_count)  # args moved to cli.py!
+        self.assertEqual(2, add.call_count)
 
-    def test_prepare_bad_root(self):
-        self.config.webroot_path = os.path.join(self.path, "null")
-        self.config.webroot_map["thing.com"] = self.config.webroot_path
-        self.assertRaises(errors.PluginError, self.auth.prepare)
-
-    def test_prepare_missing_root(self):
-        self.config.webroot_path = None
-        self.config.webroot_map = {}
-        self.assertRaises(errors.PluginError, self.auth.prepare)
-
-    def test_prepare_full_root_exists(self):
-        # prepare() has already been called once in setUp()
+    def test_prepare(self):
         self.auth.prepare()  # shouldn't raise any exceptions
 
-    def test_prepare_reraises_other_errors(self):
+    @mock.patch("letsencrypt.plugins.webroot.zope.component.getUtility")
+    def test_webroot_from_list(self, mock_get_utility):
+        self.config.webroot_path = []
+        self.config.webroot_map = {"otherthing.com": self.path}
+        mock_display = mock_get_utility()
+        mock_display.menu.return_value = (display_util.OK, 1,)
+
+        self.auth.perform([self.achall])
+        self.assertTrue(mock_display.menu.called)
+        for call in mock_display.menu.call_args_list:
+            self.assertTrue(self.achall.domain in call[0][0])
+            self.assertTrue(all(
+                webroot in call[0][1]
+                for webroot in six.itervalues(self.config.webroot_map)))
+        self.assertEqual(self.config.webroot_map[self.achall.domain],
+                         self.path)
+
+    @mock.patch("letsencrypt.plugins.webroot.zope.component.getUtility")
+    def test_webroot_from_list_help_and_cancel(self, mock_get_utility):
+        self.config.webroot_path = []
+        self.config.webroot_map = {"otherthing.com": self.path}
+
+        mock_display = mock_get_utility()
+        mock_display.menu.side_effect = ((display_util.HELP, -1),
+                                         (display_util.CANCEL, -1),)
+        self.assertRaises(errors.PluginError, self.auth.perform, [self.achall])
+        self.assertTrue(mock_display.notification.called)
+        self.assertTrue(mock_display.menu.called)
+        for call in mock_display.menu.call_args_list:
+            self.assertTrue(self.achall.domain in call[0][0])
+            self.assertTrue(all(
+                webroot in call[0][1]
+                for webroot in six.itervalues(self.config.webroot_map)))
+
+    @mock.patch("letsencrypt.plugins.webroot.zope.component.getUtility")
+    def test_new_webroot(self, mock_get_utility):
+        self.config.webroot_path = []
+        self.config.webroot_map = {}
+
+        imaginary_dir = os.path.join(os.sep, "imaginary", "dir")
+
+        mock_display = mock_get_utility()
+        mock_display.menu.return_value = (display_util.OK, 0,)
+        mock_display.directory_select.side_effect = (
+            (display_util.HELP, -1,), (display_util.CANCEL, -1,),
+            (display_util.OK, imaginary_dir,), (display_util.OK, self.path,),)
+        self.auth.perform([self.achall])
+
+        self.assertTrue(mock_display.notification.called)
+        for call in mock_display.notification.call_args_list:
+            self.assertTrue(imaginary_dir in call[0][0] or
+                            display_util.DSELECT_HELP == call[0][0])
+
+        self.assertTrue(mock_display.directory_select.called)
+        for call in mock_display.directory_select.call_args_list:
+            self.assertTrue(self.achall.domain in call[0][0])
+
+    def test_perform_missing_root(self):
+        self.config.webroot_path = None
+        self.config.webroot_map = {}
+        self.assertRaises(errors.PluginError, self.auth.perform, [])
+
+    def test_perform_reraises_other_errors(self):
         self.auth.full_path = os.path.join(self.path, "null")
         permission_canary = os.path.join(self.path, "rnd")
         with open(permission_canary, "w") as f:
@@ -80,20 +134,20 @@ class AuthenticatorTest(unittest.TestCase):
             print("Warning, running tests as root skips permissions tests...")
         except IOError:
             # ok, permissions work, test away...
-            self.assertRaises(errors.PluginError, self.auth.prepare)
+            self.assertRaises(errors.PluginError, self.auth.perform, [])
         os.chmod(self.path, 0o700)
 
     @mock.patch("letsencrypt.plugins.webroot.os.chown")
     def test_failed_chown_eacces(self, mock_chown):
         mock_chown.side_effect = OSError(errno.EACCES, "msg")
-        self.auth.prepare()  # exception caught and logged
+        self.auth.perform([self.achall])  # exception caught and logged
 
     @mock.patch("letsencrypt.plugins.webroot.os.chown")
     def test_failed_chown_not_eacces(self, mock_chown):
         mock_chown.side_effect = OSError()
-        self.assertRaises(errors.PluginError, self.auth.prepare)
+        self.assertRaises(errors.PluginError, self.auth.perform, [])
 
-    def test_prepare_permissions(self):
+    def test_perform_permissions(self):
         self.auth.prepare()
 
         # Remove exec bit from permission check, so that it
@@ -116,18 +170,6 @@ class AuthenticatorTest(unittest.TestCase):
         self.assertEqual(os.stat(self.validation_path).st_gid, parent_gid)
         self.assertEqual(os.stat(self.validation_path).st_uid, parent_uid)
 
-    def test_perform_missing_path(self):
-        self.auth.prepare()
-
-        missing_achall = achallenges.KeyAuthorizationAnnotatedChallenge(
-            challb=acme_util.HTTP01_P, domain="thing2.com", account_key=KEY)
-        self.assertRaises(
-            errors.PluginError, self.auth.perform, [missing_achall])
-
-        self.auth.full_roots[self.achall.domain] = 'null'
-        self.assertRaises(
-            errors.PluginError, self.auth.perform, [self.achall])
-
     def test_perform_cleanup(self):
         self.auth.prepare()
         responses = self.auth.perform([self.achall])
@@ -183,5 +225,52 @@ class AuthenticatorTest(unittest.TestCase):
         self.assertFalse(os.path.exists(self.validation_path))
         self.assertTrue(os.path.exists(self.root_challenge_path))
 
+
+class WebrootActionTest(unittest.TestCase):
+    """Tests for webroot argparse actions."""
+
+    achall = achallenges.KeyAuthorizationAnnotatedChallenge(
+        challb=acme_util.HTTP01_P, domain="thing.com", account_key=KEY)
+
+    def setUp(self):
+        from letsencrypt.plugins.webroot import Authenticator
+        self.path = tempfile.mkdtemp()
+        self.parser = argparse.ArgumentParser()
+        self.parser.add_argument("-d", "--domains",
+                                 action="append", default=[])
+        Authenticator.inject_parser_options(self.parser, "webroot")
+
+    def test_webroot_map_action(self):
+        args = self.parser.parse_args(
+            ["--webroot-map", '{{"thing.com":"{0}"}}'.format(self.path)])
+        self.assertEqual(args.webroot_map["thing.com"], self.path)
+
+    def test_domain_before_webroot(self):
+        args = self.parser.parse_args(
+            "-d {0} -w {1}".format(self.achall.domain, self.path).split())
+        config = self._get_config_after_perform(args)
+        self.assertEqual(config.webroot_map[self.achall.domain], self.path)
+
+    def test_domain_before_webroot_error(self):
+        self.assertRaises(errors.PluginError, self.parser.parse_args,
+                          "-d foo -w bar -w baz".split())
+        self.assertRaises(errors.PluginError, self.parser.parse_args,
+                          "-d foo -w bar -d baz -w qux".split())
+
+    def test_multiwebroot(self):
+        args = self.parser.parse_args("-w {0} -d {1} -w {2} -d bar".format(
+            self.path, self.achall.domain, tempfile.mkdtemp()).split())
+        self.assertEqual(args.webroot_map[self.achall.domain], self.path)
+        config = self._get_config_after_perform(args)
+        self.assertEqual(
+            config.webroot_map[self.achall.domain], self.path)
+
+    def _get_config_after_perform(self, config):
+        from letsencrypt.plugins.webroot import Authenticator
+        auth = Authenticator(config, "webroot")
+        auth.perform([self.achall])
+        return auth.config
+
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

letsencrypt/renewal.py

@@ -18,6 +18,7 @@ from letsencrypt import constants
 from letsencrypt import crypto_util
 from letsencrypt import errors
 from letsencrypt import interfaces
+from letsencrypt import le_util
 from letsencrypt import hooks
 from letsencrypt import storage
 from letsencrypt.plugins import disco as plugins_disco
@@ -85,8 +86,8 @@ def _reconstitute(config, full_path):
         return None
 
     try:
-        for d in renewal_candidate.names():
-            cli.process_domain(config, d)
+        config.domains = [le_util.enforce_domain_sanity(d)
+                          for d in renewal_candidate.names()]
     except errors.ConfigurationError as error:
         logger.warning("Renewal configuration file %s references a cert "
                        "that contains an invalid domain name. The problem "

letsencrypt/tests/cli_test.py

@@ -225,13 +225,6 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             self.assertTrue("The nginx plugin is not working" in ret)
             self.assertTrue("MisconfigurationError" in ret)
 
-        args = ["certonly", "--webroot"]
-        try:
-            self._call(args)
-            assert False, "Exception should have been raised"
-        except errors.PluginSelectionError as e:
-            self.assertTrue("please set either --webroot-path" in e.message)
-
         self._cli_missing_flag(["--standalone"], "With the standalone plugin, you probably")
 
         with mock.patch("letsencrypt.main._init_le_client") as mock_init:
@@ -470,60 +463,6 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         conflicts += ['--staging']
         self._check_server_conflict_message(short_args, conflicts)
 
-    def _webroot_map_test(self, map_arg, path_arg, domains_arg, # pylint: disable=too-many-arguments
-                          expected_map, expectect_domains, extra_args=None):
-        parse = self._get_argument_parser()
-        webroot_map_args = extra_args if extra_args else []
-        if map_arg:
-            webroot_map_args.extend(["--webroot-map", map_arg])
-        if path_arg:
-            webroot_map_args.extend(["-w", path_arg])
-        if domains_arg:
-            webroot_map_args.extend(["-d", domains_arg])
-        namespace = parse(webroot_map_args)
-        domains = main._find_domains(namespace, mock.MagicMock()) # pylint: disable=protected-access
-        self.assertEqual(namespace.webroot_map, expected_map)
-        self.assertEqual(set(domains), set(expectect_domains))
-
-    def test_parse_webroot(self):
-        parse = self._get_argument_parser()
-        webroot_args = ['--webroot', '-w', '/var/www/example',
-            '-d', 'example.com,www.example.com', '-w', '/var/www/superfluous',
-            '-d', 'superfluo.us', '-d', 'www.superfluo.us']
-        namespace = parse(webroot_args)
-        self.assertEqual(namespace.webroot_map, {
-            'example.com': '/var/www/example',
-            'www.example.com': '/var/www/example',
-            'www.superfluo.us': '/var/www/superfluous',
-            'superfluo.us': '/var/www/superfluous'})
-
-        webroot_args = ['-d', 'stray.example.com'] + webroot_args
-        self.assertRaises(errors.Error, parse, webroot_args)
-
-        simple_map = '{"eg.com" : "/tmp"}'
-        expected_map = {"eg.com": "/tmp"}
-        self._webroot_map_test(simple_map, None, None, expected_map, ["eg.com"])
-
-        # test merging webroot maps from the cli and a webroot map
-        expected_map["eg2.com"] = "/tmp2"
-        domains = ["eg.com", "eg2.com"]
-        self._webroot_map_test(simple_map, "/tmp2", "eg2.com,eg.com", expected_map, domains)
-
-        # test inclusion of interactively specified domains in the webroot map
-        with mock.patch('letsencrypt.display.ops.choose_names') as mock_choose:
-            mock_choose.return_value = domains
-            expected_map["eg2.com"] = "/tmp"
-            self._webroot_map_test(None, "/tmp", None, expected_map, domains)
-
-        extra_args = ['-c', test_util.vector_path('webrootconftest.ini')]
-        self._webroot_map_test(None, None, None, expected_map, domains, extra_args)
-
-        webroot_map_args = ['--webroot-map',
-                            '{"eg.com.,www.eg.com": "/tmp", "eg.is.": "/tmp2"}']
-        namespace = parse(webroot_map_args)
-        self.assertEqual(namespace.webroot_map,
-                         {"eg.com": "/tmp", "www.eg.com": "/tmp", "eg.is": "/tmp2"})
-
     def _certonly_new_request_common(self, mock_client, args=None):
         with mock.patch('letsencrypt.main._treat_as_renewal') as mock_renewal:
             mock_renewal.return_value = ("newcert", None)

letsencrypt/tests/client_test.py

@@ -136,8 +136,7 @@ class ClientTest(unittest.TestCase):
 
     # FIXME move parts of this to test_cli.py...
     @mock.patch("letsencrypt.client.logger")
-    @mock.patch("letsencrypt.cli.process_domain")
-    def test_obtain_certificate_from_csr(self, mock_process_domain, mock_logger):
+    def test_obtain_certificate_from_csr(self, mock_logger):
         self._mock_obtain_certificate()
         from letsencrypt import cli
         test_csr = le_util.CSR(form="der", file=None, data=CSR_SAN)
@@ -151,9 +150,6 @@ class ClientTest(unittest.TestCase):
             mock_parser = mock.MagicMock(cli.HelpfulArgumentParser)
             cli.HelpfulArgumentParser.handle_csr(mock_parser, mock_parsed_args)
 
-            # make sure cli processing occurred
-            cli_processed = (call[0][1] for call in mock_process_domain.call_args_list)
-            self.assertEqual(set(cli_processed), set(("example.com", "www.example.com")))
             # Now provoke an inconsistent domains error...
             mock_parsed_args.domains.append("hippopotamus.io")
             self.assertRaises(errors.ConfigurationError,