Would you consider a patch for inheriting node protections? [#286028]

Hi,
this is just an idea so far and before I start implementing this I would like to ask if this had chance to make it into the module.

We sometimes want to protect a complete subtree with the same password, e.g. for groups of students which we don't want to give a login just to download materials. Say we have a menu tree
Group 1 -> Node 1
|-- Material -> Node 2
|-- Excercises -> Node 3

and Node 1 is a protected node with a password. Then when creating Node 2 and Node 3 and giving their menu entries "Group1" as parent menu, they should become protected nodes and inherit the password from Node 1 automatically. That's because our research assistants expect sth. similar to .htaccess where they could easily protect a whole subdirectory with just one password. With protected node as is, they tend to protect Node 1 but forget to protect Node 2 and Node 3...

I could add a "inherit from parent" checkbox in the edit form for the protected node settings (plus a global option if this should be on or off by default), and if that's checked, I would hook into nodeapis 'presave' (or insert/update) to test if
- the node has a menu entry
- that menu entry has a parent
- the node of that parent menu is a protected node
If that is all true, copy the password from that parent node to the current node.

Shouldn't be very difficult. Do you think this would make sense and might go in?

Comment	File	Size	Author
#2	protected_node_inherit.patch	35.89 KB	Frank Steiner
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

mtolmacs CreditAttribution: mtolmacs commented 23 July 2008 at 08:18

I very much like the idea! Several weeks ago I promised a general solution will be implemented to solve the multi-node same-password problem along with many special cases discussed in feature requests of this module.

Currently I have no solid idea how to solve this general problem so your idea could come to rescue. If you're willing to implement this feature and send in a patch I will definitely merge it.

Thank you again!

Comment #2

Frank Steiner CreditAttribution: Frank Steiner commented 5 August 2008 at 09:55

Status:

Active

» Needs review

File	Size
protected_node_inherit.patch	35.89 KB

Hi,

here's the first try. Sorry in advance for the long post :-) The patch is quite large because of the many and sometimes complicated case distinctions.

The central changes are all in the nodeapi hook which has to distinguish a lot of cases, especially when updating or deleting a node. The rest of the changes are mainly helper functions to fetch or store information from/to the database or to handle menu trees.

Tolmi, my editor (xemacs) has a very different setting for text indents from yours, so all parts that I added or changed don't match your indent settings anymore. I re-indented the whole nodeapi and form_alter hooks because I chanced so much in them. I'm sorry for that but I couldn't work without indents :-(

The details are all explained in the nodeapi hook etc. Here I'll try to explain the genereal idea and the design decision that I had to take.

Inheritance is based on menu hierarchy. I.e., if node B has a menu entry MB, it can inherit the protected password from node A with menu entry MA if MA is the parent menu item of MB, i.e.:
MA -> A
\- MB -> B

This allows to protect a complete menu subtree with the same password and is my best try to imitate the htpasswd/.htaccess functionality for directories. Nodes that are all below the same menu item, are similar to html pages in a common directory (especially when using pathauto it looks like this).

If I'm talking about "children of a node" in the following I mean that in the sense of B being a "child" of A in this picture.

There is a new "Inherit protection from menu parent" in the node edit form. When checked, it disables the "Node is protected" checkbox because the node will inherit the password from the parent, so it will be protected or not depending on the parents state.

When saving a node which inherits, it will try to fetch its parents password and store it for itself. If the nodes menu entry has children, it will pass the password to the children so that those which inherit from the node, can update their settings, too.

Thus, changing a password for a node can spread the new password through the whole subtree.

But of course, you can give any node in the tree its own password by unchecking the inherit checkbox and using a different password for this node.

The database got a new column to store the inherit flag. You must execute update.php after applying the patch. We can have entries without a password and just the inherit flag store, if a node inherits from a not-protected parent.

On the admin settings page, you can enable inheritance and define a default behaviour for the new "Inherit protection from menu parent" checkbox in the node edit form.

When editing a node, the inheritance checkbox will be set to the global default if no record has been stored yet. You can chose to have inheritance checked by default to make sure you don't forget to protect a new node that you insert into your protected tree.

If inheritance is enabled in the admin settings, a value for the inherit flag is always stored when you save a node, even if it matches the current default and the node doesn't have (or inherit) a password. This might create many unneccessary entries in the db, but otherwise many nodes which didn't inherit when they were stored, might suddenly have to inherit when the global default flag is switched to "inherit by default". That would mean a lot of extra work and I don't consider this a good policy. If you store node and the inherit box is not checked, it should stay like that until you re-edit the node and change it.

If a node inherited a password and you uncheck the inherit box in the node edit form, the password will be kept.

If a node inherited a password and you chose a different menu parent item in the edit form, it will inherit a new password or become unprotected if the new parent doesn't have a password or if you chose the menu itself as parent item.

Thus, if you delete a menu entry for a node that inherits it will become unprotected, too, because it can't inherit a password anymore. You would have to uncheck the inherit box before deleting the menu item or moving the node below an unprotected menu parent to keep the inherited password for the node.

If inheritance is disabled in the admin settings, all inheritance flags are set to "off" in the database, while the password is kept, so all nodes stay protected.

If we kept the values for the inheritance flags, things would get inconsistend if we changed some password or protection states and then turned inheritance on again. Then nodes would have their inheritance flag "on" while their parents could have different password. The only safe way is to turn off all flags when inheritance is disabled in the global settings.

Note that you can easily protect a whole subtree if you set the global default inherit flag to "inherit by default" and enter a passwort for the top node, see above. If you haven't saved anything for this tree yet (no password, no inherit flag) every child node will inherit the password, since the global default flag is on.
But if you try to protect a subtree where some nodes have "inherit off" stored in the db (because the default was off at some time, or the nodes had a password before), it would be a lot of work to consider and changed all the nodes in the tree.

Therefore I added a "Activate inheritance for menu descendants once." option to the node edit form. If checked, it will activate inheritance for all the nodes children and save the nodes password for them. Thus, you can enter a password for a node and check this option and whole subtree will immediately inherit this password, no matter if they had own password or didn't inherit before.
If you uncheck "Node is protected" (or inherit from a non-protected parent) and activate the menu descendants options, the whole subtree will become unprotected.

When viewing nodes, information about entered passwords are inherited, too. I.e. when you entered the password for a node, you can access all of its menu children that inherit the password without entering it again. This works only top-down, so if you try to view the the child of a protected first, you must enter the password once and then once again for the node itself.

I didn't implement bottom-up-inheritance for this because usually you cannot see the children of a protected node in the menu tree before entering the password for this node, so it shouldn't happen very often tyhat you view a protected child before the protected parent node.

A node can have two or more menu items pointing to it. To decide from which parent to inherit, I use the same algorithm that the menu module uses for determining which menu item to show in the node edit form. This is e.g. needed when determing the parent menu in nodeapi 'load'. We need to store parent information there so that we know the former parent when deleting a node (to pass the former parent password to the nodes former children, see nodeapi explanations).

Missing features:

When a node is moved in or removed from a menu on the admin/build/menu page, nodeapi doesn't realize this and so no inheritance is performed. This can of course lead to inconsistent states, e.g. nodes which are inheriting have a different password than their parent node.
I didn't try to hook into the menu editing page and I'm not sure if this can be done at all. If someone ever did sth. like that, let me know.

Comment #3

Frank Steiner CreditAttribution: Frank Steiner commented 12 September 2008 at 22:39

Hmm, no one willing to test this and give me some feedback? :'-(

Comment #4

mtolmacs CreditAttribution: mtolmacs commented 15 September 2008 at 20:00

Sorry but somehow I did not get any notification on this issue through my issue view so I completely missed that you posted a patch. I'll look into it as soon as possible. Thank you for your patience!

[If I don't reply in the future, just drop me a line through my contact page]

Comment #5

Frank Steiner CreditAttribution: Frank Steiner commented 16 September 2008 at 15:13

Allright, thanks :-)

Comment #6

mtolmacs CreditAttribution: mtolmacs commented 14 October 2008 at 10:05

Still on it, its just a huge patch :)

