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.
Comment | File | Size | Author |
---|---|---|---|
kernel-csrf-tokens.patch | 5.42 KB | effulgentsia |
Comments
Comment #1
Crell CreditAttribution: Crell commentedCommitted. Thanks.
Comment #2
Dave ReidShouldn't those changes use 'return MENU_ACCESS_DENIED' and not drupal_access_denied() in this case?
Comment #3
effulgentsia CreditAttribution: effulgentsia commentedBoth 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.
Comment #4
Dave ReidOk 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.
Comment #5
Crell CreditAttribution: Crell commentedDave: Have a look at http://drupal.org/project/issues/search/drupal?issue_tags=kernel-followup for our "will be replaced eventually" list. :-)