Merge pull request #511 from christianlouis/copilot/fix-login-button-redirect

fix(auth): prevent post-login redirect to /api/auth/whoami

app/auth.py

@@ -3,9 +3,11 @@ import inspect
 import logging
 import pathlib
 from functools import wraps
+from urllib.parse import urlparse
 
 from authlib.integrations.starlette_client import OAuth
 from fastapi import APIRouter, Depends, Request, status
+from fastapi.responses import JSONResponse
 from fastapi.templating import Jinja2Templates
 from sqlalchemy.orm import Session
 from starlette.responses import RedirectResponse
@@ -81,6 +83,17 @@ def require_login(func):
     @wraps(func)
     async def wrapper(request: Request, *args, **kwargs):
         if not request.session.get("user"):
+            # For API endpoints return 401 instead of storing the URL in the session
+            # and redirecting to /login.  Without this guard, the /api/auth/whoami
+            # probe issued by common.js on every page load would overwrite
+            # redirect_after_login with the API URL, causing the post-login redirect
+            # to land on a JSON endpoint rather than the original page.
+            url_path = urlparse(str(request.url)).path
+            if url_path.startswith("/api/"):
+                return JSONResponse(
+                    status_code=status.HTTP_401_UNAUTHORIZED,
+                    content={"error": "Not authenticated"},
+                )
             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

tests/test_auth.py

@@ -175,6 +175,61 @@ class TestRequireLogin:
             assert result["message"] == "sync"
             assert result["param"] == "test_value"
 
+    @pytest.mark.asyncio
+    async def test_returns_401_for_api_paths_when_not_authenticated(self):
+        """Test that require_login returns 401 (not redirect) for /api/* paths.
+
+        This prevents the /api/auth/whoami JS probe from overwriting
+        redirect_after_login with an API URL, which would send the user to a
+        JSON endpoint after login instead of the page they actually wanted.
+        """
+        from fastapi.responses import JSONResponse
+
+        with patch("app.auth.AUTH_ENABLED", True):
+            from app.auth import require_login
+
+            @require_login
+            async def api_endpoint(request: Request):
+                return {"message": "success"}
+
+            mock_request = MagicMock(spec=Request)
+            mock_request.session = {}
+            mock_request.url = MagicMock()
+            mock_request.url.__str__ = MagicMock(return_value="http://test.com/api/auth/whoami")
+
+            result = await api_endpoint(mock_request)
+
+            assert isinstance(result, JSONResponse)
+            assert result.status_code == status.HTTP_401_UNAUTHORIZED
+            # Redirect URL must NOT be stored for API paths
+            assert "redirect_after_login" not in mock_request.session
+
+    @pytest.mark.asyncio
+    async def test_does_not_save_redirect_for_api_paths(self):
+        """Test that redirect_after_login is never set for any /api/* request."""
+        from fastapi.responses import JSONResponse
+
+        with patch("app.auth.AUTH_ENABLED", True):
+            from app.auth import require_login
+
+            @require_login
+            async def api_endpoint(request: Request):
+                return {"data": "ok"}
+
+            for api_path in ["/api/documents/upload", "/api/v1/resource", "/api/users/me"]:
+                mock_request = MagicMock(spec=Request)
+                mock_request.session = {}
+                mock_request.url = MagicMock()
+                mock_request.url.__str__ = MagicMock(return_value=f"http://test.com{api_path}")
+
+                result = await api_endpoint(mock_request)
+
+                assert isinstance(result, JSONResponse), f"Expected JSONResponse for {api_path}"
+                assert result.status_code == status.HTTP_401_UNAUTHORIZED
+                assert "redirect_after_login" not in mock_request.session, (
+                    f"redirect_after_login must not be set for {api_path}"
+                )
+
 
 @pytest.mark.integration
 class TestWhoamiEndpoint:

tests/test_auth_module.py

@@ -185,6 +185,33 @@ class TestRequireLogin:
             assert isinstance(result, RedirectResponse)
             assert result.status_code == status.HTTP_302_FOUND
 
+    @pytest.mark.asyncio
+    async def test_returns_401_for_api_path_when_not_authenticated(self):
+        """Test returns 401 for /api/* paths instead of redirect-to-login.
+
+        Prevents the common.js /api/auth/whoami probe from overwriting
+        redirect_after_login, which would send the user to a JSON endpoint
+        after login instead of the page they originally requested.
+        """
+        from fastapi.responses import JSONResponse
+
+        with patch("app.auth.AUTH_ENABLED", True):
+
+            @require_login
+            async def test_api_endpoint(request: Request):
+                return {"data": "ok"}
+
+            mock_request = MagicMock(spec=Request)
+            mock_request.session = {}
+            mock_request.url = MagicMock()
+            mock_request.url.__str__ = MagicMock(return_value="http://localhost/api/auth/whoami")
+
+            result = await test_api_endpoint(mock_request)
+
+            assert isinstance(result, JSONResponse)
+            assert result.status_code == status.HTTP_401_UNAUTHORIZED
+            assert "redirect_after_login" not in mock_request.session
+
 
 @pytest.mark.unit
 class TestOAuthConfiguration: