Updated: Comment #N

Problem/Motivation

We'd like the SearchBlockForm and UserLoginBlock forms to be cacheable by default. But we can't, because both contain a form token that depends on the session ID. And some sites initiate sessions even for anonymous users.

Proposed resolution

Remove #token from those forms, to allow them to be cached.

Alternatively: http://drupal.org/project/cacheable_csrf

Possible drawbacks are flood control.

Remaining tasks

TBD

User interface changes

None

API changes

TBD

Comments

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

My concern with flood control is the following situation:

1. Login (or whatever) form flood controlled by IP

2. CSRF attack on the login form.

This would enable someone to be locked out of their own account, despite the flood control being per-IP.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Title: Remove #token from the search and user login forms » Cacheable form blocks: Remove #token from the search and user login forms
Parent issue: » #2124957: Replace 'content' cache tag with 'rendered' and use it sparingly
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented

It looks like another drawback is that JavaScript would be required?

I think that this is a low-priority task at best. Maybe the best thing to do for now is to mark form blocks as *uncacheable*? That would prevent end users from accidentally breaking their login or search blocks and then file support requests.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: Cacheable form blocks: Remove #token from the search and user login forms » Cacheable form blocks: Remove #token from the user login form
Related issues: +#2571995: GET forms shouldn't have CSRF tokens by default

#2571995: GET forms shouldn't have CSRF tokens by default already fixed this for the search block.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Related issues: +#2463567: Push CSRF tokens for forms to placeholders + #lazy_builder, +#2503755: Switch from user login block to login menu link and search block in standard profile

#2463567: Push CSRF tokens for forms to placeholders + #lazy_builder will actually make the user login form cacheable, but it does NOT remove the #token.

If #2503755: Switch from user login block to login menu link and search block in standard profile also is finished and committed before RC1, then the login block will simply not show up by default, further diminishing this concern.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

I still have the same concerns as #1 with removing the token - there's a potential denial of service if the login form is CSRF-able and login flood control is enabled.

Even if we made the flood control start a session on first attempt then track to session, you'd still need to CSRF-protection.

So I think this is probably won't fix in terms of removing the token, and eventually becomes a duplicate of getting placeholdering working with varnish/CDNs.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Active » Closed (works as designed)

Yep, agreed. So to address the perf concern of this issue, #2503755: Switch from user login block to login menu link and search block in standard profile is a better solution: it solves the performance concern by not showing the login form on every page by default :)

See you there!