Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I think this was introduced in #477944: Fix and streamline page cache and session handling, but I'm not entirely sure when it happened. Currently anonymous users don't get a session unless one is manually started for them. In the ImageCache in Core patches (#371374: Add ImageCache UI Core and #491456: ImageCache preset API), we need a way for a user to have a token across two requests. The first request is the page itself, then a second request that asks for a generating image from Drupal. A token is passed along in the URL to prevent free access to generate image URLs, which could cause obvious drain on the server generating unnecessary images.
Anyway, the problem now is that since anonymous users don't have a session ID, there's no mechanism to create a token value and then check it on a subsequent request. This patch makes it so that the session ID is only added to the drupal_get_token() and drupal_validate_token() functions if the user actually has an open session. Otherwise it's just dropped from the MD5 hash and the token is generated purely based on $value and $private_key, which will be adequate in most anonymous user situations, and certainly better than skipping token checking entirely.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | drupal_token_anonymous.patch | 2.31 KB | quicksketch |
| #1 | drupal_token_anonymous.patch | 1.11 KB | quicksketch |
| drupal_token_anonymous.patch | 2.02 KB | quicksketch | |
Comments
Comment #1
quicksketchThat first patch needed to have a few ImageCache changes stripped out of it. Here's one that affects only necessary portions.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commenteddrupal_initialize_session() creates a random session_id() at the beginning of the request. This is to guarantee that in the case a session is actually started at the end of request, the correct session_id() was used by all the processing that happened beforehand (including token generation).
What we *could* do, is to build a special, anonymous, drupal_get_token(). This one will have fewer security and will be the same for all anonymous users.
Comment #3
quicksketchThis seems like a good idea, though I'm not sure if it's necessary to make an entirely different function. Couldn't we use something like ip_address() instead of session_id() if the session is empty? It seems like Session ID is the best indicator if it's available, but IP Address wouldn't be a bad compromise for anonymous users.
Damien gave me a good work-around for this in the ImageCache patch (basically inserting a cache table row before the request then checking it again when generating the image), but this same problem is going to pop up in other module. For example Fivestar needs to have a way to generate a token for anonymous users to prevent CSRF attacks that jack up ratings on a particular piece of content.
Comment #4
quicksketchIt took me a couple readthroughs to understand what this meant, now I get it. The current mechanism is important to keep because if a session *is* saved, the generated tokens need to be matched against that session ID. I'd still prefer not to make a totally separate function, maybe instead a parameter on drupal_get_token() and kill the $skip_anonymous flag on drupal_valid_token(). Since if we make a mechanism that works for anonymous users, we hopefully won't ever need to skip the checking.
Comment #5
quicksketchThis patch adds a parameter to drupal_get_token() and changes the last parameter in drupal_valid_token(), making it possible to get an IP-based token for anonymous users if they do not have a session. If they do have a session, that's always used, since it'll be more accurate than IP if it's available.
Comment #7
quicksketchWe ended up not needing this for the ImageCache in core patch, since we just made a cache table. However, #251792: Implement a locking framework for long operations would be a much better solution than both the cache table and this approach.
Comment #8
c4rl CreditAttribution: c4rl commentedRelated issue #1226218: Provide option for drupal_get_token() to invoke session for anonymous users.