fix(secrets): fail closed on connector credential deletion

src/govoplan_calendar/backend/router.py

@@ -298,7 +298,14 @@ def api_delete_calendar(
 ):
     _require_scope(principal, "calendar:calendar:admin")
     try:
-        delete_calendar(session, tenant_id=principal.tenant_id, calendar_id=calendar_id, payload=payload)
+        delete_calendar(
+            session,
+            tenant_id=principal.tenant_id,
+            calendar_id=calendar_id,
+            payload=payload,
+            user_id=principal.user.id,
+            api_key_id=principal.api_key_id,
+        )
         session.commit()
         return Response(status_code=status.HTTP_204_NO_CONTENT)
     except CalendarError as exc:
@@ -344,7 +351,14 @@ def api_update_sync_source(
 ):
     _require_scope(principal, "calendar:calendar:admin")
     try:
-        source = update_sync_source(session, tenant_id=principal.tenant_id, source_id=source_id, payload=payload)
+        source = update_sync_source(
+            session,
+            tenant_id=principal.tenant_id,
+            source_id=source_id,
+            payload=payload,
+            user_id=principal.user.id,
+            api_key_id=principal.api_key_id,
+        )
         session.commit()
         session.refresh(source)
         return _sync_source_response(source)
@@ -361,7 +375,13 @@ def api_delete_sync_source(
 ):
     _require_scope(principal, "calendar:calendar:admin")
     try:
-        delete_sync_source(session, tenant_id=principal.tenant_id, source_id=source_id)
+        delete_sync_source(
+            session,
+            tenant_id=principal.tenant_id,
+            source_id=source_id,
+            user_id=principal.user.id,
+            api_key_id=principal.api_key_id,
+        )
         session.commit()
         return Response(status_code=status.HTTP_204_NO_CONTENT)
     except CalendarError as exc:
@@ -479,7 +499,14 @@ def api_update_caldav_source(
 ):
     _require_scope(principal, "calendar:calendar:admin")
     try:
-        source = update_caldav_source(session, tenant_id=principal.tenant_id, source_id=source_id, payload=payload)
+        source = update_caldav_source(
+            session,
+            tenant_id=principal.tenant_id,
+            source_id=source_id,
+            payload=payload,
+            user_id=principal.user.id,
+            api_key_id=principal.api_key_id,
+        )
         session.commit()
         session.refresh(source)
         return _caldav_source_response(source)
@@ -496,7 +523,13 @@ def api_delete_caldav_source(
 ):
     _require_scope(principal, "calendar:calendar:admin")
     try:
-        delete_caldav_source(session, tenant_id=principal.tenant_id, source_id=source_id)
+        delete_caldav_source(
+            session,
+            tenant_id=principal.tenant_id,
+            source_id=source_id,
+            user_id=principal.user.id,
+            api_key_id=principal.api_key_id,
+        )
         session.commit()
         return Response(status_code=status.HTTP_204_NO_CONTENT)
     except CalendarError as exc:

src/govoplan_calendar/backend/service.py

@@ -23,6 +23,7 @@ from govoplan_core.security.outbound_http import (
     build_outbound_http_opener,
     validate_outbound_http_url,
 )
+from govoplan_core.audit.logging import audit_event
 
 from govoplan_calendar.backend.caldav import CalDAVClient, CalDAVError, CalDAVNotFound, CalDAVReportResult, CalDAVSyncUnsupported, ensure_collection_url
 from govoplan_calendar.backend.db.models import CalendarCollection, CalendarEvent, CalendarSyncCredential, CalendarSyncSource
