diff --git a/alembic/versions/007_unique_polar_customer_id.py b/alembic/versions/007_unique_polar_customer_id.py new file mode 100644 index 0000000..21a7516 --- /dev/null +++ b/alembic/versions/007_unique_polar_customer_id.py @@ -0,0 +1,27 @@ +"""add unique constraint on users.polar_customer_id + +Revision ID: 007_unique_polar_customer_id +Revises: 006_unique_user_email +Create Date: 2026-03-30 00:00:00.000000 + +""" + +from collections.abc import Sequence + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "007_unique_polar_customer_id" +down_revision: str | Sequence[str] | None = "006_unique_user_email" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + """Add UNIQUE constraint on users.polar_customer_id to prevent duplicate billing records.""" + op.create_unique_constraint("uq_users_polar_customer_id", "users", ["polar_customer_id"]) + + +def downgrade() -> None: + """Remove UNIQUE constraint on users.polar_customer_id.""" + op.drop_constraint("uq_users_polar_customer_id", "users", type_="unique") diff --git a/api/deps.py b/api/deps.py index 3e2708e..ddc4857 100644 --- a/api/deps.py +++ b/api/deps.py @@ -76,16 +76,20 @@ def _get_limits(user: User | None) -> dict: def _decode_supabase_jwt(token: str) -> dict: """Decode and verify a Supabase JWT using PyJWT (HS256). - Validates signature, expiry, audience, and not-before claims. - Returns the JWT payload dict, or raises HTTPException on failure. + Validates signature, expiry, audience, not-before, and (when supabase_url is + configured) issuer claims. Returns the JWT payload dict, or raises + HTTPException on failure. """ + decode_kwargs = { + "jwt": token, + "key": settings.supabase_jwt_secret, + "algorithms": ["HS256"], + "audience": "authenticated", + } + if settings.supabase_url: + decode_kwargs["issuer"] = settings.supabase_url.rstrip("/") + "/auth/v1" try: - return pyjwt.decode( - token, - settings.supabase_jwt_secret, - algorithms=["HS256"], - audience="authenticated", - ) + return pyjwt.decode(**decode_kwargs) except pyjwt.InvalidTokenError as e: logger.warning("JWT decode failed: %s", e) raise HTTPException(status_code=401, detail="Invalid token") from e diff --git a/api/routes/analytics.py b/api/routes/analytics.py index 1dffd03..d8b79af 100644 --- a/api/routes/analytics.py +++ b/api/routes/analytics.py @@ -3,20 +3,26 @@ from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session +from api.deps import get_optional_user from core.traces.events import get_co_picked_examples, get_event_stats, get_popular_examples from db.postgres.base import get_db +from db.postgres.models import User router = APIRouter(prefix="/analytics", tags=["analytics"]) @router.get("/stats") -def analytics_stats(db: Session = Depends(get_db)): - """Return overall curation event stats. +def analytics_stats( + user: User | None = Depends(get_optional_user), + db: Session = Depends(get_db), +): + """Return curation event stats, scoped to the authenticated user when present. - Includes total events, events by type, most searched queries, - and most picked examples. + Authenticated users see their own stats including query history. + Unauthenticated requests see aggregate counts only (queries omitted for privacy). """ - return get_event_stats(db=db) + user_id = user.supabase_id if user else None + return get_event_stats(db=db, user_id=user_id) @router.get("/popular") diff --git a/cherry_evals/config.py b/cherry_evals/config.py index 868a8a9..7fbb10a 100644 --- a/cherry_evals/config.py +++ b/cherry_evals/config.py @@ -61,18 +61,17 @@ class Settings(BaseSettings): @model_validator(mode="after") def _check_auth_secrets(self) -> "Settings": - """Disable auth if JWT secret is missing (safe dev default). + """Crash startup if AUTH_ENABLED=True but JWT secret is missing. - In production, SUPABASE_JWT_SECRET must be set when AUTH_ENABLED=True. - Locally this falls back to open access with a loud warning. + Silent fallback to AUTH_ENABLED=False is a security risk — a misconfigured + production deployment would silently accept unauthenticated requests. + If you want auth disabled for local development, set AUTH_ENABLED=False explicitly. """ if self.auth_enabled and not self.supabase_jwt_secret: - _config_logger.warning( - "AUTH_ENABLED=True but SUPABASE_JWT_SECRET is empty — " - "falling back to AUTH_ENABLED=False. " - "Set SUPABASE_JWT_SECRET for production." + raise ValueError( + "SUPABASE_JWT_SECRET must be set when AUTH_ENABLED=True. " + "Set AUTH_ENABLED=False explicitly for local development." ) - self.auth_enabled = False return self diff --git a/db/postgres/models.py b/db/postgres/models.py index 8114aeb..d040ae2 100644 --- a/db/postgres/models.py +++ b/db/postgres/models.py @@ -34,7 +34,7 @@ class User(Base): trial_ends_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) # Polar.sh subscription - polar_customer_id: Mapped[str | None] = mapped_column(String(255), nullable=True) + polar_customer_id: Mapped[str | None] = mapped_column(String(255), nullable=True, unique=True) polar_subscription_id: Mapped[str | None] = mapped_column(String(255), nullable=True) subscription_status: Mapped[str | None] = mapped_column(String(50), nullable=True) diff --git a/tests/conftest.py b/tests/conftest.py index 7c1f726..58c5ce4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,13 @@ """Shared test fixtures for all test types.""" import os + +# Set AUTH_ENABLED=False before any application modules are imported. +# The module-level `settings = Settings()` in cherry_evals/config.py will fail +# at import time if AUTH_ENABLED=True and SUPABASE_JWT_SECRET is empty. +# Individual test fixtures override settings via patch() as needed. +os.environ.setdefault("AUTH_ENABLED", "False") + from contextlib import contextmanager from pathlib import Path from unittest.mock import patch diff --git a/tests/integration/test_analytics_api.py b/tests/integration/test_analytics_api.py index d7b18bf..c83252d 100644 --- a/tests/integration/test_analytics_api.py +++ b/tests/integration/test_analytics_api.py @@ -310,3 +310,67 @@ def test_hybrid_search_records_event(self, test_client, test_db_session): # Should succeed (falls back to keyword) assert response.status_code == 200 + + +# --------------------------------------------------------------------------- +# SEC-H3: Analytics stats scoped to authenticated user +# --------------------------------------------------------------------------- + + +class TestAnalyticsStatsAuthScoping: + """SEC-H3: /analytics/stats scopes data to the authenticated user.""" + + def test_unauthenticated_stats_returns_global_counts(self, test_client, test_db_session): + """Unauthenticated request gets aggregate counts (no query history).""" + test_db_session.add( + CurationEvent(event_type="search", query="secret query", user_id="user-999") + ) + test_db_session.flush() + + response = test_client.get("/analytics/stats") + + assert response.status_code == 200 + data = response.json() + # most_searched_queries is empty for unauthenticated (privacy) + assert data["most_searched_queries"] == [] + # but total_events includes all events + assert data["total_events"] >= 1 + + def test_authenticated_stats_scoped_to_user( + self, test_client, test_db_session, authed_client_free + ): + """Authenticated user sees only their own events.""" + # Event belonging to this user (supabase_id="test-user-123" from _make_fake_user) + test_db_session.add( + CurationEvent(event_type="search", query="my query", user_id="test-user-123") + ) + # Event belonging to another user + test_db_session.add( + CurationEvent(event_type="search", query="their query", user_id="other-user-456") + ) + test_db_session.flush() + + response = authed_client_free.get("/analytics/stats") + + assert response.status_code == 200 + data = response.json() + # Authenticated user sees query history + query_strings = [item["query"] for item in data["most_searched_queries"]] + assert "my query" in query_strings + # Other user's query must not appear + assert "their query" not in query_strings + + def test_authenticated_stats_total_scoped_to_user(self, authed_client_free, test_db_session): + """Total event count is scoped to the authenticated user, not global.""" + # 1 event for this user, 5 for others + test_db_session.add(CurationEvent(event_type="pick", user_id="test-user-123")) + for i in range(5): + test_db_session.add(CurationEvent(event_type="search", user_id=f"stranger-{i}")) + test_db_session.flush() + + response = authed_client_free.get("/analytics/stats") + + assert response.status_code == 200 + data = response.json() + # The user's total should only count their own event + assert data["total_events"] == 1 diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index e2d0155..ca609a6 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -99,3 +99,37 @@ def test_settings_qdrant_api_key_empty_by_default(monkeypatch, tmp_path): settings = Settings() assert settings.qdrant_api_key == "" + + +def test_settings_auth_enabled_without_jwt_secret_raises(monkeypatch, tmp_path): + """Settings must raise ValueError when AUTH_ENABLED=True but SUPABASE_JWT_SECRET is empty. + + SEC-C5: Silent fallback to disabled auth is a security risk. + """ + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("SUPABASE_JWT_SECRET", raising=False) + + import pytest + + with pytest.raises(ValueError, match="SUPABASE_JWT_SECRET must be set"): + Settings(auth_enabled=True, supabase_jwt_secret="") + + +def test_settings_auth_disabled_without_jwt_secret_is_fine(monkeypatch, tmp_path): + """AUTH_ENABLED=False with no JWT secret should not raise — explicit opt-out is valid.""" + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("SUPABASE_JWT_SECRET", raising=False) + + settings = Settings(auth_enabled=False, supabase_jwt_secret="") + + assert settings.auth_enabled is False + + +def test_settings_auth_enabled_with_jwt_secret_is_fine(monkeypatch, tmp_path): + """AUTH_ENABLED=True with a non-empty JWT secret should not raise.""" + monkeypatch.chdir(tmp_path) + + settings = Settings(auth_enabled=True, supabase_jwt_secret="supersecret") + + assert settings.auth_enabled is True + assert settings.supabase_jwt_secret == "supersecret" diff --git a/tests/unit/test_jwt_decode.py b/tests/unit/test_jwt_decode.py new file mode 100644 index 0000000..fa156b6 --- /dev/null +++ b/tests/unit/test_jwt_decode.py @@ -0,0 +1,136 @@ +"""Unit tests for JWT decode with issuer validation (SEC-C4).""" + +from datetime import UTC, datetime, timedelta +from unittest.mock import patch + +import jwt as pyjwt +import pytest +from fastapi import HTTPException + +from cherry_evals.config import Settings + + +def _make_token(secret: str, issuer: str | None = None, audience: str = "authenticated") -> str: + """Build a minimal HS256 JWT for testing.""" + payload: dict = { + "sub": "user-abc", + "email": "user@example.com", + "aud": audience, + "exp": datetime.now(UTC) + timedelta(hours=1), + "iat": datetime.now(UTC), + } + if issuer is not None: + payload["iss"] = issuer + return pyjwt.encode(payload, secret, algorithm="HS256") + + +def _settings_with(supabase_url: str = "", supabase_jwt_secret: str = "secret") -> Settings: + return Settings( + auth_enabled=True, + supabase_jwt_secret=supabase_jwt_secret, + supabase_url=supabase_url, + ) + + +class TestJWTDecodeIssuerValidation: + """SEC-C4: JWT decode must validate issuer when supabase_url is configured.""" + + def test_valid_token_without_supabase_url_passes(self): + """When supabase_url is empty, issuer is not checked — token passes.""" + from api.deps import _decode_supabase_jwt + + settings = _settings_with(supabase_url="", supabase_jwt_secret="secret") + token = _make_token("secret") # no issuer in token + + with patch("api.deps.settings", settings): + payload = _decode_supabase_jwt(token) + + assert payload["sub"] == "user-abc" + + def test_valid_token_with_correct_issuer_passes(self): + """When supabase_url is set, token with matching issuer passes.""" + from api.deps import _decode_supabase_jwt + + supabase_url = "https://abc123.supabase.co" + expected_issuer = supabase_url + "/auth/v1" + settings = _settings_with(supabase_url=supabase_url, supabase_jwt_secret="secret") + token = _make_token("secret", issuer=expected_issuer) + + with patch("api.deps.settings", settings): + payload = _decode_supabase_jwt(token) + + assert payload["sub"] == "user-abc" + + def test_token_with_wrong_issuer_rejected(self): + """When supabase_url is set, token with wrong issuer raises 401.""" + from api.deps import _decode_supabase_jwt + + supabase_url = "https://abc123.supabase.co" + settings = _settings_with(supabase_url=supabase_url, supabase_jwt_secret="secret") + token = _make_token("secret", issuer="https://evil.example.com/auth/v1") + + with patch("api.deps.settings", settings): + with pytest.raises(HTTPException) as exc_info: + _decode_supabase_jwt(token) + + assert exc_info.value.status_code == 401 + + def test_token_missing_issuer_rejected_when_supabase_url_set(self): + """When supabase_url is set, token with no iss claim is rejected.""" + from api.deps import _decode_supabase_jwt + + supabase_url = "https://abc123.supabase.co" + settings = _settings_with(supabase_url=supabase_url, supabase_jwt_secret="secret") + token = _make_token("secret", issuer=None) # no iss field + + with patch("api.deps.settings", settings): + with pytest.raises(HTTPException) as exc_info: + _decode_supabase_jwt(token) + + assert exc_info.value.status_code == 401 + + def test_supabase_url_trailing_slash_normalised(self): + """Trailing slash on supabase_url should not cause issuer mismatch.""" + from api.deps import _decode_supabase_jwt + + supabase_url = "https://abc123.supabase.co/" + expected_issuer = "https://abc123.supabase.co/auth/v1" + settings = _settings_with(supabase_url=supabase_url, supabase_jwt_secret="secret") + token = _make_token("secret", issuer=expected_issuer) + + with patch("api.deps.settings", settings): + payload = _decode_supabase_jwt(token) + + assert payload["sub"] == "user-abc" + + def test_expired_token_rejected(self): + """Expired tokens must raise 401 regardless of issuer config.""" + from api.deps import _decode_supabase_jwt + + settings = _settings_with(supabase_jwt_secret="secret") + expired_payload = { + "sub": "user-abc", + "aud": "authenticated", + "exp": datetime.now(UTC) - timedelta(hours=1), + "iat": datetime.now(UTC) - timedelta(hours=2), + } + token = pyjwt.encode(expired_payload, "secret", algorithm="HS256") + + with patch("api.deps.settings", settings): + with pytest.raises(HTTPException) as exc_info: + _decode_supabase_jwt(token) + + assert exc_info.value.status_code == 401 + + def test_bad_signature_rejected(self): + """Tokens signed with wrong key must raise 401.""" + from api.deps import _decode_supabase_jwt + + settings = _settings_with(supabase_jwt_secret="correct-secret") + token = _make_token("wrong-secret") + + with patch("api.deps.settings", settings): + with pytest.raises(HTTPException) as exc_info: + _decode_supabase_jwt(token) + + assert exc_info.value.status_code == 401