Skip to content

oauth2 filter: encrypt tokens#39059

Merged
wbpcode merged 24 commits intoenvoyproxy:mainfrom
zhaohuabing:oauth2-encrypt-tokens
Jul 5, 2025
Merged

oauth2 filter: encrypt tokens#39059
wbpcode merged 24 commits intoenvoyproxy:mainfrom
zhaohuabing:oauth2-encrypt-tokens

Conversation

@zhaohuabing
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing commented Apr 10, 2025 •
edited
Loading

Commit Message: This PR encrypts the access, ID, and refresh tokens for the OAuth2 filter.

Risk Level: low
Testing: Unit tests and integration tests. I also manually verified that this worked using AWS Cognito.
Docs Changes: No user-facing change, as the token cookies are decrypted before forwarding to the backend service.
Release Notes: Yes
Implements #23508

@denniskniep

@zhaohuabing zhaohuabing marked this pull request as draft April 10, 2025 05:44
@zhaohuabing zhaohuabing force-pushed the oauth2-encrypt-tokens branch 4 times, most recently from 0c894d8 to 84dc803 Compare April 16, 2025 10:15
@github-actions
Copy link
Copy Markdown

github-actions bot commented May 16, 2025

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label May 16, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented May 23, 2025

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this May 23, 2025
@wbpcode wbpcode reopened this Jun 6, 2025
@zhaohuabing zhaohuabing force-pushed the oauth2-encrypt-tokens branch from 84dc803 to 47b4c52 Compare June 6, 2025 02:23
@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Jun 6, 2025
@zhaohuabing zhaohuabing force-pushed the oauth2-encrypt-tokens branch 3 times, most recently from b6b043b to 4b95c2d Compare June 14, 2025 11:48
@zhaohuabing zhaohuabing mentioned this pull request Jun 15, 2025
Merged
@zhaohuabing zhaohuabing marked this pull request as ready for review June 19, 2025 23:47
zhaohuabing added 11 commits June 19, 2025 23:48
@zhaohuabing
encrypt oauth2 access and id tokens
a1d7869 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
fix test
40b3bc5 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
format
4d82c58 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
fix test
d664edb 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
remove debug info
81f2d68 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
fix test
4c5e9ea 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
fix integration test
178f3c1 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
fix example verification
07c3367 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
Revert "fix example verification"
cf8da0f 
This reverts commit 5261a1f.

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
fixt test
eb08d5e 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
more test
3eff601 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
Copy link
Copy Markdown
Member Author

zhaohuabing commented Jun 24, 2025

  • The encrypted cookies are used only by the the OAuth2 filters - they can't be accessed by client side applications like java scripts since they're http-only cookies.

From proxy/client's perspective, some set-cookie values format are changed (no backward compatibility), I would think it's a minor behavior change. And a flag with default value true should be harmless.

Sounds good. I'm adding a runtime guard.

@zhaohuabing
add runtime guard
c2e059d 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Jun 24, 2025

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

🐱

Caused by: #39059 was synchronize by zhaohuabing.

see: more, trace.

@zhaohuabing
update change log
2322541 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
Merge remote-tracking branch 'origin/main' into oauth2-encrypt-tokens
22ab663
@zhaohuabing
make methods const
7d015d6 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
minor change
6a39095 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
fix test
b3e0f66 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
Merge remote-tracking branch 'origin/main' into oauth2-encrypt-tokens
c3985c0
@zhaohuabing

This comment was marked as resolved.

Sign in to view
@phlax
Copy link
Copy Markdown
Member

phlax commented Jun 24, 2025 •
edited
Loading

its in the wrong section - possibly you want 2 changelog entries

see below

phlax
phlax reviewed Jun 24, 2025
View reviewed changes
@zhaohuabing @phlax
Update changelogs/current.yaml
ac63f13 
Co-authored-by: phlax <phlax@users.noreply.github.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing requested review from arkodg and phlax June 25, 2025 00:40
wbpcode
wbpcode reviewed Jun 25, 2025
View reviewed changes
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 with one nit comment.

@zhaohuabing
address comment
488d2dc 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing requested a review from wbpcode June 25, 2025 10:26
@zhaohuabing
Copy link
Copy Markdown
Member Author

zhaohuabing commented Jun 26, 2025

/retest

@zhaohuabing
Merge branch 'main' into oauth2-encrypt-tokens
304f4b8 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
wbpcode
wbpcode reviewed Jun 28, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@wbpcode wbpcode left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for the delay. LGTM with code and only one last comment. Could you add only single one additional test to ensure the legacy code path (no encryption) still works as expected.

@zhaohuabing
tests for runtime flag disabled
05163db 
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@wbpcode
Copy link
Copy Markdown
Member

wbpcode commented Jul 4, 2025

/retest

@zhaohuabing zhaohuabing requested a review from wbpcode July 4, 2025 11:25
wbpcode
wbpcode approved these changes Jul 5, 2025
View reviewed changes
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 26c0c23 into envoyproxy:main Jul 5, 2025
25 checks passed
@zhaohuabing zhaohuabing mentioned this pull request Jul 8, 2025
Closed
@zetaab zetaab mentioned this pull request Aug 28, 2025
Closed
@zhaohuabing zhaohuabing deleted the oauth2-encrypt-tokens branch August 30, 2025 05:55
@mathetake
Copy link
Copy Markdown
Member

mathetake commented Mar 23, 2026

I wonder if this encryption stuff should be runtime flag in the first place vs the API knob. This is a breaking change to any existing deployment and at least we will NEVER be able to turn off the runtime flag.

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

Reviewers

@wbpcode wbpcode wbpcode approved these changes

@mattklein123 mattklein123 Awaiting requested review from mattklein123 mattklein123 is a code owner

@arkodg arkodg Awaiting requested review from arkodg

@phlax phlax Awaiting requested review from phlax

Assignees

@wbpcode wbpcode

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@zhaohuabing @wbpcode @phlax @mathetake @arkodg