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

Changing OAUTH2/OIDC dependencies response status code when Authorization header missing #5310

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

felixcapuano
Copy link

Hello humans,

I recently have to use the OAuth and OpenID Connect dependencies and was not happy with the 403 status code while I was requesting my API without Header Authorization Header.
So I re-read the status code official documentation :

The HyperText Transfer Protocol (HTTP) 401 Unauthorized response status code indicates that the client request has not been completed because it lacks valid authentication credentials for the requested resource.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/401

The HTTP 403 Forbidden response status code indicates that the server understands the request but refuses to authorize it.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/403

My understanding of these definition, and correct me if I'm wrong, is that if the Authorization header with correct scheme but an invalid token is set will be resulting in a 403 response, otherwise if no Authorization header is set response status code will be 401.

Please let me know if I'm thinking right.

Thank you,

@codecov
Copy link

codecov bot commented Aug 27, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (7b87288) compared to base (cf73051).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 7b87288 differs from pull request most recent head 23a327a. Consider uploading reports for the commit 23a327a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            master     #5310    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          540       535     -5     
  Lines        13969     13825   -144     
==========================================
- Hits         13969     13825   -144     
Impacted Files Coverage Δ
fastapi/security/oauth2.py 100.00% <100.00%> (ø)
fastapi/security/open_id_connect_url.py 100.00% <100.00%> (ø)
tests/test_security_oauth2.py 100.00% <100.00%> (ø)
tests/test_security_openid_connect.py 100.00% <100.00%> (ø)
tests/test_security_openid_connect_description.py 100.00% <100.00%> (ø)
tests/utils.py 100.00% <0.00%> (ø)
fastapi/utils.py 100.00% <0.00%> (ø)
fastapi/routing.py 100.00% <0.00%> (ø)
fastapi/encoders.py 100.00% <0.00%> (ø)
fastapi/websockets.py 100.00% <0.00%> (ø)
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link
Contributor

📝 Docs preview for commit 7b87288 at: https://6309d692a0b05a4ffa7bbdbb--fastapi.netlify.app

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

2 participants