receiving a file share with permission=2 locks the webUI

[individual-it]

Steps to reproduce

1. share a file with permission=2 to a user. curl --user admin:admin "http://localhost/owncloud-core/ocs/v1.php/apps/files_sharing/api/v1/shares" --data 'path=/data.zip&shareType=0&shareWith=uu1&permissions=2'
2. as receiver open the webUI

Expected behaviour

permission should be set to "3", at least that is the behaviour in 10.2.1
webUI should work OK

Actual behaviour

permission is set to "2"
webUI locks up

Server configuration

Operating system:
ubuntu 18.04
Web server:
Apache2
Database:
MySQL
PHP version:
7.2
ownCloud version: (see ownCloud admin page)
10.3.0
Updated from an older ownCloud or fresh install:
fresh
Where did you install ownCloud from:
git

The content of config/config.php:

{
    "system": {
        "license-key": "***REMOVED SENSITIVE VALUE***",
        "instanceid": "ocuv4s8d5xsv",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "localhost",
            "172.17.0.1"
        ],
        "datadirectory": "\/home\/artur\/www\/owncloud-core\/data",
        "overwrite.cli.url": "http:\/\/localhost\/owncloud-core",
        "dbtype": "mysql",
        "version": "10.3.0.0",
        "logtimezone": "UTC",
        "theme": "",
        "loglevel": "2",
        "maintenance": false,
        "default_language": "en",
        "singleuser": false,
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpmode": "smtp",
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "1025",
        "ldapIgnoreNamingRules": false,
        "sharing.federation.allowHttpFallback": true,
        "files_external_allow_create_new_local": "true",
        "enable_previews": false,
        "dbname": "owncloud-20190809",
        "dbhost": "localhost",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "trusted_proxies": {
            "1": "10.4.1.248",
            "2": "127.0.0.1"
        },
        "htaccess.RewriteBase": "\/owncloud-core\/",
        "apps_paths": [
            {
                "path": "\/home\/artur\/www\/owncloud-core\/apps",
                "url": "\/apps",
                "writable": false
            },
            {
                "path": "\/home\/artur\/www\/owncloud-core\/apps-external",
                "url": "\/apps-external",
                "writable": true
            }
        ],
        "memcache.local": "\\OC\\Memcache\\APCu",
        "skeletondirectory": "\/home\/artur\/www\/owncloud-core\/apps\/testing\/data\/webUISkeleton",
        "cors.allowed-domains": [
            "http:\/\/172.17.0.1:8300"
        ],
        "installed": true
    }
}

List of activated apps:

Enabled:
  - comments: 0.3.0
  - dav: 0.4.0
  - diagnostics: 0.1.0
  - enterprise_key: 0.1.4
  - federatedfilesharing: 0.5.0
  - federation: 0.1.0
  - files: 1.5.2
  - files_external: 0.7.1
  - files_sharing: 0.12.0
  - files_texteditor: 2.2.1
  - files_trashbin: 0.9.1
  - files_versions: 1.3.0
  - firewall: 2.5.0
  - gallery: 16.1.0
  - market: 0.2.3
  - notifications: 0.3.3
  - oauth2: 0.4.1
  - provisioning_api: 0.5.0
  - systemtags: 0.3.0
  - updatenotification: 0.2.1
  - windows_network_drive: 0.7.0
Disabled:
  - brute_force_protection
  - customgroups
  - encryption
  - files_antivirus
  - files_primary_s3
  - guests
  - more-fun-theme
  - multidirtest
  - password_policy
  - ransomware_protection
  - search_elastic
  - security
  - testing
  - theme-example
  - user_ldap
  - user_management

Are you using external storage, if yes which one: local/smb/sftp/...
no
Are you using encryption: yes/no
no
Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/...
no

[individual-it]

in 10.3.0 the reply to the sharing API call looks like

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>100</statuscode>
  <message/>
  <totalitems></totalitems>
  <itemsperpage></itemsperpage>
 </meta>
 <data>
  <id>3</id>
  <share_type>0</share_type>
  <uid_owner>admin</uid_owner>
  <displayname_owner>admin</displayname_owner>
  <permissions>2</permissions>
  <stime>1565710968</stime>
  <parent/>
  <expiration/>
  <token/>
  <uid_file_owner>admin</uid_file_owner>
  <displayname_file_owner>admin</displayname_file_owner>
  <path>/data.zip</path>
  <item_type>file</item_type>
  <mimetype>application/zip</mimetype>
  <storage_id>home::admin</storage_id>
  <storage>1</storage>
  <item_source>6</item_source>
  <file_source>6</file_source>
  <file_parent>2</file_parent>
  <file_target>/data.zip</file_target>
  <share_with>uu1</share_with>
  <share_with_displayname>uu1</share_with_displayname>
  <share_with_additional_info/>
  <mail_send>0</mail_send>
  <attributes/>
 </data>
