Download & Extend

Move CSRF token validation back to page callbacks

Project:WSCCI
Component:Code
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

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.

AttachmentSize
kernel-csrf-tokens.patch5.42 KB

Comments

#1

Status:needs review» fixed

Committed. Thanks.

#2

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

#3

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.

#4

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.

#5

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

#6

Status:fixed» closed (fixed)

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