Check delete permission on every page in a wiki subtree delete - #8105
Open
labkey-bpatel wants to merge 3 commits into
Open
labkey-bpatel wants to merge 3 commits into
labkey-bpatel wants to merge 3 commits into
Conversation
labkey-jeckels
approved these changes
Sep 30, 2026
| <br/><labkey:checkbox id="isDeletingSubtree" name="isDeletingSubtree" value="true" checked="false"/> Delete Entire Wiki Subtree | ||
| <br/><labkey:checkbox id="isDeletingSubtree" name="isDeletingSubtree" value="true" checked="false" disabled="<%=null != undeletableDescendant%>"/> Delete Entire Wiki Subtree | ||
| <% if (null != undeletableDescendant) { %> | ||
| <br/><span class="labkey-error">You can't delete the entire subtree because you don't have permission to delete the child page '<%=h(undeletableDescendant.getName())%>'.</span> |
Contributor
There was a problem hiding this comment.
I think this is fine to just report the first child in practice, but there could be multiple children that you don't have permission to delete.
Contributor
Author
There was a problem hiding this comment.
Yup, correct. We stop at the first one since that’s enough to block the delete. I added a comment to make that clearer.
Contributor
|
Looks like the new test isn't passing yet. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale
Deleting a wiki page with "Delete Entire Wiki Subtree" only checked delete permission on the root page. Because the creator of a page gets a contextual Owner role on it, an Author (Read + Insert, no Delete) could delete their own page's entire subtree, permanently removing child pages created by other users along with their version history and attachments. This change requires delete permission on every page in the subtree, so a subtree delete is allowed only when the user could delete each of those pages individually. Editors and Admins are unaffected, and a delete without the subtree option still moves children up a level. GH Issue 1468
Related Pull Requests
Changes
DeleteAction.handlePostwalks the whole subtree before deleting anything and rejects the request with a 403 naming the first page the user can't delete, so the delete is all-or-nothing.WikiManager.deleteWikihas a new overload that takes a per-page check, run on each descendant just before it's deleted, to catch a child added after the upfront check. The existing overload passes no check, soWikiService.deleteWikiand other callers are unchanged.