ocis icon indicating copy to clipboard operation
ocis copied to clipboard

Unexpected error sharing with users from mobile clients

Open jesmrec opened this issue 1 year ago • 14 comments

Describe the bug

Using mobile clients Android and iOS, sharing with other users returns an error (reproduced in ocis.ocis.master.owncloud.works, in stable 5.0.6 works). But, it works in web client. Something different somewhere

Steps to reproduce

  1. Log in with any user
  2. Try to share any file or folder with other user

Let's curlizy it (Android request):

curl 'https://ocis.ocis.master.owncloud.works/ocs/v2.php/apps/files_sharing/api/v1/shares?format=json' \
  -X 'POST' \
  -H 'accept-encoding: identity' \
  -H 'accept-language: en' \
  -H 'authorization: Bearer ... \
  -H 'connection: Keep-Alive' \
  -H 'content-length: 63' \
  -H 'content-type: application/x-www-form-urlencoded' \
  -H 'host: ocis.ocis.master.owncloud.works' \
  -H 'ocs-apirequest: true' \
  -H 'user-agent: Mozilla/5.0 (Android) ownCloud-android/4.3.1' \
  -H 'x-request-id: c3573187-cdef-4073-9e2a-8d1103ced807' \
  --data-raw 'path=%2FtestShare%2F&shareType=0&shareWith=katherine&permissions=31' \
  --compressed

Expected behavior

Content shared

Actual behavior

Error:

{"ocs":{"meta":{"status":"error","statuscode":400,"message":"resharing not supported"}}} 

action was not a resharing, just a direct share of an item.

Setup

reproducible in ocis.ocis.master.owncloud.works

ownCloud Web UI 9.2.0-alpha.1 Infinite Scale 6.1.0+baa0c23c3 Community

not reproducible in stable 5.0.6.... something is missing in the middle. I created the 5.0.6 instance with the following docker-compose-yml file:

version: "3.7"

services:
  ocis:
    image: owncloud/ocis:5.0.6
    ports:
      - 9200:9200
      - 9215:9215
    environment:
      OCIS_INSECURE: "true"
      OCIS_URL: "..."
      IDM_CREATE_DEMO_USERS: "true"
      IDM_ADMIN_PASSWORD: "admin"
      PROXY_ENABLE_BASIC_AUTH: "true"
      OCIS_PASSWORD_POLICY_MIN_CHARACTERS: "8"
      OCIS_PASSWORD_POLICY_MIN_LOWERCASE_CHARACTERS: "1"
      OCIS_PASSWORD_POLICY_MIN_UPPERCASE_CHARACTERS: "1"
      OCIS_PASSWORD_POLICY_MIN_DIGITS: "1"
      OCIS_PASSWORD_POLICY_MIN_SPECIAL_CHARACTERS: "1"
      OCIS_SERVICE_ACCOUNT_ID: "b0fbfad7-3dd6-49cb-b468-3f588f2f82be"
      OCIS_SERVICE_ACCOUNT_SECRET: "asaGE4DF"
    restart: "no"
    entrypoint:
      - /bin/sh
    # run ocis init to initialize a configuration file with random secrets
    # it will fail on subsequent runs, because the config file already exists
    # therefore we ignore the error and then start the ocis server
    command: ["-c", "ocis init || true; ocis server"]

jesmrec avatar Jul 24 '24 08:07 jesmrec

What was the resource Id?

This error normally happens when somebody uses the resource id of an already shared file. E.g in the shares jail.

micbar avatar Jul 24 '24 12:07 micbar

These are the remote ids in Android DB (iOS also reproduces the problem):

Screenshot 2024-07-24 at 14 32 11

In text:

997aa698-6dbb-4fd4-84dc-166bde274a91$4c510ada-c86b-4815-8820-42cdf82c3d51!2fbb52ba-aa5d-4711-b707-ee62de6ca3ab 997aa698-6dbb-4fd4-84dc-166bde274a91$4c510ada-c86b-4815-8820-42cdf82c3d51!50125742-15fb-45b9-aaed-0c98e2fa8162 997aa698-6dbb-4fd4-84dc-166bde274a91$4c510ada-c86b-4815-8820-42cdf82c3d51!04841a90-4d69-4bf5-9906-82ba2a887bdb 997aa698-6dbb-4fd4-84dc-166bde274a91$4c510ada-c86b-4815-8820-42cdf82c3d51!80780ac1-68b4-4432-8e3b-5726158eb873 997aa698-6dbb-4fd4-84dc-166bde274a91$4c510ada-c86b-4815-8820-42cdf82c3d51!51c48ece-4c47-450b-b681-240d862b68e0 997aa698-6dbb-4fd4-84dc-166bde274a91$4c510ada-c86b-4815-8820-42cdf82c3d51!fbbca3cf-1780-4546-804d-b2f0672e2c93 997aa698-6dbb-4fd4-84dc-166bde274a91$4c510ada-c86b-4815-8820-42cdf82c3d51!3711f4ba-7804-4aba-a699-350b7010e2f6

