Browse Source

fix: return 500 instead of 422 for security dependency None type mismatch

A security dependency returning None to a non-optional parameter is a
server configuration error, not a client validation error.
pull/15414/head
Ismail Pelaseyed 2 months ago
parent
commit
bd11f811c4
  1. 22
      fastapi/dependencies/utils.py
  2. 45
      tests/test_security_auto_error_none_type_safety.py

22
fastapi/dependencies/utils.py

@ -72,6 +72,7 @@ from starlette.datastructures import (
QueryParams, QueryParams,
UploadFile, UploadFile,
) )
from starlette.exceptions import HTTPException
from starlette.requests import HTTPConnection, Request from starlette.requests import HTTPConnection, Request
from starlette.responses import Response from starlette.responses import Response
from starlette.websockets import WebSocket from starlette.websockets import WebSocket
@ -700,20 +701,15 @@ async def solve_dependencies(
and sub_dependant.param_annotation is not inspect.Parameter.empty and sub_dependant.param_annotation is not inspect.Parameter.empty
and not _annotation_allows_none(sub_dependant.param_annotation) and not _annotation_allows_none(sub_dependant.param_annotation)
): ):
errors.append( raise HTTPException(
{ status_code=500,
"type": "missing", detail=(
"loc": ("dependency", sub_dependant.name), f"Security dependency returned None for parameter"
"msg": ( f" '{sub_dependant.name}' which is annotated as"
f"Dependency returned None for parameter " f" non-optional. Use 'Optional[...]' or '... | None'"
f"'{sub_dependant.name}' which is annotated as " f" when the security scheme has auto_error=False."
f"non-optional. Use 'Optional[...]' or '... | None' " ),
f"if the dependency can return None (e.g. when using "
f"auto_error=False)."
),
}
) )
continue
values[sub_dependant.name] = solved values[sub_dependant.name] = solved
if sub_dependant.cache_key not in dependency_cache: if sub_dependant.cache_key not in dependency_cache:
dependency_cache[sub_dependant.cache_key] = solved dependency_cache[sub_dependant.cache_key] = solved

45
tests/test_security_auto_error_none_type_safety.py

@ -12,7 +12,6 @@ from fastapi.security import (
OAuth2PasswordBearer, OAuth2PasswordBearer,
) )
from fastapi.testclient import TestClient from fastapi.testclient import TestClient
from inline_snapshot import snapshot
def test_oauth2_non_optional_no_auth(): def test_oauth2_non_optional_no_auth():
@ -23,20 +22,10 @@ def test_oauth2_non_optional_no_auth():
def get_me(token: str = Depends(oauth2)): def get_me(token: str = Depends(oauth2)):
return {"token": token} return {"token": token}
client = TestClient(app) client = TestClient(app, raise_server_exceptions=False)
resp = client.get("/me") resp = client.get("/me")
assert resp.status_code == 422 assert resp.status_code == 500
assert resp.json() == snapshot( assert "non-optional" in resp.json()["detail"]
{
"detail": [
{
"type": "missing",
"loc": ["dependency", "token"],
"msg": "Dependency returned None for parameter 'token' which is annotated as non-optional. Use 'Optional[...]' or '... | None' if the dependency can return None (e.g. when using auto_error=False).",
}
]
}
)
def test_oauth2_optional_no_auth(): def test_oauth2_optional_no_auth():
@ -75,9 +64,9 @@ def test_http_bearer_non_optional_no_auth():
def get_profile(creds: HTTPAuthorizationCredentials = Depends(bearer)): def get_profile(creds: HTTPAuthorizationCredentials = Depends(bearer)):
return {"scheme": creds.scheme} return {"scheme": creds.scheme}
client = TestClient(app) client = TestClient(app, raise_server_exceptions=False)
resp = client.get("/profile") resp = client.get("/profile")
assert resp.status_code == 422 assert resp.status_code == 500
def test_http_bearer_optional_no_auth(): def test_http_bearer_optional_no_auth():
@ -104,9 +93,9 @@ def test_api_key_header_non_optional_no_key():
def get_data(key: str = Depends(api_key)): def get_data(key: str = Depends(api_key)):
return {"key": key} return {"key": key}
client = TestClient(app) client = TestClient(app, raise_server_exceptions=False)
resp = client.get("/data") resp = client.get("/data")
assert resp.status_code == 422 assert resp.status_code == 500
def test_api_key_query_non_optional_no_key(): def test_api_key_query_non_optional_no_key():
@ -117,9 +106,9 @@ def test_api_key_query_non_optional_no_key():
def get_data(key: str = Depends(api_key)): def get_data(key: str = Depends(api_key)):
return {"key": key} return {"key": key}
client = TestClient(app) client = TestClient(app, raise_server_exceptions=False)
resp = client.get("/data") resp = client.get("/data")
assert resp.status_code == 422 assert resp.status_code == 500
def test_api_key_cookie_non_optional_no_key(): def test_api_key_cookie_non_optional_no_key():
@ -130,9 +119,9 @@ def test_api_key_cookie_non_optional_no_key():
def get_data(key: str = Depends(api_key)): def get_data(key: str = Depends(api_key)):
return {"key": key} return {"key": key}
client = TestClient(app) client = TestClient(app, raise_server_exceptions=False)
resp = client.get("/data") resp = client.get("/data")
assert resp.status_code == 422 assert resp.status_code == 500
def test_http_basic_non_optional_no_auth(): def test_http_basic_non_optional_no_auth():
@ -143,9 +132,9 @@ def test_http_basic_non_optional_no_auth():
def get_data(creds: HTTPBasicCredentials = Depends(basic)): def get_data(creds: HTTPBasicCredentials = Depends(basic)):
return {"user": creds.username} return {"user": creds.username}
client = TestClient(app) client = TestClient(app, raise_server_exceptions=False)
resp = client.get("/data") resp = client.get("/data")
assert resp.status_code == 422 assert resp.status_code == 500
def test_annotated_syntax_non_optional(): def test_annotated_syntax_non_optional():
@ -156,9 +145,9 @@ def test_annotated_syntax_non_optional():
def get_data(key: Annotated[str, Depends(api_key)]): def get_data(key: Annotated[str, Depends(api_key)]):
return {"key": key} return {"key": key}
client = TestClient(app) client = TestClient(app, raise_server_exceptions=False)
resp = client.get("/data") resp = client.get("/data")
assert resp.status_code == 422 assert resp.status_code == 500
resp2 = client.get("/data", headers={"X-API-Key": "secret"}) resp2 = client.get("/data", headers={"X-API-Key": "secret"})
assert resp2.status_code == 200 assert resp2.status_code == 200
@ -221,9 +210,9 @@ def test_annotated_non_optional_inner_blocked():
def get_data(key: Annotated[str, Depends(api_key)]): def get_data(key: Annotated[str, Depends(api_key)]):
return {"key": key} return {"key": key}
client = TestClient(app) client = TestClient(app, raise_server_exceptions=False)
resp = client.get("/data") resp = client.get("/data")
assert resp.status_code == 422 assert resp.status_code == 500
def test_annotated_optional_inner_allowed(): def test_annotated_optional_inner_allowed():

Loading…
Cancel
Save