It's probably not an issue most of the time but I installed this and had no way of changing anything. Then I looked through the permissions page for "php" and saw that I had permissions for everything that was php but it still said I didn't have permission.
Fortunately I'm a developer so a quick look through the code showed it was looking for the permission "Use PHP for settings" and a second search showed that was from a system module called "php filter" that I didn't enable so it was dependent on something that I didn't have enabled. A non-developer might not have such a quick solution.
Alternative ideas for this, in the case you don't want that dependency just to install:
- add your own permission so you are not dependent on the php filter module
- as above but add your own permission to the view section of permissions (since it's purely a view extension anyway.)
- do a check in code for the php filter module and if it doesn't exist where you would normally write "you don't have permission" change it to "you need to enable the "php filter module" with a link to the modules page.
Comment | File | Size | Author |
---|---|---|---|
#10 | views_php.module.diff | 889 bytes | pianomansam |
#5 | views_php_1379526_info.patch | 390 bytes | johnv |
Comments
Comment #1
Reg CreditAttribution: Reg commentedOops, meant "php" where I said "php filter".
Comment #2
pianomansam CreditAttribution: pianomansam commentedI second this. All you have to do is add one line to the info file.
Comment #3
johnvI didn't understand this issue, since I do not have PHP filer enabled. Untill I tested it with another user then user 1, who has all permissions.
Comment #4
Reg CreditAttribution: Reg commentedI've done that... forgotten I was logged in as User 1 and not realized that something was broken for every other user... you are not alone!
Comment #5
johnvHere is a proper patch.
Comment #6
Gaelan CreditAttribution: Gaelan commentedI don't think we shouldn't require PHP filter just for a permission it defines, especially because it is a dangerous module.
Comment #7
Reg CreditAttribution: Reg commented@Gaelan: Sorry to disagree with you but I don't think the need for the module is there just for the use of it's permissions. From memory the programmer uses functionality in the module.
You could call it dangerous, personally I think that's an overstatement. It has it's purpose and as a general rule, yes... you want to avoid people being able to inject PHP directly. However, this module is all about PHP coding directly and to use another module that helps with that is completely acceptable. All security comes down ultimately to who you allow to do what and you have to assume that only trusted knowledgeable people will have access to the functionality.
Also, since it's a core module it's probably the best choice if you are going to be dependent on another module since it will be the most scrutinized so personally I think the programmer made some good choices and it's perfectly fine as is.
Comment #8
johnv@Reg, you say "From memory the programmer uses functionality in the module."
- Can you use functionality from a disabled module?
- When using the dependency, would a code-cleanup be possible?
Comment #9
Heine CreditAttribution: Heine commentedThe PHP module does not provide a clean API; It comes with baggage in the form of text formats and filters. Any vulnerability in a module that messes with formats and filters (incorrectly) is amplified from XSS to full blown PHP execution the moment the PHP module is activated.
Even if this module could depend on a subsection of the PHP module's functionality, the extras make a dependency undesirable.
Comment #10
pianomansam CreditAttribution: pianomansam commented@Heine, you have a good point. If we are enabling the PHP Filter module just to get a permission, that seems to be both overkill and dangerous. Instead, this module should create its own permission. Sure, it may overlap with the PHP Filter's permission if it is enabled, but it guards against possible issues with the PHP Filter module and it prevents running more code than needed. Here is a quick patch attempt to create a separate permission.
Comment #11
brianV CreditAttribution: brianV commentedI second the patch in #10.
Comment #12
Reg CreditAttribution: Reg commentedIf the only thing the php_filter module is being used for is permissions then I certainly agree, the patch should go in although if the dependency has already been added to the .info file then removing that should probably be in the patch as well.
Comment #13
casey CreditAttribution: casey commentedUser 1 has access to use PHP filters. If you want to grant other users to use them, you should install PHP module. Only patch I accept is a note about this in (the not yet existing) README.txt.
Comment #14
Heine CreditAttribution: Heine commentedYou might want to add a dependency for bad judgement as well then.
Comment #15
casey CreditAttribution: casey commentedI accept patch for that, although I feel sorry for the 7000 users :)
Comment #16
pianomansam CreditAttribution: pianomansam commentedNot to drag this out any longer, casey, but until there is a README.txt, could you in the meantime clarify the features of this module a bit more on the project page? The project page is currently rather unclear as to who can utilize this module. If the choice is to not require the PHP Filter module out of the box, then out of the box this module only supports User 1. I think it would be very helpful to mention this on the project page. For instance, perhaps you could change this line
to something like this:
Comment #17
Elijah LynnDid this patch get committed?
Comment #18
lahode CreditAttribution: lahode commentedIt doesn't seem so... So still need to patch it all the time
Please commit!
Comment #19
pianomansam CreditAttribution: pianomansam commented@Elijah and @lahode, the maintainer has decided not to patch the module for security reasons. They also promised an update to the README, but even that (if it has happened) is insufficient in my mind. We really need the module page to mention the need for the PHP filter to be enabled for this to work for users who aren't user 1.
Comment #20
alexharries CreditAttribution: alexharries commented+1 to Piano Man Sam's comment.
Please can we either change the message "You do not have permission to modify this" be changed to "You need to enable the PHP filter module to modify this", or implement a separate permission in this module?
It seems silly to have an undocumented dependency on the PHP filter module.
Please? :)
Comment #21
Reg CreditAttribution: Reg as a volunteer commented+2 to Piano Man Sam's comment.
I don't see how security reasons even play into it or how adding a dependency line on the module's .info file is any more work than adding a note in a README.TXT file. I do see how adding a dependency line in the .info file is more natural to the Drupal way however, and, so is just setting up your own permissions as an alternative to using another module's permission.