Skip to content

Remove OAuth2 cookies before forwarding the requests#40228

Merged
wbpcode merged 1 commit intoenvoyproxy:mainfrom
zhaohuabing:fix-39196
Jul 16, 2025
Merged

Remove OAuth2 cookies before forwarding the requests#40228
wbpcode merged 1 commit intoenvoyproxy:mainfrom
zhaohuabing:fix-39196

Conversation

@zhaohuabing
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing commented Jul 14, 2025

Commit Message: This PR removes the following cookies before forwarding the request to the upstream: oauth_hmac, oauth_expires, refresh_token, oauth_nonce, code_verifier. These cookies are supposed to be used only in the OAuth2 flows and should not be exposed to the backend service. The id_token and access_token cookies are kept as is as they are user credentials. The later filters and the backend service may expect them to be present.

Additional Description:
Risk Level: low
Testing: unit test, also manually tested in my dev kind cluster with AWS Cognito.
Docs Changes: no
Release Notes: yes
Platform Specific Features:
[Optional Runtime guard:] This behavior can be reverted by setting the runtime guard envoy.reloadable_features.oauth2_cleanup_cookies to false.
[Optional Fixes #39196)]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@zhaohuabing zhaohuabing marked this pull request as draft July 14, 2025 12:29
@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: #40228 was opened by zhaohuabing.

see: more, trace.

@zhaohuabing zhaohuabing marked this pull request as ready for review July 15, 2025 06:38
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Copy link
Copy Markdown
Member

@wbpcode wbpcode 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.

@wbpcode wbpcode merged commit 5687093 into envoyproxy:main Jul 16, 2025
25 checks passed
@zhaohuabing zhaohuabing deleted the fix-39196 branch July 16, 2025 02:46
wbpcode pushed a commit 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.

OAuth2: remove internal cookies before forwarding request to backend

2 participants