Remote ids are the resource ids in the backend, so the ids that the client receives from endpoint. Is this what you asked for?

jesmrec avatar Jul 24 '24 12:07 jesmrec

Yes. Thanks.

micbar avatar Jul 24 '24 12:07 micbar

This is an ocs request. You know that ocs is deprecated?

I need to check the web request but I think web sends another parameter for the space id.

@kulmann @JammingBen can you help?

micbar avatar Jul 24 '24 12:07 micbar

Mobile clients still trusting ocs. Need some scalation?

jesmrec avatar Jul 24 '24 13:07 jesmrec

@dragotin @TheOneRing

yes. I think so. We are building a lot of new features like secure view and transparent shares which will never be available on the OCS api.

Deprecation happened with 5.0.0 in may. Web has already moved away from ocs

So we need to move together „at some point“

micbar avatar Jul 24 '24 13:07 micbar

@dragotin @TheOneRing

yes. I think so. We are building a lot of new features like secure view and transparent shares which will never be available on the OCS api.

Deprecation happened with 5.0.0 in may. Web has already moved away from ocs

So we need to move together „at some point“

web is the only client that moved away from supporting oc10. all other clients support both oc10 and ocis. moving on to SharingNG would also mean supporting both SharingNG and OCS at the same time. I know from experience that that's ugly...

Any chance that dropping support for oc10 in desktop/android/ios is on the horizon? I'd strongly recommend to only move to SharingNG when dropping oc10 support at the same time...

kulmann avatar Jul 24 '24 13:07 kulmann

We could do it like web: maintain a „LTS“ branch for hotfixing and only do new features on sharing NG.

micbar avatar Jul 24 '24 13:07 micbar

Any chance that dropping support for oc10 in desktop/android/ios is on the horizon? I'd strongly recommend to only move to SharingNG when dropping oc10 support at the same time...

any ETA for this?

And, going back to the initial problem, what was missing in the initial request? i guess that next oCIS release will be out before oC10 is dropped, so, Android and iOS will need a fix for this. Any other new parameter in requests that we should mind?

jesmrec avatar Jul 24 '24 14:07 jesmrec

Actual behavior

Error:

{"ocs":{"meta":{"status":"error","statuscode":400,"message":"resharing not supported"}}} 

action was not a resharing, just a direct share of an item.

I guess you share a resource with (sharing permission) and you get 400, which is correct because we no longer support resharing https://github.com/owncloud/ocis/issues/8842

here is tests which check this case: https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiReshare/resharing.feature#L138-L156

in stable 5.0.6 works because there's still resharing Screenshot 2024-07-31 at 14 27 59

ScharfViktor avatar Jul 31 '24 12:07 ScharfViktor

@ScharfViktor that's it,

--data-raw 'path=%2FtestShare%2F&shareType=0&shareWith=katherine&permissions=31'

31 includes share.

I need clear confirmation because that issue will require changes in both mobile clients.

jesmrec avatar Jul 31 '24 13:07 jesmrec

31 includes share.

yes, you need send 15 if you want to share with editor role

I need clear confirmation because that issue will require changes in both mobile clients.

Resharing will be disabled and removed from the product

https://doc.owncloud.com/ocis/5.0/migration/upgrading_4.0.0_5.0.0.html#manage-breaking-changes

you can also see here https://github.com/owncloud/ocis/issues/8842 or wait @micbar @kulmann confirmation

ScharfViktor avatar Jul 31 '24 13:07 ScharfViktor

thanks a lot @ScharfViktor

ETA for these changes to be consolidated? just to know when the clients must be ready for those changes @micbar @kulmann

jesmrec avatar Jul 31 '24 14:07 jesmrec

@micbar please have a look

TheOneRing avatar Jul 31 '24 15:07 TheOneRing

Having a deeper look into the current issue in ocis.ocis.master.owncloud.works, where the problem is reproducible:

Assuming that resharing capability is going to be always false in oCIS servers from now on, every item in the PROPFIND response is including the can resharing flag inside the oc:permissions property. Is that right?? The issue in the Android repo states that R permission has to be the key to know whether any individual item in the list is "reshareable" or not.

In other words: if the client propfinds the server for the list of files, and the server responses for every item in the list:

<oc:permissions>RDNVWZP</oc:permissions>

R permission meaning that the item can be reshared, the clients will let users to enable the sharing switch and finally, stumbling upon the 400 that @ScharfViktor described.

I'm not clear with the expected behaviour. Please, mind again that clients have to work consistently against any oCIS and oC10 version.

am i wrong somewhere?

jesmrec avatar Aug 01 '24 07:08 jesmrec

