EyeSeeTea / interpretation-notification

0 stars 0 forks source link

Only interpretations that user has R or R/W are visible #12

Closed adrianq closed 5 years ago

tokland commented 5 years ago

@nancyespinoza

I understand that the problem is that some user sees an interpretation whose sharing apparently grants that user no rights for that. I've been not able to reproduce the problem for a normal guest user, a non-public interpretation is not visible for them. I could reproduce this behavior for admin users, but that's because some roles of the user allow it (for example, CORE - Superuser), this is normal.

So, to be sure, I'd need more details, which interpretation is that? which user changes the sharing? which user sees the sharing when should not? (also, passwords for those users, by email)

nancyespinoza commented 5 years ago

@tokland I sent you the email with the specifications. Let me know if you need any other information.

adrianq commented 5 years ago

We found the error and this was a tricky one. If you have a look at this API: https://leap.psi-mis.org/api/charts/Y20tPdOwxtK.json?fields=id,name,publicAccess,userAccesses[id,name],userGroupAccesses[id,displayName],interpretations[id,publicAccess]

You will see that favourite permission are Read/Write: None while interpretation permissions are Read/Write. This is something that should have never happened because of the code we introduced in the analytics tool. This code is the one we discussed in our meeting on Monday. Basically, whenever you want to make a change to the sharing it checks if that change makes sense. For instance, this may have happened because of the following steps:

This last step should not be allowed because of our code however we have verified that it is actually allowed in leap. Looking at the current code we can see that our code never got into 2.30. After some investigation, it seems that we created the PR to UiO in July when 2.30 was the master branch. However, they didn't merge the code until December (4 months later) when the master branch was 2.32 and it was never backported to 2.30.

@rodmelia We will create again the same PR to 2.30 (fixing the conflicts) and let you know about the PR so you can put some pressure to Oslo

adrianq commented 5 years ago

Anyway @rodmelia we are going to change the code to handle this kind of situations as well: the user has permissions for the interpretation but he does not have permissions for the favourite. This should never happen but as we can see it may happen because of old sharing settings, changing sharings with the backend API, etc.