Increasing permission of a public link of a folder that was initially shared with share+read permissions is allowed - tests need follow-up #3881

SwikritiT · 2022-05-26T09:46:31Z

Describe the bug

Increasing permission of a public link of a folder that was initially shared with share+read permissions is allowed

Steps to reproduce

Steps to reproduce the behavior:

As user einstein create folder PARENT/CHILD
Share it with user moss with permission read, share

curl -vk -u einstein:relativity https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares -d 'path=PARENT' -d 'shareType=0' -d'permissions=17' -d'shareWith=moss'

Accept the share as moss
create a link for folder parent as moss with permission read

curl -vk -u moss:vista https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares -d 'path=Shares/PARENT' -d 'shareType=3'

update the permission of the last created public link to read,update,create,delete

curl -X PUT -vk -u moss:vista https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares/<share-id> -d 'permissions=15' |xmllint --format -

Expected behavior

As in oc10 it shouldn't be possible to increase the permission and the ocs status code should be 404 (403 seems like an appropriate status code but oc10 returns with 404)

*   Trying 127.0.0.1:80...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 80 (#0)
* Server auth using Basic with user 'testUser'
> PUT /core/ocs/v1.php/apps/files_sharing/api/v1/shares/4946 HTTP/1.1
> Host: localhost
> Authorization: Basic dGVzdFVzZXI6MTIzNDU2
> User-Agent: curl/7.68.0
> Accept: */*
> Content-Length: 14
> Content-Type: application/x-www-form-urlencoded
> 
* upload completely sent off: 14 out of 14 bytes
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Thu, 26 May 2022 09:25:45 GMT
< Server: Apache/2.4.41 (Ubuntu)
< X-Content-Type-Options: nosniff
< X-XSS-Protection: 0
< X-Robots-Tag: none
< X-Frame-Options: SAMEORIGIN
< X-Download-Options: noopen
< X-Permitted-Cross-Domain-Policies: none
< Set-Cookie: oc5soe2gvutv=2o67sn0nj1hr1mbcatopa49o9l; path=/core; HttpOnly; SameSite=Strict
< Expires: Thu, 19 Nov 1981 08:52:00 GMT
< Cache-Control: no-cache, no-store, must-revalidate
< Pragma: no-cache
< Set-Cookie: oc_sessionPassphrase=Rh%2FFS%2BLTaPu%2FPUJGYTvF7x56NxkIyIPJ1UHySSM7f%2Fz3Jjt59eSAqg%2Bo7QY7Z5NybWCdl3xGV65ti68Y5TZey1XKj0oDu8olQZKDOB%2BJPUk1sFp3IQM7JoTJTvJ8DqgA; expires=Thu, 26-May-2022 09:45:45 GMT; Max-Age=1200; path=/core; HttpOnly; SameSite=Strict
< Content-Security-Policy: default-src 'none';manifest-src 'self';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self';connect-src 'self';media-src 'self'
< Set-Cookie: oc5soe2gvutv=f6mgq3ufricsvrdd0qrkgdaibe; path=/core; HttpOnly; SameSite=Strict
< Set-Cookie: cookie_test=test; expires=Thu, 26-May-2022 10:25:45 GMT; Max-Age=3600
< Content-Length: 254
< Vary: Accept-Encoding
< Content-Type: application/xml; charset=utf-8
< 
<?xml version="1.0"?>
<ocs>
 <meta>
  <status>failure</status>
  <statuscode>404</statuscode>
  <message>Cannot set the requested share permissions for PARENT</message>
  <totalitems></totalitems>
  <itemsperpage></itemsperpage>
 </meta>
 <data/>
</ocs>
* Connection #0 to host localhost left intact

Actual behavior

Ths permission is update and the ocs status code is 100

curl -X PUT -vk -u moss:vista https://localhost:9200/ocs/v1.php/apps/files_sharing/api/v1/shares/zDgXMVaCXVDwOzQ -d 'permissions=15' |xmllint --format - 

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 127.0.0.1:9200...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 9200 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/certs/ca-certificates.crt
  CApath: /etc/ssl/certs
} [5 bytes data]
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
} [512 bytes data]
* TLSv1.3 (IN), TLS handshake, Server hello (2):
{ [122 bytes data]
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
{ [6 bytes data]
* TLSv1.3 (IN), TLS handshake, Certificate (11):
{ [835 bytes data]
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
{ [264 bytes data]
* TLSv1.3 (IN), TLS handshake, Finished (20):
{ [36 bytes data]
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
} [1 bytes data]
* TLSv1.3 (OUT), TLS handshake, Finished (20):
} [36 bytes data]
* SSL connection using TLSv1.3 / TLS_AES_128_GCM_SHA256
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: O=Acme Corp; CN=OCIS
*  start date: May 25 05:08:52 2022 GMT
*  expire date: May 25 05:08:52 2023 GMT
*  issuer: O=Acme Corp; CN=OCIS
*  SSL certificate verify result: unable to get local issuer certificate (20), continuing anyway.
* Server auth using Basic with user 'moss'
} [5 bytes data]
> PUT /ocs/v1.php/apps/files_sharing/api/v1/shares/zDgXMVaCXVDwOzQ HTTP/1.1
> Host: localhost:9200
> Authorization: Basic bW9zczp2aXN0YQ==
> User-Agent: curl/7.68.0
> Accept: */*
> Content-Length: 14
> Content-Type: application/x-www-form-urlencoded
> 
} [14 bytes data]
* upload completely sent off: 14 out of 14 bytes
{ [5 bytes data]
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
{ [130 bytes data]
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Length: 1335
< Content-Type: text/xml; charset=utf-8
< Date: Thu, 26 May 2022 09:31:09 GMT
< Ocs-Api-Version: 1
< Vary: Origin
< 
{ [1035 bytes data]
100  1349  100  1335  100    14   6953     72 --:--:-- --:--:-- --:--:--  7026
* Connection #0 to host localhost left intact
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>ok</status>
    <statuscode>100</statuscode>
    <message>OK</message>
  </meta>
  <data>
    <id>zDgXMVaCXVDwOzQ</id>
    <share_type>3</share_type>
    <uid_owner>moss</uid_owner>
    <displayname_owner>Maurice Moss</displayname_owner>
    <additional_info_owner>[email protected]</additional_info_owner>
    <permissions>15</permissions>
    <stime>1653557379</stime>
    <parent/>
    <expiration/>
    <token>YKXcZTkwuJwVehu</token>
    <uid_file_owner>einstein</uid_file_owner>
    <displayname_file_owner>Albert Einstein</displayname_file_owner>
    <additional_info_file_owner>[email protected]</additional_info_file_owner>
    <state>0</state>
    <path>/PARENT</path>
    <item_type>folder</item_type>
    <mimetype>httpd/unix-directory</mimetype>
    <storage_id>shared::/PARENT</storage_id>
    <storage>0</storage>
    <item_source>1284d238-aa92-42ce-bdc4-0b0000009157$4c510ada-c86b-4815-8820-42cdf82c3d51!315359cf-b826-4a26-bcb5-0d0f22ee1891</item_source>
    <file_source>1284d238-aa92-42ce-bdc4-0b0000009157$4c510ada-c86b-4815-8820-42cdf82c3d51!315359cf-b826-4a26-bcb5-0d0f22ee1891</file_source>
    <file_parent/>
    <file_target>/PARENT</file_target>
    <share_with_user_type>0</share_with_user_type>
    <share_with_additional_info/>
    <mail_send>0</mail_send>
    <name/>
    <url>https://localhost:9200/s/YKXcZTkwuJwVehu</url>
  </data>
</ocs>

Setup

Please describe how you started the server and provide a list of relevant environment variables.

OCIS_VERSION=latest
BRANCH=master
STORAGE_FRONTEND_UPLOAD_DISABLE_TUS=false

Additional context

Add any other context about the problem here.

The text was updated successfully, but these errors were encountered:

micbar · 2022-07-13T16:06:31Z

Is this done? Please update this ticket and close the duplicate.

kobergj · 2022-07-14T14:36:28Z

Done with cs3org/reva#3023

SwikritiT · 2022-07-15T04:31:24Z

There are still some tests that are linked to this issue in expected to fail so re-opening

ocis/tests/acceptance/expected-failures-API-on-OCIS-storage.md

Lines 550 to 555 in 99b1a40

    
           #### [Increasing permission of a public link of a folder that was initially shared with share+read permissions is allowed](https://github.com/owncloud/ocis/issues/3881) 
        
           - [apiSharePublicLink2/reShareAsPublicLinkToSharesNewDav.feature:159](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiSharePublicLink2/reShareAsPublicLinkToSharesNewDav.feature#L159) 
        
           - [apiSharePublicLink2/reShareAsPublicLinkToSharesNewDav.feature:160](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiSharePublicLink2/reShareAsPublicLinkToSharesNewDav.feature#L160) 
        
           - [apiSharePublicLink2/reShareAsPublicLinkToSharesNewDav.feature:181](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiSharePublicLink2/reShareAsPublicLinkToSharesNewDav.feature#L181) 
        
           - [apiSharePublicLink2/reShareAsPublicLinkToSharesNewDav.feature:182](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiSharePublicLink2/reShareAsPublicLinkToSharesNewDav.feature#L182)

TODO QA-team

review tests linked to this issue
remove them from expected to fail if they pass or create other issue if necessary

SwikritiT · 2022-07-15T05:32:20Z

The tests are failing currently because OC10 and OCIS gives different ocs status code for this case
This is the response of Ocis

<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>no share permission</message>
  </meta>
</ocs>

This is the response of the OC10

<?xml version="1.0"?>
<ocs>
  <meta>
    <status>failure</status>
    <statuscode>404</statuscode>
    <message>Cannot set the requested share permissions for PARENT</message>
    <totalitems/>
    <itemsperpage/>
  </meta>
  <data/>
</ocs>

What should be the expected status code for this?
cc: @micbar @phil-davis @kobergj

phil-davis · 2022-07-15T05:59:00Z

All those "create/update a public link share" actions seem to return 404 "not found" in oC10 when the sharee does an action to a received shared resource that is not allowed (tries to create a public link share when they do not have reshare permission, tries to create a public link share with more permissions than they have, tries to update a public link share to give it more permissions...)

403 "forbidden" seems a better code for this (HTTP status and OCS status).

What to do next?

adjust the tests to expect 403 (and adjust oC10 to return 403), and adjust oCIS to return 403
adjust oCIS to return 404 the same as current oC10
adjust the tests so that they only expect some 4xx status, and don't care if it is 400, 403 or 404
something else

IMO clients probably don't care too much. If they let a sharee try to create a public link share with too many privileges, then they are probably doing something similar in response to any of the 4xx response codes - telling the user that it didn't work.

kiranparajuli589 · 2022-07-19T10:06:42Z

blocked until #3881 (comment) is answered.

micbar · 2022-07-19T10:44:22Z

IMO 403 makes the most sense here. Other then that, It is not needed to change oc10 (no end user value)

Can the testsuite accept 403 or 404?

phil-davis · 2022-07-19T10:55:17Z

Can the testsuite accept 403 or 404?

Yes, we can make it do that. @SwikritiT please adjust the related tests so that they check that the staus is 403 or 404 - we don't really care which is returened by an implementation.

SwikritiT · 2022-07-19T11:15:57Z

Can the testsuite accept 403 or 404?

Yes, we can make it do that. @SwikritiT please adjust the related tests so that they check that the staus is 403 or 404 - we don't really care which is returened by an implementation.

Similar thing with HTTP status as well in ocs API version-1 both servers reply with 200
But in case of ocs API version-2 OC10 replies with 404 and OCIS 400

SwikritiT · 2022-07-19T11:30:47Z

Can the testsuite accept 403 or 404?

Yes, we can make it do that. @SwikritiT please adjust the related tests so that they check that the staus is 403 or 404 - we don't really care which is returened by an implementation.

@micbar @phil-davis
I've updated the tests to accept 404 or 403 in this PR owncloud/core#40217
This will still fail in current ocis as, ocis returns with 400.

micbar · 2022-07-19T13:13:52Z

@SwikritiT ok, we will fix ocis.

phil-davis · 2022-07-19T14:19:20Z

I've updated the tests to accept 404 or 403 in this PR owncloud/core#40217

core commit id updated in oCIS PR #4220 - we will update the core commit id in reva tomorrow morning. So the HTTP status can be adjusted in reva-oCIS and the tests should start passing.

rhafer · 2022-09-08T11:23:28Z

@micbar @phil-davis We can of course change the Status Code to 403 (btw, just to avoid confusiion, this is not the HTTP Status code, but the status code inside the xml payload of the ocs response).

But the docs seem to indicate that 404 would be more correct:

Code | Description
  -- | --
 100 | Successful
 400 | Wrong or no update parameter given
 403 | Public upload disabled by the admin
 404 | Couldn’t update share

403 would actually mean "Public upload disabled", what ever that means in the context of updating a share :-)

This is a bit of a mess 🤷‍♂️ . I'll prepare a change for reva to return 404, but we can still discuss that.

phil-davis · 2022-09-08T11:28:43Z

"I'll prepare a change for reva to return 404, but we can still discuss that."

That sounds good. At least it will match what oC10 does. Then it is a separate discussion about if the behavior of both oC10 and oCIS should be changed to return 403.

The ocs status code returned for permission errors on updates of publiclink permissions is now aligned with the documentation of the OCS share API and the behaviour of ownCloud 10 See also: owncloud/ocis#3881

SwikritiT added the Type:Bug label May 26, 2022

micbar added the Priority:p2-high Escalation, on top of current planning, release blocker label Jun 29, 2022

phil-davis mentioned this issue Jun 29, 2022

Public link resharing permissions #4061

Closed

kobergj mentioned this issue Jul 1, 2022

Fix public link resharing cs3org/reva#3023

Merged

kobergj self-assigned this Jul 1, 2022

kobergj closed this as completed Jul 14, 2022

SwikritiT changed the title ~~Increasing permission of a public link of a folder that was initially shared with share+read permissions is allowed~~ Increasing permission of a public link of a folder that was initially shared with share+read permissions is allowed - tests need follow-up Jul 15, 2022

SwikritiT unassigned kobergj Jul 15, 2022

SwikritiT added the QA:team label Jul 15, 2022

SwikritiT reopened this Jul 15, 2022

SwikritiT self-assigned this Jul 15, 2022

SwikritiT mentioned this issue Jul 19, 2022

[tests-only][full-ci]Adjust status code assertion step to accept 404 and 403 owncloud/core#40217

Merged

11 tasks

micbar added the Status:Pending label Jul 19, 2022

SwikritiT removed their assignment Jul 20, 2022

micbar added this to the 2.0.0 General Availability milestone Jul 26, 2022

rhafer self-assigned this Sep 8, 2022

rhafer mentioned this issue Sep 8, 2022

Align ocs status code for permission error on publiclink update cs3org/reva#3207

Merged

micbar mentioned this issue Sep 9, 2022

[full-ci] Add support for the jsoncs3 public share manager #4548

Merged

micbar closed this as completed in #4548 Sep 9, 2022

kulmann mentioned this issue Sep 12, 2022

[full-ci] Update reva to v2.10.0 #4558

Merged

9 tasks

micbar moved this to Done in Infinite Scale Team Board Dec 16, 2022

micbar added this to Infinite Scale Team Board Dec 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Increasing permission of a public link of a folder that was initially shared with share+read permissions is allowed - tests need follow-up #3881

Increasing permission of a public link of a folder that was initially shared with share+read permissions is allowed - tests need follow-up #3881

SwikritiT commented May 26, 2022

micbar commented Jul 13, 2022

kobergj commented Jul 14, 2022

SwikritiT commented Jul 15, 2022 •

edited

Loading

SwikritiT commented Jul 15, 2022 •

edited

Loading

phil-davis commented Jul 15, 2022

kiranparajuli589 commented Jul 19, 2022

micbar commented Jul 19, 2022

phil-davis commented Jul 19, 2022

SwikritiT commented Jul 19, 2022

SwikritiT commented Jul 19, 2022 •

edited

Loading

micbar commented Jul 19, 2022

phil-davis commented Jul 19, 2022

rhafer commented Sep 8, 2022

phil-davis commented Sep 8, 2022

Increasing permission of a public link of a folder that was initially shared with share+read permissions is allowed - tests need follow-up #3881

Increasing permission of a public link of a folder that was initially shared with share+read permissions is allowed - tests need follow-up #3881

Comments

SwikritiT commented May 26, 2022

Describe the bug

Steps to reproduce

Expected behavior

Actual behavior

Setup

Additional context

micbar commented Jul 13, 2022

kobergj commented Jul 14, 2022

SwikritiT commented Jul 15, 2022 • edited Loading

SwikritiT commented Jul 15, 2022 • edited Loading

phil-davis commented Jul 15, 2022

kiranparajuli589 commented Jul 19, 2022

micbar commented Jul 19, 2022

phil-davis commented Jul 19, 2022

SwikritiT commented Jul 19, 2022

SwikritiT commented Jul 19, 2022 • edited Loading

micbar commented Jul 19, 2022

phil-davis commented Jul 19, 2022

rhafer commented Sep 8, 2022

phil-davis commented Sep 8, 2022

SwikritiT commented Jul 15, 2022 •

edited

Loading

SwikritiT commented Jul 15, 2022 •

edited

Loading

SwikritiT commented Jul 19, 2022 •

edited

Loading