feat: optionally resolve OIDC provider secrets from file:// and base64:// URIs - #4586
Draft
jasonhernandez wants to merge 1 commit into
Draft
feat: optionally resolve OIDC provider secrets from file:// and base64:// URIs#4586jasonhernandez wants to merge 1 commit into
jasonhernandez wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jasonhernandez
added a commit
to MaterializeInc/materialize-terraform-self-managed
that referenced
this pull request
Jul 29, 2026
… mounted Secret The Ory Kratos Helm chart renders all of kratos.config into a ConfigMap, so every upstream provider's client_secret lands somewhere that is not covered by Secret encryption at rest and that is readable by anyone holding `get configmap` in the namespace. With upstream_oidc_client_secret_from_file = true, the module writes each provider's client_secret to a Kubernetes Secret, mounts it into the Kratos pod, and puts a file:// URI in the configuration instead of the credential. This depends on Kratos resolving file:// for client_secret, which is not in a release yet (ory/kratos#4586). The variable therefore defaults to false and the previous inline behaviour is unchanged, so this is safe to merge ahead of that support landing. The TLS volume locals are restructured along the way. mergo replaces lists rather than appending them, so two separate deployment blocks each setting extraVolumes would have silently dropped one; the volumes are now collected and concatenated explicitly before a single merge.
jasonhernandez
force-pushed
the
feat/oidc-client-secret-from-file
branch
from
July 29, 2026 23:14
9626ac7 to
60883e5
Compare
…4:// URIs
An OIDC provider's client_secret could previously only be supplied
inline, forcing the credential into the rendered configuration — on
Kubernetes, the Helm chart's ConfigMap. When the new opt-in setting
security.allow_secret_uris_in_oidc_config is enabled, client_secret
and apple_private_key may instead be given as file:// or base64://
URIs, which are resolved to the referenced value when the provider
is used.
- Resolution is opt-in (default false) via a ctx-scoped security
setting, following the precedent of
security.disallow_ref_in_identity_schemas: whoever can write
provider configuration also controls token_url, so an always-on
file:// resolver would be an arbitrary-file-read-and-exfiltrate
primitive in deployments where provider configuration is written
by untrusted parties. With the default, URI-shaped values pass
through verbatim, preserving existing behavior.
- Secrets are resolved in Strategy.Provider on the per-use copy of
the provider configuration: provider enumeration (form rendering,
unlink, linkability) does no file I/O, a broken reference only
fails flows that use that provider, and the shared configuration
collection is never mutated, so resolved plaintext is never
written back.
- The fetcher is constructed once per strategy with
WithAllowedSchemes("file", "base64"); remote schemes keep their
current meaning as literal secrets.
- file:// values are trimmed of exactly one trailing newline (what
echo or kubectl create secret --from-file adds); base64:// decodes
to exact bytes. An empty resolved value is a misconfiguration.
- Errors identify only the provider id and field name. The
underlying fetcher error, which contains the file path, is logged
for the operator and attached via WithWrap only — not WithDebugf,
because herodot serializes the debug field into the response
returned by the self-service errors endpoint.
- Covered by unit tests observing resolution through
Strategy.Provider (trim semantics, passthrough defaults, error
hygiene, blast radius, a real PKCS#8 key for apple_private_key)
and an end-to-end TestStrategy case in which a Hydra client's
secret exists only behind a file:// reference: browser
registration and API login complete through the callback, and
removing the file yields a 500 whose body does not contain the
path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jasonhernandez
force-pushed
the
feat/oidc-client-secret-from-file
branch
from
July 29, 2026 23:50
60883e5 to
460c949
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.
Problem
An OIDC provider's
client_secret(andapple_private_key) can only be supplied inline, so the credential has to live in the Kratos configuration itself. On Kubernetes that has an awkward consequence: the official Helm chart renders all ofkratos.configinto a ConfigMap (templates/configmap-config.yaml), so upstream IdP client secrets end up somewhere thatget configmap— a far more commonly granted verb thanget secret,kubectl describe, support bundles, and cluster backups.The chart already routes
dsn,secrets.*, andcourier.smtp.connection_urithrough a Secret +secretKeyRef(templates/secrets.yaml), so the "this value is too sensitive for a ConfigMap" pattern exists — provider secrets just can't participate while inline is the only way to express them.What this PR does
When the new opt-in setting
security.allow_secret_uris_in_oidc_configis enabled,client_secretandapple_private_keymay be given asfile://orbase64://URIs — the same indirectionmapper_urlalready supports — and are resolved to the referenced value when the provider is used:On Kubernetes, mounting a Secret via the chart's existing
deployment.extraVolumes/extraVolumeMountskeeps the credential out of the ConfigMap entirely — no chart change required.Design
false), following the precedent ofsecurity.disallow_ref_in_identity_schemas(40ce7fc). Whoever can write provider configuration also controlstoken_url, so an always-onfile://resolver would be an arbitrary-file-read-and-exfiltrate primitive wherever provider config is written by untrusted parties (e.g. through an API in multi-tenant deployments). Because the setting is read fromctx, embedders can pin it per project. With the default, URI-shaped values pass through verbatim — exactly today's behavior, so the change is fully backward compatible.(*Strategy).Provider, on the per-use copy of the provider configuration. Provider enumeration (login/registration/settings form rendering, unlink, linkability checks) does no file I/O, and a broken secret reference only fails flows that actually use that provider — never the whole login screen. The shared configuration collection is never mutated, so resolved plaintext is never cached or written back.fetcher.WithAllowedSchemes("file", "base64").http(s)://is deliberately not resolved — fetching a credential over the network on the callback path is undesirable — so anhttps://value keeps its current meaning (a literal secret).os.ReadFileper actual OAuth2 exchange.file://sources only — whatechoorkubectl create secret --from-fileof a newline-terminated file adds. Leading whitespace, trailing spaces, and additional newlines are preserved;base64://decodes to exact bytes. An empty resolved value is treated as a misconfiguration (an inline empty secret remains legal for public/PKCE clients; this is documented in the schema).idand field. The underlying fetcher error (which contains the file path) is logged server-side for the operator and attached viaWithWraponly — deliberately notWithDebugf, because herodot serializes thedebugfield into the response returned by the self-service errors endpoint. The end-to-end test asserts the path appears nowhere in the error body;base64://payloads are additionally redacted byory/x/fetcheritself.Related issue(s)
There is no design document for this, so please treat it as a starting point for discussion — I'm glad to convert it into an issue first if that's the preferred path.
configxJSON-decodes env values into arrays — verified against this tree). See below for why this PR still adds value over that route.provider.client_secretfor OIDC providers from a secret in Kratos k8s#423 — asked for exactly this capability and was closed with the whole-array env-var workaround (SELFSERVICE_METHODS_OIDC_CONFIG_PROVIDERSfrom a Secret). That workaround is legitimate but all-or-nothing: every provider's non-secret configuration moves into a single opaque JSON line inside a Secret, overriding just one provider's secret is impossible, and rotation requires a pod restart (env vars are immutable per pod) — wherefile://picks up a rotated Secret volume without one.client_secretin kratos.yml for an OIDC provider? Answered with "write it in yourkratos.yml", which is the behavior this change makes optional.file://$refin identity schemas — the security-gating pattern this PR follows.Longer term, this could live in
ory/x/configxas a general secret-reference mechanism (schema-annotation driven), sodsn,courier.smtp.connection_uri, and other Ory projects get uniform behavior. This PR is deliberately scoped to the OIDC provider fields; happy to open an issue for the general mechanism.Checklist
title/description/examplesare updated here; the ory/docs social sign-in page needs a companion PR.Testing
Unit —
TestConfigSecretResolution(provider_config_test.go), observing resolution throughStrategy.Provider:file://andbase64://resolution, single-trailing-newline trim semantics (leading/extra whitespace preserved, base64 untrimmed), inline and unsupported-scheme passthrough, default-off passthrough, missing file, empty resolved value, no path/payload leakage into the error reason or debug, a real PKCS#8 EC key forapple_private_key, no mutation of the shared collection, and pinned blast radius (a broken provider affects neitherConfig()enumeration nor other providers).End-to-end —
TestStrategy/case=client_secret is resolved from a file and reaches the token exchange: a real Hydra client whose secret exists only behind afile://reference completes a browser registration and an API login through the callback — proving the resolved secret is what reaches the OAuth2 code exchange. A final step deletes the secret file and asserts the login fails with a 500 whose body does not contain the file path.Not addressed here
(*Strategy).Configlogs the raw configuration on a decode failure (s.d.Logger().WithError(err).WithField("config", conf)instrategy.go), which writes everyclient_secretinto the application log when decoding fails. That is the mirror image of the error-hygiene work in this PR and is fixed separately in myfix/oidc-config-decode-logbranch — happy to fold it in if you'd rather have both together.