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][full-ci] adding test for listing permission of personal drive by a diffrent user #8932

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

nirajacharya2
Copy link
Contributor

Description

this pr adds test for listing permission of personal space by a different user

added scenarios

Scenario: try to lists the permissions of a users personal space by a different member

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • locally
  • ci

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@saw-jan
Copy link
Member

saw-jan commented Apr 25, 2024

@nirajacharya2 You have conflicts, please, rebase the PR

@nirajacharya2 nirajacharya2 force-pushed the list-perm-personal-not-user branch 2 times, most recently from 9518848 to 497c16f Compare April 25, 2024 08:24
Given using spaces DAV path
And user "Brian" has been created with default attributes and without skeleton files
When user "Brian" tries to list the permissions of space "Personal" owned by "Alice" using permissions endpoint of the Graph API
Then the HTTP status code should be "404"
Copy link
Member

Choose a reason for hiding this comment

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

should not it be a 403 status code? user accessing the another owned resource? just a guess.

Copy link
Member

@saw-jan saw-jan Apr 29, 2024

Choose a reason for hiding this comment

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

IMO, 404 is good here. because no user with any role should not be able to stat/access other user's personal space

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, for this sort of access attempt 404 needs to be returned whenever the request tries to access a resource that it does not have permission to access.
Otherwise, if it did return 403 for a resource that exists but the user has no permission, and 404 for a resource that does not exist at all, then the user could try to guess lots of "random" resources, and would know that the ones that return 403 do actually exist. That would reveal something that the user should not be able to know (the guessed name of the resource...)

@nirajacharya2 nirajacharya2 force-pushed the list-perm-personal-not-user branch 2 times, most recently from fc97226 to 1909682 Compare April 30, 2024 05:02
Copy link

sonarcloud bot commented Apr 30, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@saw-jan saw-jan merged commit 34b13f9 into master Apr 30, 2024
4 checks passed
@delete-merged-branch delete-merged-branch bot deleted the list-perm-personal-not-user branch April 30, 2024 06:26
ownclouders pushed a commit that referenced this pull request Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants