Skip to content

oauth2: fix single-page-app's pass_through_matcher configuration#819

Merged
phlax merged 1 commit intoenvoyproxy:mainfrom
ggmoy:oauth2-fix-passthrough-logic
Aug 19, 2025
Merged

oauth2: fix single-page-app's pass_through_matcher configuration#819
phlax merged 1 commit intoenvoyproxy:mainfrom
ggmoy:oauth2-fix-passthrough-logic

Conversation

@ggmoy
Copy link
Copy Markdown
Contributor

@ggmoy ggmoy commented Aug 18, 2025

Description

The changes introduced in envoyproxy/envoy PR #40228 and envoyproxy/examples PR #663 broke the passthrough functionality of the OAuth2 filter: OAuth2 cookies were removed for requests matching the pass_through_matcher configuration. This affected setups with multiple OAuth2 filter instances using different pass_through_matcher configurations, because the first matching instance removed the OAuth2 cookies--even when a passthrough was intended--impacting subsequent filters that still needed those cookies.

This PR fixes the pass_through_matcher configuration of single-page-app test, which started failing after fixing the passthrough behavior.

Signed-off-by: Gustavo Moyano <gustavo.g.moyano@gmail.com>
@phlax
Copy link
Copy Markdown
Member

phlax commented Aug 19, 2025

cc @zhaohuabing

Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

Copy link
Copy Markdown
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks all!

@phlax phlax merged commit e7d10bf into envoyproxy:main Aug 19, 2025
5 checks passed
wbpcode pushed a commit to envoyproxy/envoy that referenced this pull request Aug 26, 2025
…g filter (#40718)

### Description

This PR fixes a bug introduced in PR #40228, where OAuth2 cookies were
removed for requests matching the `pass_through_matcher` configuration.
This broke setups with multiple OAuth2 filter instances using different
`pass_through_matcher` configurations, because the first matching
instance removed the OAuth2 cookies--even when a passthrough was
intended--impacting subsequent filters that still needed those cookies.

The changes in this PR realign the filter behavior with the
documentation, which states that `pass_through_matcher` provides an
interface for users to specify header matching criteria such that, when
applicable, the OAuth flow is entirely skipped. When this occurs, only
the `oauth_passthrough` metric is incremented; no cookies or request
headers are modified, and no OAuth success is recorded.

In summary, when the `pass_through_matcher` configuration matches, the
filter should simply skip processing and leave the request untouched.

---

**Commit Message:** oauth2: fix passthrough logic to avoid modifying
request when skipping filter
**Risk Level:** Low
**Testing:** Unit test added and single page app test fixed by
[envoyproxy/examples PR
#819](envoyproxy/examples#819)
**Docs Changes:** N/A
**Release Notes:** Added
**Platform Specific Features:** N/A

---------

Signed-off-by: Gustavo Moyano <gustavo.g.moyano@gmail.com>
melginaldi pushed a commit to melginaldi/envoy that referenced this pull request Aug 26, 2025
…g filter (envoyproxy#40718)

### Description

This PR fixes a bug introduced in PR envoyproxy#40228, where OAuth2 cookies were
removed for requests matching the `pass_through_matcher` configuration.
This broke setups with multiple OAuth2 filter instances using different
`pass_through_matcher` configurations, because the first matching
instance removed the OAuth2 cookies--even when a passthrough was
intended--impacting subsequent filters that still needed those cookies.

The changes in this PR realign the filter behavior with the
documentation, which states that `pass_through_matcher` provides an
interface for users to specify header matching criteria such that, when
applicable, the OAuth flow is entirely skipped. When this occurs, only
the `oauth_passthrough` metric is incremented; no cookies or request
headers are modified, and no OAuth success is recorded.

In summary, when the `pass_through_matcher` configuration matches, the
filter should simply skip processing and leave the request untouched.

---

**Commit Message:** oauth2: fix passthrough logic to avoid modifying
request when skipping filter
**Risk Level:** Low
**Testing:** Unit test added and single page app test fixed by
[envoyproxy/examples PR
envoyproxy#819](envoyproxy/examples#819)
**Docs Changes:** N/A
**Release Notes:** Added
**Platform Specific Features:** N/A

---------

Signed-off-by: Gustavo Moyano <gustavo.g.moyano@gmail.com>
Signed-off-by: Melissa Ginaldi <mginaldi@google.com>
wtzhang23 pushed a commit to wtzhang23/envoy that referenced this pull request Aug 27, 2025
…g filter (envoyproxy#40718)

### Description

This PR fixes a bug introduced in PR envoyproxy#40228, where OAuth2 cookies were
removed for requests matching the `pass_through_matcher` configuration.
This broke setups with multiple OAuth2 filter instances using different
`pass_through_matcher` configurations, because the first matching
instance removed the OAuth2 cookies--even when a passthrough was
intended--impacting subsequent filters that still needed those cookies.

The changes in this PR realign the filter behavior with the
documentation, which states that `pass_through_matcher` provides an
interface for users to specify header matching criteria such that, when
applicable, the OAuth flow is entirely skipped. When this occurs, only
the `oauth_passthrough` metric is incremented; no cookies or request
headers are modified, and no OAuth success is recorded.

In summary, when the `pass_through_matcher` configuration matches, the
filter should simply skip processing and leave the request untouched.

---

**Commit Message:** oauth2: fix passthrough logic to avoid modifying
request when skipping filter
**Risk Level:** Low
**Testing:** Unit test added and single page app test fixed by
[envoyproxy/examples PR
envoyproxy#819](envoyproxy/examples#819)
**Docs Changes:** N/A
**Release Notes:** Added
**Platform Specific Features:** N/A

---------

Signed-off-by: Gustavo Moyano <gustavo.g.moyano@gmail.com>
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.

3 participants