Skip to content

Commit 57f4da5

Browse files
Clément VALENTINclaude
andcommitted
fix: address Copilot code review feedback
- settings.py: Improve COOKIE_DOMAIN comment for clarity - accounts.py: Add secure/samesite params to delete_cookie for reliable deletion - accounts.py: Clarify max_age comment (43200 min = 30 days) - main.py: Add FRONTEND_URL validation before adding to CORS origins - values.yaml: Clarify cookie.secure usage for production vs dev - CHANGELOG.md: Add Security section for httpOnly cookie migration - ApiDocs.tsx: Add comment explaining Swagger UI v5 uses Fetch API 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 0f0242c commit 57f4da5

File tree

6 files changed

+19
-8
lines changed

6 files changed

+19
-8
lines changed

apps/api/src/config/settings.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ def database_type(self) -> str:
5555
# Cookie settings
5656
COOKIE_SECURE: bool = False # Set True in production (HTTPS only)
5757
COOKIE_SAMESITE: Literal["lax", "strict", "none"] = "lax"
58-
COOKIE_DOMAIN: str = "" # Empty = current domain only
58+
COOKIE_DOMAIN: str = "" # Empty = browser uses current host. If set (e.g. ".example.com"), must match frontend domain
5959

6060
# Mailgun Email
6161
MAILGUN_API_KEY: str = ""

apps/api/src/main.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,15 @@ def get_servers() -> list[dict[str, str]]:
113113
# CORS middleware - explicit origins required for credentials (httpOnly cookies)
114114
def get_cors_origins() -> list[str]:
115115
"""Build CORS origins from settings"""
116-
origins = [settings.FRONTEND_URL]
116+
origins: list[str] = []
117+
# Add frontend URL if configured
118+
frontend_url = (settings.FRONTEND_URL or "").strip()
119+
if frontend_url:
120+
origins.append(frontend_url)
117121
# Add backend URL for Swagger UI
118-
if settings.BACKEND_URL and settings.BACKEND_URL != settings.FRONTEND_URL:
119-
origins.append(settings.BACKEND_URL)
122+
backend_url = (settings.BACKEND_URL or "").strip()
123+
if backend_url and backend_url != frontend_url:
124+
origins.append(backend_url)
120125
# Add common development origins
121126
if settings.DEBUG:
122127
origins.extend([

apps/api/src/routers/accounts.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ def _set_auth_cookie(response: Response, token: str) -> None:
155155
httponly=True, # Prevents JavaScript access (XSS protection)
156156
secure=settings.COOKIE_SECURE, # HTTPS only in production
157157
samesite=settings.COOKIE_SAMESITE, # CSRF protection
158-
max_age=settings.ACCESS_TOKEN_EXPIRE_MINUTES * 60, # Convert to seconds
158+
max_age=settings.ACCESS_TOKEN_EXPIRE_MINUTES * 60, # 43200 min = 30 days in seconds
159159
path="/", # Available for all paths
160160
domain=settings.COOKIE_DOMAIN or None, # None = current domain
161161
)
@@ -167,6 +167,8 @@ def _clear_auth_cookie(response: Response) -> None:
167167
key="access_token",
168168
path="/",
169169
domain=settings.COOKIE_DOMAIN or None,
170+
secure=settings.COOKIE_SECURE,
171+
samesite=settings.COOKIE_SAMESITE,
170172
)
171173

172174

apps/web/src/pages/ApiDocs.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -890,8 +890,8 @@ export default function ApiDocs() {
890890
displayRequestDuration={true}
891891
deepLinking={false}
892892
requestInterceptor={(req) => {
893-
// Credentials (httpOnly cookie) will be sent automatically
894-
// when withCredentials is enabled on fetch requests
893+
// Swagger UI v5+ uses Fetch API, so req.credentials = 'include' is correct
894+
// This enables httpOnly cookie to be sent with requests
895895
req.credentials = 'include'
896896
return req
897897
}}

helm/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## [1.0.0-dev.3](https://github.com/MyElectricalData/myelectricaldata_new/compare/helm/1.0.0-dev.2...helm/1.0.0-dev.3) (2025-12-21)
44

5+
### Security
6+
7+
* **auth:** migrate JWT storage from localStorage to httpOnly cookies (XSS protection)
8+
59
### Features
610

711
* add admin data sharing for PDL debugging ([cfb32b8](https://github.com/MyElectricalData/myelectricaldata_new/commit/cfb32b83ddb435c9c792aec031ead2ad2ae054ab))

helm/myelectricaldata/values.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ config:
177177

178178
# Cookie settings for httpOnly JWT authentication
179179
cookie:
180-
secure: true # Set to true for HTTPS (recommended for production)
180+
secure: true # true for HTTPS (production), false for HTTP (local dev)
181181
sameSite: "lax" # "lax", "strict", or "none" (lax recommended)
182182
domain: "" # Cookie domain (empty = current domain only)
183183

0 commit comments

Comments
 (0)