@@ -710,7 +711,15 @@ def _move_calendar_before_delete(
     return calendar
 
 
-def delete_calendar(session: Session, *, tenant_id: str, calendar_id: str, payload: CalendarCollectionDeleteRequest | None = None) -> None:
+def delete_calendar(
+    session: Session,
+    *,
+    tenant_id: str,
+    calendar_id: str,
+    payload: CalendarCollectionDeleteRequest | None = None,
+    user_id: str | None = None,
+    api_key_id: str | None = None,
+) -> None:
     payload = payload or CalendarCollectionDeleteRequest()
     calendar = get_calendar(session, tenant_id=tenant_id, calendar_id=calendar_id)
     deleted_at = utcnow()
@@ -729,7 +738,15 @@ def delete_calendar(session: Session, *, tenant_id: str, calendar_id: str, paylo
     if calendar.is_default:
         calendar.is_default = False
     calendar.deleted_at = deleted_at
-    retire_sync_sources_for_calendar(session, tenant_id=tenant_id, calendar_id=calendar.id, deleted_at=deleted_at)
+    retire_sync_sources_for_calendar(
+        session,
+        tenant_id=tenant_id,
+        calendar_id=calendar.id,
+        deleted_at=deleted_at,
+        deletion_reason="calendar_deleted",
+        user_id=user_id,
+        api_key_id=api_key_id,
+    )
     for event in calendar.events:
         if payload.event_action == "delete" and event.deleted_at is None:
             event.deleted_at = deleted_at
@@ -1135,6 +1152,9 @@ def _update_sync_source_credential_and_schedule(
     tenant_id: str,
     source: CalendarSyncSource,
     payload: CalendarSyncSourceUpdateRequest,
+    previous_auth_type: str,
+    user_id: str | None,
+    api_key_id: str | None,
 ) -> None:
     credential_value = caldav_secret_from_payload(
         auth_type=source.auth_type,
@@ -1145,15 +1165,39 @@ def _update_sync_source_credential_and_schedule(
         source.credential_ref = store_caldav_credential(
             session,
             tenant_id=tenant_id,
-            user_id=None,
+            user_id=user_id,
             source=source,
             secret=credential_value,
+            api_key_id=api_key_id,
         )
+    elif payload.auth_type is not None and source.auth_type != previous_auth_type:
+        delete_caldav_credential(
+            session,
+            tenant_id=tenant_id,
+            source_id=source.id,
+            credential_ref=source.credential_ref,
+            deletion_reason=(
+                "authentication_disabled"
+                if source.auth_type == "none"
+                else "authentication_type_changed"
+            ),
+            user_id=user_id,
+            api_key_id=api_key_id,
+        )
+        source.credential_ref = None
     if payload.sync_enabled is not None or payload.sync_interval_seconds is not None:
         source.next_sync_at = utcnow() if source.sync_enabled else None
 
 
-def update_sync_source(session: Session, *, tenant_id: str, source_id: str, payload: CalendarSyncSourceUpdateRequest) -> CalendarSyncSource:
+def update_sync_source(
+    session: Session,
+    *,
+    tenant_id: str,
+    source_id: str,
+    payload: CalendarSyncSourceUpdateRequest,
+    user_id: str | None = None,
+    api_key_id: str | None = None,
+) -> CalendarSyncSource:
     source = get_sync_source(session, tenant_id=tenant_id, source_id=source_id)
     source = _lock_sync_source_for_update(
         session,
@@ -1164,6 +1208,7 @@ def update_sync_source(session: Session, *, tenant_id: str, source_id: str, payl
         raise CalendarError(
             "Caller-supplied credential references are not accepted; provide a replacement password or bearer token"
         )
+    previous_auth_type = source.auth_type
     (
         normalized_collection_url,
         materially_reconfigured,
@@ -1194,30 +1239,87 @@ def update_sync_source(session: Session, *, tenant_id: str, source_id: str, payl
         tenant_id=tenant_id,
         source=source,
         payload=payload,
+        previous_auth_type=previous_auth_type,
+        user_id=user_id,
+        api_key_id=api_key_id,
     )
     mark_calendar_sync_source(calendar, source)
     session.flush()
     return source
 
 
-def update_caldav_source(session: Session, *, tenant_id: str, source_id: str, payload: CalendarCalDavSourceUpdateRequest) -> CalendarSyncSource:
+def update_caldav_source(
+    session: Session,
+    *,
+    tenant_id: str,
+    source_id: str,
+    payload: CalendarCalDavSourceUpdateRequest,
+    user_id: str | None = None,
+    api_key_id: str | None = None,
+) -> CalendarSyncSource:
     source = get_sync_source(session, tenant_id=tenant_id, source_id=source_id, source_kind="caldav")
-    return update_sync_source(session, tenant_id=tenant_id, source_id=source.id, payload=payload)
+    return update_sync_source(
+        session,
+        tenant_id=tenant_id,
+        source_id=source.id,
+        payload=payload,
+        user_id=user_id,
+        api_key_id=api_key_id,
+    )
 
 
-def delete_sync_source(session: Session, *, tenant_id: str, source_id: str) -> None:
+def delete_sync_source(
+    session: Session,
+    *,
+    tenant_id: str,
+    source_id: str,
+    user_id: str | None = None,
+    api_key_id: str | None = None,
+) -> None:
     source = get_sync_source(session, tenant_id=tenant_id, source_id=source_id)
-    retire_sync_source(session, tenant_id=tenant_id, source=source, deleted_at=utcnow())
+    retire_sync_source(
+        session,
+        tenant_id=tenant_id,
+        source=source,
+        deleted_at=utcnow(),
+        deletion_reason="sync_source_deleted",
+        user_id=user_id,
+        api_key_id=api_key_id,
+    )
     session.flush()
 
 
-def delete_caldav_source(session: Session, *, tenant_id: str, source_id: str) -> None:
+def delete_caldav_source(
+    session: Session,
+    *,
+    tenant_id: str,
+    source_id: str,
+    user_id: str | None = None,
+    api_key_id: str | None = None,
+) -> None:
     source = get_sync_source(session, tenant_id=tenant_id, source_id=source_id, source_kind="caldav")
-    retire_sync_source(session, tenant_id=tenant_id, source=source, deleted_at=utcnow())
+    retire_sync_source(
+        session,
+        tenant_id=tenant_id,
+        source=source,
+        deleted_at=utcnow(),
+        deletion_reason="sync_source_deleted",
+        user_id=user_id,
+        api_key_id=api_key_id,
+    )
     session.flush()
 
 
-def retire_sync_sources_for_calendar(session: Session, *, tenant_id: str, calendar_id: str, deleted_at: datetime) -> None:
+def retire_sync_sources_for_calendar(
+    session: Session,
+    *,
+    tenant_id: str,
+    calendar_id: str,
+    deleted_at: datetime,
+    deletion_reason: str = "calendar_deleted",
+    user_id: str | None = None,
+    api_key_id: str | None = None,
+) -> None:
     if not hasattr(session, "query"):
         return
     sources = (
@@ -1230,7 +1332,15 @@ def retire_sync_sources_for_calendar(session: Session, *, tenant_id: str, calend
         .all()
     )
     for source in sources:
-        retire_sync_source(session, tenant_id=tenant_id, source=source, deleted_at=deleted_at)
+        retire_sync_source(
+            session,
+            tenant_id=tenant_id,
+            source=source,
+            deleted_at=deleted_at,
+            deletion_reason=deletion_reason,
+            user_id=user_id,
+            api_key_id=api_key_id,
+        )
 
 
 def retire_caldav_sources_for_calendar(session: Session, *, tenant_id: str, calendar_id: str, deleted_at: datetime) -> None:
@@ -1252,7 +1362,13 @@ def retire_stale_sync_sources_for_url(session: Session, *, tenant_id: str, sourc
     now = utcnow()
     for source in sources:
         if source.calendar is None or source.calendar.deleted_at is not None:
-            retire_sync_source(session, tenant_id=tenant_id, source=source, deleted_at=now)
+            retire_sync_source(
+                session,
+                tenant_id=tenant_id,
+                source=source,
+                deleted_at=now,
+                deletion_reason="stale_sync_source_retired",
+            )
     session.flush()
 
 
@@ -1281,7 +1397,16 @@ def active_caldav_source_for_url(session: Session, *, tenant_id: str, collection
     return active_sync_source_for_url(session, tenant_id=tenant_id, source_kind="caldav", collection_url=collection_url)
 
 
-def retire_sync_source(session: Session, *, tenant_id: str, source: CalendarSyncSource, deleted_at: datetime) -> None:
+def retire_sync_source(
+    session: Session,
+    *,
+    tenant_id: str,
+    source: CalendarSyncSource,
+    deleted_at: datetime,
+    deletion_reason: str = "sync_source_retired",
+    user_id: str | None = None,
+    api_key_id: str | None = None,
+) -> None:
     from govoplan_calendar.backend.outbox import (
         calendar_outbox_has_live_lease,
         cancel_calendar_outbox_for_source,
@@ -1302,18 +1427,26 @@ def retire_sync_source(session: Session, *, tenant_id: str, source: CalendarSync
             "CalDAV source cannot be retired while an outbound operation has an active lease"
         )
 
+    # Delete credentials first. An external secret provider cannot participate
+    # in the SQL transaction, so provider failure must leave the source and its
+    # queued work untouched. If the later SQL commit fails after provider
+    # success, the source fails closed and the idempotent delete can be retried.
+    delete_caldav_credential(
+        session,
+        tenant_id=tenant_id,
+        source_id=source.id,
+        credential_ref=source.credential_ref,
+        deletion_reason=deletion_reason,
+        user_id=user_id,
+        api_key_id=api_key_id,
+    )
+    source.credential_ref = None
     cancel_calendar_outbox_for_source(
         session,
         source_id=source.id,
         reason="CalDAV source was retired before queued external changes were delivered",
     )
     source.deleted_at = deleted_at
-    delete_caldav_credential(
-        session,
-        tenant_id=tenant_id,
-        source_id=source.id,
-        credential_ref=source.credential_ref,
-    )
 
 
 def retire_caldav_source(session: Session, *, tenant_id: str, source: CalendarSyncSource, deleted_at: datetime) -> None:
@@ -1343,6 +1476,7 @@ def store_caldav_credential(
     user_id: str | None,
     source: CalendarSyncSource,
     secret: str,
+    api_key_id: str | None = None,
 ) -> str:
     existing = internal_caldav_credential(
         session,
@@ -1350,6 +1484,7 @@ def store_caldav_credential(
         source_id=source.id,
         credential_ref=source.credential_ref,
     )
+    replacing_existing = existing is not None
     if existing is None:
         existing = CalendarSyncCredential(
             tenant_id=tenant_id,
@@ -1361,10 +1496,45 @@ def store_caldav_credential(
         session.add(existing)
         session.flush()
 
+    old_provider_ref = _credential_provider_ref(existing)
     provider = secret_provider()
     name = f"caldav:{source.id}:{source.auth_type}"
+    if old_provider_ref and provider is None:
+        raise CalendarError(
+            "Stored sync credential cannot be replaced while its secret provider is unavailable"
+        )
     if provider is not None:
-        provider_ref = str(provider.store_secret(scope=f"calendar:{tenant_id}", name=name, value=secret))
+        try:
+            provider_ref = str(
+                provider.store_secret(
+                    scope=f"calendar:{tenant_id}",
+                    name=name,
+                    value=secret,
+                )
+            ).strip()
+        except Exception as exc:
+            raise CalendarError("Stored sync credential could not be written to its secret provider") from exc
+        if not provider_ref:
+            raise CalendarError("Secret provider returned an empty credential reference")
+        if old_provider_ref and old_provider_ref != provider_ref:
+            try:
+                provider.delete_secret(old_provider_ref)
+            except Exception as exc:
+                if not _provider_secret_is_absent(provider, old_provider_ref):
+                    compensation_failed = False
+                    try:
+                        provider.delete_secret(provider_ref)
+                    except Exception:
+                        compensation_failed = not _provider_secret_is_absent(provider, provider_ref)
+                    compensation_detail = (
+                        "; replacement cleanup also failed and requires provider-side reconciliation"
+                        if compensation_failed
+                        else ""
+                    )
+                    raise CalendarError(
+                        "Previous sync credential could not be deleted from its secret provider; "
+                        f"the replacement was not activated{compensation_detail}"
+                    ) from exc
         existing.secret_encrypted = None
         existing.metadata_ = {"source_id": source.id, "provider_ref": provider_ref}
     else:
@@ -1373,6 +1543,24 @@ def store_caldav_credential(
     existing.credential_kind = credential_kind_for_auth_type(source.auth_type)
     existing.label = source.display_name or source.collection_url
     existing.deleted_at = None
+    if replacing_existing:
+        audit_event(
+            session,
+            tenant_id=tenant_id,
+            user_id=user_id,
+            api_key_id=api_key_id,
+            action="calendar.sync_credential_rotated",
+            object_type="calendar_sync_credential",
+            object_id=existing.id,
+            details={
+                "sync_source_id": source.id,
+                "storage_backend": (
+                    "external_secret_provider"
+                    if provider is not None
+                    else "encrypted_database"
+                ),
+            },
+        )
     session.flush()
     return f"{CALDAV_INTERNAL_CREDENTIAL_PREFIX}{existing.id}"
 
@@ -1383,9 +1571,12 @@ def delete_caldav_credential(
     tenant_id: str,
     source_id: str,
     credential_ref: str | None,
-) -> None:
+    deletion_reason: str = "sync_source_retired",
+    user_id: str | None = None,
+    api_key_id: str | None = None,
+) -> bool:
     if not credential_ref:
-        return
+        return False
     credential = internal_caldav_credential(
         session,
         tenant_id=tenant_id,
@@ -1395,12 +1586,73 @@ def delete_caldav_credential(
     if credential is None:
         # Legacy external/env references have no locally provable ownership and
         # must never be passed to a provider delete operation.
-        return
+        return False
+    return _delete_calendar_owned_credential(
+        session,
+        credential=credential,
+        source_id=source_id,
+        deletion_reason=deletion_reason,
+        user_id=user_id,
+        api_key_id=api_key_id,
+    )
+
+
+def _delete_calendar_owned_credential(
+    session: Session,
+    *,
+    credential: CalendarSyncCredential,
+    source_id: str | None,
+    deletion_reason: str,
+    user_id: str | None = None,
+    api_key_id: str | None = None,
+) -> bool:
     provider_ref = _credential_provider_ref(credential)
+    if not credential.secret_encrypted and not provider_ref:
+        return False
     provider = secret_provider()
-    if provider is not None and provider_ref:
-        provider.delete_secret(provider_ref)
+    storage_backend = "encrypted_database"
+    if provider_ref:
+        if provider is None:
+            raise CalendarError(
+                "Stored sync credential cannot be deleted while its secret provider is unavailable; "
+                "the requested deletion was not completed"
+            )
+        try:
+            provider.delete_secret(provider_ref)
+        except Exception as exc:
+            if not _provider_secret_is_absent(provider, provider_ref):
+                raise CalendarError(
+                    "Stored sync credential could not be deleted from its secret provider; "
+                    "the requested deletion was not completed"
+                ) from exc
+        storage_backend = "external_secret_provider"
+    credential.secret_encrypted = None
+    credential.metadata_ = {"source_id": source_id} if source_id else {}
     credential.deleted_at = utcnow()
+    audit_event(
+        session,
+        tenant_id=credential.tenant_id,
+        user_id=user_id,
+        api_key_id=api_key_id,
+        action="calendar.sync_credential_deleted",
+        object_type="calendar_sync_credential",
+        object_id=credential.id,
+        details={
+            "sync_source_id": source_id,
+            "storage_backend": storage_backend,
+            "deletion_reason": deletion_reason,
+        },
+    )
+    return True
+
+
+def _provider_secret_is_absent(provider: Any, provider_ref: str) -> bool:
+    """Confirm an idempotent delete when a provider reports missing/error."""
+
+    try:
+        return provider.read_secret(provider_ref) is None
+    except Exception:
+        return False
 
 
 def resolve_caldav_secret(