quarkiverse / quarkus-renarde

Server-side Web Framework with Qute templating, magic/easier controllers, auth, reverse-routing
Apache License 2.0
73 stars 16 forks source link

Invalid response with two `Location` headers on invalid `QuarkusUser` cookie #194

Closed FroMage closed 5 months ago

FroMage commented 5 months ago

I'm not sure what is wrong with this cookie, perhaps it's just expired, but I'm getting invalid responses such as:

HTTP/1.1 303 See Other
Location: http://localhost:8080/_renarde/security/login
Location: http://localhost:8080/
Set-Cookie: QuarkusUser=;Version=1;Path=/;Max-Age=0
content-length: 0
set-cookie: _renarde_flash=eyJtZXNzYWdlIjoiSW52YWxpZCBzZXNzaW9uIChiYWQgc2lnbmF0dXJlKSwgeW91J3ZlIGJlZW4gbG9nZ2VkIG91dCJ9; Path=/; HTTPOnly; SameSite=Lax
set-cookie: quarkus-redirect-location=http://localhost:8080/; Path=/

Well, flash message is {"message":"Invalid session (bad signature), you've been logged out"} so that's a hint. But why two Location?

FroMage commented 5 months ago

Well, at least, the tests show this, but RestAssured doesn't care, unlike Chrome.

FroMage commented 5 months ago

Alright, so this might be a Quarkus bug, either in security, or in RESTEasy Reactive, because what happens is that I'm getting an auth challenge from HttpAuthenticationMechanism.ChallengeSender which sets the response code and adds a Location header directly to the Vert.x response, and then an AuthenticationFailedException is thrown, resulting in my @ServerExceptionMapper being called and also setting a Location by returning a Response.seeOther() and we end up with two Location headers.

I'm not entirely sure what the behaviour should be, but I suspect that at the very least, any Response data set by the exception mapper should override the ones set previously, if only because the Response is not meant to come on top of anything prior to it, but override it from scratch. So, the exception mapper's Location should "win" and override the security challenge one, don't you think @sberyozkin @geoand ?

geoand commented 5 months ago

So, the exception mapper's Location should "win" and override the security challenge one, don't you think @sberyozkin @geoand ?

I completely agree

FroMage commented 5 months ago

OK, I'll open a Quarkus issue. Meanwhile I've added a workaround.