feat(be): add sso_allow_any_domain flag to bypass the SSO domain allowlist#4045
Open
sea-snake wants to merge 2 commits into
Open
feat(be): add sso_allow_any_domain flag to bypass the SSO domain allowlist#4045sea-snake wants to merge 2 commits into
sea-snake wants to merge 2 commits into
Conversation
|
✅ No security or compliance issues detected. Reviewed everything up to 6c19897. Security Overview
Detected Code Changes| Change Type | Relevant files ... (code changes summary truncated to fit VCS comment limits.) |
aterga
added a commit
that referenced
this pull request
Jun 19, 2026
…ery/JWKS caches (#4022) OpenID sign-in dispatched through trait objects (`dyn OpenIdProvider`) and resolved SSO organization domains client-side. This reworks it into a single data-driven verification pipeline with two clearly separated JWK sources of truth, moves SSO discovery onto the canister, and makes the "cache still warming" state a first-class retry signal instead of an error. No user-visible behavior changes for the configured Google / Microsoft / Apple providers; SSO (organization) sign-in now resolves and verifies entirely canister-side. ## Changes **Backend** - Replace the `dyn OpenIdProvider` trait dispatch and the monolithic `openid/generic.rs` with a data-only provider model: one shared verification pipeline (`openid/verify.rs`, `provider.rs`, `jwks.rs`) that is identical for every provider up to the point it reads the JWK. - Two JWK sources of truth behind that shared pipeline: configured providers (`openid/configured.rs`) keep their timers + stable-storage-persisted keys (some providers can't reach HTTP-outcall consensus); SSO providers (`openid/sso.rs`) source keys on demand through the single-flight cache. - Canister-side SSO discovery: `discover_sso : (text) -> ()` (fire-and-forget drive of the two-hop discovery+JWKS fetch) and `get_sso_discovery : (text) -> (SsoDiscoveryState) query` where `SsoDiscoveryState = variant { Resolved : SsoDiscovery; Pending; NotAllowed }`. Gated by the `sso_discoverable_domains` allowlist. Replaces the client-side discovery module. - The four OpenID JWT methods take an optional SSO discovery domain and return `Pending` as a result arm (not an error) when the SSO discovery/JWKS cache is cold or has been evicted: - `openid_credential_add`, `openid_prepare_delegation`, `openid_get_delegation` return `OpenIdResult<T, E> = variant { Ok; Pending; Err }`. - `openid_identity_registration_finish` returns `variant { Ok; Pending; Err }`; its verification is hoisted into the endpoint so the JWT is verified exactly once and the pubkey-registration path is untouched. - `Pending` is removed from `OpenIdCredentialAddError` / `OpenIdDelegationError`. **Frontend** - SSO discovery polls `get_sso_discovery` and drives `discover_sso` while `Pending`; `ssoDiscovery.ts` no longer fetches discovery documents itself. - Sign-in, account-link, and signup all retry while the canister reports `Pending` (shared `retryWhilePending` / poll loop), so a cold or evicted SSO cache no longer turns into a hard error. ## Tests - New `config/sso_discovery.rs` integration tests for the `discover_sso` / `get_sso_discovery` allowlist gate. - Updated OpenID / attributes / v2_api registration integration suites and api helpers for the new return shapes. - SSO Playwright e2e green (12/12) across the discovery, sign-in, link, and signup paths. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <div align="right">Next: #4045</div> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Arshavir Ter-Gabrielyan <arshavir.ter.gabrielyan@dfinity.org>
…in allowlist Adds an `sso_allow_any_domain : opt bool` init/upgrade arg. When `true`, the SSO discovery domain gate (`is_allowed_discovery_domain`) accepts any domain, so `sso_discoverable_domains` (and its `is_production` defaults) no longer restrict which domains may be discovered as SSO providers. The strict-`https` posture is deliberately unchanged: serving discovery over plain `http` still requires the host to be on the explicit `sso_discoverable_domains` list, so opening the domain gate never lets an arbitrary host publish discovery endpoints over plain HTTP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e580440 to
26bdb1d
Compare
aterga
approved these changes
Jun 19, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new backend deploy/upgrade flag sso_allow_any_domain to bypass the SSO discovery domain allowlist (while aiming to keep the strict-HTTPS posture unchanged), and propagates the new init field through persistent state, candid/interface types, and regenerated frontend bindings.
Changes:
- Introduce/persist/report
sso_allow_any_domain: opt boolin backend config and stable storage. - Update SSO discovery gating logic to allow any domain when the flag is enabled, while keeping the HTTP relaxation gate tied to the explicit allowlist.
- Regenerate candid bindings and update frontend init literals; add unit/integration coverage for the new gate behavior.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/internet_identity/tests/integration/config/sso_discovery.rs | Adds an integration test verifying the new flag opens the domain gate. |
| src/internet_identity/src/storage/storable/storable_persistent_state.rs | Persists the new flag through stable storage conversions and defaults. |
| src/internet_identity/src/state.rs | Adds sso_allow_any_domain to PersistentState and its default. |
| src/internet_identity/src/openid/sso.rs | Implements the new “allow any domain” gate and decouples it from HTTP relaxation. |
| src/internet_identity/src/main.rs | Reports the flag via config() and persists it from install/upgrade args. |
| src/internet_identity/internet_identity.did | Extends candid InternetIdentityInit with sso_allow_any_domain. |
| src/internet_identity_interface/src/internet_identity/types.rs | Extends Rust interface InternetIdentityInit with the new field/docs. |
| src/frontend/src/routes/vc-flow/index/+page.svelte | Updates frontend init literal to include the new optional field. |
| src/frontend/src/lib/utils/iiConnection.test.ts | Updates test init literal to include the new optional field. |
| src/frontend/src/lib/generated/internet_identity_types.d.ts | Regenerates TS types to include sso_allow_any_domain. |
| src/frontend/src/lib/generated/internet_identity_idl.js | Regenerates JS IDL factory to include sso_allow_any_domain. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aterga
approved these changes
Jun 19, 2026
…ow_any_domain (#4048) Addresses the review comment on #4045 (#4045 (comment)). ## Problem Hop-1 of SSO discovery chooses its scheme via `scheme_for_allowlisted_host`, which keyed **only** on whether the host was loopback (`localhost` / `127.0.0.1`) — it never consulted the allowlist. That was safe only under the prior invariant that *every* domain reaching discovery was already explicitly allowlisted. `sso_allow_any_domain` breaks that invariant: with the flag on, `is_allowed_discovery_domain` accepts **any** domain, so a non-allowlisted `localhost` / `127.0.0.1` now reaches hop-1 and gets a plain-`http://` outcall. That is an SSRF/footgun for staging deployments and contradicts the flag's stated "strict-`https` posture untouched" goal. ## Fix Gate the loopback `http` downgrade on **explicit** allowlisting (the same gate hop-2 already uses), so the flag opens the *domain* gate but never relaxes `https`: ```rust if matches!(bare.as_str(), "localhost" | "127.0.0.1") && is_explicitly_allowlisted(host) { "http" } else { "https" } ``` - A loopback host reachable *only* via the flag (not on the explicit allowlist) now gets `https` — no plain-HTTP outcall to loopback. - The e2e provider `localhost:11107` stays on the explicit `sso_discoverable_domains` allowlist, so its `http` path is unaffected (no e2e regression). - Adds regression unit test `allow_any_domain_does_not_relax_https_for_loopback`. > Note: this is stacked on `feat/sso-allow-any-domain`; it can also just be cherry-picked as a single commit onto that branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LDnAektYFmpUWDHPLHypkQ --- _Generated by [Claude Code](https://claude.ai/code/session_01LDnAektYFmpUWDHPLHypkQ)_ Co-authored-by: Claude <noreply@anthropic.com>
Comment on lines
+243
to
+254
| /// True if `domain` is on the configured/default SSO allowlist | ||
| /// (case-insensitive). The explicit list only — independent of the | ||
| /// `sso_allow_any_domain` deploy flag. | ||
| fn is_explicitly_allowlisted(domain: &str) -> bool { | ||
| allowed_discovery_domains() | ||
| .iter() | ||
| .any(|allowed| allowed.eq_ignore_ascii_case(domain)) | ||
| } | ||
|
|
||
| pub fn is_allowed_discovery_domain(domain: &str) -> bool { | ||
| sso_allow_any_domain() || is_explicitly_allowlisted(domain) | ||
| } |
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.
Stacks on #4022. When deploying II for testing/staging we want to let any domain use the SSO discovery feature without curating the
sso_discoverable_domainsallowlist for each one. Today every SSO domain must be on that allowlist (or the built-inis_productiondefaults), which is friction for environments where we just want SSO open.This adds a backend deploy flag that opens the domain gate to everything, while leaving the strict-
httpsposture untouched.Changes
Backend
sso_allow_any_domain : opt boolonInternetIdentityInit, persisted inPersistentState(andStorablePersistentState) and reported back by the config query.None/Some(false)leave the allowlist in force;Some(true)opens the gate.is_allowed_discovery_domainreturns true for any domain when the flag is set, sodiscover_sso/get_sso_discoveryand the JWT verify path accept every domain instead of only allowlisted/default ones.https-relaxation gate (is_allowlisted_host) is decoupled from the flag and still consults the explicitsso_discoverable_domainslist only, so opening the domain gate never lets an arbitrary host serve discovery over plain HTTP.Frontend
InternetIdentityInitliterals for the new field. No behavior change.Tests
openid::sso::allow_any_domain_opens_the_gate.config::sso_discovery::sso_allow_any_domain_opens_the_gate— passes with all 3 sso_discovery tests against locally-built wasm + PocketIC 9.0.3.🤖 Generated with Claude Code