Improve authz handling. (#998)

changelogs/fragments/998-acme.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - "acme_* modules - improve handling of authz deactivation, and improve error message in case of bad authz states (https://github.com/ansible-collections/community.crypto/pull/998)."

plugins/module_utils/_acme/certificate.py

@@ -199,8 +199,10 @@ class ACMECertificateClient:
         bad_authzs = []
         for authz in order.authorizations.values():
             if authz.status not in ("valid", "pending"):
+                error_details = authz.get_error_details()
+                error = f"; {error_details}" if error_details else ""
                 bad_authzs.append(
-                    f"{authz.combined_identifier} (status={authz.status!r})"
+                    f"{authz.combined_identifier} (status={authz.status!r}{error})"
                 )
         if bad_authzs:
             bad_authzs_str = ", ".join(sorted(bad_authzs))
@@ -412,7 +414,7 @@ class ACMECertificateClient:
                 except Exception:
                     # ignore errors
                     pass
-                if authz is None or authz.status != "deactivated":
+                if authz is None or not authz.is_in_final_state(allow_valid=False):
                     self.module.warn(
                         warning=f"Could not deactivate authz object {authz_uri}."
                     )
@@ -423,7 +425,7 @@ class ACMECertificateClient:
                 except Exception:
                     # ignore errors
                     pass
-                if authz.status != "deactivated":
+                if not authz.is_in_final_state(allow_valid=False):
                     self.module.warn(
                         warning=f"Could not deactivate authz object {authz.url}."
                     )

plugins/module_utils/_acme/challenges.py

@@ -358,13 +358,8 @@ class Authorization:
                 data[challenge.type] = validation_data
         return data
 
-    def raise_error(self, *, error_msg: str, module: AnsibleModule) -> t.NoReturn:
-        """
-        Aborts with a specific error for a challenge.
-        """
+    def get_error_details(self) -> str | None:
         error_details = []
-        # multiple challenges could have failed at this point, gather error
-        # details for all of them before failing
         for challenge in self.challenges:
             if challenge.status == "invalid":
                 msg = f"Challenge {challenge.type}"
@@ -375,9 +370,17 @@ class Authorization:
                     )
                     msg = f"{msg}: {problem}"
                 error_details.append(msg)
+        return "; ".join(error_details) if error_details else None
+
+    def raise_error(self, *, error_msg: str, module: AnsibleModule) -> t.NoReturn:
+        """
+        Aborts with a specific error for a challenge.
+        """
+        error_details = self.get_error_details()
+        error_details_str = f" {error_details}" if error_details else ""
         raise ACMEProtocolException(
             module=module,
-            msg=f"Failed to validate challenge for {self.combined_identifier}: {error_msg}. {'; '.join(error_details)}",
+            msg=f"Failed to validate challenge for {self.combined_identifier}: {error_msg}.{error_details_str}",
             extras={
                 "identifier": self.combined_identifier,
                 "authorization": self.data,
@@ -429,6 +432,11 @@ class Authorization:
         """
         return self.status in ("valid", "pending")
 
+    def is_in_final_state(self, *, allow_valid: bool = True) -> bool:
+        if allow_valid and self.status == "valid":
+            return True
+        return self.status in ("invalid", "revoked", "deactivated", "expired")
+
     def deactivate(self, *, client: ACMEClient) -> bool | None:
         """
         Deactivates this authorization.
@@ -441,13 +449,9 @@ class Authorization:
         result, info = client.send_signed_request(
             self.url, authz_deactivate, fail_on_error=False
         )
-        if (
-            200 <= info["status"] < 300
-            and isinstance(result, dict)
-            and result.get("status") == "deactivated"
-        ):
-            self.status = "deactivated"
-            return True
+        if 200 <= info["status"] < 300 and isinstance(result, dict):
+            self._setup(client=client, data=result)
+            return self.status == "deactivated"
         return False
 
     @classmethod

plugins/modules/acme_certificate.py

@@ -982,7 +982,7 @@ class ACMECertificateClient:
             except Exception:
                 # ignore errors
                 pass
-            if authz.status != "deactivated":
+            if not authz.is_in_final_state(allow_valid=False):
                 self.module.warn(
                     warning=f"Could not deactivate authz object {authz.url}."
                 )