From 0371838a91feda70dcbb26701ea33f14ebc5bb26 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Mon, 27 Apr 2015 12:42:50 -0700 Subject: [PATCH] more unit tests/better flow --- letsencrypt/acme/messages2_test.py | 7 +++ letsencrypt/client/account.py | 23 ++++---- letsencrypt/client/configuration.py | 8 ++- letsencrypt/client/constants.py | 6 ++ letsencrypt/client/crypto_util.py | 12 +++- letsencrypt/client/network2.py | 8 +++ .../client/tests/configuration_test.py | 11 +++- letsencrypt/client/tests/crypto_util_test.py | 55 +++++++++++++++++++ letsencrypt/scripts/main.py | 9 ++- 9 files changed, 121 insertions(+), 18 deletions(-) diff --git a/letsencrypt/acme/messages2_test.py b/letsencrypt/acme/messages2_test.py index b9695ecd6..e1d5efe47 100644 --- a/letsencrypt/acme/messages2_test.py +++ b/letsencrypt/acme/messages2_test.py @@ -58,6 +58,7 @@ class ConstantTest(unittest.TestCase): self.MockConstant = MockConstant # pylint: disable=invalid-name self.const_a = MockConstant('a') self.const_b = MockConstant('b') + self.const_a_prime = MockConstant('a') def test_to_partial_json(self): self.assertEqual('a', self.const_a.to_partial_json()) @@ -75,6 +76,12 @@ class ConstantTest(unittest.TestCase): self.assertEqual('MockConstant(a)', repr(self.const_a)) self.assertEqual('MockConstant(b)', repr(self.const_b)) + def test_equality(self): + self.assertFalse(self.const_a == self.const_b) + self.assertTrue(self.const_a == self.const_a_prime) + + self.assertTrue(self.const_a != self.const_b) + self.assertFalse(self.const_a != self.const_a_prime) class RegistrationTest(unittest.TestCase): """Tests for letsencrypt.acme.messages2.Registration.""" diff --git a/letsencrypt/client/account.py b/letsencrypt/client/account.py index 52a5867a1..ed37b6446 100644 --- a/letsencrypt/client/account.py +++ b/letsencrypt/client/account.py @@ -180,18 +180,21 @@ class Account(object): :rtype: :class:`letsencrypt.client.account.Account` """ - code, email = zope.component.getUtility(interfaces.IDisplay).input( - "Enter email address (optional)") - if code == display_util.OK: - email = email if email != "" else None + while True: + code, email = zope.component.getUtility(interfaces.IDisplay).input( + "Enter email address (optional, press Enter to skip)") - le_util.make_or_verify_dir( - config.account_keys_dir, 0o700, os.geteuid()) - key = crypto_util.init_save_key( - config.rsa_key_size, config.account_keys_dir, email) - return cls(config, key, email) + if code == display_util.OK: + if email == "" or cls.safe_email(email): + email = email if email != "" else None - return None + le_util.make_or_verify_dir( + config.account_keys_dir, 0o700, os.geteuid()) + key = crypto_util.init_save_key( + config.rsa_key_size, config.account_keys_dir, email) + return cls(config, key, email) + else: + return None @classmethod def safe_email(cls, email): diff --git a/letsencrypt/client/configuration.py b/letsencrypt/client/configuration.py index 7c5aecbcc..5474a497d 100644 --- a/letsencrypt/client/configuration.py +++ b/letsencrypt/client/configuration.py @@ -51,12 +51,14 @@ class NamespaceConfig(object): @property def accounts_dir(self): #pylint: disable=missing-docstring return os.path.join( - self.namespace.config_dir, "accounts", self.namespace.server) + self.namespace.config_dir, constants.ACCOUNTS_DIR, + self.namespace.server.partition(":")[0]) @property def account_keys_dir(self): #pylint: disable=missing-docstring - return os.path.join(self.namespace.config_dir, "accounts", - self.namespace.server, "keys") + return os.path.join( + self.namespace.config_dir, constants.ACCOUNTS_DIR, + self.namespace.server.partition(":")[0], constants.ACCOUNT_KEYS_DIR) # TODO: This should probably include the server name @property diff --git a/letsencrypt/client/constants.py b/letsencrypt/client/constants.py index 02fab62cb..20f735779 100644 --- a/letsencrypt/client/constants.py +++ b/letsencrypt/client/constants.py @@ -61,6 +61,12 @@ CERT_KEY_BACKUP_DIR = "keys-certs" """Directory where all certificates and keys are stored (relative to IConfig.work_dir. Used for easy revocation.""" +ACCOUNTS_DIR = "accounts" +"""Directory where all accounts are saved.""" + +ACCOUNT_KEYS_DIR = "keys" +"""Directory where account keys are saved. Relative to ACCOUNTS_DIR.""" + REC_TOKEN_DIR = "recovery_tokens" """Directory where all recovery tokens are saved (relative to IConfig.work_dir).""" diff --git a/letsencrypt/client/crypto_util.py b/letsencrypt/client/crypto_util.py index e4b4311b5..c2b761d59 100644 --- a/letsencrypt/client/crypto_util.py +++ b/letsencrypt/client/crypto_util.py @@ -30,6 +30,9 @@ def init_save_key(key_size, key_dir, keyname="key-letsencrypt.pem"): :param str key_dir: Key save directory. :param str keyname: Filename of key + :returns: Key + :rtype: :class:`letsencrypt.client.le_util.Key` + :raises ValueError: If unable to generate the key given key_size. """ @@ -40,7 +43,7 @@ def init_save_key(key_size, key_dir, keyname="key-letsencrypt.pem"): raise err # Save file - le_util.make_or_verify_dir(key_dir, 0o700) + le_util.make_or_verify_dir(key_dir, 0o700, os.geteuid()) key_f, key_path = le_util.unique_file( os.path.join(key_dir, keyname), 0o600) key_f.write(key_pem) @@ -51,7 +54,7 @@ def init_save_key(key_size, key_dir, keyname="key-letsencrypt.pem"): return le_util.Key(key_path, key_pem) -def init_save_csr(privkey, names, cert_dir): +def init_save_csr(privkey, names, cert_dir, csrname="csr-letsencrypt.pem"): """Initialize a CSR with the given private key. :param privkey: Key to include in the CSR @@ -61,13 +64,16 @@ def init_save_csr(privkey, names, cert_dir): :param str cert_dir: Certificate save directory. + :returns: CSR + :rtype: :class:`letsencrypt.client.le_util.CSR` + """ csr_pem, csr_der = make_csr(privkey.pem, names) # Save CSR le_util.make_or_verify_dir(cert_dir, 0o755) csr_f, csr_filename = le_util.unique_file( - os.path.join(cert_dir, "csr-letsencrypt.pem"), 0o644) + os.path.join(cert_dir, csrname), 0o644) csr_f.write(csr_pem) csr_f.close() diff --git a/letsencrypt/client/network2.py b/letsencrypt/client/network2.py index 557446e5c..1793cdb1c 100644 --- a/letsencrypt/client/network2.py +++ b/letsencrypt/client/network2.py @@ -89,6 +89,8 @@ class Network(object): try: # TODO: This is insufficient or doesn't work as intended. + logging.error("Error: %s", jobj) + logging.error("Response from server: %s", response.content) raise messages2.Error.from_json(jobj) except jose.DeserializationError as error: # Couldn't deserialize JSON object @@ -536,9 +538,15 @@ class Network(object): def revoke(self, certr, when=messages2.Revocation.NOW): """Revoke certificate. + :param certr: Certificate Resource + :type certr: `.CertificateResource` + :param when: When should the revocation take place? Takes the same values as `.messages2.Revocation.revoke`. + :raises letsencrypt.client.errors.NetworkError: If revocation is + unsuccessful. + """ rev = messages2.Revocation(revoke=when, authorizations=tuple( authzr.uri for authzr in certr.authzrs)) diff --git a/letsencrypt/client/tests/configuration_test.py b/letsencrypt/client/tests/configuration_test.py index dde1f44cb..9385dbde3 100644 --- a/letsencrypt/client/tests/configuration_test.py +++ b/letsencrypt/client/tests/configuration_test.py @@ -10,7 +10,8 @@ class NamespaceConfigTest(unittest.TestCase): def setUp(self): from letsencrypt.client.configuration import NamespaceConfig namespace = mock.MagicMock( - work_dir='/tmp/foo', foo='bar', server='acme-server.org:443') + config_dir='/tmp/config', work_dir='/tmp/foo', foo='bar', + server='acme-server.org:443') self.config = NamespaceConfig(namespace) def test_proxy_getattr(self): @@ -23,11 +24,19 @@ class NamespaceConfigTest(unittest.TestCase): constants.IN_PROGRESS_DIR = '../p' constants.CERT_KEY_BACKUP_DIR = 'c/' constants.REC_TOKEN_DIR = '/r' + constants.ACCOUNTS_DIR = 'acc' + constants.ACCOUNT_KEYS_DIR = 'keys' + self.assertEqual(self.config.temp_checkpoint_dir, '/tmp/foo/t') self.assertEqual(self.config.in_progress_dir, '/tmp/foo/../p') self.assertEqual( self.config.cert_key_backup, '/tmp/foo/c/acme-server.org') self.assertEqual(self.config.rec_token_dir, '/r') + self.assertEqual( + self.config.accounts_dir, '/tmp/config/acc/acme-server.org') + self.assertEqual( + self.config.account_keys_dir, + '/tmp/config/acc/acme-server.org/keys') if __name__ == '__main__': diff --git a/letsencrypt/client/tests/crypto_util_test.py b/letsencrypt/client/tests/crypto_util_test.py index 9752c3d04..38fb7ef2d 100644 --- a/letsencrypt/client/tests/crypto_util_test.py +++ b/letsencrypt/client/tests/crypto_util_test.py @@ -1,15 +1,70 @@ """Tests for letsencrypt.client.crypto_util.""" +import logging import os import pkg_resources +import shutil +import tempfile import unittest import M2Crypto +import mock RSA256_KEY = pkg_resources.resource_string(__name__, 'testdata/rsa256_key.pem') RSA512_KEY = pkg_resources.resource_string(__name__, 'testdata/rsa512_key.pem') +class InitSaveKeyTest(unittest.TestCase): + """Tests for letsencrypt.client.crypto_util.init_save_key.""" + def setUp(self): + logging.disable(logging.CRITICAL) + self.key_dir = tempfile.mkdtemp('key_dir') + + def tearDown(self): + logging.disable(logging.NOTSET) + shutil.rmtree(self.key_dir) + + @classmethod + def _call(cls, key_size, key_dir): + from letsencrypt.client.crypto_util import init_save_key + return init_save_key(key_size, key_dir, 'key-letsencrypt.pem') + + @mock.patch('letsencrypt.client.crypto_util.make_key') + def test_success(self, mock_make): + mock_make.return_value = 'key_pem' + key = self._call(1024, self.key_dir) + self.assertEqual(key.pem, 'key_pem') + self.assertTrue('key-letsencrypt.pem' in key.file) + + @mock.patch('letsencrypt.client.crypto_util.make_key') + def test_key_failure(self, mock_make): + mock_make.side_effect = ValueError + self.assertRaises(ValueError, self._call, 431, self.key_dir) + + +class InitSaveCSRTest(unittest.TestCase): + """Tests for letsencrypt.client.crypto_util.init_save_csr.""" + + def setUp(self): + self.csr_dir = tempfile.mkdtemp('csr_dir') + + def tearDown(self): + shutil.rmtree(self.csr_dir) + + @mock.patch('letsencrypt.client.crypto_util.make_csr') + @mock.patch('letsencrypt.client.crypto_util.le_util.make_or_verify_dir') + def test_it(self, unused_mock_verify, mock_csr): + from letsencrypt.client.crypto_util import init_save_csr + + mock_csr.return_value = ('csr_pem', 'csr_der') + + csr = init_save_csr( + mock.Mock(pem='dummy_key'), 'example.com', self.csr_dir, + 'csr-letsencrypt.pem') + + self.assertEqual(csr.data, 'csr_der') + self.assertTrue('csr-letsencrypt.pem' in csr.file) + class ValidCSRTest(unittest.TestCase): """Tests for letsencrypt.client.crypto_util.valid_csr.""" diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index c9520e3cf..88461e7d1 100644 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -162,7 +162,11 @@ def main(): # pylint: disable=too-many-branches, too-many-statements sys.exit() if args.revoke or args.rev_cert is not None or args.rev_key is not None: - client.revoke(config, args.no_confirm, args.rev_cert, args.rev_key) + # This depends on the renewal config and cannot be completed yet. + zope.component.getUtility(interfaces.IDisplay).notification( + "Revocation is not available with the new Boulder server yet.") + + # client.revoke(config, args.no_confirm, args.rev_cert, args.rev_key) sys.exit() if args.rollback > 0: @@ -207,6 +211,9 @@ def main(): # pylint: disable=too-many-branches, too-many-statements # le_util.Key(args.authkey[0], args.authkey[1]) account = client.determine_account(config) + if account is None: + sys.exit(0) + acme = client.Client(config, account, auth, installer) # Validate the key and csr