UMN Usability + permission problem: PHP code for field value settings etc.

I think this is more than just a usability issue - allowing potentially minimally privileged admins to execute arbitrary PHP code is, imho, a very serious bug. I think at a minimum there needs to be an additional permission. Attached patch isn't perfect, but represents a possible start.

this patch get number.module too

Using an explicit permission is fine by me. I'm not sure we need to make it that dramatic - I mean, book.module has a simple 'use PHP for block visibility'.
The 'read-only' form element displayed when the user does not have the permission is cool, but :
- I'm not sure I get the "and may override other values" part in the description text - what case are you thinking of ?
- We shouldn't display anything if there is nothing to display (i.e no PHP code has been set)

Peter, can you expand on your reasons to write "Attached patch isn't perfect" ? Other than the remarks above (which I can handle if we agree), I'm fine with committing it.

the permission name is a bit too long, but I think the words "dangerous PHP" should be included if possible. Yes block module has a less scarey-sounding permission, but hopefully some more dramatic warning text will be attached in D7.

The text about "oveeride other values" is since the PHP code will supplant something but in the textarea - so an admin who can only enter values in the textarea will potentially be confused as to why it has no effect.

It would be fine to omit everything if there is no code, I jsut thought it might be more consistent and let a non-PHP admin confirm that there is no code.

So, these are all minor things to be resolved - hence the imperfection :-)

Makes sense - what do you think of this ?

Well, that permission is even more wordy, but I don't think that really matters id you're satisfied with it.

There was a parse error in content.admin.inc Fixed in the attached patch.

I also used < since I think my entering <none> was a mistake and would have been parsed as a tag.

previous patch had a parsing typo...

cross-posted. see mine in #7 which fixes the other minor problem too.

Committed #7 :-)
I won't have the time for a D5 backport tonight, though (and probably not before a few days)

how different is the code?

Er, not that different, actually - I'll try to give it a shot :-)

OK, committed to D5 and D4.7 branches.
I also changed the permission name to a more generic 'Use PHP input for field settings (dangerous - grant with care)', so that we don't painted ourselves in a corner for possible future uses of PHP input

Thanks for moving it forward! I was going to look this morning.

In 6.x-2.x at least the perms names don't match now between content.module and the other code, so I think this patch is needed.

Maybe for a future version of CCK (6.x or 7.x) this could be refactored into a module hook? At least from my perspective, including the ability to enter PHP at all is problematic.

the last patch is for 6.x-2.x

I just tested this last (trivial) patch and it works for 6.x.

Heh, I'm a jerk - and I'm not able to commit anything before tomorrow. Karen, are you there ? :-/

I got it, committed :)