Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Differences of OC10 and OCIS share permissions #4052

Closed
SwikritiT opened this issue Jun 28, 2022 · 9 comments
Closed

Differences of OC10 and OCIS share permissions #4052

SwikritiT opened this issue Jun 28, 2022 · 9 comments

Comments

@SwikritiT
Copy link
Contributor

There are some inconsistencies in sharing permission in OC10 and OCIS and due to this various tests are failing in recent ocis id bump PR: owncloud/web#7178 if this behavior is expected and remains consistent then we might need to adjust tests according to the different behavior of the respective servers

Group share

1. Default - Permission 31

  • OCIS - UI shows custom permission (all checked)
  • OC10 - UI Shows Editor

2. Permission 1

  • OCIS - UI shows Viewer
  • OC10 - UI shows Custom Permission (Read checked)

3. Permission 2

  • OCIS - UI shows Custom Permission (update checked)
  • OC10 - Same as OCIS

4. Permission 3

  • OCIS - UI shows Custom Permission (read+update checked)
  • OC10 - Same as OCIS

5. Permission 4

  • OCIS - UI shows Custom Permission (create checked)
  • OC10 - Same as OCIS

6. Permission 5

  • OCIS - UI shows Custom Permission (read+create checked)
  • OC10 - Same as OCIS

7. Permission 6

  • OCIS - UI shows Custom Permission (update+create checked)
  • OC10 - Same as OCIS

8. Permission 7

  • OCIS - UI shows Custom Permission (read+update+create checked)
  • OC10 - Same as OCIS

9. Permission 8

  • OCIS - UI shows Custom Permission (delete checked)
  • OC10 - Same as OCIS

10. Permission 9

  • OCIS - UI shows Custom Permission (read+delete checked)
  • OC10 - Same as OCIS

11. Permission 15

  • OCIS - UI shows Editor
  • OC10 - UI shows Custom permissions (read+update+create+delete checked)

