From b5acb8b71e3daf284451a705386aee4964651802 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 10:53:14 -0700 Subject: [PATCH 01/33] add add domain --- letsencrypt/cli.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 256e0c801..587a66faf 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -876,6 +876,22 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstrin args.webroot_path.append(webroot) +def add_domain(args_or_config, domain): + """Registers a new domain to be used during the current client run. + + If all domains in domain have been registered, this function has no + effect. + + :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 + + """ + args_or_config.domains.extend(le_util.enforce_domain_sanity(d.strip()) + for d in domain.split(",") if d not in args_or_config.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 From ca7049dabcdabb8182ebb6853a720ad62cafc764 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 13:21:57 -0700 Subject: [PATCH 02/33] add webroot_path parsing functions to webroot.py --- letsencrypt/plugins/webroot.py | 30 +++++++++++++++++++++++++++-- letsencrypt/plugins/webroot_test.py | 5 ----- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 6d2899511..2d87d3475 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -57,8 +57,6 @@ to serve all files under specified web root ({0}).""" "--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", @@ -157,3 +155,31 @@ to serve all files under specified web root ({0}).""" root_path) else: raise + + +def _match_webroot_with_domains(args_or_config): + """Applies the most recent webroot path to all unmatched domains. + + :param args_or_config: parsed command line arguments + :type args_or_config: argparse.Namespace or + configuration.NamespaceConfig + + """ + webroot_path = args_or_config.webroot_path[-1] + for domain in args_or_config.domains: + args_or_config.webroot_map.set_default(domain, 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) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index ed0326555..e80537260 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -55,11 +55,6 @@ class AuthenticatorTest(unittest.TestCase): self.auth.add_parser_arguments(add) self.assertEqual(0, add.call_count) # args moved to cli.py! - 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 = {} From d3fa0dd222fff0cde22b247afcf4bfa5b1dd3773 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 13:51:26 -0700 Subject: [PATCH 03/33] make add_domain more useful --- letsencrypt/cli.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 587a66faf..b9416f13f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -876,20 +876,29 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstrin args.webroot_path.append(webroot) -def add_domain(args_or_config, domain): - """Registers a new domain to be used during the current client run. +def add_domains(args_or_config, domains): + """Registers new domains to be used during the current client run. - If all domains in domain have been registered, this function has no - effect. + 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 + :returns: domains after they have been normalized and validated + :rtype: `list` of `str` + """ - args_or_config.domains.extend(le_util.enforce_domain_sanity(d.strip()) - for d in domain.split(",") if d not in args_or_config.domains) + 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) + + return validated_domains def process_domain(args_or_config, domain_arg, webroot_path=None): From c83c09e12bd0f73562e6709da2b21f26e88fbb7a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 14:07:42 -0700 Subject: [PATCH 04/33] Add _WebrootMapAction to webroot.py --- letsencrypt/plugins/webroot.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 2d87d3475..336a58f19 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -1,5 +1,7 @@ """Webroot plugin.""" +import argparse import errno +import json import logging import os from collections import defaultdict @@ -9,6 +11,7 @@ import six from acme import challenges +from letsencrypt import cli from letsencrypt import errors from letsencrypt import interfaces from letsencrypt.plugins import common @@ -157,6 +160,17 @@ to serve all files under specified web root ({0}).""" 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)): + validated_webroot_path = _validate_webroot(webroot_path) + namespace.webroot_map.update( + (d, validated_webroot_path,) + for d in cli.add_domains(namespace, domains)) + + def _match_webroot_with_domains(args_or_config): """Applies the most recent webroot path to all unmatched domains. From f663a6f9611d13d28028b8b6ae921e55dd979d35 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 14:24:52 -0700 Subject: [PATCH 05/33] add _WebrootPathAction to webroot.py --- letsencrypt/plugins/webroot.py | 40 ++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 336a58f19..9a16a4ba5 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -171,17 +171,26 @@ class _WebrootMapAction(argparse.Action): for d in cli.add_domains(namespace, domains)) -def _match_webroot_with_domains(args_or_config): - """Applies the most recent webroot path to all unmatched 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 - :param args_or_config: parsed command line arguments - :type args_or_config: argparse.Namespace or - configuration.NamespaceConfig + 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") - """ - webroot_path = args_or_config.webroot_path[-1] - for domain in args_or_config.domains: - args_or_config.webroot_map.set_default(domain, webroot_path) + if namespace.webroot_path: + # Apply previous webroot to all matched + # domains before setting the new webroot path + _match_webroot_with_domains(namespace) + elif namespace.domains: + self._domain_before_webroot = True + + namespace.webroot_path.append(_validate_webroot(webroot_path)) def _validate_webroot(webroot_path): @@ -197,3 +206,16 @@ def _validate_webroot(webroot_path): raise errors.PluginError(webroot_path + " does not exist or is not a directory") return os.path.abspath(webroot_path) + + +def _match_webroot_with_domains(args_or_config): + """Applies the most recent webroot path to all unmatched domains. + + :param args_or_config: parsed command line arguments + :type args_or_config: argparse.Namespace or + configuration.NamespaceConfig + + """ + webroot_path = args_or_config.webroot_path[-1] + for domain in args_or_config.domains: + args_or_config.webroot_map.set_default(domain, webroot_path) From b1bdc4590de09db15473501a4b6e8f6ffae18f90 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 16:16:17 -0700 Subject: [PATCH 06/33] Move webroot processing to webroot.py --- letsencrypt/cli.py | 101 +++------------------------- letsencrypt/main.py | 10 +-- letsencrypt/plugins/webroot.py | 25 +++++-- letsencrypt/plugins/webroot_test.py | 2 +- letsencrypt/renewal.py | 5 +- letsencrypt/tests/cli_test.py | 54 --------------- letsencrypt/tests/client_test.py | 6 +- 7 files changed, 38 insertions(+), 165 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index b9416f13f..5e192424a 100644 --- a/letsencrypt/cli.py +++ b/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 @@ -270,12 +269,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 [] @@ -311,7 +304,7 @@ class HelpfulArgumentParser(object): 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 + webroot plugin can know about the calls to add_domains. """ if parsed_args.verb != "certonly": raise errors.Error("Currently, a CSR file may only be specified " @@ -333,14 +326,9 @@ 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)) + add_domains(parsed_args, d) if not domains: # TODO: add CN to domains instead: @@ -572,7 +560,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.") @@ -829,51 +817,13 @@ 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.""" -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 __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) + def __call__(self, parser, namespace, domain, option_string=None): + """Just wrap add_domains in argparseese.""" + add_domains(namespace, domain) def add_domains(args_or_config, domains): @@ -899,38 +849,3 @@ def add_domains(args_or_config, domains): args_or_config.domains.append(domain) return validated_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 - - """ - 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) - 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) diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 0afccc85e..ad4e5d2cf 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -288,14 +288,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 " diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 9a16a4ba5..7c9aef509 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -38,9 +38,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 @@ -52,8 +64,10 @@ to serve all files under specified web root ({0}).""" self.performed = defaultdict(set) def prepare(self): # pylint: disable=missing-docstring - path_map = self.conf("map") + if self.conf("path"): + _match_webroot_with_domains(self.config) + path_map = self.conf("map") if not path_map: raise errors.PluginError( "Missing parts of webroot configuration; please set either " @@ -173,6 +187,7 @@ class _WebrootMapAction(argparse.Action): 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 @@ -218,4 +233,4 @@ def _match_webroot_with_domains(args_or_config): """ webroot_path = args_or_config.webroot_path[-1] for domain in args_or_config.domains: - args_or_config.webroot_map.set_default(domain, webroot_path) + args_or_config.webroot_map.setdefault(domain, webroot_path) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index e80537260..23aabd82d 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -53,7 +53,7 @@ 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_missing_root(self): self.config.webroot_path = None diff --git a/letsencrypt/renewal.py b/letsencrypt/renewal.py index 27546bec9..2fa921906 100644 --- a/letsencrypt/renewal.py +++ b/letsencrypt/renewal.py @@ -12,6 +12,7 @@ import zope.component from letsencrypt import configuration from letsencrypt import cli from letsencrypt import errors +from letsencrypt import le_util from letsencrypt import storage from letsencrypt.plugins import disco as plugins_disco @@ -78,8 +79,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 " diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 04b5a2f3c..71c6356bb 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -459,60 +459,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) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index ed4e5def0..00e03e7e4 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -135,8 +135,8 @@ 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): + @mock.patch("letsencrypt.cli.add_domains") + def test_obtain_certificate_from_csr(self, mock_add_domains, mock_logger): self._mock_obtain_certificate() from letsencrypt import cli test_csr = le_util.CSR(form="der", file=None, data=CSR_SAN) @@ -151,7 +151,7 @@ class ClientTest(unittest.TestCase): 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) + cli_processed = (call[0][1] for call in mock_add_domains.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") From 1f4daf0874a038ee3bd4c832309492171c1566c1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 16:42:45 -0700 Subject: [PATCH 07/33] factor out _create_challenge_dirs --- letsencrypt/plugins/webroot.py | 9 +++++---- letsencrypt/plugins/webroot_test.py | 29 ++++++++--------------------- letsencrypt/tests/cli_test.py | 7 ------- 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 7c9aef509..4b13b05bc 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -67,6 +67,11 @@ to serve all files under specified web root ({0}).""" if self.conf("path"): _match_webroot_with_domains(self.config) + def perform(self, achalls): # pylint: disable=missing-docstring + self._create_challenge_dirs() + return [self._perform_single(achall) for achall in achalls] + + def _create_challenge_dirs(self): path_map = self.conf("map") if not path_map: raise errors.PluginError( @@ -111,10 +116,6 @@ 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] diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 23aabd82d..0c7a2a3b0 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -55,14 +55,13 @@ class AuthenticatorTest(unittest.TestCase): self.auth.add_parser_arguments(add) self.assertEqual(2, add.call_count) - def test_prepare_missing_root(self): + def test_prepare(self): + self.auth.prepare() # shouldn't raise any exceptions + + def test_perform_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() - self.auth.prepare() # shouldn't raise any exceptions + self.assertRaises(errors.PluginError, self.auth.perform, []) def test_prepare_reraises_other_errors(self): self.auth.full_path = os.path.join(self.path, "null") @@ -75,7 +74,7 @@ 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") @@ -86,9 +85,9 @@ class AuthenticatorTest(unittest.TestCase): @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 @@ -111,18 +110,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]) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 71c6356bb..6182d99cc 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -234,13 +234,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: From 7070b996995b8b26469a8a68a2bc0f9a7f0beca0 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 16:45:59 -0700 Subject: [PATCH 08/33] make prepare a noop --- letsencrypt/plugins/webroot.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 4b13b05bc..5f3c04448 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -64,8 +64,7 @@ to serve all files under specified web root ({0}).""" self.performed = defaultdict(set) def prepare(self): # pylint: disable=missing-docstring - if self.conf("path"): - _match_webroot_with_domains(self.config) + pass def perform(self, achalls): # pylint: disable=missing-docstring self._create_challenge_dirs() From 82efffdf62e3834f0e5f24846f2c839582906524 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 16:52:48 -0700 Subject: [PATCH 09/33] inline _match_webroot_with_domains --- letsencrypt/plugins/webroot.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 5f3c04448..57bccb13c 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -201,7 +201,9 @@ class _WebrootPathAction(argparse.Action): if namespace.webroot_path: # Apply previous webroot to all matched # domains before setting the new webroot path - _match_webroot_with_domains(namespace) + 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 @@ -221,16 +223,3 @@ def _validate_webroot(webroot_path): raise errors.PluginError(webroot_path + " does not exist or is not a directory") return os.path.abspath(webroot_path) - - -def _match_webroot_with_domains(args_or_config): - """Applies the most recent webroot path to all unmatched domains. - - :param args_or_config: parsed command line arguments - :type args_or_config: argparse.Namespace or - configuration.NamespaceConfig - - """ - webroot_path = args_or_config.webroot_path[-1] - for domain in args_or_config.domains: - args_or_config.webroot_map.setdefault(domain, webroot_path) From 1acd50a0ceb93032170973c8a52cca2e1881adf2 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 17:07:54 -0700 Subject: [PATCH 10/33] Remove the need for extra processing to support --csr + --webroot --- letsencrypt/cli.py | 14 ++++---------- letsencrypt/plugins/webroot.py | 6 ++++++ letsencrypt/tests/client_test.py | 6 +----- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5e192424a..edb97ae1c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -302,10 +302,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 add_domains. - """ + """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 " @@ -327,9 +324,6 @@ class HelpfulArgumentParser(object): 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: - add_domains(parsed_args, d) - if not domains: # TODO: add CN to domains instead: raise errors.Error( @@ -337,11 +331,11 @@ class HelpfulArgumentParser(object): % parsed_args.csr[0]) parsed_args.actual_csr = (csr, typ) - csr_domains, config_domains = set(domains), set(parsed_args.domains) - if csr_domains != config_domains: + # If CSR domains are not a superset of the domains provided by the CLI + if set(parsed_args.domains) - set(domains): raise errors.ConfigurationError( "Inconsistent domain requests:\nFrom the CSR: {0}\nFrom command line/config: {1}" - .format(", ".join(csr_domains), ", ".join(config_domains))) + .format(", ".join(domains), ", ".join(parsed_args.domains))) def determine_verb(self): diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 57bccb13c..b6483b228 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -67,7 +67,13 @@ to serve all files under specified web root ({0}).""" pass def perform(self, achalls): # pylint: disable=missing-docstring + if self.conf("path"): + webroot_path = self.conf("path")[-1] + for achall in achalls: + self.conf("map").setdefault(achall.domain, webroot_path) + self._create_challenge_dirs() + return [self._perform_single(achall) for achall in achalls] def _create_challenge_dirs(self): diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 00e03e7e4..8943ffa00 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -135,8 +135,7 @@ class ClientTest(unittest.TestCase): # FIXME move parts of this to test_cli.py... @mock.patch("letsencrypt.client.logger") - @mock.patch("letsencrypt.cli.add_domains") - def test_obtain_certificate_from_csr(self, mock_add_domains, 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) @@ -150,9 +149,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_add_domains.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, From 547147b8ac0b709f11f53c558a323e91a145beef Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 17:57:46 -0700 Subject: [PATCH 11/33] Start of IDisplay code for webroot --- letsencrypt/plugins/webroot.py | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index b6483b228..428469383 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -1,13 +1,15 @@ """Webroot plugin.""" import argparse +import collections import errno +import itertools 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 @@ -61,21 +63,39 @@ 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 pass def perform(self, achalls): # pylint: disable=missing-docstring - if self.conf("path"): - webroot_path = self.conf("path")[-1] - for achall in achalls: - self.conf("map").setdefault(achall.domain, webroot_path) + self._get_webroots(achalls) self._create_challenge_dirs() return [self._perform_single(achall) for achall in achalls] + def _get_webroots(self, achalls): + if self.conf("path"): + webroot_path = self.conf("path")[-1] + for achall in achalls: + self.conf("map").setdefault(achall.domain, webroot_path) + else: + # An OrderedDict is used because it maintains + # insertion order and fast element lookup + known_webroots = collections.OrderedDict( + (path, None) for path in six.itervalues(self.conf("map"))) + for achall in achalls: + if achall.domain not in self.conf("map"): + self._prompt_for_webroot(achall.domain, known_webroots) + + def _prompt_for_webroot(self, domain, known_webroots): + display = zope.component.getUtility(interfaces.IDisplay) + display.menu( + "Select the webroot for {0}:".format(domain), + itertools.chain(("Enter a new webroot",), known_webroots), + help_label="Help") + def _create_challenge_dirs(self): path_map = self.conf("map") if not path_map: From f1f0d1de12184de179a475bee476b2095cd9420a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 18:08:27 -0700 Subject: [PATCH 12/33] premature optimization is the root of all evil --- letsencrypt/plugins/webroot.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 428469383..4e83d04ea 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -2,7 +2,6 @@ import argparse import collections import errno -import itertools import json import logging import os @@ -81,10 +80,7 @@ to serve all files under specified web root ({0}).""" for achall in achalls: self.conf("map").setdefault(achall.domain, webroot_path) else: - # An OrderedDict is used because it maintains - # insertion order and fast element lookup - known_webroots = collections.OrderedDict( - (path, None) for path in six.itervalues(self.conf("map"))) + known_webroots = list(six.itervalues(self.conf("map"))) for achall in achalls: if achall.domain not in self.conf("map"): self._prompt_for_webroot(achall.domain, known_webroots) @@ -93,7 +89,7 @@ to serve all files under specified web root ({0}).""" display = zope.component.getUtility(interfaces.IDisplay) display.menu( "Select the webroot for {0}:".format(domain), - itertools.chain(("Enter a new webroot",), known_webroots), + ["Enter a new webroot"] + known_webroots, help_label="Help") def _create_challenge_dirs(self): From ba62ed45c0b0fc5a0b4f5d42f2b4703d45c95ab1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 18:39:13 -0700 Subject: [PATCH 13/33] basic interactive webroot? --- letsencrypt/plugins/webroot.py | 37 +++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 4e83d04ea..ac4c3c1ec 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -15,6 +15,7 @@ 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 @@ -34,6 +35,9 @@ necessary validation resources to appropriate paths on the file system. It expects that there is some other HTTP server configured to serve all files under specified web root ({0}).""" + _INTERACTIVE_CANCEL = ("Every requested domain must have a " + "webroot when using the webroot plugin.") + def more_info(self): # pylint: disable=missing-docstring,no-self-use return self.MORE_INFO.format(self.conf("path")) @@ -80,17 +84,40 @@ to serve all files under specified web root ({0}).""" for achall in achalls: self.conf("map").setdefault(achall.domain, webroot_path) else: - known_webroots = list(six.itervalues(self.conf("map"))) + known_webroots = list(set(six.itervalues(self.conf("map")))) for achall in achalls: if achall.domain not in self.conf("map"): - self._prompt_for_webroot(achall.domain, known_webroots) + new_webroot = self._prompt_for_webroot(achall.domain, + known_webroots) + try: + known_webroots.remove(new_webroot) + except ValueError: + pass + known_webroots.append(new_webroot) + self.conf("map")[achall.domain] = new_webroot def _prompt_for_webroot(self, domain, known_webroots): display = zope.component.getUtility(interfaces.IDisplay) - display.menu( + code, index = display.menu( "Select the webroot for {0}:".format(domain), - ["Enter a new webroot"] + known_webroots, - help_label="Help") + ["Enter a new webroot"] + known_webroots[::-1]) + if code == display_util.CANCEL: + raise errors.PluginError(self._INTERACTIVE_CANCEL) + elif index != 0: + return known_webroots[index - 1] + + while True: + code, webroot = display.directory_select( + "Input the webroot for {0}:".format(domain)) + if code == display_util.CANCEL: + raise errors.PluginError(self._INTERACTIVE_CANCEL) + elif code == display_util.HELP: + display.notification(display_util.DSELECT_HELP) + else: + try: + return _validate_webroot(webroot) + except errors.PluginError: + pass def _create_challenge_dirs(self): path_map = self.conf("map") From 2edc288c80a75a13d187cf58d1297965f30cc548 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 1 Apr 2016 18:59:48 -0700 Subject: [PATCH 14/33] fix --csr --- letsencrypt/cli.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index edb97ae1c..0e5c15624 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -324,6 +324,11 @@ class HelpfulArgumentParser(object): logger.debug("PEM CSR parse error %s", traceback.format_exc()) raise errors.Error("Failed to parse CSR file: {0}".format(parsed_args.csr[0])) + # 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: raise errors.Error( From 119c3f125a0431ad70260f887c930ec30ab9d85a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 09:07:26 -0700 Subject: [PATCH 15/33] Revert csr_domain logic --- letsencrypt/cli.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 377ee1135..ab328747f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -380,11 +380,11 @@ class HelpfulArgumentParser(object): % parsed_args.csr[0]) parsed_args.actual_csr = (csr, typ) - # If CSR domains are not a superset of the domains provided by the CLI - if set(parsed_args.domains) - set(domains): + csr_domains, config_domains = set(domains), set(parsed_args.domains) + if csr_domains != config_domains: raise errors.ConfigurationError( "Inconsistent domain requests:\nFrom the CSR: {0}\nFrom command line/config: {1}" - .format(", ".join(domains), ", ".join(parsed_args.domains))) + .format(", ".join(csr_domains), ", ".join(config_domains))) def determine_verb(self): From 22c924bb1c261f0308815f6d3d22cf60385558ff Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 09:45:48 -0700 Subject: [PATCH 16/33] more interactive webroot polish --- letsencrypt/plugins/webroot.py | 46 ++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index ac4c3c1ec..2052d4edb 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -38,6 +38,16 @@ to serve all files under specified web root ({0}).""" _INTERACTIVE_CANCEL = ("Every requested domain must have a " "webroot when using the webroot plugin.") + _INPUT_HELP_FMT = ( + "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.\n\nTo continue, you need to " + "provide the webroot directory for {0}.") + def more_info(self): # pylint: disable=missing-docstring,no-self-use return self.MORE_INFO.format(self.conf("path")) @@ -72,13 +82,13 @@ to serve all files under specified web root ({0}).""" pass def perform(self, achalls): # pylint: disable=missing-docstring - self._get_webroots(achalls) + self._set_webroots(achalls) self._create_challenge_dirs() return [self._perform_single(achall) for achall in achalls] - def _get_webroots(self, achalls): + def _set_webroots(self, achalls): if self.conf("path"): webroot_path = self.conf("path")[-1] for achall in achalls: @@ -89,22 +99,32 @@ to serve all files under specified web root ({0}).""" 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.append(new_webroot) + known_webroots.insert(0, new_webroot) self.conf("map")[achall.domain] = new_webroot def _prompt_for_webroot(self, domain, known_webroots): display = zope.component.getUtility(interfaces.IDisplay) - code, index = display.menu( - "Select the webroot for {0}:".format(domain), - ["Enter a new webroot"] + known_webroots[::-1]) - if code == display_util.CANCEL: - raise errors.PluginError(self._INTERACTIVE_CANCEL) - elif index != 0: - return known_webroots[index - 1] + + while True: + code, index = display.menu( + "Select the webroot for {0}:".format(domain), + ["Enter a new webroot"] + known_webroots, + help_label="Help") + if code == display_util.CANCEL: + raise errors.PluginError(self._INTERACTIVE_CANCEL) + elif code == display_util.HELP: + display.notification(self._INPUT_HELP_FMT.format(domain)) + else: # code == display_util.OK + if index == 0: + break + else: + return known_webroots[index - 1] while True: code, webroot = display.directory_select( @@ -112,7 +132,11 @@ to serve all files under specified web root ({0}).""" if code == display_util.CANCEL: raise errors.PluginError(self._INTERACTIVE_CANCEL) elif code == display_util.HELP: - display.notification(display_util.DSELECT_HELP) + # Help can currently only be selected + # when using the ncurses interface + display.notification(''.join( + (self._INPUT_HELP_FMT.format(domain), + "\n\n", display_util.DSELECT_HELP,))) else: try: return _validate_webroot(webroot) From bf0c2306c625ea56531e351c781b9f26250618f1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 09:49:56 -0700 Subject: [PATCH 17/33] Try and make ncurses less ugly --- letsencrypt/plugins/webroot.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 2052d4edb..b4de56f5b 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -38,15 +38,14 @@ to serve all files under specified web root ({0}).""" _INTERACTIVE_CANCEL = ("Every requested domain must have a " "webroot when using the webroot plugin.") - _INPUT_HELP_FMT = ( + _INPUT_HELP = ( "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.\n\nTo continue, you need to " - "provide the webroot directory for {0}.") + "domain validation challenges.") def more_info(self): # pylint: disable=missing-docstring,no-self-use return self.MORE_INFO.format(self.conf("path")) @@ -119,7 +118,7 @@ to serve all files under specified web root ({0}).""" if code == display_util.CANCEL: raise errors.PluginError(self._INTERACTIVE_CANCEL) elif code == display_util.HELP: - display.notification(self._INPUT_HELP_FMT.format(domain)) + display.notification(self._INPUT_HELP) else: # code == display_util.OK if index == 0: break @@ -134,9 +133,7 @@ to serve all files under specified web root ({0}).""" elif code == display_util.HELP: # Help can currently only be selected # when using the ncurses interface - display.notification(''.join( - (self._INPUT_HELP_FMT.format(domain), - "\n\n", display_util.DSELECT_HELP,))) + display.notification(display_util.DSELECT_HELP) else: try: return _validate_webroot(webroot) From 987aa8237188c201fbd9b7aae49094a788b92201 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 10:32:01 -0700 Subject: [PATCH 18/33] more UI polish --- letsencrypt/plugins/webroot.py | 57 ++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index b4de56f5b..9bd9dc90d 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -35,18 +35,6 @@ necessary validation resources to appropriate paths on the file system. It expects that there is some other HTTP server configured to serve all files under specified web root ({0}).""" - _INTERACTIVE_CANCEL = ("Every requested domain must have a " - "webroot when using the webroot plugin.") - - _INPUT_HELP = ( - "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.") - def more_info(self): # pylint: disable=missing-docstring,no-self-use return self.MORE_INFO.format(self.conf("path")) @@ -108,6 +96,17 @@ to serve all files under specified web root ({0}).""" 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: @@ -116,29 +115,39 @@ to serve all files under specified web root ({0}).""" ["Enter a new webroot"] + known_webroots, help_label="Help") if code == display_util.CANCEL: - raise errors.PluginError(self._INTERACTIVE_CANCEL) + raise errors.PluginError( + "Every requested domain must have a " + "webroot when using the webroot plugin.") elif code == display_util.HELP: - display.notification(self._INPUT_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 - if index == 0: - break - else: - return known_webroots[index - 1] + 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.CANCEL: - raise errors.PluginError(self._INTERACTIVE_CANCEL) - elif code == display_util.HELP: + if code == display_util.HELP: # Help can currently only be selected # when using the ncurses interface display.notification(display_util.DSELECT_HELP) - else: + elif code == display_util.CANCEL: + return None + else: # code == display_util.OK try: return _validate_webroot(webroot) - except errors.PluginError: - pass + except errors.PluginError as error: + display.notification(str(error)) def _create_challenge_dirs(self): path_map = self.conf("map") From 17c495732d5bcc313f805562ee616b5cd4811172 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 10:38:34 -0700 Subject: [PATCH 19/33] Don't pause when showing errors --- letsencrypt/plugins/webroot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 9bd9dc90d..a425f0d20 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -147,7 +147,7 @@ to serve all files under specified web root ({0}).""" try: return _validate_webroot(webroot) except errors.PluginError as error: - display.notification(str(error)) + display.notification(str(error), pause=False) def _create_challenge_dirs(self): path_map = self.conf("map") From 7458324932e0776631f0870fba50ef13a5c01199 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 10:57:02 -0700 Subject: [PATCH 20/33] logging++ --- letsencrypt/plugins/webroot.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index a425f0d20..677130f12 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -78,6 +78,8 @@ to serve all files under specified web root ({0}).""" 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: From 625e9660fede8dad9f635351ed04a6a1fe2d38a5 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 11:23:53 -0700 Subject: [PATCH 21/33] test cancel/help of _prompt_with_webroot_list --- letsencrypt/plugins/webroot_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 0c7a2a3b0..bfcfe4f9c 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -10,12 +10,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 @@ -58,6 +60,22 @@ class AuthenticatorTest(unittest.TestCase): def test_prepare(self): self.auth.prepare() # shouldn't raise any exceptions + @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) + 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))) + def test_perform_missing_root(self): self.config.webroot_path = None self.config.webroot_map = {} From 713fb3433edc2f6d4b3f2e1c31003600edfc1769 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 11:29:51 -0700 Subject: [PATCH 22/33] give _prompt_with_webroot_list full test coverage --- letsencrypt/plugins/webroot_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index bfcfe4f9c..64c098995 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -60,6 +60,23 @@ class AuthenticatorTest(unittest.TestCase): def test_prepare(self): self.auth.prepare() # shouldn't raise any exceptions + @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 = [] @@ -70,6 +87,7 @@ class AuthenticatorTest(unittest.TestCase): (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( From e01cb704a33b2e8c118994f63db80fdcf7b677f2 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 12:38:03 -0700 Subject: [PATCH 23/33] test _prompt_for_new_webroot --- letsencrypt/plugins/webroot_test.py | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 64c098995..2c9de900f 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -83,8 +83,8 @@ class AuthenticatorTest(unittest.TestCase): 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)] + 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) @@ -94,6 +94,29 @@ class AuthenticatorTest(unittest.TestCase): 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 = {} From c7ef17df920eebc8c3ca757077bb988cdbc96040 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 13:05:46 -0700 Subject: [PATCH 24/33] fix references to old prepare/perform --- letsencrypt/plugins/webroot_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 2c9de900f..fe57a2b5c 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -122,7 +122,7 @@ class AuthenticatorTest(unittest.TestCase): self.config.webroot_map = {} self.assertRaises(errors.PluginError, self.auth.perform, []) - def test_prepare_reraises_other_errors(self): + 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: @@ -139,7 +139,7 @@ class AuthenticatorTest(unittest.TestCase): @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): From 641f0c7422e0d12807d5db8e616e5870609019d7 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 13:06:00 -0700 Subject: [PATCH 25/33] simplify error handling --- letsencrypt/plugins/webroot.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 677130f12..24d8d3c1c 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -196,24 +196,13 @@ to serve all files under specified web root ({0}).""" finally: os.umask(old_umask) - 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) @@ -232,7 +221,7 @@ 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) + root_path = self.full_roots[achall.domain] validation_path = self._get_validation_path(root_path, achall) logger.debug("Removing %s", validation_path) os.remove(validation_path) From 73e2fafba4f364068365f8b0a83f712f6d24a08a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 13:26:13 -0700 Subject: [PATCH 26/33] Add webroot map tests --- letsencrypt/plugins/webroot_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index fe57a2b5c..7dbe16d08 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -2,6 +2,7 @@ from __future__ import print_function +import argparse import errno import os import shutil @@ -44,6 +45,10 @@ class AuthenticatorTest(unittest.TestCase): webroot_map={"thing.com": self.path}) self.auth = Authenticator(self.config, "webroot") + self.parser = argparse.ArgumentParser() + self.parser.add_argument("--domains", default=[]) + self.auth.inject_parser_options(self.parser, self.auth.name) + def tearDown(self): shutil.rmtree(self.path) @@ -224,5 +229,10 @@ class AuthenticatorTest(unittest.TestCase): self.assertFalse(os.path.exists(self.validation_path)) self.assertTrue(os.path.exists(self.root_challenge_path)) + def test_webroot_map_action(self): + args = self.parser.parse_args( + ["--webroot-map", '{{"thing.com":"{0}"}}'.format(self.path)]) + self.assertEqual(args.webroot_map, self.config.webroot_map) + if __name__ == "__main__": unittest.main() # pragma: no cover From ed0c3810316847bef3488f1d2101039b93d6ae0a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 13:35:26 -0700 Subject: [PATCH 27/33] Add domain before webroot test --- letsencrypt/plugins/webroot_test.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 7dbe16d08..b92c0759f 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -46,7 +46,8 @@ class AuthenticatorTest(unittest.TestCase): self.auth = Authenticator(self.config, "webroot") self.parser = argparse.ArgumentParser() - self.parser.add_argument("--domains", default=[]) + self.parser.add_argument("-d", "--domains", + action="append", default=[]) self.auth.inject_parser_options(self.parser, self.auth.name) def tearDown(self): @@ -234,5 +235,14 @@ class AuthenticatorTest(unittest.TestCase): ["--webroot-map", '{{"thing.com":"{0}"}}'.format(self.path)]) self.assertEqual(args.webroot_map, self.config.webroot_map) + def test_domain_before_webroot(self): + args = self.parser.parse_args( + "-d {0} -w {1}".format(self.achall.domain, self.path).split()) + self.auth.config = args + self.auth.perform([self.achall]) + self.assertEqual(self.auth.config.webroot_map, + self.config.webroot_map) + + if __name__ == "__main__": unittest.main() # pragma: no cover From c716003b1fd69ec94cdec018a63e60c613ce5f8f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 13:38:53 -0700 Subject: [PATCH 28/33] add test for domain before multiple webroots --- letsencrypt/plugins/webroot_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index b92c0759f..155f1cf51 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -243,6 +243,12 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(self.auth.config.webroot_map, self.config.webroot_map) + 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()) + if __name__ == "__main__": unittest.main() # pragma: no cover From 68e17604cdf8239b7709934ac6c08d3b3805ee40 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 13:56:32 -0700 Subject: [PATCH 29/33] make separate action testing class --- letsencrypt/plugins/webroot_test.py | 33 ++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 155f1cf51..2609d6ed9 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -45,11 +45,6 @@ class AuthenticatorTest(unittest.TestCase): webroot_map={"thing.com": self.path}) self.auth = Authenticator(self.config, "webroot") - self.parser = argparse.ArgumentParser() - self.parser.add_argument("-d", "--domains", - action="append", default=[]) - self.auth.inject_parser_options(self.parser, self.auth.name) - def tearDown(self): shutil.rmtree(self.path) @@ -230,18 +225,31 @@ 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, self.config.webroot_map) + 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()) - self.auth.config = args - self.auth.perform([self.achall]) - self.assertEqual(self.auth.config.webroot_map, - self.config.webroot_map) + 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, @@ -249,6 +257,11 @@ class AuthenticatorTest(unittest.TestCase): self.assertRaises(errors.PluginError, self.parser.parse_args, "-d foo -w bar -d baz -w qux".split()) + 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 From aab1a080ce4b319c14e7226ff73bf3223be798c4 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 13:58:55 -0700 Subject: [PATCH 30/33] test multiwebroot --- letsencrypt/plugins/webroot_test.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 2609d6ed9..f7ed7fdbf 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -257,11 +257,20 @@ class WebrootActionTest(unittest.TestCase): 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 From 4505b68a9a91152c1c9c54c614465f7e7f091f08 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 14:05:46 -0700 Subject: [PATCH 31/33] put generator expression on one line --- letsencrypt/plugins/webroot.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 24d8d3c1c..621df3909 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -249,10 +249,9 @@ class _WebrootMapAction(argparse.Action): def __call__(self, parser, namespace, webroot_map, option_string=None): for domains, webroot_path in six.iteritems(json.loads(webroot_map)): - validated_webroot_path = _validate_webroot(webroot_path) + webroot_path = _validate_webroot(webroot_path) namespace.webroot_map.update( - (d, validated_webroot_path,) - for d in cli.add_domains(namespace, domains)) + (d, webroot_path) for d in cli.add_domains(namespace, domains)) class _WebrootPathAction(argparse.Action): From 558806e2b710495c8de0108b6a2b9c40effcbcb2 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 14:26:07 -0700 Subject: [PATCH 32/33] add cli_flag for noninteractive --- letsencrypt/plugins/webroot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 621df3909..4331031da 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -115,7 +115,7 @@ to serve all files under specified web root ({0}).""" code, index = display.menu( "Select the webroot for {0}:".format(domain), ["Enter a new webroot"] + known_webroots, - help_label="Help") + help_label="Help", cli_flag="--" + self.option_name("path")) if code == display_util.CANCEL: raise errors.PluginError( "Every requested domain must have a " From 237adfdce2711c3eee6609b129b166babb3fb029 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 4 Apr 2016 16:19:41 -0700 Subject: [PATCH 33/33] I was told to cleanup after myself --- letsencrypt/plugins/webroot.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 4331031da..4e3b8099c 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -221,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.full_roots[achall.domain] - 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: