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

API requests from an unauthorized user should return 403 #5938

Closed
amrita-shrestha opened this issue Mar 27, 2023 · 4 comments · Fixed by #6945
Closed

API requests from an unauthorized user should return 403 #5938

amrita-shrestha opened this issue Mar 27, 2023 · 4 comments · Fixed by #6945
Labels

Comments

@amrita-shrestha
Copy link
Contributor

amrita-shrestha commented Mar 27, 2023

Describe the bug

If an unauthorized user tries to do something then HTTP status code 403 should be returned

Scenarios

  1. Scenario Outline: user with role user and guest can't create Space via Graph API
  2. Scenario Outline: user with role user and guest cannot disable other space via the Graph API
  3. Scenario Outline: user with role user and guest cannot delete others disabled Space via the Graph API
  4. Scenario Outline: user other than the admin tries to add himself to a group
  5. Scenario Outline: user other than the admin can't create a group
  6. Scenario Outline: user other than the admin can't delete a group
  7. Scenario Outline: user other than the admin can't rename a group
  8. Scenario Outline: user other than the admin shouldn't get the groups list
  9. Scenario Outline: user other than the admin shouldn't get users of a group
  10. Scenario Outline: user other than the admin can't remove a user from their group

Expected behavior

HTTP status code 403

Actual behavior

HTTP status code 401 or some 4xx

@amrita-shrestha
Copy link
Contributor Author

discussed in #5742 (comment)

@saw-jan
Copy link
Member

saw-jan commented Jul 9, 2024

#5742 (comment)

In general here is a list of error codes returned by the ms graph api: https://learn.microsoft.com/en-us/graph/errors

Now the 401 Unauthorized vs 403 Forbidden vs 404 Not Found status code may look tricky, but hara are the guidelines:

  • we don't want to expose existence of resources if a user has no access to them, so we return a 404 Not Found instead of a 403 Forbidden.
  • when a user has access to a resource but tries to execute an action that he does not have enough permissions for, e.g. when he tries to write to a read only share, we return a 403 Forbidden

If we could decide on the expected behavior (status-code) on non-admin user trying to do admin things then we can adjust the tests accordingly.

CC @micbar

@micbar
Copy link
Contributor

micbar commented Jul 9, 2024

when a user has access to a resource but tries to execute an action that he does not have enough permissions for, e.g. when he tries to write to a read only share, we return a 403 Forbidden

I agree with that in the current context. So 403 is the way to go.

@saw-jan
Copy link
Member

saw-jan commented Jul 18, 2024

Scenario Outline: user with role user and guest cannot disable other space via the Graph API
Scenario Outline: user with role user and guest cannot delete others disabled Space via the Graph API

This returns 404 which (I think) is expected because the space has not been shared to them

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 a pull request may close this issue.

3 participants