Unexpected error sharing with users from mobile clients

[jesmrec]

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"]

[micbar]

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.

[jesmrec]

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

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?

[micbar]

Yes. Thanks.

[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?

[jesmrec]

Mobile clients still trusting ocs. Need some scalation?

[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“

[kulmann]

@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...

[micbar]

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

[jesmrec]

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?

[ScharfViktor]

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

[jesmrec]

@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.

[ScharfViktor]

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

[jesmrec]

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

[TheOneRing]

@micbar please have a look

[jesmrec]

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?

[micbar]

@jesmrec Thank you! That helps.

We need to understand why this is reported.

@butonic @kobergj any ideas?

[ScharfViktor]

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]

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

[kobergj]

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.

[jesmrec]

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?

[kobergj]

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.

[jesmrec]

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

[jesmrec]

Closing this one.

[TheOneRing]

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

[kobergj]

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?

[nicholas-wilson-au]

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

[tbsbdr]

@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)

[kobergj]

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

[dragotin]

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.

[kobergj]

@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]

@dragotin @jesmrec We need to adjust a lot of tests so we need to know what exactly we need to allow. See this comment: https://github.com/cs3org/reva/pull/4816#discussion_r1723418956

Should we only allow ignore the resharing when 31 is sent as permissions? Or are there other cases we need to consider? 17?

Please note that now if a user explicitly selects Share as permission, we will just silently ignore that and create the share without it.

[jesmrec]

@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.

i didn't. Will review, anyway. The attention of that curl to be paid in the payload, not in the headers.

Should we only allow ignore the resharing when 31 is sent as permissions? Or are there other cases we need to consider? 17?

If the value is greater than 16, then result = value -16

If value is 17 -> consider 1 If value is 25 -> consider 9 if value is 31 -> consider 15

Please note that now if a user explicitly selects Share as permission, we will just silently ignore that and create the share without it.

that's right.

[jesmrec]

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

@kobergj the difference could be:

path=%2FtestShare%2F&shareType=0&shareWith=katherine&permissions=31 counts 67

path=/testShare/&shareType=0&shareWith=katherine&permissions=31 counts 63

It might be confusing the %2F instead of / ? in that case, the encoded characters should count as 1.

What would you expect in that case?

(this is out of scope of that issue, if you think something is wrong, feel free to open issue in client repository).

[kobergj]

in that case, the encoded characters should count as 1.

I would disagree. The content-length header tells the server how many bytes to read. It doesn't care if there are encoded strings in there. This is standard expected behaviour.

[jesmrec]

I would disagree. The content-length header tells the server how many bytes to read. It doesn't care if there are encoded strings in there. This is standard expected behaviour.

i will move this to the android dev team

[saw-jan]

Desktop client redirects sharing to the webUI and doesn't allow resharing. So, I think desktop-client is unaffected CC @TheOneRing

[TheOneRing]

Desktop client redirects sharing to the webUI and doesn't allow resharing. So, I think desktop-client is unaffected CC @TheOneRing

Yes you are right, the issue landed in the "Needs Testing" column by accident.