Patron merge ignores group application permissions

[Bill Erickson]

Evergreen 3.0.beta / all versions

Staff with the MERGE_USERS permission can modify and delete accounts via merging even when the lack of a group application permission prevents them from modifying or deleting the same accounts in the patron editor.

Steps to reproduce (in Concerto).

1. Grant the MERGE_USERS permission to the Circulators group.
2. Log in to the browser client with a Circulator login (e.g. br1iwalton)
3. Perform a patron search that returns staff accounts.
4. Confirm attempts to edit a staff account in the patron editor result in a permission error.
5. Back in the patron search interface, attempt to merge the selected staff account with any other account.
6. Confirm the merge succeeds without permission error.

The merge should have been prevented since the Circulator login does not have permission to modify staff accounts.

Since the MERGE_USERS permission is required, I don't see this as a critical security bug. But the behavior is unexpected. I believe there is a use case for not treating the MERGE_USERS permission as a magic override for group application permissions.

[Michele Morgan (mmorgan)]

As a use case, in our consortium of public and academic institutions, academic patron records are maintained by the institution, public library staff do not have permission to edit or delete them. We use the group application permissions to enforce this. The MERGE_USERS permission should definitely not override the group application permissions.

Another twist on this bug. A logged in staff user is not able to edit their own account, but they are not prevented from deleting their own staff account (even while logged in) by merging it with another account.

We recently had a situation where a logged in staff user merged their own record with a public patron, deleting their own account and attributing all the staff assets to the public patron.

So in addition to the merge function respecting the group application permissions, the logged in user should be prevented from using the merge function on their own account.

[Bill Erickson (berick)]

Code pushed to 1) verify permission to edit all users in involved in the merge and 2) prevent staff from merging their own logged in accounts.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1718032-patron-merge-profile-perms

[Michele Morgan (mmorgan)]

This fix works for me. Tested as follows:

1. Logged in as a staff user, I attempted to merge a patron for which I have group edit permission with another patron for which I do not have group edit permission. The merge did not succeed, I was prompted to enter a username and password to override. Entering a staff login with the appropriate permission successfully merged the patrons.

2. Logged in as a staff user, I attempted to merge my staff record with another record I have permission to edit. The merge failed with an alert in the lower right hand corner which read: "Logged in account cannot be merged"

My signoff branch is at:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mmorgan/lp1718032-patron-merge-honors-group-perms-signoff

Thanks Bill!

[Bill Erickson (berick)]

Thanks Michele! Patch merged to 3.0, 3.1, and master.