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

Improve error handling. Fix logout endpoint. Handle sign-in and MFA errors. #389

Merged
merged 21 commits into from
Aug 23, 2024

Conversation

jpmckinney
Copy link
Member

@jpmckinney jpmckinney commented Aug 23, 2024

closes #349

Changes described in each commit message.

Easier to review ignoring whitespace, especially users.py https://github.com/open-contracting/credere-backend/pull/389/files?w=1

Re: 403 vs 404 for get_user() dependency viz user enumeration: It already discloses the users's non-existence in the detail message, so might as well use 404.

And with this, I end the saga that I started with – trying to better handle login errors!

If after deploy, we witness new errors in Sentry that shouldn't be there (I'm not expecting any), we can just add them to except clauses, or update the Sentry configuration in settings.py (e.g. to not report HTTP 400 errors – in case bots are sending bad requests to endpoints, for example).

@jpmckinney jpmckinney marked this pull request as draft August 23, 2024 05:27
@jpmckinney jpmckinney marked this pull request as ready for review August 23, 2024 07:35
@jpmckinney jpmckinney changed the title Improve error handling. Fix logout endpoint. Improve error handling. Fix logout endpoint. Handle sign-in and MFA errors. Aug 23, 2024
@jpmckinney jpmckinney requested a review from yolile August 23, 2024 07:38
@yolile
Copy link
Member

yolile commented Aug 23, 2024

Not sure why that test is failing

@jpmckinney
Copy link
Member Author

I'll see, and then merge. Could just be that I let too few seconds go by in the test fixtures. Works locally.

@jpmckinney jpmckinney merged commit 4632ee5 into main Aug 23, 2024
10 checks passed
@jpmckinney jpmckinney deleted the user-test branch August 23, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adding logger.error() in validate_file if file size too large
2 participants