Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JWT SecurityPolicy does not forward Authorization header by default #2295

Closed
akhenakh opened this issue Dec 11, 2023 · 4 comments
Closed

JWT SecurityPolicy does not forward Authorization header by default #2295

akhenakh opened this issue Dec 11, 2023 · 4 comments
Assignees
Labels
kind/enhancement New feature or request triage

Comments

@akhenakh
Copy link
Contributor

The JWT SecurityPolicy if enabled does not forward the Authorization header upstream.

This is the default behaviour in Proxy if forward is not set to true:
https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/jwt_authn_filter#jwtprovider

Yet it is a weird behaviour for a gateway to temper headers from the original request.

https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/jwt_authn_filter#jwtprovider
relevant code

jwtProvider := &jwtauthnv3.JwtProvider{
Issuer: irProvider.Issuer,
Audiences: irProvider.Audiences,
JwksSourceSpecifier: remote,
PayloadInMetadata: irProvider.Issuer,
ClaimToHeaders: claimToHeaders,
}

Adding Forward: true will result of not removing the Authorization header.

I'm not sure myself what would be a sane default, but this could become a parameter or at least documented in Gateway.

@akhenakh akhenakh added kind/enhancement New feature or request triage labels Dec 11, 2023
@akhenakh
Copy link
Contributor Author

@zhaohuabing
Copy link
Member

zhaohuabing commented Dec 12, 2023

Thank you @akhenakh for raising this.

We have two options here:

  1. Expose Envoy forward knob to EG SecurityPolicy API and let the users decide whether to forward the jwt or not
  2. Set forward to true when translating the xds without exposing it to EG API

I am leaning towards option 2 because it doesn't harm to pass down the JWT token header. WDYT @arkodg @envoyproxy/gateway-maintainers

@arkodg
Copy link
Contributor

arkodg commented Dec 15, 2023

looks like we went ahead with 2. which was completed with #2300, can this be closed @zhaohuabing ?

@zhaohuabing
Copy link
Member

zhaohuabing commented Dec 15, 2023

looks like we went ahead with 2. which was completed with #2300, can this be closed @zhaohuabing ?

Yes, this isssue can be closed.

We don't have to modify API with option 2. And if we need to give users the choice to move the token out later, we can add it to the API.

@zirain zirain closed this as completed Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request triage
Projects
None yet
Development

No branches or pull requests

4 participants