Skip to content

✨ Change authentication failure status from 403 to 401 #13326

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.

Sign up for GitHub

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

Jump to bottom
Closed
mnshai wants to merge 46 commits into from
Closed

✨ Change authentication failure status from 403 to 401 #13326

mnshai wants to merge 46 commits into from

Conversation

@mnshai
Copy link

@mnshai mnshai commented Feb 6, 2025 •
edited
Loading

Changes

  • Change HTTP status code from 403 to 401 for authentication failures across all security classes
  • Add appropriate WWW-Authenticate headers according to RFC standards
  • Update all related tests to expect 401 and verify headers

Requirements

  • Update security modules: api_key.py, http.py, oauth2.py, open_id_connect_url.py
  • Update corresponding tests
  • All tests pass

Standards Reference

Breaking Change

This is technically a breaking change as it modifies response status codes, but brings the implementation in line with HTTP standards where:

  • 401 Unauthorized: Client needs to authenticate first
  • 403 Forbidden: Client is authenticated but doesn't have permissions

Checklist

  • Commit message follows the guidelines
  • Tests for the changes have been added/updated

@mnshai mnshai changed the title feat(security): change authentication failure status from 403 to 401 feature(security): change authentication failure status from 403 to 401 Feb 6, 2025
@mnshai mnshai marked this pull request as draft February 6, 2025 07:47
@mnshai mnshai changed the title feature(security): change authentication failure status from 403 to 401 feat(security): change authentication failure status from 403 to 401 Feb 6, 2025
@mnshai mnshai marked this pull request as ready for review February 6, 2025 07:50
@mnshai
Copy link
Author

mnshai commented Feb 6, 2025 •
edited
Loading

Hi! Could someone please add the appropriate labels to this PR? This change modifies security status codes to follow HTTP standards, so I believe it needs a feature or bug label.

@mnshai
Copy link
Author

mnshai commented Feb 6, 2025

Fixes #10177

@alejsdev alejsdev added the feature New feature or request label Feb 6, 2025
@alejsdev alejsdev changed the title feat(security): change authentication failure status from 403 to 401 ✨ Change authentication failure status from 403 to 401 Feb 6, 2025
@davidhuser davidhuser mentioned this pull request Feb 7, 2025
Closed
@dreglad dreglad mentioned this pull request Feb 13, 2025
Closed
@Olegt0rr
Copy link

Olegt0rr commented Feb 17, 2025

Label feature looks weird for me. This PR fixes wrong behaviour, so it's definitely a bug fix.

@alv2017 alv2017 mentioned this pull request Feb 25, 2025
Closed
3 tasks
@pachewise pachewise mentioned this pull request Feb 25, 2025
Open
@mnshai
Copy link
Author

mnshai commented Mar 3, 2025

Hi maintainers,

Just wanted to check if there's any feedback on this PR or if there's anything I can improve to help move it forward.

@alejsdev - would appreciate your thoughts when you have a moment.

Thanks!

@mnshai mnshai mentioned this pull request Mar 3, 2025
Closed
Ale-Cas
Ale-Cas reviewed Mar 3, 2025
View reviewed changes
@jbw-vtl
Copy link

jbw-vtl commented May 23, 2025

Hey All, is anything pending here still to get this merged?

We currently have to maintain a custom HTTPBearer class just to change this from 403 -> 401

@YuriiMotov YuriiMotov mentioned this pull request Jun 11, 2025
Merged
@svlandeg
Copy link
Member

svlandeg commented Oct 21, 2025

@mnshai: thanks for this contribution, it's a detailed and thoughtful PR. As you mentioned: while correct, this will be a breaking change. For that reason, we'll likely go with #13786 instead, which provides an additional fall-back mechanism for users that need more time to update. As such, I'll go ahead and close this one. Anyone who has time, is welcome to help review #13786 !

@svlandeg svlandeg closed this Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

4 more reviewers

@Ale-Cas Ale-Cas Ale-Cas approved these changes

@abhishek2393 abhishek2393 abhishek2393 approved these changes

@Olegt0rr Olegt0rr Olegt0rr approved these changes

@bezbozhnik bezbozhnik bezbozhnik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@mnshai @Olegt0rr @jbw-vtl @svlandeg @abhishek2393 @bezbozhnik @Ale-Cas @alejsdev