remove the cookie instead of resetting its value

[gbourant]

remove the cookie instead of resetting its value.

prior to that, it caused a weird behavior.

assume you are authenticated (having a valid not expired QuarkusUser cookie), and then you rotate the public/private keys (even by restarting quarkus in dev mode) and then you visit a page that requires authentication, so it tries to authenticate and since the QuarkusUser value was created with the old keys, the cookie becomes invalid, so the method AuthenticationFailedExceptionMapper.authenticationFailed was invoked, which, in turn called AuthenticationFailedExceptionMapper.redirectToRoot method setting the QuarkusUser to an empty string "" and also correctly redirecting to "/".

however, now, QuarkusUser has an empty value, and if you try to access a protected page with it, it triggers authentication checks and then redirecting to "/". on the contrary, it should redirect to the login page if the @Loginpage existed or to a 401 Unauthorized error. (as if the QuarkusUser cookie did not exist)

so the solution is to remove the cookie (instead of resetting its value to an empty string), so the authentication won't kick in and then properly redirecting to the login page or return 401 Unauthorized error.

[FroMage]

?

Note that I've seen weird behaviour sometimes, with an expired session cookie, after a restart, which caused an empty page to be returned instead of invalidating the cookie and redirecting to the login page. This is something I've fixed several times in the past with AuthenticationFailedExceptionMapper, so perhaps there's a case missing there? I haven't been able to reproduce it reliably.

[gbourant]

I updated the original message. You can read more there.

Note that I've seen weird behaviour sometimes, with an expired session cookie, after a restart, which caused an empty page to be returned instead of invalidating the cookie and redirecting to the login page. This is something I've fixed several times in the past with AuthenticationFailedExceptionMapper, so perhaps there's a case missing there? I haven't been able to reproduce it reliably.

I used an expired session cookie, after a restart, and it not result to that kind of error. The following might not be related but when we play with the browser, the browser might cache the page and not make a request to the server (especially when pressing the back button, maybe even of page refresh?). We can set CacheControl.setNoStore(true) and CacheControl.setNoCache(true) headers to overwrite that.

Why AuthenticationFailedExceptionMapper.authenticationFailed method returns null ? I noticed that its not working properly in the following cases:

