Skip to content

feat(desktop): stage SAML signing certificates instead of rotating in place - #218

Merged
tiredithumans merged 3 commits into
mainfrom
feat/staged-sso-cert-rollover
Aug 12, 2026
Merged

feat(desktop): stage SAML signing certificates instead of rotating in place#218
tiredithumans merged 3 commits into
mainfrom
feat/staged-sso-cert-rollover

Conversation

@tiredithumans

Copy link
Copy Markdown
Owner

Summary

Renewing a SAML signing certificate was one button that minted a new certificate and made it live in the same breath. From that moment every assertion is signed by a key the application has never seen, so nobody can start a new session until the certificate is replaced at the other end. Downtime is decided by the service provider, not by Entra — and Entra will happily hold several certificates at once.

This splits the rotation into the phases a no-downtime rollover actually has, and keeps the old certificate in place as an instant rollback until it's explicitly retired.

  • stage_saml_signing_certificate — mints and stops. The certificate lands inactive and Entra starts publishing it in the app's federation metadata, so an application that polls metadata can pick it up before it ever goes live. Additive and reversible, so it's the only phase safe to run in bulk.
  • probe_federation_metadata — reads what the public metadata endpoint publishes. Backend reqwest, so no CSP change (connect-src governs the webview only).
  • activate_ / revert_saml_signing_certificate — one guarded PATCH shared both ways, re-resolving live state and refusing a thumbprint that's missing or expired. Revert is instant: the old certificate never left keyCredentials.
  • retire_saml_signing_certificate — refuses the active certificate and the staged one. Retiring the superseded certificate is what ends the ability to roll back, so it stays an explicit action.
  • get_signing_cert_rollover — the phase, derived from live SP state on every read. Nothing is persisted, so a rollover abandoned half way resumes where it was.

Three Graph behaviours are load-bearing, and each is table-tested in build_rollover:

  1. One certificate is two keyCredentials entries — a Sign/Verify pair sharing a customKeyIdentifier. Without dedupe every certificate lists twice and a one-certificate app reads as mid-rollover.
  2. customKeyIdentifier is uppercase while preferredTokenSigningKeyThumbprint can differ in case; a case-sensitive match shows no active certificate at all.
  3. Entra auto-promotes a staged certificate once the active one expires. That turns the active certificate's expiry into an activation deadline, which the panel now surfaces — and an expired-but-still-nominated certificate means the promotion already happened.

rotate_saml_signing_certificate stays for the application that genuinely can only hold one certificate, relabelled "Rotate and activate immediately" with a warning.

Also fixes a latent cache bug found on the way: add_token_signing_certificate is a bare POST that appends to keyCredentials without busting the SP cache. That was masked while its only caller followed it with a PATCH that invalidated — staging ends at the POST, so it would have read the pre-stage array back.

Deliberate non-guard

Activation is not gated on the metadata probe having run. A probe can fail for reasons unrelated to the rollover (offline, proxy), and blocking on it would strand an operator mid-window. It's an unchecked precondition in the UI instead, and a failed probe renders as "couldn't check", never "not published" — a false negative there argues an operator out of a safe activation.

Test plan

  • just verify — fmt, clippy, test, web-fmt-check, web-clippy, web-test, web-build all green
  • 8 new backend unit tests: the four rollover phases, the expired-active/auto-promoted case, Sign/Verify pair dedupe, case-insensitive thumbprint matching + newest-first ordering, and four metadata-XML parse cases (signing vs encryption, use-less descriptor, dedupe, junk input)
  • Demo fixtures registered for both new infallible-path reads (demo_fixture_coverage.rs passes)
  • just web-itest did NOT run — no chromedriver on this machine. No existing GUI test references the SSO tab or the strings this PR changes, so nothing known is broken, but the new rollover panel has no behavioural coverage. Worth a follow-up shard.
  • Not exercised against a live tenant — the Graph call shapes are covered by unit tests only

Follow-ups (deliberately not in this PR)

  • The audit can't see these certificates. audit/credentials.rs reads Application::key_credentials only, so an expiring SAML signing certificate is invisible in-app today — you find out from Entra's 60-day email. Fanning get_service_principal_sso_fields over SAML-mode SPs via the existing $batch path would give a tenant-wide expiry board, which is what makes rotation schedulable rather than reactive.
  • Bulk stage via run_bulk_seq — "stage certs for everything expiring within 90 days". Safe because staging changes nothing for users; activation stays per-app.
  • Auto-rollback on sign-in failureazapptoolkit-arm already queries Azure Monitor Logs, so a post-activation watch window that reverts on a failure spike is buildable on what's here.

