Develop #91

m4dm4rtig4n · 2025-12-21T00:00:51Z

No description provided.

Replace vulnerable localStorage token storage with secure httpOnly cookies to prevent XSS token theft attacks. Backend changes: - Add cookie configuration settings (secure, sameSite, domain) - Login sets httpOnly cookie instead of returning token in body - Add /accounts/logout endpoint to clear auth cookie - Auth middleware prioritizes cookie > Authorization header - CORS updated with explicit origins for credentials support Frontend changes: - Axios client uses withCredentials for automatic cookie handling - Remove all localStorage token references - Auth state checked via /accounts/me endpoint - Logout calls backend to clear cookie Helm chart: - Add config.cookie section with secure, sameSite, domain options - Backend deployment includes COOKIE_* environment variables 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

Copilot

Pull request overview

This pull request migrates the authentication mechanism from localStorage-based JWT tokens to httpOnly cookies for improved security. The change addresses XSS vulnerabilities by preventing JavaScript access to authentication tokens while maintaining API compatibility through multiple authentication methods (cookie, Bearer token, and OAuth2).

Key Changes

Implemented httpOnly cookie-based authentication with configurable security settings (secure, sameSite, domain)
Updated frontend to remove localStorage token handling and rely on automatic cookie transmission
Modified backend authentication middleware to prioritize httpOnly cookies while maintaining backward compatibility with Bearer tokens
Added explicit CORS configuration required for credential-based requests

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
helm/myelectricaldata/values.yaml	Added cookie configuration parameters (secure, sameSite, domain) for httpOnly authentication
helm/myelectricaldata/templates/backend/backend-deployment.yaml	Added environment variables for cookie settings to backend deployment
helm/myelectricaldata/Chart.yaml	Bumped chart version to 1.0.0-dev.3
helm/CHANGELOG.md	Updated changelog for version 1.0.0-dev.3 with various features and fixes
apps/web/src/stores/authStore.ts	Removed token storage, updated logout to call backend API, added authentication state management
apps/web/src/pages/Settings.tsx	Replaced JWT token copy functionality with session security information display
apps/web/src/pages/OAuthCallback.tsx	Removed access_token URL parameter passing, relying on httpOnly cookie
apps/web/src/pages/ConsentRedirect.tsx	Removed access_token URL parameter passing, relying on httpOnly cookie
apps/web/src/pages/ApiDocs.tsx	Updated Swagger UI to use credentials for authentication instead of Bearer token header
apps/web/src/pages/AdminOffers.tsx	Removed direct token checks, relying on automatic cookie authentication
apps/web/src/hooks/useAuth.ts	Updated authentication logic to check cookie validity, improved isAuthenticated state management
apps/web/src/api/client.ts	Enabled withCredentials for cookie support, removed Authorization header injection
apps/api/uv.lock	Updated API version to 1.4.0.dev1
apps/api/src/routers/accounts.py	Added cookie helper functions, updated login to set httpOnly cookie, added logout endpoint
apps/api/src/middleware/auth.py	Added cookie-based authentication with priority order: cookie > Bearer > OAuth2
apps/api/src/main.py	Updated CORS configuration with explicit origins and cookie support
apps/api/src/config/settings.py	Added cookie configuration settings (COOKIE_SECURE, COOKIE_SAMESITE, COOKIE_DOMAIN)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-21T00:04:28Z

apps/api/src/config/settings.py

+    # Cookie settings
+    COOKIE_SECURE: bool = False  # Set True in production (HTTPS only)
+    COOKIE_SAMESITE: Literal["lax", "strict", "none"] = "lax"
+    COOKIE_DOMAIN: str = ""  # Empty = current domain only


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.

Suggested change

COOKIE_DOMAIN: str = "" # Empty = current domain only

COOKIE_DOMAIN: str = "" # Empty string is treated as None / no explicit domain (browser uses current host). When setting a value (e.g. ".example.com"), ensure it matches your frontend domain/CORS setup to avoid cookie isolation issues with httpOnly cookies.

Copilot · 2025-12-21T00:04:28Z

apps/api/src/routers/accounts.py

+        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


The cookie's max_age is set to ACCESS_TOKEN_EXPIRE_MINUTES * 60, which equals 43200 * 60 = 2,592,000 seconds (30 days). However, the comment in Settings.tsx (line 598) states "Durée de validité : 30 jours" (30 days validity), which is correct. Consider adding a comment here that explicitly states this converts to 30 days for clarity, as the calculation is not immediately obvious.

Suggested change

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 · 2025-12-21T00:04:29Z

