Note on Security

The Drupal Security Team was made aware of this issue and after evaluating, determined this is not a security issue.

Problem/Motivation

With Drupal 9.2 the following patch/issue was resolved:
Make Book navigation translatable

Like I also mentioned in this issue (#241), there were patches with a request to address an issue of unexpected behavior: unpublished nodes get into the export of the book if the current user can access them

Steps to reproduce

  1. Create a book content with workflow activated
  2. Publish some nodes and subnodes
  3. Put a node in an unpublished state (e.g. archived)
  4. use the print preview / html export as a user who can access the archived node

Proposed resolution

Unpublished nodes should not be part of the html export

Remaining tasks

Create patch

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork book-3228274

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

dbielke1986 created an issue. See original summary.

dbielke1986’s picture

StatusFileSize
new125.85 KB

Could this be the root of the problem?

dbielke1986’s picture

StatusFileSize
new139.92 KB

Because the exportTraverse assumes that the access check is already done when the tree is built.

But I think that these are two different things. An export should only output the published content (it does that on the node itself, even if there are newer versions/designs here).

cilefen’s picture

Title: Unpublised bookpages gets into the html export » Unpublished book pages get into the html export
Issue tags: -book module translatable
dbielke1986’s picture

StatusFileSize
new68 bytes

This is resolving the issue:

dbielke1986’s picture

dbielke1986’s picture

StatusFileSize
new174.99 KB
joel_osc’s picture

Looks like a reasonable solution to a very serious problem.

smulvih2’s picture

Will test this solution shortly today, thanks!

smulvih2’s picture

Status: Active » Needs review
StatusFileSize
new939 bytes

@dbielke1986 your fixed worked for me. Please find attached the corresponding patched.

smulvih2’s picture

StatusFileSize
new959 bytes

New patch with proper file location.

dbielke1986’s picture

Status: Needs review » Needs work

sorry, wrong comment...

dbielke1986’s picture

Status: Needs work » Needs review
avpaderno’s picture

I am deleting a file content. I apologize for bumping this issue.

cilefen’s picture

Issue summary: View changes
cilefen’s picture

This is likely going to be Closed (works as designed).

smulvih2’s picture

Title: Unpublished book pages get into the html export » Unpublished book pages in html export - add config to restrict to published nodes
Category: Bug report » Feature request
Status: Needs review » Needs work

After reviewing this in more detail I don't think this is an issue. It seems the export function does take node access into consideration. I tested this by trying to export a book as an anonymous user, where unpublished nodes were not exported (expected). If I try exporting the same book as an authenticated user I get unpublished book pages in my export which I have access to.

I think the issue here is more of a feature request. Would be nice to control whether unpublished nodes get exported or not, maybe per book, or maybe globally. I could see a use case where authenticated users need to only export published nodes, and another case where draft content is important to export. For example, I'm working on a user guide (book) and need to export only the published revisions to distribute it, OR, I need to export the drafts for the user guide so I can print and work on the drafts offline.

Maybe we need two export buttons and two permissions; export draft VS export published. This way I could give [user_role_1] the ability to export published nodes, where [user_role_2] could export drafts.

pwolanin’s picture

Issue summary: View changes

updated the issue description

pwolanin’s picture

Issue summary: View changes

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.1.10 (June 4, 2021) and Drupal 9.2.10 (November 24, 2021) were the last bugfix releases of those minor version series. Drupal 9 bug reports should be targeted for the 9.3.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

avpaderno’s picture

Version: 9.3.x-dev » 9.4.x-dev

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

honzapara’s picture

Status: Needs work » Reviewed & tested by the community

I can confirm that this is still a problem. I did the test with the anonymous and the authenticated user as smulvih2 suggests in comment #17. I can still see the unpublished nodes in the export when seeing the export as the anonymous user.
I tried to apply the patch from comment #11 and this one seems to be working well for me. However, I don't see unpublished nodes in the export as the authenticated user, either. I don't personally mind that as the export should be very similar to what I can see directly on the actual node.
This has been tested on Drupal core 9.5.3.

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

The issue summary and title are saying this should have a configuration option, I'm not sure we should - if you're exporting, you probably want to actually use that export and then having unpublished nodes in it could make it confusing. However that needs to be sorted out before this is RTBC and this issue could also use test coverage.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

Status: Needs work » Postponed

This extension is being deprecated, see #3376070: [Meta] Tasks to deprecate Book module. It will be removed from core and moved to a contrib project, #3376101: [11.x] [Meta] Tasks to remove Book.

This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

This issue may be re-opened if it can be considered critical, If unsure, re-open the issue and ask in a comment.

smustgrave’s picture

Project: Drupal core » Book
Version: 11.x-dev » 1.0.x-dev
Component: book.module » Code
Status: Postponed » Needs work
smustgrave’s picture

Version: 1.0.x-dev » 2.0.x-dev

mdranove made their first commit to this issue’s fork.

smustgrave’s picture

Status: Needs work » Fixed
Issue tags: -Needs tests

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.