</ocs>

in 10.2.1

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>100</statuscode>
  <message/>
  <totalitems></totalitems>
  <itemsperpage></itemsperpage>
 </meta>
 <data>
  <id>1</id>
  <share_type>0</share_type>
  <uid_owner>admin</uid_owner>
  <displayname_owner>admin</displayname_owner>
  <permissions>3</permissions>
  <stime>1565710870</stime>
  <parent/>
  <expiration/>
  <token/>
  <uid_file_owner>admin</uid_file_owner>
  <displayname_file_owner>admin</displayname_file_owner>
  <path>/lorem.txt</path>
  <item_type>file</item_type>
  <mimetype>text/plain</mimetype>
  <storage_id>home::admin</storage_id>
  <storage>1</storage>
  <item_source>13</item_source>
  <file_source>13</file_source>
  <file_parent>3</file_parent>
  <file_target>/lorem.txt</file_target>
  <share_with>uu1</share_with>
  <share_with_displayname>uu1</share_with_displayname>
  <share_with_additional_info/>
  <mail_send>0</mail_send>
  <attributes/>
 </data>
</ocs>

[phil-davis]

It will be interesting to decide what to do here. In the recent sharing API permissions refactoring/fixing it was made more "consistent" - in most cases the permissions are set exactly as the caller requested. So you can create shares with just "change" or just "create" or just "delete" permission. I suspect that any share that exists but does not have read permission might be a challenge for the files list on the UI - because the sharee is not entitled to "see" the file (but if they can guess or otherwise know the file name, they can overwrite it, delete it...)

[sharidas]

Looks like propfind for the user files throws exception:

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\NotFound</s:exception>
  <s:message/>
</d:error>

Most likely the permission issue I believe.

[sharidas]

The root cause from where the exception is triggered is here https://github.com/owncloud/core/blob/master/apps/dav/lib/Connector/Sabre/FilesPlugin.php#L286.
Reason: the permission being set to 2, hence 2 & 1 == 0 ( 1, is for read permission ). And it becomes the file is not readable, because we don't get the value as 1 ( as per the code ).
Now one can also argue that if the file/folder has update permission (i.e, permission = 2) then it should be readable.

[jvillafanez]

Unless @pmaier1 says otherwise, I'd keep it simple: if the user hasn't set appropiate permissions, blame the user. There are already cases where this isn't happening, and the code is unnecessary complex -> people expect different things -> bugs are introduced.

In any case, the server mustn't crash. This is something we must fix.

[phil-davis]

If the sharee does not have read access, then I guess that a PROPFIND should not work. NotFound might be the right sort of thing to return, because if the server returns NotFound for items that really do not exist, and some other NotAuthorized for items that do exist but do not have read permission, then a sharee can query lots of different file names and work out which ones exist or not by the different errors returned.

This brings up the question about "what does it mean to not have read permission"?

For an individual file:

1. cannot read the content of the file
2. cannot read various file metadata (dates of creation/modification/whatever, tags, comments)
3. cannot see even the filename (yes, technically that is also a piece of file metadata)

For a folder:

1. cannot list entries in the folder (or see the content or metadata or anything about the entries)
2. cannot read various metadata about the folder (dates of creation/modification/whatever, tags, comments that are on the folder)
3. cannot even see the folder name

(1) seems "obvious". (2) probably also should apply. (3) is questionable?

Because if you apply rule (3) and have received a folder share for which you have, for example, create permission, then you have the permission to upload (create) entries in the folder. But how should the folder be presented to you if you do not have permission to see the name of the folder?

[PVince81]

I thought the Share API was already forbidding some combinations of non-sensical permissions ? or maybe the restriction I remember is only for public links

[karakayasemi]

I thought the Share API was already forbidding some combinations of non-sensical permissions ? or maybe the restriction I remember is only for public links

Previously, we were setting default read permission. However, there was a bug in this intented behavior, it is explained in here: #35922 (comment) . Afterwards, we decided to remove this default read permission and set whatever permissions the client asks for. This change implemented in here: #35884 .

Looks like desktop client also has some problems related to this change: owncloud/client#7419. We may consider correctly re-add this default read permission behaviour.