fix(auth): pass request as keyword arg in require_login to fix path-param endpoints
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+6
-3
@@ -83,11 +83,14 @@ def require_login(func):
|
|||||||
if not request.session.get("user"):
|
if not request.session.get("user"):
|
||||||
request.session["redirect_after_login"] = str(request.url)
|
request.session["redirect_after_login"] = str(request.url)
|
||||||
return RedirectResponse(url="/login", status_code=status.HTTP_302_FOUND)
|
return RedirectResponse(url="/login", status_code=status.HTTP_302_FOUND)
|
||||||
# Check if the wrapped function is a coroutine function
|
# Pass request as a keyword argument so that endpoints whose first
|
||||||
|
# parameter is a path variable (e.g. pipeline_id) are not accidentally
|
||||||
|
# bound to the request object when FastAPI supplies all arguments as
|
||||||
|
# keyword arguments.
|
||||||
if inspect.iscoroutinefunction(func):
|
if inspect.iscoroutinefunction(func):
|
||||||
return await func(request, *args, **kwargs)
|
return await func(*args, request=request, **kwargs)
|
||||||
else:
|
else:
|
||||||
return func(request, *args, **kwargs)
|
return func(*args, request=request, **kwargs)
|
||||||
|
|
||||||
return wrapper
|
return wrapper
|
||||||
|
|
||||||
|
|||||||
+44
-2
@@ -1,5 +1,6 @@
|
|||||||
"""Tests for app/auth.py module."""
|
"""Tests for app/auth.py module."""
|
||||||
|
|
||||||
|
import asyncio
|
||||||
import hashlib
|
import hashlib
|
||||||
from unittest.mock import AsyncMock, MagicMock, patch
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
|
|
||||||
@@ -168,13 +169,54 @@ class TestRequireLogin:
|
|||||||
mock_request.session = {"user": {"id": "test"}}
|
mock_request.session = {"user": {"id": "test"}}
|
||||||
|
|
||||||
# Call the decorated sync function
|
# Call the decorated sync function
|
||||||
import asyncio
|
|
||||||
|
|
||||||
result = asyncio.run(sync_endpoint(request=mock_request, param="test_value"))
|
result = asyncio.run(sync_endpoint(request=mock_request, param="test_value"))
|
||||||
|
|
||||||
assert result["message"] == "sync"
|
assert result["message"] == "sync"
|
||||||
assert result["param"] == "test_value"
|
assert result["param"] == "test_value"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_path_param_before_request_async(self):
|
||||||
|
"""Regression: endpoints with a path param before request must not get
|
||||||
|
'multiple values for argument' when AUTH_ENABLED=True.
|
||||||
|
|
||||||
|
FastAPI passes all resolved parameters as keyword arguments to the
|
||||||
|
wrapper. The wrapper must forward ``request`` as a keyword argument
|
||||||
|
too, otherwise the positional ``request`` object would bind to the
|
||||||
|
first parameter (e.g. ``pipeline_id``) while FastAPI simultaneously
|
||||||
|
supplies ``pipeline_id`` as a keyword argument → TypeError.
|
||||||
|
"""
|
||||||
|
with patch("app.auth.AUTH_ENABLED", True):
|
||||||
|
from app.auth import require_login
|
||||||
|
|
||||||
|
@require_login
|
||||||
|
async def endpoint_with_path_param(pipeline_id: int, request: Request, extra: str = ""):
|
||||||
|
return {"pipeline_id": pipeline_id, "extra": extra}
|
||||||
|
|
||||||
|
mock_request = MagicMock(spec=Request)
|
||||||
|
mock_request.session = {"user": {"id": "test"}}
|
||||||
|
|
||||||
|
# Simulate how FastAPI calls the wrapper: all args as keyword args.
|
||||||
|
result = await endpoint_with_path_param(request=mock_request, pipeline_id=42, extra="hello")
|
||||||
|
|
||||||
|
assert result["pipeline_id"] == 42
|
||||||
|
assert result["extra"] == "hello"
|
||||||
|
|
||||||
|
def test_path_param_before_request_sync(self):
|
||||||
|
"""Regression: same as above but for synchronous endpoint functions."""
|
||||||
|
with patch("app.auth.AUTH_ENABLED", True):
|
||||||
|
from app.auth import require_login
|
||||||
|
|
||||||
|
@require_login
|
||||||
|
def sync_endpoint_with_path_param(item_id: int, request: Request):
|
||||||
|
return {"item_id": item_id}
|
||||||
|
|
||||||
|
mock_request = MagicMock(spec=Request)
|
||||||
|
mock_request.session = {"user": {"id": "test"}}
|
||||||
|
|
||||||
|
result = asyncio.run(sync_endpoint_with_path_param(request=mock_request, item_id=7))
|
||||||
|
|
||||||
|
assert result["item_id"] == 7
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.integration
|
@pytest.mark.integration
|
||||||
class TestWhoamiEndpoint:
|
class TestWhoamiEndpoint:
|
||||||
|
|||||||
Reference in New Issue
Block a user