From a4885e491aba09c1e2adedda7750887f515eae78 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 10 Nov 2015 16:04:56 -0800 Subject: [PATCH] Finish user agent changes: - revert changes to acme/, instead living with its current baroque API - add an extremely mockable test case --- acme/acme/client.py | 5 ++--- letsencrypt/client.py | 7 +++--- letsencrypt/tests/cli_test.py | 38 +++++++++++++++++++++----------- letsencrypt/tests/client_test.py | 2 +- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index 082dcb5cd..0e9319f9c 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -37,7 +37,6 @@ class Client(object): # pylint: disable=too-many-instance-attributes :ivar key: `.JWK` (private) :ivar alg: `.JWASignature` :ivar bool verify_ssl: Verify SSL certificates? - :ivar str ua: User agent string to send to the server. :ivar .ClientNetwork net: Client network. Useful for testing. If not supplied, it will be initialized using `key`, `alg` and `verify_ssl`. @@ -46,7 +45,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes DER_CONTENT_TYPE = 'application/pkix-cert' def __init__(self, directory, key, alg=jose.RS256, verify_ssl=True, - net=None, ua="acme-python"): + net=None): """Initialize. :param directory: Directory Resource (`.messages.Directory`) or @@ -54,7 +53,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes """ self.key = key - self.net = ClientNetwork(key, alg, verify_ssl, user_agent) if net is None else net + self.net = ClientNetwork(key, alg, verify_ssl) if net is None else net if isinstance(directory, six.string_types): self.directory = messages.Directory.from_json( diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 0b4297791..d48230f04 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -31,9 +31,9 @@ logger = logging.getLogger(__name__) def _acme_from_config_key(config, key): # TODO: Allow for other alg types besides RS256 - return acme_client.Client(directory=config.server, key=key, - verify_ssl=(not config.no_verify_ssl), - ua=config.user_agent) + net = acme_client.ClientNetwork(key, verify_ssl=(not config.no_verify_ssl), + user_agent=config.user_agent) + return acme_client.Client(directory=config.server, key=key, net=net) def register(config, account_storage, tos_cb=None): @@ -99,6 +99,7 @@ def register(config, account_storage, tos_cb=None): acc = account.Account(regr, key) account.report_new_account(acc, config) account_storage.save(acc) + return acc, acme diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index da5286a10..b6c56558e 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -10,6 +10,7 @@ import unittest import mock from letsencrypt import account +from letsencrypt import cli from letsencrypt import configuration from letsencrypt import errors @@ -30,15 +31,15 @@ class CLITest(unittest.TestCase): self.config_dir = os.path.join(self.tmp_dir, 'config') self.work_dir = os.path.join(self.tmp_dir, 'work') self.logs_dir = os.path.join(self.tmp_dir, 'logs') + self.standard_args = ['--text', '--config-dir', self.config_dir, + '--work-dir', self.work_dir, '--logs-dir', self.logs_dir, + '--agree-dev-preview'] def tearDown(self): shutil.rmtree(self.tmp_dir) def _call(self, args): - from letsencrypt import cli - args = ['--text', '--config-dir', self.config_dir, - '--work-dir', self.work_dir, '--logs-dir', self.logs_dir, - '--agree-dev-preview'] + args + args = self.standard_args + args with mock.patch('letsencrypt.cli.sys.stdout') as stdout: with mock.patch('letsencrypt.cli.sys.stderr') as stderr: with mock.patch('letsencrypt.cli.client') as client: @@ -50,10 +51,7 @@ class CLITest(unittest.TestCase): Variant of _call that preserves stdout so that it can be mocked by the caller. """ - from letsencrypt import cli - args = ['--text', '--config-dir', self.config_dir, - '--work-dir', self.work_dir, '--logs-dir', self.logs_dir, - '--agree-dev-preview'] + args + args = self.standard_args + args with mock.patch('letsencrypt.cli.sys.stderr') as stderr: with mock.patch('letsencrypt.cli.client') as client: ret = cli.main(args) @@ -112,9 +110,27 @@ class CLITest(unittest.TestCase): self.assertTrue("--key-path" not in out) out = self._help_output(['-h']) - from letsencrypt import cli self.assertTrue(cli.usage_strings(plugins)[0] in out) + + @mock.patch('letsencrypt.cli.sys.stdout') + @mock.patch('letsencrypt.cli.sys.stderr') + @mock.patch('letsencrypt.cli.client.acme_client.ClientNetwork') + @mock.patch('letsencrypt.cli.client.acme_client.Client') + @mock.patch('letsencrypt.cli._determine_account') + @mock.patch('letsencrypt.cli.client.Client.obtain_and_enroll_certificate') + @mock.patch('letsencrypt.cli._auth_from_domains') + def test_user_agent(self, _afd, _obt, det, _client, acme_net, _out, _err): + ua = "bandersnatch" + # Normally the client is totally mocked out, but here we need more + # arguments to automate it... + args = ["--user-agent", ua, "--standalone", "certonly", + "-m", "none@none.com", "-d", "example.com", '--agree-tos'] + args += self.standard_args + det.return_value = mock.MagicMock(), None + cli.main(args) + acme_net.assert_called_once_with(mock.ANY, verify_ssl=True, user_agent=ua) + @mock.patch('letsencrypt.cli.display_ops') def test_installer_selection(self, mock_display_ops): self._call(['install', '--domain', 'foo.bar', '--cert-path', 'cert', @@ -281,8 +297,6 @@ class CLITest(unittest.TestCase): @mock.patch('letsencrypt.cli.sys') def test_handle_exception(self, mock_sys): # pylint: disable=protected-access - from letsencrypt import cli - mock_open = mock.mock_open() with mock.patch('letsencrypt.cli.open', mock_open, create=True): exception = Exception('detail') @@ -433,8 +447,6 @@ class MockedVerb(object): """ def __init__(self, verb_name): - from letsencrypt import cli - self.verb_dict = cli.HelpfulArgumentParser.VERBS self.verb_func = None self.verb_name = verb_name diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 3caea4f05..4a61194f3 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -71,7 +71,7 @@ class ClientTest(unittest.TestCase): def test_init_acme_verify_ssl(self): self.acme_client.assert_called_once_with( - directory=mock.ANY, key=mock.ANY, ua=mock.ANY, verify_ssl=True) + directory=mock.ANY, key=mock.ANY, verify_ssl=True) def _mock_obtain_certificate(self): self.client.auth_handler = mock.MagicMock()