-
Notifications
You must be signed in to change notification settings - Fork 0
Develop #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Develop #91
Changes from 2 commits
842a943
577860e
eb6eff7
0f0242c
57f4da5
a3a2a12
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -110,13 +110,34 @@ def get_servers() -> list[dict[str, str]]: | |||||||||||
| "backend", | ||||||||||||
| ]) | ||||||||||||
|
|
||||||||||||
| # CORS middleware | ||||||||||||
| # CORS middleware - explicit origins required for credentials (httpOnly cookies) | ||||||||||||
| def get_cors_origins() -> list[str]: | ||||||||||||
| """Build CORS origins from settings""" | ||||||||||||
| origins = [settings.FRONTEND_URL] | ||||||||||||
|
||||||||||||
| origins = [settings.FRONTEND_URL] | |
| origins: list[str] = [] | |
| frontend_url = (settings.FRONTEND_URL or "").strip() | |
| if frontend_url: | |
| origins.append(frontend_url) |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |||||||||
| from typing import Optional | ||||||||||
|
|
||||||||||
| import httpx | ||||||||||
| from fastapi import APIRouter, Depends, HTTPException, status, Form, Request, Body, Query | ||||||||||
| from fastapi import APIRouter, Depends, HTTPException, status, Form, Request, Body, Query, Response | ||||||||||
| from sqlalchemy import select | ||||||||||
| from sqlalchemy.ext.asyncio import AsyncSession | ||||||||||
| from sqlalchemy.orm import selectinload | ||||||||||
|
|
@@ -18,7 +18,6 @@ | |||||||||
| UserLogin, | ||||||||||
| UserResponse, | ||||||||||
| ClientCredentials, | ||||||||||
| TokenResponse, | ||||||||||
| APIResponse, | ||||||||||
| ErrorDetail, | ||||||||||
| ) | ||||||||||
|
|
@@ -148,8 +147,32 @@ async def signup( | |||||||||
| ) | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def _set_auth_cookie(response: Response, token: str) -> None: | ||||||||||
| """Set httpOnly cookie with JWT token""" | ||||||||||
| response.set_cookie( | ||||||||||
| key="access_token", | ||||||||||
| value=token, | ||||||||||
| httponly=True, # Prevents JavaScript access (XSS protection) | ||||||||||
| secure=settings.COOKIE_SECURE, # HTTPS only in production | ||||||||||
| samesite=settings.COOKIE_SAMESITE, # CSRF protection | ||||||||||
| max_age=settings.ACCESS_TOKEN_EXPIRE_MINUTES * 60, # Convert to seconds | ||||||||||
|
||||||||||
| max_age=settings.ACCESS_TOKEN_EXPIRE_MINUTES * 60, # Convert to seconds | |
| max_age=settings.ACCESS_TOKEN_EXPIRE_MINUTES * 60, # Convert minutes to seconds (e.g. 43200 min = 30 days) |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The delete_cookie method should include the same 'secure' and 'samesite' parameters that were used when setting the cookie. Cookie deletion can fail if these parameters don't match the original cookie's settings. Add 'secure=settings.COOKIE_SECURE' and 'samesite=settings.COOKIE_SAMESITE' to ensure reliable cookie deletion.
| domain=settings.COOKIE_DOMAIN or None, | |
| domain=settings.COOKIE_DOMAIN or None, | |
| secure=settings.COOKIE_SECURE, | |
| samesite=settings.COOKIE_SAMESITE, |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||
| import SwaggerUI from 'swagger-ui-react' | ||||||||||
| import 'swagger-ui-react/swagger-ui.css' | ||||||||||
| import { Key } from 'lucide-react' | ||||||||||
| import { useEffect, useMemo } from 'react' | ||||||||||
| import { useEffect } from 'react' | ||||||||||
| import { useThemeStore } from '@/stores/themeStore' | ||||||||||
| import { Link } from 'react-router-dom' | ||||||||||
|
|
||||||||||
|
|
@@ -16,8 +16,6 @@ declare global { | |||||||||
| } | ||||||||||
|
|
||||||||||
| export default function ApiDocs() { | ||||||||||
| // Get the access token from localStorage | ||||||||||
| const accessToken = useMemo(() => localStorage.getItem('access_token'), []) | ||||||||||
| const { isDark } = useThemeStore() | ||||||||||
| // Use runtime config first, then build-time env, then default | ||||||||||
| const apiBaseUrl = window.__ENV__?.VITE_API_BASE_URL || import.meta.env.VITE_API_BASE_URL || '/api' | ||||||||||
|
|
@@ -892,18 +890,11 @@ export default function ApiDocs() { | |||||||||
| displayRequestDuration={true} | ||||||||||
| deepLinking={false} | ||||||||||
| requestInterceptor={(req) => { | ||||||||||
| // Add Authorization header with the access token | ||||||||||
| if (accessToken) { | ||||||||||
| req.headers.Authorization = `Bearer ${accessToken}` | ||||||||||
| } | ||||||||||
| // Credentials (httpOnly cookie) will be sent automatically | ||||||||||
| // when withCredentials is enabled on fetch requests | ||||||||||
| req.credentials = 'include' | ||||||||||
|
||||||||||
| // when withCredentials is enabled on fetch requests | |
| req.credentials = 'include' | |
| // when withCredentials is enabled on XHR requests | |
| req.withCredentials = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using httpOnly cookies with CORS, the COOKIE_DOMAIN setting can cause issues if not properly configured. If the domain is set to a parent domain (e.g., ".example.com"), but the frontend is on a subdomain, this might lead to cookie isolation issues. Consider adding validation or documentation to ensure COOKIE_DOMAIN matches the deployment architecture. Additionally, when COOKIE_DOMAIN is empty string, it's converted to None which is correct, but this behavior should be explicitly documented in the configuration comments.