feat(oauth2): expose assent_config/2 (plus an optional IdP-launch pre-exchange helper) - #2
Open
pinetops wants to merge 2 commits into
Open
feat(oauth2): expose assent_config/2 (plus an optional IdP-launch pre-exchange helper)#2pinetops wants to merge 2 commits into
pinetops wants to merge 2 commits into
Conversation
`ash` 3.32.1 carries eight published advisories, including `Ash.update_many/4`'s atomic path skipping resource policy authorization and the ETS and Mnesia data layers overwriting an existing record on create instead of enforcing primary key uniqueness. `mix hex.audit` reports no advisories after the update. `mix.exs` is unchanged; the `~> 3.7` requirement already permitted this version.
pinetops
force-pushed
the
feat/idp-launch-primitives
branch
2 times, most recently
from
September 2, 2026 04:06
e9e9787 to
689d0cf
Compare
pinetops
force-pushed
the
feat/idp-launch-primitives
branch
from
September 2, 2026 04:23
689d0cf to
68a00ae
Compare
…-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
force-pushed
the
feat/idp-launch-primitives
branch
from
September 2, 2026 04:54
68a00ae to
493f6c3
Compare
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.
What this is
This makes it easier to write a correct custom plug for one specific case: a multi-tenant app using
idp_initiated_login?(an app-launcher/aggregator IdP redirecting straight to your callback with acodeand no session), which needs to figure out which tenant the launch belongs to before the request phase restarts. Nothing about normal sign-in changes — this is exclusively about a plug an app writes and mounts itself.Today, writing that plug means hand-building an OAuth2 config for a session-less peek at the provider, and getting the CSRF opt-outs right by hand. Both are easy to get subtly wrong — a hand-picked secret list looks sufficient and silently isn't for a
private_key_jwtprovider (Apple, some enterprise IdPs); the:session_paramsopt-out is documented by Assent as optional while its code hard-requires it, which is exactly the bug #1 shipped with. This PR exposes the two pieces the library already has internally so a custom plug doesn't have to rebuild them:assent_config/2— the real configrequest/2/callback/2already build, so a plug's config can't drift from what the strategy actually needs.fetch_idp_launch_profile/2— the correct, session-less peek at the launch, with the opt-outs set explicitly rather than left to be rediscovered.A full worked example (both same-host and per-tenant-host deployments, plus the HTTP sequence) is in the new guide,
documentation/topics/multi-tenant-idp-initiated-launches.md— added by this PR, linked from the docs nav. This is not a new capability: a hand-rolled plug doing this is already in production (the one this was extracted from), and skipping it entirely costs nothing but one extra redirect through the provider's generic picker screen.What changed
assent_config/2(was the privateconfig_for/2, now public) — the substantive part. The exact configrequest/2/callback/2already build to drivestrategy.assent_strategy, exposed so an app can call the strategy's own Assent functions with a config that cannot drift out of sync with production: secret resolution (including the{module, opts}storage shape, an internal detail), redirect-URI construction, HTTP-adapter selection, and the provider-dependent key set all come from the one place that already knows them.fetch_idp_launch_profile/2— a thin convenience on top:assent_config/2plus three documented Assent opt-outs (state: false,code_verifier: false, an emptysession_params) around a direct call tostrategy.assent_strategy.callback/2. An app could do the same withassent_config/2and Assent directly — but the opt-outs are exactly where hand-rolled code goes wrong:Assent.fetch_config/2's own docs call:session_params"optional" oncallback/3while the code hard-requires it, the precise trap feat(oauth2): tenant-aware IdP-initiated restart (context + cross-host) #1 fell into. Shipping them once, as tested library code, beats a cookbook page every adopter re-implements. 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 options, no new context key threaded through secret resolution, no launch policy baked into the plug's control flow — the app composes its own small routing plug from the two primitives (below).
Verification
The new test file drives a real
Assent.Strategy.OAuth2.callback/3round-trip via a realAssent.HTTPAdapterdouble (StubHTTPAdapterstubs HTTP responses, not the strategy module) — reverting to #1's call shape reproduces the exactMissingConfigErrorabove. Full suite green (768 tests), format and credo clean.How to use
The plug is small — resolve the tenant, then either
set_tenant/2(same-host) or redirect to the tenant's own host (per-tenant-host deployments, wherestate's session cookie must be scoped to the host that will receive the callback):The full guide has both deployment shapes end to end (including the per-tenant-host redirect variant), the complete HTTP sequence for each (which host sets the
statecookie, why cross-host needs its own request-phase run), and the security notes governing what the profile may and may not be used for:documentation/topics/multi-tenant-idp-initiated-launches.md.Out of scope
idp_initiated_login_handler): deferred — these two primitives suffice; a hook can be proposed once real usage exists.iss/login_hint, no code exchange): already possible same-host with conn-aware secrets; worth its own proposal forDynamicOidc, whose compile-time rejection ofidp_initiated_login?this PR doesn't touch.