As exported flag is PHP code, the import has security implications.
The 'administer flags' permission currently used for the flag import menu item, does not mark it as such.
solutions:
- add 'restrict access' = TRUE to 'administer flags' permission
- create new permission and add 'restrict access' = TRUE
- do the access check as views module: user_access('administer flags') && user_access('use PHP for settings')
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 1372850.flag_.d6.flag_.php-import-perm.patch | 687 bytes | joachim |
| #5 | 1372850.flag_.php-import-perm.patch | 827 bytes | joachim |
Comments
Comment #1
joachim commented> - do the access check as views module: user_access('administer flags') && user_access('use PHP for settings')
That forces sites that want flags to be importable to also enable the PHP filter module, which they may not want to do.
I am leaning towards:
- create new permission and add 'restrict access' = TRUE
But I'll see what my new co-maintainer thinks about it too :)
Comment #2
pasqualleThere is a new permission "use ctools import" introduced in #870938, that would be the best solution. Please help with that patch for ctools, then you just need to change the flag import menu item access to this new permission.
Comment #3
joachim commented> There is a new permission "use ctools import" introduced in #870938, that would be the best solution
Right, but Flag module doesn't depend on CTools.
(Though feasibly, we might want to look into using the CTools exportables system rather than roll our own, but that shouldn't hold up this issue.)
Comment #4
pasquallehmm, you are right. I thought every module uses the CTools exportables already..
I would go with the new permission also, as enabling the PHP module is just wrong. "use flag import" would be a good name if ctools module will have "use ctools import".
Comment #5
joachim commentedHere's a patch.
Comment #6
socketwench commentedLooks good to me. We really should use CTools import, but that'd be a bit of work to convert to. Create a separate issue?
Comment #7
joachim commented#1679916: use CTools exportables? -- though there seem to be reasons why we shouldn't, or at least, why we should tread carefully.
Comment #8
joachim commentedCommitted. Thanks everyone!
Comment #9
joachim commentedCommitted this patch to 6.x-2.x.
Comment #10.0
(not verified) commentedbetter English