feat(be,fe): SSO sign-in via two-hop OIDC discovery#3785
feat(be,fe): SSO sign-in via two-hop OIDC discovery#3785timothyaterton wants to merge 1 commit intodfinity:mainfrom
Conversation
19f0eac to
a9731ba
Compare
…ecurity (dfinity#3784) ## Summary Add the `aud` (audience / client_id) field to `OpenIdCredentialKey`, changing it from `(iss, sub)` to `(iss, sub, aud)`. This is a security prerequisite for SSO: since SSO allows anyone to provide a `client_id` via their `ii-openid-configuration` endpoint, without `aud` in the key two different OIDC clients at the same provider with the same user `sub` would collide, enabling impersonation. ## Changes - **Type update**: `OpenIdCredentialKey` type alias changed from `(Iss, Sub)` to `(Iss, Sub, Aud)` in both `internet_identity_interface` and the `openid` module - **CBOR encoding**: `StorableOpenIdCredentialKey` rewritten with manual `Encode`/`Decode` impls — new entries use CBOR map format `{0:iss, 1:sub, 2:aud}`; the decoder also handles legacy CBOR array format `[iss, sub]` for backward compatibility - **Migration**: `post_upgrade` drains the credential key index via `pop_first`, resolves `aud` from each anchor's `StorableOpenIdCredential` (which already stores `aud` at CBOR index `#[n(2)]`), and re-inserts with the complete `(iss, sub, aud)` key. Unresolvable entries are preserved with empty `aud` for retry on next upgrade. - **Key construction**: Updated `OpenIdCredential::key()`, `StorableOpenIdCredential::key()`, `calculate_delegation_seed()`, and all call sites - **Candid interface**: Updated `.did` file and generated JS/TS declarations - **Frontend**: Updated credential removal call to pass `aud` - **Tests**: Added unit tests for new CBOR map encoding, legacy array decoding, and round-trip serialization. Updated existing test assertions to use 3-tuple keys. ## Delegation seed backward compatibility The `calculate_delegation_seed` function already receives `client_id` (which equals `aud`) as a separate parameter. The seed calculation is unchanged — `aud` from the key tuple is ignored (`_aud`) in the destructuring, preserving identical `Principal` derivation for existing credentials. ## Migration safety - Uses `pop_first()` to drain the BTreeMap, avoiding byte-level encoding mismatches between legacy array-encoded keys and new map-encoded keys - Resolves `aud` from the anchor's stored `StorableOpenIdCredential` which already has `aud` at CBOR index 2 - Falls back to re-inserting with empty `aud` if resolution fails, with a logged warning — the entry is preserved for retry on next upgrade - Idempotent: safe to run on every upgrade; entries already in the new format are preserved unchanged ## Test plan - [x] All 209 unit tests pass (including Candid interface compatibility) - [ ] Integration tests (require canister WASM build — pass in CI) - [ ] Deploy to testnet and verify migration of existing credentials - [ ] Verify credential lookup works after migration - [ ] Verify new credential registration includes `aud` in key --- [< Previous PR](dfinity#3778) | [Next PR >](dfinity#3785) --------- Co-authored-by: Claude Agent <noreply@anthropic.com> Co-authored-by: Arshavir Ter-Gabrielyan <arshavir.ter.gabrielyan@dfinity.org>
There was a problem hiding this comment.
Pull request overview
Adds frontend-side OIDC discovery for a new “discoverable” provider configuration list (oidc_configs), enabling the UI/auth flow to fetch provider discovery documents on demand (instead of relying solely on backend-provided openid_configs).
Changes:
- Extend backend config decoding/types to include
oidc_configs(discoverable providers withdiscovery_url+ optionalclient_id). - Add
oidcDiscovery.tswith caching/rate limiting/validation and integrate it into auth + OpenID config lookup. - Render additional provider buttons for
oidc_configsand add unit tests for discovery + config lookup.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/frontend/src/lib/utils/openID.ts | Extend findConfig() to synthesize an OpenIdConfig from cached OIDC discovery + oidc_configs. |
| src/frontend/src/lib/utils/openID.test.ts | Add tests covering findConfig() behavior with oidc_configs and discovery cache mocking. |
| src/frontend/src/lib/utils/oidcDiscovery.ts | New module implementing OIDC discovery fetch, validation, caching, concurrency + rate limiting. |
| src/frontend/src/lib/utils/oidcDiscovery.test.ts | New unit tests for discovery fetch, caching, and validation behavior. |
| src/frontend/src/lib/globals.ts | Add candid IDL + TS types to decode oidc_configs from .config.did.bin. |
| src/frontend/src/lib/flows/authFlow.svelte.ts | Add continueWithOidc() that fetches discovery and reuses the existing OpenID flow. |
| src/frontend/src/lib/components/wizards/auth/views/PickAuthenticationMethod.svelte | Render provider buttons for oidc_configs and wire them to a new handler. |
| src/frontend/src/lib/components/wizards/auth/AuthWizard.svelte | Wire continueWithOidc handler into the auth wizard flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0ebd6e2 to
3d587a1
Compare
|
@copilot-pull-request-reviewer the PR has been restructured significantly in 3d587a1 (see previous comment thread replies). The earlier |
41c03a6 to
815d71e
Compare
815d71e to
19d0014
Compare
II admins register SSO provider domains via `add_discoverable_oidc_config` (an update call, not init args). A user then clicks "Sign in with SSO", types their organization domain, and the frontend runs a two-hop discovery chain to resolve the provider's OAuth endpoint before redirecting them to sign in. Discovery is lazy and user-initiated — the picker does not render one button per organization, just a single "Sign in with SSO" entry that leads to the domain input screen. If the user types a domain that isn't registered, they get "This domain is not registered as an OIDC provider." inline. Replaces dfinity#3786, which is closed. # Changes — backend ## Fix: `add_discoverable_oidc_config` now refreshes `/.config.did.bin` `add_oidc_config` persisted to state but did not re-certify `/.config.did.bin`. The certified asset only got rebuilt in `initialize()`, so runtime SSO registrations never reached the frontend — the SSO icon stayed hidden until the next canister upgrade. Added `refresh_config_asset()` which re-runs `init_assets(&config())` + `update_root_hash()` after every state write. ## Remove: `oidc_configs` from init args `InternetIdentityInit` previously carried `oidc_configs`, but applying it had never been wired through `apply_install_arg` — it was silently dropped on install/upgrade. Since SSO registration goes through the `add_discoverable_oidc_config` update call anyway, the init-args slot was a footgun: callers could pass it and get no effect. Removed: - The `oidc_configs` field on `InternetIdentityInit`. - The `From<&InternetIdentityInit> for InternetIdentitySynchronizedConfig` impl (can no longer copy the field that doesn't exist). - The `oidc_configs` line in the Candid interface of `InternetIdentityInit`. - The `oidc_configs` line in `config()`'s return. `InternetIdentitySynchronizedConfig` (served as `/.config.did.bin`) still has `oidc_configs` — that's how the frontend sees registered domains. `get_static_assets` now builds the synchronized config by pulling `openid_configs` from the passed-in init config and `oidc_configs` from persistent state. `setup_oidc` at post-upgrade reads from persistent state directly rather than going through the removed init-arg field, so the in-memory `OIDC_CONFIGS` thread-local is correctly repopulated across upgrades. # Changes — frontend **Type alignment with backend.** `DiscoverableOidcConfig` is `{ discovery_domain: string }`. **Two-hop discovery (`ssoDiscovery.ts`, new).** 1. `GET https://{domain}/.well-known/ii-openid-configuration` returns `{ client_id, openid_configuration }`. 2. `GET {openid_configuration}` is the provider's standard OIDC discovery, yielding `authorization_endpoint` / `scopes_supported`. Both hops run from the browser. (The backend has its own copy in `openid/generic.rs`; keeping the implementations separate for now minimizes BE↔FE synchronization.) **SSO flow UI.** - `SignInWithSso.svelte` (new): domain input. Framed icon chip, title "Sign In With SSO", subtitle "Enter your company domain", placeholder `company.domain.com`. On submit: DNS-validate → membership check against `oidc_configs` from synchronized config → `discoverSsoConfig` → `continueWithSso` → redirect. - `SsoIcon.svelte` (new): key icon. - `PickAuthenticationMethod.svelte`: "Continue with passkey" at the top; OIDC provider icons + SSO key icon render in the row below at equal width. SSO icon appears only when `oidc_configs` is non-empty. - `authFlow.svelte.ts`: new `signInWithSso` view state and `continueWithSso()` method. - Subtitle text across all three sign-in entry points updated from "Choose method to continue" → "Choose an authentication method to continue". **Security.** - Domain input DNS-format validated. - `oidc_configs` from the synchronized config is the sole allowlist — no hardcoded domain list in frontend code. - All three URLs (`/.well-known/ii-openid-configuration`, provider discovery, authorization endpoint) must be HTTPS. - The `openid_configuration` URL must be on a trusted-provider allowlist (Google, Apple, Microsoft, Okta, `login.dfinity.org`). - Issuer and `authorization_endpoint` hostnames must match the `openid_configuration` hostname exactly or as a true subdomain — `endsWith` alone would accept look-alikes like `evildfinity.okta.com`. - Per-domain rate limit (1 / 10 min), max 2 concurrent discoveries, 4-hour cache per hop, exponential backoff, timeouts with `clearTimeout` in `finally`. # Tests - 1 new integration test in `http.rs`: `add_discoverable_oidc_config_refreshes_config_asset` — install empty, call the update, assert `/.config.did.bin` reflects the new registration immediately (no upgrade required). - `config/oidc_configs.rs::should_coexist_with_openid_configs` updated to query SSO state via `discovered_oidc_configs` instead of `config().oidc_configs`. - 23 unit tests in `ssoDiscovery.test.ts` (domain validation, two-hop chain, cache, retry, trusted-provider check, HTTPS enforcement, exact-hostname match for issuer/auth endpoint, off-host rejection). - 22 unit tests in `openID.test.ts` preserved, including 4 for `selectAuthScopes`. - `cargo test -p internet_identity --bin internet_identity`: 219/219 pass. - `cargo clippy --all-targets -D warnings`: clean. - `npm run lint` + `svelte-check` on touched files: clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
19d0014 to
914f7e9
Compare
II admins register SSO provider domains via
add_discoverable_oidc_config(an update call, not init args). A user then clicks "Sign in with SSO", types their organization domain, and the frontend runs a two-hop discovery chain to resolve the provider's OAuth endpoint before redirecting them to sign in.Discovery is lazy and user-initiated — the picker does not render one button per organization, just a single "Sign in with SSO" entry that leads to the domain input screen. If the user types a domain that isn't registered, they get "This domain is not registered as an OIDC provider." inline.
Replaces #3786, which is closed.
Changes
Backend
Fix:
add_discoverable_oidc_confignow refreshes/.config.did.binadd_oidc_configpersisted to state but did not re-certify/.config.did.bin. The certified asset only got rebuilt ininitialize(), so runtime SSO registrations never reached the frontend — the SSO feature was unreachable until the next canister upgrade. Addedrefresh_config_asset()which re-runsinit_assets(&config())+update_root_hash()after every state write.Removed:
oidc_configsfrom init argsInternetIdentityInitpreviously carriedoidc_configs, but applying it had never been wired throughapply_install_arg— it was silently dropped on install/upgrade. Since SSO registration goes through theadd_discoverable_oidc_configupdate call anyway, the init-args slot was a footgun: callers could pass it and get no effect. Removed:oidc_configsfield onInternetIdentityInit(Rust + Candid).From<&InternetIdentityInit> for InternetIdentitySynchronizedConfigimpl (no longer coherent).oidc_configsline inconfig()'s return.InternetIdentitySynchronizedConfig(served as/.config.did.bin) still hasoidc_configs— that's how the frontend sees registered domains.get_static_assetsbuilds the synchronized config by pullingopenid_configsfrom the passed-in init config andoidc_configsfrom persistent state.setup_oidcat post-upgrade reads from persistent state directly, so the in-memoryOIDC_CONFIGSthread-local is correctly repopulated across upgrades without needing the init-arg round-trip.Frontend
Type alignment with backend.
DiscoverableOidcConfigis{ discovery_domain: string }— matches the Candid shape exactly.Two-hop discovery (
ssoDiscovery.ts, new).GET https://{domain}/.well-known/ii-openid-configurationreturns{ client_id, openid_configuration }. The domain owner publishes this at their DNS-backed origin.GET {openid_configuration}is the provider's standard OIDC discovery, yieldingauthorization_endpointandscopes_supported.Both hops run from the browser. (The backend has its own copy in
openid/generic.rs; keeping the implementations separate for now minimizes BE↔FE synchronization.)SSO flow UI.
SignInWithSso.svelte(new): domain input screen. Framed icon chip (FeaturedIcon), title"Sign In With SSO", subtitle"Enter your company domain", placeholder"company.domain.com". On submit: DNS-validate the input → membership check againstoidc_configsfrom the synchronized config →discoverSsoConfig→continueWithSso→ redirect. Unregistered domains render"This domain is not registered as an OIDC provider."inline.SsoIcon.svelte(new): key icon.PickAuthenticationMethod.svelte: "Continue with passkey" is the top button; OIDC provider icons + SSO key icon render in the row below at equal width. The SSO icon is always rendered — even in deployments with no registered domains, so users know the mechanism exists.authFlow.svelte.ts: newsignInWithSsoview state andcontinueWithSso()method that synthesizes anOpenIdConfigfrom discovery results and hands off to the existingcontinueWithOpenIdflow."Choose method to continue"→"Choose an authentication method to continue"to match the design.Security.
oidc_configsfrom the synchronized config is the sole allowlist of which organizations can initiate SSO — no hardcoded domain list in frontend code./.well-known/ii-openid-configuration, provider discovery, authorization endpoint) must be HTTPS.openid_configurationURL from hop 1 must be on a trusted-provider allowlist (Google, Apple, Microsoft, Okta,login.dfinity.org).authorization_endpointhostnames must match theopenid_configurationhostname exactly or as a true subdomain —endsWithalone would accept look-alikes likeevildfinity.okta.com.clearTimeoutinfinally.Tests
http.rs—add_discoverable_oidc_config_refreshes_config_asset: install empty, call the update, assert/.config.did.binreflects the new registration immediately (no upgrade required).config/oidc_configs.rs::should_coexist_with_openid_configsupdated to query SSO state viadiscovered_oidc_configsinstead of the removedconfig().oidc_configs.ssoDiscovery.test.ts(domain validation, two-hop chain, cache, retry, trusted-provider check, HTTPS enforcement, exact-hostname match for issuer / auth endpoint, off-host rejection).openID.test.tspreserved, including 4 forselectAuthScopes(shared defaults-fallback helper).cargo test -p internet_identity --bin internet_identity: 219/219 pass.cargo clippy --all-targets -D warnings: clean.npm run lint+svelte-checkon touched files: clean.< Previous PR