feat(oidc): support post_logout_redirect_uri in logout - #9608
zhaohuabing wants to merge 1 commit into
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9608 +/- ##
==========================================
+ Coverage 81.32% 81.35% +0.03%
==========================================
Files 264 265 +1
Lines 41008 41110 +102
==========================================
+ Hits 33349 33447 +98
- Misses 7658 7662 +4
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
549c833 to
4eac001
Compare
4eac001 to
9159185
Compare
e81c74f to
6e3bb2f
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9159185c5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case plr.URI != nil && plr.Disabled != nil: | ||
| return errors.New("only one of OIDC.PostLogoutRedirect.uri or OIDC.PostLogoutRedirect.disabled must be set") |
There was a problem hiding this comment.
When a parent policy supplies uri and a route policy using mergeType supplies disabled: false, mergeSecurityPolicy retains both pointers and this presence-only check rejects the merged policy. This contradicts the new API documentation that disabled: false is equivalent to leaving postLogoutRedirect unset; an actually unset route value would inherit the parent's URI. Normalize false to an unset value before merging/validation, or only treat disabled: true as conflicting, so this valid route policy does not become Accepted=False merely because it is attached beneath a parent URI.
Useful? React with 👍 / 👎.
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
068e77f to
960f964
Compare
|
/retest |
960f964 to
4b177ae
Compare
|
/retest |
4b177ae to
d278111
Compare
|
/retest |
Adds spec.oidc.postLogoutRedirect to SecurityPolicy, controlling the post_logout_redirect_uri parameter Envoy sends to the OIDC provider's end session endpoint during RP-Initiated Logout. Set uri to send a specific value, or disabled to omit the parameter entirely. Envoy previously hardcoded this parameter to <scheme>://<host>/, the root of the inbound request's host. Many providers require the post logout redirect URI to be registered for the client and reject the logout request otherwise, so that default made RP-Initiated Logout unusable for them. The uri accepts the %REQ(header)% command operator so one policy can serve several hosts. Other operators are rejected up front, because Envoy fails filter creation on an unknown operator and would NACK the xDS update rather than report the mistake on the policy. The uri/disabled exclusivity is re-checked after policy merge as well, since CEL only ever sees an individual policy and a StrategicMerge can leave both fields set. Fixes envoyproxy#7349 Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
d278111 to
3173b27
Compare
|
/retest |
This branch has not been deployed
1 participant
What this PR does:
Adds
spec.oidc.postLogoutRedirectto SecurityPolicy, exposing Envoy's new OAuth2post_logout_redirect_urisetting for OIDC RP-Initiated Logout:Envoy previously hardcoded this parameter to
<scheme>://<host>/, the root of the inbound request's host. Per RP-Initiated Logout the parameter is optional, but when supplied it must be pre-registered with the provider — and the ingress root generally is not, so providers reject the whole logout request.urilets operators point at a registered landing page;disabledcovers providers that reject any unregistered value, where the parameter itself has to go.Release Notes: Yes
Fixes #7349