From d966acbef46ba96af1f1a6a0bbe459df40677626 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 5 Apr 2026 11:45:11 +0200 Subject: [PATCH] Improve authz handling. (#998) --- changelogs/fragments/998-acme.yml | 2 ++ plugins/module_utils/_acme/certificate.py | 8 +++--- plugins/module_utils/_acme/challenges.py | 32 +++++++++++++---------- plugins/modules/acme_certificate.py | 2 +- 4 files changed, 26 insertions(+), 18 deletions(-) create mode 100644 changelogs/fragments/998-acme.yml diff --git a/changelogs/fragments/998-acme.yml b/changelogs/fragments/998-acme.yml new file mode 100644 index 00000000..c10a071b --- /dev/null +++ b/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)." diff --git a/plugins/module_utils/_acme/certificate.py b/plugins/module_utils/_acme/certificate.py index 6bcec77d..61456689 100644 --- a/plugins/module_utils/_acme/certificate.py +++ b/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}." ) diff --git a/plugins/module_utils/_acme/challenges.py b/plugins/module_utils/_acme/challenges.py index 8ab1c141..8b6d3e7d 100644 --- a/plugins/module_utils/_acme/challenges.py +++ b/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 diff --git a/plugins/modules/acme_certificate.py b/plugins/modules/acme_certificate.py index 457745a7..314962da 100644 --- a/plugins/modules/acme_certificate.py +++ b/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}." )