acme: use order "status" to determine action during finalizationacme-fix-finalization-status

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

3 files changed, 24 insertions, 6 deletions

diff --git a/acme/acme/client.py b/acme/acme/client.py
index b5021b447..aa7085fb0 100644
--- a/acme/acme/client.py
+++ b/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.error is not None:
-                raise errors.IssuanceError(body.error)
-            if body.certificate is not None:
+            if body.status == messages.STATUS_INVALID:
+                if body.error is not None:
+                    raise errors.IssuanceError(body.error)
+                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:
diff --git a/acme/tests/client_test.py b/acme/tests/client_test.py
index 2eeceee18..27cb49a9e 100644
--- a/acme/tests/client_test.py
+++ b/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,
diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md
index ba307eae6..ba45d46e4 100644
--- a/certbot/CHANGELOG.md
+++ b/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