Dynamically create token value string based on route path [#2133439]

Updated: Comment #N

Problem/Motivation

Now that we have initial patch for CSRF integration with the routing/access system it's evident that we should probably just dynamically create the value passed to CsrfTokenGenerator::get() so stuff like 'aggregator/update/' . $aggregator_feed->id()) would get taken care of automatically.

Proposed resolution

Amend the CsrfAccessCheck and RouteProcessorCsrf classes to use the route path. In the route processor we will have to cycle through the parameters and replace them in the path to generate the token. Initial patch attached with just the proposed idea.

Remaining tasks

Establish best way, patch, amend/add to test coverage

User interface changes

None

API changes

Possibly* _csrf_token: 'my-value-here' will just be replaced with _csrf_token 'TRUE' etc... as the value will become irrelevant.

Comment	File	Size	Author
#5	interdiff-2133439-5.txt	2.09 KB	damiankloip
#5	2133439-5.patch	6.86 KB	damiankloip
#5
#4	interdiff-2133439-4.txt	4.75 KB	damiankloip
#4	2133439-4.patch	6.56 KB	damiankloip
#4
	d8.csrf-dynamic-token-value.patch	1.82 KB	damiankloip

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

damiankloip CreditAttribution: damiankloip commented 11 November 2013 at 18:43

Comment #2

Crell CreditAttribution: Crell commented 11 November 2013 at 20:00

Damian and I discussed this in IRC earlier. This gives us a small DX win (the user is no longer responsible for deciding on a token seed key) and reverts a slight security regression from D7. (The key in HEAD right now is only as unique as the route, not as the path; that's not a critical part of it's randomness which is based on the user and site key as well, but this neatly resolves that.)

+++ b/core/lib/Drupal/Core/Access/CsrfAccessCheck.php
@@ -51,7 +51,7 @@ public function appliesTo() {
-      return $this->csrfToken->validate($request->query->get('token'), $route->getRequirement('_csrf_token')) ? static::ALLOW : static::KILL;
+      return $this->csrfToken->validate($request->query->get('token'), $request->getPathInfo()) ? static::ALLOW : static::KILL;

I think this needs to be checking the system path, not pathInfo. Remember, pathInfo is raw; system path is post-dealiasing.