Debatable the value of $access is. But administer filters users can enter PHP code anyways, so that looked like a good candidate.

Comments

Bèr Kessels’s picture

I dislike the use of administer settings. We should avoid using untransparaqnt logic for showing (or hiding) stuff. For users who cannot read PHP it will appear as if the options show at random.

FWIW: some more options to solve this issue:
* Remove the "visibility settings" alltoghether and leave it to the themes
to choose where, when and how to display blocks.(my favorite option)
* Limit the allowed PHP. this, i fear is a very, very hard one. One that will
render php mode unusable too.
* Only show (and save!!) the phpmode option for uid 1. I dislike this, because
I prefer to do nothing with uid1.

chx’s picture

themes should not have this heavy logic. The PHP code here can have DB interaction. No limit to PHP.Why only UID 1? Limit to users who have PHP right. The only debatable point IMNSHO is whether we introduce a new permission or reuse one. I reused administer filters (and not administer settings).

Bèr Kessels’s picture

«Limit to users who have PHP righ» sounds like a nice thing to me. But, there is always a but :)

* We don't have such a permission. Filters have no permissions, but define their own logic for "permissions"
* We don't have a PHP permission, if we want to introduce a "Use PHP code" permission, we should extend this to the filters, this would mean that php filters and the filters admin UI, will have to follow these permissions somehow.

I would love such a permission, but foresee that introducing that, is a quite big project. Not something we can catch in a small patch. Not if we want to do it clean, and respect the usability.

So if we fix this issue in the way CHX proposes, we will have to go trough two small steps. Both, IMO are too big to slip into 4.7.
* Redo the filters, and remove the role administration from the filter admin UI. Then add a normal permission per format, named something like "Use %format_name input format".
* Redo the permissions and introduce a "Can post evaluated PHP code" permission. The above mentioned permissions will respect this one, obviously.

Secondly, Dries proposed to just add the filter-formats to that box. IMO that is not a good option, for we will then list options that make no sense at all for the visibility-settings. We will then often show things like "[[link|linkname]] will be converted to a link" with the visibility settings area. It will confuse people.

This issue is harier than it might seem from a distance.
PHP input is dirty. Dangerous yet powerfull. For the lonely site admin its very usefull, for the hosting provider, or drupal-sites-server maintainer this is Huge-Never-Ever-Do-Thing.
Respecting both, allowing the power for those that can handle it, yet not break drupal if it is switched off completely is a hard thing to do right.

My shot at this, is a whole different route: I am developing a *cough*nodes-are-blocks.module*cough* named microcontent.module :) maybe that completely different appraoch will fix this issue too, who knows?

chx’s picture

Then create a new permission for this thing alone. Do not release Drupal 4.7 without this solved.

Bèr Kessels’s picture

StatusFileSize
new3.51 KB

Here is an updated patch, following down the "lets do this quick for now" route.

I introduced a new permission in system.module called "has PHP rights".
The dirty part, is that, IMO this permission should be side wide and thus influence the filters too. This patch *does not* do that, though.

So while this is a solution that is just good enough for 4.7. I am not too happy with it from usability and consistancy point of view. But we all know, that if in this last minute we are going to completely overhaul our filter system, we will be stuck in a far too big problem to get this one solved.

I added the has PHP rights perm to system.module on purpose, because, eventhough it is only used in block module, it should be used system wide, in future.

Steven’s picture

The reason I didn't use permissions for input formats is because the permissions would change when you rename an input format. Now that we have a proper permission table, this isn't so much of a problem.

But I'm not sure how the unified PHP permission should look for the filter system. Do we just have a "use %format input format"? What happens if you delete the PHP input format or modify it?

It is still too unclear how this will work, so I'm against a generically named "has PHP rights" permission for now. We can rename the permission through an update for 4.8, and name it appropriately specific for now.

chx’s picture

StatusFileSize
new3.01 KB

OK, I created a separate permission.

Steven’s picture

Status: Needs review » Fixed

Committed to HEAD.

Bèr Kessels’s picture

Status: Fixed » Needs work

Chx, please look at my patch, how I generate the help and the options.
Your patch has a bug, since it shows help about how to use PHP input to people who do not have these rights.

chx’s picture

Ber, could you please finish my patch ie. change the description to a variable? Thanks.

Bèr Kessels’s picture

Category: feature » bug
Priority: Critical » Normal

The help text bug, introduced by this patch, is a non critical bug.

Jaza’s picture

Version: x.y.z » 6.x-dev

Moving to 6.x-dev queue.

I don't think that this is a problem, personally: call me 'too assuming', but I think it's safe to assume that 'administer blocks' users can be trusted with PHP-mode.

chx’s picture

Status: Needs work » Fixed

I think this is fixed with the introduction of php module.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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