diff --git a/CHANGELOG.md b/CHANGELOG.md index ef663762..e372a7d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,14 +38,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([`d36ba88`](https://github.com/christianlouis/DocuElevate/commit/d36ba88de765b688888c6e661256f8508da86d89)) -## Unreleased - -### Continuous Integration - -- Fix CodeQL javascript language identifier mismatch - ([`3f8a95d`](https://github.com/christianlouis/DocuElevate/commit/3f8a95d8081c4ce3ad380e6391afa3bebb57bbb7)) - - ## v0.91.0 (2026-03-08) ### Features diff --git a/app/auth.py b/app/auth.py index f570bc2a..5852a6dd 100644 --- a/app/auth.py +++ b/app/auth.py @@ -96,11 +96,14 @@ def require_login(func): ) request.session["redirect_after_login"] = str(request.url) 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): - return await func(request, *args, **kwargs) + return await func(*args, request=request, **kwargs) else: - return func(request, *args, **kwargs) + return func(*args, request=request, **kwargs) return wrapper diff --git a/tests/test_auth.py b/tests/test_auth.py index 81b6276e..052c1f6e 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,5 +1,6 @@ """Tests for app/auth.py module.""" +import asyncio import hashlib from unittest.mock import AsyncMock, MagicMock, patch @@ -168,8 +169,6 @@ class TestRequireLogin: mock_request.session = {"user": {"id": "test"}} # Call the decorated sync function - import asyncio - result = asyncio.run(sync_endpoint(request=mock_request, param="test_value")) assert result["message"] == "sync" @@ -230,6 +229,49 @@ class TestRequireLogin: f"redirect_after_login must not be set for {api_path}" ) + @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 class TestWhoamiEndpoint: