Commit 44d65ce9 authored by Jérome Perrin's avatar Jérome Perrin

stack/erp5: backport a Products.BTreeFolder2 fix for "Delete All Objects"

parent 57324798
From cfdd177131b8f453c024ad3008da4951aab16a50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <jerome@nexedi.com>
Date: Tue, 2 May 2023 13:53:46 +0900
Subject: [PATCH] Add a confirmation prompt on ``Delete All Objects`` button
(#19)
Co-authored-by: Michael Howitz <icemac@gmx.net>
patch amended to remove the changelog entry
---
src/Products/BTreeFolder2/contents.dtml | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/Products/BTreeFolder2/contents.dtml b/src/Products/BTreeFolder2/contents.dtml
index d00e04a..9629700 100644
--- a/src/Products/BTreeFolder2/contents.dtml
+++ b/src/Products/BTreeFolder2/contents.dtml
@@ -82,7 +82,7 @@ function toggleSelect() {
<input class="form-element" type="submit" name="manage_delObjects:method"
value="Delete" />
<input class="form-element" type="submit" name="manage_delAllObjects:method"
- value="Delete All Objects" />
+ value="Delete All Objects" onClick="return confirm('Really delete all objects?')"/>
</dtml-if>
<dtml-if "_.SecurityCheckPermission('Import/Export objects', this())">
<input class="form-element" type="submit"
--
2.39.1
......@@ -690,10 +690,12 @@ extra-paths =
patch-binary = ${patch:location}/bin/patch
Acquisition-patches = ${:_profile_base_location_}/../../component/egg-patch/Acquisition/aq_dynamic-4.7.patch#85b0090e216cead0fc86c5c274450d96
Acquisition-patch-options = -p1
Products.DCWorkflow-patches = ${:_profile_base_location_}/../../component/egg-patch/Products.DCWorkflow/workflow_method-2.4.1.patch#ec7bb56a9f1d37fcbf960cd1e96e6e6d
Products.DCWorkflow-patch-options = -p1
Products.BTreeFolder2-patches = ${:_profile_base_location_}/../../component/egg-patch/Products.BTreeFolder2/0001-Add-a-confirmation-prompt-on-Delete-All-Objects-butt.patch#44de3abf382e287b8766c2f29ec1cf74
Products.BTreeFolder2-patch-options = -p1
Products.CMFCore-patches = ${:_profile_base_location_}/../../component/egg-patch/Products.CMFCore/portal_skins_ZMI_find.patch#19ec05c0477c50927ee1df6eb75d1e7f
Products.CMFCore-patch-options = -p1
Products.DCWorkflow-patches = ${:_profile_base_location_}/../../component/egg-patch/Products.DCWorkflow/workflow_method-2.4.1.patch#ec7bb56a9f1d37fcbf960cd1e96e6e6d
Products.DCWorkflow-patch-options = -p1
PyPDF2-patches = ${:_profile_base_location_}/../../component/egg-patch/PyPDF2/0001-Custom-implementation-of-warnings.formatwarning-remo.patch#d25bb0f5dde7f3337a0a50c2f986f5c8
PyPDF2-patch-options = -p1
python-magic-patches = ${:_profile_base_location_}/../../component/egg-patch/python_magic/magic.patch#de0839bffac17801e39b60873a6c2068
......@@ -801,6 +803,7 @@ parso = 0.5.1
Pillow = 6.2.2
polib = 1.0.8
pprofile = 2.0.4
Products.BTreeFolder2 = 4.4+SlapOSPatched001
Products.CMFCore = 2.7.0+SlapOSPatched001
Products.ExternalMethod = 4.7
Products.GenericSetup = 2.3.0
......
  • continuing discussion from 247de005 ( the old commit before rebase and push to master )

    Jérome Perrin @jerome · 9 hours ago

    @yusei I had backported this patch already, but I could not find the time to test it and push it

    Yusei Tahara @yusei · 52 minutes ago

    I did not notice that we had patches in slapos. I am really worry about this functionality, it is too dangerous. What about to override manage_delAllObjects and completely disable this function. At least I will make such a patch for all my projects.

    Yes, this was a dangerous feature and not so useful. I sometimes want to remove all documents in modules, so I don't feel it is 100% useless, but not really useful. Now that there is a confirmation I was thinking this reduces the risk of accidentally clicking and deleting all, maybe it's enough ( but if we remove the button it's also OK for me )

  • In my experience, I saw some developers who was unbelievably careless and made serious mistakes. I might see such a developer again in future and our customer might lose their important data. If there is no self-destruct function, then even careless developer can't destroy ERP5. So I really would like to disable manage_delAllObjects to save our future.

  • So, I will make a hot-fix to disable manage_delAllObjects in ERP5 repository.

  • In a sense it's similar to "rm" command which deletes files and can delete important files, but this would be the equivalent of a desktop shortcut for "rm -rf /" :)

    There's also a security issue with this, because manage_delAllObjects does not check the request (some sensitive ZMI methods now check that REQUEST is POST method), so an attacker could try to trick admin into clicking a malicious link, or viewing a malicious image, that would delete a module content. If we are going to patch more, the safest seems to make manage_delAllObjects do nothing.

  • There's also a security issue with this

    Ah that is a good point. Such a function is OK when user stores less important data in BTreeFolder2. However, it is not suitable for us because ERP5 is a mission-critical application.

Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment