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
- Create a book content with workflow activated
- Publish some nodes and subnodes
- Put a node in an unpublished state (e.g. archived)
- 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
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | remove-unpublished-book-pages-from-export-3228274-11.patch | 959 bytes | smulvih2 |
| #7 | solution.png | 174.99 KB | dbielke1986 |
| #3 | BookExport.png | 139.92 KB | dbielke1986 |
| #2 | Bug.png | 125.85 KB | dbielke1986 |
Issue fork book-3228274
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
Comment #2
dbielke1986 commentedCould this be the root of the problem?
Comment #3
dbielke1986 commentedBecause 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).
Comment #4
cilefen commentedComment #5
dbielke1986 commentedThis is resolving the issue:
Comment #6
dbielke1986 commentedComment #7
dbielke1986 commentedComment #8
joel_osc commentedLooks like a reasonable solution to a very serious problem.
Comment #9
smulvih2Will test this solution shortly today, thanks!
Comment #10
smulvih2@dbielke1986 your fixed worked for me. Please find attached the corresponding patched.
Comment #11
smulvih2New patch with proper file location.
Comment #12
dbielke1986 commentedsorry, wrong comment...
Comment #13
dbielke1986 commentedComment #14
avpadernoI am deleting a file content. I apologize for bumping this issue.
Comment #15
cilefen commentedComment #16
cilefen commentedThis is likely going to be Closed (works as designed).
Comment #17
smulvih2After 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.
Comment #18
pwolanin commentedupdated the issue description
Comment #19
pwolanin commentedComment #21
avpadernoComment #24
honzapara commentedI 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.
Comment #25
catchThe 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.
Comment #27
quietone commentedThis 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.
Comment #28
smustgrave commentedComment #29
smustgrave commentedComment #32
smustgrave commented