Skip to content

Commit 91ec9ac

Browse files
inimazinimaz
authored
fix(api): logout flow in the dashboard (#1074)
* 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
1 parent 145fe99 commit 91ec9ac

5 files changed

Lines changed: 103 additions & 13 deletions

File tree

‎carbonserver/carbonserver/api/routers/authenticate.py‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -107,18 +107,7 @@ async def get_login(
107107
if settings.frontend_url != "":
108108
base_url = settings.frontend_url + "/"
109109
url = f"{base_url}home?auth=true&creds={creds}"
110-
111-
# NOTE: RedirectResponse doesn't work with clevercloud
112-
# response = RedirectResponse(url=url)
113-
content = f"""<html>
114-
<head>
115-
<script>
116-
window.location.href = "{url}";
117-
</script>
118-
</head>
119-
</html>
120-
"""
121-
response = Response(content=content)
110+
response = auth_provider.create_redirect_response(url)
122111

123112
response.set_cookie(
124113
SESSION_COOKIE_NAME,
@@ -128,3 +117,28 @@ async def get_login(
128117
)
129118
return response
130119
return await auth_provider.get_authorize_url(request, str(login_url))
120+
121+
122+
@router.get("/auth/logout", name="logout")
123+
@inject
124+
async def logout(
125+
request: Request,
126+
response: Response,
127+
auth_user: UserWithAuthDependency = Depends(UserWithAuthDependency),
128+
auth_provider: Optional[OIDCAuthProvider] = Depends(
129+
Provide[ServerContainer.auth_provider]
130+
),
131+
):
132+
"""
133+
Logout user by clearing session and removing cookie
134+
"""
135+
if auth_provider is None:
136+
raise HTTPException(status_code=501, detail="Authentication not configured")
137+
base_url = request.base_url
138+
response = auth_provider.create_redirect_response(str(base_url))
139+
response.delete_cookie(SESSION_COOKIE_NAME)
140+
if hasattr(request, "session"):
141+
request.session.clear()
142+
143+
# TODO: also revoke the token at auth provider level if possible
144+
return response

‎carbonserver/carbonserver/api/services/auth_providers/oidc_auth_provider.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from authlib.integrations.starlette_client import OAuth
1212
from authlib.jose import JsonWebKey
1313
from authlib.jose import jwt as jose_jwt
14+
from fastapi import Response
1415
from fief_client import FiefAsync
1516

1617
from carbonserver.config import settings
@@ -81,3 +82,20 @@ async def validate_access_token(self, token: str) -> bool:
8182
async def get_user_info(self, access_token: str) -> Dict[str, Any]:
8283
decoded_token = await self._decode_token(access_token)
8384
return decoded_token
85+
86+
@staticmethod
87+
def create_redirect_response(url: str) -> Response:
88+
"""RedirectResponse doesn't work with clevercloud, so we return a HTML page with a script to redirect the user
89+
90+
Ideally we should be able to do `response = RedirectResponse(url=url)` instead.
91+
"""
92+
content = f"""<html>
93+
<head>
94+
<script>
95+
window.location.href = "{url}";
96+
</script>
97+
</head>
98+
</html>
99+
"""
100+
response = Response(content=content)
101+
return response

‎carbonserver/tests/api/routers/test_authenticate.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import pytest
2+
from fastapi import FastAPI
3+
from fastapi.testclient import TestClient
4+
from starlette.middleware.sessions import SessionMiddleware
5+
6+
from carbonserver.api.routers import authenticate
7+
from carbonserver.container import ServerContainer
8+
9+
SESSION_COOKIE_NAME = "user_session"
10+
11+
12+
@pytest.fixture
13+
def custom_test_server():
14+
container = ServerContainer()
15+
container.wire(modules=[authenticate])
16+
app = FastAPI()
17+
app.container = container
18+
app.add_middleware(SessionMiddleware, secret_key="test-secret-key")
19+
app.include_router(authenticate.router)
20+
yield app
21+
22+
23+
@pytest.fixture
24+
def client(custom_test_server):
25+
yield TestClient(custom_test_server)
26+
27+
28+
def test_logout_clears_cookie_and_session(client, monkeypatch):
29+
class DummySession(dict):
30+
def clear(self):
31+
self["cleared"] = True
32+
33+
dummy_session = DummySession()
34+
35+
def fake_request():
36+
class FakeRequest:
37+
base_url = "http://testserver/"
38+
session = dummy_session
39+
40+
return FakeRequest()
41+
42+
monkeypatch.setattr("carbonserver.api.routers.authenticate.Request", fake_request)
43+
44+
# Set cookie and session in request
45+
cookies = {SESSION_COOKIE_NAME: "dummy_token"}
46+
with client as c:
47+
# Set session data by making a request that sets session
48+
c.cookies.set(SESSION_COOKIE_NAME, "dummy_token")
49+
# There is no direct way to set session data before logout, so just call logout
50+
response = c.get("/auth/logout", cookies=cookies)
51+
assert response.status_code == 200
52+
assert (
53+
SESSION_COOKIE_NAME not in response.cookies
54+
or response.cookies.get(SESSION_COOKIE_NAME) == ""
55+
)
56+
# We cannot directly check session cleared, but can check that logout returns redirect
57+
assert "window.location.href" in response.text

‎pyproject.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ api = [
104104
"rapidfuzz",
105105
"PyJWT",
106106
"logfire[fastapi]>=1.0.1",
107+
"itsdangerous",
107108
]
108109
dev = [
109110
"taskipy",

‎webapp/src/components/navbar.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ export default function NavBar({
268268
<NavItem
269269
onClick={() => {
270270
setSheetOpened?.(false);
271-
router.push("/logout");
271+
router.push("/auth/logout");
272272
}}
273273
isSelected={false}
274274
paddingY={1.5}

0 commit comments

Comments
 (0)