From 64f011c8bb494e107aacf2fae9b28fbfda8b0122 Mon Sep 17 00:00:00 2001 From: Will Oller Date: Sun, 30 Nov 2014 12:12:31 -0800 Subject: [PATCH 1/5] Parameter checking and filling in TODOs --- letsencrypt/client/acme.py | 82 +++++++++++++++++++++++++++++---- letsencrypt/client/acme_test.py | 16 ++++++- 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/letsencrypt/client/acme.py b/letsencrypt/client/acme.py index 50ab26e8b..5e66b6020 100644 --- a/letsencrypt/client/acme.py +++ b/letsencrypt/client/acme.py @@ -70,32 +70,67 @@ def challenge_request(name): :rtype: dict """ + + if type(name) is not unicode: + raise TypeError("Parameter `name` must be unicode") + + if not name: + raise ValueError("Parameter `name` must not be empty") + return { "type": "challengeRequest", "identifier": name, } -def authorization_request(req_id, name, server_nonce, responses, key_file): +def authorization_request(req_id, name, server_nonce, responses, key): """Create ACME "authorizationRequest" message. - :param req_id: TODO - :param name: TODO - :param server_nonce: TODO - :param responses: TODO - :param key_file: TODO + :param str req_id: SessionID from the server challenge + :param unicode name: Hostname + :param str server_nonce: Nonce from the server challenge + :param list responses: List of completed challenges + :param str key: Key in string form. Accepted formats + are the same as for `Crypto.PublicKey.RSA.importKey`. :returns: ACME "authorizationRequest" message. :rtype: dict """ + if type(req_id) is not str: + raise TypeError("Parameter `req_id` must be of type str") + + if not req_id: + raise ValueError("Parameter `req_id` must not be empty") + + if type(name) is not unicode: + raise TypeError("Parameter `name` must be of type unicode") + + if not name: + raise ValueError("Parameter `name` must not be empty") + + if type(server_nonce) is not str: + raise TypeError("Parameter `server_nonce` must be of type str") + + if not server_nonce: + raise ValueError("Parameter `server_nonce` must not be empty") + + if type(responses) is not list: + raise TypeError("Parameter `responses` must be of type list") + + if type(key) is not str: + raise TypeError("Parameter `key` must be of type str") + + if not key: + raise ValueError("Parameter `key` must not be empty") + return { "type": "authorizationRequest", "sessionID": req_id, "nonce": server_nonce, "responses": responses, "signature": crypto_util.create_sig( - name + le_util.jose_b64decode(server_nonce), key_file), + name + le_util.jose_b64decode(server_nonce), key), } @@ -103,12 +138,25 @@ def certificate_request(csr_der, key): """Create ACME "certificateRequest" message. :param str csr_der: DER encoded CSR. - :param key: TODO + :param str key: Key in string form. Accepted formats + are the same as for `Crypto.PublicKey.RSA.importKey`. :returns: ACME "certificateRequest" message. :rtype: dict """ + if type(csr_der) is not str: + raise TypeError("Parameter `csr_der` must be of type str") + + if not csr_der: + raise ValueError("Parameter `csr_der` must not be empty") + + if type(key) is not str: + raise TypeError("Parameter `key` must be of type str") + + if not key: + raise ValueError("Parameter `key` must not be empty") + return { "type": "certificateRequest", "csr": le_util.jose_b64encode(csr_der), @@ -128,6 +176,18 @@ def revocation_request(key_file, cert_der): :rtype: dict """ + if type(key_file) is not str: + raise TypeError("Parameter `key_file` must be of type str") + + if not key_file: + raise ValueError("Parameter `key_file` must not be empty") + + if type(cert_der) is not str: + raise TypeError("Parameter `cert_der` must be of type str") + + if not cert_der: + raise ValueError("Parameter `cert_der` must not be empty") + return { "type": "revocationRequest", "certificate": le_util.jose_b64encode(cert_der), @@ -144,6 +204,12 @@ def status_request(token): :rtype: dict """ + if type(token) is not str: + raise TypeError("Parameter `token` must be of type str") + + if not token: + raise ValueError("Parameter `token` must not be empty") + return { "type": "statusRequest", "token": token, diff --git a/letsencrypt/client/acme_test.py b/letsencrypt/client/acme_test.py index f51a40d08..864dc938b 100644 --- a/letsencrypt/client/acme_test.py +++ b/letsencrypt/client/acme_test.py @@ -53,11 +53,25 @@ class PrettyTest(unittest.TestCase): self._call('{"foo": {"bar": "baz"}}'), '{\n "foo": {\n "bar": "baz"\n }\n}') + class ChallengeRequestTest(unittest.TestCase): - """Tests for letsencrypt.client.acme.challenge_request_test""" + """Tests for letsencrypt.client.acme.challenge_request""" + + def test_empty_parameter(self): + """Test showing challenge_request does not allow empty unicode""" + + from letsencrypt.client.acme import challenge_request + self.assertRaises(ValueError, challenge_request, u'') + + def test_string_not_supported(self): + """Test error when passing non-unicode string""" + + from letsencrypt.client.acme import challenge_request + self.assertRaises(TypeError, challenge_request, 'string') def test_supports_unicode(self): """Test support unicode parameter""" + from letsencrypt.client.acme import challenge_request self.assertEqual( challenge_request(u'unicode'), From 8885f608a28e4d0dd1102523723d6aac69cd1532 Mon Sep 17 00:00:00 2001 From: Will Oller Date: Sun, 30 Nov 2014 15:47:59 -0800 Subject: [PATCH 2/5] Single quotes instead of tics --- letsencrypt/client/acme.py | 42 +++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/letsencrypt/client/acme.py b/letsencrypt/client/acme.py index 5e66b6020..7aec6e595 100644 --- a/letsencrypt/client/acme.py +++ b/letsencrypt/client/acme.py @@ -72,10 +72,10 @@ def challenge_request(name): """ if type(name) is not unicode: - raise TypeError("Parameter `name` must be unicode") + raise TypeError("Parameter 'name' must be unicode") if not name: - raise ValueError("Parameter `name` must not be empty") + raise ValueError("Parameter 'name' must not be empty") return { "type": "challengeRequest", @@ -98,31 +98,31 @@ def authorization_request(req_id, name, server_nonce, responses, key): """ if type(req_id) is not str: - raise TypeError("Parameter `req_id` must be of type str") + raise TypeError("Parameter 'req_id' must be of type str") if not req_id: - raise ValueError("Parameter `req_id` must not be empty") + raise ValueError("Parameter 'req_id' must not be empty") if type(name) is not unicode: - raise TypeError("Parameter `name` must be of type unicode") + raise TypeError("Parameter 'name' must be of type unicode") if not name: - raise ValueError("Parameter `name` must not be empty") + raise ValueError("Parameter 'name' must not be empty") if type(server_nonce) is not str: - raise TypeError("Parameter `server_nonce` must be of type str") + raise TypeError("Parameter 'server_nonce' must be of type str") if not server_nonce: - raise ValueError("Parameter `server_nonce` must not be empty") + raise ValueError("Parameter 'server_nonce' must not be empty") if type(responses) is not list: - raise TypeError("Parameter `responses` must be of type list") + raise TypeError("Parameter 'responses' must be of type list") if type(key) is not str: - raise TypeError("Parameter `key` must be of type str") + raise TypeError("Parameter 'key' must be of type str") if not key: - raise ValueError("Parameter `key` must not be empty") + raise ValueError("Parameter 'key' must not be empty") return { "type": "authorizationRequest", @@ -146,16 +146,16 @@ def certificate_request(csr_der, key): """ if type(csr_der) is not str: - raise TypeError("Parameter `csr_der` must be of type str") + raise TypeError("Parameter 'csr_der' must be of type str") if not csr_der: - raise ValueError("Parameter `csr_der` must not be empty") + raise ValueError("Parameter 'csr_der' must not be empty") if type(key) is not str: - raise TypeError("Parameter `key` must be of type str") + raise TypeError("Parameter 'key' must be of type str") if not key: - raise ValueError("Parameter `key` must not be empty") + raise ValueError("Parameter 'key' must not be empty") return { "type": "certificateRequest", @@ -177,16 +177,16 @@ def revocation_request(key_file, cert_der): """ if type(key_file) is not str: - raise TypeError("Parameter `key_file` must be of type str") + raise TypeError("Parameter 'key_file' must be of type str") if not key_file: - raise ValueError("Parameter `key_file` must not be empty") + raise ValueError("Parameter 'key_file' must not be empty") if type(cert_der) is not str: - raise TypeError("Parameter `cert_der` must be of type str") + raise TypeError("Parameter 'cert_der' must be of type str") if not cert_der: - raise ValueError("Parameter `cert_der` must not be empty") + raise ValueError("Parameter 'cert_der' must not be empty") return { "type": "revocationRequest", @@ -205,10 +205,10 @@ def status_request(token): """ if type(token) is not str: - raise TypeError("Parameter `token` must be of type str") + raise TypeError("Parameter 'token' must be of type str") if not token: - raise ValueError("Parameter `token` must not be empty") + raise ValueError("Parameter 'token' must not be empty") return { "type": "statusRequest", From 3184dba80ca16ca73e9cf2f3228d7fe5f8ff0e5a Mon Sep 17 00:00:00 2001 From: Will Oller Date: Sun, 30 Nov 2014 15:52:34 -0800 Subject: [PATCH 3/5] PEP8: Use isinstance(name, unicode) --- letsencrypt/client/acme.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/letsencrypt/client/acme.py b/letsencrypt/client/acme.py index 7aec6e595..52ac1e4bd 100644 --- a/letsencrypt/client/acme.py +++ b/letsencrypt/client/acme.py @@ -70,8 +70,7 @@ def challenge_request(name): :rtype: dict """ - - if type(name) is not unicode: + if not isinstance(name, unicode): raise TypeError("Parameter 'name' must be unicode") if not name: @@ -97,28 +96,28 @@ def authorization_request(req_id, name, server_nonce, responses, key): :rtype: dict """ - if type(req_id) is not str: + if not isinstance(req_id, str): raise TypeError("Parameter 'req_id' must be of type str") if not req_id: raise ValueError("Parameter 'req_id' must not be empty") - if type(name) is not unicode: + if not isinstance(name, unicode): raise TypeError("Parameter 'name' must be of type unicode") if not name: raise ValueError("Parameter 'name' must not be empty") - if type(server_nonce) is not str: + if not isinstance(server_nonce, str): raise TypeError("Parameter 'server_nonce' must be of type str") if not server_nonce: raise ValueError("Parameter 'server_nonce' must not be empty") - if type(responses) is not list: + if not isinstance(responses, list): raise TypeError("Parameter 'responses' must be of type list") - if type(key) is not str: + if not isinstance(key, str): raise TypeError("Parameter 'key' must be of type str") if not key: @@ -145,13 +144,13 @@ def certificate_request(csr_der, key): :rtype: dict """ - if type(csr_der) is not str: + if not isinstance(csr_der, str): raise TypeError("Parameter 'csr_der' must be of type str") if not csr_der: raise ValueError("Parameter 'csr_der' must not be empty") - if type(key) is not str: + if not isinstance(key, str): raise TypeError("Parameter 'key' must be of type str") if not key: @@ -176,13 +175,13 @@ def revocation_request(key_file, cert_der): :rtype: dict """ - if type(key_file) is not str: + if not isinstance(key_file, str): raise TypeError("Parameter 'key_file' must be of type str") if not key_file: raise ValueError("Parameter 'key_file' must not be empty") - if type(cert_der) is not str: + if not isinstance(cert_der, str): raise TypeError("Parameter 'cert_der' must be of type str") if not cert_der: @@ -204,7 +203,7 @@ def status_request(token): :rtype: dict """ - if type(token) is not str: + if not isinstance(token, str): raise TypeError("Parameter 'token' must be of type str") if not token: From 30d4aeb463b6089825c2a203621e55b1b182850b Mon Sep 17 00:00:00 2001 From: Will Oller Date: Sun, 30 Nov 2014 15:52:45 -0800 Subject: [PATCH 4/5] Remove some whitespace --- letsencrypt/client/acme_test.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/letsencrypt/client/acme_test.py b/letsencrypt/client/acme_test.py index 864dc938b..b00dd8443 100644 --- a/letsencrypt/client/acme_test.py +++ b/letsencrypt/client/acme_test.py @@ -59,19 +59,16 @@ class ChallengeRequestTest(unittest.TestCase): def test_empty_parameter(self): """Test showing challenge_request does not allow empty unicode""" - from letsencrypt.client.acme import challenge_request self.assertRaises(ValueError, challenge_request, u'') def test_string_not_supported(self): """Test error when passing non-unicode string""" - from letsencrypt.client.acme import challenge_request self.assertRaises(TypeError, challenge_request, 'string') def test_supports_unicode(self): """Test support unicode parameter""" - from letsencrypt.client.acme import challenge_request self.assertEqual( challenge_request(u'unicode'), From e49cf1899b081e1e4cdf4f74940198c379a05564 Mon Sep 17 00:00:00 2001 From: Will Oller Date: Sun, 30 Nov 2014 16:06:58 -0800 Subject: [PATCH 5/5] Remove aggro type checking --- letsencrypt/client/acme.py | 63 --------------------------------- letsencrypt/client/acme_test.py | 10 ------ 2 files changed, 73 deletions(-) diff --git a/letsencrypt/client/acme.py b/letsencrypt/client/acme.py index 52ac1e4bd..138d0d851 100644 --- a/letsencrypt/client/acme.py +++ b/letsencrypt/client/acme.py @@ -70,12 +70,6 @@ def challenge_request(name): :rtype: dict """ - if not isinstance(name, unicode): - raise TypeError("Parameter 'name' must be unicode") - - if not name: - raise ValueError("Parameter 'name' must not be empty") - return { "type": "challengeRequest", "identifier": name, @@ -96,33 +90,6 @@ def authorization_request(req_id, name, server_nonce, responses, key): :rtype: dict """ - if not isinstance(req_id, str): - raise TypeError("Parameter 'req_id' must be of type str") - - if not req_id: - raise ValueError("Parameter 'req_id' must not be empty") - - if not isinstance(name, unicode): - raise TypeError("Parameter 'name' must be of type unicode") - - if not name: - raise ValueError("Parameter 'name' must not be empty") - - if not isinstance(server_nonce, str): - raise TypeError("Parameter 'server_nonce' must be of type str") - - if not server_nonce: - raise ValueError("Parameter 'server_nonce' must not be empty") - - if not isinstance(responses, list): - raise TypeError("Parameter 'responses' must be of type list") - - if not isinstance(key, str): - raise TypeError("Parameter 'key' must be of type str") - - if not key: - raise ValueError("Parameter 'key' must not be empty") - return { "type": "authorizationRequest", "sessionID": req_id, @@ -144,18 +111,6 @@ def certificate_request(csr_der, key): :rtype: dict """ - if not isinstance(csr_der, str): - raise TypeError("Parameter 'csr_der' must be of type str") - - if not csr_der: - raise ValueError("Parameter 'csr_der' must not be empty") - - if not isinstance(key, str): - raise TypeError("Parameter 'key' must be of type str") - - if not key: - raise ValueError("Parameter 'key' must not be empty") - return { "type": "certificateRequest", "csr": le_util.jose_b64encode(csr_der), @@ -175,18 +130,6 @@ def revocation_request(key_file, cert_der): :rtype: dict """ - if not isinstance(key_file, str): - raise TypeError("Parameter 'key_file' must be of type str") - - if not key_file: - raise ValueError("Parameter 'key_file' must not be empty") - - if not isinstance(cert_der, str): - raise TypeError("Parameter 'cert_der' must be of type str") - - if not cert_der: - raise ValueError("Parameter 'cert_der' must not be empty") - return { "type": "revocationRequest", "certificate": le_util.jose_b64encode(cert_der), @@ -203,12 +146,6 @@ def status_request(token): :rtype: dict """ - if not isinstance(token, str): - raise TypeError("Parameter 'token' must be of type str") - - if not token: - raise ValueError("Parameter 'token' must not be empty") - return { "type": "statusRequest", "token": token, diff --git a/letsencrypt/client/acme_test.py b/letsencrypt/client/acme_test.py index b00dd8443..dde50d5ad 100644 --- a/letsencrypt/client/acme_test.py +++ b/letsencrypt/client/acme_test.py @@ -57,16 +57,6 @@ class PrettyTest(unittest.TestCase): class ChallengeRequestTest(unittest.TestCase): """Tests for letsencrypt.client.acme.challenge_request""" - def test_empty_parameter(self): - """Test showing challenge_request does not allow empty unicode""" - from letsencrypt.client.acme import challenge_request - self.assertRaises(ValueError, challenge_request, u'') - - def test_string_not_supported(self): - """Test error when passing non-unicode string""" - from letsencrypt.client.acme import challenge_request - self.assertRaises(TypeError, challenge_request, 'string') - def test_supports_unicode(self): """Test support unicode parameter""" from letsencrypt.client.acme import challenge_request