@jesmrec Thank you! That helps.

We need to understand why this is reported.

@butonic @kobergj any ideas?

micbar avatar Aug 01 '24 15:08 micbar

R permission meaning that the item can be reshared, the clients will let users to enable the sharing switch and finally

are you sure that R is resharing? I think it's R (Read)

ScharfViktor avatar Aug 01 '24 17:08 ScharfViktor

are you sure that R is resharing? I think it's R (Read)

No, you're right. if I found it in the right place https://github.com/cs3org/reva/blob/dde65a44013db3c4a8e8e5219a7707674838e410/internal/http/services/owncloud/ocs/conversions/role.go#L91-L107

ScharfViktor avatar Aug 01 '24 17:08 ScharfViktor

I think there is a misunderstanding between server and client here:

The "R" flag means "Shareable" to the server. That means the user is allowed to share the file/folder with other users. It doesn't mean the user is allowed to share with share permissions. In fact you could have the "R" flag but only be allowed to share with view permissions.

kobergj avatar Aug 13 '24 12:08 kobergj

No, you're right. if I found it in the right place https://github.com/cs3org/reva/blob/dde65a44013db3c4a8e8e5219a7707674838e410/internal/http/services/owncloud/ocs/conversions/role.go#L91-L107

So, is that wrong?

in that case, the source of truth for resharing is the capability and only the capability?

jesmrec avatar Aug 13 '24 12:08 jesmrec

So, is that wrong?

No that is not wrong. It is standing in this line exactly: https://github.com/cs3org/reva/blob/dde65a44013db3c4a8e8e5219a7707674838e410/internal/http/services/owncloud/ocs/conversions/role.go#L97

"R" means Shareable. Always has been in ocis.

in that case, the source of truth for resharing is the capability and only the capability?

Exactly. Only capability decides if "share" can be added as permission on Shares.

kobergj avatar Aug 13 '24 12:08 kobergj

thanks for clarifying @kobergj , we will follow that path.

jesmrec avatar Aug 13 '24 12:08 jesmrec

Closing this one.

jesmrec avatar Aug 13 '24 12:08 jesmrec

Please correct me if I'm wrong, but this still breaks ever single client(ios&android) out there?

TheOneRing avatar Aug 13 '24 13:08 TheOneRing

Well yes. As far as I understood we still have a bug in ios and android clients which is allowing you to share with share permission even if you can't. But I'm not sure what we can do against it. (Except fix it in the client.)

The server behaves correctly in rejecting the share request as resharing is not permitted. I don't think we should magically reduce permissions as this will confuse the user even more.

The "R" flag is also interpreted (and added) correctly as it simply shows the permission to share.

We can also not reactivate resharing as ocis doesn't support it any more.

Any other ideas how to fix this?

kobergj avatar Aug 13 '24 13:08 kobergj

@hodyroff as experienced live in the EOSC demo...

nicholas-wilson-au avatar Aug 19 '24 12:08 nicholas-wilson-au

@kobergj is this still being worked on? (sharing in the mobile clients with rolling ocis is currently not possible and the mobile clients won't implement Sharing NG until the next production release in Nov. 2024)

tbsbdr avatar Aug 20 '24 11:08 tbsbdr

@TheOneRing reopened this ticket. From my side same as stated before: works as expected. No work needs to be done on server side.

kobergj avatar Aug 20 '24 12:08 kobergj

Unfortunately we need to be "bug compatible", as we can not update all clients at once, and we can not afford to have broken sharing in all mobiles. @kobergj please just accept the wrong mask and ignore it instead of sending an error, so that clients remain functionable.

dragotin avatar Aug 20 '24 13:08 dragotin

@jesmrec during implementation of the fix I recognized your curl request is broken. I hope you handcrafted it because if that is what the client is sending we might have bigger problem.

curl 'https://ocis.ocis.master.owncloud.works/ocs/v2.php/apps/files_sharing/api/v1/shares?format=json' \
  -X 'POST' \
  -H 'accept-encoding: identity' \
  -H 'accept-language: en' \
  -H 'authorization: Bearer ... \
  -H 'connection: Keep-Alive' \
  -H 'content-length: 63' \
  -H 'content-type: application/x-www-form-urlencoded' \
  -H 'host: ocis.ocis.master.owncloud.works' \
  -H 'ocs-apirequest: true' \
  -H 'user-agent: Mozilla/5.0 (Android) ownCloud-android/4.3.1' \
  -H 'x-request-id: c3573187-cdef-4073-9e2a-8d1103ced807' \
  --data-raw 'path=%2FtestShare%2F&shareType=0&shareWith=katherine&permissions=31' \
  --compressed

-H 'content-length: 63' needs to be -H 'content-length: 67. Otherwise the request is cut short and essential information is getting lost.

kobergj avatar Aug 20 '24 14:08 kobergj