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

moveShareInsideAnotherShare behaves differently on oCIS than oC10 #3047

Closed
phil-davis opened this issue Jan 28, 2022 · 8 comments
Closed

moveShareInsideAnotherShare behaves differently on oCIS than oC10 #3047

phil-davis opened this issue Jan 28, 2022 · 8 comments
Labels

Comments

@phil-davis
Copy link
Contributor

phil-davis commented Jan 28, 2022

core PR owncloud/core#39724 added new test scenarios that test moving one share inside another share. They were made to add test coverage of the types of things described in core issue owncloud/core#39000

When these are run on oCIS, some of them fail: https://drone.owncloud.com/owncloud/ocis/8983/25/6

runsh: Total unexpected failed scenarios throughout the test run:
apiShareManagementToShares/moveShareInsideAnotherShare.feature:25
apiShareManagementToShares/moveShareInsideAnotherShare.feature:45
apiShareManagementToShares/moveShareInsideAnotherShare.feature:56
apiShareManagementToShares/moveShareInsideAnotherShare.feature:73
runsh: There were no unexpected success.

so the behavior on oCIS is different to on oC10.

The differences need to be investigated and decided which ones need fixing on oCIS, and which ones need tests adjusted. I will post the results of each test scenario below.

Update: after refactoring the tests, the failing scenarios are now https://drone.owncloud.com/owncloud/ocis/8993/25/6

apiShareManagementToShares/moveShareInsideAnotherShare.feature:25
apiShareManagementToShares/moveShareInsideAnotherShare.feature:86
apiShareManagementToShares/moveShareInsideAnotherShare.feature:100
@phil-davis
Copy link
Contributor Author

  Background:                                                                             # /srv/app/testrunner/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature:7
    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 using OCS API version "1"                                                         # FeatureContext::usingOcsApiVersion()
    And these users have been created with default attributes and without skeleton files: # FeatureContext::theseUsersHaveBeenCreatedWithDefaultAttributesAndWithoutSkeletonFiles()
      | username |
      | Alice    |
      | Brian    |
    And user "Alice" has created folder "folderA"                                         # FeatureContext::userHasCreatedFolder()
    And user "Alice" has created folder "folderB"                                         # FeatureContext::userHasCreatedFolder()
    And user "Alice" has uploaded file with content "text A" to "/folderA/fileA.txt"      # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has uploaded file with content "text B" to "/folderB/fileB.txt"      # FeatureContext::userHasUploadedAFileWithContentTo()
    And user "Alice" has shared folder "folderA" with user "Brian"                        # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    And user "Alice" has shared folder "folderB" with user "Brian"                        # FeatureContext::userHasSharedFileWithUserUsingTheSharingApi()
    And user "Brian" has accepted share "/folderA" offered by user "Alice"                # FeatureContext::userHasReactedToShareOfferedBy()
    And user "Brian" has accepted share "/folderB" offered by user "Alice"                # FeatureContext::userHasReactedToShareOfferedBy()

  Scenario: share receiver cannot move a whole share inside another share                            # /srv/app/testrunner/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature:25
    When user "Brian" moves folder "Shares/folderB" to "Shares/folderA/folderB" using the WebDAV API # FeatureContext::userMovesFileUsingTheAPI()
    Then the HTTP status code should be "403"                                                        # FeatureContext::thenTheHTTPStatusCodeShouldBe()
      HTTP status code 201 is not the expected value 403
      Failed asserting that 201 matches expected '403'.
    And as "Alice" folder "/folderB" should exist                                                    # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" folder "/Shares/folderB" should exist                                             # FeatureContext::asFileOrFolderShouldExist()
    And as "Alice" file "/folderB/fileB.txt" should exist                                            # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" file "/Shares/folderB/fileB.txt" should exist          

A share receiver tries to move the whole of a received share inside another received share. On oC10 that is not allowed, and a 403 status is returned. oCIS is returning 201 and letting "something" happen.

@phil-davis
Copy link
Contributor Author

phil-davis commented Jan 28, 2022

Scenario: share receiver moves a whole share inside a local folder                              # /srv/app/testrunner/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature:45
    Given user "Brian" has created folder "localFolder"                                           # FeatureContext::userHasCreatedFolder()
    And user "Brian" has uploaded file with content "local text" to "/localFolder/localFile.txt"  # FeatureContext::userHasUploadedAFileWithContentTo()
    When user "Brian" moves folder "Shares/folderB" to "localFolder/folderB" using the WebDAV API # FeatureContext::userMovesFileUsingTheAPI()
    Then the HTTP status code should be "201"                                                     # FeatureContext::thenTheHTTPStatusCodeShouldBe()
      HTTP status code 501 is not the expected value 201
      Failed asserting that 501 matches expected '201'.
    And as "Brian" folder "/localFolder/folderB" should exist                                     # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" file "/localFolder/folderB/fileB.txt" should exist                             # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" file "/localFolder/localFile.txt" should exist                                 # FeatureContext::asFileOrFolderShouldExist()
    But as "Brian" file "/Shares/folderB/fileB.txt" should not exist                              # FeatureContext::asFileOrFolderShouldNotExist()

A share receiver moves a whole received share out of the Shares folder and into one of their own local folders. oCIS returns a 501.

oCIS currently implements a kind of "Share Jail" where the received shares have to stay in the Shares folder. So IMO we need a different test scenario for this on oCIS. The attempt to move the received share should fail, but it should return some 4xx status code, not 501.

Update: this scenario has been skipped on oCIS.

@phil-davis
Copy link
Contributor Author

phil-davis commented Jan 28, 2022

  Scenario: share receiver moves a whole share inside a local folder then moves the local folder inside a received share # /srv/app/testrunner/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature:56
    Given user "Brian" has created folder "localFolder"                                                                  # FeatureContext::userHasCreatedFolder()
    And user "Brian" has uploaded file with content "local text" to "/localFolder/localFile.txt"                         # FeatureContext::userHasUploadedAFileWithContentTo()
    When user "Brian" moves folder "Shares/folderB" to "localFolder/folderB" using the WebDAV API                        # FeatureContext::userMovesFileUsingTheAPI()
    And user "Brian" moves folder "localFolder" to "Shares/folderA/localFolder" using the WebDAV API                     # FeatureContext::userMovesFileUsingTheAPI()
    Then the HTTP status code should be "201"                                                                            # FeatureContext::thenTheHTTPStatusCodeShouldBe()
      HTTP status code 501 is not the expected value 201
      Failed asserting that 501 matches expected '201'.
    And as "Alice" folder "/folderA/localFolder" should exist                                                            # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" folder "/Shares/folderA/localFolder" should exist                                                     # FeatureContext::asFileOrFolderShouldExist()
    And as "Alice" file "/folderA/localFolder/localFile.txt" should exist                                                # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" file "/Shares/folderA/localFolder/localFile.txt" should exist                                         # FeatureContext::asFileOrFolderShouldExist()
    And as "Alice" file "/folderB/fileB.txt" should exist                                                                # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" file "/Shares/folderB/fileB.txt" should exist                                                         # FeatureContext::asFileOrFolderShouldExist()
    But as "Alice" folder "/folderA/localFolder/folderB" should not exist                                                # FeatureContext::asFileOrFolderShouldNotExist()
    And as "Brian" folder "/Shares/folderA/localFolder/folderB" should not exist

A user tries to move a local folder of theirs into the Shares folder. That works on oC10, but, similar to the other scenario above, should probably not be allowed on oCIS. On oCIS it gives a 501 status. It should give some 4xx status. We can make a different test scenario for oCIS.

Update: this scenario has been skipped on oCIS.

@phil-davis
Copy link
Contributor Author

phil-davis commented Jan 28, 2022

  Scenario: share receiver moves a whole share inside a local folder then tries to move the share inside a received share # /srv/app/testrunner/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature:73
    Given user "Brian" has created folder "localFolder"                                                                   # FeatureContext::userHasCreatedFolder()
    And user "Brian" has uploaded file with content "local text" to "/localFolder/localFile.txt"                          # FeatureContext::userHasUploadedAFileWithContentTo()
    When user "Brian" moves folder "Shares/folderB" to "localFolder/folderB" using the WebDAV API                         # FeatureContext::userMovesFileUsingTheAPI()
    And user "Brian" moves folder "localFolder/folderB" to "Shares/folderA/folderB" using the WebDAV API                  # FeatureContext::userMovesFileUsingTheAPI()
    Then the HTTP status code should be "403"                                                                             # FeatureContext::thenTheHTTPStatusCodeShouldBe()
      HTTP status code 404 is not the expected value 403
      Failed asserting that 404 matches expected '403'.
    And as "Alice" file "/folderB/fileB.txt" should exist                                                                 # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" folder "/localFolder/folderB" should exist                                                             # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" file "/localFolder/folderB/fileB.txt" should exist                                                     # FeatureContext::asFileOrFolderShouldExist()
    But as "Alice" folder "/folderA/folderB" should not exist                                                             # FeatureContext::asFileOrFolderShouldNotExist()
    And as "Brian" folder "/Shares/folderA/folderB" should not exist   

