api(oidc): Allow override of all OIDC CookieNames#5655
api(oidc): Allow override of all OIDC CookieNames#5655coro wants to merge 1 commit intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5655 +/- ##
==========================================
- Coverage 65.28% 65.22% -0.07%
==========================================
Files 213 213
Lines 34132 34132
==========================================
- Hits 22282 22261 -21
- Misses 10515 10531 +16
- Partials 1335 1340 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zhaohuabing
left a comment
There was a problem hiding this comment.
@coro Thanks for picking this up. Could you please also add CodeVerifier cookie?
reference: envoyproxy/envoy#37849
|
A second thought, we probably shouldn't expose these cookies to the API - not like the ID and Access tokens, they're some trivial internal implementation details. I'd rather remove these cookies by default in the OAuth2 filter. |
|
@zhaohuabing so just to confirm, you'd like no configurability for any of the |
|
@coro sorry for the confusion. I meant that the existing AccessToken and IDToken cookies are needed in the API because some backend applications may need them for auth purposes/getting user identity, but all the other cookies should not be exposed to the user-facing API and just be deleted before forwarding the request to the Backend applications. We should fix this in the Envoy OAuth2 filter. Does this make sense? |
|
That makes sense. I'm happy for this to be closed. I'll leave a comment in the original issue about this. |
|
Tracked in this Enovy issue: envoyproxy/envoy#39196 |
What type of PR is this?
What this PR does / why we need it:
Allows for:
Which issue(s) this PR fixes:
Fixes #5152
Release Notes: No