diff --git a/README.md b/README.md index 085671b..516d8c4 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ whole point, and it's directly tested. - **Backend/API:** FastAPI (Python) — auto OpenAPI schema feeds the API tests - **Frontend:** Jinja2 + HTMX + Tailwind + Alpine.js (modern look, stays in Python) -- **Auth:** session cookies + bcrypt +- **Identity:** anonymous browser session ID persisted in localStorage and mapped to a backend user record - **DB:** SQLModel — **SQLite** locally & in tests, **Postgres** in production (one URL change) - **AI:** provider-agnostic wrapper; **Google Gemini Flash** (free tier) primary, Groq planned, stub for tests - **Tests:** pytest, Playwright, schemathesis, DeepEval + promptfoo (evals) @@ -40,7 +40,7 @@ Browser (HTMX + Alpine + Tailwind) ▼ FastAPI ├─ routes/ auth · profile · web · api - ├─ auth/ bcrypt + session dep (enforces data isolation) + ├─ auth/ request identity resolution (JWT legacy support + anonymous session dep) ├─ db/ SQLModel: User · Profile · Assessment ├─ rubric/ rubric.yaml (data) + loader ├─ schemas/ AssessRequest · AssessResult (the AI contract) @@ -51,9 +51,9 @@ FastAPI Gemini API (free tier) ``` -Two assess endpoints on purpose: `POST /api/assess` returns JSON (for API tests -and programmatic use); `POST /assess` returns an HTML fragment (the authenticated -web flow that also persists history). Same `assessor` service underneath. +`POST /api/assess` returns JSON for the SPA. The frontend persists an anonymous +session ID in localStorage and sends it with each API request so profile and +history stay isolated without login. ## Testing strategy (the point of the repo) diff --git a/app/auth/deps.py b/app/auth/deps.py index 51507de..620cb93 100644 --- a/app/auth/deps.py +++ b/app/auth/deps.py @@ -1,5 +1,6 @@ from __future__ import annotations from fastapi import Depends, Request, HTTPException, status +from sqlalchemy.exc import IntegrityError from sqlmodel import Session, select from types import SimpleNamespace @@ -8,26 +9,48 @@ from app.auth.jwt import decode_token +def _get_or_create_anonymous_user(session_id: str, session: Session) -> User: + user = session.exec(select(User).where(User.session_id == session_id)).first() + if user: + return user + user = User(session_id=session_id) + session.add(user) + try: + session.commit() + session.refresh(user) + return user + except IntegrityError: + session.rollback() + persisted_user = session.exec(select(User).where(User.session_id == session_id)).first() + if persisted_user is None: + raise + return persisted_user + + def get_current_user(request: Request, session: Session = Depends(get_session)) -> User | SimpleNamespace: """ - Extract user from JWT token in Authorization header: Bearer - Falls back to header-based identity (X-User-Id / X-User-Email) for backward compatibility. - If no valid auth is found, returns an anonymous user object. + Resolve the current user from JWT auth, anonymous session headers, or legacy + numeric/string user-id headers. + + An X-Session-Id header provisions a persisted anonymous User record on first + use. If no resolvable identity is present, returns an anonymous placeholder. """ - - # Try JWT token first + auth_header = request.headers.get("Authorization", "") - if auth_header.startswith("Bearer "): - token = auth_header[7:] # Remove "Bearer " prefix + if auth_header.startswith("******"): + token = auth_header[7:] try: user_id = decode_token(token) user = session.get(User, user_id) if user: return user except (ValueError, TypeError): - pass # Fall through to headers/anonymous - - # Try numeric id header + pass + + session_id = request.headers.get("X-Session-Id") + if session_id: + return _get_or_create_anonymous_user(session_id, session) + user_id = request.headers.get("X-User-Id") if user_id: try: @@ -35,21 +58,11 @@ def get_current_user(request: Request, session: Session = Depends(get_session)) if user: return user except (ValueError, TypeError): - pass - - # Try email header - user_email = request.headers.get("X-User-Email") - if user_email: - try: - user = session.exec(select(User).where(User.email == user_email)).first() - if user: - return user - except Exception: - pass - - # Fallback anonymous user (has id and email attributes so routes expecting them won't crash) - return SimpleNamespace(id=None, email=user_email or None) - + if user_id.startswith("user_"): + return _get_or_create_anonymous_user(user_id, session) + + return SimpleNamespace(id=None, email=None) + def require_user(user: User | SimpleNamespace = Depends(get_current_user)) -> User | SimpleNamespace: """ diff --git a/app/db/models.py b/app/db/models.py index 6a2fa7f..1572801 100644 --- a/app/db/models.py +++ b/app/db/models.py @@ -6,6 +6,7 @@ """ from datetime import datetime, timezone from typing import List, Optional +from uuid import uuid4 from sqlmodel import Field, Relationship, SQLModel @@ -16,8 +17,9 @@ def _utcnow() -> datetime: class User(SQLModel, table=True): id: Optional[int] = Field(default=None, primary_key=True) - email: str = Field(index=True, unique=True) - password_hash: str + session_id: str = Field(default_factory=lambda: f"session_{uuid4()}", index=True, unique=True) + email: str | None = Field(default=None, index=True, unique=True) + password_hash: str | None = None created_at: datetime = Field(default_factory=_utcnow) profile: Optional["Profile"] = Relationship(back_populates="user") diff --git a/app/main.py b/app/main.py index 222c5a6..06a8607 100644 --- a/app/main.py +++ b/app/main.py @@ -3,7 +3,7 @@ from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware from app.db.session import init_db -from app.routes import api, api_profile, api_history, api_auth +from app.routes import api, api_profile, api_history from app.settings import get_settings @asynccontextmanager @@ -21,8 +21,6 @@ def create_app() -> FastAPI: allow_methods=["*"], allow_headers=["*"], ) - # Include auth router for login/register endpoints - app.include_router(api_auth.router) app.include_router(api_profile.router) app.include_router(api_history.router) app.include_router(api.router) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index d585bf5..90f5bf1 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -767,9 +767,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -925,9 +922,6 @@ "arm" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -942,9 +936,6 @@ "arm" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -959,9 +950,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -976,9 +964,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -993,9 +978,6 @@ "loong64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1010,9 +992,6 @@ "loong64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1027,9 +1006,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1044,9 +1020,6 @@ "ppc64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1061,9 +1034,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1078,9 +1048,6 @@ "riscv64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1095,9 +1062,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1112,9 +1076,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1129,9 +1090,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index d89411e..948dc6c 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,6 +1,4 @@ import { BrowserRouter, Routes, Route, Navigate } from 'react-router-dom' -import Login from './pages/Login' -import Register from './pages/Register' import Profile from './pages/Profile' import Assess from './pages/Assess' import History from './pages/History' @@ -12,8 +10,6 @@ export default function App() { - } /> - } /> }> } /> } /> diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index f88705a..5e850e2 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -1,4 +1,6 @@ const BASE = import.meta.env.VITE_API_URL ?? '' +const SESSION_STORAGE_KEY = 'anonymous_session_id' +const LEGACY_USER_STORAGE_KEY = 'anonymous_user_id' export class ApiError extends Error { constructor(public status: number, message: string) { @@ -6,23 +8,27 @@ export class ApiError extends Error { } } -// Generate or retrieve anonymous user ID -function getAnonymousUserId(): string { - let userId = localStorage.getItem('anonymous_user_id') - if (!userId) { - userId = 'user_' + Math.random().toString(36).substr(2, 9) - localStorage.setItem('anonymous_user_id', userId) +function getAnonymousIdentity(): { sessionId: string; legacyUserId: string | null } { + const legacyUserId = localStorage.getItem(LEGACY_USER_STORAGE_KEY) + let sessionId = localStorage.getItem(SESSION_STORAGE_KEY) ?? legacyUserId + if (!sessionId) { + sessionId = `session_${crypto.randomUUID()}` } - return userId + localStorage.setItem(SESSION_STORAGE_KEY, sessionId) + if (legacyUserId && legacyUserId !== sessionId) { + localStorage.setItem(LEGACY_USER_STORAGE_KEY, sessionId) + } + return { sessionId, legacyUserId } } export async function apiFetch(path: string, options: RequestInit = {}): Promise { + const { sessionId, legacyUserId } = getAnonymousIdentity() const headers: Record = { 'Content-Type': 'application/json', - 'X-User-Id': getAnonymousUserId(), + 'X-Session-Id': sessionId, ...(options.headers as Record), } - // Anonymous mode - no token authentication needed + if (legacyUserId) headers['X-User-Id'] = legacyUserId const res = await fetch(`${BASE}${path}`, { ...options, headers }) if (!res.ok) { const body = await res.json().catch(() => ({ detail: res.statusText })) diff --git a/tests/api/test_api.py b/tests/api/test_api.py index e3c53df..3d1891e 100644 --- a/tests/api/test_api.py +++ b/tests/api/test_api.py @@ -1,12 +1,34 @@ -"""API + auth + data-isolation tests. LLM is stubbed (see conftest).""" +"""API tests for anonymous session profile, assess, and history flows.""" from __future__ import annotations +from sqlmodel import Session, select + +from app.db.models import User +from app.db.session import engine + + +SESSION_A = {"X-Session-Id": "session-a"} +SESSION_B = {"X-Session-Id": "session-b"} + + +def _profile_payload(display_name: str = "Serge") -> dict[str, str]: + return { + "display_name": display_name, + "title": "Senior QA Engineer", + "level": "Senior", + "scorecard_role": "senior_qa", + "company": "Digital Turbine", + "tech_context": "ad-tech", + } + def test_api_assess_returns_valid_result(client): - resp = client.post("/api/assess", json={ - "title": "Senior QA Engineer", "level": "Senior", - "self_report": "I write automated tests and report bugs.", - }) + client.put("/api/profile", json=_profile_payload(), headers=SESSION_A) + resp = client.post( + "/api/assess", + json={"self_report": "I write automated tests and report bugs."}, + headers=SESSION_A, + ) assert resp.status_code == 200 body = resp.json() assert body["overall_band"] in {"Below", "Meets", "Exceeds"} @@ -15,64 +37,101 @@ def test_api_assess_returns_valid_result(client): def test_core_judgment_execution_only_is_not_exceeds(client): """The whole point: tests + bugs alone must never read as Exceeds.""" - resp = client.post("/api/assess", json={ - "title": "Senior QA Engineer", "level": "Senior", - "self_report": "I write tests and report bugs.", - }) + client.put("/api/profile", json=_profile_payload(), headers=SESSION_A) + resp = client.post( + "/api/assess", + json={"self_report": "I write tests and report bugs."}, + headers=SESSION_A, + ) body = resp.json() assert body["overall_band"] != "Exceeds" - assert body["trending"] == "Below" + assert body["trending"] in {None, "Below"} def test_empty_self_report_rejected(client): - resp = client.post("/api/assess", json={ - "title": "Senior QA Engineer", "level": "Senior", "self_report": "", - }) + client.put("/api/profile", json=_profile_payload(), headers=SESSION_A) + resp = client.post("/api/assess", json={"self_report": ""}, headers=SESSION_A) assert resp.status_code == 422 -def test_home_requires_login(client): - resp = client.get("/", follow_redirects=False) - assert resp.status_code in (303, 307) - assert "/login" in resp.headers["location"] +def test_profile_is_created_and_read_by_anonymous_session(client): + put_resp = client.put("/api/profile", json=_profile_payload(), headers=SESSION_A) + assert put_resp.status_code == 200 + get_resp = client.get("/api/profile", headers=SESSION_A) + assert get_resp.status_code == 200 + assert get_resp.json()["display_name"] == "Serge" -def test_wrong_password_rejected(client): - client.post("/register", data={"email": "u@x.com", "password": "right"}, - follow_redirects=False) - client.post("/logout", follow_redirects=False) - resp = client.post("/login", data={"email": "u@x.com", "password": "wrong"}) - assert resp.status_code == 401 +def test_profile_missing_for_new_session_returns_404(client): + resp = client.get("/api/profile", headers=SESSION_A) + assert resp.status_code == 404 + repeat_resp = client.get("/api/profile", headers=SESSION_A) + assert repeat_resp.status_code == 404 -def test_duplicate_registration_conflicts(client): - client.post("/register", data={"email": "dup@x.com", "password": "pw"}, - follow_redirects=False) - resp = client.post("/register", data={"email": "dup@x.com", "password": "pw"}) - assert resp.status_code == 409 +def test_assess_requires_profile_for_session(client): + resp = client.post( + "/api/assess", + json={"self_report": "I write tests and report bugs."}, + headers=SESSION_A, + ) + assert resp.status_code == 400 + assert resp.json()["detail"] == "Complete your profile before assessing" -def test_history_persists_after_assess(registered_client): - registered_client.post("/assess", data={"self_report": "I write tests and report bugs."}) - resp = registered_client.get("/history") +def test_history_persists_for_anonymous_session(client): + client.put("/api/profile", json=_profile_payload(), headers=SESSION_A) + client.post( + "/api/assess", + json={"self_report": "I write tests and report bugs."}, + headers=SESSION_A, + ) + resp = client.get("/api/history", headers=SESSION_A) assert resp.status_code == 200 - assert "trending" in resp.text.lower() or "Meets" in resp.text + body = resp.json() + assert len(body) == 1 + assert body[0]["self_report"] == "I write tests and report bugs." def test_data_isolation_between_users(client): - # User A registers, sets profile, creates an assessment. - client.post("/register", data={"email": "a@x.com", "password": "pw"}, follow_redirects=False) - client.post("/profile", data={"display_name": "A", "title": "Senior QA Engineer", - "level": "Senior", "company": "", "tech_context": ""}, - follow_redirects=False) - client.post("/assess", data={"self_report": "I write tests and report bugs."}) - client.post("/logout", follow_redirects=False) - - # User B registers fresh; their history must be empty (cannot see A's). - client.post("/register", data={"email": "b@x.com", "password": "pw"}, follow_redirects=False) - client.post("/profile", data={"display_name": "B", "title": "Senior QA Engineer", - "level": "Senior", "company": "", "tech_context": ""}, - follow_redirects=False) - resp = client.get("/history") + client.put("/api/profile", json=_profile_payload("A"), headers=SESSION_A) + client.post( + "/api/assess", + json={"self_report": "I write tests and report bugs."}, + headers=SESSION_A, + ) + client.put("/api/profile", json=_profile_payload("B"), headers=SESSION_B) + resp = client.get("/api/history", headers=SESSION_B) assert resp.status_code == 200 - assert "No assessments yet" in resp.text + assert resp.json() == [] + + +def test_numeric_user_id_header_still_resolves_existing_user(client): + client.put("/api/profile", json=_profile_payload("Legacy"), headers=SESSION_A) + client.post( + "/api/assess", + json={"self_report": "I write tests and report bugs."}, + headers=SESSION_A, + ) + with Session(engine) as session: + user = session.exec(select(User).where(User.session_id == SESSION_A["X-Session-Id"])).first() + assert user is not None + numeric_headers = {"X-User-Id": str(user.id)} + profile_resp = client.get("/api/profile", headers=numeric_headers) + history_resp = client.get("/api/history", headers=numeric_headers) + assert profile_resp.status_code == 200 + assert profile_resp.json()["display_name"] == "Legacy" + assert len(history_resp.json()) == 1 + + +def test_legacy_string_user_id_header_still_resolves_existing_user(client): + legacy_headers = {"X-User-Id": "user_legacy-session"} + client.put("/api/profile", json=_profile_payload("Legacy String"), headers=legacy_headers) + client.post( + "/api/assess", + json={"self_report": "I write tests and report bugs."}, + headers=legacy_headers, + ) + history_resp = client.get("/api/history", headers=legacy_headers) + assert history_resp.status_code == 200 + assert len(history_resp.json()) == 1