diff --git a/api/app/app/modules/auth/router.py b/api/app/app/modules/auth/router.py index 4b8e888..0795f45 100644 --- a/api/app/app/modules/auth/router.py +++ b/api/app/app/modules/auth/router.py @@ -1,6 +1,6 @@ import secrets -from fastapi import APIRouter, Depends, HTTPException, status +from fastapi import APIRouter, Depends, HTTPException, Request, Response, status from fastapi.responses import RedirectResponse from sqlalchemy.orm import Session @@ -28,13 +28,12 @@ EmailVerificationConfirm, EmailVerificationRequest, LoginRequest, - LogoutRequest, LogoutResponse, PasswordResetConfirm, PasswordResetRequest, - RefreshRequest, SimpleStatusResponse, Token, + TokenResponse, ) from app.modules.auth.service import ( get_or_create_oauth_user, @@ -49,12 +48,31 @@ router = APIRouter(prefix="/auth", tags=["auth"]) +REFRESH_COOKIE_NAME = "refresh_token" +# Scoped to /api/auth so it's only ever sent on refresh/logout, not on +# every request to the API. +REFRESH_COOKIE_PATH = "/api/auth" -def _oauth_redirect_url(token: Token, invite_token: str | None) -> str: + +def _set_refresh_cookie(response: Response, refresh_token: str) -> None: + response.set_cookie( + key=REFRESH_COOKIE_NAME, + value=refresh_token, + max_age=settings.refresh_token_expire_days * 24 * 60 * 60, + httponly=True, + secure=settings.environment == "production", + samesite="lax", + path=REFRESH_COOKIE_PATH, + ) + + +def _clear_refresh_cookie(response: Response) -> None: + response.delete_cookie(key=REFRESH_COOKIE_NAME, path=REFRESH_COOKIE_PATH) + + +def _oauth_redirect_url(access_token: str, invite_token: str | None) -> str: base = f"{settings.frontend_url.rstrip('/')}/auth/callback" - params = [f"token={token.access_token}"] - if token.refresh_token: - params.append(f"refresh_token={token.refresh_token}") + params = [f"token={access_token}"] if invite_token: params.append(f"invite_token={invite_token}") return f"{base}?{'&'.join(params)}" @@ -67,9 +85,10 @@ def get_current_user_profile( return to_user_response(current_user) -@router.post("/login", response_model=Token) +@router.post("/login", response_model=TokenResponse) def login_endpoint( data: LoginRequest, + response: Response, db: Session = Depends(get_db), _rl: None = Depends(enforce_login_rate_limit), ) -> Token: @@ -79,23 +98,36 @@ def login_endpoint( status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid email or password", ) + if token.refresh_token: + _set_refresh_cookie(response, token.refresh_token) return token -@router.post("/refresh", response_model=Token) -def refresh_endpoint(data: RefreshRequest) -> Token: - token = rotate_refresh_token(data.refresh_token) +@router.post("/refresh", response_model=TokenResponse) +def refresh_endpoint(request: Request, response: Response) -> Token: + refresh_token = request.cookies.get(REFRESH_COOKIE_NAME) + if not refresh_token: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Missing refresh token", + ) + token = rotate_refresh_token(refresh_token) if not token: + _clear_refresh_cookie(response) raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid or expired refresh token", ) + if token.refresh_token: + _set_refresh_cookie(response, token.refresh_token) return token @router.post("/logout", response_model=LogoutResponse) -def logout_endpoint(data: LogoutRequest) -> LogoutResponse: - logout(data.refresh_token) +def logout_endpoint(request: Request, response: Response) -> LogoutResponse: + refresh_token = request.cookies.get(REFRESH_COOKIE_NAME) + logout(refresh_token) + _clear_refresh_cookie(response) return LogoutResponse() @@ -205,7 +237,12 @@ async def github_callback( token_pair = issue_token_pair(str(user.id)) raw_invite = state_data.get("invite_token") invite_token = raw_invite if isinstance(raw_invite, str) and raw_invite else None - return RedirectResponse(url=_oauth_redirect_url(token_pair, invite_token)) + redirect = RedirectResponse( + url=_oauth_redirect_url(token_pair.access_token, invite_token) + ) + if token_pair.refresh_token: + _set_refresh_cookie(redirect, token_pair.refresh_token) + return redirect @router.get("/google") @@ -246,4 +283,9 @@ async def google_callback( token_pair = issue_token_pair(str(user.id)) raw_invite = state_data.get("invite_token") invite_token = raw_invite if isinstance(raw_invite, str) and raw_invite else None - return RedirectResponse(url=_oauth_redirect_url(token_pair, invite_token)) + redirect = RedirectResponse( + url=_oauth_redirect_url(token_pair.access_token, invite_token) + ) + if token_pair.refresh_token: + _set_refresh_cookie(redirect, token_pair.refresh_token) + return redirect diff --git a/api/app/app/modules/auth/schemas.py b/api/app/app/modules/auth/schemas.py index cff050c..0e678ac 100644 --- a/api/app/app/modules/auth/schemas.py +++ b/api/app/app/modules/auth/schemas.py @@ -2,22 +2,27 @@ class Token(BaseModel): + """Internal shape returned by the auth service layer. Never sent to + the client as-is -- routes use TokenResponse (no refresh_token) and + set the refresh token as an httpOnly cookie instead.""" + access_token: str token_type: str = "bearer" refresh_token: str | None = None -class LoginRequest(BaseModel): - email: EmailStr - password: str - +class TokenResponse(BaseModel): + """What actually goes in the JSON body of login/refresh/OAuth + responses. The refresh token travels only via the httpOnly + `refresh_token` cookie, never in a response client-side JS can read.""" -class RefreshRequest(BaseModel): - refresh_token: str + access_token: str + token_type: str = "bearer" -class LogoutRequest(BaseModel): - refresh_token: str | None = None +class LoginRequest(BaseModel): + email: EmailStr + password: str class LogoutResponse(BaseModel): diff --git a/apps/frontend/src/components/layout/DashboardLayout.tsx b/apps/frontend/src/components/layout/DashboardLayout.tsx index a288d24..eeba841 100644 --- a/apps/frontend/src/components/layout/DashboardLayout.tsx +++ b/apps/frontend/src/components/layout/DashboardLayout.tsx @@ -9,8 +9,6 @@ import { useWorkspaces } from "@/hooks/useWorkspaces"; import { useAuthStore } from "@/stores/authStore"; import { useDashboardStore } from "@/stores/dashboardStore"; -const API_BASE = import.meta.env.VITE_API_URL || "http://localhost:8000"; - export interface DashboardOutletContext { workspaces: { id: string; name: string; owner_id: string }[]; selectedWorkspaceId: string | null; @@ -58,9 +56,7 @@ export function DashboardLayout() { navigate("/login"); return; } - fetch(`${API_BASE}/api/auth/me`, { - headers: { Authorization: `Bearer ${token}` }, - }) + apiFetch("/api/auth/me") .then((res) => { if (!res.ok) { logout(); diff --git a/apps/frontend/src/lib/api.test.ts b/apps/frontend/src/lib/api.test.ts index 9ec1c10..1de81a0 100644 --- a/apps/frontend/src/lib/api.test.ts +++ b/apps/frontend/src/lib/api.test.ts @@ -15,7 +15,6 @@ describe("apiFetch refresh error handling", () => { vi.useFakeTimers(); useAuthStore.setState({ token: "old-access-token", - refreshToken: "refresh-token-1", user: null, }); }); @@ -43,8 +42,9 @@ describe("apiFetch refresh error handling", () => { expect(result.status).toBe(401); // A network-level failure during refresh must NOT be treated as a - // confirmed-dead session: auth state should be untouched. - expect(useAuthStore.getState().refreshToken).toBe("refresh-token-1"); + // confirmed-dead session: the access token should be untouched. The + // refresh token itself lives only in an httpOnly cookie the browser + // manages -- there's nothing client-side to assert on for it. expect(useAuthStore.getState().token).toBe("old-access-token"); expect(fetchMock).toHaveBeenCalledTimes(3); }); @@ -59,7 +59,6 @@ describe("apiFetch refresh error handling", () => { const result = await apiFetch("/api/boards"); expect(result.status).toBe(401); - expect(useAuthStore.getState().refreshToken).toBeNull(); expect(useAuthStore.getState().token).toBeNull(); }); @@ -68,12 +67,7 @@ describe("apiFetch refresh error handling", () => { .fn() .mockResolvedValueOnce(new Response(null, { status: 401 })) .mockRejectedValueOnce(new TypeError("network error")) - .mockResolvedValueOnce( - jsonResponse({ - access_token: "new-token", - refresh_token: "refresh-token-2", - }), - ) + .mockResolvedValueOnce(jsonResponse({ access_token: "new-token" })) .mockResolvedValueOnce(new Response(null, { status: 200 })); vi.stubGlobal("fetch", fetchMock); @@ -83,6 +77,17 @@ describe("apiFetch refresh error handling", () => { expect(result.status).toBe(200); expect(useAuthStore.getState().token).toBe("new-token"); - expect(useAuthStore.getState().refreshToken).toBe("refresh-token-2"); + }); + + it("sends credentials: include so the httpOnly refresh_token cookie is attached", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(new Response(null, { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + + await apiFetch("/api/boards"); + + const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(init.credentials).toBe("include"); }); }); diff --git a/apps/frontend/src/lib/api.ts b/apps/frontend/src/lib/api.ts index 15dca06..d094997 100644 --- a/apps/frontend/src/lib/api.ts +++ b/apps/frontend/src/lib/api.ts @@ -59,14 +59,13 @@ async function refreshAccessToken(): Promise { if (refreshInFlight) return refreshInFlight; const run = async (): Promise => { - const refreshToken = useAuthStore.getState().refreshToken; - if (!refreshToken) return null; - + // The refresh token itself is never visible to JS -- it lives only + // in the httpOnly `refresh_token` cookie (scoped to /api/auth) and + // is sent automatically by the browser via credentials: "include". const attemptRefresh = () => fetch(`${API_BASE}/api/auth/refresh`, { method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ refresh_token: refreshToken }), + credentials: "include", }); let res: Response; @@ -91,22 +90,17 @@ async function refreshAccessToken(): Promise { if (!res.ok) { // A real response from the server saying the refresh token itself - // is invalid/expired -- this is a confirmed-dead session. + // is invalid/expired (or there was no cookie at all) -- this is a + // confirmed-dead session. useAuthStore.getState().logout(); return null; } - const data: { - access_token?: string; - refresh_token?: string | null; - } = await res.json(); + const data: { access_token?: string } = await res.json(); if (!data.access_token) { useAuthStore.getState().logout(); return null; } - useAuthStore.getState().setTokens({ - token: data.access_token, - refreshToken: data.refresh_token ?? null, - }); + useAuthStore.getState().setToken(data.access_token); return data.access_token; }; @@ -117,17 +111,13 @@ async function refreshAccessToken(): Promise { } export async function logoutAndClear(): Promise { - const refreshToken = useAuthStore.getState().refreshToken; - if (refreshToken) { - try { - await fetch(`${API_BASE}/api/auth/logout`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ refresh_token: refreshToken }), - }); - } catch { - // Clear locally even if the server call fails; refresh TTL bounds damage. - } + try { + await fetch(`${API_BASE}/api/auth/logout`, { + method: "POST", + credentials: "include", + }); + } catch { + // Clear locally even if the server call fails; refresh TTL bounds damage. } useAuthStore.getState().logout(); } @@ -139,19 +129,20 @@ export async function apiFetch( const token = getToken(); const response = await fetch(`${API_BASE}${path}`, { ...options, + credentials: "include", headers: buildHeaders(options.headers, token), }); - // Skip the refresh path itself to avoid infinite recursion. - if ( - response.status === 401 && - !path.startsWith("/api/auth/refresh") && - useAuthStore.getState().refreshToken - ) { + // Skip the refresh path itself to avoid infinite recursion. There's no + // client-visible way to know whether a refresh_token cookie exists, so + // any other 401 is worth one refresh attempt -- the backend just + // returns 401 again immediately if there's no cookie to use. + if (response.status === 401 && !path.startsWith("/api/auth/refresh")) { const fresh = await refreshAccessToken(); if (fresh) { return fetch(`${API_BASE}${path}`, { ...options, + credentials: "include", headers: buildHeaders(options.headers, fresh), }); } diff --git a/apps/frontend/src/pages/auth/AuthCallbackPage.tsx b/apps/frontend/src/pages/auth/AuthCallbackPage.tsx index d91566b..74f2336 100644 --- a/apps/frontend/src/pages/auth/AuthCallbackPage.tsx +++ b/apps/frontend/src/pages/auth/AuthCallbackPage.tsx @@ -1,6 +1,7 @@ /** - * OAuth callback — API redirects here with ?token=...&refresh_token=... - * Stores the token pair and navigates to the dashboard. + * OAuth callback — API redirects here with ?token=... . The refresh + * token is never in this URL: the backend sets it as an httpOnly cookie + * directly on the redirect response, before the browser ever lands here. */ import { useEffect } from "react"; import { useNavigate, useSearchParams } from "react-router-dom"; @@ -10,9 +11,8 @@ import { useAuthStore } from "@/stores/authStore"; export function AuthCallbackPage() { const [searchParams] = useSearchParams(); const navigate = useNavigate(); - const setTokens = useAuthStore((s) => s.setTokens); + const setToken = useAuthStore((s) => s.setToken); const token = searchParams.get("token"); - const refreshToken = searchParams.get("refresh_token"); const inviteToken = searchParams.get("invite_token"); const API_BASE = import.meta.env.VITE_API_URL || "http://localhost:8000"; @@ -22,7 +22,7 @@ export function AuthCallbackPage() { return; } - setTokens({ token, refreshToken: refreshToken ?? null }); + setToken(token); const acceptMaybe = async () => { if (inviteToken) { await fetch( @@ -39,7 +39,7 @@ export function AuthCallbackPage() { navigate("/dashboard", { replace: true }); }; void acceptMaybe(); - }, [token, refreshToken, inviteToken, navigate, setTokens, API_BASE]); + }, [token, inviteToken, navigate, setToken, API_BASE]); return (
diff --git a/apps/frontend/src/pages/auth/LoginPage.tsx b/apps/frontend/src/pages/auth/LoginPage.tsx index 241d4c2..13812f4 100644 --- a/apps/frontend/src/pages/auth/LoginPage.tsx +++ b/apps/frontend/src/pages/auth/LoginPage.tsx @@ -13,7 +13,7 @@ export function LoginPage() { const { t } = useI18n(); const navigate = useNavigate(); const [searchParams] = useSearchParams(); - const setTokens = useAuthStore((s) => s.setTokens); + const setToken = useAuthStore((s) => s.setToken); const [email, setEmail] = useState(""); const [password, setPassword] = useState(""); const [error, setError] = useState(""); @@ -26,6 +26,7 @@ export function LoginPage() { try { const res = await fetch(`${API_BASE}/api/auth/login`, { method: "POST", + credentials: "include", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ email, password }), }); @@ -34,10 +35,7 @@ export function LoginPage() { setError(formatApiError(data.detail, "Login failed")); return; } - setTokens({ - token: data.access_token, - refreshToken: data.refresh_token ?? null, - }); + setToken(data.access_token); const inviteToken = searchParams.get("invite_token"); if (inviteToken) { const acceptRes = await fetch( diff --git a/apps/frontend/src/stores/authStore.ts b/apps/frontend/src/stores/authStore.ts index cfdb5d7..98ebc56 100644 --- a/apps/frontend/src/stores/authStore.ts +++ b/apps/frontend/src/stores/authStore.ts @@ -13,15 +13,12 @@ export interface AuthUser { } interface AuthState { + // Short-lived access token only. The refresh token lives exclusively + // in an httpOnly cookie set by the backend -- it's never readable by + // JS, so it has no place in this client-side store. token: string | null; - refreshToken: string | null; user: AuthUser | null; setToken: (token: string | null) => void; - setRefreshToken: (token: string | null) => void; - setTokens: (tokens: { - token: string | null; - refreshToken?: string | null; - }) => void; setUser: (user: AuthUser | null) => void; logout: () => void; } @@ -30,22 +27,14 @@ export const useAuthStore = create()( persist( (set) => ({ token: null, - refreshToken: null, user: null, setToken: (token) => set({ token }), - setRefreshToken: (refreshToken) => set({ refreshToken }), - setTokens: ({ token, refreshToken }) => - set((s) => ({ - token, - refreshToken: - refreshToken === undefined ? s.refreshToken : refreshToken, - })), setUser: (user) => set({ user }), - logout: () => set({ token: null, refreshToken: null, user: null }), + logout: () => set({ token: null, user: null }), }), { name: "loomy-auth", - partialize: (s) => ({ token: s.token, refreshToken: s.refreshToken }), + partialize: (s) => ({ token: s.token }), }, ), );