From 053d217c2239e5b170455931b9855faddff2646b Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 01:31:55 +0100 Subject: [PATCH 01/10] Implement acme.sh-based certificate revocation lifecycle --- README.md | 23 +++++ acme_api/backend/acmesh_backend.py | 22 +++++ acme_api/backend/mock_backend.py | 11 +++ acme_api/backend/protocol.py | 9 ++ acme_api/models/__init__.py | 3 + acme_api/models/certificate.py | 6 ++ acme_api/models/certificate_revocation.py | 57 +++++++++++ acme_api/routers/certificates.py | 36 +++++++ acme_api/schemas/certificate.py | 31 ++++++ acme_api/services/certificate_revocations.py | 98 +++++++++++++++++++ ...7c3e9d2b4f1_add_certificate_revocations.py | 65 ++++++++++++ tests/helpers/api.py | 64 ++++++++++-- tests/integration/test_e2e_lifecycle.py | 11 +++ tests/unit/test_api_phase5.py | 72 ++++++++++++++ tests/unit/test_backend_commands.py | 27 +++++ tests/unit/test_scheduler.py | 11 +++ 16 files changed, 539 insertions(+), 7 deletions(-) create mode 100644 acme_api/models/certificate_revocation.py create mode 100644 acme_api/services/certificate_revocations.py create mode 100644 alembic/versions/a7c3e9d2b4f1_add_certificate_revocations.py diff --git a/README.md b/README.md index d96a146..0c1d107 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,7 @@ OpenAPI is generated at `/openapi.json`; Swagger UI is available at `/docs`. | `POST` | `/v1/certificates/{id}/authorize` | operator | Authorize or retry DNS Persist issuance after publishing TXT | | `POST` | `/v1/certificates/{id}/release` | operator | Release a held DNS Persist revision; requires `Idempotency-Key` and `{ "revision": n }` | | `POST` | `/v1/certificates/{id}/renew` | operator | Queue manual renewal | +| `POST` | `/v1/certificates/{id}/revoke` | operator | Revoke the issued primary domain through acme.sh; requires `Idempotency-Key` | | `DELETE` | `/v1/certificates/{id}` | operator | Soft-delete as revoked | | `GET` | `/v1/accounts` | readonly | List configured ACME accounts | | `GET` | `/v1/providers` | readonly | List configured DNS providers | @@ -195,6 +196,28 @@ curl \ http://localhost:8080/v1/certificates ``` +### Certificate revocation + +`DELETE /v1/certificates/{id}` only changes the local request record; it does +not contact a certificate authority. To revoke an issued certificate at its +configured CA, call `POST /v1/certificates/{id}/revoke` with an +`Idempotency-Key` header and, optionally, an RFC 5280 reason: + +```sh +curl -X POST \ + -H "Authorization: Bearer $ACME_API_KEY" \ + -H "Idempotency-Key: revoke-example-20260724" \ + -H "Content-Type: application/json" \ + --data '{"reason": 1}' \ + http://localhost:8080/v1/certificates/$CERTIFICATE_ID/revoke +``` + +The operation invokes acme.sh as `--revoke --domain ` and adds +`--revoke-reason` when requested. It does not delete deployed artifacts, +disable renewal, or otherwise modify the local certificate record. Reusing the +same key returns the durable original result without another acme.sh command. +Reasons `0` through `10` are accepted except `7`, which RFC 5280 leaves unused. + ## Certificate Deployment Successful issuance and renewal deploy artifacts under the `deployment_directory` diff --git a/acme_api/backend/acmesh_backend.py b/acme_api/backend/acmesh_backend.py index b2cfbd9..b9fcd1d 100644 --- a/acme_api/backend/acmesh_backend.py +++ b/acme_api/backend/acmesh_backend.py @@ -238,6 +238,28 @@ async def renew_certificate( domains=domains, ) + async def revoke_certificate( + self, + domain: str, + *, + reason: int | None = None, + account_key_path: str | None = None, + server_url: str | None = None, + ) -> None: + """Revoke acme.sh's certificate for ``domain`` using the configured account.""" + args = ["--revoke", "--domain", domain] + if reason is not None: + args += ["--revoke-reason", str(reason)] + if server_url is not None: + args += ["--server", server_url] + if account_key_path is not None: + args += ["--accountkey-file", account_key_path] + try: + await self._run(args) + except TerminalAcmeShError as exc: + if "already revoked" not in exc.stderr.lower() and "already revoked" not in str(exc).lower(): + raise + async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: args = [ "--in", diff --git a/acme_api/backend/mock_backend.py b/acme_api/backend/mock_backend.py index 11912bd..cef895c 100644 --- a/acme_api/backend/mock_backend.py +++ b/acme_api/backend/mock_backend.py @@ -61,6 +61,17 @@ async def renew_certificate( domains=domains, ) + async def revoke_certificate( + self, + domain: str, + *, + reason: int | None = None, + account_key_path: str | None = None, + server_url: str | None = None, + ) -> None: + """Accept a deterministic revocation without contacting an ACME server.""" + del domain, reason, account_key_path, server_url + async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: """Return a predictable expiry for the supplied certificate path.""" cert = self._cert_expiry("example.com") diff --git a/acme_api/backend/protocol.py b/acme_api/backend/protocol.py index be5a033..a9e2f57 100644 --- a/acme_api/backend/protocol.py +++ b/acme_api/backend/protocol.py @@ -48,4 +48,13 @@ async def renew_certificate( force_renewal: bool = False, ) -> IssuanceResult: ... + async def revoke_certificate( + self, + domain: str, + *, + reason: int | None = None, + account_key_path: str | None = None, + server_url: str | None = None, + ) -> None: ... + async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: ... diff --git a/acme_api/models/__init__.py b/acme_api/models/__init__.py index 6f24c8d..c861c42 100644 --- a/acme_api/models/__init__.py +++ b/acme_api/models/__init__.py @@ -18,6 +18,7 @@ from acme_api.models.api_key import APIKey, APIKeyRole from acme_api.models.base import Base, TimestampMixin from acme_api.models.certificate import Certificate, CertificateStatus +from acme_api.models.certificate_revocation import CertificateRevocation, CertificateRevocationStatus from acme_api.models.event import Event from acme_api.models.renewal_attempt import RenewalAttempt from acme_api.models.webhook import WebhookConfig @@ -28,6 +29,8 @@ "Base", "Certificate", "CertificateStatus", + "CertificateRevocation", + "CertificateRevocationStatus", "Event", "RenewalAttempt", "TimestampMixin", diff --git a/acme_api/models/certificate.py b/acme_api/models/certificate.py index 446f2af..5aa85ce 100644 --- a/acme_api/models/certificate.py +++ b/acme_api/models/certificate.py @@ -175,3 +175,9 @@ def deployment_directory(self) -> str: lazy="selectin", doc="Records of each renewal attempt made against this certificate.", ) + revocations = relationship( + "acme_api.models.certificate_revocation.CertificateRevocation", + back_populates="certificate", + lazy="selectin", + doc="Domain-based ACME revocation requests for this certificate.", + ) diff --git a/acme_api/models/certificate_revocation.py b/acme_api/models/certificate_revocation.py new file mode 100644 index 0000000..a74c48c --- /dev/null +++ b/acme_api/models/certificate_revocation.py @@ -0,0 +1,57 @@ +"""Durable records for domain-based ACME certificate revocations.""" + +from __future__ import annotations + +import datetime as _dt +import enum +import uuid as _uuid + +from sqlalchemy import DateTime, Enum, ForeignKey, Integer, String, UniqueConstraint, text +from sqlalchemy.orm import Mapped, mapped_column, relationship + +from acme_api.models.base import Base + + +class CertificateRevocationStatus(enum.StrEnum): + """Terminal and in-flight states for an acme.sh revocation command.""" + + PENDING = "pending" + SUCCEEDED = "succeeded" + FAILED = "failed" + + +class CertificateRevocation(Base): + """One idempotent, domain-based revocation request for a certificate.""" + + __tablename__ = "certificate_revocations" + __table_args__ = ( + UniqueConstraint( + "certificate_id", + "idempotency_key", + name="uq_certificate_revocations_certificate_idempotency_key", + ), + ) + + id: Mapped[_uuid.UUID] = mapped_column(primary_key=True, default=_uuid.uuid4) + certificate_id: Mapped[_uuid.UUID] = mapped_column(ForeignKey("certificates.id"), index=True, nullable=False) + domain: Mapped[str] = mapped_column(String(253), nullable=False) + reason: Mapped[int | None] = mapped_column(Integer, nullable=True) + idempotency_key: Mapped[str] = mapped_column(String(255), nullable=False) + actor: Mapped[str | None] = mapped_column(String(255), nullable=True) + status: Mapped[CertificateRevocationStatus] = mapped_column( + Enum(CertificateRevocationStatus), + default=CertificateRevocationStatus.PENDING, + nullable=False, + ) + error_category: Mapped[str | None] = mapped_column(String(32), nullable=True) + error_details: Mapped[str | None] = mapped_column(String(2048), nullable=True) + created_at: Mapped[_dt.datetime] = mapped_column( + DateTime(timezone=True), server_default=text("CURRENT_TIMESTAMP"), nullable=False + ) + completed_at: Mapped[_dt.datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) + + certificate = relationship( + "acme_api.models.certificate.Certificate", + back_populates="revocations", + lazy="selectin", + ) diff --git a/acme_api/routers/certificates.py b/acme_api/routers/certificates.py index ca8e80e..3aa42a5 100644 --- a/acme_api/routers/certificates.py +++ b/acme_api/routers/certificates.py @@ -19,15 +19,19 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from acme_api.auth.hash import AuthenticatedUser from acme_api.auth.rbac import require_operator, require_readonly from acme_api.db import get_db_session from acme_api.deployer import DeploymentError from acme_api.models.certificate import Certificate, CertificateStatus +from acme_api.models.certificate_revocation import CertificateRevocation from acme_api.schemas.certificate import ( CertificateCreate, CertificateGenerationSelect, CertificateRead, CertificateRelease, + CertificateRevocationCreate, + CertificateRevocationRead, ) from acme_api.services.certificate_contracts import ( CertificateBackendUnavailableError, @@ -36,6 +40,7 @@ CertificateNotFoundError, CertificateNotRenewableError, ) +from acme_api.services.certificate_revocations import request_certificate_revocation from acme_api.services.certificates import CertificateLifecycleService router = APIRouter(prefix="/v1/certificates", tags=["Certificates"]) @@ -228,6 +233,37 @@ async def select_certificate_generation( raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc +@router.post( + "/{certificate_id}/revoke", + response_model=CertificateRevocationRead, + summary="Revoke a certificate at its ACME certificate authority", + responses={ + **_NOT_FOUND_RESPONSE, + 409: {"description": "Certificate is unissued or request conflicts."}, + }, +) +async def revoke_certificate_at_ca( + certificate_id: uuid.UUID, + payload: CertificateRevocationCreate, + request: Request, + idempotency_key: str = Header(alias="Idempotency-Key", min_length=1, max_length=255), + actor: AuthenticatedUser = Depends(require_operator), +) -> CertificateRevocation: + """Revoke the managed certificate's primary domain through acme.sh.""" + try: + return await request_certificate_revocation( + _certificate_service(request), + certificate_id, + reason=payload.reason, + idempotency_key=idempotency_key, + actor=actor.name, + ) + except CertificateNotFoundError as exc: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc + except CertificateLifecycleError as exc: + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc + + @router.delete( "/{certificate_id}", status_code=status.HTTP_204_NO_CONTENT, diff --git a/acme_api/schemas/certificate.py b/acme_api/schemas/certificate.py index 2c5ab92..8c7187c 100644 --- a/acme_api/schemas/certificate.py +++ b/acme_api/schemas/certificate.py @@ -70,6 +70,37 @@ class CertificateRelease(BaseModel): revision: int = Field(ge=1) +class CertificateRevocationCreate(BaseModel): + """Optional standardized reason for an acme.sh domain revocation.""" + + reason: int | None = Field(default=None, ge=0, le=10) + + @field_validator("reason") + @classmethod + def _reject_unused_reason(cls, reason: int | None) -> int | None: + """Reject the unused RFC 5280 reason-code value.""" + if reason == 7: + raise ValueError("Revocation reason code 7 is unused.") + return reason + + +class CertificateRevocationRead(BaseModel): + """Safe persisted state for a certificate revocation request.""" + + model_config = ConfigDict(from_attributes=True) + + id: uuid.UUID + certificate_id: uuid.UUID + domain: str + reason: int | None = None + actor: str | None = None + status: Literal["pending", "succeeded", "failed"] + error_category: Literal["transient", "terminal"] | None = None + error_details: str | None = None + created_at: datetime + completed_at: datetime | None = None + + class DnsPersistChallenge(BaseModel): """One-time account-bound TXT record required by DNS Persist mode.""" diff --git a/acme_api/services/certificate_revocations.py b/acme_api/services/certificate_revocations.py new file mode 100644 index 0000000..0528b1f --- /dev/null +++ b/acme_api/services/certificate_revocations.py @@ -0,0 +1,98 @@ +"""Domain-based acme.sh certificate revocation lifecycle operations.""" + +# pylint: disable=protected-access +from __future__ import annotations + +import datetime as dt +import uuid +from typing import TYPE_CHECKING + +from sqlalchemy import select +from sqlalchemy.exc import IntegrityError + +from acme_api.backend.acmesh_errors import AcmeShError, TransientAcmeShError +from acme_api.models.certificate import Certificate +from acme_api.models.certificate_revocation import CertificateRevocation, CertificateRevocationStatus +from acme_api.services.certificate_contracts import CertificateLifecycleError, CertificateNotFoundError +from acme_api.services.certificate_utilities import account_key_path + +if TYPE_CHECKING: + from acme_api.services.certificates import CertificateLifecycleService + + +async def request_certificate_revocation( + service: CertificateLifecycleService, + certificate_id: uuid.UUID, + *, + reason: int | None, + idempotency_key: str, + actor: str | None, +) -> CertificateRevocation: + """Run one durable idempotent acme.sh domain revocation request.""" + async with service._session_factory() as session: + certificate = await session.get(Certificate, certificate_id) + if certificate is None: + raise CertificateNotFoundError("Certificate not found.") + if certificate.expiry_date is None: + raise CertificateLifecycleError("Certificate has not been issued and cannot be revoked.") + + revocation = CertificateRevocation( + certificate_id=certificate.id, + domain=certificate.domains[0], + reason=reason, + idempotency_key=idempotency_key, + actor=actor, + ) + session.add(revocation) + try: + await session.commit() + except IntegrityError: + await session.rollback() + existing = await session.scalar( + select(CertificateRevocation).where( + CertificateRevocation.certificate_id == certificate_id, + CertificateRevocation.idempotency_key == idempotency_key, + ) + ) + if existing is None: + raise + if existing.reason != reason: + raise CertificateLifecycleError( + "Idempotency key was already used with another revocation reason." + ) from None + return existing + + account = service._acme_account(certificate.acme_account_ref) + try: + await service._backend.revoke_certificate( + revocation.domain, + reason=reason, + account_key_path=account_key_path(account), + server_url=account.server_url, + ) + except (AcmeShError, OSError) as exc: + revocation.status = CertificateRevocationStatus.FAILED + revocation.error_category = "transient" if isinstance(exc, TransientAcmeShError) else "terminal" + revocation.error_details = str(exc) + revocation.completed_at = dt.datetime.now(dt.UTC) + await service._record_event( + session, + certificate, + "certificate.revocation_failed", + {"domain": revocation.domain, "category": revocation.error_category}, + ) + await session.commit() + await session.refresh(revocation) + return revocation + + revocation.status = CertificateRevocationStatus.SUCCEEDED + revocation.completed_at = dt.datetime.now(dt.UTC) + await service._record_event( + session, + certificate, + "certificate.revoked_at_ca", + {"domain": revocation.domain, "reason": reason, "actor": actor}, + ) + await session.commit() + await session.refresh(revocation) + return revocation diff --git a/alembic/versions/a7c3e9d2b4f1_add_certificate_revocations.py b/alembic/versions/a7c3e9d2b4f1_add_certificate_revocations.py new file mode 100644 index 0000000..bdefbc5 --- /dev/null +++ b/alembic/versions/a7c3e9d2b4f1_add_certificate_revocations.py @@ -0,0 +1,65 @@ +"""Add durable acme.sh certificate revocation requests. + +Revision ID: a7c3e9d2b4f1 +Revises: f4a9b2c7d6e1 +Create Date: 2026-07-24 +""" + +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa + +from alembic import op + +revision: str = "a7c3e9d2b4f1" +down_revision: str | Sequence[str] | None = "f4a9b2c7d6e1" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + """Create the durable certificate-revocation request table.""" + op.create_table( + "certificate_revocations", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("certificate_id", sa.Uuid(), nullable=False), + sa.Column("domain", sa.String(length=253), nullable=False), + sa.Column("reason", sa.Integer(), nullable=True), + sa.Column("idempotency_key", sa.String(length=255), nullable=False), + sa.Column("actor", sa.String(length=255), nullable=True), + sa.Column( + "status", + sa.Enum( + "PENDING", + "SUCCEEDED", + "FAILED", + name="certificaterevocationstatus", + ), + nullable=False, + ), + sa.Column("error_category", sa.String(length=32), nullable=True), + sa.Column("error_details", sa.String(length=2048), nullable=True), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("CURRENT_TIMESTAMP"), + nullable=False, + ), + sa.Column("completed_at", sa.DateTime(timezone=True), nullable=True), + sa.ForeignKeyConstraint(["certificate_id"], ["certificates.id"]), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint( + "certificate_id", + "idempotency_key", + name="uq_certificate_revocations_certificate_idempotency_key", + ), + ) + op.create_index("ix_certificate_revocations_certificate_id", "certificate_revocations", ["certificate_id"]) + + +def downgrade() -> None: + """Drop the durable certificate-revocation request table.""" + op.drop_index("ix_certificate_revocations_certificate_id", table_name="certificate_revocations") + op.drop_table("certificate_revocations") diff --git a/tests/helpers/api.py b/tests/helpers/api.py index 6a778fb..57885f2 100644 --- a/tests/helpers/api.py +++ b/tests/helpers/api.py @@ -5,6 +5,7 @@ import datetime as dt from collections.abc import AsyncGenerator from contextlib import asynccontextmanager +from dataclasses import dataclass, field from pathlib import Path from fastapi import FastAPI @@ -26,6 +27,23 @@ from acme_api.models.api_key import APIKey, APIKeyRole +@dataclass +class RevocationState: + """Mutable revocation controls and observations for ``ArtifactBackend``.""" + + requests: list[tuple[str, int | None, str | None, str | None]] = field(default_factory=list) + error: AcmeShError | None = None + + +@dataclass +class PersistValueState: + """Mutable DNS Persist controls and observations for ``ArtifactBackend``.""" + + calls: int = 0 + error: AcmeShError | None = None + requests: list[tuple[str, bool]] = field(default_factory=list) + + class ArtifactBackend: """Test backend that writes deployable certificate artifacts.""" @@ -34,14 +52,33 @@ def __init__(self, root: Path) -> None: self.issue_calls = 0 self.fail_issues = False self.renew_calls = 0 - self.persist_value_calls = 0 - self.persist_value_error: AcmeShError | None = None - self.persist_value_requests: list[tuple[str, bool]] = [] + self._persist_value = PersistValueState() + self.revocation = RevocationState() async def register_account(self, email: str, server_url: str) -> AccountInfo: """Return deterministic account metadata for protocol completeness.""" return AccountInfo(key_path="account.key", email=email, server_url=server_url) + @property + def persist_value_calls(self) -> int: + """Return the number of generated DNS Persist values.""" + return self._persist_value.calls + + @property + def persist_value_error(self) -> AcmeShError | None: + """Return the configured DNS Persist generation error.""" + return self._persist_value.error + + @persist_value_error.setter + def persist_value_error(self, error: AcmeShError | None) -> None: + """Configure the DNS Persist generation error.""" + self._persist_value.error = error + + @property + def persist_value_requests(self) -> list[tuple[str, bool]]: + """Return generated DNS Persist scope requests.""" + return self._persist_value.requests + async def make_dns_persist_value( self, domain: str, @@ -52,10 +89,10 @@ async def make_dns_persist_value( ) -> str: """Return a stable, account-bound test instruction.""" del account_key_path, server_url - self.persist_value_calls += 1 - self.persist_value_requests.append((domain, wildcard)) - if self.persist_value_error is not None: - raise self.persist_value_error + self._persist_value.calls += 1 + self._persist_value.requests.append((domain, wildcard)) + if self._persist_value.error is not None: + raise self._persist_value.error return f"persist-value-for-{domain}" async def issue_certificate( @@ -81,6 +118,19 @@ async def renew_certificate( self.renew_calls += 1 return self._result(domains, "renew", "account.key") + async def revoke_certificate( + self, + domain: str, + *, + reason: int | None = None, + account_key_path: str | None = None, + server_url: str | None = None, + ) -> None: + """Record a deterministic acme.sh revocation request.""" + self.revocation.requests.append((domain, reason, account_key_path, server_url)) + if self.revocation.error is not None: + raise self.revocation.error + async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: raise NotImplementedError diff --git a/tests/integration/test_e2e_lifecycle.py b/tests/integration/test_e2e_lifecycle.py index a1c9214..d96a9bd 100644 --- a/tests/integration/test_e2e_lifecycle.py +++ b/tests/integration/test_e2e_lifecycle.py @@ -94,6 +94,17 @@ async def renew_certificate( self.renew_calls += 1 return self._result(domains, "renew", "account.key") + async def revoke_certificate( + self, + domain: str, + *, + reason: int | None = None, + account_key_path: str | None = None, + server_url: str | None = None, + ) -> None: + """Satisfy the ACME backend protocol for lifecycle integration tests.""" + del domain, reason, account_key_path, server_url + async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: """Return deterministic expiry metadata for a certificate path.""" return CertExpiry( diff --git a/tests/unit/test_api_phase5.py b/tests/unit/test_api_phase5.py index 043b0d0..1add002 100644 --- a/tests/unit/test_api_phase5.py +++ b/tests/unit/test_api_phase5.py @@ -52,6 +52,78 @@ def test_certificate_lifecycle_endpoints(tmp_path: Path) -> None: assert revoked.json()["status"] == "revoked" +def test_certificate_authority_revocation_is_idempotent(tmp_path: Path) -> None: + """Revoke one issued domain through the backend without changing local deployment.""" + headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "revoke-once"} + app = make_api_app(tmp_path) + backend = app.state.acme_backend + assert isinstance(backend, ArtifactBackend) + with TestClient(app) as client: + created = client.post( + "/v1/certificates", + headers=headers, + json={ + "name": "revoke-authority-cert", + "domains": ["example.com"], + "acme_account_ref": "letsencrypt-production", + "dns_provider_ref": "cloudflare-main", + }, + ) + certificate_id = created.json()["id"] + revoked = client.post( + f"/v1/certificates/{certificate_id}/revoke", + headers=headers, + json={"reason": 1}, + ) + repeated = client.post( + f"/v1/certificates/{certificate_id}/revoke", + headers=headers, + json={"reason": 1}, + ) + + assert revoked.status_code == 200 + assert revoked.json()["status"] == "succeeded" + assert repeated.json()["id"] == revoked.json()["id"] + assert backend.revocation.requests == [("example.com", 1, None, "https://acme-v02.api.letsencrypt.org/directory")] + + +def test_certificate_authority_revocation_persists_terminal_failure(tmp_path: Path) -> None: + """Keep a terminal acme.sh revocation result durable and idempotent.""" + headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "terminal-revoke"} + app = make_api_app(tmp_path) + backend = app.state.acme_backend + assert isinstance(backend, ArtifactBackend) + backend.revocation.error = TerminalAcmeShError("certificate cannot be revoked") + with TestClient(app) as client: + created = client.post( + "/v1/certificates", + headers=headers, + json={ + "name": "terminal-revoke-cert", + "domains": ["example.net"], + "acme_account_ref": "letsencrypt-production", + "dns_provider_ref": "cloudflare-main", + }, + ) + certificate_id = created.json()["id"] + failed = client.post( + f"/v1/certificates/{certificate_id}/revoke", + headers=headers, + json={"reason": 4}, + ) + repeated = client.post( + f"/v1/certificates/{certificate_id}/revoke", + headers=headers, + json={"reason": 4}, + ) + + assert failed.status_code == 200 + assert failed.json()["status"] == "failed" + assert failed.json()["error_category"] == "terminal" + assert repeated.json()["id"] == failed.json()["id"] + assert len(backend.revocation.requests) == 1 + + def test_dns_persist_lifecycle_endpoints(tmp_path: Path) -> None: """DNS Persist requests wait for explicit authorization and retain instructions.""" headers = {"Authorization": "Bearer operator-key-12345"} diff --git a/tests/unit/test_backend_commands.py b/tests/unit/test_backend_commands.py index 7ea1ebe..868d672 100644 --- a/tests/unit/test_backend_commands.py +++ b/tests/unit/test_backend_commands.py @@ -242,6 +242,33 @@ async def test_renew_certificate_force(self, backend: AcmeShBackend) -> None: assert result.domains == ["example.com", "www.example.com"] +class TestRevokeCertificate: + """Verify revocations use acme.sh's documented domain interface.""" + + @pytest.mark.anyio + async def test_revoke_certificate_with_reason_and_account( + self, + backend: AcmeShBackend, + ) -> None: + """Pass the selected account and optional standardized reason to acme.sh.""" + with patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_run: + mock_run.return_value = successful_process() + + await backend.revoke_certificate( + "example.com", + reason=1, + account_key_path="/acmesh/account.key", + server_url="https://ca.example/directory", + ) + + call_args = mock_run.call_args.args + assert "--revoke" in call_args + assert has_flag_pair(call_args, "--domain", "example.com") + assert has_flag_pair(call_args, "--revoke-reason", "1") + assert has_flag_pair(call_args, "--accountkey-file", "/acmesh/account.key") + assert has_flag_pair(call_args, "--server", "https://ca.example/directory") + + class TestParsing: """Verify supported acme.sh output parsing variants.""" diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 80e6475..cbb30b1 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -81,6 +81,17 @@ async def renew_certificate(self, domains: list[str], force_renewal: bool = Fals domains=domains, ) + async def revoke_certificate( + self, + domain: str, + *, + reason: int | None = None, + account_key_path: str | None = None, + server_url: str | None = None, + ) -> None: + """Satisfy the ACME backend protocol for scheduler-only tests.""" + del domain, reason, account_key_path, server_url + async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: """Return deterministic expiry info.""" result = _cert_expiry() From a034329f32092435b19a0fff6263ce44f3a54327 Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 02:04:19 +0100 Subject: [PATCH 02/10] resume pending revocation idempotency records --- acme_api/services/certificate_revocations.py | 6 ++-- tests/helpers/api.py | 2 +- tests/unit/test_api_phase5.py | 37 ++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/acme_api/services/certificate_revocations.py b/acme_api/services/certificate_revocations.py index 0528b1f..6f6f41b 100644 --- a/acme_api/services/certificate_revocations.py +++ b/acme_api/services/certificate_revocations.py @@ -60,7 +60,9 @@ async def request_certificate_revocation( raise CertificateLifecycleError( "Idempotency key was already used with another revocation reason." ) from None - return existing + if existing.status is not CertificateRevocationStatus.PENDING: + return existing + revocation = existing account = service._acme_account(certificate.acme_account_ref) try: @@ -91,7 +93,7 @@ async def request_certificate_revocation( session, certificate, "certificate.revoked_at_ca", - {"domain": revocation.domain, "reason": reason, "actor": actor}, + {"domain": revocation.domain, "reason": revocation.reason, "actor": revocation.actor}, ) await session.commit() await session.refresh(revocation) diff --git a/tests/helpers/api.py b/tests/helpers/api.py index 57885f2..a58d401 100644 --- a/tests/helpers/api.py +++ b/tests/helpers/api.py @@ -32,7 +32,7 @@ class RevocationState: """Mutable revocation controls and observations for ``ArtifactBackend``.""" requests: list[tuple[str, int | None, str | None, str | None]] = field(default_factory=list) - error: AcmeShError | None = None + error: Exception | None = None @dataclass diff --git a/tests/unit/test_api_phase5.py b/tests/unit/test_api_phase5.py index 1add002..d1aaca9 100644 --- a/tests/unit/test_api_phase5.py +++ b/tests/unit/test_api_phase5.py @@ -124,6 +124,43 @@ def test_certificate_authority_revocation_persists_terminal_failure(tmp_path: Pa assert len(backend.revocation.requests) == 1 +def test_certificate_authority_revocation_resumes_persisted_pending_request(tmp_path: Path) -> None: + """Retry a request after an interrupted acme.sh invocation left it pending.""" + headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "resume-pending"} + app = make_api_app(tmp_path) + backend = app.state.acme_backend + assert isinstance(backend, ArtifactBackend) + backend.revocation.error = RuntimeError("worker stopped after request persistence") + with TestClient(app, raise_server_exceptions=False) as client: + created = client.post( + "/v1/certificates", + headers=headers, + json={ + "name": "resume-pending-revoke-cert", + "domains": ["example.org"], + "acme_account_ref": "letsencrypt-production", + "dns_provider_ref": "cloudflare-main", + }, + ) + certificate_id = created.json()["id"] + interrupted = client.post( + f"/v1/certificates/{certificate_id}/revoke", + headers=headers, + json={"reason": 5}, + ) + backend.revocation.error = None + resumed = client.post( + f"/v1/certificates/{certificate_id}/revoke", + headers=headers, + json={"reason": 5}, + ) + + assert interrupted.status_code == 500 + assert resumed.status_code == 200 + assert resumed.json()["status"] == "succeeded" + assert len(backend.revocation.requests) == 2 + + def test_dns_persist_lifecycle_endpoints(tmp_path: Path) -> None: """DNS Persist requests wait for explicit authorization and retain instructions.""" headers = {"Authorization": "Bearer operator-key-12345"} From 895bec86b15e35762c485edfe1988e9bebb2affd Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 02:27:30 +0100 Subject: [PATCH 03/10] fix: harden acme.sh certificate revocation per review notes --- acme_api/backend/acmesh_backend.py | 3 + acme_api/backend/mock_backend.py | 3 +- acme_api/backend/protocol.py | 1 + acme_api/models/certificate_revocation.py | 1 + acme_api/services/certificate_revocations.py | 43 ++++++- ...c8d4e1f2a6b9_add_revocation_claim_lease.py | 28 +++++ tests/helpers/api.py | 2 + tests/integration/test_e2e_lifecycle.py | 3 +- tests/unit/test_api_phase5.py | 109 ------------------ tests/unit/test_backend_commands.py | 11 ++ tests/unit/test_certificate_revocations.py | 106 +++++++++++++++++ tests/unit/test_scheduler.py | 3 +- 12 files changed, 197 insertions(+), 116 deletions(-) create mode 100644 alembic/versions/c8d4e1f2a6b9_add_revocation_claim_lease.py create mode 100644 tests/unit/test_certificate_revocations.py diff --git a/acme_api/backend/acmesh_backend.py b/acme_api/backend/acmesh_backend.py index b9fcd1d..376a9fe 100644 --- a/acme_api/backend/acmesh_backend.py +++ b/acme_api/backend/acmesh_backend.py @@ -243,11 +243,14 @@ async def revoke_certificate( domain: str, *, reason: int | None = None, + key_algorithm: str = "ecdsa", account_key_path: str | None = None, server_url: str | None = None, ) -> None: """Revoke acme.sh's certificate for ``domain`` using the configured account.""" args = ["--revoke", "--domain", domain] + if key_algorithm == "ecdsa": + args.append("--ecc") if reason is not None: args += ["--revoke-reason", str(reason)] if server_url is not None: diff --git a/acme_api/backend/mock_backend.py b/acme_api/backend/mock_backend.py index cef895c..0004f63 100644 --- a/acme_api/backend/mock_backend.py +++ b/acme_api/backend/mock_backend.py @@ -66,11 +66,12 @@ async def revoke_certificate( domain: str, *, reason: int | None = None, + key_algorithm: str = "ecdsa", account_key_path: str | None = None, server_url: str | None = None, ) -> None: """Accept a deterministic revocation without contacting an ACME server.""" - del domain, reason, account_key_path, server_url + del domain, reason, key_algorithm, account_key_path, server_url async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: """Return a predictable expiry for the supplied certificate path.""" diff --git a/acme_api/backend/protocol.py b/acme_api/backend/protocol.py index a9e2f57..c2b2ec0 100644 --- a/acme_api/backend/protocol.py +++ b/acme_api/backend/protocol.py @@ -53,6 +53,7 @@ async def revoke_certificate( domain: str, *, reason: int | None = None, + key_algorithm: str = "ecdsa", account_key_path: str | None = None, server_url: str | None = None, ) -> None: ... diff --git a/acme_api/models/certificate_revocation.py b/acme_api/models/certificate_revocation.py index a74c48c..cbd9e48 100644 --- a/acme_api/models/certificate_revocation.py +++ b/acme_api/models/certificate_revocation.py @@ -48,6 +48,7 @@ class CertificateRevocation(Base): created_at: Mapped[_dt.datetime] = mapped_column( DateTime(timezone=True), server_default=text("CURRENT_TIMESTAMP"), nullable=False ) + attempt_started_at: Mapped[_dt.datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) completed_at: Mapped[_dt.datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) certificate = relationship( diff --git a/acme_api/services/certificate_revocations.py b/acme_api/services/certificate_revocations.py index 6f6f41b..c697c4d 100644 --- a/acme_api/services/certificate_revocations.py +++ b/acme_api/services/certificate_revocations.py @@ -5,10 +5,12 @@ import datetime as dt import uuid -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any, cast -from sqlalchemy import select +from sqlalchemy import or_, select, update +from sqlalchemy.engine import CursorResult from sqlalchemy.exc import IntegrityError +from sqlalchemy.ext.asyncio import AsyncSession from acme_api.backend.acmesh_errors import AcmeShError, TransientAcmeShError from acme_api.models.certificate import Certificate @@ -19,6 +21,8 @@ if TYPE_CHECKING: from acme_api.services.certificates import CertificateLifecycleService +_REVOCATION_LEASE = dt.timedelta(minutes=10) + async def request_certificate_revocation( service: CertificateLifecycleService, @@ -28,7 +32,7 @@ async def request_certificate_revocation( idempotency_key: str, actor: str | None, ) -> CertificateRevocation: - """Run one durable idempotent acme.sh domain revocation request.""" + """Run one durable, claimed, idempotent acme.sh revocation request.""" async with service._session_factory() as session: certificate = await session.get(Certificate, certificate_id) if certificate is None: @@ -48,6 +52,9 @@ async def request_certificate_revocation( await session.commit() except IntegrityError: await session.rollback() + certificate = await session.get(Certificate, certificate_id) + if certificate is None: + raise CertificateNotFoundError("Certificate not found.") from None existing = await session.scalar( select(CertificateRevocation).where( CertificateRevocation.certificate_id == certificate_id, @@ -64,11 +71,15 @@ async def request_certificate_revocation( return existing revocation = existing + if not await _claim_pending_revocation(session, revocation): + return revocation + account = service._acme_account(certificate.acme_account_ref) try: await service._backend.revoke_certificate( revocation.domain, - reason=reason, + reason=revocation.reason, + key_algorithm=certificate.key_algorithm, account_key_path=account_key_path(account), server_url=account.server_url, ) @@ -98,3 +109,27 @@ async def request_certificate_revocation( await session.commit() await session.refresh(revocation) return revocation + + +async def _claim_pending_revocation(session: AsyncSession, revocation: CertificateRevocation) -> bool: + """Atomically lease a pending request, allowing recovery after a dead worker.""" + now = dt.datetime.now(dt.UTC) + result = await session.execute( + update(CertificateRevocation) + .where( + CertificateRevocation.id == revocation.id, + CertificateRevocation.status == CertificateRevocationStatus.PENDING, + or_( + CertificateRevocation.attempt_started_at.is_(None), + CertificateRevocation.attempt_started_at < now - _REVOCATION_LEASE, + ), + ) + .values(attempt_started_at=now) + .execution_options(synchronize_session=False) + ) + await session.commit() + if cast(CursorResult[Any], result).rowcount != 1: + await session.refresh(revocation) + return False + await session.refresh(revocation) + return True diff --git a/alembic/versions/c8d4e1f2a6b9_add_revocation_claim_lease.py b/alembic/versions/c8d4e1f2a6b9_add_revocation_claim_lease.py new file mode 100644 index 0000000..5a7ca0c --- /dev/null +++ b/alembic/versions/c8d4e1f2a6b9_add_revocation_claim_lease.py @@ -0,0 +1,28 @@ +"""Add a lease for resumable certificate revocation requests. + +Revision ID: c8d4e1f2a6b9 +Revises: a7c3e9d2b4f1 +Create Date: 2026-07-24 +""" + +from __future__ import annotations + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "c8d4e1f2a6b9" +down_revision: str | Sequence[str] | None = "a7c3e9d2b4f1" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + """Add the timestamp that leases a pending revocation to one worker.""" + op.add_column("certificate_revocations", sa.Column("attempt_started_at", sa.DateTime(timezone=True), nullable=True)) + + +def downgrade() -> None: + """Remove the revocation worker lease timestamp.""" + op.drop_column("certificate_revocations", "attempt_started_at") diff --git a/tests/helpers/api.py b/tests/helpers/api.py index a58d401..d5205ee 100644 --- a/tests/helpers/api.py +++ b/tests/helpers/api.py @@ -123,10 +123,12 @@ async def revoke_certificate( domain: str, *, reason: int | None = None, + key_algorithm: str = "ecdsa", account_key_path: str | None = None, server_url: str | None = None, ) -> None: """Record a deterministic acme.sh revocation request.""" + del key_algorithm self.revocation.requests.append((domain, reason, account_key_path, server_url)) if self.revocation.error is not None: raise self.revocation.error diff --git a/tests/integration/test_e2e_lifecycle.py b/tests/integration/test_e2e_lifecycle.py index d96a9bd..4ef105e 100644 --- a/tests/integration/test_e2e_lifecycle.py +++ b/tests/integration/test_e2e_lifecycle.py @@ -99,11 +99,12 @@ async def revoke_certificate( domain: str, *, reason: int | None = None, + key_algorithm: str = "ecdsa", account_key_path: str | None = None, server_url: str | None = None, ) -> None: """Satisfy the ACME backend protocol for lifecycle integration tests.""" - del domain, reason, account_key_path, server_url + del domain, reason, key_algorithm, account_key_path, server_url async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: """Return deterministic expiry metadata for a certificate path.""" diff --git a/tests/unit/test_api_phase5.py b/tests/unit/test_api_phase5.py index d1aaca9..043b0d0 100644 --- a/tests/unit/test_api_phase5.py +++ b/tests/unit/test_api_phase5.py @@ -52,115 +52,6 @@ def test_certificate_lifecycle_endpoints(tmp_path: Path) -> None: assert revoked.json()["status"] == "revoked" -def test_certificate_authority_revocation_is_idempotent(tmp_path: Path) -> None: - """Revoke one issued domain through the backend without changing local deployment.""" - headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "revoke-once"} - app = make_api_app(tmp_path) - backend = app.state.acme_backend - assert isinstance(backend, ArtifactBackend) - with TestClient(app) as client: - created = client.post( - "/v1/certificates", - headers=headers, - json={ - "name": "revoke-authority-cert", - "domains": ["example.com"], - "acme_account_ref": "letsencrypt-production", - "dns_provider_ref": "cloudflare-main", - }, - ) - certificate_id = created.json()["id"] - revoked = client.post( - f"/v1/certificates/{certificate_id}/revoke", - headers=headers, - json={"reason": 1}, - ) - repeated = client.post( - f"/v1/certificates/{certificate_id}/revoke", - headers=headers, - json={"reason": 1}, - ) - - assert revoked.status_code == 200 - assert revoked.json()["status"] == "succeeded" - assert repeated.json()["id"] == revoked.json()["id"] - assert backend.revocation.requests == [("example.com", 1, None, "https://acme-v02.api.letsencrypt.org/directory")] - - -def test_certificate_authority_revocation_persists_terminal_failure(tmp_path: Path) -> None: - """Keep a terminal acme.sh revocation result durable and idempotent.""" - headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "terminal-revoke"} - app = make_api_app(tmp_path) - backend = app.state.acme_backend - assert isinstance(backend, ArtifactBackend) - backend.revocation.error = TerminalAcmeShError("certificate cannot be revoked") - with TestClient(app) as client: - created = client.post( - "/v1/certificates", - headers=headers, - json={ - "name": "terminal-revoke-cert", - "domains": ["example.net"], - "acme_account_ref": "letsencrypt-production", - "dns_provider_ref": "cloudflare-main", - }, - ) - certificate_id = created.json()["id"] - failed = client.post( - f"/v1/certificates/{certificate_id}/revoke", - headers=headers, - json={"reason": 4}, - ) - repeated = client.post( - f"/v1/certificates/{certificate_id}/revoke", - headers=headers, - json={"reason": 4}, - ) - - assert failed.status_code == 200 - assert failed.json()["status"] == "failed" - assert failed.json()["error_category"] == "terminal" - assert repeated.json()["id"] == failed.json()["id"] - assert len(backend.revocation.requests) == 1 - - -def test_certificate_authority_revocation_resumes_persisted_pending_request(tmp_path: Path) -> None: - """Retry a request after an interrupted acme.sh invocation left it pending.""" - headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "resume-pending"} - app = make_api_app(tmp_path) - backend = app.state.acme_backend - assert isinstance(backend, ArtifactBackend) - backend.revocation.error = RuntimeError("worker stopped after request persistence") - with TestClient(app, raise_server_exceptions=False) as client: - created = client.post( - "/v1/certificates", - headers=headers, - json={ - "name": "resume-pending-revoke-cert", - "domains": ["example.org"], - "acme_account_ref": "letsencrypt-production", - "dns_provider_ref": "cloudflare-main", - }, - ) - certificate_id = created.json()["id"] - interrupted = client.post( - f"/v1/certificates/{certificate_id}/revoke", - headers=headers, - json={"reason": 5}, - ) - backend.revocation.error = None - resumed = client.post( - f"/v1/certificates/{certificate_id}/revoke", - headers=headers, - json={"reason": 5}, - ) - - assert interrupted.status_code == 500 - assert resumed.status_code == 200 - assert resumed.json()["status"] == "succeeded" - assert len(backend.revocation.requests) == 2 - - def test_dns_persist_lifecycle_endpoints(tmp_path: Path) -> None: """DNS Persist requests wait for explicit authorization and retain instructions.""" headers = {"Authorization": "Bearer operator-key-12345"} diff --git a/tests/unit/test_backend_commands.py b/tests/unit/test_backend_commands.py index 868d672..80bbe4c 100644 --- a/tests/unit/test_backend_commands.py +++ b/tests/unit/test_backend_commands.py @@ -267,6 +267,17 @@ async def test_revoke_certificate_with_reason_and_account( assert has_flag_pair(call_args, "--revoke-reason", "1") assert has_flag_pair(call_args, "--accountkey-file", "/acmesh/account.key") assert has_flag_pair(call_args, "--server", "https://ca.example/directory") + assert "--ecc" in call_args + + @pytest.mark.anyio + async def test_revoke_rsa_certificate_without_ecc_flag(self, backend: AcmeShBackend) -> None: + """Select acme.sh's RSA certificate slot when the managed key is RSA.""" + with patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_run: + mock_run.return_value = successful_process() + + await backend.revoke_certificate("example.com", key_algorithm="rsa-2048") + + assert "--ecc" not in mock_run.call_args.args class TestParsing: diff --git a/tests/unit/test_certificate_revocations.py b/tests/unit/test_certificate_revocations.py new file mode 100644 index 0000000..a3ec329 --- /dev/null +++ b/tests/unit/test_certificate_revocations.py @@ -0,0 +1,106 @@ +"""Certificate-authority revocation API contract tests.""" + +from __future__ import annotations + +from datetime import timedelta +from pathlib import Path + +import pytest +from fastapi.testclient import TestClient + +from acme_api.backend.acmesh_errors import TerminalAcmeShError +from acme_api.services import certificate_revocations +from tests.helpers.api import ArtifactBackend, make_api_app + + +def test_certificate_authority_revocation_is_idempotent(tmp_path: Path) -> None: + """Revoke one issued domain through the backend without changing local deployment.""" + headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "revoke-once"} + app = make_api_app(tmp_path) + backend = app.state.acme_backend + assert isinstance(backend, ArtifactBackend) + with TestClient(app) as client: + created = client.post( + "/v1/certificates", + headers=headers, + json={ + "name": "revoke-authority-cert", + "domains": ["example.com"], + "acme_account_ref": "letsencrypt-production", + "dns_provider_ref": "cloudflare-main", + }, + ) + certificate_id = created.json()["id"] + revoked = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 1}) + repeated = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 1}) + + assert revoked.status_code == 200 + assert revoked.json()["status"] == "succeeded" + assert repeated.json()["id"] == revoked.json()["id"] + assert backend.revocation.requests == [("example.com", 1, None, "https://acme-v02.api.letsencrypt.org/directory")] + + +def test_certificate_authority_revocation_persists_terminal_failure(tmp_path: Path) -> None: + """Keep a terminal acme.sh revocation result durable and idempotent.""" + headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "terminal-revoke"} + app = make_api_app(tmp_path) + backend = app.state.acme_backend + assert isinstance(backend, ArtifactBackend) + backend.revocation.error = TerminalAcmeShError("certificate cannot be revoked") + with TestClient(app) as client: + created = client.post( + "/v1/certificates", + headers=headers, + json={ + "name": "terminal-revoke-cert", + "domains": ["example.net"], + "acme_account_ref": "letsencrypt-production", + "dns_provider_ref": "cloudflare-main", + }, + ) + certificate_id = created.json()["id"] + failed = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 4}) + repeated = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 4}) + + assert failed.status_code == 200 + assert failed.json()["status"] == "failed" + assert failed.json()["error_category"] == "terminal" + assert repeated.json()["id"] == failed.json()["id"] + assert len(backend.revocation.requests) == 1 + + +def test_certificate_authority_revocation_resumes_expired_pending_request( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Retry a request after a dead worker's revocation lease expires.""" + headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "resume-pending"} + app = make_api_app(tmp_path) + backend = app.state.acme_backend + assert isinstance(backend, ArtifactBackend) + backend.revocation.error = RuntimeError("worker stopped after request persistence") + with TestClient(app, raise_server_exceptions=False) as client: + created = client.post( + "/v1/certificates", + headers=headers, + json={ + "name": "resume-pending-revoke-cert", + "domains": ["example.org"], + "acme_account_ref": "letsencrypt-production", + "dns_provider_ref": "cloudflare-main", + }, + ) + certificate_id = created.json()["id"] + interrupted = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 5}) + backend.revocation.error = None + in_progress = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 5}) + assert in_progress.status_code == 200 + assert in_progress.json()["status"] == "pending" + assert len(backend.revocation.requests) == 1 + monkeypatch.setattr(certificate_revocations, "_REVOCATION_LEASE", timedelta()) + resumed = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 5}) + + assert interrupted.status_code == 500 + assert resumed.status_code == 200 + assert resumed.json()["status"] == "succeeded" + assert len(backend.revocation.requests) == 2 diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index cbb30b1..244e246 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -86,11 +86,12 @@ async def revoke_certificate( domain: str, *, reason: int | None = None, + key_algorithm: str = "ecdsa", account_key_path: str | None = None, server_url: str | None = None, ) -> None: """Satisfy the ACME backend protocol for scheduler-only tests.""" - del domain, reason, account_key_path, server_url + del domain, reason, key_algorithm, account_key_path, server_url async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: """Return deterministic expiry info.""" From df7a5b6a749d10d6e3fb1ae46e93c9f307265711 Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 02:52:54 +0100 Subject: [PATCH 04/10] revocation webhook and handling missing account config --- acme_api/services/certificate_revocations.py | 6 ++- tests/unit/test_certificate_revocations.py | 49 +++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/acme_api/services/certificate_revocations.py b/acme_api/services/certificate_revocations.py index c697c4d..69005f0 100644 --- a/acme_api/services/certificate_revocations.py +++ b/acme_api/services/certificate_revocations.py @@ -13,6 +13,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from acme_api.backend.acmesh_errors import AcmeShError, TransientAcmeShError +from acme_api.deployer import DeploymentError from acme_api.models.certificate import Certificate from acme_api.models.certificate_revocation import CertificateRevocation, CertificateRevocationStatus from acme_api.services.certificate_contracts import CertificateLifecycleError, CertificateNotFoundError @@ -74,8 +75,8 @@ async def request_certificate_revocation( if not await _claim_pending_revocation(session, revocation): return revocation - account = service._acme_account(certificate.acme_account_ref) try: + account = service._acme_account(certificate.acme_account_ref) await service._backend.revoke_certificate( revocation.domain, reason=revocation.reason, @@ -83,7 +84,7 @@ async def request_certificate_revocation( account_key_path=account_key_path(account), server_url=account.server_url, ) - except (AcmeShError, OSError) as exc: + except (AcmeShError, DeploymentError, OSError) as exc: revocation.status = CertificateRevocationStatus.FAILED revocation.error_category = "transient" if isinstance(exc, TransientAcmeShError) else "terminal" revocation.error_details = str(exc) @@ -108,6 +109,7 @@ async def request_certificate_revocation( ) await session.commit() await session.refresh(revocation) + await service._dispatch_webhook(session, "certificate.revoked_at_ca", certificate) return revocation diff --git a/tests/unit/test_certificate_revocations.py b/tests/unit/test_certificate_revocations.py index a3ec329..d9234ff 100644 --- a/tests/unit/test_certificate_revocations.py +++ b/tests/unit/test_certificate_revocations.py @@ -4,16 +4,22 @@ from datetime import timedelta from pathlib import Path +from typing import NoReturn +from unittest.mock import ANY, AsyncMock import pytest from fastapi.testclient import TestClient from acme_api.backend.acmesh_errors import TerminalAcmeShError +from acme_api.deployer import DeploymentError from acme_api.services import certificate_revocations from tests.helpers.api import ArtifactBackend, make_api_app -def test_certificate_authority_revocation_is_idempotent(tmp_path: Path) -> None: +def test_certificate_authority_revocation_is_idempotent( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: """Revoke one issued domain through the backend without changing local deployment.""" headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "revoke-once"} app = make_api_app(tmp_path) @@ -31,6 +37,8 @@ def test_certificate_authority_revocation_is_idempotent(tmp_path: Path) -> None: }, ) certificate_id = created.json()["id"] + dispatched = AsyncMock() + monkeypatch.setattr(app.state.certificate_service, "_dispatch_webhook", dispatched) revoked = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 1}) repeated = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 1}) @@ -38,6 +46,7 @@ def test_certificate_authority_revocation_is_idempotent(tmp_path: Path) -> None: assert revoked.json()["status"] == "succeeded" assert repeated.json()["id"] == revoked.json()["id"] assert backend.revocation.requests == [("example.com", 1, None, "https://acme-v02.api.letsencrypt.org/directory")] + dispatched.assert_awaited_once_with(ANY, "certificate.revoked_at_ca", ANY) def test_certificate_authority_revocation_persists_terminal_failure(tmp_path: Path) -> None: @@ -69,6 +78,44 @@ def test_certificate_authority_revocation_persists_terminal_failure(tmp_path: Pa assert len(backend.revocation.requests) == 1 +def test_certificate_authority_revocation_persists_missing_account_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Persist a missing configured ACME account as a terminal revocation failure.""" + headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "missing-account"} + app = make_api_app(tmp_path) + backend = app.state.acme_backend + assert isinstance(backend, ArtifactBackend) + with TestClient(app) as client: + created = client.post( + "/v1/certificates", + headers=headers, + json={ + "name": "missing-account-revoke-cert", + "domains": ["missing-account.example"], + "acme_account_ref": "letsencrypt-production", + "dns_provider_ref": "cloudflare-main", + }, + ) + certificate_id = created.json()["id"] + + def missing_account(_name: str) -> NoReturn: + raise DeploymentError("ACME account not configured: letsencrypt-production") + + monkeypatch.setattr(app.state.certificate_service, "_acme_account", missing_account) + failed = client.post( + f"/v1/certificates/{certificate_id}/revoke", + headers=headers, + json={"reason": 1}, + ) + + assert failed.status_code == 200 + assert failed.json()["status"] == "failed" + assert failed.json()["error_category"] == "terminal" + assert not backend.revocation.requests + + def test_certificate_authority_revocation_resumes_expired_pending_request( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, From 5ac7332b223a7d74ad35abc0419572098db18920 Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 03:08:24 +0100 Subject: [PATCH 05/10] fix: support bodyless certificate revocations --- acme_api/routers/certificates.py | 4 ++-- acme_api/services/certificate_revocations.py | 2 ++ tests/unit/test_certificate_revocations.py | 9 ++++++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/acme_api/routers/certificates.py b/acme_api/routers/certificates.py index 3aa42a5..5382e1b 100644 --- a/acme_api/routers/certificates.py +++ b/acme_api/routers/certificates.py @@ -244,8 +244,8 @@ async def select_certificate_generation( ) async def revoke_certificate_at_ca( certificate_id: uuid.UUID, - payload: CertificateRevocationCreate, request: Request, + payload: CertificateRevocationCreate | None = None, idempotency_key: str = Header(alias="Idempotency-Key", min_length=1, max_length=255), actor: AuthenticatedUser = Depends(require_operator), ) -> CertificateRevocation: @@ -254,7 +254,7 @@ async def revoke_certificate_at_ca( return await request_certificate_revocation( _certificate_service(request), certificate_id, - reason=payload.reason, + reason=payload.reason if payload is not None else None, idempotency_key=idempotency_key, actor=actor.name, ) diff --git a/acme_api/services/certificate_revocations.py b/acme_api/services/certificate_revocations.py index 69005f0..421e020 100644 --- a/acme_api/services/certificate_revocations.py +++ b/acme_api/services/certificate_revocations.py @@ -1,5 +1,7 @@ """Domain-based acme.sh certificate revocation lifecycle operations.""" +# This orchestration helper intentionally uses lifecycle internals to share its +# configured session, backend, event recording, and webhook dispatch boundary. # pylint: disable=protected-access from __future__ import annotations diff --git a/tests/unit/test_certificate_revocations.py b/tests/unit/test_certificate_revocations.py index d9234ff..9ed1f9e 100644 --- a/tests/unit/test_certificate_revocations.py +++ b/tests/unit/test_certificate_revocations.py @@ -39,13 +39,16 @@ def test_certificate_authority_revocation_is_idempotent( certificate_id = created.json()["id"] dispatched = AsyncMock() monkeypatch.setattr(app.state.certificate_service, "_dispatch_webhook", dispatched) - revoked = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 1}) - repeated = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 1}) + revoked = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers) + repeated = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers) assert revoked.status_code == 200 assert revoked.json()["status"] == "succeeded" + assert revoked.json()["reason"] is None assert repeated.json()["id"] == revoked.json()["id"] - assert backend.revocation.requests == [("example.com", 1, None, "https://acme-v02.api.letsencrypt.org/directory")] + assert backend.revocation.requests == [ + ("example.com", None, None, "https://acme-v02.api.letsencrypt.org/directory") + ] dispatched.assert_awaited_once_with(ANY, "certificate.revoked_at_ca", ANY) From 80e3c9b17af6f61798c36d9d8cfa231a11fb5810 Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 03:39:51 +0100 Subject: [PATCH 06/10] note revoke limits imposed by the current acme.sh revoke implementation --- README.md | 7 +++++++ acme_api/routers/certificates.py | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/README.md b/README.md index 0c1d107..931710e 100644 --- a/README.md +++ b/README.md @@ -218,6 +218,13 @@ disable renewal, or otherwise modify the local certificate record. Reusing the same key returns the durable original result without another acme.sh command. Reasons `0` through `10` are accepted except `7`, which RFC 5280 leaves unused. +acme.sh selects the certificate it revokes from its managed domain and key-type +slot; it does not accept a certificate file, serial number, fingerprint, or +deployment generation. Generation selection only repoints acme.api's deployed +artifact view and does not modify acme.sh's managed certificate. Consequently, +CA revocation through this endpoint cannot target a retained historical +generation independently of the certificate currently managed by acme.sh. + ## Certificate Deployment Successful issuance and renewal deploy artifacts under the `deployment_directory` diff --git a/acme_api/routers/certificates.py b/acme_api/routers/certificates.py index 5382e1b..f9f12ac 100644 --- a/acme_api/routers/certificates.py +++ b/acme_api/routers/certificates.py @@ -237,6 +237,10 @@ async def select_certificate_generation( "/{certificate_id}/revoke", response_model=CertificateRevocationRead, summary="Revoke a certificate at its ACME certificate authority", + description=( + "Revokes the certificate selected by acme.sh's managed primary-domain and key-type slot. " + "It cannot select an acme.api deployment generation or historical artifact." + ), responses={ **_NOT_FOUND_RESPONSE, 409: {"description": "Certificate is unissued or request conflicts."}, From 1ca6c94dd4df6f10066324da1a582df86e7c8ab1 Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 04:33:41 +0100 Subject: [PATCH 07/10] accept alreadyRevoked as idempotent success --- acme_api/backend/acmesh_backend.py | 3 ++- tests/unit/test_backend_commands.py | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/acme_api/backend/acmesh_backend.py b/acme_api/backend/acmesh_backend.py index 376a9fe..68dc54a 100644 --- a/acme_api/backend/acmesh_backend.py +++ b/acme_api/backend/acmesh_backend.py @@ -260,7 +260,8 @@ async def revoke_certificate( try: await self._run(args) except TerminalAcmeShError as exc: - if "already revoked" not in exc.stderr.lower() and "already revoked" not in str(exc).lower(): + error_text = f"{exc.stderr}\n{exc}".lower() + if "already revoked" not in error_text and "alreadyrevoked" not in error_text: raise async def get_certificate_expiry(self, cert_path: str) -> CertExpiry: diff --git a/tests/unit/test_backend_commands.py b/tests/unit/test_backend_commands.py index 80bbe4c..64193f2 100644 --- a/tests/unit/test_backend_commands.py +++ b/tests/unit/test_backend_commands.py @@ -279,6 +279,16 @@ async def test_revoke_rsa_certificate_without_ecc_flag(self, backend: AcmeShBack assert "--ecc" not in mock_run.call_args.args + @pytest.mark.anyio + async def test_revoke_already_revoked_problem_is_idempotent(self, backend: AcmeShBackend) -> None: + """Treat the RFC 8555 alreadyRevoked problem type as a completed revocation.""" + with patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_run: + mock_run.return_value = failed_process( + "urn:ietf:params:acme:error:alreadyRevoked: certificate is already revoked" + ) + + await backend.revoke_certificate("example.com") + class TestParsing: """Verify supported acme.sh output parsing variants.""" From d77814ec2788454306224a5c2e645c2b3f39dc0f Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 04:49:11 +0100 Subject: [PATCH 08/10] use correct key slot when revoking, dispatch revoke fail webhooks --- acme_api/backend/acmesh_backend.py | 3 +++ acme_api/services/certificate_revocations.py | 1 + acme_api/services/certificates.py | 1 + docs/outline.md | 4 ++++ tests/helpers/api.py | 3 ++- tests/unit/test_backend_commands.py | 17 +++++++++++++++++ tests/unit/test_certificate_revocations.py | 10 +++++++++- 7 files changed, 37 insertions(+), 2 deletions(-) diff --git a/acme_api/backend/acmesh_backend.py b/acme_api/backend/acmesh_backend.py index 68dc54a..928ec40 100644 --- a/acme_api/backend/acmesh_backend.py +++ b/acme_api/backend/acmesh_backend.py @@ -186,6 +186,9 @@ async def issue_certificate( *chain_args_for(domains), ] + key_algorithm = str(challenge_params.get("key_algorithm", "ecdsa")) + if key_algorithm != "ecdsa": + args += ["--keylength", key_algorithm.removeprefix("rsa-")] if server_url is not None: # Without an explicit server acme.sh falls back to its default CA, # silently ignoring the account's configured directory URL. diff --git a/acme_api/services/certificate_revocations.py b/acme_api/services/certificate_revocations.py index 421e020..27fe6f2 100644 --- a/acme_api/services/certificate_revocations.py +++ b/acme_api/services/certificate_revocations.py @@ -99,6 +99,7 @@ async def request_certificate_revocation( ) await session.commit() await session.refresh(revocation) + await service._dispatch_webhook(session, "certificate.revocation_failed", certificate) return revocation revocation.status = CertificateRevocationStatus.SUCCEEDED diff --git a/acme_api/services/certificates.py b/acme_api/services/certificates.py index 7851904..0112e87 100644 --- a/acme_api/services/certificates.py +++ b/acme_api/services/certificates.py @@ -317,6 +317,7 @@ async def _issue_and_deploy( try: account = self._acme_account(certificate.acme_account_ref) challenge_params: dict[str, object] = {} + challenge_params["key_algorithm"] = certificate.key_algorithm if method == "dns-01": if certificate.dns_provider_ref is None: raise DeploymentError("DNS provider is required for dns-01 issuance.") diff --git a/docs/outline.md b/docs/outline.md index b3006f3..ecda011 100644 --- a/docs/outline.md +++ b/docs/outline.md @@ -287,6 +287,10 @@ certificate.failed certificate.expiring certificate.revoked + +certificate.revoked_at_ca + +certificate.revocation_failed ``` Webhook payload example: diff --git a/tests/helpers/api.py b/tests/helpers/api.py index d5205ee..3ccdfb6 100644 --- a/tests/helpers/api.py +++ b/tests/helpers/api.py @@ -32,6 +32,7 @@ class RevocationState: """Mutable revocation controls and observations for ``ArtifactBackend``.""" requests: list[tuple[str, int | None, str | None, str | None]] = field(default_factory=list) + key_algorithms: list[str] = field(default_factory=list) error: Exception | None = None @@ -128,7 +129,7 @@ async def revoke_certificate( server_url: str | None = None, ) -> None: """Record a deterministic acme.sh revocation request.""" - del key_algorithm + self.revocation.key_algorithms.append(key_algorithm) self.revocation.requests.append((domain, reason, account_key_path, server_url)) if self.revocation.error is not None: raise self.revocation.error diff --git a/tests/unit/test_backend_commands.py b/tests/unit/test_backend_commands.py index 64193f2..d1f3082 100644 --- a/tests/unit/test_backend_commands.py +++ b/tests/unit/test_backend_commands.py @@ -173,6 +173,23 @@ async def test_issue_certificate_dns_01(self, backend: AcmeShBackend) -> None: assert "30" in call_args assert result.domains == ["example.com", "www.example.com"] + @pytest.mark.anyio + async def test_issue_rsa_certificate_selects_rsa_slot(self, backend: AcmeShBackend) -> None: + """Pass the stored RSA key length to acme.sh instead of using its ECC default.""" + with patch("asyncio.create_subprocess_exec", new_callable=AsyncMock) as mock_run: + mock_run.return_value = successful_process() + + await backend.issue_certificate( + domains=["example.com"], + method="dns-01", + challenge_params={ + "dns_provider": "cloudflare", + "env_vars_file": None, + "key_algorithm": "rsa-4096", + }, + ) + assert has_flag_pair(mock_run.call_args.args, "--keylength", "4096") + @pytest.mark.anyio async def test_dns_credentials_are_passed_per_subprocess( self, backend: AcmeShBackend, tmp_path: pathlib.Path diff --git a/tests/unit/test_certificate_revocations.py b/tests/unit/test_certificate_revocations.py index 9ed1f9e..44ece42 100644 --- a/tests/unit/test_certificate_revocations.py +++ b/tests/unit/test_certificate_revocations.py @@ -34,6 +34,7 @@ def test_certificate_authority_revocation_is_idempotent( "domains": ["example.com"], "acme_account_ref": "letsencrypt-production", "dns_provider_ref": "cloudflare-main", + "key_algorithm": "rsa-2048", }, ) certificate_id = created.json()["id"] @@ -49,10 +50,14 @@ def test_certificate_authority_revocation_is_idempotent( assert backend.revocation.requests == [ ("example.com", None, None, "https://acme-v02.api.letsencrypt.org/directory") ] + assert backend.revocation.key_algorithms == ["rsa-2048"] dispatched.assert_awaited_once_with(ANY, "certificate.revoked_at_ca", ANY) -def test_certificate_authority_revocation_persists_terminal_failure(tmp_path: Path) -> None: +def test_certificate_authority_revocation_persists_terminal_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: """Keep a terminal acme.sh revocation result durable and idempotent.""" headers = {"Authorization": "Bearer operator-key-12345", "Idempotency-Key": "terminal-revoke"} app = make_api_app(tmp_path) @@ -71,6 +76,8 @@ def test_certificate_authority_revocation_persists_terminal_failure(tmp_path: Pa }, ) certificate_id = created.json()["id"] + dispatched = AsyncMock() + monkeypatch.setattr(app.state.certificate_service, "_dispatch_webhook", dispatched) failed = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 4}) repeated = client.post(f"/v1/certificates/{certificate_id}/revoke", headers=headers, json={"reason": 4}) @@ -79,6 +86,7 @@ def test_certificate_authority_revocation_persists_terminal_failure(tmp_path: Pa assert failed.json()["error_category"] == "terminal" assert repeated.json()["id"] == failed.json()["id"] assert len(backend.revocation.requests) == 1 + dispatched.assert_awaited_once_with(ANY, "certificate.revocation_failed", ANY) def test_certificate_authority_revocation_persists_missing_account_failure( From 98b3f4f22cc1c114878cd273b055393a2c016110 Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 04:53:31 +0100 Subject: [PATCH 09/10] a little proactive fix --- acme_api/services/certificate_revocations.py | 13 ++++++++++++- tests/unit/test_certificate_revocations.py | 3 ++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/acme_api/services/certificate_revocations.py b/acme_api/services/certificate_revocations.py index 27fe6f2..62a58cb 100644 --- a/acme_api/services/certificate_revocations.py +++ b/acme_api/services/certificate_revocations.py @@ -27,6 +27,17 @@ _REVOCATION_LEASE = dt.timedelta(minutes=10) +def _public_failure_detail(error: Exception) -> str: + """Return a diagnostic that cannot expose acme.sh command output or secrets.""" + if isinstance(error, DeploymentError): + return "The configured ACME account is unavailable." + if isinstance(error, OSError): + return "acme.sh could not be executed." + if isinstance(error, TransientAcmeShError): + return "acme.sh reported a retryable revocation failure." + return "acme.sh reported a terminal revocation failure." + + async def request_certificate_revocation( service: CertificateLifecycleService, certificate_id: uuid.UUID, @@ -89,7 +100,7 @@ async def request_certificate_revocation( except (AcmeShError, DeploymentError, OSError) as exc: revocation.status = CertificateRevocationStatus.FAILED revocation.error_category = "transient" if isinstance(exc, TransientAcmeShError) else "terminal" - revocation.error_details = str(exc) + revocation.error_details = _public_failure_detail(exc) revocation.completed_at = dt.datetime.now(dt.UTC) await service._record_event( session, diff --git a/tests/unit/test_certificate_revocations.py b/tests/unit/test_certificate_revocations.py index 44ece42..c2b9a31 100644 --- a/tests/unit/test_certificate_revocations.py +++ b/tests/unit/test_certificate_revocations.py @@ -63,7 +63,7 @@ def test_certificate_authority_revocation_persists_terminal_failure( app = make_api_app(tmp_path) backend = app.state.acme_backend assert isinstance(backend, ArtifactBackend) - backend.revocation.error = TerminalAcmeShError("certificate cannot be revoked") + backend.revocation.error = TerminalAcmeShError("private-key-material-must-not-leak") with TestClient(app) as client: created = client.post( "/v1/certificates", @@ -85,6 +85,7 @@ def test_certificate_authority_revocation_persists_terminal_failure( assert failed.json()["status"] == "failed" assert failed.json()["error_category"] == "terminal" assert repeated.json()["id"] == failed.json()["id"] + assert failed.json()["error_details"] == "acme.sh reported a terminal revocation failure." assert len(backend.revocation.requests) == 1 dispatched.assert_awaited_once_with(ANY, "certificate.revocation_failed", ANY) From 2f38ab8a81ff7b43a75b70eacbeb445dbae55e59 Mon Sep 17 00:00:00 2001 From: Streaky Date: Fri, 24 Jul 2026 05:06:32 +0100 Subject: [PATCH 10/10] preserve key_algorithm for DNS-01 issuance --- acme_api/services/certificates.py | 10 ++++++---- tests/integration/test_e2e_lifecycle.py | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/acme_api/services/certificates.py b/acme_api/services/certificates.py index 0112e87..2d7e4aa 100644 --- a/acme_api/services/certificates.py +++ b/acme_api/services/certificates.py @@ -322,10 +322,12 @@ async def _issue_and_deploy( if certificate.dns_provider_ref is None: raise DeploymentError("DNS provider is required for dns-01 issuance.") provider = self._dns_provider(certificate.dns_provider_ref) - challenge_params = { - "dns_provider": provider.provider_name, - "env_vars_file": str(provider.env_vars_file_path), - } + challenge_params.update( + { + "dns_provider": provider.provider_name, + "env_vars_file": str(provider.env_vars_file_path), + } + ) result = await self._backend.issue_certificate( domains=certificate.domains, method=method, diff --git a/tests/integration/test_e2e_lifecycle.py b/tests/integration/test_e2e_lifecycle.py index 4ef105e..1c4338c 100644 --- a/tests/integration/test_e2e_lifecycle.py +++ b/tests/integration/test_e2e_lifecycle.py @@ -248,6 +248,7 @@ def test_full_certificate_lifecycle_with_webhooks( "domains": ["example.com", "www.example.com"], "acme_account_ref": "letsencrypt-staging", "dns_provider_ref": "cloudflare-main", + "key_algorithm": "rsa-4096", }, ) assert created.status_code == 202 @@ -265,6 +266,7 @@ def test_full_certificate_lifecycle_with_webhooks( { "dns_provider": "cloudflare", "env_vars_file": str(Path("tests/fixtures/sample_dns.env").resolve()), + "key_algorithm": "rsa-4096", } ]