Delete/update link issues if PublicLink.Write permission is absent

[kulmann]

Describe the bug

@JammingBen and myself discovered two issues today when trying to work with links with a user that doesn't have the PublicLink.Write permission:

1. Delete is possible for the non-internal link
2. Renaming, setting expiration date is not possible for the internal link

When fixing (1) please make sure that deletion should still be possible for the internal link.

Steps to reproduce

Steps to reproduce the behavior:

1. Login with a user that does have the PublicLink.Write permission
2. Create a public link with any role (e.g. viewer) and a link with role internal
3. Logout, change the role of that very user to a role that doesn't have the PublicLink.Write permission

For issue (1):
4. Try to delete the non-internal link

For issue (2):
4. Try to rename the internal link
5. Try to set an expiration date for the internal link

Expected behavior

For issue (1):
Deleting the non-internal link should not be possible.

For issue (2):
Renaming or setting an expiration date for the internal link should be possible.

Actual behavior

For issue (1):
Deleting the non-internal link is possible.

For issue (2):
Renaming or setting an expiration date for the internal link is not possible.

Setup

https://0001.schule.owncloud.works

Additional context

There is a bug in web for a user who doesn't have the PublicLink.Write permission: Links are always created with the default role, which is Viewer and then fails for the user without the PublicLink.Write permission. For the user without that permission the link should be created with the Internal role instead. See owncloud/web#8788 for the issue and owncloud/web#8796 for the web fix.

[kobergj]

@kulmann I think this is not per se a bug.

For issue (1):
The issue here is that the user created the public link. You are always allowed to delete links (or shares) you created yourself. Even if you lost the permission. This is intended behaviour of ocis.

For issue (2):
I would argue they shouldn't be able to change the link in any way (except deleting them). They lost the permission to update a link, so they shouldn't be able to add any additional features (such as expiration date and name). Imo it doesn't matter if this is an internal link or not.

[kobergj]

@kulmann @micbar should we change server behaviour? Otherwise I would close this ticket

[kulmann]

Argh, sorry, didn't see your pings :-(

For issue (1): agreed, that makes sense. Let's keep that like it is.

For issue (2): product expectation is actually that you can work with Internal links (i.e. permission 0), meaning create, update and delete. We only need to prevent that the link can become a public link (i.e. permission > 0).

I added an issue (3) to the issue description, because creating internal links is not possible either. When I created this issue here web was behaving incorrectly. Now it's behaving correctly for link creation, but oCIS doesn't allow the internal link creation. It is supposed to be possible to create a link with permission: 0, even if the PublicLink.Write permission is absent.

cc @micbar @tbsbdr

[micbar]

I agree. We need to fix it.

@kobergj I am happy to discuss a small refactoring of the permissions.

@kulmann That might involve web.

[kulmann]

I agree. We need to fix it.

@kobergj I am happy to discuss a small refactoring of the permissions.

@kulmann That might involve web.

Sure, just ping me. Happy to join! 💪

[kobergj]

After investigating a little deeper I think there is some confusion about the permissions involved. Let me clear that up a little:

PublicLink.Write is a permission bound to the global user role (admin, spaceadmin and user but not guest)
This permission is always checked when a public link is created/updated/deleted. Without it you can't even delete your own links

The permissions used to create/update/delete public links are CreateGrant, UpdateGrant and RemoveGrant (currently only available for a space manager). As outlined before the check for these permissions is omitted when you created the link yourself.

You can never add more permissions to a link than you have on a file.

To understand this correctly: The desired behaviour is that any user can create/update/delete any internal public link regardless of permissions? That means I could delete the internal links of other users by mistake (or will)?

[kobergj]

Decision:

Do not check for PublicLink.Write when creating an internal link. Continue checking for AddGrant when creating

Do not check for PublicLink.Write when updating a link created by yourself to an internal link

Allow deleting own links, even if PublicLink.Write is not present.

cc @kulmann @micbar

[kobergj]

The change is in cs3org/reva#3819

One open point:

• web will still allow a user to set an expiration date on another users internal link (however that is blocked by the server)

And one more question as I am not sure if this is a problem:

• users without PublicLink.Write will still be able to see (and copy) public links created by other users. Is this intended behaviour?

@kulmann @micbar

[kobergj]

@kulmann @micbar Open points are still valid. Please close if we need no follow ups

[micbar]

From my POV, we can move on with these open points.