From 98a161574eaf923ea3e9d680cd879f235f19d7c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=A8=B8=EB=8B=88=ED=8E=98=EB=8B=88?= Date: Wed, 18 Mar 2026 22:30:47 +0900 Subject: [PATCH] security: migrate JWT from localStorage to httpOnly cookie Eliminates XSS token theft by storing JWT in httpOnly Secure cookie instead of localStorage. Backend sets cookie on login and clears on logout. Token extraction uses cookie-first with Authorization header fallback for backward compatibility with existing tests. Co-Authored-By: Claude Opus 4.6 --- backend/app/api/auth.py | 29 ++++++++++++++--- backend/app/api/deps.py | 19 ++++++++--- frontend/src/components/layout/new-header.tsx | 4 +-- frontend/src/lib/api.ts | 32 +++---------------- 4 files changed, 46 insertions(+), 38 deletions(-) diff --git a/backend/app/api/auth.py b/backend/app/api/auth.py index 9fbf0b1..fb668da 100644 --- a/backend/app/api/auth.py +++ b/backend/app/api/auth.py @@ -5,6 +5,7 @@ from datetime import timedelta from typing import Annotated from fastapi import APIRouter, Depends, HTTPException, status +from fastapi.responses import JSONResponse from sqlalchemy.orm import Session from app.core.database import get_db @@ -22,7 +23,7 @@ router = APIRouter(prefix="/api/auth", tags=["auth"]) settings = get_settings() -@router.post("/login", response_model=Token) +@router.post("/login") async def login( login_data: LoginRequest, db: Annotated[Session, Depends(get_db)], @@ -42,7 +43,19 @@ async def login( expires_delta=timedelta(minutes=settings.access_token_expire_minutes), ) - return Token(access_token=access_token) + response = JSONResponse( + content={"access_token": access_token, "token_type": "bearer"}, + ) + response.set_cookie( + key="access_token", + value=access_token, + httponly=True, + samesite="lax", + secure=False, # Set True in production behind HTTPS + path="/", + max_age=settings.access_token_expire_minutes * 60, + ) + return response @router.post("/register", response_model=UserResponse, status_code=status.HTTP_201_CREATED) @@ -65,5 +78,13 @@ async def get_current_user_info(current_user: CurrentUser): @router.post("/logout") async def logout(): - """Logout (client should discard token).""" - return {"message": "Successfully logged out"} + """Logout by clearing the access_token cookie.""" + response = JSONResponse(content={"message": "Successfully logged out"}) + response.delete_cookie( + key="access_token", + httponly=True, + samesite="lax", + secure=False, + path="/", + ) + return response diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index f262b11..8e896ba 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -1,9 +1,9 @@ """ API dependencies. """ -from typing import Annotated +from typing import Annotated, Optional -from fastapi import Depends, HTTPException, status +from fastapi import Cookie, Depends, HTTPException, Request, status from fastapi.security import OAuth2PasswordBearer from sqlalchemy.orm import Session @@ -11,20 +11,29 @@ from app.core.database import get_db from app.core.security import decode_access_token from app.models.user import User -oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/api/auth/login") +oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/api/auth/login", auto_error=False) async def get_current_user( + request: Request, db: Annotated[Session, Depends(get_db)], - token: Annotated[str, Depends(oauth2_scheme)], + bearer_token: Annotated[Optional[str], Depends(oauth2_scheme)] = None, ) -> User: - """Get the current authenticated user.""" + """Get the current authenticated user. + + Token extraction order: httpOnly cookie first, then Authorization header fallback. + """ credentials_exception = HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Could not validate credentials", headers={"WWW-Authenticate": "Bearer"}, ) + # Cookie first, then Authorization header fallback + token = request.cookies.get("access_token") or bearer_token + if token is None: + raise credentials_exception + payload = decode_access_token(token) if payload is None: raise credentials_exception diff --git a/frontend/src/components/layout/new-header.tsx b/frontend/src/components/layout/new-header.tsx index be45981..ebf68d8 100644 --- a/frontend/src/components/layout/new-header.tsx +++ b/frontend/src/components/layout/new-header.tsx @@ -45,8 +45,8 @@ export function NewHeader({ username, onMenuClick, showMenuButton = false }: New const router = useRouter(); const pageTitle = getPageTitle(pathname); - const handleLogout = () => { - api.logout(); + const handleLogout = async () => { + await api.logout(); router.push('/login'); }; diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 2e51011..7538076 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -10,27 +10,9 @@ async function hashPassword(password: string): Promise { class ApiClient { private baseUrl: string; - private token: string | null = null; constructor(baseUrl: string) { this.baseUrl = baseUrl; - if (typeof window !== 'undefined') { - this.token = localStorage.getItem('token'); - } - } - - setToken(token: string) { - this.token = token; - if (typeof window !== 'undefined') { - localStorage.setItem('token', token); - } - } - - clearToken() { - this.token = null; - if (typeof window !== 'undefined') { - localStorage.removeItem('token'); - } } private async request( @@ -42,13 +24,10 @@ class ApiClient { ...options.headers, }; - if (this.token) { - (headers as Record)['Authorization'] = `Bearer ${this.token}`; - } - const response = await fetch(`${this.baseUrl}${endpoint}`, { ...options, headers, + credentials: 'include', }); if (!response.ok) { @@ -89,6 +68,7 @@ class ApiClient { headers: { 'Content-Type': 'application/json', }, + credentials: 'include', body: JSON.stringify({ username, password: hashedPassword }), }); @@ -97,13 +77,11 @@ class ApiClient { throw new Error(error.detail || 'Login failed'); } - const data = await response.json(); - this.setToken(data.access_token); - return data; + return response.json(); } - logout() { - this.clearToken(); + async logout() { + await this.post('/api/auth/logout'); } async getCurrentUser() {