Async CSRF token generation on user interaction

[croxton]

A pattern I've been using for some time on static-cached pages with forms is to regenerate CSRF tokens when the user first interacts with a form input. The advantage of this approach, over automatically regenerating tokens on window load, is that it reduces the total number of requests sent from cached pages (since most visitors do not interact with forms) and eliminates the vast majority of automated contact form spam, which will post the stale token present in the static cache and therefore be blocked.

I use this approach to refresh other tokens as well (see https://docs.solspace.com/craft/freeform/v5/templates/caching/#static-page-caching-cdn-blitz), but this is the gist of it:

<form method="post" data-refresh-tokens="true">
   <fieldset>
      <input type="hidden" name="CRAFT_CSRF_TOKEN" value="xxx">
      <input type="text" name="something" value="This input can be focussed">
   </fieldset>
</form>

const forms = document.querySelectorAll('form[data-refresh-tokens]');
let refreshed = false;

forms.forEach((form) => {
  
  let firstInput = form.querySelector('input:not([type=hidden])'); // could possibly add select,textarea

  if (firstInput) {
      let request = new XMLHttpRequest();
      firstInput.addEventListener('focus', (e) => {
          if (refreshed) return;
          request.onload = () => {
              let data = JSON.parse(request.response);
              // When a user interacts with a form for the first time,
              // locate and update tokens in *every* form in the page
              forms.forEach((f) => {
                  // Replace CSFR tokens
                  let csrf = data.csrf;
                  let csrfInput = f.querySelector('input[name=' + csrf.name + ']');
                  if (csrfInput) {
                      csrfInput.value = csrf.value;
                  }
              });
              refreshed = true;
          };
          // endpoint to get fresh tokens
          request.open("GET", '/api/form-tokens');
          request.setRequestHeader('X-Requested-With', 'XMLHttpRequest');
          request.setRequestHeader("HTTP_X_REQUESTED_WITH", "XMLHttpRequest");
          request.send();

      }, { once: true });
  }
});

It would be neat if this was a configurable option for CSRF token regeneration, maybe by passing a selector to asyncCsrfInputs, e.g. asyncCsrfInputs="form[data-refresh-tokens]".

[timkelty]

I like it!

Question – what is the purpose of rendering the CSRF in the form initially at all? Just as a last-ditch fallback if JS is disabled or broken? Of course, that won't help if it's a cached value.

I'm working on rolling this into the upcoming version of Craft:

• GeneralConfig::$asyncCsrf = false (default)
  • CSRF inputs rendered, no-cache headers still sent in response
• GeneralConfig::$asyncCsrf = true
  • CSRF inputs rendered, no-cache headers are not sent.
  • on focus, update values as described

[croxton]

Great! Yes I think you're right, the CSRF hidden field should initially have an empty value when GeneralConfig::$asyncCsrf = true, then even if a spammer hits the form before it's cached their submissions will fail.

[mmikkel]

@timkelty Is the idea that the new asyncCsrf config setting would replace the asyncCsrfInputs setting, or is it an additional setting? If the latter, how does it relate to the asyncCsrfInputs setting, and what happens if asyncCsrfInputs and asyncCsrf are both true?

In general, I feel like user interaction generated/replaced CSRF inputs is a really neat idea, but it's certainly more complex than the current behavior w/ asyncCsrfInputs, and I'd be a little bit worried about the edge cases where it might not work. For example, what if there's something on the page preventing the focus event? What if there are no focusable inputs, or the form is possible to submit without interacting with any focusable inputs? Etc :)

[timkelty]

@mmikkel I just mis-typed, the idea was they would be the same setting.

To address concerns like the ones you've listed, I think we'd also have a fail-safe that happened on form submit as well, if it hadn't already been swapped. But yeah, it does become a bit more complex for sure.

[leevigraham]

Question – what is the purpose of rendering the CSRF in the form initially at all? Just as a last-ditch fallback if JS is disabled or broken?

I could see having a initial value be useful if you're testing a page with automation / php unit where there isn't a js context.

That said changing the CRAFT_ASYNC_CSRF_INPUTS based on environment would also work.

[timkelty]

@leevigraham If you're not submitting the form with JS and requesting users/session-info in between, you're going to need the value in the form.

While the async csrf option uses/assumes JS to work – it doesn't assume you're submitting your form that way.

[thisisjamessmith]

I'd also add that rather than arbitrarily targeting the first focusable field as the trigger, you should also ensure that the trigger is a required field - there have been instances where the first focusable field in a given form is not a required field, and as such users can opt to not focus it at all and therefore not get a CSRF refresh.

Edit, ah sorry wrote this before fully reading mmikel's comment basically saying the same thing, nevermind.

[mmikkel]

Safest-ish option is probably to listen for the focus event on any form input?

[croxton]

How about using the focusin event instead, since unlike focus it will bubble up from any focussed element inside the form:

const forms = document.querySelectorAll('form[data-refresh-tokens]');
let refreshed = false;

forms.forEach((form) => {
  let request = new XMLHttpRequest();
  form.addEventListener('focusin', (e) => {
      if (refreshed) return;
      request.onload = () => {
          let data = JSON.parse(request.response);
          // When a user interacts with a form for the first time,
          // locate and update tokens in *every* form in the page
          forms.forEach((f) => {
              // Replace CSFR tokens
              let csrf = data.csrf;
              let csrfInput = f.querySelector('input[name=' + csrf.name + ']');
              if (csrfInput) {
                  csrfInput.value = csrf.value;
              }
          });
          refreshed = true;
      };
      // endpoint to get fresh tokens
      request.open("GET", '/api/form-tokens');
      request.setRequestHeader('X-Requested-With', 'XMLHttpRequest');
      request.setRequestHeader("HTTP_X_REQUESTED_WITH", "XMLHttpRequest");
      request.send();

  }, { once: true });
});

https://developer.mozilla.org/en-US/docs/Web/API/Element/focusin_event

[timkelty]

ohhh! lovely

[timkelty]

@markhuot @mmikkel I've been thinking more about this…

Instead of worrying about focus/focusin, why not just use the form's submit event? This would cover some of @mmikkel's comments about un-focusable inputs or forms with no interaction. The only thing it wouldn't cover would be if you were serializing the formdata in js yourself and then making your own request with it, rather than submitting the form. I'm not sure that's even something to worry about, as 1 - I can't think of a good reason why you'd do that, and 2 - if you are, you're already submitting with JS and have the opportunity to get a refreshed CSRF yourself.

[mmikkel]

The only thing it wouldn't cover would be if you were serializing the formdata in js yourself
I can't think of a good reason why you'd do that

Yeah, my worry with using the submit event would be forms being posted using Ajax. I actually don't think this is an uncommon thing to do (I wonder how this would play with Sprig/HTMX? 🤔), for a plethora of reasons (UX probably being the most common), so hinging everything on the submit event would likely mean that the new behavior wouldn't work in even more cases, vs. if the focus/focusin event is used (or both focusin+submit).

if you are, you're already submitting with JS and have the opportunity to get a refreshed CSRF yourself

True – although effectively, you'd be replacing behavior that works out of the box 99.99% of the time, with something that isn't going to work at all for any AJAX-submitted form, and potentially other cases 🤷

...TBH, while I think this is a really neat idea, the more I think about it, I think it's going to be difficult to implement this as a core feature without Craft having to make some assumptions about how a site's front end works, that I don't know if it should make? I do think the idea has a lot of merit, I just wonder if it's better served as, for example, a KB article linked from the docs, for devs who want to adopt this strategy, rather then being the core behavior w/ asyncCsrfInputs=true.

[timkelty]

AJAX submissions should still be firing a submit event though, so those should work fine.

[croxton]

Doing it on submit would likely cause order of execution problems for plugins like Freeform and Formie, which validate and process forms on submission with ajax.

[mmikkel]

AJAX submissions should still be firing a submit event though, so those should work fine.

Right, I'm just not seeing how this wouldn't necessarily break the AJAX implementation for those forms. Maybe I'm missing something though, would be happy to be proved wrong :)

[thisisjamessmith]

I think @mmikkel might be right about Craft going too far here, though I also like the idea of this feature in general. How might it work, for example, if I have a SPA-like site that uses JS-based full-page transitions like Barba JS or similar? When it's my own JS doing the CSRF token refreshing, it's trivial to just fire the function again when the new page has transitioned-in - but when it's someone else's JS that ultimately comes from an unknown location deep within the guts of Craft, it all gets a bit more opaque. Can I/should I call Craft's function again after a page has transitioned? Would I even need to if it uses event delegation on an element that's outside of my <main> element which happens to be the one getting updated?

[thisisjamessmith]

I hadn't even thought of doing it that way, but yep that's a good approach, thanks!

Setting asyncCsrfInputs=true in the config sounds like it will be pretty much required for any site using static caching, though, otherwise Craft will send its no-cache headers. For sites that are already handling the CSRF refresh themselves, we'll need to be careful to remove our own code that does the same thing. Looking at some of mine, I'm also piggybacking off my CSRF refresher to set the token value on window so it can be reused by other fetches - specifically I have one that fetches an "Edit This" link and injects it into the page if the visitor is a logged-in editor (come to think of it I could probably do that with a GET request instead of a POST, which, correct me if I'm wrong, doesn't require a CSRF token?).

[thisisjamessmith]

Maybe we need a way to opt out of Craft's injected JS but still opt-in to the HTTP headers?

[croxton]

refreshCsrfTokens({ waitForInteraction: true }) (or whatever you call the function to setup the form listeners) could be a global initially called on page load, but an SPA could also call it after a page transition / dom swap. And maybe it could dispatch a custom event so other scripts can piggyback their own behaviour.

[thisisjamessmith]

@timkelty As you mention - there are some plugins that will automatically render a {{ csrfInput() }} - I'm working with Formie at the moment, where I don't have the luxury of omitting that tag as I'm using their automatic rendering tag. How can I then retrigger the token refresh after new page injection? Not setting asyncCsrfInputs = true and doing it the way I always have myself doesn't seem like a viable option anymore given that Craft will now be sending nocache headers, which surely would cripple the cacheability(?) Sorry I'm a bit confused by all this now.

Edit: I've ended up just merging a copy of Craft's version into my own so that I'm refreshing both the raw <input> elements and any ones that might be output with {{ csrfInput() }} like this:

refreshCSRFs: function() {
    fetch('/index.php?p=actions/users/session-info', {
        headers: {
            'Accept': 'application/json',
        }
    })
        .then(response => response.json())
        .then(data => {
            document.querySelectorAll('[name="CRAFT_CSRF_TOKEN"]').forEach(item => {
                item.value = data.csrfTokenValue;
            });

            // also store in window so it can be reused by other scripts
            window.csrfTokenValue = data.csrfTokenValue;

            // also replace any of Craft's custom <craft-csrf-input> elements
            document.querySelectorAll('craft-csrf-input').forEach(element => {
                const input = document.createElement('input');
                input.type = 'hidden';
                input.name = data.csrfTokenName;
                input.value = data.csrfTokenValue;
                element.replaceWith(input);
            });
        });
}

Downside here is that on an initial page load with a Formie form, the CSRF token will be refreshed twice (once by Craft natively, and again by my own code), along with the wasted double fetch too, which is a shame.

[thisisjamessmith]

It'd be great to have a more robust solution to this issue. I'm still having to do a manual CSRF refresh for dynamically-injected forms, which don't even require a refresh anyway (because I completely exclude all ajax requests from being statically cached).

One solution I can think of would be to ditch the approach of using a custom <craft-csrf-input> element and just output the proper csrf input with a value. Then it would continue to work on non-cached templates. For cached templates, target that element instead of the custom element with your JS to refresh the value.

[elivz]

I've been looking into some similar issues for a project too. Honestly I wonder if the simplest & best solution would be to keep Craft's current naive CSRF-refresh behavior, but provide a way to opt out of having the fetch() code inserted automatically. That way those of us who want to do something fancy and custom can switch to only having Craft output the <craft-csrf-input> custom element from the csrfInput helper, and then write out own JS to replace it as we see fit. That also gets you out of having to foresee and deal with all the weird edge cases in core code.

Could be as simple a change as:

    public static function csrfInput(array $options = []): string
    {
        $request = Craft::$app->getRequest();
        $async = (bool)(ArrayHelper::remove($options, 'async') ?? Craft::$app->getConfig()->getGeneral()->asyncCsrfInputs);
        $refreshAutomatically = (bool)(Craft::$app->getConfig()->getGeneral()->refreshAsyncCsrfInputsAutomatically ?? true);

        if (!$async) {
            Craft::$app->getResponse()->setNoCacheHeaders();
            return static::hiddenInput($request->csrfParam, $request->getCsrfToken(), $options);
        }

        if ($refreshAutomatically) {
            Craft::$app->getView()->registerHtml(
                Craft::$app->getView()->renderTemplate(
                    '_special/async-csrf-input',
                    [
                        'url' => UrlHelper::actionUrl('users/session-info'),
                    ],
                    View::TEMPLATE_MODE_CP,
                )
            );
        }

        return static::tag('craft-csrf-input');
    }

[timkelty]

This is already possible via \yii\base\Controller::EVENT_AFTER_ACTION, for this (or any) controller:

        \yii\base\Event::on(
            \craft\controllers\UsersController::class,
            \yii\base\Controller::EVENT_AFTER_ACTION,
            function (\yii\base\ActionEvent $event) {
                if ($event->action->id !== 'session-info') {
                    return;
                }

                $event->result->data['foo'] = 'bar';
                $event->result->data['username'] = \Craft::$app->getUser()->getIdentity()?->username;
            }
        );

[elivz]

Oh, nice! I would still need a way to opt out of the default CSRF-fetching script though, or I would end up loading that session-info data twice on each page.

[thisisjamessmith]

Correct me if I'm wrong but I don't think you'd need to "opt-out of the default CSRF-fetching script" with that example, because you'd be piggybacking off of it in order to return your own extra data alongside the csrf token (cart contents etc) - so you'd just use Craft's call and delete your own.

I'm still a bit uncomfortable with these two bits of functionality (the ajax call + the no-cache http headers) being so inextricably intertwined, feels like Craft is doing too much. But assuming that the custom module approach Tim suggested above works, it's a viable solution to avoid the double fetch.

[timkelty]

I'm still a bit uncomfortable with these two bits of functionality (the ajax call + the no-cache http headers)

I hear you in regards to the ajax call, which is why it is strictly opt-in. As for the no-cache header, it feels like the correct behavior to me – Craft is returning un-cachable content, so it is setting headers to reflect that. The same is true with PHP – when a session is started, it will set nocache headers based on session.cache_limiter.

That all said, I know this has been a pain point for many, so I'm all ears on ways to improve it or change for the future.

[thisisjamessmith]

I think the docs should be updated at least to describe that not only does this setting inject some JS into your page, but also it prevents Craft from sending the no-cache headers. (The latter is really the only reason I'm using this setting - I think most devs who use static caching are well-acquainted with the need to refresh CSRF tokens in their own code).

[leevigraham]

Related to caching / user sessions I've just created this discussion: #17275