From aed7d0252c199326bbf6578990143224fff28e1f Mon Sep 17 00:00:00 2001 From: Connor Howington Date: Tue, 24 Jun 2025 14:23:34 -0400 Subject: [PATCH] Commit AI-gen security fixes (review needed) --- config/settings/base.py | 8 ++ config/settings/production.py | 4 + config/urls.py | 2 + frontend/nginx.conf | 1 + frontend/src/apiClient/index.ts | 37 +++++++-- frontend/src/utils/utils.ts | 44 ++++++++++ .../users/api/views.py | 33 ++++++-- .../users/tests/test_security.py | 83 +++++++++++++++++++ 8 files changed, 200 insertions(+), 12 deletions(-) create mode 100644 spectrumx_visualization_platform/users/tests/test_security.py diff --git a/config/settings/base.py b/config/settings/base.py index 1f59932b..08189607 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -221,8 +221,16 @@ SESSION_COOKIE_HTTPONLY = True # https://docs.djangoproject.com/en/dev/ref/settings/#csrf-cookie-httponly CSRF_COOKIE_HTTPONLY = True +# https://docs.djangoproject.com/en/dev/ref/settings/#csrf-cookie-samesite +CSRF_COOKIE_SAMESITE = "Lax" +# https://docs.djangoproject.com/en/dev/ref/settings/#session-cookie-samesite +SESSION_COOKIE_SAMESITE = "Lax" # https://docs.djangoproject.com/en/dev/ref/settings/#x-frame-options X_FRAME_OPTIONS = "DENY" +# https://docs.djangoproject.com/en/dev/ref/settings/#csrf-trusted-origins +CSRF_TRUSTED_ORIGINS = env.list("DJANGO_CSRF_TRUSTED_ORIGINS", default=[]) +# https://docs.djangoproject.com/en/dev/ref/settings/#csrf-use-sessions +CSRF_USE_SESSIONS = True # EMAIL # ------------------------------------------------------------------------------ diff --git a/config/settings/production.py b/config/settings/production.py index ee932832..137ec97e 100644 --- a/config/settings/production.py +++ b/config/settings/production.py @@ -46,6 +46,10 @@ CSRF_COOKIE_SECURE = True # https://docs.djangoproject.com/en/dev/ref/settings/#csrf-cookie-name CSRF_COOKIE_NAME = "__Secure-csrftoken" +# https://docs.djangoproject.com/en/dev/ref/settings/#csrf-cookie-samesite +CSRF_COOKIE_SAMESITE = "Strict" +# https://docs.djangoproject.com/en/dev/ref/settings/#session-cookie-samesite +SESSION_COOKIE_SAMESITE = "Strict" # https://docs.djangoproject.com/en/dev/topics/security/#ssl-https # https://docs.djangoproject.com/en/dev/ref/settings/#secure-hsts-seconds # TODO: set this to 60 seconds first and then to 518400 once you prove the former works diff --git a/config/urls.py b/config/urls.py index b7906211..975a4095 100644 --- a/config/urls.py +++ b/config/urls.py @@ -16,6 +16,7 @@ get_session_info, api_token, test_sdk_connection, + create_auth_token, ) from spectrumx_visualization_platform.users.api.sds import ( get_sds_files, @@ -50,6 +51,7 @@ # DRF auth token path("api/auth-token/", obtain_auth_token), path("api/session-info/", get_session_info), + path("api/create-auth-token/", create_auth_token), path("api/api-token/", api_token), path("api/schema/", SpectacularAPIView.as_view(), name="api-schema"), path( diff --git a/frontend/nginx.conf b/frontend/nginx.conf index c448739e..c6486c0e 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -10,6 +10,7 @@ server { add_header X-Frame-Options "SAMEORIGIN"; add_header X-XSS-Protection "1; mode=block"; add_header X-Content-Type-Options "nosniff"; + add_header Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' ws: wss:; frame-ancestors 'none';"; location / { root /usr/share/nginx/html; diff --git a/frontend/src/apiClient/index.ts b/frontend/src/apiClient/index.ts index 494bbf45..5bd42bfe 100644 --- a/frontend/src/apiClient/index.ts +++ b/frontend/src/apiClient/index.ts @@ -30,8 +30,12 @@ export const useFetchSessionInfo = async () => { if (response.ok) { const data = await response.json(); - localStorage.setItem('authToken', data.auth_token); - localStorage.setItem('csrfToken', data.csrf_token); + // Only store auth token if it exists (don't create unnecessary tokens) + if (data.auth_token) { + localStorage.setItem('authToken', data.auth_token); + } else { + localStorage.removeItem('authToken'); + } const username = data.user.username; setUsername(username); @@ -41,7 +45,6 @@ export const useFetchSessionInfo = async () => { } catch (error) {} localStorage.removeItem('authToken'); - localStorage.removeItem('csrfToken'); // Null means the user isn't logged in setUsername(null); }; @@ -51,6 +54,28 @@ export const useFetchSessionInfo = async () => { }, [setUsername]); }; +/** + * Create an authentication token for API access. + * This should be called when the user needs to access protected API endpoints. + */ +export const createAuthToken = async (): Promise => { + try { + const response = await fetch(API_HOST + '/api/create-auth-token/', { + method: 'POST', + credentials: 'include', + }); + + if (response.ok) { + const data = await response.json(); + localStorage.setItem('authToken', data.auth_token); + return data.auth_token; + } + } catch (error) { + console.error('Failed to create auth token:', error); + } + return null; +}; + const apiClient = axios.create({ baseURL: API_HOST, headers: { 'Content-Type': 'application/json' }, @@ -58,15 +83,13 @@ const apiClient = axios.create({ apiClient.interceptors.request.use((config) => { const authToken = localStorage.getItem('authToken'); - const csrfToken = localStorage.getItem('csrfToken'); if (authToken) { config.headers.Authorization = `Token ${authToken}`; } - if (csrfToken) { - config.headers['X-CSRFToken'] = csrfToken; - } + // CSRF tokens are automatically handled by Django's middleware via HTTP-only cookies + // No need to manually set X-CSRFToken header return config; }); diff --git a/frontend/src/utils/utils.ts b/frontend/src/utils/utils.ts index 99aadc08..0e017ba9 100644 --- a/frontend/src/utils/utils.ts +++ b/frontend/src/utils/utils.ts @@ -1,4 +1,5 @@ import _ from 'lodash'; +import { createAuthToken } from '../apiClient'; export function formatHertz(freq: number, decimals = 2) { if (freq === 0) return 'MHz'; @@ -17,3 +18,46 @@ export function sortByDate(a: any, b: any, dateField: string) { const dateB = new Date(_.get(b, dateField)); return dateB.getTime() - dateA.getTime(); } + +/** + * Check if the user has a valid authentication token + */ +export const hasValidAuthToken = (): boolean => { + const token = localStorage.getItem('authToken'); + return token !== null && token.trim() !== ''; +}; + +/** + * Get the current authentication token + */ +export const getAuthToken = (): string | null => { + return localStorage.getItem('authToken'); +}; + +/** + * Remove the authentication token + */ +export const removeAuthToken = (): void => { + localStorage.removeItem('authToken'); +}; + +/** + * Ensure the user has an authentication token, creating one if necessary + */ +export const ensureAuthToken = async (): Promise => { + if (hasValidAuthToken()) { + return getAuthToken(); + } + + // Try to create a new token + return await createAuthToken(); +}; + +/** + * Clear all authentication data + */ +export const clearAuthData = (): void => { + removeAuthToken(); + // Note: CSRF tokens are handled by Django's HTTP-only cookies + // No need to manually clear them +}; diff --git a/spectrumx_visualization_platform/users/api/views.py b/spectrumx_visualization_platform/users/api/views.py index 7d7306aa..ee7870f4 100644 --- a/spectrumx_visualization_platform/users/api/views.py +++ b/spectrumx_visualization_platform/users/api/views.py @@ -1,4 +1,3 @@ -from django.middleware.csrf import get_token from rest_framework import status from rest_framework.authtoken.models import Token from rest_framework.decorators import action @@ -37,14 +36,19 @@ def me(self, request): @api_view(["GET"]) def get_session_info(request): + """ + Get session information for the authenticated user. + + Note: CSRF tokens are automatically handled by Django's middleware via HTTP-only cookies. + Auth tokens are only created when explicitly requested. + """ if request.user.is_authenticated: - auth_token = Token.objects.get_or_create(user=request.user)[0] - csrf_token = get_token(request) + # Get existing auth token if it exists, don't create new ones unnecessarily + auth_token = Token.objects.filter(user=request.user).first() return Response( { - "auth_token": str(auth_token), - "csrf_token": csrf_token, + "auth_token": str(auth_token) if auth_token else None, "user": { "id": request.user.id, "username": request.user.username, @@ -54,6 +58,25 @@ def get_session_info(request): return Response({"error": "User not authenticated"}, status=401) +@api_view(["POST"]) +@permission_classes([IsAuthenticated]) +def create_auth_token(request): + """ + Create a new authentication token for the user. + This endpoint should be called when the user needs a token for API access. + """ + if request.user.is_authenticated: + auth_token = Token.objects.get_or_create(user=request.user)[0] + return Response( + { + "auth_token": str(auth_token), + "message": "Authentication token created successfully", + }, + status=status.HTTP_201_CREATED, + ) + return Response({"error": "User not authenticated"}, status=401) + + @api_view(["POST", "GET"]) @permission_classes([IsAuthenticated]) def api_token(request): diff --git a/spectrumx_visualization_platform/users/tests/test_security.py b/spectrumx_visualization_platform/users/tests/test_security.py new file mode 100644 index 00000000..c33f7205 --- /dev/null +++ b/spectrumx_visualization_platform/users/tests/test_security.py @@ -0,0 +1,83 @@ +""" +Security tests for authentication and CSRF protection. +""" + +from django.test import Client +from django.test import TestCase +from django.urls import reverse +from rest_framework.test import APIClient + +from spectrumx_visualization_platform.users.models import User + + +class SecurityTestCase(TestCase): + """Test security features and token handling.""" + + def setUp(self): + """Set up test data.""" + self.client = Client() + self.api_client = APIClient() + self.user = User.objects.create_user( + username="testuser", email="test@example.com", password="testpass123" + ) + + def test_csrf_token_not_exposed_in_session_info(self): + """Test that CSRF tokens are not exposed in session info endpoint.""" + self.client.force_login(self.user) + response = self.client.get(reverse("api:session-info")) + + self.assertEqual(response.status_code, 200) + data = response.json() + + # CSRF token should not be in the response + self.assertNotIn("csrf_token", data) + + # Auth token should only be present if it exists + if "auth_token" in data: + self.assertIsInstance(data["auth_token"], (str, type(None))) + + def test_auth_token_creation_endpoint(self): + """Test that auth tokens are only created when explicitly requested.""" + self.api_client.force_authenticate(user=self.user) + + # First, check session info without creating token + response = self.api_client.get(reverse("api:session-info")) + self.assertEqual(response.status_code, 200) + data = response.json() + + # Initially, no auth token should exist + self.assertIsNone(data.get("auth_token")) + + # Now create a token explicitly + response = self.api_client.post(reverse("api:create-auth-token")) + self.assertEqual(response.status_code, 201) + data = response.json() + + # Should return a new token + self.assertIn("auth_token", data) + self.assertIsInstance(data["auth_token"], str) + + # Session info should now include the token + response = self.api_client.get(reverse("api:session-info")) + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertIsNotNone(data.get("auth_token")) + + def test_csrf_protection_enabled(self): + """Test that CSRF protection is properly enabled.""" + # This test verifies that CSRF middleware is working + # by checking that the CSRF cookie is set + response = self.client.get("/") + self.assertIn("csrftoken", response.cookies) + + def test_secure_cookie_settings(self): + """Test that secure cookie settings are applied.""" + response = self.client.get("/") + + # Check that CSRF cookie has proper attributes + csrf_cookie = response.cookies.get("csrftoken") + if csrf_cookie: + # In production, these should be True + # In development, they might be False + self.assertIsInstance(csrf_cookie.get("httponly"), bool) + self.assertIsInstance(csrf_cookie.get("samesite"), str)