Skip to content

Conversation

@machadoug
Copy link

It's better to use 401 for lack of permission and 409 if the user already exists
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/401
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409

if not crud.user.is_superuser(current_user):
raise HTTPException(
status_code=400, detail="The user doesn't have enough privileges"
status_code=401, detail="The user doesn't have enough privileges"

Choose a reason for hiding this comment

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

This should be a 403

Choose a reason for hiding this comment

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

@machadoug Can you change it?

Copy link

Choose a reason for hiding this comment

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

agree, it should be a 403

@menkotoglou
Copy link

Seems like a stale PR. Could that be closed now?

@codespearhead
Copy link

codespearhead commented May 10, 2024

@tiangolo
Copy link
Member

I think this was handled in another PR, so I'll close this one now. But if someone thinks things should be improved, feel free to create a new PR. ☕ 🤓

@tiangolo tiangolo closed this Sep 27, 2024
alejsdev added a commit that referenced this pull request Dec 19, 2024
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.

6 participants