Here's a feature request that would be required to allow a module implement hook_field_access() and grant access to a field based on node data.

The problem: Actually, hook_field_access() can only be used to grant/deny access to a field based on user permissions.

Use case: We need to implement a module that grants/denies access to a field based on node author provided settings. For example, we are using content_profile, which is CCK attached to the user profile. We want to let each user decide if certain fields are private (only the user can see the field), or these fields can be viewed by all users, or these fields can be viewed by (say) friends.

We can do this with a module such as CCK Private Fields, which is in the kitchen right now. This module allows the user select the privacy options for certain fields in the content_profile. But then, to check if another user can view these fields, when implementing hook_field_access(), we need to get access to the node to load the privacy settings defined for that particular user (that content_profile node). So, ideally, we would have to provide the $node to hook_field_access().

If the node was not available, then we could not check the privacy settings for those fields, so we would have to deny access to them for privacy reasons.

I've been looking at all places where content_access() is invoked, and it seems pretty easy to add the $node as an additional argument that could be passed to hook_field_access(). This feature would not affect existing code because an additional argument does not affect the behavior of those who implement hook_field_access().

I hope it makes sense.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markus_petrux’s picture

Status: Active » Needs review
FileSize
4.38 KB
4.34 KB

Ok, here's the patch. Well, one patch for CCK2, and another one for CCK3.

We're adding here an additional argument to content_access(), which is passed to hook_field_access(). The new argument is the node being processed, and this is aimed to provide context for the modules that implement this hook.

Since this new argument $node is optional (NULL by default), it should not generate any compatibility issues with existing code.

The $node can be provided everywhere content_access() is invoked, except for the 'access callback' used for view fields. This callback is evaluated by views before a real node exists, so we don't provide one. However, since the fields in views are rendered using content_format(), and we have a $node here, then no problem because content_format() already invokes content_access(), and now a $node can be provided. Then, it is up to the module implementing hook_field_access() to react accordingly when a $node is provided or not. In any case this new possibility should not affect existing code because this new argument would be completely ignored.

The benefit of this new argument is that a module implementing hook_field_access() can grant/deny access based on data existing in the node, which opens a lot of possibilities. :)

pcambra’s picture

Hi

I am testing this for CCK3 and it's working fine with CCK Private Fields module (http://drupal.org/project/cck_private_fields).

I think that having $node when content_access() is invoked is very handy for modules like this one.

yched’s picture

The views case might be a little trickier: when content_format() checks content_access(), there is a $node, but it's a pseudo-node built from the values retrieved by the Views query, not a full fledged node - so hook_content_access() implementations relying on some $node properties might not find them.

Also, not sure what is the $node on a node creation form ?

CPyle’s picture

I have a similar need: #519404: CCK Field Privacy integration.

This would be used to patch CCK Field Privacy.

markus_petrux’s picture

@yched at #3:

Thanks for checking! :)

Also, not sure what is the $node on a node creation form ?

It's built by the node module. First time it just contains uid, name, type, language, created, status, etc. When using preview, if no error is found, the node structure is then updated with form values. So, it is basically a node that can be used to check for 'edit' permission. It is reliable enough, even when fields are updated in the form. The only place where that node is not updated, is when the form is being validated, but that does not affect our content_access('edit') check, which is evaluated when the form is built/rebuilt. Form rebuild will happen after the form is validated for preview, and then the node will be correctly updated with form values, so content_access('edit') can be used again to take into account changes supplied by the user in the node edit form.

The views case might be a little trickier: when content_format() checks content_access(), there is a $node, but it's a pseudo-node built from the values retrieved by the Views query, not a full fledged node - so hook_content_access() implementations relying on some $node properties might not find them.

True. This is only a pseudo node with minimal information, but this can be checked by those implementing hook_field_access('view') and node_load() when necessary. That way we don't need to blindly perform a node_load() in the render() method of CCK fields in views. I think this is enough as it provides the way to do it for contrib modules. Otherwise, it is a lot harder to check access to fields based on node data. This method makes things a lot easier.

I might have missed something that could be improved in this patches. :-/ Suggestions?

markus_petrux’s picture

@CPyle: Have you had the chance to test this patch? Worked for you?

I think we can commit this when we're 100% sure it can really be used in node edit form, node view, and Views where fields are subject to privacy settings.

My theory is that it works, but it would be nice to get some feedback from real world. :)

CPyle’s picture

In my testing, this patch allowed me to integrate CCK Field Privacy with node viewing, views, and panels without the need for a views.inc or any nodeapi functions, all through implementing the field_access hook. It works like a charm and eliminates something like 3 pages of code.

I noticed no irregularities in my node edit forms, but then again, I don't know exactly what kind of irregularities I'm supposed to be testing for.

markus_petrux’s picture

The weakest point in this is mostly in Views. You may want to test if a private fields in views is visible only to the users who really can. If that works, then this proves the approach in this patch works, and then I think we can commit. :)

Could you please confirm how it goes using private fields in views?

CPyle’s picture

Views integration seems okay. I made an openly accessible view containing a node with a private field. The field's value shows up when I access the view as the author but not when I view it as another user. Panels integration is working as well. I am not using any modules that would affect individual field access other than cck_field_privacy.

yched’s picture

@Cpyle: you tested this with a view that lists individual fields, not full nodes, right ?

CPyle’s picture

@yched: Yes.

The view used an unformatted style with fields. I had the node title and one other field added. That field was able to be made private.

The original way CCK Field Privacy worked was to include a bunch of prerender altering in a views.inc file. I was able to completely delete that file and use a field_access hook instead to accomplish the same thing.

Again, my node edit forms and everything else seem to be unaffected by the change, but I don't know what irregularities I would be looking to test for.

markus_petrux’s picture

For normal node views, content_access('view') will contain a full node, so a content profile viewed by someone else should hide the fields the owner defined as private, which is something similar to what happens in views, but not so complex.

Then, content_access('edit') is checked when the form is built, but I think this does not affect Fields Privacy.

The critical factor here was to confirm whether the model works in views, I think. @yched?

yched’s picture

Great, works for me.
Thanks for the quick feedback, CPyle !

markus_petrux’s picture

Status: Needs review » Fixed

Committed to CCK2 and CCK3.

Thank you all for testing and providing feedback so quickly. :)

Status: Fixed » Closed (fixed)

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