apps/api/src/routers/accounts.py

+    response.delete_cookie(
+        key="access_token",
+        path="/",
+        domain=settings.COOKIE_DOMAIN or None,


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.

Suggested change

domain=settings.COOKIE_DOMAIN or None,

domain=settings.COOKIE_DOMAIN or None,

secure=settings.COOKIE_SECURE,

samesite=settings.COOKIE_SAMESITE,

Copilot · 2025-12-21T00:04:29Z

apps/api/src/main.py

+# CORS middleware - explicit origins required for credentials (httpOnly cookies)
+def get_cors_origins() -> list[str]:
+    """Build CORS origins from settings"""
+    origins = [settings.FRONTEND_URL]


The CORS configuration adds FRONTEND_URL unconditionally to the allowed origins, even if it's empty or set to a default value. If settings.FRONTEND_URL is empty or invalid, this could cause CORS issues. Consider adding a check to ensure FRONTEND_URL is properly configured before adding it to the origins list, or add a fallback validation.

Suggested change

origins = [settings.FRONTEND_URL]

origins: list[str] = []

frontend_url = (settings.FRONTEND_URL or "").strip()

if frontend_url:

origins.append(frontend_url)

Copilot · 2025-12-21T00:04:29Z

helm/myelectricaldata/values.yaml


+  # Cookie settings for httpOnly JWT authentication
+  cookie:
+    secure: true            # Set to true for HTTPS (recommended for production)


The default value for 'cookie.secure' is set to 'true' in the Helm values, which is appropriate for production. However, the comment suggests this should be set "to true for HTTPS", which implies it might need to be false for non-HTTPS environments. The corresponding Python setting (COOKIE_SECURE) defaults to False for development. This mismatch could cause issues where Helm deployments expect HTTPS by default but the application code doesn't. Consider aligning the defaults or adding clearer documentation about when to use true vs false.

Suggested change

secure: true # Set to true for HTTPS (recommended for production)

secure: true # true = use Secure cookies (assumes HTTPS, recommended for production).

# Set to false for local/non-HTTPS environments and ensure this matches

# the backend COOKIE_SECURE (or equivalent) setting for consistency.

Copilot · 2025-12-21T00:04:30Z

helm/CHANGELOG.md

+## [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)
+
+### Features
+
+* add admin data sharing for PDL debugging ([cfb32b8](https://github.com/MyElectricalData/myelectricaldata_new/commit/cfb32b83ddb435c9c792aec031ead2ad2ae054ab))
+* **api:** add Slack notifications for new contributions ([4a87243](https://github.com/MyElectricalData/myelectricaldata_new/commit/4a872435d5c019122ed32d9df031583af17d7300))
+* **simulator:** add PDF export per offer and calculation explanations ([bdfa4aa](https://github.com/MyElectricalData/myelectricaldata_new/commit/bdfa4aa5ba3a2400af4fc083b4cc38b74c0032dc))
+* **web:** add JSON download button for API credentials ([ed5bc85](https://github.com/MyElectricalData/myelectricaldata_new/commit/ed5bc85d70b080793dd3c9ea0a09f8eed9765a3d))
+* **web:** mask client_secret and update warning message ([8824238](https://github.com/MyElectricalData/myelectricaldata_new/commit/8824238120167c9fd5ec17cc9a423e70f3e8f652))
+* **workflow:** update sync process to trigger on successful Release workflow ([a2d039b](https://github.com/MyElectricalData/myelectricaldata_new/commit/a2d039b3d552655959c0b7a0ff68a4ad92779b26))
+
+### Bug Fixes
+
+* address Copilot review suggestions ([f78115a](https://github.com/MyElectricalData/myelectricaldata_new/commit/f78115a8dbed44f5a03fd410f8be5da01b2554db))
+* **api:** correct type annotation for cached_data in admin router ([9b383f1](https://github.com/MyElectricalData/myelectricaldata_new/commit/9b383f13dade4ea6332254165e075837c17fd26b))
+* **api:** fix type annotations in SlackService ([bb055e9](https://github.com/MyElectricalData/myelectricaldata_new/commit/bb055e9296a3426c68a0bbd2dbff0e4383209141))
+* **ci:** disable ARM64 build by default to speed up CI ([3b3e6cb](https://github.com/MyElectricalData/myelectricaldata_new/commit/3b3e6cbc327074f432614651ea1220c0655a54b1))
+* **ci:** move sync-develop into Release workflow ([74e3240](https://github.com/MyElectricalData/myelectricaldata_new/commit/74e3240fdd84e4cb5b4bc10894396784408c1800))
+* **ci:** remove 'v' prefix from helm tags ([8f2557e](https://github.com/MyElectricalData/myelectricaldata_new/commit/8f2557e4d2816f668e4c24211d61e2d9ca73e73c))
+* **web:** add defensive type checks for offpeak_hours parsing ([a82441f](https://github.com/MyElectricalData/myelectricaldata_new/commit/a82441f5b5374bef197302cf16df4bbd3485f74d))
+* **web:** handle nested arrays in offpeak_hours parsing ([609661b](https://github.com/MyElectricalData/myelectricaldata_new/commit/609661bdcd45913e37224b238264181a68594caf))
+* **web:** make login button full width on signup success ([07786f8](https://github.com/MyElectricalData/myelectricaldata_new/commit/07786f87493d7a508558fe529621f37cf45cdb5b))
+* **web:** resolve race condition in OfferSelector initial load ([d1fe1db](https://github.com/MyElectricalData/myelectricaldata_new/commit/d1fe1db96fcd460589f4459706bee0c28c042370))
+* **web:** sync OfferSelector state on page navigation ([2ec0cf1](https://github.com/MyElectricalData/myelectricaldata_new/commit/2ec0cf13ec462efa2e57a75d9f871a87f9d13ff0))
+
+### Refactoring
+
+* **web:** consolidate info blocks in Simulator page ([1c4d5d5](https://github.com/MyElectricalData/myelectricaldata_new/commit/1c4d5d59e5750ce1279300274811c5f664e93ac0))


The CHANGELOG for version 1.0.0-dev.3 does not mention the migration from localStorage JWT tokens to httpOnly cookies, which is a significant security-related change and breaking change for the authentication flow. This major architectural change should be documented in the CHANGELOG under Features or Breaking Changes section to inform users about the authentication mechanism update.

Copilot · 2025-12-21T00:04:30Z

apps/web/src/pages/ApiDocs.tsx

+              // when withCredentials is enabled on fetch requests
+              req.credentials = 'include'


The requestInterceptor sets 'req.credentials' to 'include', but the correct property name for Swagger UI's request interceptor is 'req.withCredentials'. The 'credentials' property is used by the Fetch API, but Swagger UI uses XMLHttpRequest which requires 'withCredentials'. This should be changed to 'req.withCredentials = true' to ensure cookies are sent with API requests from the Swagger UI.

Suggested change

// when withCredentials is enabled on fetch requests

req.credentials = 'include'

// when withCredentials is enabled on XHR requests

req.withCredentials = true

- client.ts: Skip 401 redirect for /accounts/me and /login page - auth.ts: Return success: false instead of throwing on 401 - Login.tsx: Redirect to dashboard if already authenticated 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

- 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]>

Clément VALENTIN and others added 2 commits December 21, 2025 00:59

chore(helm): release 1.0.0-dev.3 [skip ci]

577860e

Copilot AI review requested due to automatic review settings December 21, 2025 00:00

Copilot started reviewing on behalf of m4dm4rtig4n December 21, 2025 00:01 View session

Copilot AI reviewed Dec 21, 2025

View reviewed changes

Clément VALENTIN and others added 4 commits December 21, 2025 01:09

chore(release): 1.4.1-dev.1 [skip ci]

0f0242c

chore(helm): release 1.0.0-dev.4 [skip ci]

a3a2a12

m4dm4rtig4n merged commit e27762f into main Dec 21, 2025
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Develop #91

Develop #91

Uh oh!

m4dm4rtig4n commented Dec 21, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Copilot AI Dec 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	COOKIE_DOMAIN: str = "" # Empty = current domain only
	COOKIE_DOMAIN: str = "" # Empty string is treated as None / no explicit domain (browser uses current host). When setting a value (e.g. ".example.com"), ensure it matches your frontend domain/CORS setup to avoid cookie isolation issues with httpOnly cookies.

	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)

-    origins = [settings.FRONTEND_URL]
+    origins: list[str] = []
+    frontend_url = (settings.FRONTEND_URL or "").strip()
+    if frontend_url:
+        origins.append(frontend_url)

-    secure: true            # Set to true for HTTPS (recommended for production)
+    secure: true            # true = use Secure cookies (assumes HTTPS, recommended for production).
+                            # Set to false for local/non-HTTPS environments and ensure this matches
+                            # the backend COOKIE_SECURE (or equivalent) setting for consistency.

		// when withCredentials is enabled on fetch requests
		req.credentials = 'include'

Develop #91

Develop #91

Uh oh!

Conversation

m4dm4rtig4n commented Dec 21, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants