fix(api): logout flow in the dashboard (#1074)

inimaz · davidberenstein1957 · commit 07d5c67a2ae7 · 2026-02-22T22:56:13.000+01:00
* fix(api): logout flow in the dashboard

* fix:allow only authenticated users to logout

* test: fix test_authenticate

* fix: move create_redirect_response to auth_provider
diff --git a/carbonserver/carbonserver/api/routers/authenticate.py b/carbonserver/carbonserver/api/routers/authenticate.py
@@ -107,18 +107,7 @@ async def get_login(
         if settings.frontend_url != "":
             base_url = settings.frontend_url + "/"
         url = f"{base_url}home?auth=true&creds={creds}"
-
-        # NOTE: RedirectResponse doesn't work with clevercloud
-        # response = RedirectResponse(url=url)
-        content = f"""<html>
-        <head>
-        <script>
-        window.location.href = "{url}";
-        </script>
-        </head>
-        </html>
-        """
-        response = Response(content=content)
+        response = auth_provider.create_redirect_response(url)
 
         response.set_cookie(
             SESSION_COOKIE_NAME,
@@ -128,3 +117,28 @@ async def get_login(
         )
         return response
     return await auth_provider.get_authorize_url(request, str(login_url))
+
+
+@router.get("/auth/logout", name="logout")
+@inject
+async def logout(
+    request: Request,
+    response: Response,
+    auth_user: UserWithAuthDependency = Depends(UserWithAuthDependency),
+    auth_provider: Optional[OIDCAuthProvider] = Depends(
+        Provide[ServerContainer.auth_provider]
+    ),
+):
+    """
+    Logout user by clearing session and removing cookie
+    """
+    if auth_provider is None:
+        raise HTTPException(status_code=501, detail="Authentication not configured")
+    base_url = request.base_url
+    response = auth_provider.create_redirect_response(str(base_url))
+    response.delete_cookie(SESSION_COOKIE_NAME)
+    if hasattr(request, "session"):
+        request.session.clear()
+
+    # TODO: also revoke the token at auth provider level if possible
+    return response
diff --git a/carbonserver/carbonserver/api/services/auth_providers/oidc_auth_provider.py b/carbonserver/carbonserver/api/services/auth_providers/oidc_auth_provider.py
@@ -11,6 +11,7 @@
 from authlib.integrations.starlette_client import OAuth
 from authlib.jose import JsonWebKey
 from authlib.jose import jwt as jose_jwt
+from fastapi import Response
 from fief_client import FiefAsync
 
 from carbonserver.config import settings
@@ -81,3 +82,20 @@ async def validate_access_token(self, token: str) -> bool:
     async def get_user_info(self, access_token: str) -> Dict[str, Any]:
         decoded_token = await self._decode_token(access_token)
         return decoded_token
+
+    @staticmethod
+    def create_redirect_response(url: str) -> Response:
+        """RedirectResponse doesn't work with clevercloud, so we return a HTML page with a script to redirect the user
+
+        Ideally we should be able to do `response = RedirectResponse(url=url)` instead.
+        """
+        content = f"""<html>
+            <head>
+            <script>
+            window.location.href = "{url}";
+            </script>
+            </head>
+            </html>
+            """
+        response = Response(content=content)
+        return response
diff --git a/carbonserver/tests/api/routers/test_authenticate.py b/carbonserver/tests/api/routers/test_authenticate.py
@@ -0,0 +1,57 @@
+import pytest
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+from starlette.middleware.sessions import SessionMiddleware
+
+from carbonserver.api.routers import authenticate
+from carbonserver.container import ServerContainer
+
+SESSION_COOKIE_NAME = "user_session"
+
+
+@pytest.fixture
+def custom_test_server():
+    container = ServerContainer()
+    container.wire(modules=[authenticate])
+    app = FastAPI()
+    app.container = container
+    app.add_middleware(SessionMiddleware, secret_key="test-secret-key")
+    app.include_router(authenticate.router)
+    yield app
+
+
+@pytest.fixture
+def client(custom_test_server):
+    yield TestClient(custom_test_server)
+
+
+def test_logout_clears_cookie_and_session(client, monkeypatch):
+    class DummySession(dict):
+        def clear(self):
+            self["cleared"] = True
+
+    dummy_session = DummySession()
+
+    def fake_request():
+        class FakeRequest:
+            base_url = "http://testserver/"
+            session = dummy_session
+
+        return FakeRequest()
+
+    monkeypatch.setattr("carbonserver.api.routers.authenticate.Request", fake_request)
+
+    # Set cookie and session in request
+    cookies = {SESSION_COOKIE_NAME: "dummy_token"}
+    with client as c:
+        # Set session data by making a request that sets session
+        c.cookies.set(SESSION_COOKIE_NAME, "dummy_token")
+        # There is no direct way to set session data before logout, so just call logout
+        response = c.get("/auth/logout", cookies=cookies)
+        assert response.status_code == 200
+        assert (
+            SESSION_COOKIE_NAME not in response.cookies
+            or response.cookies.get(SESSION_COOKIE_NAME) == ""
+        )
+        # We cannot directly check session cleared, but can check that logout returns redirect
+        assert "window.location.href" in response.text
diff --git a/pyproject.toml b/pyproject.toml
@@ -104,6 +104,7 @@ api  = [
     "rapidfuzz",
     "PyJWT",
     "logfire[fastapi]>=1.0.1",
+    "itsdangerous",
 ]
 dev = [
     "taskipy",
diff --git a/webapp/src/components/navbar.tsx b/webapp/src/components/navbar.tsx
@@ -268,7 +268,7 @@ export default function NavBar({
                         <NavItem
                             onClick={() => {
                                 setSheetOpened?.(false);
-                                router.push("/logout");
+                                router.push("/auth/logout");
                             }}
                             isSelected={false}
                             paddingY={1.5}

Original file line number	Diff line number	Diff line change
`@@ -104,6 +104,7 @@ api = [`
`104`	`104`	`"rapidfuzz",`
`105`	`105`	`"PyJWT",`
`106`	`106`	`"logfire[fastapi]>=1.0.1",`
	`107`	`+ "itsdangerous",`
`107`	`108`	`]`
`108`	`109`	`dev = [`
`109`	`110`	`"taskipy",`