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.
CommentFileSizeAuthor
#10 views_php.module.diff889 bytespianomansam
#5 views_php_1379526_info.patch390 bytesjohnv

Comments

reg’s picture

reg commented

Oops, meant "php" where I said "php filter".

pianomansam’s picture

pianomansam commented
Status: Active » Reviewed & tested by the community

I second this. All you have to do is add one line to the info file.

johnv’s picture

johnv
Primary language Dutch
commented

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

reg’s picture

reg commented

I'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!

johnv’s picture

johnv
Primary language Dutch
commented
StatusFileSize
new views_php_1379526_info.patch390 bytes

Here is a proper patch.

Gaelan’s picture

Gaelan commented
Status: Closed (won't fix) » Reviewed & tested by the community

I don't think we shouldn't require PHP filter just for a permission it defines, especially because it is a dangerous module.

reg’s picture

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.

johnv’s picture

johnv
Primary language Dutch
commented

@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?

heine’s picture

heine commented

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

pianomansam’s picture

pianomansam commented
StatusFileSize
new views_php.module.diff889 bytes

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

brianV’s picture

brianV commented

I second the patch in #10.

reg’s picture

reg commented

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

casey’s picture

casey commented
Status: Reviewed & tested by the community » Closed (won't fix)

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

heine’s picture

heine commented

You might want to add a dependency for bad judgement as well then.

casey’s picture

casey commented

I accept patch for that, although I feel sorry for the 7000 users :)

pianomansam’s picture

pianomansam commented

Not 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

This module allows administrators to add fields, filters and sorts to views which use PHP code.

to something like this:

This module allows the admin account (user 1) to add fields, filters and sorts to views which use PHP code. It also supports additional accounts when the core PHP Filter module is enabled.

elijah lynn’s picture

elijah lynn
Location Portland, Oregon
commented
Status: Reviewed & tested by the community » Closed (won't fix)

Did this patch get committed?

lahode’s picture

lahode commented

It doesn't seem so... So still need to patch it all the time

Please commit!

pianomansam’s picture

pianomansam commented
Component: Code » Documentation
Status: Closed (won't fix) » Needs review

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

alexgreyhead’s picture

alexgreyhead commented
Issue summary: View changes

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

reg’s picture

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