-
Notifications
You must be signed in to change notification settings - Fork 687
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
v0.50.1 AuthService drops all but one set-cookie
response header
#1211
Comments
I observed the same behavior. Does it work for you in previous versions? |
We observe the same behavior. It works for us in 0.40 Version . |
Anyone knows when can we expect fix for that? |
@mianelson @cornelius-keller Thanks for reporting it. Just to confirm, what you are saying is that given the following response from the authorization server:
Only cookie If correct, that's a bug probably attributed to this logic: IRC, it's was a bug fixing due to a segfault that we found in the previous implementation. FYI @FrancoCorleone |
I have different http code but the rest is correct. I have two cookies and only one is set. I checked by proxing direcetly to auth service bypassing ambassador and both are set so it has to be further than auth service itself |
Yeah, that's the correct behaviour for setting multiple cookies. I will need to get my hands into Envoy just to confirm it, but it seems that the logic described above is what causing the issue. |
Ok let me know when it's fixed. Thanks! |
@gsagula yes, can confirm as well that that is the behavior we're experiencing. It's been a while since I've worked in C++, but that line you tracked down in ext_authz.cc is where my search ended as well. |
Any update on this issue? When it going to be fixed? We can not update to 50 version because of this issue. |
Same here. Any updates? @gsagula ? |
@kflynn I’m aware of the RFC and that it is a bug. However, it’s not up to
me to prioritize the fix.
|
@gsagula, any suggested workaround till then as there is no date for a fix planned? |
@FrancoCorleone As I mentioned in #1211 (comment), the problem is in Envoy proxy. I don't think that there is a workaround other than just trying to set one cookie instead of multiples if possible. Regarding prioritazation, it is more of quetion for @kflynn. |
Fixed in 0.51. |
@richarddli Are you sure it is fixed? I am using quay.io/datawire/ambassador:0.51.1 and I still observe that issue. |
@FrancoCorleone Thanks for reporting, I'm looking into that. |
@gsagula thanks for quick response. Let me know when you find out anything. For any interested parties. Until it works for you, you can easily work around that issue by making inner redirects setting additional cookies. In my case, I have only two cookies so just one additional redirect. |
@FrancoCorleone We found the problem. I will make sure that we get a new image tomorrow. Thanks! |
Thanks, @gsagula. |
@gsagula I can confirm the issue still exists even in the latest version of ambassador (0.52.0). |
@woraphol-j it worked for me in 0.51.2. @gsagula is something broken again in newer version? |
@FrancoCorleone @woraphol-j It works for me with
docker-compose:
config:
|
@gsagula Thanks your quick reply. Really appreciated it. However, I tried the exact same configuration as yours and it turned out it DOES NOT work (at least as I expected) as it shows in the result (as I commented):
Note that when I call the auth-service directly, it returns a response with 3 set-cookies correctly
|
I’m not sure what you mean by not working, I can see the 3 set-cookie
headers in the response.
< set-cookie: foo=foo
< set-cookie: bar=bar
< set-cookie: baz=baz
…--
Gabriel Linden Sagula
|
Hi @gsagula there is only one
I think the flow is (Please correct me if I am wrong)
|
Describe the bug
When AuthService returns a non-200 response to Ambassador, only one
set-cookie
header can be sent back to the client, all otherset-cookie
headers are stripped. All cookies have the same domain, max_age, etc.To Reproduce
Steps to reproduce the behavior:
set-cookie
header on auth responsesset-cookie
header on responseset-cookie
headers are present on response from AuthService as the response passes back through Ambassador; there should be >1set-cookie
header on the response from the AuthService and only 1set-cookie
header on the response that Ambassador filters and sends back to the clientExpected behavior
We expect all
set-cookie
headers to be returned by Ambassador when the AuthService returns a response. Rolling the service back to Ambassador v0.40.2 results in the expected behavior.Versions (please complete the following information):
Additional context
Here is logging we captured of the issue. Notice that
x-request-destination
is preserved butsession
is removed in the final response.Here's the AuthService annotation that we are using:
We also verified this behavior against normal non-AuthService request/response flows, and did not see Ambassador filtering any response headers.
The text was updated successfully, but these errors were encountered: