Unexpected error sharing with users from mobile clients

Question

Unexpected error sharing with users from mobile clients

Closed this issue 3 months ago · 37 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

Log in with any user
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"]

Answer 1 · 2024-07-24T12:16:54.000Z

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.

Answer 2 · 2024-07-24T12:37:02.000Z

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?

Answer 3 · 2024-07-24T12:47:16.000Z

Yes. Thanks.

Answer 4 · 2024-07-24T12:49:39.000Z

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?

Answer 5 · 2024-07-24T13:07:04.000Z

Mobile clients still trusting ocs. Need some scalation?

Answer 6 · 2024-07-24T13:11:27.000Z

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

Answer 7 · 2024-07-24T13:14:21.000Z

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

Answer 8 · 2024-07-24T13:19:39.000Z

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

Answer 9 · 2024-07-24T14:47:19.000Z

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?

Answer 10 · 2024-07-31T12:34:48.000Z

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 #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

Answer 11 · 2024-07-31T13:09:45.000Z

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

Answer 12 · 2024-07-31T13:52:45.000Z

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 #8842 or wait @micbar @kulmann confirmation

Answer 13 · 2024-07-31T14:31:23.000Z

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

Answer 14 · 2024-07-31T15:43:45.000Z

@micbar please have a look

Answer 15 · 2024-08-01T07:57:38.000Z

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?

Answer 16 · 2024-08-01T15:17:56.000Z

@jesmrec Thank you! That helps.

We need to understand why this is reported.

@butonic @kobergj any ideas?

Answer 17 · 2024-08-01T17:09:37.000Z

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)

Answer 18 · 2024-08-01T17:38:54.000Z

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

Answer 19 · 2024-08-13T12:18:26.000Z

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.

Answer 20 · 2024-08-13T12:22:41.000Z

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?

Answer 21 · 2024-08-13T12:27:31.000Z

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.

Answer 22 · 2024-08-13T12:31:23.000Z

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

Answer 23 · 2024-08-13T12:43:04.000Z

Closing this one.

Answer 24 · 2024-08-13T13:08:25.000Z

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

Answer 25 · 2024-08-13T13:39:31.000Z

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?

Answer 26 · 2024-08-19T12:09:52.000Z

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

Answer 27 · 2024-08-20T11:25:58.000Z

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

Answer 28 · 2024-08-20T12:25:47.000Z

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

Answer 29 · 2024-08-20T13:08:34.000Z

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.

Answer 30 · 2024-08-20T14:21:44.000Z

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

Answer 31 · 2024-08-20T14:33:27.000Z

@dragotin @jesmrec We need to adjust a lot of tests so we need to know what exactly we need to allow. See this comment: cs3org/reva#4816 (comment)

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.

Answer 32 · 2024-08-20T14:57:41.000Z

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

Answer 33 · 2024-08-20T16:23:07.000Z

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

Answer 34 · 2024-08-21T07:31:57.000Z

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.

Answer 35 · 2024-08-21T08:09:20.000Z

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

Answer 36 · 2024-09-18T12:24:37.000Z

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

Answer 37 · 2024-09-19T07:40:58.000Z

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.