Apply review feedback.v2-orders

Also turn raises of bare Exceptions into typed exceptions.

3 files changed, 61 insertions, 28 deletions

diff --git a/acme/acme/client.py b/acme/acme/client.py
index c20d55894..dd3a2ae3c 100644
--- a/acme/acme/client.py
+++ b/acme/acme/client.py
@@ -556,13 +556,17 @@ class ClientV2(ClientBase):
             acme_version=2)
         # "Instance of 'Field' has no key/contact member" bug:
         # pylint: disable=no-member
-        return self._regr_from_response(response)
+        regr = self._regr_from_response(response)
+        self.net.account = regr
+        return regr
 
     def new_order(self, csr_pem):
-        """Request challenges.
+        """Request a new Order object from the server.
+
+        :param str csr_pem: A CSR in PEM format.
 
-        :returns: List of Authorization Resources.
-        :rtype: `list` of `.AuthorizationResource`
+        :returns: The newly created order.
+        :rtype: OrderResource
         """
         csr = cryptography.x509.load_pem_x509_csr(csr_pem,
             cryptography.hazmat.backends.default_backend())
@@ -576,20 +580,10 @@ class ClientV2(ClientBase):
                 value=name))
         order = messages.NewOrder(identifiers=identifiers)
         response = self.net.post(self.directory['newOrder'], order)
-        order_response = self._order_resource_from_response(
-            response, csr_pem=csr_pem)
-        return order_response
-
-    def _order_resource_from_response(self, response, uri=None, csr_pem=None):
         body = messages.Order.from_json(response.json())
         authorizations = []
         for url in body.authorizations:
             authorizations.append(self._authzr_from_response(self.net.get(url)))
-        fullchain_pem = None
-        if body.certificate is not None:
-            certificate_response = self.net.get(body.certificate, content_type=None)
-            if certificate_response.ok:
-                fullchain_pem = certificate_response.text
         return messages.OrderResource(
             body=body,
             uri=response.headers.get('Location', uri),
@@ -608,19 +602,24 @@ class ClientV2(ClientBase):
         responses = []
         for url in orderr.body.authorizations:
             while datetime.datetime.now() < deadline:
-                time.sleep(1)
                 authzr = self._authzr_from_response(self.net.get(url), uri=url)
                 if authzr.body.status != messages.STATUS_PENDING:
                     responses.append(authzr)
                     break
+                time.sleep(1)
+        # If we didn't get a response for every authorization, we fell through
+        # the bottom of the loop due to hitting the deadline.
+        if len(responses) > orderr.body.authorizations:
+            raise TimeoutError()
+        failed = []
         for authzr in responses:
             if authzr.body.status != messages.STATUS_VALID:
                 for chall in authzr.body.challenges:
                     if chall.error != None:
-                        raise Exception("failed challenge for %s: %s" %
-                            (authzr.body.identifier.value, chall.error))
-                raise Exception("failed authorization: %s" % authzr.body)
-        return self._order_resource_from_response(self.net.get(orderr.uri), uri=orderr.uri)
+                        failed.append(authzr)
+        if len(failed) > 0:
+            raise ValidationError(failed)
+        return orderr.update(authorizations=responses)
 
     def finalize_order(self, orderr, deadline):
         csr = OpenSSL.crypto.load_certificate_request(
@@ -629,10 +628,14 @@ class ClientV2(ClientBase):
         self.net.post(latest.body.finalize, wrapped_csr)
         while datetime.datetime.now() < deadline:
             time.sleep(1)
-            latest = self._order_resource_from_response(self.net.get(orderr.uri), uri=orderr.uri)
-            if latest.fullchain_pem is not None:
-                return latest
-        return None
+            response = self.net.get(orderr.uri)
+            body = messages.Order.from_json(response.json())
+            if body.error is not None:
+                raise IssuanceError(body.error)
+            if body.certificate is not None:
+                certificate_response = self.net.get(body.certificate).text
+                return orderr.update(fullchain_pem=certificate_response)
+        raise TimeoutError()
 
 class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
     """Wrapper around requests that signs POSTs for authentication.
