From 12899d0c38090c705be488f50bb2a346cd15e94b Mon Sep 17 00:00:00 2001 From: James Kasten Date: Wed, 22 Apr 2015 23:17:53 -0700 Subject: [PATCH] unittest and lint cleanup --- letsencrypt/client/account.py | 3 +- letsencrypt/client/auth_handler.py | 14 +++++-- letsencrypt/client/client.py | 26 +++++++------ letsencrypt/client/network2.py | 12 +++++- .../client/plugins/apache/configurator.py | 3 +- .../plugins/apache/tests/configurator_test.py | 18 ++++++--- .../client/plugins/apache/tests/dvsni_test.py | 28 ++++++++------ .../standalone/tests/authenticator_test.py | 23 ++++++++---- letsencrypt/client/tests/account_test.py | 8 +--- letsencrypt/client/tests/achallenges_test.py | 24 ++---------- letsencrypt/client/tests/acme_util.py | 10 +++-- letsencrypt/client/tests/auth_handler_test.py | 37 ++++++++++++++----- letsencrypt/client/tests/client_test.py | 9 +++-- .../client/tests/continuity_auth_test.py | 14 +++---- letsencrypt/client/tests/display/util_test.py | 6 +-- letsencrypt/client/tests/network2_test.py | 9 +---- .../client/tests/recovery_token_test.py | 15 +++++--- letsencrypt/scripts/main.py | 2 - 18 files changed, 146 insertions(+), 115 deletions(-) diff --git a/letsencrypt/client/account.py b/letsencrypt/client/account.py index ff4956364..52a5867a1 100644 --- a/letsencrypt/client/account.py +++ b/letsencrypt/client/account.py @@ -1,3 +1,4 @@ +"""Creates ACME accounts for server.""" import logging import os import re @@ -197,5 +198,5 @@ class Account(object): """Scrub email address before using it.""" if re.match(cls.EMAIL_REGEX, email): return bool(not email.startswith(".") and ".." not in email) - logging.warn("Invalid email address: using default address.") + logging.warn("Invalid email address.") return False diff --git a/letsencrypt/client/auth_handler.py b/letsencrypt/client/auth_handler.py index 882303b6d..f27e69081 100644 --- a/letsencrypt/client/auth_handler.py +++ b/letsencrypt/client/auth_handler.py @@ -10,8 +10,8 @@ from letsencrypt.client import achallenges from letsencrypt.client import constants from letsencrypt.client import errors - -class AuthHandler(object): # pylint: disable=too-many-instance-attributes +# pylint: disable=too-many-instance-attributes, too-few-public-methods +class AuthHandler(object): """ACME Authorization Handler for a client. :ivar dv_auth: Authenticator capable of solving @@ -108,7 +108,7 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes if self.dv_c: dv_resp = self.dv_auth.perform(self.dv_c) # This will catch both specific types of errors. - except errors.AuthorizationError as err: + except errors.AuthorizationError: logging.critical("Failure in setting up challenges.") logging.info("Attempting to clean up outstanding challenges...") self._cleanup_challenges() @@ -211,12 +211,18 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes return completed, failed - def _get_chall_status(self, authzr, achall): + def _get_chall_status(self, authzr, achall): # pylint: disable=no-self-use """Get the status of the challenge. .. warning:: This assumes only one instance of type of challenge in each challenge resource. + :param authzr: Authorization Resource + :type authzr: :class:`letsencrypt.acme.messages2.AuthorizationResource` + + :param achall: Annotated challenge for which to get status + :type achall: :class:`letsencrypt.client.achallenges.AnnotatedChallenge` + """ for authzr_challb in authzr.body.challenges: if type(authzr_challb.chall) is type(achall.challb.chall): diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index a6ce76432..96fcc46b1 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -46,7 +46,7 @@ class Client(object): """ - def __init__(self, config, account, dv_auth, installer): + def __init__(self, config, account_, dv_auth, installer): """Initialize a client. :param dv_auth: IAuthenticator that can solve the @@ -56,14 +56,14 @@ class Client(object): :type dv_auth: :class:`letsencrypt.client.interfaces.IAuthenticator` """ - self.account = account + self.account = account_ self.installer = installer # TODO: Allow for other alg types besides RS256 self.network = network2.Network( "https://%s/acme/new-reg" % config.server, - jwk.JWKRSA.load(account.key.pem)) + jwk.JWKRSA.load(self.account.key.pem)) self.config = config @@ -74,7 +74,7 @@ class Client(object): else: self.auth_handler = None - def register(self, save=True): + def register(self): """New Registration with the ACME server.""" self.account = self.network.register_from_account(self.account) if self.account.terms_of_service: @@ -167,16 +167,18 @@ class Client(object): if certr.cert_chain_uri: # TODO: Except chain_cert = self.network.fetch_chain(certr.cert_chain_uri) - chain_file, act_chain_path = le_util.unique_file(chain_path, 0o644) - try: - chain_file.write(chain_cert.to_pem()) - finally: - chain_file.close() + if chain_cert: + chain_file, act_chain_path = le_util.unique_file( + chain_path, 0o644) + try: + chain_file.write(chain_cert.to_pem()) + finally: + chain_file.close() - logging.info("Cert chain written to %s", act_chain_path) + logging.info("Cert chain written to %s", act_chain_path) - # This expects a valid chain file - cert_chain_abspath = os.path.abspath(act_chain_path) + # This expects a valid chain file + cert_chain_abspath = os.path.abspath(act_chain_path) return os.path.abspath(act_cert_path), cert_chain_abspath diff --git a/letsencrypt/client/network2.py b/letsencrypt/client/network2.py index e740b4240..e26ee741e 100644 --- a/letsencrypt/client/network2.py +++ b/letsencrypt/client/network2.py @@ -470,6 +470,15 @@ class Network(object): return self.request_issuance(csr, updated_authzrs), updated_authzrs def _get_cert(self, uri): + """Returns certificate from URI. + + :param str uri: URI of certificate + + :returns: tuple of the form + (response, :class:`letsencrypt.acme.jose.ComparableX509`) + :rtype: tuple + + """ content_type = self.DER_CONTENT_TYPE # TODO: make it a param response = self._get(uri, headers={'Accept': content_type}, content_type=content_type) @@ -521,7 +530,8 @@ class Network(object): """ if certr.cert_chain_uri is not None: - return self._get_cert(certr.cert_chain_uri) + _, cert = self._get_cert(certr.cert_chain_uri) + return cert def revoke(self, certr, when=messages2.Revocation.NOW): """Revoke certificate. diff --git a/letsencrypt/client/plugins/apache/configurator.py b/letsencrypt/client/plugins/apache/configurator.py index f3b952915..e826c011a 100644 --- a/letsencrypt/client/plugins/apache/configurator.py +++ b/letsencrypt/client/plugins/apache/configurator.py @@ -1008,7 +1008,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): sni_response = apache_dvsni.perform() if sni_response: # Must restart in order to activate the challenges. - # Handled here because we may be able to load up other challenge types + # Handled here because we may be able to load up other challenge + # types self.restart() # Go through all of the challenges and assign them to the proper diff --git a/letsencrypt/client/plugins/apache/tests/configurator_test.py b/letsencrypt/client/plugins/apache/tests/configurator_test.py index 91758d196..ae2097b3e 100644 --- a/letsencrypt/client/plugins/apache/tests/configurator_test.py +++ b/letsencrypt/client/plugins/apache/tests/configurator_test.py @@ -18,6 +18,8 @@ from letsencrypt.client.plugins.apache import parser from letsencrypt.client.plugins.apache.tests import util +from letsencrypt.client.tests import acme_util + class TwoVhost80Test(util.ApacheTest): """Test two standard well configured HTTP vhosts.""" @@ -157,14 +159,18 @@ class TwoVhost80Test(util.ApacheTest): # Note: As more challenges are offered this will have to be expanded auth_key = le_util.Key(self.rsa256_file, self.rsa256_pem) achall1 = achallenges.DVSNI( - chall=challenges.DVSNI( - r="jIq_Xy1mXGN37tb4L6Xj_es58fW571ZNyXekdZzhh7Q", - nonce="37bc5eb75d3e00a19b4f6355845e5a18"), + challb=acme_util.chall_to_challb( + challenges.DVSNI( + r="jIq_Xy1mXGN37tb4L6Xj_es58fW571ZNyXekdZzhh7Q", + nonce="37bc5eb75d3e00a19b4f6355845e5a18"), + "pending"), domain="encryption-example.demo", key=auth_key) achall2 = achallenges.DVSNI( - chall=challenges.DVSNI( - r="uqnaPzxtrndteOqtrXb0Asl5gOJfWAnnx6QJyvcmlDU", - nonce="59ed014cac95f77057b1d7a1b2c596ba"), + challb=acme_util.chall_to_challb( + challenges.DVSNI( + r="uqnaPzxtrndteOqtrXb0Asl5gOJfWAnnx6QJyvcmlDU", + nonce="59ed014cac95f77057b1d7a1b2c596ba"), + "pending"), domain="letsencrypt.demo", key=auth_key) dvsni_ret_val = [ diff --git a/letsencrypt/client/plugins/apache/tests/dvsni_test.py b/letsencrypt/client/plugins/apache/tests/dvsni_test.py index 9bddfc481..1d1b0e652 100644 --- a/letsencrypt/client/plugins/apache/tests/dvsni_test.py +++ b/letsencrypt/client/plugins/apache/tests/dvsni_test.py @@ -14,6 +14,8 @@ from letsencrypt.client.plugins.apache.obj import Addr from letsencrypt.client.plugins.apache.tests import util +from letsencrypt.client.tests import acme_util + class DvsniPerformTest(util.ApacheTest): """Test the ApacheDVSNI challenge.""" @@ -39,18 +41,22 @@ class DvsniPerformTest(util.ApacheTest): auth_key = le_util.Key(rsa256_file, rsa256_pem) self.achalls = [ achallenges.DVSNI( - chall=challenges.DVSNI( - r="\x8c\x8a\xbf_-f\\cw\xee\xd6\xf8/\xa5\xe3\xfd\xeb9\xf1" - "\xf5\xb9\xefVM\xc9w\xa4u\x9c\xe1\x87\xb4", - nonce="7\xbc^\xb7]>\x00\xa1\x9bOcU\x84^Z\x18", - ), domain="encryption-example.demo", key=auth_key), + challb=acme_util.chall_to_challb( + challenges.DVSNI( + r="\x8c\x8a\xbf_-f\\cw\xee\xd6\xf8/\xa5\xe3\xfd\xeb9" + "\xf1\xf5\xb9\xefVM\xc9w\xa4u\x9c\xe1\x87\xb4", + nonce="7\xbc^\xb7]>\x00\xa1\x9bOcU\x84^Z\x18", + ), "pending"), + domain="encryption-example.demo", key=auth_key), achallenges.DVSNI( - chall=challenges.DVSNI( - r="\xba\xa9\xda?