1. Assume you have a valid token eyJjdHkiOiJKV1QiLCJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkEyNTZHQ00ifQ.XO-NxLKWDDu-w9YOHS4WSq2ystBu1JHLEYfJURRJ0ZOj4Cere1YnxPMf9RpfiFI598XSasfWez1DWoC3xgJVTc6aKAKp6rfzfd2n1E5MrKHxMBbFbZWv3nIL13xCQSMBbK6T1K_YhEZ3PLBi1y1JauXVYbbQCGCfQAKDOkzB0LgCqUwNTq_Nq2JpUsrP0ilTRCD7KgraZ97sv2rNgoGyyeslf_-j7w-_L9uGPTIdRS27djF0NtXnYjg2oygAzKmT6wQRXYg1qT95J2kp90aX_5jFqtXD8nADh2ZoH2b4fWMi_8GMHHSdcEh4Axde4o4mfU-tTcMW76gQnUUXEiLp0g.FVgzxIjbnuixHzIX.xJpURSCQwsjkcFI6pCpNMvByjlWwbT6skFqHPExISAgWbaTgOrN0pZSRT8BgFP6u5lJSK0YzdgZJ6jpRkiVvIQ7oBid0_EN6r4TbXQmN4DN7qc6YkmbtrCfYdvEOenY6-FZKqPqWd9AZ4ajvsCY16dLMRiBuEODxRuIJYpKdg2D9330gW9GOa7_QNG2nybN3SpPB9c-GN02G-Pb5ZazbKNtnYK47wIlyIPG0eP_yRBRFNOEB0EMFeVSKrHbpEfVad-weFgeJabGi7Z8RZYekZLHayeJfmTW-jf6ryi-c6j20OgWt6mPs4W3tYgfHuP3wGB9N1JlsHKd274xDGMRAbn1TiVNGu-agJMvs84oJfnbQFJAV-nMDM0O1KlD0e3udfhCmIKpjX9RyNBpCgZGXCIMYchJwJktqF34voRTJ0a38FZSq9Vz6H3jNGfz5VRwBFJzwxBcEhB0Pz7ErjCul7AfaZq09GBDrh8vx3leNGYePJmjZGIwkDZXol76EYtRrBlS52l12xJNqceHBsI9Fl9_8k7MTRDnstBZVjQPh0gp2LR5GoZO8q01Y2HqqS9JX4zSZrlqsm5aL528Hdh7-A8V5ozZo4crAtYkR7GDaC0YsLoR0Jr6snwkZy-TaTsP5y1Y6B-KJV6mu3qd20q0WskiOMfX-hDNvWqEY7CkbbPSYTeoR8v_9L4iS8Y66PVcARbxJvVg-93hcw0_X0Jczj28XrgpbdJ-CW1hzKV0Cmrf23XFz7YRjgN_PCpnIle1XQKABmC-Jki4.zTtXgVHMIbd91DUcAwVi3Q if you append something at the first part of the jwt something like a number 1eyJjdHkiOiJKV1QiLCJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkEyNTZHQ00ifQ.XO-NxLKWDDu-w9YOHS4WSq2ystBu1JHLEYfJURRJ0ZOj4Cere1YnxPMf9RpfiFI598XSasfWez1DWoC3xgJVTc6aKAKp6rfzfd2n1E5MrKHxMBbFbZWv3nIL13xCQSMBbK6T1K_YhEZ3PLBi1y1JauXVYbbQCGCfQAKDOkzB0LgCqUwNTq_Nq2JpUsrP0ilTRCD7KgraZ97sv2rNgoGyyeslf_-j7w-_L9uGPTIdRS27djF0NtXnYjg2oygAzKmT6wQRXYg1qT95J2kp90aX_5jFqtXD8nADh2ZoH2b4fWMi_8GMHHSdcEh4Axde4o4mfU-tTcMW76gQnUUXEiLp0g.FVgzxIjbnuixHzIX.xJpURSCQwsjkcFI6pCpNMvByjlWwbT6skFqHPExISAgWbaTgOrN0pZSRT8BgFP6u5lJSK0YzdgZJ6jpRkiVvIQ7oBid0_EN6r4TbXQmN4DN7qc6YkmbtrCfYdvEOenY6-FZKqPqWd9AZ4ajvsCY16dLMRiBuEODxRuIJYpKdg2D9330gW9GOa7_QNG2nybN3SpPB9c-GN02G-Pb5ZazbKNtnYK47wIlyIPG0eP_yRBRFNOEB0EMFeVSKrHbpEfVad-weFgeJabGi7Z8RZYekZLHayeJfmTW-jf6ryi-c6j20OgWt6mPs4W3tYgfHuP3wGB9N1JlsHKd274xDGMRAbn1TiVNGu-agJMvs84oJfnbQFJAV-nMDM0O1KlD0e3udfhCmIKpjX9RyNBpCgZGXCIMYchJwJktqF34voRTJ0a38FZSq9Vz6H3jNGfz5VRwBFJzwxBcEhB0Pz7ErjCul7AfaZq09GBDrh8vx3leNGYePJmjZGIwkDZXol76EYtRrBlS52l12xJNqceHBsI9Fl9_8k7MTRDnstBZVjQPh0gp2LR5GoZO8q01Y2HqqS9JX4zSZrlqsm5aL528Hdh7-A8V5ozZo4crAtYkR7GDaC0YsLoR0Jr6snwkZy-TaTsP5y1Y6B-KJV6mu3qd20q0WskiOMfX-hDNvWqEY7CkbbPSYTeoR8v_9L4iS8Y66PVcARbxJvVg-93hcw0_X0Jczj28XrgpbdJ-CW1hzKV0Cmrf23XFz7YRjgN_PCpnIle1XQKABmC-Jki4.zTtXgVHMIbd91DUcAwVi3Q (notice the 1 at the start) it causes the org.jose4j.json.internal.json_simple.parser.ParseException and the AuthenticationFailedExceptionMapper.authenticationFailed returns null
2. If you do the same as in the previous example for the second part of the jwt it causes the java.security.InvalidKeyException and the AuthenticationFailedExceptionMapper.authenticationFailed returns null
3. If you have a valid jwt eyJjdHkiOiJKV1QiLCJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkEyNTZHQ00ifQ.RoFt1PQqeWF_XVw8OoZf-ix81_CUkWs5VXOT678Vwd3vWkLMXCHW3v39GBSXU0sHlU3BNBgXJ1eKFPEoImDaLIgdAcBa-tkzeW5NpoTZBErOJ8Ksp7BzsQrW3GJUwBzUwApwLZlvlqXdgg5x4AYdFj9voLBolmNMRysXFYa67PZE5P1ihSWtOL84l1ZgT2U_qCOs4SfQFRcaO9QA59_zaUUbtmkbcnc3w7usdMmcEqYYWi4DzgPQjyzmNgrD7MjDThtbtKG3vvlI_jRUBhNjtTEPbOS15E6I0nqFX5yuQPjUbMAKRrD7KG3Px1mCKr0hBrYiJyMwNFVxMgoQg7w9EA.irV6QlKn2nMqsTCz.-6WbwES9NxSHhXIyuGGsazGnLMGi4tKabsmNq_ECK8mF5XPYy-q0PDk5eBswvPdbiVheEp9NSIFaeipG6dE12Qu0JOizQLlZU0aRnlx4SLr-5KMSTwjnDJ3SeDCo1_W5FbsQuhNY4rwOYW8CursKoW_977d4SigkcDNcjoYzYsZI7bJIcloXVmRBfIFFjSDDGSbP-lF1WtsDqOM4cwGbT0zrucqLpw28OtfBGHeql6naftegqopCdbMhkgORLwY1kXY00bL_yHdfxonLJl7HB4jlOwRxT5eF2HkvfoabejjCNcF9S-d02NRC1I8YPS-jXfsxmzPyQJcz6hGdeFkDQRYGDKS8d00MV3s-G_0O_33Wh81CS1RPESG7xam3Ux38nsjmnb7F7hQHi1mPcZ5Zg8Q1_Hc14FSwntCulyx_qo4XW4J1Enfr78vLJRMfPGLZlJn6dj-JRSPckHKKClbt7xrBJT0MrUMStF1IOymliNR_GsEUZpd71yeHieH8eSmtPPUlmAE1_3z-14fxNPD6s820zMpdJ__9K1reMPX6QxfdSHu0d-fMF6kGSsDwaM2O7l4O6CKq4QNvgxbximNKk9R9pPTm5MQutGTFMo0unmI8Hu6D6ROLQmeKd1S2R8h4adTZbKmjSbr8kD2NJKAj_N1ajtSfItL4b6UaZhJg5FeiSf-a3TnnFaMqDUX4FYO29CKPWQhXUuUm5k9vQc_j42pdb50qHNvKaox-ptToYj_tXH868picpdlnlpILWB7byqN20g9U0M8.A9ITpy-rK6PTMsTuejKdRA and you remove some part of the last part of the jwt or if you send an empty value QuarkusUser = "" eyJjdHkiOiJKV1QiLCJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkEyNTZHQ00ifQ.RoFt1PQqeWF_XVw8OoZf-ix81_CUkWs5VXOT678Vwd3vWkLMXCHW3v39GBSXU0sHlU3BNBgXJ1eKFPEoImDaLIgdAcBa-tkzeW5NpoTZBErOJ8Ksp7BzsQrW3GJUwBzUwApwLZlvlqXdgg5x4AYdFj9voLBolmNMRysXFYa67PZE5P1ihSWtOL84l1ZgT2U_qCOs4SfQFRcaO9QA59_zaUUbtmkbcnc3w7usdMmcEqYYWi4DzgPQjyzmNgrD7MjDThtbtKG3vvlI_jRUBhNjtTEPbOS15E6I0nqFX5yuQPjUbMAKRrD7KG3Px1mCKr0hBrYiJyMwNFVxMgoQg7w9EA.irV6QlKn2nMqsTCz it causes org.jose4j.lang.JoseException

