Printer Friendly Version of Book Does Not Take Into Account Node Access

Can we straightforwardly port the security fix and then work on cleaning it up with a lesser priority?

Yeah, in this case the code debt made what should have been an obvious patch (add a new access callback) into a more complicated issue for the security team, which is low on resources for core issues as it is... so it would be great to fix it here and prevent that problem in the future.

I would say in this issue we should at least switch to book/export/%/%node (arguably even book/export/html/%node, since contrib modules can easily define their own menu entries for alternative export methods if they want to; but that part might be out of scope).

Untested patch, but let's let the testbot test it...

Above patch only switches to book/export/html/%node by the way.

#6: book-printer-friendly-1890748-6.patch queued for re-testing.

+++ b/core/modules/book/book.moduleundefined
@@ -196,6 +197,16 @@ function book_menu() {
+ * @param Drupal\node\Node $node

Should be @param \Drupal\... :)

+++ b/core/modules/book/book.moduleundefined
@@ -196,6 +197,16 @@ function book_menu() {
+function _book_export_access($node) {
+  return user_access('access printer-friendly version') && node_access('view', $node);

Why mark this is a private function?

Hm, well in both cases I just copied the way Book module was already doing it :)

Fixed Berdir's comments and added proper type hinting.

I would want to replace node_access() with $node->access(), but I guess the node access controller is not ready yet.

Looks good, has tests and fixes the bug.

The callback there is a bit tricky, as that seems to be pluggable, so someone could provide book/export/pdf/123. Might require a change notice?

Awesome work, folks.

Committed and pushed to 8.x. I think the tests should probably be backported to 7.x yes?

The test case for Drupal 7 has already been committed with the security fix.

Yup, comparing the Drupal 7 to Drupal 8 commits for these tests I don't see any differences.

Restoring original issue priority also.