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')

read: #870938: Add new permission for controlling imports

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

> - 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 :)

Pasqualle’s picture

There 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.

joachim’s picture

> 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.)

Pasqualle’s picture

hmm, 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".

joachim’s picture

Status: Active » Needs review
FileSize
827 bytes

Here's a patch.

socketwench’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me. We really should use CTools import, but that'd be a bit of work to convert to. Create a separate issue?

joachim’s picture

#1679916: use CTools exportables? -- though there seem to be reasons why we shouldn't, or at least, why we should tread carefully.

joachim’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks everyone!

joachim’s picture

Version: 7.x-2.x-dev » 6.x-2.x-dev
FileSize
687 bytes

Committed this patch to 6.x-2.x.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

better English