As a user i want be able to re-share a shared file

[ChrisEdS]

@ChrisEdS commented on Mon Feb 05 2018

Steps to reproduce

1. Create user "Carlos", "Pasquale" and "Michael" in demo.owncloud.org
2. As "Carlos" share /Documents/Example.odt with "Pasquale" - turn options "can share" on, and "can edit" off

3. Under Windows (make sure you logged in as Pasquale in your ownCloud client) use right click on the shared file - then click on ownCloud -> Share

4. Try to share with "Michael"

Expected behaviour

Sharing with Michael should be possible

Actual behaviour

Sharing not possible - Error message "Cannot increase permission of ..."

Server configuration

ownCloud version: (see ownCloud admin page)
10.0.3

Client configuration

Client:
2.4.0
Operating system:
Windows 10

---

@phil-davis commented on Mon Feb 05 2018

The complication here is that Pasquale is only given read access to the shared file. So when he on-shares it to Michael, he can only give Micheal read access.

The webUI does this correctly, and does not even show the "Can edit" checkbox when Pasquale is sharing the file onwards.

I guess that the Windows client integration is (not yet) smart enough to understand the received access privileges on the file, and to only use those when initiating on-share of the file.

Should this therefore be logged in the client repo?

---

@ChrisEdS commented on Mon Feb 05 2018

Ok, make sense. As you explain it, it's actually more of a case for the client repo.

[SamuAlfageme]

I think there are several issues at hand here. I'm checking the oc:permissions of such file (shared with me; cannot edit but can reshare):

<oc:permissions>SRDNVW</oc:permissions>

... this is returned in both WebDAV endpoints (old & new). Based on client mappings:

client/src/common/remotepermissions.h

Lines 42 to 54 in cedf728

	enum Permissions {
	CanWrite = 1, // W
	CanDelete = 2, // D
	CanRename = 3, // N
	CanMove = 4, // V
	CanAddFile = 5, // C
	CanAddSubDirectories = 6, // K
	CanReshare = 7, // R
	// Note: on the server, this means SharedWithMe, but in discoveryphase.cpp we also set
	// this permission when the server reports the any "share-types"
	IsShared = 8, // S
	IsMounted = 9, // M
	IsMountedSub = 10, // m (internal: set if the parent dir has IsMounted)

Is not consistent with the semantics of "Shared with me; can reshare but nothing else". There's, however, an additional piece of info in the response:

<oc:share-types>
    <oc:share-type>0</oc:share-type>
</oc:share-types>

Which I think it's responsible of adjusting the behavior @phil-davis reports on the browser (hiding the checkbox in the WebUI JS share widget). However, we currently don't have any special logic to handle this (we just assume the file was "shared with me"):

client/src/libsync/discoveryphase.cpp

Lines 341 to 345 in cedf728

	// S means shared with me.
	// But for our purpose, we want to know if the file is shared. It does not matter
	// if we are the owner or not.
	// Piggy back on the persmission field
	file_stat->remotePerm.setPermission(RemotePermissions::IsShared);

And all things considered,m the webUI is also not consistent with this permissions; i.e. "Rename" option is still displayed for these files even when renaming operation results in 403 forbidden:

Fot everything to be sound with the files backend behavior, the permissions returned for that file should be:

<oc:permissions>SRDV</oc:permissions>

cc/ @PVince81 where should we aim to fix the behavior?

[PVince81]

I think the web UI looks at different permissions, the ones from the share itself as requested through the OCS Share API: http://localhost/owncloud/ocs/v2.php/apps/files_sharing/api/v1/shares?format=json&path=%2Ftest&shared_with_me=true

This contains a "permissions" field which value is 17 (READ (1) + SHARE (16)) which is used to define what reshare permissions are allowed.

Not sure yet why the DAV permissions do not reflect this. Could be this bug: owncloud/core#20685

[SamuAlfageme]

@PVince81 Hm; you're right; the bigger difference with the web UI is that we don't send the permissions (set to 17 - which, when not present, defaults to 31 in core, therefore failing with the "Cannot increase permissions" error)

When we open the share dialog; the ocs request performed is (notice the query parameters here):

curl  \
  -H 'Ocs-APIREQUEST:true' \ 
  -H 'Content-Type:application/x-www-form-urlencoded' \
 'https://localhost/ocs/v1.php/apps/files_sharing/api/v1/shares?path=/test&format=json'

While the web UI plays 2 different requests:

• http://localhost/ocs/v2.php/apps/files_sharing/api/v1/shares?format=json&path=/test&reshares=true (I think it's the equivalent to the one the client does; i.e. no query param at all)
• http://localhost/ocs/v2.php/apps/files_sharing/api/v1/shares?format=json&path=/test&shared_with_me=true - that returns :

{
   "ocs":{
      "meta":{
         "status":"ok",
         "statuscode":200,
         "message":null
      },
      "data":[
         {
            "id":"2",
            "share_type":0,
            "uid_owner":"demo",
            "displayname_owner":"demo",
            "permissions":17,
            "stime":1517903774,
            "parent":null,
            "expiration":null,
            "token":null,
            "uid_file_owner":"demo",
            "displayname_file_owner":"demo",
            "path":"\/test",
            "item_type":"file",
            "mimetype":"application\/pdf",
            "storage_id":"shared::\/test",
            "storage":3,
            "item_source":40,
            "file_source":40,
            "file_parent":15,
            "file_target":"\/test",
            "share_with":"admin",
            "share_with_displayname":"admin",
            "share_with_additional_info":null,
            "mail_send":0
         }
      ]
   }
}

Where the permissions "17" can be extracted.

Not sure yet why the DAV permissions do not reflect this.

I'll open a core ticket requesting this. Think it makes sense.

[SamuAlfageme]

Core ticket about WebDAV permissions in: owncloud/core#30382

[SamuAlfageme]

@ckamm what'cha think about bending to the core behavior described in #6346 (comment)? Not sure if it's the proper way to go or we should push for change server-side (at the end of the day we just need the right oc:permissions to adjust the share UI)

[ckamm]

@SamuAlfageme Huh, the client already has a concept of maxSharingPermission that's based on the share-permissions DAV field. I'll take a look at this.

[ckamm]

PR #6352 fixes the problem in my tests.

[SamuAlfageme]

Working as desired on RC1 🎉

owncloud/core#20426 (comment) should also be taken into account in the future to reduce the differences between DAV and OCS and simplify things.