fix: return 403 statuscode instead of 404 when there's not enough share permission

[saw-jan]

When trying to reshare a resource (that doesn't have share permission), the ocs response code is 404 but instead it should be 403.

This PR fixes that issue and now the request returns 403 code.

Before:

<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>404</statuscode>
    <message>No share permission</message>
  </meta>
</ocs>

This PR:

<?xml version="1.0" encoding="UTF-8"?>
<ocs>
  <meta>
    <status>error</status>
    <statuscode>403</statuscode>
    <message>No share permission</message>
  </meta>
</ocs>

Motivation: owncloud/ocis#5742 (comment)

In general here is a list of error codes returned by the ms graph api: https://learn.microsoft.com/en-us/graph/errors

Now the 401 Unauthorized vs 403 Forbidden vs 404 Not Found status code may look tricky, but hara are the guidelines:

• we don't want to expose existence of resources if a user has no access to them, so we return a 404 Not Found instead of a 403 Forbidden.
• when a user has access to a resource but tries to execute an action that he does not have enough permissions for, e.g. when he tries to write to a read only share, we return a 403 Forbidden

Issue: owncloud/ocis#6670

[saw-jan]

CC @2403905 @kobergj

[saw-jan]

Looks like some test expectation needs to be changed.
Build: https://drone.owncloud.com/cs3org/reva/2471/20/7

  Scenario Outline: creating a public link from a share with read permission only is not allowed # /drone/src/tmp/testrunner/tests/acceptance/features/coreApiSharePublicLink2/reShareAsPublicLinkToSharesNewDav.feature:14
    Given using OCS API version "<ocs_api_version>"                                              # FeatureContext::usingOcsApiVersion()
    And user "Alice" has created folder "/test"                                                  # FeatureContext::userHasCreatedFolder()
    And user "Alice" has shared folder "/test" with user "Brian" with permissions "read"         # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    And user "Brian" has accepted share "/test" offered by user "Alice"                          # FeatureContext::userHasReactedToShareOfferedBy()
    When user "Brian" creates a public link share using the sharing API with settings            # FeatureContext::userCreatesAPublicLinkShareWithSettings()
      | path         | /Shares/test |
      | publicUpload | false        |
    Then the OCS status code should be "404"                                                     # OCSContext::theOCSStatusCodeShouldBe()
    And the HTTP status code should be "<http_status_code>"                                      # FeatureContext::thenTheHTTPStatusCodeShouldBe()

    Examples:
      | ocs_api_version | http_status_code |
      | 1               | 200              |
        Failed step: Then the OCS status code should be "404"
        OCS status code is not any of the expected values 404 got 403
        Failed asserting that an array contains '403'.

I would think, for this scenario, the status code should be 403 instead of 404.

[kobergj]

👍 Awesome

[saw-jan]

I will test it in ocis (owncloud/ocis#6919), update expected failure list, merge it and then bump the ocis (after owncloud/ocis#6919)

[phil-davis]

I suppose that test expectations need to be adjusted. Maybe you could adjust the test scenarios in ocis so they have the expected results. They will fail in the current ocis. Add them to expected-failures in ocis. Then bump ocis here in this reva PR - the tests should pass here. Then bump reva into ocis (removing the test scenarios from expected-failures in ocis)
Unless you have a better way to do this sequence of code and test changes.

[saw-jan]

I suppose that test expectations need to be adjusted. Maybe you could adjust the test scenarios in ocis so they have the expected results. They will fail in the current ocis. Add them to expected-failures in ocis. Then bump ocis here in this reva PR - the tests should pass here. Then bump reva into ocis (removing the test scenarios from expected-failures in ocis) Unless you have a better way to do this sequence of code and test changes.

I was thinking of adding failures to expected-failure list here, merge it and bump reva in ocis, update test expectation (passing tests), merge it and then bump ocis here, remove tests from expected-failure list. Just the opposite 😅

[phil-davis]

Yes, that's fine to do it that way.

[saw-jan]

changelog is failing. I don't know why. fixed

[saw-jan]

Tested in https://drone.owncloud.com/owncloud/ocis/24901