Why jwt has more than three dots ? Is it because of the signed and encrypted ? So in 3rd example i remove the 4th part of the jwt.

Jwt.issuer(jwtIssuer)
                .upn(user.userId())
                .groups(user.roles())
                .expiresIn(duration)
                .innerSign().encrypt()

[FroMage]

Damn, I missed that PR, so sorry. This looks very sensible. Do you think you could add a test in JWTTest?

[FroMage]

I actually already have a test for an expired token, in testProtectedPageWithInvalidJwt, which calls assertRedirectWithMessage, I think it's this method you need to tweak to make sure the removal is correct, no?

[gbourant]

Yeap, i can do that.

[gbourant]

Should we make a static method to generate the logout cookie (or a constant) ?

[FroMage]

Huh, like RenardeSecurity.makeLogoutResponse() you mean? :(

This seems to have the same problem. But I don't understand, because this one works, it certainly logs me out. So, why? What's the difference?

Notice that this version also makes sure that any OIDC session gets terminated as well.

[gbourant]

Huh, like RenardeSecurity.makeLogoutResponse() you mean? :(

No, i mean something like:

    public static NewCookie createLogoutCookie(String cookieName) {
        return new NewCookie.Builder(cookieName).expiry(new Date(0)).build();
    }

which will be used by RenardeSecurity.makeLogoutResponse().

This seems to have the same problem. But I don't understand, because this one works, it certainly logs me out. So, why? What's the difference?

I'm not quite sure, what do you mean? The previous implementation of RenardeSecurity.makeLogoutResponse() would successfully log you out by creating an invalid (with empty value) session cookie Set-Cookie: QuarkusUser=;Version=1;Path=/ (in http, a session cookie is a one that is missing Expires or Max-Age which will be lost/deleted if you close the browser). So after the logout, if you tried to access a protected page (with the invalid cookie), then it would trigger the error that i described in my original post.

The new implementation adds the Expires attribute so the browser can handle and remove it properly.

Set-Cookie: QuarkusUser=;Version=1;Expires=Thu, 01-Jan-1970 00:00:00 GMT

Notice that this version also makes sure that any OIDC session gets terminated as well.

You are right, i updated the code.

[gbourant]

I'm thinking about changing return new NewCookie.Builder(cookieName).expiry(new Date(0)).build(); with return new NewCookie.Builder(cookieName).maxAge(0).build();. I will test that later today.

Which will set the following cookie QuarkusUser=;Version=1;Max-Age=0

[gbourant]

According to the HTTP spec, the correct way to remove a cookie is to use the Expires attribute (as we do) with an old date.

   Finally, to remove a cookie, the server returns a Set-Cookie header
   with an expiration date in the past.  The server will be successful
   in removing the cookie only if the Path and the Domain attribute in
   the Set-Cookie header match the values used when the cookie was
   created.

But we can also achieve the same with Max-Age.

The Max-Age Attribute

   The Max-Age attribute indicates the maximum lifetime of the cookie,
   represented as the number of seconds until the cookie expires.  The
   user agent is not required to retain the cookie for the specified
   duration.  In fact, user agents often evict cookies due to memory
   pressure or privacy concerns.

      NOTE: Some existing user agents do not support the Max-Age
      attribute.  User agents that do not support the Max-Age attribute
      ignore the attribute.

   If a cookie has both the Max-Age and the Expires attribute, the Max-
   Age attribute has precedence and controls the expiration date of the
   cookie.  If a cookie has neither the Max-Age nor the Expires
   attribute, the user agent will retain the cookie until "the current
   session is over" (as defined by the user agent).

The Max-Age has precedence and also it is smaller in size. I tried it and it is working as expected.

Do we want to stick to the spec or use the more efficient Max-Age ? (premature optimization is the root of all evil)

[FroMage]

LGTM

[FroMage]

I mean Max-Age seems fine.

[FroMage]

Thanks a lot!