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

Api tests scenario related to share permission needs changes #5249

Closed
2 tasks done
amrita-shrestha opened this issue Dec 20, 2022 · 8 comments
Closed
2 tasks done

Api tests scenario related to share permission needs changes #5249

amrita-shrestha opened this issue Dec 20, 2022 · 8 comments
Assignees
Labels

Comments

@amrita-shrestha
Copy link
Contributor

amrita-shrestha commented Dec 20, 2022

The behavior of share permission is acting differently in ocis and oc10 .
Discussed in this comment: #4052 (comment)
According to this comment, it is actual behavior so we need to refactor our test scenario of share permission in ocis
Related issue

Example

In this tests scenario permission is 7 but currently permission 7 is unavailable in ocis

  Scenario Outline: Public can or can-not delete file through publicly shared link depending on having delete permissions using the public WebDAV API
    Given user "Alice" has created a public link share with settings
      | path        | /PARENT       |
      | permissions | <permissions> |
    When the public deletes file "parent.txt" from the last public link share using the <public-webdav-api-version> public WebDAV API
    Then the HTTP status code should be "<http-status-code>"
    And as "Alice" file "PARENT/parent.txt" <should-or-not> exist

    Examples:
      | permissions               | http-status-code | should-or-not | public-webdav-api-version |
      | read,update,create        | 403              | should        | new                       |

QA Todo

  • search for similar tests scenario which uses share permission refactor scenario (there are other permission which behave different in oc10 and ocis please check in detail)
  • remove the issue from the expected-failure file
@amrita-shrestha
Copy link
Contributor Author

amrita-shrestha commented Dec 20, 2022

  1. Permission 7 (read, update,create)
  • 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 @kulmann @fschade @individual-it @phil-davis @SwikritiT is this actual behavior?

@kulmann
Copy link
Member

kulmann commented Dec 21, 2022

@micbar ^

@phil-davis
Copy link
Contributor

IMO the currently-implemented general rule is:

In oC10: ignore permissions that are not useful/relevant to the resource (for example, create permission is not relevant to a shared file - the file is already created when it is shared). Mask off the irrelevant permission bits, and create (or update) the share with just the useful permission bits from those requested.

In ocis: if the requested permissions include permissions that are not useful/relevant to the resource, then fail the request. The client must request only permission bits that are useful/relevant.

Can someone confirm that these 2 different implementations are both acceptable?

Then we can make the understand what to expect for each backend.

@micbar
Copy link
Contributor

micbar commented Dec 22, 2022

Can someone confirm that these 2 different implementations are both acceptable?

@phil-davis Thanks for your analysis. That is obviously a difference. From my POV, the permission bitmask is nearing deprecation an is only be there because of legacy reasons. I am always against "magical" Api behavior, especially in this case where we are dealing with permissions. The API should return "Bad Request" and a reason.

Conclusion: we accept the behavior as it is.

@phil-davis
Copy link
Contributor

phil-davis commented Dec 28, 2022

@amrita-shrestha you (or you can assign someone) can "skip on ocis" the oC10 scenarios that fail because of this different behavior. Then out scenarios that demonstrate the agreed ocis behavior.

Maybe it will be easiest to have examples for ocis in the existing scenario outlines?

Anyway, however you do it, coordinate with @kiranparajuli589 as he copies various parts of the core acceptance tests across to the ocis repo - we don't want to accidentally lose anything.

@saw-jan
Copy link
Member

saw-jan commented Jan 2, 2023

Will be blocked until #5094 is completed
Copying task has been completed with PRs #5280 and #5306

@amrita-shrestha
Copy link
Contributor Author

all PR has been merged so closing this issue

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

No branches or pull requests

5 participants