Unauthorized user can change access policy on update

[laurenwalker]

I came across this issue today while testing the access policy editor in MetacatUI.

Issue description:
I created a portal object and added a user (uid=kepler,o=unaffiliated,dc=ecoinformatics,dc=org) to the access policy with only read and write access. I logged in as this user and despite not having changePermission permission, I was able to update the portal object with a different access policy in the system metadata.

I noticed this only happens when I update the object. If I only update the system metadata, metacat responds with a 401, which is expected:

<?xml version="1.0" encoding="UTF-8"?><error detailCode="4861" errorCode="401" name="NotAuthorized">
    <description>Can't update the system metacat of the object with id urn:uuid:4a7ead0f-a741-4c72-89f1-aeeeb03b0efe since CHANGE_PERMISSION not allowed on urn:uuid:4a7ead0f-a741-4c72-89f1-aeeeb03b0efe for subject[s]: public; authenticatedUser; uid=kepler,o=unaffiliated,dc=ecoinformatics,dc=org; </description>

Expected behavior: Metacat should accept my update to the object, but should not save the changes I made to the system metadata, since I only have read and write access.

To reproduce:

1. Create an object in Metacat and add an access rule that gives read and write access to a user.
2. Get the auth token for this user.
3. Via curl, send a request to the isAuthorized API with changePermission, and note that you get the expected 401 response.
4. Edit the access policy for the object in some way.
5. Send an updateSystemMetadata() PUT request, and note you get the expected 401 response.
6. Send an update() PUT request, and you will get a 200 response and see that the update to the object and system metadata was successful. This is the bug.

[taojing2002]

I checked the code and found we only check if the submitter has the write permission on the existing object in the update method:

authDel.doUpdateAuth(session, existingSysMeta, Permission.WRITE, this.getCurrentNodeId());

[laurenwalker]

I would propose a following check (see pseudo-code):

if( the user has only WRITE permission ){
 if( existingAccessPolicy != newAccessPolicy ){
    perform update with existingAccessPolicy
 }
}

[taojing2002]

Do we swallow the error with existingAccessPolicy or throw an exception to reject the request?

[laurenwalker]

If we can, I think it would be great to accept the UPDATE, but with the original access policy. If we can't do that, then we should throw an exception and reject the UPDATE.

[taojing2002]

Now in the update method, Metacat will check if the user has the CHANGE_PERMISSION. If it does, Metacat just process it. If it does not, Metacat will check WRITE_PERMISSION and the modification of the access rules.