Skip to content

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Aug 1, 2019

We should not allow the creation of mail shares if link shares are disabled (because it will fail when accessing the share afterwards anyway)

Also we should have the same checks and permissions.
Currently if you create a mail share on a folder without specifying anything default perms are applied (31) but the max perms for a link share is OC.PERMISSION_READ | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE = 15

Not sure we should backport as the ui might needs fixes. But for 18 ui will fit those changes.


We do not allow editing link shares that the current user doesn't own. This is confusing and lead to errors when someone else edit a password or expiration date without the share owner knowing about it.
We only allow deletion

@skjnldsv

This comment has been minimized.

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 3, 2019
@danxuliu danxuliu force-pushed the fix/shareapi/mail-default-permissions branch from 81a028b to 6f42131 Compare October 3, 2019 21:56
@danxuliu
Copy link
Member

danxuliu commented Oct 3, 2019

Rebased on master to trigger CI again.

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the fix/shareapi/mail-default-permissions branch from 6f42131 to c49469c Compare October 4, 2019 06:19
@skjnldsv skjnldsv merged commit 9fb56e2 into master Oct 4, 2019
@skjnldsv skjnldsv deleted the fix/shareapi/mail-default-permissions branch October 4, 2019 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: sharing security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants