acme: use order "status" to determine action during finalization (#9297)

Rather than deducing the status of an order by the "certificate"
and "error" fields, use the "status" field directly.

acme/acme/client.py

@@ -797,9 +797,13 @@ class ClientV2(ClientBase):
             time.sleep(1)
             response = self._post_as_get(orderr.uri)
             body = messages.Order.from_json(response.json())
+            if body.status == messages.STATUS_INVALID:
                 if body.error is not None:
                     raise errors.IssuanceError(body.error)
-            if body.certificate is not None:
+                raise errors.Error(
+                    "The certificate order failed. No further information was provided "
+                    "by the server.")
+            elif body.status == messages.STATUS_VALID and body.certificate is not None:
                 certificate_response = self._post_as_get(body.certificate)
                 orderr = orderr.update(body=body, fullchain_pem=certificate_response.text)
                 if fetch_alternative_chains:

acme/tests/client_test.py

@@ -822,7 +822,8 @@ class ClientV2Test(ClientTestBase):
 
     def test_finalize_order_success(self):
         updated_order = self.order.update(
-            certificate='https://www.letsencrypt-demo.org/acme/cert/')
+            certificate='https://www.letsencrypt-demo.org/acme/cert/',
+            status=messages.STATUS_VALID)
         updated_orderr = self.orderr.update(body=updated_order, fullchain_pem=CERT_SAN_PEM)
 
         self.response.json.return_value = updated_order.to_json()
@@ -832,12 +833,22 @@ class ClientV2Test(ClientTestBase):
         self.assertEqual(self.client.finalize_order(self.orderr, deadline), updated_orderr)
 
     def test_finalize_order_error(self):
-        updated_order = self.order.update(error=messages.Error.with_code('unauthorized'))
+        updated_order = self.order.update(
+            error=messages.Error.with_code('unauthorized'),
+            status=messages.STATUS_INVALID)
         self.response.json.return_value = updated_order.to_json()
 
         deadline = datetime.datetime(9999, 9, 9)
         self.assertRaises(errors.IssuanceError, self.client.finalize_order, self.orderr, deadline)
 
+    def test_finalize_order_invalid_status(self):
+        # https://github.com/certbot/certbot/issues/9296
+        order = self.order.update(error=None, status=messages.STATUS_INVALID)
+        self.response.json.return_value = order.to_json()
+        with self.assertRaises(errors.Error) as error:
+            self.client.finalize_order(self.orderr, datetime.datetime(9999, 9, 9))
+        self.assertIn("The certificate order failed", str(error.exception))
+
     def test_finalize_order_timeout(self):
         deadline = datetime.datetime.now() - datetime.timedelta(seconds=60)
         self.assertRaises(errors.TimeoutError, self.client.finalize_order, self.orderr, deadline)
@@ -845,6 +856,7 @@ class ClientV2Test(ClientTestBase):
     def test_finalize_order_alt_chains(self):
         updated_order = self.order.update(
             certificate='https://www.letsencrypt-demo.org/acme/cert/',
+            status=messages.STATUS_VALID
         )
         updated_orderr = self.orderr.update(body=updated_order,
                                             fullchain_pem=CERT_SAN_PEM,

certbot/CHANGELOG.md

@@ -10,7 +10,9 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
-*
+* A change to order finalization has been made to the `acme` module and Certbot:
+  - An order's `certificate` field will only be processed if the order's `status` is `valid`.
+  - An order's `error` field will only be processed if the order's `status` is `invalid`.
 
 ### Fixed