oauth2: set :scheme header on token endpoint requests from the configured URI#46110
Open
davidvo-lyft wants to merge 1 commit into
Open
oauth2: set :scheme header on token endpoint requests from the configured URI#46110davidvo-lyft wants to merge 1 commit into
davidvo-lyft wants to merge 1 commit into
Conversation
…ured URI The OAuth2 filter never set the :scheme pseudo-header on token endpoint requests, so the embedded async client router defaulted the missing scheme to http from absent downstream TLS state, sending :scheme http even when the configured token endpoint URI used https. This mirrors the jwks_fetcher fix in PR envoyproxy#37978: the scheme is now taken from the configured URI via the existing prepareHeaders include_scheme argument. The change is gated by the runtime guard envoy.reloadable_features.oauth2_use_scheme_from_token_endpoint_uri (default on) so it can be reverted per deployment. Scheme-less URIs are left unchanged, so existing configurations see no behavior change. Fixes envoyproxy#38608. Signed-off-by: David Vo <davidvo@lyft.com>
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.
Commit Message: oauth2: set :scheme header on token endpoint requests from the configured URI
Additional Description:
The OAuth2 filter never set the
:schemepseudo-header on token endpoint requests, so the embeddedasync client defaulted the missing scheme to
http(derived from absent downstream TLS state),sending
:scheme httpeven when the configured token endpoint URI usedhttps. This takes thescheme from the configured URI via the existing
include_schemeargument ofHttp::Utility::prepareHeaders, mirroring the jwks_fetcher fix in PR #37978. The behavior is gatedby the runtime guard
envoy.reloadable_features.oauth2_use_scheme_from_token_endpoint_uri(defaulton) so it can be reverted per deployment. Scheme-less configured URIs leave
:schemeunset, soexisting configurations see no change.
Additional scope note: issue #38608 suggests auditing all
prepareHeaderscallers. This PR isintentionally scoped to the OAuth2 filter, matching the focused single-caller shape maintainers
requested for the jwks_fetcher fix. The near identical caller in
source/extensions/http/injected_credentials/oauth2/oauth_client.h(a separate extension needingits own runtime guard) and the OpenTelemetry access log, stat sink, and tracer callers are left as
deliberate follow-ups rather than widening this change.
Risk Level: Low (guarded, default on, behavior changes only for token endpoints configured with
an explicit scheme; scheme-less URIs unchanged).
Testing: Added
OAuth2ClientTest.SchemeHeaderSetFromHttpsUri,SchemeHeaderSetFromHttpUri,SchemeHeaderNotSetForSchemelessUri, andSchemeHeaderNotSetWhenRuntimeGuardDisabled. With the fixreverted, the two scheme-setting tests fail and the two safety tests still pass; with the fix all
four pass. The full
//test/extensions/filters/http/oauth2:oauth_client_testtarget passes locallyin the CI docker image (clang).
check_formatpasses.Docs Changes: N/A
Release Notes:
changelogs/current/bug_fixes/oauth2__set-scheme-header-on-token-endpoint-requests.rstPlatform Specific Features: N/A
Fixes #38608
Generative AI disclosure: This change was developed with AI assistance (Claude Code). I have
reviewed and understand every line, the async client scheme defaulting behavior, the runtime guard
rationale, and the test methodology, and I take full ownership of the submission and of addressing
review feedback.