feat(desktop): stage SAML signing certificates instead of rotating in place - #218
Merged
Conversation
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
This was referenced Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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. Backendreqwest, so no CSP change (connect-srcgoverns 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 leftkeyCredentials.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:keyCredentialsentries — aSign/Verifypair sharing acustomKeyIdentifier. Without dedupe every certificate lists twice and a one-certificate app reads as mid-rollover.customKeyIdentifieris uppercase whilepreferredTokenSigningKeyThumbprintcan differ in case; a case-sensitive match shows no active certificate at all.rotate_saml_signing_certificatestays 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_certificateis a bare POST that appends tokeyCredentialswithout 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 greenSign/Verifypair dedupe, case-insensitive thumbprint matching + newest-first ordering, and four metadata-XML parse cases (signing vs encryption,use-less descriptor, dedupe, junk input)demo_fixture_coverage.rspasses)just web-itestdid 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.Follow-ups (deliberately not in this PR)
audit/credentials.rsreadsApplication::key_credentialsonly, so an expiring SAML signing certificate is invisible in-app today — you find out from Entra's 60-day email. Fanningget_service_principal_sso_fieldsover SAML-mode SPs via the existing$batchpath would give a tenant-wide expiry board, which is what makes rotation schedulable rather than reactive.run_bulk_seq— "stage certs for everything expiring within 90 days". Safe because staging changes nothing for users; activation stays per-app.azapptoolkit-armalready 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