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.41% 81.44% +0.02%
==========================================
Files 266 266
Lines 41300 41360 +60
==========================================
+ Hits 33626 33685 +59
- Misses 7673 7674 +1
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 |
d278111 to
3173b27
Compare
|
/retest |
3173b27 to
f0e9fe1
Compare
|
/retest |
f0e9fe1 to
89e51fc
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>
89e51fc to
abee8d7
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