diff --git a/carbonserver/carbonserver/api/routers/authenticate.py b/carbonserver/carbonserver/api/routers/authenticate.py index a0b0089e1..bfd4c75c0 100644 --- 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""" - - - - - """ - 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 index ad17ad624..f1cc38a05 100644 --- 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""" + + + + + """ + response = Response(content=content) + return response diff --git a/carbonserver/tests/api/routers/test_authenticate.py b/carbonserver/tests/api/routers/test_authenticate.py new file mode 100644 index 000000000..b1296e4a1 --- /dev/null +++ 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 index f74617486..f4f489b58 100644 --- 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 index 2894a1a84..b8e4fe0e1 100644 --- a/webapp/src/components/navbar.tsx +++ b/webapp/src/components/navbar.tsx @@ -268,7 +268,7 @@ export default function NavBar({ { setSheetOpened?.(false); - router.push("/logout"); + router.push("/auth/logout"); }} isSelected={false} paddingY={1.5}