Return the provider document from the configuration loaders - #537
Merged
Conversation
maennchen
previously approved these changes
Aug 4, 2026
ericmj
marked this pull request as ready for review
August 4, 2026 13:56
Member
|
@ericmj Hm, you have a conflict. |
`load_configuration/2` and `load_jwks/2` hand back only the decoded record and an expiry, and the record cannot be re-encoded losslessly: `extra_fields` holds only the keys the decoder did not recognize, every other key is coerced into a typed record field, and `issuer_regex` is not an OpenID Discovery field at all. Callers that persist provider metadata therefore have no way to get at what the provider actually served. `load_configuration_raw/2` and `load_jwks_raw/2` return the decoded JSON document alongside the record. The existing functions delegate to them and drop it, so there is one loading path. The document is what came off the wire, before the `document_overrides` quirk is merged in while decoding. Both loaders now also reject a body that parses but is not a document. A discovery response of `null` used to reach `maps:merge/2` in `decode_configuration/2` and take the caller down; a JWKS may legitimately be a bare array of keys, so only scalars are rejected there and the type says so.
ericmj
force-pushed
the
provider-configuration-raw-document
branch
from
August 4, 2026 14:06
9f79cc1 to
3d2cfb3
Compare
maennchen
approved these changes
Aug 4, 2026
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.
#oidcc_provider_configuration{}can't be re-encoded back into the document it camefrom.
extra_fieldsholds only the keys the decoder didn't recognize, every other key iscoerced into a typed record field, and
issuer_regexisn't an OpenID Discovery field atall. So an RP that wants to persist what a provider served, rather than re-fetch it, has
no way to get it.
That's the normal shape for anything not using
oidcc_provider_configuration_worker.hex.pm runs OIDC SSO per organization: the provider config lives in Postgres, is shared
across web nodes, and every request rebuilds the client context from the stored document
with
oidcc_client_context:from_manual/4. Storing the record isn't an option, since it'san Erlang record whose shape changes between oidcc versions and a serialized copy breaks
on upgrade. The JSON is the stable thing to store.
With no way to ask oidcc for it, the workaround is to capture it out of band. hex.pm's
oidcc_http_adapterstashes every response in the process dictionary and the callerreads it back and decodes the body a second time. That works, but it's correct only by
accident: the stash is last-write-wins and unkeyed, so it depends on
load_configuration/2and
load_jwks/2each issuing exactly one HTTP request. Any change inside oidcc that addsa second request to either one, a redirect follow or a retry, silently hands back the
wrong document with no error. That's a hard dependency on an implementation detail oidcc
never promised.
This adds
load_configuration_raw/2andload_jwks_raw/2, returning{ok, {Configuration, Expiry, Document}}. The existing 2-arity functions become one-linedelegations that drop the third element, so nothing existing changes shape.
Documentisthe decoded map as it came off the wire, taken before
decode_configuration/2mergesdocument_overridesinto it, so what comes back is what the provider sent and not whatoidcc made of it.
Two alternatives I discarded. A new field on
#oidcc_provider_configuration{}would reachthe Elixir struct for free, but the hrl is public API shipped in the Hex package, so it
forces downstream recompilation, and
#oidcc_client_context{}embeds the configurationrecord and gets passed to every operation, so every context would carry a second copy of
the document. An opts flag that changes the return shape has no precedent anywhere in
src/orlib/, and dialyzer can't express it. The closest existing precedent for a widersuccess tuple is
oidcc_profile:apply_profiles/2returning{ok, ClientContext, Opts}.Splitting the loaders surfaced a separate crash, fixed here because the fix is two lines
in the code this PR already moves. Both loaders assumed the decoded JSON was an object. A
provider answering
nullunder a JSON content type reachedmaps:merge/2insidedecode_configuration/2, and for JWKS a scalar reachedjose_jwk:from/1, either of whichtakes the calling process down. Both now return
{error, {invalid_document, Document}}, anew member of
oidcc_provider_configuration:error(). JWKS accepts an object or a barearray, since
jose_jwk:from/1takes both and some providers serve the array form.The Elixir wrappers mirror both functions and
@typeentries are updated by hand as usual.