Add sharePermissions webdav property #22834

rullzer · 2016-03-03T20:03:35Z

This property can be queries by the clients so they know the max
permissions they can use to share a file with. This will improve the UX.

The oc:permissions proptery is not enough since mountpoints have
different permissions (delete + move by default).

By making it a new property the clients can just request it. On older
servers it will just return a 404 for that property (and thus they know
they have to fall back to their hacky work arounds). But if the property
is returned the client can show proper info.

Added unit tests

CC: @PVince81 @DeepDiver1975 @nickvergessen @MorrisJobke @LukasReschke @schiesbn

mention-bot · 2016-03-03T20:03:37Z

By analyzing the blame information on this pull request, we identified @PVince81, @DeepDiver1975 and @icewind1991 to be potential reviewers

rullzer · 2016-03-04T08:37:30Z

Or course we could also just use the CRUDS permission letters

schiessle · 2016-03-04T10:46:34Z

I can't see a reshare permission, but maybe I miss something.

rullzer · 2016-03-04T11:01:50Z

apps/dav/lib/connector/sabre/node.php

+			return '';
+		}
+
+		$p .= 'R';


@schiesbn reshare permission is right there... basically we return an empty string if we do not have reshare permissions... because if you don't have share permissions you also can't share RW etc.

PVince81 · 2016-03-09T12:33:20Z

Makes sense, but I wonder if we should just return the permission as integer instead of using these archaic letters. Once again I forgot why we got those in the first place...

rullzer · 2016-03-09T12:46:15Z

@PVince81 well that is fine with me of course. I just reused the letters. But we can use CRUDS letters. Or just the permissions.

Permissions might even be cleaner. Let me change the PR.

PVince81 · 2016-03-09T12:49:47Z

The problem with the letters is that you need weird magic to convert them back into the integer.
So not 100% sure...

rullzer · 2016-03-09T13:48:23Z

@PVince81 see e80337f

IMO that looks better indeed.

rullzer · 2016-03-09T16:08:27Z

@owncloud/desktop-developers @owncloud/android-developers @owncloud/ios-developers

This PR will add a property oc:sharePermissions to webdav. This will provide the max permissions you can share this file with for better sharing UX. Right now it is an integer (see https://github.com/owncloud/core/blob/master/lib/public/constants.php#L64-L69). Which is identical to the permissions you send to the sharing API.

The oc:permissions field translates this integer into a string. But I think an integer makes more sense here.

Any objections to that?

rullzer · 2016-03-10T11:40:06Z

Ok lets go with the integer then. Rebased and squashed.

rullzer · 2016-03-10T11:40:33Z

Please review: @PVince81 @DeepDiver1975 @schiesbn

rullzer · 2016-03-10T11:42:48Z

@DeepDiver1975 @karlitschek I think backporthing this makes sense. It will allow our clients to properly show share info for a file/folder that has been shared with them. Since our clients are doing more and more with sharing I would say this is a feature that is very welcome.

But your call.

nickvergessen · 2016-03-10T12:38:56Z

Should have an integration test or something like that, to make sure the API doesnt break

rullzer · 2016-03-10T12:42:50Z

Yes working on those :)

karlitschek · 2016-03-10T15:00:07Z

makes sense. 👍 but not a critical bugfix so no backport 👎

rullzer · 2016-03-10T19:05:07Z

Ok no backport got it :)

Anyway ready for review so I can instruct all our clients to start implenting this already ;)
Error in OCI is unrelated I think. At least I would not know how it could be affected by this..

PVince81 · 2016-03-31T18:03:16Z

apps/dav/lib/connector/sabre/filesplugin.php

