Previous field values erased if user does not have edit permission for that field
Harry Slaughter - October 26, 2007 - 16:42
| Project: | CCK Field Permissions |
| Version: | 5.x-1.10 |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | patch (code needs review) |
Description
To reproduce this:
- Create a node as a user with full field permissions, set values for all fields and save.
- Edit the node as a user who does not have permission to edit some of the set fields and save the node without changing any actual node values.
The fields that the under-privileged user does not have access to edit will be set to empty (or possibly default values).
The following issues may be related to this problem
Also, there was an issue opened for CCK for this same problem, however the proposed solution does not seem to work: http://drupal.org/node/144263
#1
attached patch is against 5.x-1.10
this is not a pretty fix, but it's a start.
the current module completely removes form elements if a user does not have edit access for those fields. however, as far as i can tell, if you submit a cck node with missing form elements, then these fields are overwritten with the default values for the corresponding fields instead of preserving the existing values of those fields.
this patch leaves the form elements in the form and disables them so that the user cannot change them.
this may conflict with the 'view' field setting. if a user has neither view nor edit access to a field, he will still see the grayed out value when he edits a node.
the ideal way to fix this problem would be to somehow tell cck not to write certain fields to the DB, but I don't think that's possible.
the only other option i'm aware of is to make these hidden values, but the user would obviously still be able to 'view' them in the source.
IMHO, this patch is highly preferable to values being erased when a user does not have edit permissions.
#2
bah, i think the above patch is fubard, i'm just going to attach the already patched module.
#3
disregard this patch, it does not fix the problem.
this problem seems to only occur with userreference fields. it probably can happen with other oddball fields whose data structure is borked.
i'm going to work around this problem by using a native CCK data type (like int/select for user reference).
#4
The attached patch DOES fix the problem, but it's got some comments and stuff in it that need to be removed before commit.
I'd be willing to help maintain this module.
#5
thanks Harry,
could you attach the complete new .module file instead of the patch ?
#6
subscribing as patch didn't work in our case...
#7
subscribing...
#8
I am experiencing this same problem, having installed the CCK Permissions module before. It's not on my site anymore, but I still cannot edit nodes without the previous data in all of the fields disappearing. Has my data been messed up because of this module? I'm not sure what I can do.
I don't have any usereference fields, but I do have nodereference and nodereferrer fields.
#9
subscribing
#10
Another use case for this is if you want the CCK field to only use the default value, and not let the user change it. This makes it so that the user doesn't see the widget when editing/creating the node, but the default value still gets saved. Related issue.
#11
Tidying up a bit, http://drupal.org/node/118513 basically covers the same.
#12
I am not sure why this isn't working at the first place but anyhow, I added one line to the _cfp_form_group_fieldset_helper function. Seems to be working for me now but I'm not quite sure if I'm doing the right thing.
Can you even have something like this?
<?php$form['blabla']['bla'] = array(
'#tree' => TRUE,
'#type' => 'hidden'
);
?>
Here's the snippet:
<?php
function _cfp_form_group_fieldset_helper(&$form, $disallowed, $type, $verb){
if (module_exists("fieldgroup")){
foreach($form as $name => $item){
if($disallowed[$name]){
if (!(user_access(_cfp_content_to_readable($type, $name, $verb)))){
// derick: START **********
$form[$name]['#type'] = 'hidden';
//unset($form[$name]);
// derick: END **********
}
}
// check this item to see if it is a group
// if group, recurse to check for sub groups and or fieldsets
if (strstr($name, "group")){
if(is_array($form[$name])){
_cfp_form_group_fieldset_helper($form[$name], $disallowed, $type, $verb);
}
}
}
}
}
?>
#13
Hi, I think this (Patch at #4 above) will solve my problem, but I am not quite sure of the syntax you used for the patch.
"+" "-" vs. "<" :>"
You also have some "unset" but I am not quite sure where they are to be inserted.
Pardon my newbiness :) And thanks!
Chris
#14
Just to add my $.02 in to this.
- any of the patches above that do type = hidden are not the right way to do this: hidden is only hidden as long as the person doesn't look at the html source; plus using firebug it is pretty easy to still submit values that are "hidden"
I have patched one of the functions in cck perms to fix up an issue i had posted that Harry lists at the top:
"when a field that is hidden from a user is submitted by that user; the value gets lost" - possibly this may solve other issues here that are similar.
original code (5.10)
function _cfp_form_group_fieldset_helper(&$form, $disallowed, $type, $verb){
if (module_exists("fieldgroup")){
foreach($form as $name => $item){
if($disallowed[$name]){
if (!(user_access(_cfp_content_to_readable($type, $name, $verb)))){
unset($form[$name]);
}
}
// check this item to see if it is a group
// if group, recurse to check for sub groups and or fieldsets
if (strstr($name, "group")){
if(is_array($form[$name])){
_cfp_form_group_fieldset_helper($form[$name], $disallowed, $type, $verb);
}
}
}
}
}
the clue is that the function is ONLY meant to help remove the fieldset itself (hence the function name) - NOT any actual fields. To remove a fieldset it is required to do an unset; whereas if you do an unset on a field; the value will be lost when a submit is done.
my patch simply mods the one if statement to this:
if($disallowed[$name] && $item['#type'] == 'fieldset'){so that this routine only does what it was intended to do.
the routine which calls this then does the correct way to "hide" fields which is to set #access = false.
i have tested this on select fields, simple text fields, and field groups - haven't tested it with noderefs or other fields.
Peter Lindstrom
LiquidCMS - Content Management Solution Experts
#15
#16
I made that one line fix, and got the following error:
#17
good to know - but as te last line of my earlier post said:
i have tested this on select fields, simple text fields, and field groups - haven't tested it with noderefs or other fields.
that would include userrefs
is it safe to say (as i would expect) that without the line change it doesnt work either?
#18
Hi ptalindstrom
Oh I see - The error is just for the userref... ok. I must have been really tired and didn't even look at the error (oops).
But this is what I wanted to hide - actually two fields... One is a user ref, the other is a select field.
Right - without the line change, the default values aren't recorded when users submit a node that don't have field permissions for those fields.
You know - I've read the several related posts that discuss this issue... and I understand the need to do this the 'right way' (that is, so people can't see the fields just by looking at the html, or with firebug). But in my particular case right now, I'm not trying to keep the data private, but just trying to keep certain users from having to see things they don't care about (though certain roles do need to change the default value, so I can't just hide it with CSS unless dynamically per role). With that in mind, do you have any advice for how I might be able to 'work around' this issue on a per role basis? That would be way helpful...
:)
Scott
#19
sadly though, it doesn't work for userreference fields.. :(
#20
didn't find a clean solution to make this work for userreference fields; but i do have a solution. But first a few comments:
I had previously patched this module to work with fieldgroups correctly - bnot sure if this is still an outstanding issue or not. Part of the requirement for that was to have this modules system weight higher than that of the fieldgroup module (9). In a separate utility module for the site i am working on i adjust the CFP module weight to be +10.
This is important since the solution for making this work with userrefs needs to have a system weight less than that of the userref module (0). So to get 2 different weights i needed to create a new module just to make CFP work with userrefs. The code for this module is:
/**
* Implementation of hook_help().
*/
function cck_fieldperms_userref_help($section) {
if ($section == 'admin/modules#description') {
return t('Helper module to get cck_field_perms to work with userreference fields. Install hook needs to
set system Weight = -20.');
}
}
/**
* Implementation of hook_nodeapi().
*
*/
function cck_fieldperms_userref_nodeapi(&$node, $op, $form = NULL, $a4 = NULL) {
// bad hack to get around cck_field_perms not working correctly with
// userreference fields
if ($op == 'validate' || $op == 'submit') {
$type = $node->type;
if ($types = variable_get('cfp_types', null)) {
if ($types[$type]) {
$disallowed_fields = unserialize(variable_get('cfp_values', null));
if ($disallowed_fields) {
foreach ($disallowed_fields[$type] as $disallowed_field => $value ) {
if ($value == 0) continue;
if (!(user_access(_cfp_content_to_readable($type, $disallowed_field, "view")))) {
if (isset($node->{$disallowed_field}['uids'])) {
$node->{$disallowed_field}['uids'] = $node->{$disallowed_field}['uids'][0];
}
}
}
}
}
}
}
}