feat(oidc): forward id token#9367
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
zhaohuabing
force-pushed
the
forward_id_token
branch
2 times, most recently
from
b20f2c5 to
be813bc
Compare
|
@codex review |
zhaohuabing
force-pushed
the
forward_id_token
branch
2 times, most recently
from
2331c21 to
3558ee6
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for forwarding an OIDC ID token to upstream requests via a new SecurityPolicy.spec.oidc.forwardIDToken field, wiring it through Gateway API translation into the generated Envoy OAuth2 filter config, and updating docs/tests accordingly.
Changes:
- Introduces
forwardIDTokenAPI + CRD schema validation (header name constraints, Host disallow, Authorization conflict withforwardAccessToken). - Translates
forwardIDTokeninto IR and Envoy OAuth2 per-route config (ForwardIdToken), including special handling when targetingAuthorization. - Updates e2e/translation/CEL tests, generated docs, and bumps go-control-plane to a version that includes the needed Envoy API.
Reviewed changes
Copilot reviewed 14 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
api/v1alpha1/oidc_types.go |
Adds forwardIDToken API field + kubebuilder/CEL validations and header constraints. |
charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml |
Generated CRD schema updates for forwardIDToken.header validation/description. |
charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml |
Same CRD schema updates in the other Helm chart packaging. |
go.mod |
Bumps go-control-plane deps to pick up required Envoy OAuth2 API. |
go.sum |
Corresponding sum updates for module dependency bump. |
internal/gatewayapi/securitypolicy.go |
Validates forwardIDToken during translation and plumbs it into IR. |
internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml |
Adds forwardIDToken input to GatewayAPI translation testdata. |
internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml |
Updates expected GatewayAPI translation output/IR with forwardIDToken. |
internal/ir/xds.go |
Extends IR OIDC struct with ForwardIDToken. |
internal/ir/zz_generated.deepcopy.go |
Generated deepcopy support for new IR field. |
internal/xds/translator/oidc.go |
Emits Envoy OAuth2 ForwardIdToken and adjusts PreserveAuthorizationHeader when needed. |
internal/xds/translator/testdata/in/xds-ir/oidc.yaml |
Adds forwardIDToken to translator IR input testdata. |
internal/xds/translator/testdata/out/xds-ir/oidc.routes.yaml |
Updates expected translator output to include forwardIdToken config. |
release-notes/current/new_features/9366-oidc-forward-id-token.md |
Release note fragment describing the new feature. |
site/content/en/latest/api/extension_types.md |
Generated API docs updated for the new forwardIDToken field. |
test/cel-validation/securitypolicy_test.go |
Adds CEL validation test cases for header constraints and Authorization conflict. |
test/e2e/testdata/oidc-securitypolicy.yaml |
Configures e2e SecurityPolicy to enable ID token forwarding on a custom header. |
test/e2e/tests/oidc.go |
Extends OIDC e2e test to assert the upstream receives the configured forwarded header. |
test/go.mod |
Updates test module dependencies in lockstep with root module. |
test/go.sum |
Corresponding sum updates for the test module dependency bump. |
Files not reviewed (1)
- internal/ir/zz_generated.deepcopy.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: be813bc40d
ℹ️ 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".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9367 +/- ##
==========================================
+ Coverage 75.18% 75.21% +0.03%
==========================================
Files 252 252
Lines 41037 41056 +19
==========================================
+ Hits 30854 30881 +27
+ Misses 8088 8081 -7
+ Partials 2095 2094 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
zhaohuabing
force-pushed
the
forward_id_token
branch
from
3558ee6 to
57217e8
Compare
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
zhaohuabing
force-pushed
the
forward_id_token
branch
from
57217e8 to
f728b58
Compare
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
gateway/api/v1alpha1/oidc_types.go
Line 19 in b741a02
Reject passthrough conflicts with ID-token headers
This validation only blocks the forwardAccessToken/Authorization collision, but the new field is still accepted with passThroughAuthHeader: true and forwardIDToken.header equal to a JWT extraction header, including the default Authorization header. The new API comment below documents that Envoy strips the configured forward-ID-token header before JWT authn can read it; with the existing OAuth2-before-JWT filter order and buildHeaderMatchers defaulting passthrough to Authorization: Bearer, such non-browser bearer requests are stripped and redirected/rejected instead of being validated by JWT. Please reject this combination in validation/translation or avoid stripping the passthrough header path.
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 553ce53810
ℹ️ 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".
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
zhaohuabing
force-pushed
the
forward_id_token
branch
from
553ce53 to
6ac3cb9
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ac3cb9cf7
ℹ️ 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".
| if strings.EqualFold(fwdHeader, "Authorization") { | ||
| return fmt.Errorf("the OIDC.ForwardIDToken header %q cannot be the Authorization header when passThroughAuthHeader is enabled and a JWT provider reads from it", fwdHeader) |
There was a problem hiding this comment.
When passThroughAuthHeader is enabled with forwardIDToken.header: Authorization and a JWT provider uses the default Authorization extractor, this rejects the policy during translation. Envoy's OAuth2 filter checks pass_through_matcher before it sanitizes the Authorization header, so bearer-token clients are not stripped in this case; rejecting it blocks the intended mixed flow where browser OIDC sessions forward the ID token on Authorization while non-browser clients pass their bearer token through to JWT validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Envoy rejects configs whose pass_through_matcher matches forward_id_token header, because forward_id_token header is owned by envoy and should not be sent by the clients.
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
|
/retest |
# Conflicts: # examples/extension-server/go.mod # examples/extension-server/go.sum # go.mod # go.sum # test/go.mod # test/go.sum
What this PR does / why we need it:
Which issue(s) this PR fixes:
Implements #7343
PR Checklist
git commit -s). See DCO: Sign your work./api), the API was discussed and agreed before the implementation. The API change can be in a separate PR, or in the same PR, but the API must be agreed before implementation. N/A if this PR does not contain API changes.make generate gen-check,make lint, and the unit-test/coverage build pass. (Flaky e2e failures are not considered breakages, butgen-check,lint, and coverage MUST pass.)release-notes/current/<section>/<pr-number>-<slug>.md(seerelease-notes/current/README.mdfor sections and naming). N/A if this PR does not contain non-trivial changes.make gen-checkand committed the result if API/helm charts/modules changed.release-notes/current/breaking_changes/.