Skip to content

oauth2: set :scheme header on token endpoint requests from the configured URI#46110

Open
davidvo-lyft wants to merge 1 commit into
envoyproxy:mainfrom
davidvo-lyft:davidvo/38608-oauth2-scheme
Open

oauth2: set :scheme header on token endpoint requests from the configured URI#46110
davidvo-lyft wants to merge 1 commit into
envoyproxy:mainfrom
davidvo-lyft:davidvo/38608-oauth2-scheme

Conversation

@davidvo-lyft

Copy link
Copy Markdown
Contributor

Commit Message: oauth2: set :scheme header on token endpoint requests from the configured URI

Additional Description:
The OAuth2 filter never set the :scheme pseudo-header on token endpoint requests, so the embedded
async client defaulted the missing scheme to http (derived from absent downstream TLS state),
sending :scheme http even when the configured token endpoint URI used https. This takes the
scheme from the configured URI via the existing include_scheme argument of
Http::Utility::prepareHeaders, mirroring the jwks_fetcher fix in PR #37978. The behavior 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 configured URIs leave :scheme unset, so
existing configurations see no change.

Additional scope note: issue #38608 suggests auditing all prepareHeaders callers. This PR is
intentionally 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 needing
its 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, and SchemeHeaderNotSetWhenRuntimeGuardDisabled. With the fix
reverted, 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_test target passes locally
in the CI docker image (clang). check_format passes.

Docs Changes: N/A

Release Notes: changelogs/current/bug_fixes/oauth2__set-scheme-header-on-token-endpoint-requests.rst

Platform 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.

…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>
@repokitteh-read-only

Copy link
Copy Markdown

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #46110 was opened by davidvo-lyft.

see: more, trace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

use correct :scheme header in async_client requests

1 participant