diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index 76665c327..e1ffd9551 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -19,7 +19,6 @@ from letsencrypt.client import le_util from letsencrypt.client import network from letsencrypt.client import reverter from letsencrypt.client import revoker -from letsencrypt.client import standalone_authenticator from letsencrypt.client.apache import configurator from letsencrypt.client.display import ops @@ -346,22 +345,17 @@ def init_csr(privkey, names, cert_dir): return le_util.CSR(csr_filename, csr_der, "der") # This should be controlled by commandline parameters -def determine_authenticator(config): +def determine_authenticator(all_auths): """Returns a valid IAuthenticator. - :param config: Configuration. - :type config: :class:`letsencrypt.client.interfaces.IConfig` + :param list all_auths: Where each is a tuple of the form + ('description', 'IAuthenticator', *options..) where IAuthenticator is a + :class:`letsencrypt.client.interfaces.IAuthenticator` object or class + and options are the parameters used to initialize the authenticator. :returns: Valid Authenticator object or None """ - # list of (Description, Known Authenticator classes, init arguments) - all_auths = [ - ("Apache Web Server", configurator.ApacheConfigurator, config), - ("Standalone Authenticator", - standalone_authenticator.StandaloneAuthenticator), - ] - # Available Authenticator objects avail_auths = [] # Error messages for misconfigured authenticators @@ -370,14 +364,15 @@ def determine_authenticator(config): for pot_auth in all_auths: try: # I do not think this a great solution but haven't come up with - # anything better yet... + # anything better yet... other than constricting init functions for + # authenticators if len(pot_auth) == 2: # pylint: disable=no-value-for-parameter avail_auths.append((pot_auth[0], pot_auth[1]())) elif len(pot_auth) == 3: avail_auths.append((pot_auth[0], pot_auth[1](pot_auth[2]))) else: - errors.LetsEncryptClientError( + raise errors.LetsEncryptClientError( "IAuthenticator: Number of parameters not supported") except errors.LetsEncryptMisconfigurationError as err: errs[pot_auth[1]] = err @@ -388,7 +383,7 @@ def determine_authenticator(config): if len(avail_auths) > 1: auth = ops.choose_authenticator(avail_auths, errs) elif len(avail_auths) == 1: - auth = avail_auths[0] + auth = avail_auths[0][1] else: auth = None diff --git a/letsencrypt/client/display/ops.py b/letsencrypt/client/display/ops.py index 34a036c71..9cd8e16e5 100644 --- a/letsencrypt/client/display/ops.py +++ b/letsencrypt/client/display/ops.py @@ -23,7 +23,8 @@ def choose_authenticator(auths, errs): :rtype: :class:`letsencrypt.client.interfaces.IAuthenticator` """ - descs = [auth[0] for auth in auths] + descs = [auth[0] if auth[1] not in errs else "%s (Misconfigured)" % auth[0] + for auth in auths] iauths = [auth[1] for auth in auths] while True: diff --git a/letsencrypt/client/display/util.py b/letsencrypt/client/display/util.py index 04db3ebb2..ad324b1b8 100644 --- a/letsencrypt/client/display/util.py +++ b/letsencrypt/client/display/util.py @@ -175,6 +175,9 @@ class FileDisplay(object): # pylint: disable=unused-argument """Display a menu. + .. todo:: This doesn't enable the help label/button (I wasn't sold on + any interface I came up with for this). It would be a nice feature + :param str message: title of menu :param choices: Menu lines, len must be > 0 :type choices: list of tuples (tag, item) or diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py index e6ed243c4..1dbe930d8 100644 --- a/letsencrypt/client/interfaces.py +++ b/letsencrypt/client/interfaces.py @@ -57,7 +57,7 @@ class IAuthenticator(zope.interface.Interface): """ - def more_info(self): + def more_info(): """Human-readable string to help the user. Should describe the steps taken and any relevant info to help the user diff --git a/letsencrypt/client/standalone_authenticator.py b/letsencrypt/client/standalone_authenticator.py index 2c870068f..d7b78c9cf 100755 --- a/letsencrypt/client/standalone_authenticator.py +++ b/letsencrypt/client/standalone_authenticator.py @@ -414,4 +414,4 @@ class StandaloneAuthenticator(object): "is attained, it will be saved in the " "(TODO) current working directory.{0}{0}" "Port 443 must be open in order to use the " - "Standalone Authenticator.") + "Standalone Authenticator.".format(os.linesep)) diff --git a/letsencrypt/client/tests/client_test.py b/letsencrypt/client/tests/client_test.py index c490df770..41aaf38a4 100644 --- a/letsencrypt/client/tests/client_test.py +++ b/letsencrypt/client/tests/client_test.py @@ -6,10 +6,66 @@ import mock from letsencrypt.client import errors +class DetermineAuthenticatorTest(unittest.TestCase): + def setUp(self): + from letsencrypt.client.apache.configurator import ApacheConfigurator + from letsencrypt.client.standalone_authenticator \ + import StandaloneAuthenticator + + self.mock_stand = mock.MagicMock(spec=StandaloneAuthenticator) + self.mock_apache = mock.MagicMock(spec=ApacheConfigurator) + + self.mock_config = mock.Mock() + + self.all_auths = [ + ("Apache Web Server", self.mock_apache, self.mock_config), + ("Standalone", self.mock_stand), + ] + + @classmethod + def _call(cls, all_auths): + from letsencrypt.client.client import determine_authenticator + return determine_authenticator(all_auths) + + @mock.patch("letsencrypt.client.client.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()) + + def test_accept_one(self): + self.assertEqual( + self._call(self.all_auths[:1]), self.mock_apache(self.mock_config)) + + def test_no_installation_one(self): + self.mock_apache.side_effect = errors.LetsEncryptNoInstallationError + + self.assertEqual(self._call(self.all_auths), self.mock_stand()) + + def test_no_installations(self): + self.mock_apache.side_effect = errors.LetsEncryptNoInstallationError + self.mock_stand.side_effect = errors.LetsEncryptNoInstallationError + + self.assertTrue(self._call(self.all_auths) is None) + + @mock.patch("letsencrypt.client.client.logging") + @mock.patch("letsencrypt.client.client.ops.choose_authenticator") + def test_misconfigured(self, mock_choose, mock_log): + self.mock_apache.side_effect = errors.LetsEncryptMisconfigurationError + mock_choose.return_value = self.mock_apache + + self.assertRaises(SystemExit, self._call, self.all_auths) + + def test_too_many_params(self): + self.assertRaises( + errors.LetsEncryptClientError, + self._call, + [("desc", self.mock_apache, "1", "2", "3", "4", "5")]) + class RollbackTest(unittest.TestCase): """Test the rollback function.""" def setUp(self): - self.m_install = mock.MagicMock() + from letsencrypt.client.apache.configurator import ApacheConfigurator + self.m_install = mock.MagicMock(spec=ApacheConfigurator) @classmethod def _call(cls, checkpoints): diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 9ec42bd44..2e102ffa6 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -19,6 +19,8 @@ from letsencrypt.client import client from letsencrypt.client import interfaces from letsencrypt.client import le_util from letsencrypt.client import log +from letsencrypt.client import standalone_authenticator +from letsencrypt.client.apache import configurator from letsencrypt.client.display import util as display_util from letsencrypt.client.display import ops @@ -134,9 +136,13 @@ def main(): # pylint: disable=too-many-branches if not args.eula: display_eula() - # Make sure we actually get an installer that is functioning properly - # before we begin to try to use it. - auth = client.determine_authenticator(config) + # list of (Description, Known Authenticator classes, init arguments) + all_auths = [ + ("Apache Web Server", configurator.ApacheConfigurator, config), + ("Standalone Authenticator", + standalone_authenticator.StandaloneAuthenticator), + ] + auth = client.determine_authenticator(all_auths) if auth is None: logging.critical("Unable to find a way to authenticate the server.") sys.exit(4)