feat: allow the same OIDC configuration target multiple HTTPRoutes #3252
Conversation
zhaohuabing
force-pushed
the
oidc-enhancement
branch
2 times, most recently
from
2fc6b38
to
a131c18
Compare
| // We should change this back to use policy UID after Gateway API supports | ||
| // targeting a policy to multiple routes. | ||
| // See https://github.com/kubernetes-sigs/gateway-api/discussions/2927#discussioncomment-8991869 | ||
| suffix := utils.Digest32(*oidc.RedirectURL) |
There was a problem hiding this comment.
The oidc is not present in every security policy and, if it's present, the redirectURL might be missing. I think the code should default to policy.UID, unless the both conditions are met
There was a problem hiding this comment.
If oidc is not present, then the oauth2 filter won't be created. If RedirectURL is missing, a default RedirectURL is used. However, we should use ClientID here.
| @@ -589,7 +589,18 @@ func (t *Translator) buildOIDC( | |||
| } | |||
|
|
|||
| // Generate a unique cookie suffix for oauth filters | |||
| suffix := utils.Digest32(string(policy.UID)) | |||
| // We use the digest of the redirect URL to generate the cookie suffix so that | |||
There was a problem hiding this comment.
i dont think this is is needed
#2944 adds support for a single oauth2 filter based on the policy name instead of 1 per route, so using a suffix off policy should work
There was a problem hiding this comment.
For SecurityPolicy targeting Gateway, this is not needed. The purpose is to support multiple routes with the same OIDC configuration.
There was a problem hiding this comment.
the issue linked is #2913 which is referring to targeting a SecurityPolicy to a Gateway with oidc set
suggest opening a new issue to handle this case of how to author different SecurityPolicies targeting different routes and yet reuse the same cookie prefix, for that case, my suggestion would be to expose an optional CookiePrefix field in the API
There was a problem hiding this comment.
I believe only the OIDC configurations with the same ClientID should/must use the same CookiePrefix. If this is true, exposing CookiePrefix to API would be unnecessary and result in abuse.
zhaohuabing
force-pushed
the
oidc-enhancement
branch
from
a131c18
to
f47625e
Compare
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
zhaohuabing
force-pushed
the
oidc-enhancement
branch
from
f47625e
to
7087045
Compare
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
zhaohuabing
changed the title
|
@sadovnikov In today's meeting, @arkodg mentioned that EG is going to support targeting a policy to multiple targets. I suggest we hold this and focus on #2999. |
|
@zhaohuabing sounds good. Do you have an idea when https://github.com/kubernetes-sigs/gateway-api will have a new release? |
The decision has been made, and It doesn't rely on gateway API since |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
This PR changes OIDC cookie suffix to OIDC
clientIDso multipleHTTPRoutescan share the same OIDC configuration.Related: #3253
@sadovnikov