Quarkus REST CSRF: inform users in DEV mode when token size changed that previously generated tokens are invalid

[michalvavrik]

• Closes CSRF custom size of CSRF token doesn't work #51334

[github-actions]

😭 Deploy PR Preview failed.

[michalvavrik]

JVM Integration Tests - JDK 17 fails over:

2025-12-05T21:39:04.7538696Z Downloading https://services.gradle.org/distributions/gradle-9.2.1-all.zip
2025-12-05T21:39:05.6591107Z ......................10%......................20%......................30%...
2025-12-05T21:39:05.6612262Z ##[error]Exception in thread "main" java.net.SocketException: Connection reset
2025-12-05T21:39:05.6613395Z 	at java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:328)
2025-12-05T21:39:05.6613737Z 	at java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:355)

which doesn't seem related.

[sberyozkin]

Hi Michal, thanks for taking care of it, appreciated, so it is a dev mode situation.

The log message is useful, my only proposal is not to create devmode specific check against the previous cookie size, but warn the cookies sizes do not match and only log the extra advice to make sure the browser cache is cleared in devmode, without trying to be very precise.

The reason I suggest is that that any property that impacts the cookie presentation, when changed in devmode, will lead to 400 in this case - signature key is another case, so if we write a size specific processing then it would be only natural to expect also keep the previous signature key value for devmode to suggest why the signature verification might've failed...

And then we have quarkus-oidc, form authentication with similar situations...

Something like

        if (cookieSize != this.config.tokenSize()) {
             Logger.getLogger(RestCsrfConfigHolder.class).infof("Cookie size %d is not equal to the required %d size", cookieSize, this.config.tokenSize());
        }
       
        if (LaunchMode.current() == LaunchMode.DEVELOPMENT) {
             Logger.getLogger(RestCsrfConfigHolder.class).info("Make sure the browser cache is cleared"));
        }

This is much less precise, I agree, but it should give a good enough hint.

By the way, I think having a static int solution might not really work if you change both a cookie name and size in devmode at the same time before saving. So let's you have csrf_cookie with size 16 recorded, then you have changed it to the_csrf_cookie and 32, and there will be a warning which would be confusing because a new cookie which does not match the other cookie...

[michalvavrik]

The reason I suggest is that that any property that impacts the cookie presentation, when changed in devmode, will lead to 400 in this case - signature key is another case, so if we write a size specific processing then it would be only natural to expect also keep the previous signature key value for devmode to suggest why the signature verification might've failed...

That is not quite correct. When signature key changes and you use HTTP method that doesn't require token, like GET when Quarkus serves the form with token, a new cookie is generated. Therefore you do not get 400. Idea that users go directly to paths/methods that require cookie after DEV more restart, that doesn't fit form flow, when you go to a page, fill form and click to submit. While if the token size changes, it will always lead to 400, thus we need to inform this is the token size consequence.

This is much less precise, I agree, but it should give a good enough hint.

OK, I'll tell you directly personally I think my version is more useful to users, but it is really unimportant matter. I'll do it.

By the way, I think having a static int solution might not really work if you change both a cookie name and size in devmode at the same time before saving. So let's you have csrf_cookie with size 16 recorded, then you have changed it to the_csrf_cookie and 32, and there will be a warning which would be confusing because a new cookie which does not match the other cookie...

If you change the cookie name, you have no related issue. I think it wouldn't be confusing because that log message I added:

1. is not a warning, it is information log level
2. everything in that log message is still true

but I think it is a good point, I should have improved it with remembering cookie name. Anyway, it doesn't matter anymore as I'll update this PR with your suggestion.

[michalvavrik]

On the other hand, I like the idea to have logging in non-dev mode as you suggest. I think there are not many normal situations that this should happen, so it will be useful.

[michalvavrik]

@sberyozkin so I pushed slightly different version because there already was debug logging, so I just made sure that users can see it in the DEV mode and add your suggestion about deleting cache. I think it should be enough and it solves the issue. Let me know if you want some tweaks.

[michalvavrik]

This is much less precise, I agree, but it should give a good enough hint.

OK, I'll tell you directly personally I think my version is more useful to users, but it is really unimportant matter. I'll do it.

I suppose users don't need to know about previous token size. It was more about knowing that issues are unintentional. More I think about it, users are unlikely to test bad cookies in DEV mode, so it is good.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 48f96e9.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

You can consult the Develocity build scans.

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 48f96e9.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

[sberyozkin]

Hi @michalvavrik

Totally agreed your original message was more useful, my concern was about the necessity of having to introduce a dedicated static property to handle this specific dev-mode only situation, that would more or less require us to keep adding more such properties (signature key).

Sorry, the token name update case was not a good example, with respect to the original code

Thanks for your help