file_target in share response

[phil-davis]

In oC10, set share_folder to Shares in config.php (this makes it match what OCIS does - received shares go in a folder called Shares)
Also disable auto-accept shares, so that shares have to be accepted by the receiver - this is compatible with the OCIS default behavior.

Share a file to some other user, and look at the response. file_target has value like /Shares/textfile0.txt

Do the same in OCIS. file_target has value like /textfile0.txt

Which is the correct behavior and which is the "bug"?

Here is an example of a scenario failing on OCIS (it passes on oC10):

  Background:                                                                                       # /srv/app/testrunner/tests/acceptance/features/apiShareManagementBasic/createShareToSharesFolder.feature:4
    Given the administrator has set the default folder for received shares to "Shares"              # OccContext::theAdministratorHasSetTheDefaultFolderForReceivedSharesTo()
    And auto-accept shares has been disabled                                                        # FeatureContext::autoAcceptSharesHasBeenDisabled()
    And user "Alice" has been created with default attributes and without skeleton files            # FeatureContext::userHasBeenCreatedWithDefaultAttributesAndWithoutSkeletonFiles()
    And user "Alice" has uploaded file with content "ownCloud test text file 0" to "/textfile0.txt" # FeatureContext::userHasUploadedAFileWithContentTo()

Scenario Outline: Creating a share of a file with a user, the default permissions are read(1)+update(2)+can-share(16) # /srv/app/testrunner/tests/acceptance/features/apiShareManagementBasic/createShareToSharesFolder.feature:12
    Given using OCS API version "<ocs_api_version>"                                                                     # FeatureContext::usingOcsApiVersion()
    And user "Brian" has been created with default attributes and without skeleton files                                # FeatureContext::userHasBeenCreatedWithDefaultAttributesAndWithoutSkeletonFiles()
    When user "Alice" shares file "textfile0.txt" with user "Brian" using the sharing API                               # FeatureContext::userSharesFileWithUserUsingTheSharingApi()
    Then the OCS status code should be "<ocs_status_code>"                                                              # OCSContext::theOCSStatusCodeShouldBe()
    And the HTTP status code should be "200"                                                                            # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And the fields of the last response to user "Alice" sharing with user "Brian" should include                        # FeatureContext::checkFieldsOfLastResponseToUser()
      | share_with             | %username%            |
      | share_with_displayname | %displayname%         |
      | file_target            | /Shares/textfile0.txt |
      | path                   | /textfile0.txt        |
      | permissions            | share,read,update     |
      | uid_owner              | %username%            |
      | displayname_owner      | %displayname%         |
      | item_type              | file                  |
      | mimetype               | text/plain            |
      | storage_id             | ANY_VALUE             |
      | share_type             | user                  |
    When user "Brian" accepts the share "/textfile0.txt" offered by user "Alice" using the sharing API                  # FeatureContext::userReactsToShareOfferedBy()
    Then the OCS status code should be "<ocs_status_code>"                                                              # OCSContext::theOCSStatusCodeShouldBe()
    And the HTTP status code should be "200"                                                                            # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And the content of file "/Shares/textfile0.txt" for user "Brian" should be "ownCloud test text file 0"              # FeatureContext::contentOfFileForUserShouldBe()

    Examples:
      | ocs_api_version | ocs_status_code |
      | 1               | 100             |
        │ file_target has unexpected value '/textfile0.txt'
        │ 
        file_target doesn't have value '/Shares/textfile0.txt'
        Failed asserting that false is true.
      | 2               | 200             |
        │ file_target has unexpected value '/textfile0.txt'
        │ 
        file_target doesn't have value '/Shares/textfile0.txt'
        Failed asserting that false is true.

[butonic]

The correct behavior is /Shares/textfile0.txt

The sharing api uses paths that are relative to the users home, aka the /webdav and /dav/files/userid endpoints.

When we introduce an endpoint that allows listing the accessible storages we may need to add new properties to reference the storage and the relative path in it. But that is nothing for the old sharing /ocs api IMNSHO.

[phil-davis]

OK - then this is an OCIS bug.

This will block a lot of core sharing scenarios from passing, because many of them have these sort of "bulk" checks of a list of expected field values. So the scenario stops at the first thing that is wrong, and we don't get to know if the next steps (accepting the share, accessing the share) do actually work.

If this bug is easy to fix, then no problem. But if it will take some time to fix then we need to think about whether we refactor the core tests into more separate scenarios that test different parts of the outcome.

[PVince81]

I had this in Jira but forgot to exhalate: #204

This also breaks the link in the "shared with me" page

[phil-davis]

This kind-of-effectively-breaks sharing, because the share receivers have trouble automatically finding their shares.
Fixing this would help a lot of sharing to "come alive" - is it easy to fix?

[PVince81]

@phil-davis I'm not sure how easy it is. If this is only about reading the "share_folder" config var and pre-pending it there, then it should be easy. Also assuming that shares cannot be moved out of that folder nor renamed, so the share target would never change, at least for now.

Maybe @butonic can confirm.

[butonic]

the sharing code lives in the ocs handler (reva ... in transition to ocis-ocs by @C0rby unless he is dragged elsewhere) in the listSharesWithOthers function: https://github.com/cs3org/reva/blob/137283e20aee81015d8cdb3398184659c914a827/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go#L1080-L1124

Further down the call trace there is https://github.com/cs3org/reva/blob/137283e20aee81015d8cdb3398184659c914a827/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go#L1329-L1430

which has a few TODOs: https://github.com/cs3org/reva/blob/137283e20aee81015d8cdb3398184659c914a827/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go#L1339-L1346

FIleTarget and Path need to be clarified. info.Path should contain the full path, including /Shares or whatever was configured. It might be enough to omit the path.Base() call.

[micbar]

should be also fixed by owncloud/ocis#994