menu items that link to privatized nodes stll appear on the menu to blocked roles.

Comments

dschuetz’s picture

dschuetz commented
Title: node privacy by role not integrated with menu » Is this an issue with menu.inc?

This is an issue for me, as well. However, I think the problem goes beyond node_privacy, as the same issue occurs with taxonomy_access_control.

I did some poking around, and it seems that in menu.inc there's a function _menu_build_visible_tree, that goes through the menu tree and prunes off any branches that are inaccessible to the current user. This should be a good place to check, but I can't figure out quite how to do it. I'm *guessing* that as the system walks through the tree, it could call node_access() to see if that node is available to the current user. And it's my understanding that node_access() would then call other modules such as node_privacy_by_role or taxonomy_access_control, to determine whether access should be granted.

Problem is (assuming that my understanding is even nearly correct), at this point we've only got a bunch of paths, not actual node objects, and it seems like node_access wants a node object, not a path to a node.

Would this be the right approach? Should this really be a very simple matter of converting a path to a node, checking whether the current user has read access for that node, and if not, then simply stop following down that menu branch?

Anonymous’s picture

(not verified) commented
Title: Is this an issue with menu.inc? » Blocked menu items still appear in menu

I've done a little more research into this and have come to a couple of conclusions.

First, we can't simply "check" to see if a given menu item points to a "screen" to which the user has access. We could if menus could only point to specific nodes, but they often point to pages that are themselves comprised of other content, which may or may not be viewable by the user. We'd have to essentially create each page, check whether anything was actually visible, and then decide whether to include the menu item.

I solved this for myself (well, I've *hacked* a solution, but it's not nearly complete) by simply adding a check in _menu_item_is_accessible(), in menu.inc. I do a basic check of the user's roles against a comma-separated list of roles, stored in a "roles" field for the menu item. It seems to work, but I've had some issues with it that I haven't had time to investigate.

I'm not sure that we can hook into this routine, and I couldn't find any other way to accomplish this, so any fix to this will require a modification to menu.inc in the core, I think.

Is anyone else even looking into this?

dschuetz’s picture

dschuetz commented

that last post was me -- I forgot to log in. Also, I copied my comment to a more general forum discussion here: http://drupal.org/node/14444 in the hopes that more people will chime in.

ankur’s picture

ankur commented

Will look into it.

-Ankur (maintainer)

ankur’s picture

ankur commented

Refreshing the issue for the 4.5 -> 4.6 port.

ankur’s picture

ankur commented

Caught up by reading the comments above and following the links. You're right: menu items are not nodes and therefore cannot have permissions for them set by node_privacy_byrole. In fact, as much as I know, the privacy of menu items is usually determined by the modules that generate them.

When drupal decides whether a navigation link should be displayed, it does so by asking the function in the module that generates it. This usually is something in the menu hook that has code that either calls user_access() or checks some other condition to see if a menu item should be displayed. Of course, user_access() checks the permissions by seeing what roles have been granted what privelleges on the roles/permissions page.

Finally, there may be a way to do custom menu items and have a seperate roles-based permissioning system for who gets to see those menu items, but it may require the implementation (or modfication) of a seperate module for handling privacy on an item-by-item basis. Not sure; would have to research it.

-Ankur