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

[Tests-Only] Refactor webdav auth #37691

Merged
merged 1 commit into from
Jul 18, 2020
Merged

[Tests-Only] Refactor webdav auth #37691

merged 1 commit into from
Jul 18, 2020

Conversation

kiranparajuli589
Copy link
Contributor

No description provided.

@phil-davis phil-davis changed the title [Test-Only] Refactor webdav auth [Tests-Only] Refactor webdav auth Jul 14, 2020
@owncloud owncloud deleted a comment from update-docs bot Jul 14, 2020
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that there are also scenarios like these in apiAuthOCS suite:
ocsDELETEAuth
ocsGETAuth
ocsPUTAuth

Those look like they need similar refactoring.

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see what else CI tells us.
And then please rebase and squash whne making the last small fixes.

@kiranparajuli589 kiranparajuli589 force-pushed the refactor-webdav-auth branch 3 times, most recently from 2e9277c to 47f9c26 Compare July 15, 2020 11:59
@kiranparajuli589
Copy link
Contributor Author

@phil-davis addressed your comments, please review!

@kiranparajuli589 kiranparajuli589 force-pushed the refactor-webdav-auth branch 3 times, most recently from d05b03e to 624d177 Compare July 16, 2020 03:42
@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/25942/50/13

  Scenario: using OCS with admin basic auth                                 # /drone/src/tests/acceptance/features/apiAuthOcs/ocsGETAuth.feature:235
    When the administrator requests these endpoint with "GET"               # AuthContext::adminRequestsEndpoint()
      | endpoint                 |
      | /ocs/v1.php/cloud/apps   |
      | /ocs/v1.php/cloud/groups |
      | /ocs/v1.php/cloud/users  |
    Then the HTTP status code of responses on all endpoints should be "200" # FeatureContext::theHTTPStatusCodeOfResponsesOnAllEndpointsShouldBe()
      │ array(1) {
      │   [0]=>
      │   int(200)
      │ }
      │ 
    And the OCS status code of responses on all endpoints should be "100"   # FeatureContext::theOCSStatusCodeOfResponsesOnAllEndpointsShouldBe()
    When the administrator requests these endpoint with "GET"               # AuthContext::adminRequestsEndpoint()
      | endpoint                 |
      | /ocs/v2.php/cloud/apps   |
      | /ocs/v2.php/cloud/groups |
      | /ocs/v2.php/cloud/users  |
    Then the HTTP status code of responses on all endpoints should be "200" # FeatureContext::theHTTPStatusCodeOfResponsesOnAllEndpointsShouldBe()
      │ array(1) {
      │   [0]=>
      │   int(200)
      │ }
      │ 
    And the OCS status code of responses on all endpoints should be "200"   # FeatureContext::theOCSStatusCodeOfResponsesOnAllEndpointsShouldBe()
      Expected same but found different ocs status codes of last requested responses (Exception)

It is going to be annoying not to know what were the different status codes returned (for OCS and similar for HTTP).

It would be nice to report the list of status codes. Because they are numbers, it will be easy and clear enough to put a comma-separated list of all the actual returned codes into the exception message. So it could say:

Expected all responses to have OCS status 200 but got 200,201,404,200,200

Then someone reading it can have a chance to "guess" which request(s) had the unexpected status.

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/25954/34/6
drone had some "random" yarn install errors.
I will let the acceptance tests finish so we can see any fails there, then restart drone.

@phil-davis phil-davis self-requested a review July 17, 2020 05:15
@phil-davis
Copy link
Contributor

@kiranparajuli589 https://drone.owncloud.com/owncloud/core/25956/51/13

  Scenario: send MKCOL requests to another user's webDav endpoints as normal user                # /drone/src/tests/acceptance/features/apiAuthWebDav/webDavMKCOLAuth.feature:37
    When user "Brian" requests these endpoints with "MKCOL" including body "" about user "Alice" # OCSContext::userSendsRequestToTheseEndpointsWithBody()
      | endpoint                                           |
      | /remote.php/dav/files/%username%/textfile0.txt     |
      | /remote.php/dav/files/%username%/PARENT            |
      | /remote.php/dav/files/%username%/PARENT/parent.txt |
    Then the HTTP status code of responses on all endpoints should be "403"                      # FeatureContext::theHTTPStatusCodeOfResponsesOnAllEndpointsShouldBe()
      Expected same but found different http status codes of last requested responses.Found status codes: 403,403,409 (Exception)

needs looking at.

After that, IMO the general code here is good, so squash all the commits and maybe this can be merged.

@phil-davis phil-davis force-pushed the refactor-webdav-auth branch from 361a972 to c4c9a4d Compare July 17, 2020 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants