Clients do not have sufficient permissions to create patients

[DSquizzle]

Steps to reproduce

Using Senaite.Core 2.0.0rc2 and Senaite.LIMS 2.0.0rc2 with the most recent commit on the Senaite.Health 2.x branch, perform the following steps:

• In the Site Setup, create a new user "Client1"
• Create a new external Client "Clinic1"
• Under the new "Clinic1" Client, create a new Contact "Contact 1"
• Link the Contact "Contact 1" with the user "Client1" (This step seems to fail using Plone 5 as users no longer have the getTool method. You can force the step to work by commenting out lines 48 and 53 in the file senaite.health/src/bika/health/monkeys/contact.py
• Log in as user "Client1" and create a new patient "Patient 1." You will be able to view and fill out the form, but when you click the "Save" button, you will be redirected to the "Insufficient Privileges" page, and the patient will not be created.

Current behavior

By setting

verbose-security = on

in your buildout.cfg and removing Unauthorized from the list of ignored exceptions inside the error_log within the site root folder, then running the instance in the foreground with bin/instance fg, you can see that patient creation by clients fails because clients do not have the Delete Objects permission in the portal.patients folder.

More specifically, when bika.health.utils.move_obj calls destination.manage_pasteObjects(cp) (src/bika/health/utils.py, line 240), the function implicitly calls Products.CMFCore.PortalFolder._verifyObjectPaste, which raises AccessControl_Unauthorized at lines 389-390, after checking that the Client role does not have the DeleteObjects role within the portal.patients folder.

Expected behavior

The patient is created under the client that employs the contact who is creating the patient.

Possible Solution?

After noting that this issue does not manifest in the 1.x branch, it appears that the issue was introduced in the commit 00554370, in the file src/bika/health/utils.py, lines 233-250. Looking at the docstring in the version of the file that existed before this commit, it seems that the previous version intentionally bypassed the permission check, so that clients did not need the Delete Objects permission. In the new version, however, the permission is needed. Here is the docstring from the old code:

    """
    This function has the same effect as:

        id = obj.getId()
        cp = origin.manage_cutObjects(id)
        destination.manage_pasteObjects(cp)

    but with slightly better performance and **without permission checks**. The
    code is mostly grabbed from OFS.CopySupport.CopyContainer_pasteObjects
    """

The new version replaces most of the logic in the function with the three lines mentioned in the docstring. As I do not know why the function was changed in the 00554370 commit, I cannot say whether the older version is better, but I can say that my particular issue does not exist when the function is replaced by the older version.