diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 477cb653f..208d629b5 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -29,7 +29,6 @@ from letsencrypt import configuration from letsencrypt import constants from letsencrypt import client from letsencrypt import crypto_util -from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt import log @@ -38,6 +37,7 @@ from letsencrypt import storage from letsencrypt.display import util as display_util from letsencrypt.display import ops as display_ops +from letsencrypt.errors import Error, PluginSelectionError, CertStorageError from letsencrypt.plugins import disco as plugins_disco @@ -50,7 +50,7 @@ logger = logging.getLogger(__name__) # This is the stub to include in help generated by argparse SHORT_USAGE = """ - letsencrypt [SUBCOMMAND] [options] [domains] + letsencrypt [SUBCOMMAND] [options] [-d domain] [-d domain] ... The Let's Encrypt agent can obtain and install HTTPS/TLS/SSL certificates. By default, it will attempt to use a webserver both for obtaining and installing @@ -92,7 +92,7 @@ def _find_domains(args, installer): domains = args.domains if not domains: - raise errors.Error("Please specify --domains, or --installer that " + raise Error("Please specify --domains, or --installer that " "will help in domain names autodiscovery") return domains @@ -145,9 +145,9 @@ def _determine_account(args, config): try: acc, acme = client.register( config, account_storage, tos_cb=_tos_cb) - except errors.Error as error: + except Error as error: logger.debug(error, exc_info=True) - raise errors.Error( + raise Error( "Unable to register an account with ACME server") args.account = acc.id @@ -185,7 +185,7 @@ def _find_duplicative_certs(domains, config, renew_config): rc_config.filename = full_path candidate_lineage = storage.RenewableCert( rc_config, config_opts=None, cli_config=cli_config) - except (configobj.ConfigObjError, errors.CertStorageError, IOError): + except (configobj.ConfigObjError, CertStorageError, IOError): logger.warning("Renewal configuration file %s is broken. " "Skipping.", full_path) continue @@ -257,7 +257,7 @@ def _treat_as_renewal(config, domains): br=os.linesep ), reporter_util.HIGH_PRIORITY) - raise errors.Error( + raise Error( "User did not use proper CLI and would like " "to reinvoke the client.") @@ -298,7 +298,7 @@ def _auth_from_domains(le_client, config, domains, plugins): # TREAT AS NEW REQUEST lineage = le_client.obtain_and_enroll_certificate(domains, plugins) if not lineage: - raise errors.Error("Certificate could not be obtained") + raise Error("Certificate could not be obtained") _report_new_cert(lineage.cert) reporter_util = zope.component.getUtility(interfaces.IReporter) @@ -310,31 +310,109 @@ def _auth_from_domains(le_client, config, domains, plugins): return lineage +def set_configurator(previously, now): + """ + Setting configurators multiple ways is okay, as long as they all agree + :param string previously: previously identified request for the installer/authenticator + :param string requested: the request currently being processed + """ + if now is None: + # we're not actually setting anything + return previously + if previously: + if previously != now: + msg = "Too many flags setting configurators/installers/authenticators %s -> %s" + raise PluginSelectionError, msg % (`previously`, `now`) + return now + +def diagnose_configurator_problem(cfg_type, requested, plugins): + """ + Raise the most helpful error message about a plugin being unavailable + + :param string cfg_type: either "installer" or "authenticator" + :param string requested: the plugin that was requested + :param PluginRegistry plugins: available plugins + + :raises error.PluginSelectionError: if there was a problem + """ + + if requested: + if requested not in plugins: + msg = "The requested {0} plugin does not appear to be installed".format(requested) + else: + msg = ("The {0} plugin is not working; there may be problems with " + "your existing configuration").format(requested) + elif cfg_type == "installer": + if os.path.exists("/etc/debian_version"): + # Debian... installers are at least possible + msg = ('No installers seem to be present and working on your system; ' + 'fix that or try running letsencrypt with the "auth" command') + else: + # XXX update this logic as we make progress on #788 and nginx support + msg = ('No installers are available on your OS yet; try running ' + '"letsencrypt-auto auth" to get a cert you can install manually') + else: + msg = "{0} could not be determined or is not installed".format(cfg_type) + raise PluginSelectionError, msg + + +def choose_configurator_plugins(args, config, plugins, verb): + """ + Figure out which configurator we're going to use + + :raises error.PluginSelectionError if there was a problem + """ + + # Which plugins do we need? + need_inst = need_auth = (verb == "run") + if verb == "auth": + need_auth = True + if verb == "install": + need_inst = True + if args.authenticator: + logger.warn("Specifying an authenticator doesn't make sense in install mode") + + # Which plugins did the user request? + req_inst = req_auth = args.configurator + req_inst = set_configurator(req_inst, args.installer) + req_auth = set_configurator(req_auth, args.authenticator) + if args.nginx: + req_inst = set_configurator(req_inst, "nginx") + req_auth = set_configurator(req_auth, "nginx") + if args.apache: + req_inst = set_configurator(req_inst, "apache") + req_auth = set_configurator(req_auth, "apache") + logger.debug("Requested authenticator %s and installer %s", req_auth, req_inst) + + # Try to meet the user's request and/or ask them to pick plugins + authenticator = installer = None + if verb == "run" and req_auth == req_inst: + # Unless the user has explicitly asked for different auth/install, + # only consider offering a single choice + authenticator = installer = display_ops.pick_configurator(config, req_inst, plugins) + else: + if need_inst or req_inst: + installer = display_ops.pick_installer(config, req_inst, plugins) + if need_auth: + authenticator = display_ops.pick_authenticator(config, req_auth, plugins) + logger.debug("Selected authenticator %s and installer %s", authenticator, installer) + + if need_inst and not installer: + diagnose_configurator_problem("installer", req_inst, plugins) + if need_auth and not authenticator: + diagnose_configurator_problem("authenticator", req_auth, plugins) + + return installer, authenticator + # TODO: Make run as close to auth + install as possible # Possible difficulties: args.csr was hacked into auth def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-locals """Obtain a certificate and install.""" - # Begin authenticator and installer setup - if args.configurator is not None and (args.installer is not None or - args.authenticator is not None): - return ("Either --configurator or --authenticator/--installer" - "pair, but not both, is allowed") - - if args.authenticator is not None or args.installer is not None: - installer = display_ops.pick_installer( - config, args.installer, plugins) - authenticator = display_ops.pick_authenticator( - config, args.authenticator, plugins) - else: - # TODO: this assumes that user doesn't want to pick authenticator - # and installer separately... - authenticator = installer = display_ops.pick_configurator( - config, args.configurator, plugins) - - if installer is None or authenticator is None: - return "Configurator could not be determined" - # End authenticator and installer setup + try: + installer, authenticator = choose_configurator_plugins(args, config, plugins, "run") + except PluginSelectionError, e: + return e.message domains = _find_domains(args, installer) @@ -362,15 +440,11 @@ def auth(args, config, plugins): # supplied, check if CSR matches given domains? return "--domains and --csr are mutually exclusive" - authenticator = display_ops.pick_authenticator( - config, args.authenticator, plugins) - if authenticator is None: - return "Authenticator could not be determined" - - if args.installer is not None: - installer = display_ops.pick_installer(config, args.installer, plugins) - else: - installer = None + try: + # installers are used in auth mode to determine domain names + installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth") + except PluginSelectionError, e: + return e.message # TODO: Handle errors from _init_le_client? le_client = _init_le_client(args, config, authenticator, installer) @@ -390,9 +464,12 @@ def auth(args, config, plugins): def install(args, config, plugins): """Install a previously obtained cert in a server.""" # XXX: Update for renewer/RenewableCert - installer = display_ops.pick_installer(config, args.installer, plugins) - if installer is None: - return "Installer could not be determined" + + try: + installer, _ = choose_configurator_plugins(args, config, plugins, "auth") + except PluginSelectionError, e: + return e.message + domains = _find_domains(args, installer) le_client = _init_le_client( args, config, authenticator=None, installer=installer) @@ -663,6 +740,10 @@ def create_parser(plugins, args): None, "-t", "--text", dest="text_mode", action="store_true", help="Use the text output instead of the curses UI.") helpful.add(None, "-m", "--email", help=config_help("email")) + helpful.add(None, "--apache", action="store_true", + help="Obtain and install certs using Apache") + helpful.add(None, "--nginx", action="store_true", + help="Obtain and install certs using Nginx") # positional arg shadows --domains, instead of appending, and # --domains is useful, because it can be stored in config #for subparser in parser_run, parser_auth, parser_install: @@ -835,7 +916,7 @@ def _plugins_parsing(helpful, plugins): helpful.add( "plugins", "-a", "--authenticator", help="Authenticator plugin name.") helpful.add( - "plugins", "-i", "--installer", help="Installer plugin name.") + "plugins", "-i", "--installer", help="Installer plugin name (also used to find domains).") helpful.add( "plugins", "--configurator", help="Name of the plugin that is " "both an authenticator and an installer. Should not be used " @@ -922,7 +1003,7 @@ def _handle_exception(exc_type, exc_value, trace, args): sys.exit("".join( traceback.format_exception(exc_type, exc_value, trace))) - if issubclass(exc_type, errors.Error): + if issubclass(exc_type, Error): sys.exit(exc_value) else: # Tell the user a bit about what happened, without overwhelming @@ -986,7 +1067,7 @@ def main(cli_args=sys.argv[1:]): eula = pkg_resources.resource_string("letsencrypt", "EULA") if not zope.component.getUtility(interfaces.IDisplay).yesno( eula, "Agree", "Cancel"): - raise errors.Error("Must agree to TOS") + raise Error("Must agree to TOS") if not os.geteuid() == 0: logger.warning( @@ -1003,4 +1084,7 @@ def main(cli_args=sys.argv[1:]): if __name__ == "__main__": - sys.exit(main()) # pragma: no cover + err_string = main() + if err_string: + logger.warn("Exiting with message %s", err_string) + sys.exit(err_string) # pragma: no cover diff --git a/letsencrypt/errors.py b/letsencrypt/errors.py index 98c24bf50..406b9c12c 100644 --- a/letsencrypt/errors.py +++ b/letsencrypt/errors.py @@ -24,7 +24,6 @@ class SubprocessError(Error): class CertStorageError(Error): """Generic `.CertStorage` error.""" - # Auth Handler Errors class AuthorizationError(Error): """Authorization error.""" @@ -65,6 +64,8 @@ class DvsniError(DvAuthError): class PluginError(Error): """Let's Encrypt Plugin error.""" +class PluginSelectionError(Error): + """A problem with plugin/configurator selection or setup""" class NoInstallationError(PluginError): """Let's Encrypt No Installation error.""" diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index a3efd9d40..447c19d7a 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -13,6 +13,8 @@ from letsencrypt import account from letsencrypt import configuration from letsencrypt import errors +from letsencrypt.plugins import disco + from letsencrypt.tests import renewer_test from letsencrypt.tests import test_util @@ -75,7 +77,6 @@ class CLITest(unittest.TestCase): output.truncate(0) self.assertRaises(SystemExit, self._call_stdout, ['-h', 'nginx']) out = output.getvalue() - from letsencrypt.plugins import disco if "nginx" in disco.PluginsRegistry.find_all(): # may be false while building distributions without plugins self.assertTrue("--nginx-ctl" in out) @@ -93,6 +94,27 @@ class CLITest(unittest.TestCase): from letsencrypt import cli self.assertTrue(cli.USAGE in out) + def test_configurator_selection(self): + real_plugins = disco.PluginsRegistry.find_all() + args = ['--agree-eula', '--apache', '--authenticator', 'standalone'] + + # This needed two calls to find_all(), which we're avoiding for now + # because of possible side effects: + # https://github.com/letsencrypt/letsencrypt/commit/51ed2b681f87b1eb29088dd48718a54f401e4855 + #with mock.patch('letsencrypt.cli.plugins_testable') as plugins: + # plugins.return_value = {"apache": True, "nginx": True} + # ret, _, _, _ = self._call(args) + # self.assertTrue("Too many flags setting" in ret) + + args = ["install", "--nginx", "--cert-path", "/tmp/blah", "--key-path", "/tmp/blah", + "--nginx-server-root", "/nonexistent/thing", "-d", + "example.com", "--debug"] + if "nginx" in real_plugins: + # Sending nginx a non-existent conf dir will simulate misconfiguration + # (we can only do that if letsencrypt-nginx is actually present) + ret, _, _, _ = self._call(args) + self.assertTrue("The nginx plugin is not working" in ret) + def test_rollback(self): _, _, _, client = self._call(['rollback']) self.assertEqual(1, client.rollback.call_count) @@ -117,7 +139,7 @@ class CLITest(unittest.TestCase): self.assertEqual(ret, '--domains and --csr are mutually exclusive') ret, _, _, _ = self._call(['-a', 'bad_auth', 'auth']) - self.assertEqual(ret, 'Authenticator could not be determined') + self.assertEqual(ret, 'The requested bad_auth plugin does not appear to be installed') @mock.patch('letsencrypt.cli.zope.component.getUtility') def test_auth_new_request_success(self, mock_get_utility):