Skip to content
Merged
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
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
*secret*
*token*

# The broad secret-name guards above are meant for secret *files*; never let
# them swallow tracked Python source or DB migrations (e.g. a migration named
# "...users_api_keys...py"). Secrets are never .py files.
!**/*.py

# Python cache
__pycache__/
*.py[cod]
Expand Down Expand Up @@ -76,3 +81,4 @@ site/
# Serena local tooling
.serena/memories/
.serena/project.local.yml
tests/_intelligent_cleaning_output/
47 changes: 46 additions & 1 deletion ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
| API Framework | FastAPI | >=0.115.0 | ADR-001 |
| Database | PostgreSQL + SQLAlchemy | 16 + 2.0 | ADR-002 |
| Job Queue | ARQ | >=0.26.0 | ADR-003 |
| Authentication | API Key (SHA-256 hashed) | — | ADR-004 |
| Authentication | API Key (SHA-256) + Clerk JWT (dashboard) | — | ADR-004, ADR-008 |
| Rate Limiting | slowapi + Redis | — | ADR-005 |
| Billing | Stripe | — | ADR-006 |
| Deployment | Railway | — | ADR-007 |
Expand Down Expand Up @@ -280,6 +280,51 @@

---

### ADR-008 — Dual Authentication: API Keys (programmatic) + Clerk (dashboard)

| Field | Value |
|-------|-------|
| **Date** | 2026-06-17 |
| **Status** | Accepted (supersedes the single-auth assumption of ADR-004) |

**Context:** ADR-004 chose hashed API keys as the sole auth mechanism, optimised
for programmatic/SDK access. A developer dashboard (SC001) was subsequently built
and needs interactive, browser-based human login (sessions, sign-up, social login)
— a poor fit for raw API keys. This was discovered during the 2026-06-17 baseline
reconciliation as an undocumented divergence from ADR-004.

**Decision:** Run **two complementary auth mechanisms**, segmented by surface:

| Surface | Mechanism | Identity | Verified by |
|---------|-----------|----------|-------------|
| Programmatic API (`/api/v1/*`) | API key (`pk_…`, SHA-256 hashed) | `User` via `APIKey` | `APIKeyRepository.verify` |
| Developer dashboard (`/api/v1/dashboard/*`) | Clerk RS256 JWT | `User.clerk_user_id` (verified `sub`) | `clerk_auth.get_dashboard_user` |

Both resolve to the **same `User`** record, so billing/tiers/usage are shared.
A dashboard user provisions/links to a `User` on first login; the account's stable
identity is the verified Clerk `sub`, never a client-supplied email.

**Alternatives Considered:**

| Alternative | Why Rejected |
|-------------|-------------|
| API keys only (original ADR-004) | No session/sign-up/social-login UX for a browser dashboard |
| Clerk only | Breaks existing SDK/programmatic flows that depend on API keys |
| Roll our own JWT/session auth | Re-implements password reset, MFA, social login — high cost & risk |

**Consequences:**
- Two code paths to secure. The Clerk path is hardened (issuer + `azp` verified,
RS256/`kid` asserted, verified-email-only, dormant-only account linking).
- Shared `User` model keeps billing/usage coherent across both surfaces.
- New config required in production: `clerk_jwks_url`, `clerk_issuer`,
`clerk_authorized_parties`. Auth fails closed if the issuer is unset.

**Revisit When:**
- If Clerk becomes the primary identity, consider issuing API keys *from* the
dashboard and deprecating standalone API-key signup.

---

## Data Flow

### Scraping Request Flow
Expand Down
26 changes: 14 additions & 12 deletions PROJECT-STATUS.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ dashboard (backend + React/TypeScript frontend) — **delivered**, no original i
|---|------|-------|------------|--------|
| R1 | crawl4ai breaking changes | maint | Pin version, integration tests | Monitoring |
| R2 | OpenAI API cost spikes | maint | Usage limits, cost tracking | Monitoring |
| R4 | Baseline quality debt (89 ruff errors, 10 test-collection errors) | maint | Dedicated cleanup cycle before next feature work | Open |
| R5 | Clerk auth (SC001) diverges from ADR-004 API-key decision | maint | Document dual-auth model or supersede ADR-004 | Open |
| R4 | Baseline quality debt | maint | Resolved 2026-06-17: ruff clean, mypy clean (api+src), 570 tests pass | ✅ Closed |
| R5 | Clerk auth (SC001) diverges from ADR-004 | maint | Resolved 2026-06-17: documented in ADR-008 (dual-auth model) | ✅ Closed |
| R6 | Security debt in dashboard auth path | maint | Resolved 2026-06-17: JWT issuer/azp/RS256 hardened, squat-proof + race-safe, 15 tests | ✅ Closed |

---

Expand All @@ -98,13 +99,14 @@ dashboard (backend + React/TypeScript frontend) — **delivered**, no original i

## Quality Metrics

| Metric | Current | Target |
|--------|---------|--------|
| pytest | 307 collected, 10 collection errors | 0 errors |
| ruff Warnings | 89 errors (66 auto-fixable) | 0 |
| Test Coverage | Not measured (collection errors block run) | 80% |
| mypy Errors | Not measured | 0 |

> Quality debt is **expected** for a legacy baseline. Recommended first future
> cycle: a cleanup phase (`ruff --fix`, resolve test-collection errors) before new
> features. These are tracked as risks R4/R5 above, not as silent passes.
| Metric | Current (2026-06-17) | Target |
|--------|----------------------|--------|
| pytest | 570 passed, 3 skipped, 0 errors | 0 errors |
| ruff | All checks passed | 0 |
| mypy (api/) | Clean (36 files) | 0 |
| mypy (src/) | Clean (19 files, gradual config) | 0 |

> Production-hardening pass (2026-06-17) closed R4/R5/R6: security-hardened the
> dashboard auth path, brought the test suite from 307→570 passing (fixing a real
> shared-registry concurrency bug and several cleaning-engine bugs), and made
> lint + types clean. Tooling config (ruff/mypy/pytest) added to pyproject.
4 changes: 4 additions & 0 deletions alembic.ini
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
[alembic]
script_location = alembic

# Put the repository root on sys.path so env.py can import the `api` package
# (alembic does not add the invocation cwd automatically).
prepend_sys_path = .

# Blank — env.py reads the URL from settings.
sqlalchemy.url =

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
"""initial schema: users, api_keys, jobs, usage_records

Revision ID: 69c02554
Revises:
Create Date: 2026-03-22 00:00:00.000000

"""
from __future__ import annotations

from typing import Sequence, Union

import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql

# revision identifiers, used by Alembic.
revision: str = "69c02554"
down_revision: Union[str, None] = None
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
# --- users -----------------------------------------------------------
op.create_table(
"users",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("user_id", sa.String(length=64), nullable=False),
sa.Column("email", sa.String(length=255), nullable=False),
sa.Column("name", sa.String(length=255), nullable=True),
sa.Column("tier", sa.String(length=32), nullable=False),
sa.Column("stripe_customer_id", sa.String(length=64), nullable=True),
sa.Column("stripe_subscription_id", sa.String(length=64), nullable=True),
sa.Column("created_at", sa.DateTime(timezone=True), nullable=False),
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("email"),
sa.UniqueConstraint("user_id"),
)
op.create_index("ix_users_email", "users", ["email"])
op.create_index("ix_users_user_id", "users", ["user_id"])

# --- api_keys --------------------------------------------------------
op.create_table(
"api_keys",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("key_id", sa.String(length=64), nullable=False),
sa.Column("user_id", sa.String(length=64), nullable=False),
sa.Column("name", sa.String(length=100), nullable=False),
sa.Column("prefix", sa.String(length=32), nullable=False),
sa.Column("key_hash", sa.String(length=255), nullable=False),
sa.Column("is_active", sa.Boolean(), nullable=False),
sa.Column("created_at", sa.DateTime(timezone=True), nullable=False),
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("key_hash"),
sa.UniqueConstraint("key_id"),
)
op.create_index("ix_api_keys_key_hash", "api_keys", ["key_hash"])
op.create_index("ix_api_keys_key_id", "api_keys", ["key_id"])
op.create_index("ix_api_keys_user_id", "api_keys", ["user_id"])

# --- jobs ------------------------------------------------------------
op.create_table(
"jobs",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("job_id", sa.String(length=64), nullable=False),
sa.Column("status", sa.String(length=32), nullable=False),
sa.Column("url", sa.Text(), nullable=True),
sa.Column("job_type", sa.String(length=32), nullable=False),
sa.Column("max_pages", sa.Integer(), nullable=True),
sa.Column("output_format", sa.String(length=32), nullable=True),
sa.Column("webhook_url", sa.Text(), nullable=True),
sa.Column("created_at", sa.DateTime(timezone=True), nullable=False),
sa.Column("started_at", sa.DateTime(timezone=True), nullable=True),
sa.Column("completed_at", sa.DateTime(timezone=True), nullable=True),
sa.Column("pages_scraped", sa.Integer(), nullable=False),
sa.Column("pages_failed", sa.Integer(), nullable=False),
sa.Column("progress", sa.Float(), nullable=False),
sa.Column("error_message", sa.Text(), nullable=True),
sa.Column("output_files", postgresql.JSONB(astext_type=sa.Text()), nullable=True),
sa.Column("summary", postgresql.JSONB(astext_type=sa.Text()), nullable=True),
sa.Column("owner_key_id", sa.String(length=64), nullable=True),
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("job_id"),
)
op.create_index("ix_jobs_job_id", "jobs", ["job_id"])
op.create_index("ix_jobs_owner_key_id", "jobs", ["owner_key_id"])
op.create_index("ix_jobs_owner_key_id_status", "jobs", ["owner_key_id", "status"])

# --- usage_records ---------------------------------------------------
op.create_table(
"usage_records",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("timestamp", sa.DateTime(timezone=True), nullable=False),
sa.Column("endpoint", sa.String(length=255), nullable=False),
sa.Column("method", sa.String(length=10), nullable=False),
sa.Column("status_code", sa.Integer(), nullable=False),
sa.Column("response_time_ms", sa.Float(), nullable=False),
sa.Column("key_id", sa.String(length=64), nullable=True),
sa.Column("pages_count", sa.Integer(), nullable=False),
sa.PrimaryKeyConstraint("id"),
)
op.create_index("ix_usage_records_key_id", "usage_records", ["key_id"])
op.create_index(
"ix_usage_records_key_id_timestamp", "usage_records", ["key_id", "timestamp"]
)


def downgrade() -> None:
op.drop_index("ix_usage_records_key_id_timestamp", table_name="usage_records")
op.drop_index("ix_usage_records_key_id", table_name="usage_records")
op.drop_table("usage_records")

op.drop_index("ix_jobs_owner_key_id_status", table_name="jobs")
op.drop_index("ix_jobs_owner_key_id", table_name="jobs")
op.drop_index("ix_jobs_job_id", table_name="jobs")
op.drop_table("jobs")

op.drop_index("ix_api_keys_user_id", table_name="api_keys")
op.drop_index("ix_api_keys_key_id", table_name="api_keys")
op.drop_index("ix_api_keys_key_hash", table_name="api_keys")
op.drop_table("api_keys")

op.drop_index("ix_users_user_id", table_name="users")
op.drop_index("ix_users_email", table_name="users")
op.drop_table("users")
3 changes: 2 additions & 1 deletion api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ def create_app() -> FastAPI:

# Rate limiting
app.state.limiter = limiter
app.add_exception_handler(RateLimitExceeded, rate_limit_exceeded_handler)
# slowapi's handler signature is narrower than Starlette's typed contract.
app.add_exception_handler(RateLimitExceeded, rate_limit_exceeded_handler) # type: ignore[arg-type]

# --- Routers ---

Expand Down
6 changes: 6 additions & 0 deletions api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ class Settings(BaseSettings):
# Clerk (dashboard auth)
clerk_jwks_url: str = ""
clerk_publishable_key: str = ""
# Expected token issuer (Clerk Frontend API URL, e.g.
# "https://clerk.your-domain.com"). When set, the `iss` claim is verified.
clerk_issuer: str = ""
# Comma-separated allowlist of authorized parties (`azp` claim). Defaults to
# ``dashboard_origin`` when empty. Tokens whose `azp` is not allowed are rejected.
clerk_authorized_parties: str = ""

# Dashboard
dashboard_origin: str = "http://localhost:5173"
Expand Down
2 changes: 1 addition & 1 deletion api/db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class User(Base):
user_id: Mapped[str] = mapped_column(String(64), nullable=False, unique=True)
email: Mapped[str] = mapped_column(String(255), nullable=False, unique=True)
clerk_user_id: Mapped[Optional[str]] = mapped_column(
String(64), nullable=True, unique=True, index=True
String(64), nullable=True, unique=True
)
name: Mapped[Optional[str]] = mapped_column(String(255), nullable=True)
tier: Mapped[str] = mapped_column(String(32), nullable=False, default="free")
Expand Down
106 changes: 106 additions & 0 deletions api/db/repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,112 @@ async def get_by_clerk_user_id(self, clerk_user_id: str) -> Optional[User]:
)
return result.scalar_one_or_none()

async def get_or_create_by_clerk_id(
self,
clerk_user_id: str,
email: str,
name: Optional[str] = None,
email_is_verified: bool = False,
) -> User:
"""Resolve a Clerk identity to a ``User``, creating it on first login.

The account's stable identity is the (server-issued, signature-verified)
``clerk_user_id``, never the client-supplied email — this prevents
account-squatting via a forged or unverified email claim.

Account linking to a pre-existing email-based account happens *only* when
the email is verified, preventing a hijack of an existing account.

Args:
clerk_user_id: Verified Clerk subject (``sub``) claim.
email: Email to associate (a safe ``@clerk.local`` placeholder when
the token carries no verified email).
name: Optional display name.
email_is_verified: Whether ``email`` came from a verified claim.

Returns:
The resolved ``User``.

Raises:
ValueError: If the verified email already belongs to a *different*
Clerk identity (caller should surface this as HTTP 409).
"""
user = await self.get_by_clerk_user_id(clerk_user_id)
if user is not None:
return user

# Security invariant (enforced here, the data layer, not just the
# caller): an unverified or missing email is NEVER stored or linked.
# Fall back to a placeholder derived from the verified Clerk subject so
# a forged email claim can never squat a real address.
if not email_is_verified or not email or email.endswith("@clerk.local"):
email = f"{clerk_user_id}@clerk.local"
email_is_verified = False

# Link to an existing account only via a verified, real email — and
# only when that account is DORMANT (never authenticated: no API keys,
# no Stripe customer). This lets a dormant pre-registration be claimed
# but prevents silently capturing an active account via a verified email.
if email_is_verified and not email.endswith("@clerk.local"):
existing = await self.get_by_email(email)
if existing is not None:
if existing.clerk_user_id is None and await self._is_dormant(
existing
):
existing.clerk_user_id = clerk_user_id
await self._session.flush()
logger.info(
"Linked Clerk ID %s to dormant user %s via verified email",
clerk_user_id,
existing.user_id,
)
return existing
# Active account or already linked → require explicit merge.
raise ValueError(
f"Email {email} already belongs to an existing account"
)

user_id = f"user_{uuid.uuid4().hex[:12]}"
user = User(
user_id=user_id,
email=email,
name=name,
tier="free",
clerk_user_id=clerk_user_id,
created_at=datetime.now(timezone.utc),
)
self._session.add(user)
try:
await self._session.flush()
except IntegrityError:
# Concurrent first-login for the same Clerk ID — return the winner
# rather than leaving a half-initialised row behind.
await self._session.rollback()
winner = await self.get_by_clerk_user_id(clerk_user_id)
if winner is not None:
return winner
raise

logger.info(
"Auto-created user %s for Clerk ID %s", user_id, clerk_user_id
)
return user

async def _is_dormant(self, user: User) -> bool:
"""Return ``True`` if the account has never been activated.

Dormant = no Stripe customer/subscription and no API keys. Only dormant
accounts may be auto-claimed via verified-email linking.
"""
if user.stripe_customer_id or user.stripe_subscription_id:
return False
key_count = await self._session.scalar(
select(func.count())
.select_from(APIKey)
.where(APIKey.user_id == user.user_id)
)
return not key_count


# ---------------------------------------------------------------------------
# APIKeyRepository
Expand Down
Loading
Loading