Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions backend/common/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,22 @@ async def dispatch(self, request: Request, call_next):
status_code=500,
content={"detail": "Internal Server Error"}
)

class CORSLoggingMiddleware(BaseHTTPMiddleware):
"""
Middleware to log potential CORS issues.
This should be added outside CORSMiddleware so it can see the final response headers.
"""
async def dispatch(self, request: Request, call_next):
response = await call_next(request)

origin = request.headers.get("origin")
if origin:
allow_origin = response.headers.get("access-control-allow-origin")
if not allow_origin:
# If it's a 404/500, sometimes headers are missing if not handled correctly.
# But generally CORSMiddleware adds them even for errors.
logger = get_logger("backend.middleware.cors")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (performance): Instantiate the logger once at module or class level instead of per-request.

Looking up the logger inside dispatch adds unnecessary overhead in a hot path. Define logger = get_logger("backend.middleware.cors") at module scope (or as a class attribute) and reuse it inside dispatch instead.

Suggested implementation:

            if not allow_origin:
                # If it's a 404/500, sometimes headers are missing if not handled correctly.
                # But generally CORSMiddleware adds them even for errors.
                logger.warning(f"CORS Warning: Origin '{origin}' requested but no 'Access-Control-Allow-Origin' header in response. Status: {response.status_code}")
logger = get_logger("backend.middleware.cors")


class CORSLoggingMiddleware(BaseHTTPMiddleware):

logger.warning(f"CORS Warning: Origin '{origin}' requested but no 'Access-Control-Allow-Origin' header in response. Status: {response.status_code}")

return response
7 changes: 5 additions & 2 deletions backend/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from fastapi import FastAPI
from fastapi.middleware.cors import CORSMiddleware

from backend.common.middleware import PathRewriteMiddleware, ExceptionHandlingMiddleware
from backend.common.middleware import PathRewriteMiddleware, ExceptionHandlingMiddleware, CORSLoggingMiddleware
from backend.api.problems import router as problems_router
from backend.api.sql import router as sql_router
from backend.api.stats import router as stats_router
Expand Down Expand Up @@ -101,7 +101,7 @@ def start_scheduler_background():
# Cloud Run 도메인 및 Regex 정의 (환경 무관하게 참조 가능하도록)
cloud_origins = [
"https://query-craft-frontend-53ngedkhia-uc.a.run.app",
"https://query-craft-frontend-758178119666.us-central1.run.app",
"https://query-craft-frontend-758178119666.us-central1.run.app", # Verified as per issue report
"https://query-craft-frontend-758178119666.a.run.app", # 추가
"https://querycraft.run.app", # 커스텀 도메인 예비
]
Expand Down Expand Up @@ -133,6 +133,9 @@ def start_scheduler_background():
allow_methods=["*"],
allow_headers=["*"],
)

# CORS Logging Middleware (CORS 설정 이후에 추가하여 바깥쪽에서 감싸도록 함)
app.add_middleware(CORSLoggingMiddleware)

# 404 및 기타 에러 로깅 미들웨어
@app.middleware("http")
Expand Down
47 changes: 24 additions & 23 deletions tests/test_cors_config.py
Original file line number Diff line number Diff line change
@@ -1,28 +1,39 @@

import os
import sys
import pytest
from unittest.mock import patch
import importlib
from fastapi.testclient import TestClient

# Set ENV to production before importing backend.main to ensure production CORS settings are used
os.environ["ENV"] = "production"
@pytest.fixture(scope="module")
def client():
"""
Fixture to set ENV to production and reload backend.main
to apply production CORS settings.
Restores the original environment and reloads backend.main afterwards.
"""
# Temporarily set ENV to production
with patch.dict(os.environ, {"ENV": "production"}):
import backend.main
importlib.reload(backend.main)
from backend.main import app
yield TestClient(app)

try:
from backend.main import app
except ImportError:
sys.path.append(os.getcwd())
from backend.main import app
# Reload backend.main to restore original (development) state
# This is crucial so other tests (like test_integration.py)
# don't see the production app instance or stale env settings.
import backend.main
importlib.reload(backend.main)

class TestCORSConfig:
"""Test CORS configuration for the backend."""

def test_cors_specific_origin_allowed(self):
def test_cors_specific_origin_allowed(self, client):
"""
Verify that the specific frontend origin reported in the issue is allowed.
Origin: https://query-craft-frontend-758178119666.us-central1.run.app
"""
origin = "https://query-craft-frontend-758178119666.us-central1.run.app"
client = TestClient(app)

# 1. Test Preflight (OPTIONS)
response = client.options(
Expand All @@ -38,18 +49,16 @@ def test_cors_specific_origin_allowed(self):
assert response.headers.get("access-control-allow-credentials") == "true"

# 2. Test GET request
# /auth/me might return 200 (logged_in=False) or 401 depending on logic, but headers must be present
response = client.get(
"/auth/me",
headers={"Origin": origin}
)
assert response.headers.get("access-control-allow-origin") == origin
assert response.headers.get("access-control-allow-credentials") == "true"

def test_cors_cloud_run_domain_regex(self):
def test_cors_cloud_run_domain_regex(self, client):
"""Verify that other Cloud Run domains matching the regex are also allowed."""
origin = "https://query-craft-frontend-random-hash.a.run.app"
client = TestClient(app)

response = client.options(
"/auth/me",
Expand All @@ -61,10 +70,9 @@ def test_cors_cloud_run_domain_regex(self):
assert response.status_code == 200
assert response.headers.get("access-control-allow-origin") == origin

def test_cors_disallowed_origin(self):
def test_cors_disallowed_origin(self, client):
"""Verify that a random origin is NOT allowed."""
origin = "https://evil-site.com"
client = TestClient(app)

response = client.options(
"/auth/me",
Expand All @@ -73,21 +81,14 @@ def test_cors_disallowed_origin(self):
"Access-Control-Request-Method": "GET",
}
)
# Standard behavior for disallowed origin in FastAPI CORSMiddleware is
# usually 200 OK but WITHOUT Access-Control-Allow-Origin header,
# or sometimes 400.
# Starlette CORSMiddleware just ignores it and processes request as normal non-CORS,
# or returns response without CORS headers.

assert "access-control-allow-origin" not in response.headers

def test_path_rewrite_does_not_break_cors(self):
def test_path_rewrite_does_not_break_cors(self, client):
"""
Verify that PathRewriteMiddleware (which rewrites /auth/me to /api/auth/me)
does not interfere with CORS headers.
"""
origin = "https://query-craft-frontend-758178119666.us-central1.run.app"
client = TestClient(app)

# Send request to /auth/me (rewritten to /api/auth/me)
response = client.get(
Expand Down