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

headersToDownstreamOnAllow in envoyExtAuthzHttp should not include content-type #48086

Closed
chocolat2000 opened this issue Nov 29, 2023 · 4 comments
Labels
kind/docs kind/enhancement lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while

Comments

@chocolat2000
Copy link

(This is used to request new product features, please visit https://discuss.istio.io for questions on using Istio)

Describe the feature request
In documentation https://istio.io/latest/docs/tasks/security/authorization/authz-custom/#define-the-external-authorizer an example for oauth2-proxy is given and headersToDownstreamOnAllow includes content-type.
When doing this, both content-type from the auth-proxy and the upstream are mixed (same beavior as this issue #30470) resulting in a corrupted content-type header if they are different. Which is almost the case since oauth2-proxy always replies with text/plain.
Describe alternatives you've considered
Documentation should not recommend including content-type in headersToDownstreamOnAllow.
Even better, it should warn that if same header is present in both auth-proxy and upstream they will be combined and separated with a comma (https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.2).
Affected product area (please put an X in all that apply)

[ ] Ambient
[X] Docs
[ ] Dual Stack
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

Affected features (please put an X in all that apply)

[ ] Multi Cluster
[ ] Virtual Machine
[ ] Multi Control Plane

Additional context

@hanxiaop
Copy link
Member

Even better, it should warn that if same header is present in both auth-proxy and upstream they will be combined and separated with a comma (https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.2).

I think this is related to Envoy logic that cannot be validated and warned by Istio/Istioctl?

@hanxiaop
Copy link
Member

The doc has been revised in istio/istio.io#14274.

@zirain
Copy link
Member

zirain commented Dec 13, 2023

istio/istio.io#14274 just remove content-type, should be better if someone can send a PR to comment this usecase.

@hanxiaop hanxiaop removed their assignment Dec 13, 2023
@istio-policy-bot istio-policy-bot added the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label Jun 10, 2024
@istio-policy-bot
Copy link

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2023-12-13. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/docs kind/enhancement lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while
Projects
None yet
Development

No branches or pull requests

4 participants