From 73824c859ac07a22f5da6e642b9394f76012ebed Mon Sep 17 00:00:00 2001 From: Garrett Robinson Date: Thu, 9 Apr 2015 11:50:17 -0700 Subject: [PATCH 1/4] Ignore emacs autosave files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index ae5116fcb..6bf969454 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ m3 *~ .vagrant *.swp +\#*# \ No newline at end of file From 5d2abc30f0700366196cebb39a5a1a2275fb9d01 Mon Sep 17 00:00:00 2001 From: Garrett Robinson Date: Tue, 7 Apr 2015 17:09:34 -0700 Subject: [PATCH 2/4] Add cmd line arg for the authenticator --- letsencrypt/client/client.py | 46 +++++++++++++++++++++++++------- letsencrypt/client/interfaces.py | 6 +++++ letsencrypt/scripts/main.py | 15 ++++++++--- 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 2fcb45d40..19b982502 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -349,13 +349,29 @@ def init_csr(privkey, names, cert_dir): return le_util.CSR(csr_filename, csr_der, "der") +def list_available_authenticators(avail_auths): + """Return a pretty-printed list of authenticators. + + This is used to provide helpful feedback in the case where a user + specifies an invalid authenticator on the command line. + + """ + output_lines = ["Available authenticators:"] + for auth_name, auth in avail_auths.iteritems(): + output_lines.append(" - %s : %s" % (auth_name, auth.description)) + return '\n'.join(output_lines) + + # This should be controlled by commandline parameters -def determine_authenticator(all_auths): +def determine_authenticator(all_auths, config): """Returns a valid IAuthenticator. :param list all_auths: Where each is a :class:`letsencrypt.client.interfaces.IAuthenticator` object + :param config: Used if an authenticator was specified on the command line. + :type config: :class:`letsencrypt.client.interfaces.IConfig` + :returns: Valid Authenticator object or None :raises letsencrypt.client.errors.LetsEncryptClientError: If no @@ -363,23 +379,33 @@ def determine_authenticator(all_auths): """ # Available Authenticator objects - avail_auths = [] + avail_auths = {} # Error messages for misconfigured authenticators errs = {} - for pot_auth in all_auths: + for auth_name, auth in all_auths.iteritems(): try: - pot_auth.prepare() + auth.prepare() except errors.LetsEncryptMisconfigurationError as err: - errs[pot_auth] = err + errs[auth] = err except errors.LetsEncryptNoInstallationError: continue - avail_auths.append(pot_auth) + avail_auths[auth_name] = auth - if len(avail_auths) > 1: - auth = display_ops.choose_authenticator(avail_auths, errs) - elif len(avail_auths) == 1: - auth = avail_auths[0] + # If an authenticator was specified on the command line, try to use it + if config.authenticator: + try: + auth = avail_auths[config.authenticator] + except KeyError: + logging.error( + "The specified authenticator '%s' could not be found", + config.authenticator) + logging.info(list_available_authenticators(avail_auths)) + return + elif len(avail_auths) > 1: + auth = display_ops.choose_authenticator(avail_auths.values(), errs) + elif len(avail_auths.keys()) == 1: + auth = avail_auths[avail_auths.keys()[0]] else: raise errors.LetsEncryptClientError("No Authenticators available.") diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py index 6779d4e1e..0f032a92e 100644 --- a/letsencrypt/client/interfaces.py +++ b/letsencrypt/client/interfaces.py @@ -13,6 +13,10 @@ class IAuthenticator(zope.interface.Interface): """ + description = zope.interface.Attribute( + "Short description of this authenticator. " + "Used in interactive configuration.") + def prepare(): """Prepare the authenticator. @@ -89,6 +93,8 @@ class IConfig(zope.interface.Interface): server = zope.interface.Attribute( "CA hostname (and optionally :port). The server certificate must " "be trusted in order to avoid further modifications to the client.") + authenticator = zope.interface.Attribute( + "Authenticator to use for responding to challenges.") rsa_key_size = zope.interface.Attribute("Size of the RSA key.") config_dir = zope.interface.Attribute("Configuration directory.") diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 3b4b7c10d..269f66744 100644 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -32,6 +32,8 @@ SETUPTOOLS_AUTHENTICATORS_ENTRY_POINT = "letsencrypt.authenticators" def init_auths(config): """Find (setuptools entry points) and initialize Authenticators.""" + # TODO: handle collisions in authenticator names. Or is this + # already handled for us by pkg_resources? auths = {} for entrypoint in pkg_resources.iter_entry_points( SETUPTOOLS_AUTHENTICATORS_ENTRY_POINT): @@ -44,7 +46,7 @@ def init_auths(config): "%r object does not provide IAuthenticator, skipping", entrypoint.name) else: - auths[auth] = entrypoint.name + auths[entrypoint.name] = auth return auths @@ -60,6 +62,12 @@ def create_parser(): add("-s", "--server", default="letsencrypt-demo.org:443", help=config_help("server")) + # TODO: we should generate the list of choices from the set of + # available authenticators, but that is tricky due to the + # dependency between init_auths and config. Hardcoding it for now. + add("-a", "--authenticator", dest="authenticator", + help=config_help("authenticator")) + add("-k", "--authkey", type=read_file, help="Path to the authorized key file") add("-B", "--rsa-key-size", type=int, default=2048, metavar="N", @@ -159,9 +167,10 @@ def main(): # pylint: disable=too-many-branches, too-many-statements display_eula() all_auths = init_auths(config) - logging.debug('Initialized authenticators: %s', all_auths.values()) + logging.debug('Initialized authenticators: %s', all_auths.keys()) try: - auth = client.determine_authenticator(all_auths.keys()) + auth = client.determine_authenticator(all_auths, config) + logging.debug("Selected authenticator: %s", auth) except errors.LetsEncryptClientError: logging.critical("No authentication mechanisms were found on your " "system.") From 79f5ebe734d18ddbc70dfbd22de4ce76f995a20a Mon Sep 17 00:00:00 2001 From: Garrett Robinson Date: Thu, 9 Apr 2015 15:32:20 -0700 Subject: [PATCH 3/4] Update unit tests for determine_authenticator The last commit refactored determine_authenticator to take a mapping of authenticator names to authenticators instead of a list of authenticators. This commit updates the existing unit tests to work with this refactor. --- .gitignore | 2 +- letsencrypt/client/tests/client_test.py | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 6bf969454..e2ec0622c 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,4 @@ m3 *~ .vagrant *.swp -\#*# \ No newline at end of file +\#*# diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py index 1c1a0d68a..2310dbe87 100644 --- a/letsencrypt/client/tests/client_test.py +++ b/letsencrypt/client/tests/client_test.py @@ -1,4 +1,5 @@ """letsencrypt.client.client.py tests.""" +from collections import namedtuple import unittest import mock @@ -20,12 +21,18 @@ class DetermineAuthenticatorTest(unittest.TestCase): self.mock_config = mock.Mock() - self.all_auths = [self.mock_apache, self.mock_stand] + self.all_auths = { + 'apache': self.mock_apache, + 'standalone': self.mock_stand + } @classmethod def _call(cls, all_auths): from letsencrypt.client.client import determine_authenticator - return determine_authenticator(all_auths) + # TODO: add tests for setting the authenticator via the command line + mock_config = namedtuple("Config", ['authenticator']) + return determine_authenticator(all_auths, + mock_config(authenticator=None)) @mock.patch("letsencrypt.client.client.display_ops.choose_authenticator") def test_accept_two(self, mock_choose): @@ -35,7 +42,8 @@ class DetermineAuthenticatorTest(unittest.TestCase): def test_accept_one(self): self.mock_apache.prepare.return_value = self.mock_apache self.assertEqual( - self._call(self.all_auths[:1]), self.mock_apache) + self._call(dict(apache=self.all_auths['apache'])), + self.mock_apache) def test_no_installation_one(self): self.mock_apache.prepare.side_effect = ( From 0d7f32fa984e2e82918d644dfe6913bfe765055f Mon Sep 17 00:00:00 2001 From: Garrett Robinson Date: Thu, 9 Apr 2015 17:37:24 -0700 Subject: [PATCH 4/4] Unit tests for setting authenticator via cmd line --- letsencrypt/client/client.py | 7 ++-- letsencrypt/client/tests/client_test.py | 50 ++++++++++++++++++------- letsencrypt/scripts/main.py | 5 +-- 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 19b982502..91b271784 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -397,11 +397,10 @@ def determine_authenticator(all_auths, config): try: auth = avail_auths[config.authenticator] except KeyError: - logging.error( - "The specified authenticator '%s' could not be found", - config.authenticator) logging.info(list_available_authenticators(avail_auths)) - return + raise errors.LetsEncryptClientError( + "The specified authenticator '%s' could not be found" % + config.authenticator) elif len(avail_auths) > 1: auth = display_ops.choose_authenticator(avail_auths.values(), errs) elif len(avail_auths.keys()) == 1: diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py index 2310dbe87..63170b517 100644 --- a/letsencrypt/client/tests/client_test.py +++ b/letsencrypt/client/tests/client_test.py @@ -1,9 +1,9 @@ """letsencrypt.client.client.py tests.""" -from collections import namedtuple import unittest import mock +from letsencrypt.client import configuration from letsencrypt.client import errors @@ -19,7 +19,8 @@ class DetermineAuthenticatorTest(unittest.TestCase): self.mock_apache = mock.MagicMock( spec=ApacheConfigurator, description="Standalone Authenticator") - self.mock_config = mock.Mock() + self.mock_config = mock.MagicMock( + spec=configuration.NamespaceConfig, authenticator=None) self.all_auths = { 'apache': self.mock_apache, @@ -27,29 +28,30 @@ class DetermineAuthenticatorTest(unittest.TestCase): } @classmethod - def _call(cls, all_auths): + def _call(cls, all_auths, config): from letsencrypt.client.client import determine_authenticator - # TODO: add tests for setting the authenticator via the command line - mock_config = namedtuple("Config", ['authenticator']) - return determine_authenticator(all_auths, - mock_config(authenticator=None)) + return determine_authenticator(all_auths, config) @mock.patch("letsencrypt.client.client.display_ops.choose_authenticator") def test_accept_two(self, mock_choose): mock_choose.return_value = self.mock_stand() - self.assertEqual(self._call(self.all_auths), self.mock_stand()) + self.assertEqual(self._call(self.all_auths, self.mock_config), + self.mock_stand()) def test_accept_one(self): self.mock_apache.prepare.return_value = self.mock_apache - self.assertEqual( - self._call(dict(apache=self.all_auths['apache'])), - self.mock_apache) + one_avail_auth = { + 'apache': self.mock_apache + } + self.assertEqual(self._call(one_avail_auth, self.mock_config), + self.mock_apache) def test_no_installation_one(self): self.mock_apache.prepare.side_effect = ( errors.LetsEncryptNoInstallationError) - self.assertEqual(self._call(self.all_auths), self.mock_stand) + self.assertEqual(self._call(self.all_auths, self.mock_config), + self.mock_stand) def test_no_installations(self): self.mock_apache.prepare.side_effect = ( @@ -59,7 +61,8 @@ class DetermineAuthenticatorTest(unittest.TestCase): self.assertRaises(errors.LetsEncryptClientError, self._call, - self.all_auths) + self.all_auths, + self.mock_config) @mock.patch("letsencrypt.client.client.logging") @mock.patch("letsencrypt.client.client.display_ops.choose_authenticator") @@ -68,7 +71,26 @@ class DetermineAuthenticatorTest(unittest.TestCase): errors.LetsEncryptMisconfigurationError) mock_choose.return_value = self.mock_apache - self.assertTrue(self._call(self.all_auths) is None) + self.assertTrue(self._call(self.all_auths, self.mock_config) is None) + + def test_choose_valid_auth_from_cmd_line(self): + standalone_config = mock.MagicMock(spec=configuration.NamespaceConfig, + authenticator='standalone') + self.assertEqual(self._call(self.all_auths, standalone_config), + self.mock_stand) + + apache_config = mock.MagicMock(spec=configuration.NamespaceConfig, + authenticator='apache') + self.assertEqual(self._call(self.all_auths, apache_config), + self.mock_apache) + + def test_choose_invalid_auth_from_cmd_line(self): + invalid_config = mock.MagicMock(spec=configuration.NamespaceConfig, + authenticator='foobar') + self.assertRaises(errors.LetsEncryptClientError, + self._call, + self.all_auths, + invalid_config) class RollbackTest(unittest.TestCase): diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 269f66744..20813f11e 100644 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -171,9 +171,8 @@ def main(): # pylint: disable=too-many-branches, too-many-statements try: auth = client.determine_authenticator(all_auths, config) logging.debug("Selected authenticator: %s", auth) - except errors.LetsEncryptClientError: - logging.critical("No authentication mechanisms were found on your " - "system.") + except errors.LetsEncryptClientError as err: + logging.critical(str(err)) sys.exit(1) if auth is None: