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

make owncloud driver check permissions #1202

Merged
merged 13 commits into from
Oct 1, 2020

Conversation

butonic
Copy link
Contributor

@butonic butonic commented Sep 28, 2020

The storageprovider and ocdav services were both swallowing permissions errors by converting them to internal server errors.

The fixes in them were discovered while adding permissions checks on the owncloud storage driver, which is why the commits are mixed. For now, I just want to know what the testsuite says.

fixes:

AFAICT @issue-ocis-thumbnails-191 never existed, it seems to be a mixup of

related pr for accessing shared files in the root of the share folder: #1170

@update-docs
Copy link

update-docs bot commented Sep 28, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@butonic butonic marked this pull request as draft September 28, 2020 19:09
@butonic
Copy link
Contributor Author

butonic commented Sep 28, 2020

@phil-davis it seems thisi tests got 'fixed', as in they now return a proper 403.

1401 | @issue-ocis-thumbnails-191 @skipOnOcis-EOS-Storage @issue-ocis-reva-308 @skipOnOcis-OCIS-Storage
1402 | Scenario: download previews of other users files                                                                           # /drone/src/tests/acceptance/features/apiOcisSpecific/apiWebdavPreviews-previews.feature:74
1403 | Given user "Brian" has been created with default attributes and without skeleton files                                   # FeatureContext::userHasBeenCreatedWithDefaultAttributesAndWithoutSkeletonFiles()
1404 | And user "Alice" has uploaded file "filesForUpload/lorem.txt" to "/parent.txt"                                           # FeatureContext::userHasUploadedAFileTo()
1405 | When user "Brian" downloads the preview of "/parent.txt" of "Alice" with width "32" and height "32" using the WebDAV API # FeatureContext::downloadPreviewOfOtherUser()
1406 | Then the HTTP status code should be "500"                                                                                # FeatureContext::thenTheHTTPStatusCodeShouldBe()
1407 | HTTP status code 403 is not the expected value 500
1408 | Failed asserting that 403 matches expected '500'.
367 | @issue-ocis-reva-9 @skipOnOcis-EOS-Storage @issue-ocis-reva-303 @skipOnOcis-OCIS-Storage | 85s
369 | Scenario: send PROPFIND requests to another user's webDav endpoints as normal user                          # /drone/src/tests/acceptance/features/apiOcisSpecific/apiAuthWebDav-webDavPROPFINDAuth.feature:14 | 85s
370 | When user "Brian" requests these endpoints with "PROPFIND" to get property "d:getetag" about user "Alice" # OCSContext::userSendsRequestToTheseEndpointsWithProperty() | 85s
371 | \| endpoint                                           \| | 85s
372 | \| /remote.php/dav/files/%username%/textfile0.txt     \| | 85s
373 | \| /remote.php/dav/files/%username%/PARENT            \| | 85s
374 | \| /remote.php/dav/files/%username%/PARENT/parent.txt \| | 85s
375 | Then the HTTP status code of responses on all endpoints should be "207"                                   # FeatureContext::theHTTPStatusCodeOfResponsesOnAllEndpointsShouldBe() | 85s
376 | Responses did not return expected http status code | 85s
377 | Failed asserting that 403 is identical to 207.

IIRC we now need to enable the core tests for them and get rid of them in the tests/acceptance/features/apiOcisSpecific/apiWebdavPreviews-previews.feature file, right?

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic changed the title [WIP] No longer swallow permissions errors [WIP] make owncloud driver check permissions Sep 30, 2020
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic changed the title [WIP] make owncloud driver check permissions make owncloud driver check permissions Sep 30, 2020
@butonic butonic marked this pull request as ready for review September 30, 2020 14:57
@labkode labkode merged commit 50924a9 into cs3org:master Oct 1, 2020
@butonic butonic deleted the owncloud-enforce-permissions branch October 1, 2020 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants