Non-superusers can permanently delete pages from the trash

[adrianbj]

Short description of the issue

If a user with page delete permissions visits a page in the trash they will see the "Delete Permanently" option.

Expected behavior

Only superusers should be able to permanently delete pages

Actual behavior

Anyone with page delete permissions can permanently delete pages from the trash.

Optional: Suggestion for a possible fix

Simple fix would be to prevent non superusers from seeing the Delete tab when a page is in the trash. I think perhaps a better solution would be separate page-trash and page-delete permissions, but that sounds like a breaking change.

[ryancramerdesign]

@adrianbj The page-delete permission does allow deleting of pages, though only pages they have page-delete permission assigned to. The user would also need access to the trash in order to get there. Would the page-edit-trash-created permission be better for your use case?

[adrianbj]

@ryancramerdesign - I don't think the page-edit-trash-created helps either. Basically I don't want any non-superusers to be able to delete anything permanently.

The problem is that even though non-superusers can't see the trash branch, if they have a direct link to edit a page in the trash they can access it and use the delete permanently option.

Perhaps my only solution is to hook into ProcessPageEdit::buildForm and remove the delete tab and Pages::delete just to make sure.

[adrianbj]

Hi @ryancramerdesign - could you please consider this again. I added the following hook the prevent the Delete tab from being visible:

$this->addHookAfter('ProcessPageEdit::buildForm', function($event) {

    $p = $event->object->getPage();

    if($p->isTrash()){
        $form = $event->return;

        $fieldset = $form->find("id=ProcessPageEditDelete")->first();
        $form->remove($fieldset);
        $event->object->removeTab("ProcessPageEditDelete");

        $event->return = $form;
    }

});

which does everything I thought was needed but I was still seeing a site editor actually deleting pages. Knowing the person who was doing it, I suspected they might be a rampant double-clicker, and lo and behold, I discovered that a non-superuser can permanently delete a page by double clicking the "Move to Trash" button on the Delete tab. I can reproduce this every time and I think it's something that really needs to be fixed in the core.

Thanks!

[ryancramerdesign]

@adrianbj I can't duplicate the "double-click trash to permanently delete page" that you mentioned. I also don't see how it could be possible without something added on to it from a hook or module. It doesn't look like your hook does that, though since it removes the "delete" tab, how is there a "Move to Trash" button for them to click on? I think I need more instructions on how to reproduce the issue.

Regarding editing pages in the trash, this should be possible if the user has page-delete permission or at least page-edit-trash-created permission. Though it's not intended that they can delete the page permanently unless they have page-delete permission. If I understand what you are saying, the user doesn't have a page-delete permission, but they can still permanently delete a page?

[adrianbj]

@ryancramerdesign - the Move to Trash button is on the delete tab while the page is still in the main page tree. I want users to be able to trash pages but not delete them.

I am not sure why the double-click on that button is causing them to be deleted for me and not you, but here is a screencast

https://github.com/user-attachments/assets/1c1d8d6a-47ac-4624-b165-4df517465262

Obviously you can't that I am double-clicking but the message at the top clearly shows that the page was moved to the trash and then deleted from it.

If I understand correctly, a user needs the page-delete permission to be able to trash a page, but that permission also allows them to permanently delete it if they have the URL to the page in the trash, or if they double-click when initially moving to the trash.

I have been playing around with the hook options a little more and it seems this does what I need:

$this->wire()->addHookAfter('Page::deleteable', function($event) {
    $event->return = false;
});

This removes the need for the ProcessPageEdit::buildForm to remove the delete tab on pages in the trash because it takes care of this already, but more importantly it prevents the double-click delete issue as well so it should mean there is now no way to ever accidentally fully delete a page.

Yes, I could limit this to non-superusers, but in reality I don't want anyone being able to fully delete.

[adrianbj]

PS - here is the version where I don't double-click - notice it just goes to the trash, but not fully deleted.

https://github.com/user-attachments/assets/59ace516-6962-4881-874f-d1a5d55df066

[ryancramerdesign]

@adrianbj After watching the video I tried to duplicate it again, but am not able to. I am wondering if that double click behavior might be related to the admin theme you are using or some module that's installed? Can you try with the Uikit admin theme included with the core, or maybe a clean installation?

[adrianbj]

@ryancramerdesign - I think the double-click issue is somehow related to https://github.com/baumrock/RockAdminTweaks/tree/main/tweaks/General/BypassTrash

The weird thing is that the tweak wasn't enabled when I was seeing this behavior (although it was before), but now after re-enabling and disabling again, I can no longer reproduce it. It should be limited to superusers only and there is no double-click feature anyway - maybe somehow double-clicking enables the delete checkbox (although it wasn't visible for non-superusers anyway). I really don't know at the moment, but I think it is unlikely a core bug.

That said, I still really believe there needs to be separation between trashing and deleting permissions because a non-superuser can still fully delete a page even if they don't have access to the trash. Surely I can't be the only person who wants users to be able to trash but not delete?