Right now the delete block instance form is simply a menu callback. This means that there isn't any warning at all to prevent a user from accidentally deleting a block instance. It also makes the block instances vulnerable to Cross-site Request Forgery attacks (aka CSRF attacks). Meaning that a malicious site that an administrator visits while logged into the Drupal site could delete block instances simply by requesting the delete block URLs.
Adding a confirmation form prevents this problem by adding the security of form_tokens.
This patch adds such a form, and provides a central API function for getting all the blocks defined by multiblock (multiblock_get_block).
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | multiblock-delete.patch | 5.61 KB | andrewlevine |
| #1 | multiblock-delete.patch | 5.83 KB | quicksketch |
| multiblock-delete.patch | 5.68 KB | quicksketch |
Comments
Comment #1
quicksketchThis updated patch adds the $reset flag to the multiblock_get_block_list() function and call it from the hook_block() implementation. This makes it so that if _block_rehash() is called a second time in one page load, a new list of blocks is returned instead of the cached version.
Comment #2
andrewlevine commentedquicksketch,
These are good changes. I made a couple modifications, let me know if this is acceptable to you and I'll commit it.
1. I removed multiblock_get_block_list() because it is only a few lines and I'm not sure it would be used anywhere else
2. I set $reset of multiblock_get_block() to FALSE in hook_block because this is where we would gain any performance from saving in a static variable
3. I set $reset of multiblock_get_block() to TRUE in multiblock_general because we need the most up to date information for the admin to see and the admin for will rarely be used
I think that's all I changed in the patch, let me know.
Comment #3
quicksketchThis all looks great to me. That's fine that the reset flag has been switched around from my original patch, I'm probably in a bit of a strange situation where I'm manually calling _block_rehash() in my module that depends on Multiblock. I can easily call multiblock_get_block(NULL, TRUE) to reset the list manually before calling my own _block_rehash. Tested this all out and it looks just fine.
Comment #4
andrewlevine commentedcommitted, thanks so much for the patch.
Comment #5
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.