Editing a top level book page inadvertently changes parent

I dont know what "mitigated" means, but this bug is a dupe of the abovementioned.

hmm. My fault. Some silly tabs thing. I edited the wrong issue. Meant to reply to another book issue. NMy fault, resetting status.

Has this bug been addressed? I am using 4.5.2 which still allows a user without node administration rights to change a top level book, thus inadvertently changing the parent.

Would it be better to have a permission to allow editing of a top-level book? I would like to have a book whose parent can only be altered by the owner, but that has content that can be modified by those with permission.

I have the problem with 4.6.3. Is there a formal fix to this issue?

Didn't this recently get fixed?

Escalating this to critical

I am supplying a patch which provides one possible fix for this issue.

• If a user does not have 'create new books' permission, then when he goes to edit a book page, the 'parent' selector will contain only "" and subpages of the current book. This prevents him from creating top level pages, allows him to edit any page within a book, and to move pages within a book, but does not allow him to move pages from one book to another.
• To support this fix, I have arranged for book_toc() to take an optional additional parameter which specifies the book node at which the TOC hierarchy will be rooted.
• I have cached the results of the (somewhat expensive) database query of book_toc() in a static variable; book_toc() is called at least twice per page if the book navigation block is enabled.
• I have removed the function _book_get_depth(). Its result can be computed using count(book_location()) + 1);
• book_location() now accepts numeric nids as parameters, as well as book node objects.

Note: Another quite different approach would restrict users without 'create new books' permissions from moving top level pages. In this approach, any page of depth > 1 could be moved freely among existing books. I've tried this (there's less code involved) but it doesn't feel right. I'm open to being convinced otherwise, though.

Sigh, HTML encoding...

The description above should read

If a user does not have 'create new books' permission, then when he goes to edit a book page, the 'parent' selector will contain only "<current-book>" and subpages of the current book. This prevents him from creating top level pages, allows him to edit any page within a book, and to move pages within a book, but does not allow him to move pages from one book to another.

OK, I could not resist adding some function documentation.

This is ugly:

 function book_location($node, $nodes = array()) {
+  if (is_numeric($node)) { 
+      $node = node_load($node); 
+  }

You're right... here's a new patch.

I've removed the is_numeric() hack and set up to call book_location() with the arguments it expects.

I've also removed the field n.status from the query in book_admin_orphan() since it is not subsequently used anywhere.

I set up a user who's role does not have 'create new books' permission, only 'edit book pages', 'create book pages' and 'edit own book pages'.

The user can now see the "< top-level >" option when editing an existing book page, but there are no other items in the menu!

So patch failed.

ignore the above, clicked 'submit' rather than 'preview'.

My user does not have 'create new books' permission.

When they go to ?q=node/add/book there is no way to select what book the page should add to. The 'parent' menu just contains <top-level>. How should a book page author attach a page to an existing book?

If the user navigates to an existing top-level book page, and clicks 'add child page' link, the 'parent' menu lists only <top-level>, rather than <current-book>, or something similar.

Submitting the new child page assigns the book page to no parent, i.e. its made a new book of its own - should it not have become an orphan book page instead? Or better still -- actually associated the new child page with the book page to was added to.

The user didn't have 'create new books' permission yet has managed to create a new book (as seen from the admin side at ?q=admin/node/book)

Yup, I messed up - I didn't consider that the user would be adding pages as well as just editing.

Here's a fix.

I've changed the behaviour to be more uniform, if a little less satisfying: If a user does not have 'create new books' permission, when he goes to add a new page page, or edit an existing page, <top-level> does not appear as an option in the list of parents. This means that the user can create a new page in any existing book, or move pages among existing books, but not create a new book.

Moshe, the problem which you initially described still exists for a user without the 'create new books' permission.

I tested the patch. There was some conflitcs with rev 1.352 (patch was done with rev1.350). So, i made a new patch based on 1.352 than i am attaching.

The resulting patch doesn't seem to work either: a user (without create new book permissions) who edit a "top level" page will put the page as a child to another book (since "top level) will not appear).

I'll spend a little bit more time on this now.

Please don't change formatting only. This makes it hard to review a patch.

To sum up: if someone has 'edit book pages' permission but not 'create new' then we have a problem. On it.

Let this patch commemorate my dear grandmother who passed away four hours ago.

Gerhard is right that book_outline could use some cleaning as parent weight are already added by book_load and log is moved to node_revisions.

I went a bit far...

I'm not sure if this is the right patch. Unless I'm mistaken, an unpriviledged user could move away a top-level item down the tree, but then be disallowed to move it back.

Define "unpriviledged". I think this is the correct patch. I added back into editing when you can edit but not when adding when you can't add.

applied.

Works for me too.

Follow up:
http://drupal.org/node/53956