This one fails because the first When step does not work (see previous scenario above). And so localFolder/folderB does not exist, and we get a 404. I will skip that on oCIS because oCIS can't get a received share out of the "Share Jail" in the first place, so a test that tries to move it back is not useful.

Update: this scenario has been skipped on oCIS.

@phil-davis
Copy link
Contributor Author

core PR owncloud/core#39728 refactors these tests. I will update here after that is merged.

@phil-davis
Copy link
Contributor Author

phil-davis commented Jan 28, 2022

  Scenario: share receiver moves a local folder inside a received share (local folder does not have a share in it) # /srv/app/testrunner/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature:86

    Given user "Brian" has created folder "localFolder"                                                            # FeatureContext::userHasCreatedFolder()

    Given user "Brian" has created folder "localFolder/subFolder"                                                  # FeatureContext::userHasCreatedFolder()

    And user "Brian" has uploaded file with content "local text" to "/localFolder/localFile.txt"                   # FeatureContext::userHasUploadedAFileWithContentTo()

    And user "Brian" moves folder "localFolder" to "Shares/folderA/localFolder" using the WebDAV API               # FeatureContext::userMovesFileUsingTheAPI()

    Then the HTTP status code should be "201"                                                                      # FeatureContext::thenTheHTTPStatusCodeShouldBe()

      HTTP status code 501 is not the expected value 201

      Failed asserting that 501 matches expected '201'.

    And as "Alice" folder "/folderA/localFolder" should exist                                                      # FeatureContext::asFileOrFolderShouldExist()

    And as "Brian" folder "/Shares/folderA/localFolder" should exist                                               # FeatureContext::asFileOrFolderShouldExist()

    And as "Alice" folder "/folderA/localFolder/subFolder" should exist                                            # FeatureContext::asFileOrFolderShouldExist()

    And as "Brian" folder "/Shares/folderA/localFolder/subFolder" should exist                                     # FeatureContext::asFileOrFolderShouldExist()

    And as "Alice" file "/folderA/localFolder/localFile.txt" should exist                                          # FeatureContext::asFileOrFolderShouldExist()

    And as "Brian" file "/Shares/folderA/localFolder/localFile.txt" should exist                                   # FeatureContext::asFileOrFolderShouldExist()

Trying to move a local folder into a received share results in a 501. It should give a 201 and work.

@phil-davis
Copy link
Contributor Author

  Scenario: share receiver tries to move a whole share inside a local folder                      # /srv/app/testrunner/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature:100
    Given user "Brian" has created folder "localFolder"                                           # FeatureContext::userHasCreatedFolder()
    And user "Brian" has uploaded file with content "local text" to "/localFolder/localFile.txt"  # FeatureContext::userHasUploadedAFileWithContentTo()
    When user "Brian" moves folder "Shares/folderB" to "localFolder/folderB" using the WebDAV API # FeatureContext::userMovesFileUsingTheAPI()
    Then the HTTP status code should be "403"                                                     # FeatureContext::thenTheHTTPStatusCodeShouldBe()
      HTTP status code 501 is not the expected value 403
      Failed asserting that 501 matches expected '403'.
    And as "Alice" file "/folderB/fileB.txt" should exist                                         # FeatureContext::asFileOrFolderShouldExist()
    And as "Brian" file "/Shares/folderB/fileB.txt" should exist                                  # FeatureContext::asFileOrFolderShouldExist()

Brian tries to move a received shared folder out of Shares and into a local folder of his. That action should be rejected with some 4xx status, but it fails with a 501. The 501 needs to be fixed.

@saw-jan
Copy link
Member

saw-jan commented Jan 2, 2024

Fixed in cs3org/reva#4443

@saw-jan saw-jan closed this as completed Jan 2, 2024
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

2 participants