feat(oauth2): tenant-aware IdP-initiated restart (context + cross-host) - #1
Draft
pinetops wants to merge 7 commits into
Draft
feat(oauth2): tenant-aware IdP-initiated restart (context + cross-host)#1pinetops wants to merge 7 commits into
pinetops wants to merge 7 commits into
Conversation
…ontext
`idp_initiated_login?` restarts the request phase from a stateless callback
to mint a verifiable `state` (OIDC Core §4). For a single-tenant provider
that is enough. For a multi-tenant one whose `authorize_url` is per-tenant it
is not: on an IdP-initiated launch the tenant is only knowable from the launch
itself — the profile behind the `code` — which the plain restart never reads,
so `authorize_url` cannot route and falls back to a generic picker.
On the restart, read-only and best-effort, exchange the launch's `code` and
fetch the profile, then surface it to the request phase's secret-resolution
context as `:idp_initiated_user_info`. A tenant-aware `authorize_url` /
`redirect_uri` secret function reads it to route the restart at the launch's
tenant:
def secret_for([_, _, _, :authorize_url], _resource, _opts, context) do
case context do
%{idp_initiated_user_info: %{"id" => id}} -> {:ok, tenant_url(id)}
_ -> {:ok, @default_authorize_url}
end
end
- `request/2` -> `request/3` with an optional `context_extra` merged into the
`%{conn: conn}` secret context (the existing secret-resolution seam).
- `maybe_reflect_or_fail` pre-exchanges via `config_for` + the assent
strategy's `callback`; fails open to a plain restart on any error.
- `AshAuthentication.Secret` documents the new `:idp_initiated_user_info`
context key.
Read-only (mints no session/token/user), fail-open (single-tenant and existing
behaviour untouched — the full OAuth2/OIDC suite passes, and the pre-existing
`idp_initiated` tests still hold), and single-use-code safe (the restart mints
a fresh authorize -> fresh code, so the code consumed here is not reused).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extends the IdP-initiated context work with the *cross-host* case, which
per-tenant-subdomain deployments actually need.
Two shapes of multi-tenant IdP-initiated routing, depending on where the
tenant lives:
* Same host, per-tenant config — the launch profile is surfaced to the
request phase's secret context as `:idp_initiated_user_info`, and a
tenant-aware `authorize_url`/`redirect_uri` secret routes on it.
* Per-tenant HOST — each tenant is served from its own host (subdomain).
The restart must run *on that host*, because the CSRF `state` is stored
by the host that will receive the callback (host-scoped cookies); a
restart on the wrong host loses it. The new optional
`idp_initiated_request_url` secret resolves the tenant's request-phase
URL from the launch context, and the plug redirects the browser there
instead of restarting inline. That host then runs the normal request
phase and stores `state` where the callback will read it.
- `idp_initiated_request_url` DSL option + struct field (a secret;
resolved only on the restart, with the launch context).
- `maybe_reflect_or_fail` → `idp_initiated_restart`: resolve the request
URL; redirect cross-host if present, else restart inline.
- `AshAuthentication.Secret` documents the new secret and that a
context-aware secret must be a module (an inline function secret never
receives context).
Backward compatible: the field defaults to nil → inline restart, existing
behaviour. Full OAuth2/OIDC suite green; new tests cover the cross-host
handoff (redirects to the resolved host, stores no local `state`) and the
inline fall-through.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The read-only pre-exchange that surfaces the launch profile to context is a
token+profile round-trip. It is only worth doing when something consumes the
profile, so run it only when the strategy opts in:
- `idp_initiated_request_url` is set (its consumer, cross-host), or
- `resolve_idp_initiated_launch?` is `true` (same-host: a tenant-aware
`authorize_url`/`redirect_uri` secret reads the surfaced profile).
Otherwise the restart is a plain redirect with no exchange — byte-identical
to the pre-existing behaviour, no wasted call. This avoids exchanging the
launch code twice on the common inline path (once to populate context, then
again on the fresh authorize round-trip) when nothing reads the profile.
Adds the `resolve_idp_initiated_launch?` DSL option + struct field; documents
the gate on `AshAuthentication.Secret`. New test asserts no pre-exchange
happens without opt-in.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…secret
The pre-exchange gate was a plain boolean on the strategy, so it fired on
every IdP-initiated launch even when the tenant was already determinable
from the request (e.g. the tenant's own subdomain host) — a wasted
round-trip on those launches.
Let `resolve_idp_initiated_launch?` also be a `Secret` module, resolved with
`%{conn: conn}` *before* the exchange. The app can then decide per-request —
"pre-exchange only when the host carries no tenant" — so a request that
already identifies its tenant skips the exchange entirely. Boolean form is
unchanged; `idp_initiated_request_url` still implies the gate.
(A context-aware gate must be a Secret module; an inline function secret does
not receive the conn.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…quest_url "district" was education-domain jargon leaking into a generic framework option. Reword the doc string to "per-tenant subdomain" and regenerate the cheat sheets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Picks up idp_initiated_login?'s doc-string mentioning the not state_param?(conn) fail-closed guard (from the rebase onto upstream's team-alembic#1205, which added that hardening after this branch's commits were originally written), plus the new idp_initiated_request_url / resolve_idp_initiated_launch? fields.
Credo flagged the fully-qualified Assent.Strategy.OAuth2 references inside the new IdP-initiated pre-exchange tests as nested modules that should be aliased at the top of the invoking module.
pinetops
force-pushed
the
feat/idp-initiated-tenant-context
branch
from
September 2, 2026 00:50
067e35b to
44bbc85
Compare
pinetops
changed the base branch from
fix/oauth2-allow-empty-session-params
to
main
September 2, 2026 00:50
pinetops
added a commit
that referenced
this pull request
Sep 2, 2026
…-exchange helper) Supersedes the previous branch (idp_initiated_request_url / resolve_idp_initiated_launch? DSL options plus a built-in pre-exchange in the plug's control flow). Review of that branch found its central mechanism -- calling strategy.assent_strategy.callback/2 with no :session_params -- silently no-ops against every stock OAuth2/OIDC strategy (Assent.Strategy.OAuth2.callback/3 requires :session_params and raises MissingConfigError without it); its own tests hid this by Mimic-stubbing the strategy module. This branch replaces it with two small, independently useful primitives: 1. assent_config/2 (was the private config_for/2, now public): the exact config request/2 and callback/2 build to drive strategy.assent_strategy, exposed so an app can call the strategy's own Assent functions with a config that matches production instead of re-deriving secret resolution, redirect-URI construction, and HTTP-adapter selection by hand. 2. fetch_idp_launch_profile/2: optional convenience, not a capability gap -- an app can call assent_config/2 and Assent directly with the same three opt-outs (state: false, code_verifier: false, an empty session_params). This just spares every caller from re-deriving them, one of which (session_params) Assent's own docs call "optional" while the code hard-requires it -- exactly the trap PR #1 fell into. Its doc carries the load-bearing security properties: read-only; the profile is provider-authenticated but not session-bound (routing only, never sign-in/provisioning); the spent code is never reused (the restart mints a fresh one); redirect targets derived from the profile must be allowlist- or DB-validated. No new DSL surface, no context key threaded through secret resolution, no launch policy baked into the plug -- an app composes its own small routing plug from the two primitives. Of the two, only assent_config/2 closes a real gap: the flow this is for (multi-tenant IdP-initiated routing) is already achievable today with zero library changes -- a hand-rolled app plug doing exactly this is in production use -- but a hand-rolled config builder is fragile against internals (the {module, opts} secret-storage shape, which secrets a given auth_method actually needs; a client_secret-only recipe silently breaks for private_key_jwt providers like Apple). fetch_idp_launch_profile/2 is take-it-or-leave-it sugar on top. Verified with a test that drives a real Assent.Strategy.OAuth2 round-trip via a real Assent.HTTPAdapter double (StubHTTPAdapter, stubbing HTTP responses, not the strategy module) -- reverting to the old call shape reproduces the exact MissingConfigError. Full suite green (768 tests), format and credo clean. Original-ask: "ask fable if there's a different approach worth taking? is there a more general customization tactic that isn't so tied to clever/classlink?" followed by "ok, update PR to implement the two small primitives" Considered-alternatives: - Fixing the previous branch's pre-exchange bug in place, keeping its DSL options: rejected -- the problem is narrower than those options assume (US K-12 SSO aggregators redirecting straight to the callback with a bare code), and encoding that pattern as permanent DSL surface is worse than composable primitives. - A pluggable resolver-hook behaviour (idp_initiated_login_handler): deferred, not rejected -- these two primitives suffice; a hook can be proposed once real usage exists. Out-of-scope: real OIDC Core §4 third-party-initiated login (a dedicated initiation endpoint carrying iss/login_hint, no code exchange) -- already possible same-host with conn-aware secrets, and worth its own proposal for DynamicOidc, whose compile-time rejection of idp_initiated_login? this change does not touch.
pinetops
added a commit
that referenced
this pull request
Sep 2, 2026
…-exchange helper) Supersedes the previous branch (idp_initiated_request_url / resolve_idp_initiated_launch? DSL options plus a built-in pre-exchange in the plug's control flow). Review of that branch found its central mechanism -- calling strategy.assent_strategy.callback/2 with no :session_params -- silently no-ops against every stock OAuth2/OIDC strategy (Assent.Strategy.OAuth2.callback/3 requires :session_params and raises MissingConfigError without it); its own tests hid this by Mimic-stubbing the strategy module. This branch replaces it with two small, independently useful primitives: 1. assent_config/2 (was the private config_for/2, now public): the exact config request/2 and callback/2 build to drive strategy.assent_strategy, exposed so an app can call the strategy's own Assent functions with a config that matches production instead of re-deriving secret resolution, redirect-URI construction, and HTTP-adapter selection by hand. 2. fetch_idp_launch_profile/2: optional convenience, not a capability gap -- an app can call assent_config/2 and Assent directly with the same three opt-outs (state: false, code_verifier: false, an empty session_params). This just spares every caller from re-deriving them, one of which (session_params) Assent's own docs call "optional" while the code hard-requires it -- exactly the trap PR #1 fell into. Its doc carries the load-bearing security properties: read-only; the profile is provider-authenticated but not session-bound (routing only, never sign-in/provisioning); the spent code is never reused (the restart mints a fresh one); redirect targets derived from the profile must be allowlist- or DB-validated. No new DSL surface, no context key threaded through secret resolution, no launch policy baked into the plug -- an app composes its own small routing plug from the two primitives. Of the two, only assent_config/2 closes a real gap: the flow this is for (multi-tenant IdP-initiated routing) is already achievable today with zero library changes -- a hand-rolled app plug doing exactly this is in production use -- but a hand-rolled config builder is fragile against internals (the {module, opts} secret-storage shape, which secrets a given auth_method actually needs; a client_secret-only recipe silently breaks for private_key_jwt providers like Apple). fetch_idp_launch_profile/2 is a thin convenience kept because the opt-outs it encodes are exactly where hand-rolled code goes wrong, and adopting both lets the production comparator delete 100+ lines of parallel security-adjacent code, including a custom Assent strategy fork that existed only to tolerate a missing session_params key. Verified with a test that drives a real Assent.Strategy.OAuth2 round-trip via a real Assent.HTTPAdapter double (StubHTTPAdapter, stubbing HTTP responses, not the strategy module) -- reverting to the old call shape reproduces the exact MissingConfigError. Full suite green (768 tests), format and credo clean. Original-ask: "ask fable if there's a different approach worth taking? is there a more general customization tactic that isn't so tied to clever/classlink?" followed by "ok, update PR to implement the two small primitives" Considered-alternatives: - Fixing the previous branch's pre-exchange bug in place, keeping its DSL options: rejected -- the problem is narrower than those options assume (US K-12 SSO aggregators redirecting straight to the callback with a bare code), and encoding that pattern as permanent DSL surface is worse than composable primitives. - A pluggable resolver-hook behaviour (idp_initiated_login_handler): deferred, not rejected -- these two primitives suffice; a hook can be proposed once real usage exists. Out-of-scope: real OIDC Core §4 third-party-initiated login (a dedicated initiation endpoint carrying iss/login_hint, no code exchange) -- already possible same-host with conn-aware secrets, and worth its own proposal for DynamicOidc, whose compile-time rejection of idp_initiated_login? this change does not touch.
pinetops
added a commit
that referenced
this pull request
Sep 2, 2026
…-exchange helper) Supersedes the previous branch (idp_initiated_request_url / resolve_idp_initiated_launch? DSL options plus a built-in pre-exchange in the plug's control flow). Review of that branch found its central mechanism -- calling strategy.assent_strategy.callback/2 with no :session_params -- silently no-ops against every stock OAuth2/OIDC strategy (Assent.Strategy.OAuth2.callback/3 requires :session_params and raises MissingConfigError without it); its own tests hid this by Mimic-stubbing the strategy module. This branch replaces it with two small, independently useful primitives: 1. assent_config/2 (was the private config_for/2, now public): the exact config request/2 and callback/2 build to drive strategy.assent_strategy, exposed so an app can call the strategy's own Assent functions with a config that matches production instead of re-deriving secret resolution, redirect-URI construction, and HTTP-adapter selection by hand. 2. fetch_idp_launch_profile/2: optional convenience, not a capability gap -- an app can call assent_config/2 and Assent directly with the same three opt-outs (state: false, code_verifier: false, an empty session_params). This just spares every caller from re-deriving them, one of which (session_params) Assent's own docs call "optional" while the code hard-requires it -- exactly the trap PR #1 fell into. Its doc carries the load-bearing security properties: read-only; the profile is provider-authenticated but not session-bound (routing only, never sign-in/provisioning); the spent code is never reused (the restart mints a fresh one); redirect targets derived from the profile must be allowlist- or DB-validated. No new DSL surface, no context key threaded through secret resolution, no launch policy baked into the plug -- an app composes its own small routing plug from the two primitives. Of the two, only assent_config/2 closes a real gap: the flow this is for (multi-tenant IdP-initiated routing) is already achievable today with zero library changes -- a hand-rolled app plug doing exactly this is in production use -- but a hand-rolled config builder is fragile against internals (the {module, opts} secret-storage shape, which secrets a given auth_method actually needs; a client_secret-only recipe silently breaks for private_key_jwt providers like Apple). fetch_idp_launch_profile/2 is a thin convenience kept because the opt-outs it encodes are exactly where hand-rolled code goes wrong, and adopting both lets the production comparator delete 100+ lines of parallel security-adjacent code, including a custom Assent strategy fork that existed only to tolerate a missing session_params key. Verified with a test that drives a real Assent.Strategy.OAuth2 round-trip via a real Assent.HTTPAdapter double (StubHTTPAdapter, stubbing HTTP responses, not the strategy module) -- reverting to the old call shape reproduces the exact MissingConfigError. Full suite green (768 tests), format and credo clean. Original-ask: "ask fable if there's a different approach worth taking? is there a more general customization tactic that isn't so tied to clever/classlink?" followed by "ok, update PR to implement the two small primitives" Considered-alternatives: - Fixing the previous branch's pre-exchange bug in place, keeping its DSL options: rejected -- the problem is narrower than those options assume (US K-12 SSO aggregators redirecting straight to the callback with a bare code), and encoding that pattern as permanent DSL surface is worse than composable primitives. - A pluggable resolver-hook behaviour (idp_initiated_login_handler): deferred, not rejected -- these two primitives suffice; a hook can be proposed once real usage exists. Out-of-scope: real OIDC Core §4 third-party-initiated login (a dedicated initiation endpoint carrying iss/login_hint, no code exchange) -- already possible same-host with conn-aware secrets, and worth its own proposal for DynamicOidc, whose compile-time rejection of idp_initiated_login? this change does not touch.
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.
The problem
idp_initiated_login?(from team-alembic#1184) restarts the request phase from a stateless callback to mint a verifiablestate(OIDC Core §4). Enough for a single-tenant provider. A multi-tenant one needs to know which tenant to restart for — and on an IdP-initiated launch the tenant is only knowable from the launch itself (the profile behind thecode), which the plain restart never reads. So the restart can't route, and lands on a generic picker/login screen.Two flavours of this, depending on where the tenant lives:
authorize_url/redirect_urivary by tenant.stateis stored by the host that will receive the callback (host-scoped cookies), so restarting on the wrong host loses it. (This is the Ed.link district-SSO case: a global Clever launch on the bare host must end up authenticating on the district's subdomain.)Both are solvable today only with a bespoke app plug that session-lessly pre-exchanges the code, resolves the tenant, and redirects. Every consumer reinvents it. It should be the strategy's job.
The change (five commits)
1. Surface the launch profile to context (
4ce8025)On the restart, read-only and best-effort, exchange the launch's
code, fetch the profile, and put it in the secret-resolution context as:idp_initiated_user_info— the same map yourregister/sign_inaction already receives. A tenant-aware secret reads it:2. Route the restart to a resolved host (
23fec10)New optional
idp_initiated_request_urlsecret. When present, the plug resolves the tenant's request-phase URL from the launch context and redirects the browser there instead of restarting inline — so the target host runs the request phase and storesstatewhere the callback will read it. Absent → inline restart (unchanged).3. Gate the pre-exchange behind opt-in (
de0a523)The pre-exchange now only runs when
resolve_idp_initiated_launch?is truthy — a wasted round-trip on providers that never configure a tenant-aware secret is avoided by default.4. Allow a conn-aware gate (
3b79191)resolve_idp_initiated_launch?can also be aSecretmodule, resolved with%{conn: conn}before the exchange — e.g. "only pre-exchange when the request's host carries no tenant." A plain boolean still works unchanged;idp_initiated_request_urlstill implies the gate.5. Docs wording (
067e35b)Properties
state-verified auth is unchanged.code.Secretmodule (secret_for/4) receivescontext; an inline function secret is called with just(name, resource). Documented onAshAuthentication.Secret.Open questions for the maintainer
Gating the pre-exchange.Resolved (seede0a523/3b79191): opt-in viaresolve_idp_initiated_launch?, which can also be a conn-awareSecretmodule (e.g. "only pre-exchange when the host carries no tenant") — not just a plain boolean.idp_initiated_request_urlstill implies the gate.:idp_initiated_user_infoin context (same-host) andidp_initiated_request_url(cross-host) are separate mechanisms. Is that the right split, or should cross-host also be expressed through the existing secrets?:idp_initiated_user_info,idp_initiated_request_url.config_for. The pre-exchange reuses the plug's privateconfig_for/2; making it public would let apps share one config builder.Downstream payoff
The Ed.link district-SSO stack collapses: the consuming app deletes its entire bespoke IdP-subdomain plug (bare-host detection, session-less exchange, redirect, router wiring). It configures
idp_initiated_request_urlas aSecretmodule that maps the launch's person id → the district's subdomain request URL — the app keeps only that genuinely app-specific mapping, and the framework owns the mechanism (including the host-scopedstatecorrectness that a naive app plug can get wrong).🤖 Generated with Claude Code