Per the review in #1463656-137: Add a Drupal kernel; leverage HttpFoundation and HttpKernel and related discussion in later comments, validating CSRF tokens within menu router access callbacks is incorrect, because those callbacks are invoked during menu link generation in addition to request handling. IMO, the correct long term solution is to add another column to {menu_router} for identifying routes that require CSRF protection, and handling token validation as a separate listener, but because that requires a schema update, I think it should be deferred to #755584: Built-in support for csrf tokens in links and menu router rather than be included in the initial kernel patch.

Therefore, this patch reverts to token validation in page callbacks.

CommentFileSizeAuthor
kernel-csrf-tokens.patch5.42 KBeffulgentsia
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

Status: Needs review » Fixed

Committed. Thanks.

Dave Reid’s picture

Shouldn't those changes use 'return MENU_ACCESS_DENIED' and not drupal_access_denied() in this case?

effulgentsia’s picture

Both will be made obsolete in a post-kernel follow-up: #1591604: Replace drupal_access_denied() with throw AccessDeniedHttpException, so doesn't matter. Also, these have todos to replace with a csrf token listener anyway.

Dave Reid’s picture

Ok thanks for the clarification Alex! Getting started it's hard for me to know what is and is not being replaced eventually. It is a lot to absorb.

Crell’s picture

Dave: Have a look at http://drupal.org/project/issues/search/drupal?issue_tags=kernel-followup for our "will be replaced eventually" list. :-)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.