diff --git a/SECURITY.md b/SECURITY.md index 64a69ec..1ca7fba 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -24,6 +24,8 @@ land. short-lived access tokens plus refresh tokens (hashed with SHA-256 in the database) via `/auth/register`, `/auth/login`, `/auth/refresh`, `/auth/logout`, and `/auth/me`. Passwords are hashed with bcrypt. + Refresh rotation requires a non-revoked token whose `expires_at` is still in + the future; expired tokens return 401 and are not rotated. - An `APIKey` model (`si_`-prefixed, SHA-256 hashed, with scopes and rate limits) exists in the schema, but no route currently authenticates via it — JWT bearer tokens are the active mechanism. diff --git a/pyproject.toml b/pyproject.toml index 235821e..fa0459e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -58,7 +58,7 @@ airflow = [ dev = [ "pytest>=8.2.0", "pytest-asyncio>=0.23.6", - "ruff>=0.4.4", + "ruff>=0.4.4,<0.16", "pre-commit>=3.7.0", ] diff --git a/startupintel/api/routes/auth.py b/startupintel/api/routes/auth.py index b3383b8..4f64826 100644 --- a/startupintel/api/routes/auth.py +++ b/startupintel/api/routes/auth.py @@ -95,11 +95,13 @@ async def login( ) refresh_token, token_hash = create_refresh_token(user_id=user.id) - db.add(RefreshToken( - user_id=user.id, - token_hash=token_hash, - expires_at=datetime.now(UTC) + timedelta(days=30), - )) + db.add( + RefreshToken( + user_id=user.id, + token_hash=token_hash, + expires_at=datetime.now(UTC) + timedelta(days=30), + ) + ) user.last_login_at = datetime.now(UTC) await db.commit() await db.refresh(user) @@ -126,20 +128,36 @@ async def refresh( ) from uuid import UUID + user_id = UUID(payload["sub"]) token_hash = hashlib.sha256(request.refresh_token.encode()).hexdigest() + now = datetime.now(UTC) - stmt = ( - select(RefreshToken) - .where( + stmt = select(RefreshToken).where( + RefreshToken.token_hash == token_hash, + RefreshToken.user_id == user_id, + RefreshToken.revoked_at.is_(None), + RefreshToken.expires_at > now, + ) + db_token = (await db.execute(stmt)).scalar_one_or_none() + + if not db_token: + # Distinguish revoked vs expired for clearer client handling when possible. + expired_or_revoked = select(RefreshToken).where( RefreshToken.token_hash == token_hash, RefreshToken.user_id == user_id, - RefreshToken.revoked_at.is_(None), ) - ) - db_token = (await db.execute(stmt)).scalar_one_or_none() + existing = (await db.execute(expired_or_revoked)).scalar_one_or_none() + if existing is not None and existing.is_expired and existing.revoked_at is None: + detail = "Refresh token expired" + else: + detail = "Refresh token revoked or expired" + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail=detail, + ) - if not db_token or db_token.is_revoked: + if db_token.is_expired or db_token.revoked_at is not None: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Refresh token revoked or expired", @@ -157,11 +175,13 @@ async def refresh( new_refresh_token, new_hash = create_refresh_token(user_id=user.id) db_token.revoked_at = datetime.now(UTC) - db.add(RefreshToken( - user_id=user.id, - token_hash=new_hash, - expires_at=datetime.now(UTC) + timedelta(days=30), - )) + db.add( + RefreshToken( + user_id=user.id, + token_hash=new_hash, + expires_at=datetime.now(UTC) + timedelta(days=30), + ) + ) await db.commit() await db.refresh(user) diff --git a/startupintel/db/models.py b/startupintel/db/models.py index 7ca2014..b67fa49 100644 --- a/startupintel/db/models.py +++ b/startupintel/db/models.py @@ -42,18 +42,24 @@ class Startup(Base): class StartupScore(Base): __tablename__ = "startup_scores" __table_args__ = ( - UniqueConstraint("startup_id", "bot_name", "computed_at", name="uq_startup_bot_computed_at"), + UniqueConstraint( + "startup_id", "bot_name", "computed_at", name="uq_startup_bot_computed_at" + ), ) id: Mapped[UUID] = mapped_column(primary_key=True, default=uuid4) - startup_id: Mapped[UUID] = mapped_column(ForeignKey("startups.id", ondelete="CASCADE"), index=True) + startup_id: Mapped[UUID] = mapped_column( + ForeignKey("startups.id", ondelete="CASCADE"), index=True + ) bot_name: Mapped[str] = mapped_column(String(40), nullable=False, index=True) score: Mapped[float] = mapped_column(Float, nullable=False) signal_breakdown: Mapped[dict] = mapped_column(JSONB, default=dict) llm_diagnosis: Mapped[str | None] = mapped_column(Text) similar_cases: Mapped[list] = mapped_column(JSONB, default=list) raw_signals: Mapped[dict] = mapped_column(JSONB, default=dict) - computed_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=utcnow, index=True) + computed_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), default=utcnow, index=True + ) startup: Mapped[Startup] = relationship(back_populates="scores") @@ -114,9 +120,13 @@ class HeadcountSnapshot(Base): __tablename__ = "headcount_snapshots" id: Mapped[UUID] = mapped_column(primary_key=True, default=uuid4) - startup_id: Mapped[UUID] = mapped_column(ForeignKey("startups.id", ondelete="CASCADE"), index=True) + startup_id: Mapped[UUID] = mapped_column( + ForeignKey("startups.id", ondelete="CASCADE"), index=True + ) headcount: Mapped[int] = mapped_column(Integer, nullable=False) - snapshot_date: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False, index=True) + snapshot_date: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, index=True + ) source: Mapped[str] = mapped_column(String(80), nullable=False) startup: Mapped[Startup] = relationship(back_populates="headcount_snapshots") @@ -126,15 +136,18 @@ class SignalEvent(Base): __tablename__ = "signal_events" id: Mapped[UUID] = mapped_column(primary_key=True, default=uuid4) - startup_id: Mapped[UUID] = mapped_column(ForeignKey("startups.id", ondelete="CASCADE"), index=True) + startup_id: Mapped[UUID] = mapped_column( + ForeignKey("startups.id", ondelete="CASCADE"), index=True + ) event_type: Mapped[str] = mapped_column(String(120), nullable=False, index=True) payload: Mapped[dict] = mapped_column(JSONB, default=dict) - emitted_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=utcnow, index=True) + emitted_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), default=utcnow, index=True + ) startup: Mapped[Startup] = relationship(back_populates="signal_events") - class Organization(Base): __tablename__ = "organizations" @@ -163,7 +176,9 @@ class User(Base): hashed_password: Mapped[str] = mapped_column(String(255), nullable=False) first_name: Mapped[str | None] = mapped_column(String(100)) last_name: Mapped[str | None] = mapped_column(String(100)) - role: Mapped[str] = mapped_column(String(20), default="analyst", index=True) # admin, analyst, viewer + role: Mapped[str] = mapped_column( + String(20), default="analyst", index=True + ) # admin, analyst, viewer is_active: Mapped[bool] = mapped_column(default=True) email_verified: Mapped[bool] = mapped_column(default=False) last_login_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) @@ -198,19 +213,32 @@ class RefreshToken(Base): user: Mapped[User] = relationship(back_populates="refresh_tokens") + @property + def is_expired(self) -> bool: + """True when expires_at is in the past (timezone-safe).""" + expires = self.expires_at + if expires.tzinfo is None: + expires = expires.replace(tzinfo=UTC) + return expires < utcnow() + @property def is_revoked(self) -> bool: - return self.revoked_at is not None or self.expires_at < utcnow() + """True when explicitly revoked or past expires_at.""" + return self.revoked_at is not None or self.is_expired class APIKey(Base): __tablename__ = "api_keys" id: Mapped[UUID] = mapped_column(primary_key=True, default=uuid4) - organization_id: Mapped[UUID] = mapped_column(ForeignKey("organizations.id", ondelete="CASCADE"), index=True) + organization_id: Mapped[UUID] = mapped_column( + ForeignKey("organizations.id", ondelete="CASCADE"), index=True + ) user_id: Mapped[UUID | None] = mapped_column(ForeignKey("users.id", ondelete="SET NULL")) name: Mapped[str] = mapped_column(String(100), nullable=False) - key_prefix: Mapped[str] = mapped_column(String(8), nullable=False, index=True) # First 8 chars for identification + key_prefix: Mapped[str] = mapped_column( + String(8), nullable=False, index=True + ) # First 8 chars for identification key_hash: Mapped[str] = mapped_column(String(255), nullable=False, unique=True) scopes: Mapped[list] = mapped_column(JSONB, default=list) # ["read", "write", "admin"] rate_limit_per_minute: Mapped[int] = mapped_column(Integer, default=60) @@ -281,5 +309,3 @@ def is_accessible(self) -> bool: if self.virus_scan_status == "infected": return False return True - - diff --git a/tests/test_api/test_auth_api.py b/tests/test_api/test_auth_api.py index 9ce2085..239eab2 100644 --- a/tests/test_api/test_auth_api.py +++ b/tests/test_api/test_auth_api.py @@ -8,18 +8,25 @@ pytestmark = pytest.mark.asyncio -async def _register(client: AsyncClient, *, email: str = "alice@example.com", password: str = "str0ngP@ss") -> dict: - resp = await client.post("/auth/register", json={ - "email": email, - "password": password, - "first_name": "Alice", - "last_name": "Tester", - }) +async def _register( + client: AsyncClient, *, email: str = "alice@example.com", password: str = "str0ngP@ss" +) -> dict: + resp = await client.post( + "/auth/register", + json={ + "email": email, + "password": password, + "first_name": "Alice", + "last_name": "Tester", + }, + ) assert resp.status_code == 201, resp.text return resp.json() -async def _login(client: AsyncClient, *, email: str = "alice@example.com", password: str = "str0ngP@ss") -> dict: +async def _login( + client: AsyncClient, *, email: str = "alice@example.com", password: str = "str0ngP@ss" +) -> dict: resp = await client.post("/auth/login", json={"email": email, "password": password}) assert resp.status_code == 200, resp.text return resp.json() @@ -34,10 +41,13 @@ async def test_register_creates_user_and_org(client: AsyncClient): async def test_register_duplicate_email_rejects(client: AsyncClient): await _register(client) - resp = await client.post("/auth/register", json={ - "email": "alice@example.com", - "password": "str0ngP@ss", - }) + resp = await client.post( + "/auth/register", + json={ + "email": "alice@example.com", + "password": "str0ngP@ss", + }, + ) assert resp.status_code == 400 assert "already registered" in resp.json()["detail"].lower() @@ -53,10 +63,13 @@ async def test_login_returns_tokens(client: AsyncClient): async def test_login_wrong_password_rejects(client: AsyncClient): await _register(client) - resp = await client.post("/auth/login", json={ - "email": "alice@example.com", - "password": "wrong", - }) + resp = await client.post( + "/auth/login", + json={ + "email": "alice@example.com", + "password": "wrong", + }, + ) assert resp.status_code == 401 @@ -64,9 +77,12 @@ async def test_refresh_rotates_token(client: AsyncClient): await _register(client) tokens = await _login(client) - resp = await client.post("/auth/refresh", json={ - "refresh_token": tokens["refresh_token"], - }) + resp = await client.post( + "/auth/refresh", + json={ + "refresh_token": tokens["refresh_token"], + }, + ) assert resp.status_code == 200 new_tokens = resp.json() assert new_tokens["access_token"] != tokens["access_token"] @@ -85,13 +101,41 @@ async def test_refresh_revoked_token_rejects(client: AsyncClient): assert resp.status_code == 401 +async def test_refresh_expired_token_rejects(client: AsyncClient, db_session): + """Expired refresh tokens must not rotate into a new pair (#41).""" + from datetime import UTC, datetime, timedelta + import hashlib + + from sqlalchemy import select + + from startupintel.db.models import RefreshToken + + await _register(client) + tokens = await _login(client) + token_hash = hashlib.sha256(tokens["refresh_token"].encode()).hexdigest() + + result = await db_session.execute( + select(RefreshToken).where(RefreshToken.token_hash == token_hash) + ) + row = result.scalar_one() + row.expires_at = datetime.now(UTC) - timedelta(seconds=1) + await db_session.commit() + + resp = await client.post("/auth/refresh", json={"refresh_token": tokens["refresh_token"]}) + assert resp.status_code == 401 + assert "expired" in resp.json()["detail"].lower() + + async def test_me_returns_profile(client: AsyncClient): await _register(client) tokens = await _login(client) - resp = await client.get("/auth/me", headers={ - "Authorization": f"Bearer {tokens['access_token']}", - }) + resp = await client.get( + "/auth/me", + headers={ + "Authorization": f"Bearer {tokens['access_token']}", + }, + ) assert resp.status_code == 200 assert resp.json()["email"] == "alice@example.com" @@ -105,14 +149,20 @@ async def test_logout_revokes_token(client: AsyncClient): await _register(client) tokens = await _login(client) - resp = await client.post("/auth/logout", json={ - "refresh_token": tokens["refresh_token"], - }) + resp = await client.post( + "/auth/logout", + json={ + "refresh_token": tokens["refresh_token"], + }, + ) assert resp.status_code == 200 assert "logged out" in resp.json()["message"].lower() # revoked token should fail on refresh - resp = await client.post("/auth/refresh", json={ - "refresh_token": tokens["refresh_token"], - }) + resp = await client.post( + "/auth/refresh", + json={ + "refresh_token": tokens["refresh_token"], + }, + ) assert resp.status_code == 401