Downloading the older version of shared file as share receiver gives 403

[SwikritiT]

Describe the bug

It is not possible to download the older version of a shared file

Steps to reproduce

Steps to reproduce the behavior:

1. As user Marie create a file
   curl -X PUT -vk -u marie:radioactivity https://localhost:9200/remote.php/dav/files/marie/test.txt -d"original"
2. overwrite the file
   curl -X PUT -vk -u marie:radioactivity https://localhost:9200/remote.php/dav/files/marie/test.txt -d"version 1"
3. Share the file with user einstein and accept the share
4. find the file id
   curl -X PROPFIND -vk -u einstein:relativity https://localhost:9200/remote.php/dav/files/einstein/Shares/test.txt | xmllint --format -
5. get the version of the file
   curl -X PROPFIND -vk -u einstein:relativity https://localhost:9200/remote.php/dav/meta/<fileid>/v | xmllint --format -
6. download data from the link mentioned in the href element
   curl -X GET -vk -u einstein:relativity https://localhost:9200/<href>

Expected behavior

It should be possible to download the content of old version of the file

Actual behavior

Gives 404 status code

Setup

Current master

Additional context

Add any other context about the problem here.

[ScharfViktor]

ownCloud Web UI 5.5.0-rc.8
Infinite Scale 2.0.0-beta.3+275b551cb

I can't reproduce this on an actual master. 6 step gives me 200

[SwikritiT]

ownCloud Web UI 5.5.0-rc.8 Infinite Scale 2.0.0-beta.3+275b551cb

I can't reproduce this on an actual master. 6 step gives me 200

I can still reproduce it. But now it returns 403
the related test is in expected to fail the file

ocis/tests/acceptance/expected-failures-API-on-OCIS-storage.md

Lines 36 to 38 in 0676828

	#### [Downloading the older version of shared file gives 404](https://github.com/owncloud/ocis/issues/3868)
	- [apiVersions/fileVersionsSharingToShares.feature:306](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiVersions/fileVersionsSharingToShares.feature#L306)

The tests fails on CI as well : https://drone.owncloud.com/owncloud/ocis/12666/47/6

 @skipOnOcV10.6 @skipOnOcV10.7 @skipOnOcV10.8.0
  Scenario: download old versions of a shared file as share receiver                           # /srv/app/testrunner/tests/acceptance/features/apiVersions/fileVersionsSharingToShares.feature:306
    Given user "Brian" has been created with default attributes and without skeleton files     # FeatureContext::userHasBeenCreatedWithDefaultAttributesAndWithoutSkeletonFiles()
    And user "Alice" has uploaded file with content "uploaded content" to "textfile0.txt"      # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has uploaded file with content "version 1" to "textfile0.txt"             # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has uploaded file with content "version 2" to "textfile0.txt"             # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has shared file "textfile0.txt" with user "Brian"                         # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    And user "Brian" has accepted share "/textfile0.txt" offered by user "Alice"               # FeatureContext::userHasReactedToShareOfferedBy()
    When user "Brian" downloads the version of file "/Shares/textfile0.txt" with the index "1" # FilesVersionsContext::downloadVersion()
    Then the HTTP status code should be "200"                                                  # FeatureContext::thenTheHTTPStatusCodeShouldBe()
      HTTP status code 403 is not the expected value 200
      Failed asserting that 403 matches expected '200'.
    And the following headers should be set                                                    # FeatureContext::theFollowingHeadersShouldBeSet()
      | header              | value                                                                |
      | Content-Disposition | attachment; filename*=UTF-8''textfile0.txt; filename="textfile0.txt" |
    And the downloaded content should be "version 1"                                           # FeatureContext::downloadedContentShouldBe()
    When user "Brian" downloads the version of file "/Shares/textfile0.txt" with the index "2" # FilesVersionsContext::downloadVersion()
    Then the HTTP status code should be "200"                                                  # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And the following headers should be set                                                    # FeatureContext::theFollowingHeadersShouldBeSet()
      | header              | value                                                                |
      | Content-Disposition | attachment; filename*=UTF-8''textfile0.txt; filename="textfile0.txt" |
    And the downloaded content should be "uploaded content"                                    # FeatureContext::downloadedContentShouldBe()

[JammingBen]

I can also reproduce it, although I'm getting a 404 response. It can also be reproduced via Web:

• userA creates somefile.txt and edits it to create a new version
• userA shares somefile.txt with userB
• userB accepts the share on the "Shared with me"-page
• userB tries to download (head request) or revert (copy request) the previous version

[butonic]

we are copying all extended attributes between versions and the code has these nice comments:

		err = xattrs.CopyMetadataWithSourceLock(targetPath, versionsPath, func(attributeName string) bool {
			return true
			// TODO determine all attributes that must be copied, currently we just copy all and overwrite changed properties
			/*
				return strings.HasPrefix(attributeName, xattrs.GrantPrefix) || // for grants
					strings.HasPrefix(attributeName, xattrs.MetadataPrefix) || // for arbitrary metadata
					strings.HasPrefix(attributeName, xattrs.FavPrefix) || // for favorites
					strings.HasPrefix(attributeName, xattrs.SpaceNameAttr) || // for a shared file
			*/
		}, lock)

That means that new grants do not apply to old versions ... hmm ... we may not even have to copy metadata at all ... since I only use the version nodes to keep track of the blob id ...

🤔

would make the code a lot simpler ...

🤔

since I am rewriting the FinishUpload code in cs3org/reva#3473 already I can look into this ...

[butonic]

fixed by cs3org/reva#3473 ... but needs a review and reva ci has some hiccups.