Refresh Tokens Cached on Craft Cloud

shennyg commented 3 months ago

Describe the bug

Hey there. I'm launching a Craft Cloud site tomorrow and getting past some final QA bugs.

Looks like https://PROJECT-NAME-production-5cd1c590.preview.craft.cloud/actions/formie/forms/refresh-tokens?form=footerSignup is a GET and returning a cache HIT. Craft Cloud is cached using CloudFlare.

I am on Formie version 2.1.17 and I don't see any changes in the changelog since related to refresh tokens. I am looking at the latest on v4: https://github.com/verbb/formie/blob/craft-4/src/controllers/FormsController.php#L371-L398

It looks like we need to add $this->response->setNoCacheHeaders(); More info from Craft on opting out of caching here: https://craftcms.com/knowledge-base/cloud-static-caching#opting-out

As a hotfix I've done this, we are using your Cached Forms Advanced Usage approach:

 -       fetch('/actions/formie/forms/refresh-tokens?form={{ form.handle }}')
+        fetch('/actions/formie/forms/refresh-tokens?form={{ form.handle }}&cache-workaround=' + Math.random())

Thanks so much!

Steps to reproduce

setup site in craft cloud
set to production mode so caching is enabled
setup advanced usage style cached form
submit form twice and get a CSRF error

Form settings

Multi-page form: No
Submission Method: Ajax
Client-side Validation: Yes
Custom Form Templates: No

Craft CMS version

4.10.2

Plugin version

2.1.17

Multi-site?

No

Additional context

No response

engram-design commented 3 months ago

I'm not a Craft Cloud expert (yet), but I'm surprised that any control action endpoints are being cached - I don't think they should be. Probably only an issue for GET requests. I'll check out the setNoCacheHeaders() option, that's probably a quick-and-easy fix.

Fixed for the next release. To get this early, run composer require verbb/formie:"dev-craft-4 as 2.1.20".

shennyg commented 3 months ago

Thanks @engram-design

timkelty commented 3 months ago

but I'm surprised that any control action endpoints are being cached - I don't think they should be.

Craft Cloud doesn't discriminate on any specific paths/actions for caching. It will cache any 200 GET response unless no-cache headers have been set.

The problem is here:

        // Ensure that the session has started, just in case
        Session::exists();

That comment is not accurate – The \craft\helpers\Session methods specifically don't start a session if they don't need to, including that one. If you want to ensure a session has started, you should call \craft\web\Session::open, which in turn would set no-cache headers when starting a session, and the request wouldn't be cached.

That all said, I'm wondering if maybe \craft\web\Request::getCsrfToken should just set no-cache headers so the caller wouldn't have to worry about cache 🤔

timkelty commented 3 months ago

@engram-design as of 4.10.4/5.2.5, any request that generates a CSRF token will get no-cache headers applied.

engram-design commented 3 months ago

Appreciate the heads-up there, not sure why I went with Session::exists(). And thanks for that change!

engram-design commented 2 months ago

Fixed in 2.1.21

verbb / formie