12. Permission 16

  • OCIS - UI shows custom permission (nothing checked - probably because reshare isn't fully implemented yet) for this the API gives 200 and the share isn't shown for receiver in UI
  • OC10 - UI shows custom permission (share checked)

13. Permission 17 (Share+ read)

  • OCIS - UI shows custom permission (read checked)
  • OC10 - UI shows Viewer

14. Permission 19 (Share+ read+update)

  • OCIS - UI shows custom permission (read and update checked)
  • OC10 - UI shows custom permission (read+update+share checked)

User Share

1. Default - Permission 31

  • OCIS - UI shows custom permission (all checked)
  • OC10 - UI Shows Editor

2. Permission 1

  • OCIS - UI shows Viewer
  • OC10 - UI shows Custom permission (read checked)

3. Permission 2

  • OCIS - UI shows Custom Permission (update checked)
  • OC10 - Same as OCIS

4. Permission 3

  • OCIS - UI shows Custom Permission (read+update checked)
  • OC10 - Same as OCIS

5. Permission 4

  • OCIS - UI shows Custom Permission (create checked)
  • OC10 - Same as OCIS

6. Permission 5

  • OCIS - UI shows Custom Permission (read+create checked)
  • OC10 - Same as OCIS

7. Permission 6

  • OCIS - UI shows Custom Permission (update+create checked)
  • OC10 - Same as OCIS

8. Permission 7

  • OCIS - UI shows Custom Permission (read+update+create checked)
  • OC10 - Same as OCIS

9. Permission 8

  • OCIS - UI shows Custom Permission (delete checked)
  • OC10 - Same as OCIS

10. Permission 9

  • OCIS - UI shows Custom Permission (read+delete checked)
  • OC10 - Same as OCIS

11. Permission 15

  • OCIS - UI shows Editor
  • OC10 - UI shows Custom permissions (read+update+create+delete checked)

12. Permission 16

  • OCIS - UI shows custom permission (nothing checked - probably because reshare isn't fully implemented yet) for this the API gives 200 and the share isn't shown for receiver in UI
  • OC10 - UI shows custom permission (share checked)

13. Permission 17 (Share+ read)

  • OCIS - UI shows custom permission (read checked)
  • OC10 - UI shows viewer

14. Permission 19 (Share+ read+update)

  • OCIS - UI shows custom permission (read and update checked)
  • OC10 - UI shows custom permission (read + update + share checked)

Link Share

1. Default - Permission 1

  • OCIS - UI shows viewer
  • OC10 - same as OCIS

2. Permission 2

  • OCIS - Share not created
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>Could not read permission from request</message>
  </meta>
</ocs>
  • OC10 - Share with permission 1 is created. Viewer role assigned.

3. Permission 3

4. Permission 4

  • OCIS - UI shows Uploader
  • OC10 - UI shows Uploader

5. Permission 5

  • OCIS - UI shows Contributor
  • OC10 - UI shows Contributor

6. Permission 6

  • OCIS - Share not created
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>Could not read permission from request</message>
  </meta>
</ocs>
  • OC10 - Share with permission 1 is created. Viewer role assigned.

7. Permission 7

  • OCIS - Share not created
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>Could not read permission from request</message>
  </meta>
</ocs>
  • OC10 - Share with permission 1 is created. Viewer role assigned.

8. Permission 8

  • OCIS - Share not created
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>Could not read permission from request</message>
  </meta>
</ocs>
  • OC10 - Share with permission 1 is created. Viewer role assigned.

9. Permission 9

  • OCIS - Share not created
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>Could not read permission from request</message>
  </meta>
</ocs>
  • OC10 - Share with permission 1 is created. Viewer role assigned.

10. Permission 15

  • OCIS - UI shows Editor
  • OC10 - UI shows Editor

11. Permission 16

  • OCIS - Share not created
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>Could not read permission from request</message>
  </meta>
</ocs>
  • OC10 - Share with permission 1 is created. Viewer role assigned.

12. Permission 17 (Share+ read)

  • OCIS - Share not created
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>Could not read permission from request</message>
  </meta>
</ocs>
  • OC10 - Share with permission 1 is created. Viewer role assigned.

13. Permission 19 (Share+ read+update)

  • OCIS - Share not created
<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>400</statuscode>
    <message>Could not read permission from request</message>
  </meta>
</ocs>
  • OC10 - Share with permission 1 is created. Viewer role assigned.
@kobergj
Copy link
Collaborator

kobergj commented Jun 28, 2022

From what I understand there a 3 different issues here. The web UI should

  • show "Viewer" if permissions are 17
  • show "Editor" if permissions are 31
  • check share box if share permission is given

The rest seems correct behavior to me. Please correct me if I am wrong

Maybe some extra words regarding public links:
It was officially agreed that we do act differently than oc10. If the permissions requested by the client are not applicable we return an error (unlike oc10 which will downgrade permissions to 1)

@kulmann
Copy link
Member

kulmann commented Jun 28, 2022

From what I understand there a 3 different issues here. The web UI should

  • show "Viewer" if permissions are 17
  • show "Editor" if permissions are 31
  • check share box if share permission is given

The web ui checks for an exact permission match. The permissions for Viewer are 17 if resharing is enabled and 1 if resharing is disabled. Same for Editor: 31 if resharing is enabled and 15 if resharing is disabled.

@fschade
Copy link
Contributor

fschade commented Jun 28, 2022

@SwikritiT i checked most of the mentioned permissions manually seconds ago,
i cannot confirm (or maybe i got something wrong?) the cases.

For example User Share -> Permission 17 (Share+ read):
web share with re-sharing enabled: custom permission (Share + read) -> 17 result viewer
web share with re-sharing off: custom permission (read) -> 1 result custom permission

so the detail here is the resharing part, which must be enabled by the env FRONTEND_ENABLE_RESHARING=true to make it really work, this is done for example here: https://github.com/owncloud/web/blob/2cc6aa4f6ca3161334cf9d04887ccfae2b5d5dbc/.drone.star#L2199

Long story short, can you ping me that we can have a look into it together and bring more light into the dark 🤗

Beside that i started working on a pr that only provides the available roles to the ui.

@fschade
Copy link
Contributor

fschade commented Jun 28, 2022

i added a card in our beta board which will be discussed tomorrow after our standup to decide how we proceed

https://github.com/owncloud/web/projects/7#card-83655801

@SwikritiT
Copy link
Contributor Author

SwikritiT commented Jun 29, 2022

@SwikritiT i checked most of the mentioned permissions manually seconds ago, i cannot confirm (or maybe i got something wrong?) the cases.

For example User Share -> Permission 17 (Share+ read): web share with re-sharing enabled: custom permission (Share + read) -> 17 result viewer web share with re-sharing off: custom permission (read) -> 1 result custom permission

so the detail here is the resharing part, which must be enabled by the env FRONTEND_ENABLE_RESHARING=true to make it really work, this is done for example here: https://github.com/owncloud/web/blob/2cc6aa4f6ca3161334cf9d04887ccfae2b5d5dbc/.drone.star#L2199

Long story short, can you ping me that we can have a look into it together and bring more light into the dark hugs

Beside that i started working on a pr that only provides the available roles to the ui.

Thanks! Didn't know about the FRONTEND_ENABLE_RESHARING=true. I see the variable will be added in the PR owncloud/web#7086. I was trying to bump ocis id for tests in PR owncloud/web#7178 and suddenly all the tests related to sharing started to fail, locally too and I got a little confused about what was happening. After enabling the variable the tests pass and like you mentioned resharing seems to be the main part. Sorry for the trouble, thank you for the clarification!

@fschade
Copy link
Contributor

fschade commented Jun 29, 2022

Cool 😎 feel free to ping us whenever we can help, looking forward to speak to you in person 😘

@micbar
Copy link
Contributor

micbar commented Jun 30, 2022

@SwikritiT Thank you for digging, you brought up a very heavy topic where we need to fight a lot of technical debt.

@micbar micbar reopened this Jun 30, 2022
@micbar micbar added Type:Technical-Debt Priority:p1-urgent Consider a hotfix release with only that fix labels Jun 30, 2022
@micbar
Copy link
Contributor

micbar commented Jun 30, 2022

I made this a p1 because we currently have a regression with that. Let me dig deeper and follow up.

@micbar
Copy link
Contributor

micbar commented Jun 30, 2022

needs separate ticket after discussion with @kobergj

This topic is resolved as far as the UI is involved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants