diff --git a/wiki/src/org/labkey/wiki/WikiController.java b/wiki/src/org/labkey/wiki/WikiController.java index 91cd8a0319d..c254ea7675f 100644 --- a/wiki/src/org/labkey/wiki/WikiController.java +++ b/wiki/src/org/labkey/wiki/WikiController.java @@ -392,7 +392,8 @@ public ModelAndView getConfirmView(WikiNameForm form, BindException errors) if (!perms.allowDelete(_wiki)) throw new UnauthorizedException("You do not have permissions to delete this wiki page"); - return new JspView<>("/org/labkey/wiki/view/wikiDelete.jsp", _wiki); + // GH Issue 1468: warn up front, rather than failing on submit, when the subtree can't be deleted + return new JspView<>("/org/labkey/wiki/view/wikiDelete.jsp", new WikiDeleteBean(_wiki, findUndeletableDescendant(perms, _wiki))); } @Override @@ -407,10 +408,23 @@ public boolean handlePost(WikiNameForm form, BindException errors) throws Except if (!perms.allowDelete(_wiki)) throw new UnauthorizedException("You do not have permissions to delete this wiki page"); + // GH Issue 1468: permission on the root page (e.g., as its creator) doesn't extend to descendants created by + // others. Check the whole subtree up front so the delete is all-or-nothing, then check each page again just + // before it's deleted to catch any child added concurrently. + if (form.getIsDeletingSubtree()) + { + Wiki blocked = findUndeletableDescendant(perms, _wiki); + if (null != blocked) + throw undeletableDescendantException(blocked); + } + try { //delete page and all versions - getWikiManager().deleteWiki(getUser(), c, _wiki, form.getIsDeletingSubtree()); + getWikiManager().deleteWiki(getUser(), c, _wiki, form.getIsDeletingSubtree(), descendant -> { + if (!perms.allowDelete(descendant)) + throw undeletableDescendantException(descendant); + }); } catch (OptimisticConflictException e) { @@ -419,6 +433,27 @@ public boolean handlePost(WikiNameForm form, BindException errors) throws Except return true; } + // Returns the first descendant (depth-first) the user cannot delete, or null if all are deletable. + // A single blocked page prevents the subtree delete, so there's no need to collect every blocked page. + private @Nullable Wiki findUndeletableDescendant(BaseWikiPermissions perms, Wiki wiki) + { + for (Wiki child : wiki.children()) + { + if (!perms.allowDelete(child)) + return child; + + Wiki blocked = findUndeletableDescendant(perms, child); + if (null != blocked) + return blocked; + } + return null; + } + + private UnauthorizedException undeletableDescendantException(Wiki descendant) + { + return new UnauthorizedException("You do not have permissions to delete the child wiki page '" + descendant.getName() + "', so this wiki subtree can't be deleted"); + } + @Override public void validateCommand(WikiNameForm wikiNameForm, Errors errors) { @@ -443,6 +478,8 @@ public ActionURL getFailURL(WikiNameForm wikiNameForm, BindException errors) } } + public record WikiDeleteBean(Wiki wiki, @Nullable Wiki undeletableDescendant){} + public enum NextAction { page(PageAction.class), manage(ManageAction.class), edit(EditWikiAction.class); diff --git a/wiki/src/org/labkey/wiki/WikiManager.java b/wiki/src/org/labkey/wiki/WikiManager.java index dae042c2411..bccca39a0ee 100644 --- a/wiki/src/org/labkey/wiki/WikiManager.java +++ b/wiki/src/org/labkey/wiki/WikiManager.java @@ -95,6 +95,7 @@ import java.util.Objects; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import static org.labkey.api.action.SpringActionController.ERROR_MSG; import static org.labkey.api.security.WikiTermsOfUseProvider.TERMS_OF_USE_WIKI_NAME; @@ -373,9 +374,19 @@ public void replaceAliases(Wiki wiki, Collection newAliases, @Nullable B } public void deleteWiki(User user, Container c, Wiki wiki, boolean isDeletingSubtree) throws SQLException + { + deleteWiki(user, c, wiki, isDeletingSubtree, null); + } + + /** + * @param descendantCheck when deleting a subtree, invoked on each descendant immediately before it's deleted; it + * should throw to prevent the deletion. Pages deleted before the throw stay deleted, but a + * page is never deleted before its descendants, so the remaining tree is never orphaned. + */ + public void deleteWiki(User user, Container c, Wiki wiki, boolean isDeletingSubtree, @Nullable Consumer descendantCheck) throws SQLException { //shift children to new parent, or delete recursively if deleting the whole subtree - handleChildren(user, c, wiki, isDeletingSubtree); + handleChildren(user, c, wiki, isDeletingSubtree, descendantCheck); DbScope scope = comm.getSchema().getScope(); @@ -405,7 +416,7 @@ public void deleteWiki(User user, Container c, Wiki wiki, boolean isDeletingSubt } - private void handleChildren(User user, Container c, Wiki wiki, boolean isDeletingSubtree) throws SQLException + private void handleChildren(User user, Container c, Wiki wiki, boolean isDeletingSubtree, @Nullable Consumer descendantCheck) throws SQLException { //shift any children upward so they are not orphaned @@ -417,7 +428,11 @@ private void handleChildren(User user, Container c, Wiki wiki, boolean isDeletin if(isDeletingSubtree) { for(Wiki childWiki : children) - deleteWiki(user, c, childWiki, true); + { + if (null != descendantCheck) + descendantCheck.accept(childWiki); + deleteWiki(user, c, childWiki, true, descendantCheck); + } } else { diff --git a/wiki/src/org/labkey/wiki/view/wikiDelete.jsp b/wiki/src/org/labkey/wiki/view/wikiDelete.jsp index 373357f7bd7..68e4ea7a290 100644 --- a/wiki/src/org/labkey/wiki/view/wikiDelete.jsp +++ b/wiki/src/org/labkey/wiki/view/wikiDelete.jsp @@ -18,15 +18,21 @@ <%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> <%@ page import="org.labkey.api.view.HttpView" %> <%@ page import="org.labkey.api.view.JspView" %> +<%@ page import="org.labkey.wiki.WikiController.WikiDeleteBean" %> <%@ page import="org.labkey.wiki.model.Wiki" %> <%@ page extends="org.labkey.api.jsp.JspBase" %> <% - JspView me = HttpView.currentView(); - Wiki wiki = me.getModelBean(); + JspView me = HttpView.currentView(); + WikiDeleteBean bean = me.getModelBean(); + Wiki wiki = bean.wiki(); + Wiki undeletableDescendant = bean.undeletableDescendant(); %> Are you sure you want to delete this page?

name: <%=h(wiki.getName())%>
title: <%=h(wiki.getLatestVersion().getTitle())%>
-
Delete Entire Wiki Subtree +
Delete Entire Wiki Subtree +<% if (null != undeletableDescendant) { %> +
You can't delete the entire subtree because you don't have permission to delete the child page '<%=h(undeletableDescendant.getName())%>'. +<% } %>