feat(be): add DiscoverableOidcConfig type with OIDC discovery support#3778
feat(be): add DiscoverableOidcConfig type with OIDC discovery support#3778aterga merged 12 commits intodfinity:mainfrom
Conversation
|
Dear @timothyaterton, In order to potentially merge your code in this open-source repository and therefore proceed with your contribution, we need to have your approval on DFINITY's CLA. If you decide to agree with it, please visit this issue and read the instructions there. Once you have signed it, re-trigger the workflow on this PR to see if your code can be merged. — The DFINITY Foundation |
There was a problem hiding this comment.
Pull request overview
Adds first-class OIDC discovery-based configuration to Internet Identity, allowing providers to be configured with a discovery URL and periodically refreshed metadata, and exposes the resolved discovery state via a new query endpoint.
Changes:
- Introduces
DiscoverableOidcConfig(+oidc_configs) alongside existingopenid_configs, and persists it in canister state/stable storage. - Adds a discovery-driven OpenID provider implementation with periodic discovery refresh and a new
discovered_oidc_configsquery returning resolvedOidcConfig. - Extends the Candid interface and integration tests to cover initialization/upgrade/retention and querying of the new config.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/internet_identity_interface/src/internet_identity/types.rs |
Adds new OIDC discovery config/return types and threads them into synchronized config/init args. |
src/internet_identity/tests/integration/http.rs |
Updates synchronized-config decoding assertion for the new optional field. |
src/internet_identity/tests/integration/config/oidc_configs.rs |
New integration tests covering init/upgrade/retention and discovery query behavior. |
src/internet_identity/tests/integration/config.rs |
Wires the new oidc_configs integration test module. |
src/internet_identity/src/storage/storable/storable_persistent_state.rs |
Persists oidc_configs through stable memory serialization/deserialization. |
src/internet_identity/src/state.rs |
Adds oidc_configs to PersistentState and its defaults. |
src/internet_identity/src/openid/generic.rs |
Implements DiscoverableProvider, discovery timers/tasks, issuer-domain validation, and discovery HTTP fetch/transform. |
src/internet_identity/src/openid.rs |
Adds OIDC setup/storage and builds OidcConfig results for the new query. |
src/internet_identity/src/main.rs |
Exposes discovered_oidc_configs query and applies mutual exclusivity rules when applying init/upgrade args. |
src/internet_identity/internet_identity.did |
Extends the public Candid interface with new record types and query method. |
src/internet_identity/Cargo.toml |
Adds the url dependency used for issuer/discovery host validation. |
src/canister_tests/src/api/internet_identity.rs |
Adds test API binding for discovered_oidc_configs. |
Cargo.lock |
Updates lockfile for the new url dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Introduce a new `OidcConfig` type and `oidc_configs` init field (XOR with existing `openid_configs`) that relies on OIDC discovery instead of requiring all provider details in the static config. Backend changes: - New `OidcConfig` type: name, logo, discovery_url, client_id (opt), email_verification (opt) - New `ActiveOidcConfig` return type for resolved provider state - `DiscoverableProvider` that fetches `.well-known/openid-configuration` to discover issuer and jwks_uri for JWT verification - Issuer domain validation (prevents impersonation via discovery) - `active_oidc_configs` query endpoint - XOR validation: canister traps if both openid_configs and oidc_configs are set simultaneously - State/storage persistence with Candid backward compatibility Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Rename active_oidc_configs → discovered_oidc_configs (type + query) - Remove dead config field from DiscoverableProvider - Use `url` crate for issuer domain validation instead of manual parsing - Move DISCOVERY constants to top of generic.rs with docstring - Refactor to set_timer_interval pattern (init_discovery_timers) - Don't trap in transform_discovery — propagate errors instead - Remove unused DiscoveryDocument fields (authorization_endpoint, scopes_supported) - Populate discovered issuer in query response via discovered_issuer_for() - Add deprecation notes on openid_configs and new_flow_origins Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…onfig → OidcConfig The init config type (with discovery_url) becomes DiscoverableOidcConfig, and the resolved query return type becomes the simpler OidcConfig name. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When both openid_configs and oidc_configs are provided, prefer openid_configs as the proven production path instead of trapping. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Enforce XOR between new_flow_origins and oidc_configs (clearing one when the other is set) to match documented mutual exclusivity - Remove SSO mode documentation from DiscoverableOidcConfig that described unsupported client_id=None behavior - Return "discovery in progress" error instead of "Unsupported issuer" when discoverable providers haven't completed initialization - Track last seen jwks_uri per discovery task and restart cert fetching when it changes (instead of one-shot certs_started gate) - Enable url crate std feature explicitly for wasm32 compatibility Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The .did file still referenced the unsupported SSO mode where client_id is null. This was removed from the Rust docs in the prior commit but the Candid interface comment was missed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
93db43d to
8413af8
Compare
Review dismissed by automation script.
- Use const initializers for thread_local RefCell values - Remove unnecessary Nat::from() in status comparison Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Functions validate_issuer_domain, transform_discovery, struct DiscoveryDocument, and constant FETCH_DISCOVERY_INTERVAL_SECONDS are only reachable from #[cfg(not(test))] code. Mark them accordingly so clippy doesn't flag them as dead code when compiling the test binary. Fields certs_ref and last_jwks_uri on DiscoveryState are written in all builds but only read by the non-test periodic timer; suppress with #[allow(dead_code)] to keep the struct layout consistent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…very Addresses reviewer feedback on dfinity#3778: - DiscoverableOidcConfig becomes { discovery_domain } (strip name, logo, client_id, email_verification). client_id is instead discovered at runtime from the SSO indirection endpoint, not declared in config. - Two-hop discovery: fetch {domain}/.well-known/ii-openid-configuration for { client_id, openid_configuration }, then fetch openid_configuration for { issuer, jwks_uri }. Validate that the discovered issuer host matches the openid_configuration host (the domain->openid_configuration indirection is intentional, so don't re-require it to match the discovery_domain). - OpenIdProvider::issuer/client_id now return Option<String>. Providers with pending discovery are skipped during JWT matching and surface a "discovery in progress" error instead of a generic "unsupported issuer". - with_provider now matches on the (iss, aud) pair. PartialClaims.aud accepts either a string or a string array per RFC 7519. - OidcConfig output exposes all three discovered fields (client_id, openid_configuration, issuer) as Option<String>. - Canary allowlist: setup_oidc traps on any discovery_domain that isn't on ALLOWED_DISCOVERY_DOMAINS (currently just "dfinity.org"). A TODO comment on DISCOVERY_TASKS notes the unbounded-tasks concern to revisit once the canary allowlist is lifted. - Update Candid (.did) to match the new record shapes. - Unit tests: ExampleProvider gains parameterized issuer/client_id, new tests cover pending-discovery skip and iss+aud disambiguation. - Integration tests: example_oidc_config uses discovery_domain = "dfinity.org", discovered_oidc_configs test asserts all discovered fields are None pre-fetch, new test verifies disallowed domains are rejected at init. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new “two-hop OIDC discovery” configuration path for SSO providers, allowing II to periodically resolve provider metadata (client_id / issuer / JWKS) from discovery endpoints instead of requiring fully static OpenID configs.
Changes:
- Introduces
DiscoverableOidcConfig/oidc_configsinit field and persists it in stable state. - Implements a
DiscoverableProviderthat performs periodic two-hop discovery and uses discovered metadata for JWT verification. - Adds a
discovered_oidc_configsquery endpoint + integration tests covering init/upgrade/XOR/query behavior.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/internet_identity_interface/src/internet_identity/types.rs | Adds oidc_configs to init/sync config and introduces DiscoverableOidcConfig + OidcConfig types. |
| src/internet_identity/tests/integration/http.rs | Updates synchronized-config integration assertion to include oidc_configs. |
| src/internet_identity/tests/integration/config/oidc_configs.rs | New integration tests covering init/upgrade/retain/XOR/query and canary allowlist behavior. |
| src/internet_identity/tests/integration/config.rs | Registers the new oidc_configs integration test module. |
| src/internet_identity/src/storage/storable/storable_persistent_state.rs | Persists oidc_configs in stable state via candid encoding. |
| src/internet_identity/src/state.rs | Extends PersistentState with oidc_configs. |
| src/internet_identity/src/openid/generic.rs | Implements discovery tasks/timers, issuer-host validation, and the new DiscoverableProvider. |
| src/internet_identity/src/openid.rs | Extends provider trait to Option issuer/client_id, adds OIDC setup and query surface, and matches providers by (iss, aud). |
| src/internet_identity/src/main.rs | Wires config persistence + XOR rules, calls setup_oidc, and exposes discovered_oidc_configs query. |
| src/internet_identity/src/anchor_management/registration/registration_flow_v2.rs | Adapts registration flow to new issuer() -> Option<String> API. |
| src/internet_identity/internet_identity.did | Extends Candid interface with new types and discovered_oidc_configs query. |
| src/internet_identity/Cargo.toml | Adds url dependency for URL/host validation. |
| src/canister_tests/src/api/internet_identity.rs | Adds PocketIC wrapper for discovered_oidc_configs. |
| Cargo.lock | Locks url dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…R logic Address sea-snake review feedback: - Remove mutual exclusivity between oidc_configs, openid_configs, and new_flow_origins — a canister can have both openid_configs and oidc_configs, and new_flow_origins is unrelated to SSO. - Add add_discoverable_oidc_config update call so SSO providers can be added at any time without a canister upgrade. - Keep oidc_configs in init args for backward compat and config() output but no longer process them in apply_install_arg. - Update integration tests to use the new update call pattern.
…ror surface Addresses review feedback on the DiscoverableOidcConfig PR: - JWT `aud` claim: treat arrays as a set (match any entry against the configured client_id) instead of only the first element. Applied in both `with_provider` and `verify_claims`; `Claims.aud` now accepts string or array (RFC 7519). - `jwks_uri`: validate as https before scheduling periodic outcalls so a malformed discovery doc doesn't burn cycles on a doomed fetch loop. - Empty certs state: `DiscoverableProvider::verify` now returns a dedicated "JWKS not yet fetched" error instead of the generic "Certificate not found", so the frontend can retry during the post-discovery / pre-JWKS window. - Registration flow: replace `provider.issuer().unwrap_or_default()` with an explicit error to fail fast if the "discovery complete" invariant ever breaks, rather than silently recording an empty issuer. - Store the verified `client_id` (not the raw `aud` claim) as the credential `aud` — canonical value for keying delegations. - Drop `should_restore_oidc_configs_from_init_args_on_upgrade`: init args no longer persist `oidc_configs` (configs are added via the update call). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review dismissed by automation script.
…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>
II admins can register an organization by `discovery_domain` via `add_discoverable_oidc_config` (backend, already shipped in dfinity#3778). This PR adds the matching frontend: a user clicks "Sign in with SSO", types their organization domain, and the frontend performs 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 doesn't render one button per organization, just a single "Sign in with SSO" entry that leads to the domain input screen. # Changes **Type alignment with backend.** The frontend `DiscoverableOidcConfig` now matches main's Candid type exactly: `{ discovery_domain: string }`. Everything else (`client_id`, `logo`, `name`, etc.) is resolved on demand during discovery — the backend only stores the domain. **Two-hop discovery (`ssoDiscovery.ts`, new).** 1. `GET https://{domain}/.well-known/ii-openid-configuration` returns `{ client_id, openid_configuration }`. The domain owner is responsible for publishing this at their DNS-backed origin. 2. `GET {openid_configuration}` is the provider's standard OIDC discovery, yielding `authorization_endpoint` and `scopes_supported`. Both hops run entirely from the browser. (The backend does its own two-hop discovery via HTTPS outcalls in `src/internet_identity/src/ openid/generic.rs`; keeping the two implementations separate for now simplifies BE↔FE synchronization.) **SSO flow UI.** - `SignInWithSso.svelte` (new): domain input screen. On submit it validates DNS format, checks the domain is in the backend's `oidc_configs`, runs `discoverSsoConfig`, then calls `continueWithSso` to redirect. If the domain isn't registered, shows "This domain is not registered as an OIDC provider." inline. - `SsoIcon.svelte` (new): key icon for the SSO button. - `PickAuthenticationMethod.svelte`: renders the SSO button whenever `oidc_configs` is non-empty. Does not render per-provider buttons — users don't know which IdP their org uses, they just type their domain. - `authFlow.svelte.ts`: new `signInWithSso` view + `continueWithSso()` method that synthesizes an `OpenIdConfig` from discovery results and hands off to the existing `continueWithOpenId` flow. **Security.** - Domain input is DNS-format validated (length, label length, no special characters). - `oidc_configs` from the backend is the sole allowlist of which organizations can initiate SSO. No hardcoded domain allowlist in frontend code. - All three URLs (the .well-known, the discovery, the auth endpoint) must be HTTPS. - The `openid_configuration` URL from hop 1 must be on a trusted OIDC provider domain (Google, Apple, Microsoft, Okta, login.dfinity.org). - Issuer hostname in the provider discovery must match the `openid_configuration` hostname *exactly* or as a true subdomain — using `endsWith` alone would accept look-alikes like `evildfinity.okta.com`. - Authorization endpoint hostname is constrained to the same, not just HTTPS-validated, so a tampered discovery response can't redirect the auth step off-host. - Per-domain rate limit (1 attempt per 10 min), max 2 concurrent discoveries, 4-hour cache per hop, exponential backoff, timeouts (5s for hop 1, 10s for hop 2) with `clearTimeout` in `finally` so a failed fetch can't leak an armed abort timer. **Cleanup of stale assumptions.** - Removed the earlier draft's eager per-provider button rendering in `PickAuthenticationMethod` — those were based on a richer `DiscoverableOidcConfig` shape that didn't survive into main. - Removed `authFlow.continueWithOidc` and `openID.findConfig`'s `oidc_configs` extension for the same reason; they assumed the config contained a pre-supplied `client_id`, which it no longer does. # Tests - 23 tests in `ssoDiscovery.test.ts`: domain validation, allowlist-at-caller discipline, two-hop happy path, cache, retry, trusted-provider check, HTTPS enforcement, issuer/auth-endpoint hostname exact-match (including an `evildfinity.okta.com` regression case), off-host auth-endpoint rejection, non-object responses. - 22 tests in `openID.test.ts` preserved, including 4 for `selectAuthScopes` (the shared defaults-fallback helper used by `continueWithSso`). - `npm run lint` and `svelte-check` on touched files: clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
II admins can register an organization by `discovery_domain` via `add_discoverable_oidc_config` (backend, already shipped in dfinity#3778). This PR adds the matching frontend: a user clicks "Sign in with SSO", types their organization domain, and the frontend performs 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 doesn't render one button per organization, just a single "Sign in with SSO" entry that leads to the domain input screen. # Changes **Type alignment with backend.** The frontend `DiscoverableOidcConfig` now matches main's Candid type exactly: `{ discovery_domain: string }`. Everything else (`client_id`, `logo`, `name`, etc.) is resolved on demand during discovery — the backend only stores the domain. **Two-hop discovery (`ssoDiscovery.ts`, new).** 1. `GET https://{domain}/.well-known/ii-openid-configuration` returns `{ client_id, openid_configuration }`. The domain owner is responsible for publishing this at their DNS-backed origin. 2. `GET {openid_configuration}` is the provider's standard OIDC discovery, yielding `authorization_endpoint` and `scopes_supported`. Both hops run entirely from the browser. (The backend does its own two-hop discovery via HTTPS outcalls in `src/internet_identity/src/ openid/generic.rs`; keeping the two implementations separate for now simplifies BE↔FE synchronization.) **SSO flow UI.** - `SignInWithSso.svelte` (new): domain input screen. On submit it validates DNS format, checks the domain is in the backend's `oidc_configs`, runs `discoverSsoConfig`, then calls `continueWithSso` to redirect. If the domain isn't registered, shows "This domain is not registered as an OIDC provider." inline. - `SsoIcon.svelte` (new): key icon for the SSO button. - `PickAuthenticationMethod.svelte`: renders the SSO button whenever `oidc_configs` is non-empty. Does not render per-provider buttons — users don't know which IdP their org uses, they just type their domain. - `authFlow.svelte.ts`: new `signInWithSso` view + `continueWithSso()` method that synthesizes an `OpenIdConfig` from discovery results and hands off to the existing `continueWithOpenId` flow. **Security.** - Domain input is DNS-format validated (length, label length, no special characters). - `oidc_configs` from the backend is the sole allowlist of which organizations can initiate SSO. No hardcoded domain allowlist in frontend code. - All three URLs (the .well-known, the discovery, the auth endpoint) must be HTTPS. - The `openid_configuration` URL from hop 1 must be on a trusted OIDC provider domain (Google, Apple, Microsoft, Okta, login.dfinity.org). - Issuer hostname in the provider discovery must match the `openid_configuration` hostname *exactly* or as a true subdomain — using `endsWith` alone would accept look-alikes like `evildfinity.okta.com`. - Authorization endpoint hostname is constrained to the same, not just HTTPS-validated, so a tampered discovery response can't redirect the auth step off-host. - Per-domain rate limit (1 attempt per 10 min), max 2 concurrent discoveries, 4-hour cache per hop, exponential backoff, timeouts (5s for hop 1, 10s for hop 2) with `clearTimeout` in `finally` so a failed fetch can't leak an armed abort timer. **Cleanup of stale assumptions.** - Removed the earlier draft's eager per-provider button rendering in `PickAuthenticationMethod` — those were based on a richer `DiscoverableOidcConfig` shape that didn't survive into main. - Removed `authFlow.continueWithOidc` and `openID.findConfig`'s `oidc_configs` extension for the same reason; they assumed the config contained a pre-supplied `client_id`, which it no longer does. # Tests - 23 tests in `ssoDiscovery.test.ts`: domain validation, allowlist-at-caller discipline, two-hop happy path, cache, retry, trusted-provider check, HTTPS enforcement, issuer/auth-endpoint hostname exact-match (including an `evildfinity.okta.com` regression case), off-host auth-endpoint rejection, non-object responses. - 22 tests in `openID.test.ts` preserved, including 4 for `selectAuthScopes` (the shared defaults-fallback helper used by `continueWithSso`). - `npm run lint` and `svelte-check` on touched files: clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
DiscoverableOidcConfigtype andoidc_configsinit field (mutually exclusive with existingopenid_configs) that relies on OIDC discovery (.well-known/openid-configuration) instead of requiring all provider details in the static configDiscoverableProviderthat periodically fetches discovery metadata to obtainissuerandjwks_urifor JWT verificationdiscovered_oidc_configsquery endpoint returningOidcConfigwith resolved provider stateissuerdomain matches thediscovery_urldomain (prevents impersonation)openid_configsandoidc_configsare provided, falls back toopenid_configsas the proven pathTest plan
config/oidc_configs.rs(init, upgrade, retain, XOR, query)openid_configstests pass unchanged (backward compat)oidc_configs🤖 Generated with Claude Code
Next PR >