failed WRONG_FIX UNSUBMITTED wrong_fix_unsubmitted(timeout) · 20 tool calls · 0 s · fastapi/fastapi
🐛 Use `401` status code in security classes when credentials are missing
## Warning
This description is partially outdated after changes described in [this comment](https://github.com/fastapi/fastapi/pull/13786#pullrequestreview-3501695067).
## Description
This PR is an attempt to finally solve the issue with security tools returning error responses with status code `403` instead of `401` when credentials are not provided.
## Breaking changes and workaround
These changes can break projects that rely on old behavior.
In order to mitigate this, the `not_authenticated_status_code` is introduced. If set to `403`, it will make it work the same way as it was before changes (return `403` status code).
This option should be treated as a temporary workaround to give developers more time to update Clients to follow the new behavior.
## Changes and reasoning
### APIKeyQuery, APIKeyHeader, APIKeyCookie
* **Standard:**
* These schemes are not covered by standards, but developers usually follow the same rules as for other standards
* **Actions:**
* The default status code for not providing API key was changed from 403 to 401.
* Temporary `not_authenticated_status_code` parameter can be used to revert this behavior back to returning 403 error code without sending `WWW-Authenticate`.
* **Notes:**
* It’s considered to be a good practice to include in `WWW-Authenticate` information needed to understand how the key is supposed to be passed. I implemented default format (`WWW-Authenticate: ApiKey in="...", name="..."` ), but it’s possible to override the template for `WWW-Authenticate` by subclassing and defining the `format_www_authenticate_header_value` method
### HTTP Basic
* **Standard:**
* https://datatracker.ietf.org/doc/html/rfc7617
* **Actions:**
* No needed. This scheme already acts according to the standard in terms of returning 401 status code with `WWW-Authenticate` header on a lack of credentials
* **Notes:**
* `realm` is required according to the RFC, but optional in the current implementation. Fixing this would introduce breaking changes. Considering this is not a problem for people who want to follow the standard, I suggest we leave it as it is.
### HTTP Digest
* **Standard:**
* https://datatracker.ietf.org/doc/html/rfc7616
* **Actions:**
* The default status code for not providing the authorization parameter was changed from 403 to 401.
* `WWW-Authenticate` is just a stub for now (just `WWW-Authenticate: Digest`) (see notes)
* Temporary `not_authenticated_status_code` parameter can be used to revert this behavior back to returning 403 error code without sending `WWW-Authenticate`.
* **Notes:**
* Since the current `HTTPDigest` implementation is just a stub, we can’t follow standards (we don’t generate `nonce`'s, don’t have `realm`, …). I suggest we just change the error status code and add a stub for `WWW-Authenticate` (just `WWW-Authenticate: Digest`). For now `HTTPDigest` can’t be used as it is, so, this is not a problem.
* We can later add full implementation of `Digest` scheme. There have been made several attempts to implement it (#9825, #3071)
* Should we add a note that `HTTPDigest` is just a stub?
### HTTP Bearer, OAuth2 schemes, OIDC
* **Standard:**
* https://datatracker.ietf.org/doc/html/rfc6750
* **Actions:**
* For `OAuth2PasswordBearer` and `OAuth2AuthorizationCodeBearer`: not needed.
* They already return a 401 error code. Implementation is probably not 100% correct (see notes), but considering nobody argued, I think we can leave it as it is for now.
* For `HTTPBearer` and `OpenIdConnect`:
* The default status code for not providing the authorization parameter was changed from 403 to 401. The suggested implementation will be in line with the current implementations of `OAuth2PasswordBearer` and `OAuth2AuthorizationCodeBearer`.
* Temporary `not_authenticated_status_code` parameter added to `HTTPBearer` can be used to revert this behavior back to returning 403 error code without sending `WWW-Authenticate`.
* **Notes:**
* It’s [recommended to return 400 error response](https://datatracker.ietf.org/doc/html/rfc6750#section-3.1) if the parameter is missed or of an unsupported type, but this is not a strict requirement (word SHOULD is used). I suggest we ignore this and follow the approach that is consistent with other schemes.
* The format of `WWW-Authenticate` is not clearly described: It’s said that the value "Bearer" MUST be followed by one or more auth-param values. At the same time, all auth-param attributes are optional. In examples they always add `realm`. Since we don’t have `realm`, I suggest we just skip it and send just `WWW-Authenticate: Bearer`
* We can later improve the `WWW-Authenticate` format by adding `realm` and `scope`
## Links
* https://github.com/fastapi/fastapi/issues/10177
* https://github.com/fastapi/fastapi/pull/2120
* https://github.com/fastapi/fastapi/pull/5332
* https://github.com/fastapi/fastapi/pull/12507
* https://github.com/fastapi/fastapi/pull/5310
* https://github.com/fastapi/fastapi/pull/13729
* https://github.com/fastapi/fastapi/pull/13514
* https://github.com/fastapi/fastapi/pull/13326
| # | Tool | Arguments | Result |
|---|---|---|---|
| No trace captured. | |||
diff --git a/fastapi/security/api_key.py b/fastapi/security/api_key.py
index 496c815a..57c76b1d 100644
--- a/fastapi/security/api_key.py
+++ b/fastapi/security/api_key.py
@@ -5,17 +5,21 @@ from fastapi.openapi.models import APIKey, APIKeyIn
from fastapi.security.base import SecurityBase
from starlette.exceptions import HTTPException
from starlette.requests import Request
-from starlette.status import HTTP_403_FORBIDDEN
+from starlette.status import HTTP_401_UNAUTHORIZED, HTTP_403_FORBIDDEN
from typing_extensions import Annotated
class APIKeyBase(SecurityBase):
@staticmethod
- def check_api_key(api_key: Optional[str], auto_error: bool) -> Optional[str]:
+ def check_api_key(
+ api_key: Optional[str],
+ auto_error: bool,
+ not_authenticated_status_code: int = HTTP_401_UNAUTHORIZED,
+ ) -> Optional[str]:
if not api_key:
if auto_error:
raise HTTPException(
- status_code=HTTP_403_FORBIDDEN, detail="Not authenticated"
+ status_code=not_authenticated_status_code, detail="Not authenticated"
)
return None
return api_key
@@ -99,6 +103,10 @@ class APIKeyQuery(APIKeyBase):
"""
),
] = True,
+ not_authenticated_status_code: Annotated[
+ int,
+ Doc("Status code to use when credentials are missing."),
+ ] = HTTP_401_UNAUTHORIZED,
):
self.model: APIKey = APIKey(
**{"in": APIKeyIn.query},
@@ -107,10 +115,13 @@ class APIKeyQuery(APIKeyBase):
)
self.scheme_name = scheme_name or self.__class__.__name__
self.auto_error = auto_error
+ self.not_authenticated_status_code = not_authenticated_status_code
async def __call__(self, request: Request) -> Optional[str]:
api_key = request.query_params.get(self.model.name)
- return self.check_api_key(api_key, self.auto_error)
+ return self.check_api_key(
+ api_key, self.auto_error, self.not_authenticated_status_code
+ )
class APIKeyHeader(APIKeyBase):
@@ -187,6 +198,10 @@ class APIKeyHeader(APIKeyBase):
"""
),
] = True,
+ not_authenticated_status_code: Annotated[
+ int,
+ Doc("Status code to use when credentials are missing."),
+ ] = HTTP_401_UNAUTHORIZED,
):
self.model: APIKey = APIKey(
**{"in": APIKeyIn.header},
@@ -195,10 +210,13 @@ class APIKeyHeader(APIKeyBase):
)
self.scheme_name = scheme_name or self.__class__.__name__
self.auto_error = auto_error
+ self.not_authenticated_status_code = not_authenticated_status_code
async def __call__(self, request: Request) -> Optional[str]:
api_key = request.headers.get(self.model.name)
- return self.check_api_key(api_key, self.auto_error)
+ return self.check_api_key(
+ api_key, self.auto_error, self.not_authenticated_status_code
+ )
class APIKeyCookie(APIKeyBase):
@@ -275,6 +293,10 @@ class APIKeyCookie(APIKeyBase):
"""
),
] = True,
+ not_authenticated_status_code: Annotated[
+ int,
+ Doc("Status code to use when credentials are missing."),
+ ] = HTTP_401_UNAUTHORIZED,
):
self.model: APIKey = APIKey(
**{"in": APIKeyIn.cookie},
@@ -283,7 +305,10 @@ class APIKeyCookie(APIKeyBase):
)
self.scheme_name = scheme_name or self.__class__.__name__
self.auto_error = auto_error
+ self.not_authenticated_status_code = not_authenticated_status_code
async def __call__(self, request: Request) -> Optional[str]:
api_key = request.cookies.get(self.model.name)
- return self.check_api_key(api_key, self.auto_error)
+ return self.check_api_key(
+ api_key, self.auto_error, self.not_authenticated_status_code
+ )
.F
=================================== FAILURES ===================================
_________________________ test_security_api_key_no_key _________________________
def test_security_api_key_no_key():
client = TestClient(app)
response = client.get("/users/me")
assert response.status_code == 401, response.text
assert response.json() == {"detail": "Not authenticated"}
> assert response.headers["WWW-Authenticate"] == "APIKey"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tests/test_security_api_key_cookie.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = Headers({'content-length': '30', 'content-type': 'application/json'})
key = 'WWW-Authenticate'
def __getitem__(self, key: str) -> str:
"""
Return a single header value.
If there are multiple headers with the same key, then we concatenate
them with commas. See: https://tools.ietf.org/html/rfc7230#section-3.2.2
"""
normalized_key = key.lower().encode(self.encoding)
items = [
header_value.decode(self.encoding)
for _, header_key, header_value in self._list
if header_key == normalized_key
]
if items:
return ", ".join(items)
> raise KeyError(key)
E KeyError: 'WWW-Authenticate'
/Users/jp/repos/kaggle-gemini-coding-agent-post-training/.envs/fastapi/lib/python3.13/site-packages/httpx/_models.py:302: KeyError
=============================== warnings summary ===============================
../../../../../../../Users/jp/repos/kaggle-gemini-coding-agent-post-training/.envs/overlays/starlette-0.50.0-py3-none-any/starlette/testclient.py:45
/Users/jp/repos/kaggle-gemini-coding-agent-post-training/.envs/overlays/starlette-0.50.0-py3-none-any/starlette/testclient.py:45: DeprecationWarning: The anyio.abc.BlockingPortal alias is deprecated, use anyio.from_thread.BlockingPortal instead.
_PortalFactoryType = Callable[[], AbstractContextManager[anyio.abc.BlockingPortal]]
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!
1 failed, 1 passed, 1 warning in 0.62s