@@ -648,7 +651,8 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
 
     :param josepy.JWK key: Account private key
     :param messages.RegistrationResource account: Account object. Required if you are
-            planning to use .post() with acme_version=2.
+            planning to use .post() with acme_version=2 for anything other than
+            creating a new account; may be set later after registering.
     :param josepy.JWASignature alg: Algoritm to use in signing JWS.
     :param bool verify_ssl: Whether to verify certificates on SSL connections.
     :param str user_agent: String to send as User-Agent header.
diff --git a/acme/acme/errors.py b/acme/acme/errors.py
index de5f9d1f4..624ccb3d9 100644
--- a/acme/acme/errors.py
+++ b/acme/acme/errors.py
@@ -83,6 +83,27 @@ class PollError(ClientError):
         return '{0}(exhausted={1!r}, updated={2!r})'.format(
             self.__class__.__name__, self.exhausted, self.updated)
 
+class ValidationError(Error):
+    """Error for authorization failures. Contains a list of authorization
+    resources, each of which is invalid and should have an error field.
+    """
+    def __init__(self, failed_authzrs):
+        self.failed_authzrs = failed_authzrs
+        super(ClientError, self).__init__()
+
+class TimeoutError(Error):
+    """Error for when polling an authorization or an order times out."""
+
+class IssuanceError(Error):
+    """Error sent by the server after requesting issuance of a certificate."""
+
+    def __init__(self, error):
+        """Initialize.
+
+        :param messages.Error error: The error provided by the server.
+        """
+        self.error = error
+
 class ConflictError(ClientError):
     """Error for when the server returns a 409 (Conflict) HTTP status.
 
diff --git a/acme/acme/messages.py b/acme/acme/messages.py
index 64455d04a..9c7855976 100644
--- a/acme/acme/messages.py
+++ b/acme/acme/messages.py
@@ -489,10 +489,17 @@ class Revocation(jose.JSONObjectWithFields):
 class Order(ResourceBody):
     """Order Resource Body.
 
-    :ivar buffer csr: CSR in pem format.
+    .. note:: Parsing of identifiers on response doesn't work right now; to make
+        it work we would need to set up the equivalent of Identifier.from_json, but
+        for a list.
+    :ivar list of .Identifier: List of identifiers for the certificate.
     :ivar acme.messages.Status status:
-    :ivar list of string authorizations: URLs of authorizations.
+    :ivar list of str authorizations: URLs of authorizations.
+    :ivar str certificate: URL to download certificate as a fullchain PEM.
+    :ivar str finalize: URL to POST to to request issuance once all
+        authorizations have "valid" status.
     :ivar datetime.datetime expires: When the order expires.
+    :ivar .Error error: Any error that occurred during finalization, if applicable.
     """
     identifiers = jose.Field('identifiers', omitempty=True)
     status = jose.Field('status', decoder=Status.from_json,
@@ -501,15 +508,16 @@ class Order(ResourceBody):
     certificate = jose.Field('certificate', omitempty=True)
     finalize = jose.Field('finalize', omitempty=True)
     expires = fields.RFC3339Field('expires', omitempty=True)
+    error = jose.Field('error', omitempty=True, decoder=Error.from_json)
 
 class OrderResource(ResourceWithURI):
     """Order Resource.
 
     :ivar acme.messages.Order body:
+    :ivar str csr_pem: The CSR this Order will be finalized with.
     :ivar list of acme.messages.AuthorizationResource authorizations:
         Fully-fetched AuthorizationResource objects.
-    :ivar string csr_pem: The CSR this Order will be finalized with.
-    :ivar string fullchain_pem: The fetched contents of the certificate URL
+    :ivar str fullchain_pem: The fetched contents of the certificate URL
         produced once the order was finalized, if it's present.
     """
     body = jose.Field('body', decoder=Order.from_json)