@@ -45,6 +45,7 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin {
 	const FILEID_PROPERTYNAME = '{http://owncloud.org/ns}id';
 	const INTERNAL_FILEID_PROPERTYNAME = '{http://owncloud.org/ns}fileid';
 	const PERMISSIONS_PROPERTYNAME = '{http://owncloud.org/ns}permissions';
+	const SHARE_PERMISSIONS_PROPERTYNAME = '{http://owncloud.org/ns}sharePermissions';


Please make this "share-permissions"

PVince81 · 2016-03-31T18:03:27Z

👎 please adjust property name

This property can be queries by the clients so they know the max permissions they can use to share a file with. This will improve the UX. The oc:permissions proptery is not enough since mountpoints have different permissions (delete + move by default). By making it a new property the clients can just request it. On older servers it will just return a 404 for that property (and thus they know they have to fall back to their hacky work arounds). But if the property is returned the client can show proper info. * unit tests * intergration test

rullzer · 2016-03-31T18:32:21Z

Fixed the external mountpoint permissions.

rullzer · 2016-03-31T18:32:34Z

@PVince81 property renamed.

PVince81 · 2016-03-31T18:35:47Z

👍

nickvergessen · 2016-04-04T08:09:31Z

apps/dav/lib/connector/sabre/filesplugin.php

@@ -45,6 +45,7 @@ class FilesPlugin extends \Sabre\DAV\ServerPlugin {
 	const FILEID_PROPERTYNAME = '{http://owncloud.org/ns}id';
 	const INTERNAL_FILEID_PROPERTYNAME = '{http://owncloud.org/ns}fileid';
 	const PERMISSIONS_PROPERTYNAME = '{http://owncloud.org/ns}permissions';
+	const SHARE_PERMISSIONS_PROPERTYNAME = '{http://owncloud.org/ns}share-permissions';


btw @DeepDiver1975 always replaces configs with the real strings in his patches all the time. maybe we should have a decision whats the way to go and then all use the same way? 😉

lock · 2019-08-06T16:01:36Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

rullzer added enhancement feature:sharing app:dav labels Mar 3, 2016

rullzer added this to the 9.1-next milestone Mar 3, 2016

DeepDiver1975 added the in progress label Mar 3, 2016

rullzer reviewed Mar 4, 2016
View reviewed changes

This was referenced Mar 7, 2016

OCS Sharing API v2 #21798

Closed

Disable share permission checkboxes that are unavailable when resharing owncloud/client#4383

Closed

rullzer added the 3 - To Review label Mar 9, 2016

rullzer force-pushed the webdav_property_for_share_permissions branch from 53d1a56 to e80337f Compare March 9, 2016 13:47

LukasReschke mentioned this pull request Mar 9, 2016

Verify Sync Protocol Specification owncloud/client#4455

Closed

rullzer force-pushed the webdav_property_for_share_permissions branch from e80337f to c6d355d Compare March 10, 2016 11:39

rullzer added 2 - Developing and removed 3 - To Review labels Mar 10, 2016

rullzer added the 3 - To Review label Mar 10, 2016

PVince81 reviewed Mar 31, 2016
View reviewed changes

PVince81 added 3 - To Review 2 - Developing and removed 4 - To release 3 - To Review labels Mar 31, 2016

rullzer added 2 commits March 31, 2016 20:12

Correct share permissions for external storages

5334bcc

rullzer force-pushed the webdav_property_for_share_permissions branch from fb9eb9a to 5334bcc Compare March 31, 2016 18:31

rullzer added 3 - To Review and removed 2 - Developing labels Mar 31, 2016

PVince81 added 4 - To release and removed 3 - To Review labels Mar 31, 2016

Fix unit tests

89478a0

rullzer added 2 - Developing 4 - To release and removed 4 - To release 2 - Developing labels Mar 31, 2016

DeepDiver1975 merged commit 63b01a3 into master Mar 31, 2016

DeepDiver1975 removed the in progress label Mar 31, 2016

DeepDiver1975 deleted the webdav_property_for_share_permissions branch March 31, 2016 21:19

This was referenced Apr 1, 2016

Use share-permissions webdav property owncloud/android#1584

Open

Use share-permissions webdav property owncloud/ios-legacy#621

Open

Namespace of share-permission webdav property should not be the ownCloud namespace #23741

Closed

nickvergessen reviewed Apr 4, 2016
View reviewed changes

lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add sharePermissions webdav property #22834

Add sharePermissions webdav property #22834

rullzer commented Mar 3, 2016

mention-bot commented Mar 3, 2016

rullzer commented Mar 4, 2016

schiessle commented Mar 4, 2016

rullzer Mar 4, 2016

PVince81 commented Mar 9, 2016

rullzer commented Mar 9, 2016

PVince81 commented Mar 9, 2016

rullzer commented Mar 9, 2016

rullzer commented Mar 9, 2016

rullzer commented Mar 10, 2016

rullzer commented Mar 10, 2016

rullzer commented Mar 10, 2016

nickvergessen commented Mar 10, 2016

rullzer commented Mar 10, 2016

karlitschek commented Mar 10, 2016

rullzer commented Mar 10, 2016

PVince81 Mar 31, 2016

PVince81 commented Mar 31, 2016

rullzer commented Mar 31, 2016

rullzer commented Mar 31, 2016

PVince81 commented Mar 31, 2016

nickvergessen Apr 4, 2016

lock bot commented Aug 6, 2019

Add sharePermissions webdav property #22834

Add sharePermissions webdav property #22834

Conversation

rullzer commented Mar 3, 2016

mention-bot commented Mar 3, 2016

rullzer commented Mar 4, 2016

schiessle commented Mar 4, 2016

rullzer Mar 4, 2016

Choose a reason for hiding this comment

PVince81 commented Mar 9, 2016

rullzer commented Mar 9, 2016

PVince81 commented Mar 9, 2016

rullzer commented Mar 9, 2016

rullzer commented Mar 9, 2016

rullzer commented Mar 10, 2016

rullzer commented Mar 10, 2016

rullzer commented Mar 10, 2016

nickvergessen commented Mar 10, 2016

rullzer commented Mar 10, 2016

karlitschek commented Mar 10, 2016

rullzer commented Mar 10, 2016

PVince81 Mar 31, 2016

Choose a reason for hiding this comment

PVince81 commented Mar 31, 2016

rullzer commented Mar 31, 2016

rullzer commented Mar 31, 2016

PVince81 commented Mar 31, 2016

nickvergessen Apr 4, 2016

Choose a reason for hiding this comment

lock bot commented Aug 6, 2019