Comment #7

Frank Steiner CreditAttribution: Frank Steiner commented 14 October 2008 at 21:33

Yes, sorry, I know it must be a hassle to test this :-( Testing took me longer than writing :-)

Comment #8

Frank Steiner CreditAttribution: Frank Steiner commented 15 January 2009 at 06:52

Tolmi, can you let me know what your further plans are about this patch? I won't be able to rewrite this for every future release of the module for myself, so I will have to look for an alternative if there is no chance to get this patch included. I just need to know :-)

Thanks!

Comment #9

mtolmacs CreditAttribution: mtolmacs commented 20 January 2009 at 09:01

Currently I can't promise anything as bug reports are in the queue and I intend to fix those first. After I've done with them I get back to your patch. Sorry for delaying this for so long but unfortunately I also have things to attend to besides developing modules for free. ;)

Comment #10

Frank Steiner CreditAttribution: Frank Steiner commented 20 January 2009 at 14:32

Ok :-)

Comment #11

AlexisWilke CreditAttribution: AlexisWilke commented 15 April 2011 at 11:25

Issue tags:

+menu

Frank,

Darn! This is huge! 8-}

If I understand your change, you use the "parent" of a node based on the menu where the node is found. What if the node appears in multiple menus?

I suppose this means a whole book can be protected by adding a single password on the root of said book, which is a great feature to have...

If I find the time, I may look into it as that would interest me...

Thank you.
Alexis

Comment #12

Frank Steiner CreditAttribution: Frank Steiner commented 26 May 2011 at 10:10

As far as I remember $node->menu['plid']; is uniqe and that's what the patch works on. If you create another menu entry for the same node, inheritance won't work on this menu branch. I wasn't caring about this as in our environment it is not possible to create menu entries other than from the node edit page...

Considering more than the menu linked to from the node won't work anyway. Guess you put the node into two menus and its parents have different settings, e.g. both have inheritance set, one with protection enabled, the other without.

In short: Inheritance can only work in a tree not in a graph.

Comment #13

AlexisWilke CreditAttribution: AlexisWilke commented 27 May 2011 at 09:10

Frank,

The loading of the menu item is done using the following snippet of code from module/menu/menu.module (around line 325).

To my point of view, there is a bit of magic in there. If you create nodes organized in a tree and add the root node to a menu other than the "default menu" (so called in the code and happens to be "primary-links" by default.) Later add a link the root node of your tree to the "default menu" and bing! you lose connections with the tree (at least in regard to that $node->menu['plid'] you mentioned.)

        // Prepare the node for the edit form so that $node->menu always exists.
        $menu_name = variable_get('menu_default_node_menu', 'primary-links');
        $item = array();
        if (isset($node->nid)) {
          // Give priority to the default menu
          $mlid = db_result(db_query_range("SELECT mlid FROM {menu_links} WHERE link_path = 'node/%d' AND menu_name = '%s' AND module = 'menu' ORDER BY mlid ASC", $node->nid, $menu_name, 0, 1));
          // Check all menus if a link does not exist in the default menu.
          if (!$mlid) {
            $mlid = db_result(db_query_range("SELECT mlid FROM {menu_links} WHERE link_path = 'node/%d' AND module = 'menu' ORDER BY mlid ASC", $node->nid, 0, 1));
          }
          if ($mlid) {
            $item = menu_link_load($mlid);
          }
        }

Anyway, that can be marked as a side effect or weakness in the implementation and there is no work around. On the other hand, I guess that as long as you only put the root in the "default menu," then you're safe. The problem would be for another node to appear there because then it would break the track of the "regular" protected tree.

Thank you.
Alexis