🤖 Generated with Claude Code

https://claude.ai/code/session_017BSW4ReFWuL64BfTkEP74S

tiredithumans and others added 3 commits August 12, 2026 09:11
Two client changes the staged rollover flow needs:

- `remove_service_principal_key_credential` — the SP-side twin of
  `remove_key_credential`, which only targets applications. `keyCredentials`
  is a full-collection PATCH, so it re-reads live state and writes the whole
  array back, round-tripping survivors as raw JSON. Graph stores a signing
  certificate as a `Sign`/`Verify` pair sharing one `customKeyIdentifier`, so
  it drops both halves — removing one strands the other.

- `add_token_signing_certificate` now self-invalidates the SP cache. It is a
  bare POST that appends to `keyCredentials`, which the cached SP projection
  `$select`s. That was masked while the only caller followed it with a PATCH
  that invalidated; staging a certificate ends at the POST, so without this
  the rollover view reads the pre-stage array back.

`invalidate_sp_cache` becomes `pub(crate)` so the credential mutators bust
through the same door rather than growing a second sweep.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017BSW4ReFWuL64BfTkEP74S
… place

Rotating a SAML signing certificate was one command that minted a new
certificate and PATCHed `preferredTokenSigningKeyThumbprint` in the same
breath. From that moment every assertion is signed by a key the application
has never seen, so no one can start a new session until the certificate is
replaced at the other end.

Splits it into the phases a no-downtime rollover actually has:

- `stage_saml_signing_certificate` — mints and stops. The certificate lands
  inactive and Entra starts publishing it in the app's federation metadata,
  so an application that polls metadata can pick it up before it goes live.
  Additive and reversible, so this is the only phase safe to run in bulk.
- `probe_federation_metadata` — reads what the public metadata endpoint
  publishes (backend reqwest; `connect-src` governs the webview only).
- `activate_saml_signing_certificate` / `revert_saml_signing_certificate` —
  one guarded PATCH shared both ways, re-resolving live state and refusing a
  thumbprint that is missing or expired. Revert is instant because the old
  certificate never left `keyCredentials`.
- `retire_saml_signing_certificate` — refuses the active certificate and the
  staged one; retiring the superseded certificate is what ends the ability to
  roll back, so it stays explicit.
- `get_signing_cert_rollover` — the phase, derived from live SP state on
  every read. Nothing is persisted, so a rollover abandoned half way resumes
  where it was and two operators can't disagree about it.

Three Graph behaviours are load-bearing and table-tested in `build_rollover`:
one certificate is a `Sign`/`Verify` pair sharing a `customKeyIdentifier`
(dedupe, or every certificate lists twice); `customKeyIdentifier` is uppercase
while the preferred thumbprint can differ in case; and Entra auto-promotes a
staged certificate once the active one expires — which turns that expiry into
an activation deadline the panel now surfaces.

`rotate_saml_signing_certificate` stays for the application that can only hold
one certificate, relabelled "Rotate and activate immediately" and carrying a
warning, since that is exactly what it does.

Deliberately not gated on the metadata probe: it can fail for reasons
unrelated to the rollover, and blocking on it would strand an operator
mid-window. A failed probe renders as "couldn't check", never as "not
published" — a false negative there argues against a safe activation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017BSW4ReFWuL64BfTkEP74S
The file sat 7 bytes under its 28 000-byte ceiling, so the new rollover
invariant tripped the budget test. Applied the remedy the hook prescribes —
deep detail to docs/architecture/, one invariant plus a pointer here:

- the rollover entry itself is a pointer to auth-and-consent.md, which carries
  the phase machine, the Graph behaviours, and the guard rationale;
- dropped the tenant-scoped-UI bullet's enumeration of which signals live in
  `TenantScopedUi` — frontend-workspace.md already lists them;
- dropped two clauses of test-implementation trivia (how the cancel invariant
  derives its call sites) that are visible on opening the test.

No invariant lost: every trimmed clause is stated in the linked deep-dive.
Leaves ~27 bytes of headroom, so the next addition will need its own trim.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017BSW4ReFWuL64BfTkEP74S
@tiredithumans
tiredithumans merged commit a562ca5 into main Aug 12, 2026
9 checks passed
@tiredithumans
tiredithumans deleted the feat/staged-sso-cert-rollover branch August 12, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant