The problem at hand is that when Features are used to create content types Drupal registers Features as the content type's module which means that access control for example is being met by features_access() function.
The use-case is that while Features are great to export/import content type and other entities it may very well be required to control real access to the content type in question (remember that besides 'create', 'delete' and 'edit' there's also 'view' access which sadly there's no permission for).

Without this patch, to control access to view/load a specific content type would be only possible by:
1. patching and making changes to features_access()
2. implement logic using the node_access system which is really an overkill.

The provided patch hands the access control to whatever modules that wish to implement the required hook and in that sense it plays well with others and truly provides modular access control.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lirantal’s picture

lirantal’s picture

Status: Needs work » Needs review

anyone willing to comment?

Grayside’s picture

Status: Needs review » Needs work

It is trivial to go into the hook_node_type() implementation and swap out 'features' for the current module name. It might even be possible to have that be cooked in to how the node type is exported.

No need for strange workarounds.

lirantal’s picture

@Grayside
Ah? but then it requires to modify the feature code after it was created... and what will happen in successive re-create of the feature?

It's not a "strange workaround" it's called modularity and provides other modules to hook into the access system of features which is an exported module code.

Grayside’s picture

See if hook_node_type_alter() works for you.

hefox’s picture

Status: Needs work » Closed (won't fix)
  • this is not relevant to d7 (since there is a hook_node_access that anyone can use)
  • there's a reasonable, standard way to do it (May need a patch; there was a bug with changing module key, but I think it got commited).
  • There's also hook_node_records, etc.

thanks

mgriego’s picture

Status: Closed (won't fix) » Active

I'd like to re-open this. I know that this issue isn't a problem for D7, but for those of us still using D6 (and who really don't want to enable the weighty node permissions system), this is still a problem. I've worked around it with the code here, as I personally think this patch is closer to how D6 is "supposed" to work. Can this be considered for inclusion in the D6 version?

function features_access($op, $node, $account) {
  // Allow the feature that owns the node type to determine access permissions
  module_load_include('inc', 'features', 'features.export');
  $modules = features_get_default_map('node');
  $type = is_string($node) ? $node : (is_array($node) ? $node['type'] : $node->type);
  $access_function = (function_exists($modules[$type] . '_access')) ? $modules[$type] . '_access' : FALSE;
  if ($access_function) {
    return $access_function($op, $node, $account);
  }
  else {
    return node_content_access($op, $node, $account);
  }
}

It might also be prudent to modify features_perm() to not include the standard create/delete/edit permission definitions for a node type if the owning feature implements hook_access().

Thoughts?

amorsent’s picture

I like #7

malcomio’s picture

FileSize
843 bytes

I've changed #7 slightly for coding style - here's a patch

malcomio’s picture

Status: Active » Needs review
ryan_courtnage’s picture

Status: Needs review » Reviewed & tested by the community

#7 does a fine job of solving the problem, TY. Use of hook_access() in Features-created modules is going through our QA dept right now. The patch is solid when applied against 6.x-1.2.

Edit: I should add that we've tested both with and without the use of a node access module (content_access).

malcomio’s picture

FileSize
1005 bytes

My team did some performance testing of the patch in #9, and for low traffic sites, the patch should be fine, but we did see a performance impact.

Here's a new patch which caches the features node map

malcomio’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.04 KB

After more testing, we realised that this was causing problems with creating content in certain cases, so here's a revised patch.