diff --git a/acme/acme/client.py b/acme/acme/client.py index 0e9319f9c..08d476783 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -246,9 +246,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes def retry_after(cls, response, default): """Compute next `poll` time based on response ``Retry-After`` header. - :param response: Response from `poll`. - :type response: `requests.Response` - + :param requests.Response response: Response from `poll`. :param int default: Default value (in seconds), used when ``Retry-After`` header is not present or invalid. @@ -323,22 +321,21 @@ class Client(object): # pylint: disable=too-many-instance-attributes body=jose.ComparableX509(OpenSSL.crypto.load_certificate( OpenSSL.crypto.FILETYPE_ASN1, response.content))) - def poll_and_request_issuance(self, csr, authzrs, mintime=5): + def poll_and_request_issuance( + self, csr, authzrs, mintime=5, max_attempts=10): """Poll and request issuance. This function polls all provided Authorization Resource URIs until all challenges are valid, respecting ``Retry-After`` HTTP headers, and then calls `request_issuance`. - .. todo:: add `max_attempts` or `timeout` - - :param csr: CSR. - :type csr: `OpenSSL.crypto.X509Req` wrapped in `.ComparableX509` - + :param .ComparableX509 csr: CSR (`OpenSSL.crypto.X509Req` + wrapped in `.ComparableX509`) :param authzrs: `list` of `.AuthorizationResource` - :param int mintime: Minimum time before next attempt, used if ``Retry-After`` is not present in the response. + :param int max_attempts: Maximum number of attempts before + `PollError` with non-empty ``waiting`` is raised. :returns: ``(cert, updated_authzrs)`` `tuple` where ``cert`` is the issued certificate (`.messages.CertificateResource.), @@ -348,6 +345,9 @@ class Client(object): # pylint: disable=too-many-instance-attributes as the input ``authzrs``. :rtype: `tuple` + :raises PollError: in case of timeout or if some authorization + was marked by the CA as invalid + """ # priority queue with datetime (based on Retry-After) as key, # and original Authorization Resource as value @@ -356,7 +356,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes # recently updated one updated = dict((authzr, authzr) for authzr in authzrs) - while waiting: + while waiting and max_attempts: + max_attempts -= 1 # find the smallest Retry-After, and sleep if necessary when, authzr = heapq.heappop(waiting) now = datetime.datetime.now() @@ -371,11 +372,16 @@ class Client(object): # pylint: disable=too-many-instance-attributes updated[authzr] = updated_authzr # pylint: disable=no-member - if updated_authzr.body.status != messages.STATUS_VALID: + if updated_authzr.body.status not in ( + messages.STATUS_VALID, messages.STATUS_INVALID): # push back to the priority queue, with updated retry_after heapq.heappush(waiting, (self.retry_after( response, default=mintime), authzr)) + if not max_attempts or any(authzr.body.status == messages.STATUS_INVALID + for authzr in six.itervalues(updated)): + raise errors.PollError(waiting, updated) + updated_authzrs = tuple(updated[authzr] for authzr in authzrs) return self.request_issuance(csr, updated_authzrs), updated_authzrs diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 2df7b5313..58f55b293 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -271,9 +271,9 @@ class ClientTest(unittest.TestCase): # result, increment clock clock.dt += datetime.timedelta(seconds=2) - if not authzr.retries: # no more retries + if len(authzr.retries) == 1: # no more retries done = mock.MagicMock(uri=authzr.uri, times=authzr.times) - done.body.status = messages.STATUS_VALID + done.body.status = authzr.retries[0] return done, [] # response (2nd result tuple element) is reduced to only @@ -289,7 +289,8 @@ class ClientTest(unittest.TestCase): mintime = 7 - def retry_after(response, default): # pylint: disable=missing-docstring + def retry_after(response, default): + # pylint: disable=missing-docstring # check that poll_and_request_issuance correctly passes mintime self.assertEqual(default, mintime) return clock.dt + datetime.timedelta(seconds=response) @@ -302,8 +303,10 @@ class ClientTest(unittest.TestCase): csr = mock.MagicMock() authzrs = ( - mock.MagicMock(uri='a', times=[], retries=(8, 20, 30)), - mock.MagicMock(uri='b', times=[], retries=(5,)), + mock.MagicMock(uri='a', times=[], retries=( + 8, 20, 30, messages.STATUS_VALID)), + mock.MagicMock(uri='b', times=[], retries=( + 5, messages.STATUS_VALID)), ) cert, updated_authzrs = self.client.poll_and_request_issuance( @@ -327,6 +330,17 @@ class ClientTest(unittest.TestCase): ]) self.assertEqual(clock.dt, datetime.datetime(2015, 3, 27, 0, 1, 7)) + # CA sets invalid | TODO: move to a separate test + invalid_authzr = mock.MagicMock(times=[], retries=[messages.STATUS_INVALID]) + self.assertRaises( + errors.PollError, self.client.poll_and_request_issuance, + csr, authzrs=(invalid_authzr,), mintime=mintime) + + # exceeded max_attemps | TODO: move to a separate test + self.assertRaises( + errors.PollError, self.client.poll_and_request_issuance, + csr, authzrs, mintime=mintime, max_attempts=2) + def test_check_cert(self): self.response.headers['Location'] = self.certr.uri self.response.content = CERT_DER diff --git a/acme/acme/errors.py b/acme/acme/errors.py index 9a96ec43a..0385667c7 100644 --- a/acme/acme/errors.py +++ b/acme/acme/errors.py @@ -51,3 +51,31 @@ class MissingNonce(NonceError): return ('Server {0} response did not include a replay ' 'nonce, headers: {1}'.format( self.response.request.method, self.response.headers)) + + +class PollError(ClientError): + """Generic error when polling for authorization fails. + + This might be caused by either timeout (`waiting` will be non-empty) + or by some authorization being invalid. + + :ivar waiting: Priority queue with `datetime.datatime` (based on + ``Retry-After``) as key, and original `.AuthorizationResource` + as value. + :ivar updated: Mapping from original `.AuthorizationResource` + to the most recently updated one + + """ + def __init__(self, waiting, updated): + self.waiting = waiting + self.updated = updated + super(PollError, self).__init__() + + @property + def timeout(self): + """Was the error caused by timeout?""" + return bool(self.waiting) + + def __repr__(self): + return '{0}(waiting={1!r}, updated={2!r})'.format( + self.__class__.__name__, self.waiting, self.updated) diff --git a/acme/acme/errors_test.py b/acme/acme/errors_test.py index 3790d91ed..45b269a0b 100644 --- a/acme/acme/errors_test.py +++ b/acme/acme/errors_test.py @@ -1,4 +1,5 @@ """Tests for acme.errors.""" +import datetime import unittest import mock @@ -29,5 +30,25 @@ class MissingNonceTest(unittest.TestCase): self.assertTrue("{}" in str(self.error)) +class PollErrorTest(unittest.TestCase): + """Tests for acme.errors.PollError.""" + + def setUp(self): + from acme.errors import PollError + self.timeout = PollError( + waiting=[(datetime.datetime(2015, 11, 29), mock.sentinel.AR)], + updated={}) + self.invalid = PollError(waiting=[], updated={ + mock.sentinel.AR: mock.sentinel.AR2}) + + def test_timeout(self): + self.assertTrue(self.timeout.timeout) + self.assertFalse(self.invalid.timeout) + + def test_repr(self): + self.assertEqual('PollError(waiting=[], updated={sentinel.AR: ' + 'sentinel.AR2})', repr(self.invalid)) + + if __name__ == "__main__": unittest.main() # pragma: no cover