Skip to content

Improve JavaScriptServlet client side caching strategy #327

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

Conversation

jayblanc
Copy link
Contributor

This PR aims to improve client caching strategy for javascript csrf client.

Context :
In our usage, we inject the script on pages using a ServletFilter that build the <script> and add it in the outgoing html. At that time, we have session information about user which query the page and we can decide a particular client side caching strategy for the csrf javascript client code using a watermark (tag) in the URL. That tag is based on the MD5 of the JS generated for that particular client and can then be cached during the session.
If anything changes in the JS (including CSRF master token) the link won't be the same and not retrieved from browser cache.

The JavaScript servlet has been modified to take into consideration that query param (tag) and include a specific client cache header for that purpose.
When param is not used, the servlet now include a ETag with a must revalidate strategy that will compare existing client side ETag avoiding, in case of same hash, to send again the content but a 304 response status code.

Modifications are mainly :

  • Add Etag in JS Servlet to avoid network bandwidth usage when file is the same (using md5)
  • Externalize JavaScript Etag to allow custom client side strategy using the Etag in a query param for that script.
  • Add a specific configuration to override cache-control when using tag query param to retrieve the clientside JS script.

…the same (using md5)

Externalize JavaScript Etag to allow custom client side strategy using the Etag in a query param for that script.
@jayblanc jayblanc changed the title Improve JavaScriptServlet client side caching strategy Improve JavaScriptServlet client side caching strategy #326 Feb 13, 2025
@jayblanc jayblanc changed the title Improve JavaScriptServlet client side caching strategy #326 Improve JavaScriptServlet client side caching strategy Feb 13, 2025
jayblanc added a commit to Jahia/jahia-csrf-guard that referenced this pull request Feb 13, 2025
…g management of the script. (OWASP/www-project-csrfguard#327)

If the PR is merged on the original project, the client side optimization could be propagated to Jahia usage.
@jayblanc
Copy link
Contributor Author

Hi @forgedhallpass, do you think it is possible to have a feedback on that PR ? Is it something that could be merged ?

@forgedhallpass
Copy link
Member

Hello @jayblanc,

Thank you for your contributions. Sorry for the late response, I've been quite busy lately. I'll try to review everything next week and, if all goes well, create a release with the changes.

@jayblanc
Copy link
Contributor Author

Hello @forgedhallpass ,
Thanks for your message and for starting the review ; If I can help in any way providing explanation or anything else do not hesitate to send me a message. As csrf guard is included in Jahia and because you mentioned a new release ; we would really appreciate to upgrade the dependency taking benefit of it in a stable way.
Best regards.

@forgedhallpass
Copy link
Member

Hello @jayblanc,

Apologies again for the delayed response.

I've taken some time to consider the different configuration scenarios, including page tokens, token rotation, the fact that the master token is embedded in the script, and both stateful and stateless modes. While the caching approach might not be suitable in every case, I still think it's a valuable improvement that can benefit many setups.

I've added a few minor observations for consideration. Once you've had a chance to respond, we can move forward with preparing a new release.

Thanks again for your contribution!

@jayblanc
Copy link
Contributor Author

Hello @forgedhallpass ,
I cleaned the PR regarding your observations. Do you think it's ok like this ?
Best regards.

@forgedhallpass forgedhallpass merged commit 5d603cb into OWASP:master Mar 31, 2025
1 check passed
@forgedhallpass forgedhallpass linked an issue Mar 31, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BACKLOG-23424 - General refactoring of CSRF guard & caching / loading improvements Improve JavaScriptServlet client cache strategy
2 participants