Skip to content

Commit e289fd1

Browse files
tylerhuntingtonclaudeCopilotlsetiawan
authored
feat: per-client API keys with per-key rate limiting (sustainability-software-lab#248)
* feat: add per-client API keys with rate limiting Introduces X-API-Key authentication as a third auth path alongside Bearer JWT and HTTP-only cookie, so each consumer (frontend, external researcher, partner) gets an individually revocable key with per-key sliding-window rate limiting. Key design choices: - Store only Argon2 hash + 8-char prefix; raw key returned once at creation and never persisted - Prefix-based lookup narrows Argon2 candidates before expensive verify - Rate limit state (window_start/count) stored on the api_key row with SELECT FOR UPDATE to handle concurrent requests - rate_limit_per_minute=0 means unlimited (for trusted internal callers) - Existing JWT/cookie auth unchanged — no regression for browser clients New endpoints (admin-only): POST /v1/auth/api-keys GET /v1/auth/api-keys DELETE /v1/auth/api-keys/{id} PATCH /v1/auth/api-keys/{id} Includes Alembic migration (down_revision=eacbc6544a10) and 14-test suite covering create/list/revoke/update/auth/rate-limit scenarios. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Update src/ca_biositing/webservice/ca_biositing/webservice/services/auth_service.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/ca_biositing/webservice/ca_biositing/webservice/services/auth_service.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update alembic/versions/b3f2d1c8e9a0_add_api_key_table.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/ca_biositing/datamodels/ca_biositing/datamodels/models/auth/api_key.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: address security review findings from PR sustainability-software-lab#248 Resolves all 'before merge' items from @lsetiawan's audit plus two correctness bugs flagged by Copilot: FINDING-01 (HIGH): Add constant-time dummy Argon2 verify when no prefix candidates exist, preventing prefix enumeration via timing side-channel. Mirrors the existing authenticate_user / DUMMY_HASH pattern. FINDING-03 (MEDIUM): Admin endpoints now require JWT-only auth via a new get_jwt_user() dependency. X-API-Key is intentionally excluded from AdminUserDep so a leaked key can never manage other keys regardless of the linked account's role. Returns 401 (not 403) on missing JWT. FINDING-05 (LOW): Rename 'sliding window' to 'fixed window' throughout — comments, docstrings, and model class docstring. Field names unchanged to avoid a migration. Added burst-tolerance note. FINDING-07 (LOW): GET /v1/auth/api-keys now supports optional api_user_id filter and limit/offset pagination. Covered by a new test. FINDING-12 (MEDIUM): validate_api_key early-exits for keys longer than 128 chars before touching the DB or running Argon2. FINDING-02 (MEDIUM): logger.warning on auth failure and rate limit hit; logger.info on key creation and revocation. Raw key/hash never logged. Copilot: rollback before return False when rate limit is exceeded, so the SELECT FOR UPDATE row lock is released immediately rather than held until session teardown. Copilot: re-check ApiKey.is_active in the FOR UPDATE query to handle concurrent revocation between validate_api_key and the counter update. Copilot: fix timezone mismatch — rate_window_start and last_used_at now use an explicit sa.DateTime(timezone=True) column to match the migration and avoid naive/aware subtraction errors on Postgres. All 168 unit tests pass (3 pre-existing integration failures unchanged). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: rebase alembic migration down_revision to current main head The api_user migration pointed to an older revision (97a23076c0d9), creating multiple Alembic heads and failing CI. Updated to point to 60b08397200f (current main head). * fix: resolve circular dependency in migration chain eacbc6544a10 was incorrectly rebased onto 60b08397200f, which is a descendant of eacbc6544a10 itself, creating a cycle that caused alembic upgrade head to hang indefinitely. Restore eacbc6544a10 to its original down_revision (97a23076c0d9) and point the new b3f2d1c8e9a0 (api_key table) at 60b08397200f, the actual current chain head. * fix: use timezone-aware DateTime in api_key migration Align migration column types with the model's sa.DateTime(timezone=True) for rate_window_start and last_used_at to fix alembic check drift. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Landung 'Don' Setiawan <landungs@uw.edu>
1 parent 9e86f2a commit e289fd1

9 files changed

Lines changed: 928 additions & 30 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
"""Add api_key table
2+
3+
Revision ID: b3f2d1c8e9a0
4+
Revises: 60b08397200f
5+
Create Date: 2026-04-02 00:00:00.000000
6+
7+
"""
8+
from typing import Sequence, Union
9+
10+
from alembic import op
11+
import sqlalchemy as sa
12+
import sqlmodel
13+
14+
# revision identifiers, used by Alembic.
15+
revision: str = 'b3f2d1c8e9a0'
16+
down_revision: Union[str, Sequence[str], None] = '60b08397200f'
17+
branch_labels: Union[str, Sequence[str], None] = None
18+
depends_on: Union[str, Sequence[str], None] = None
19+
20+
21+
def upgrade() -> None:
22+
"""Add api_key table for per-client API key authentication."""
23+
op.create_table(
24+
'api_key',
25+
sa.Column('id', sa.Integer(), nullable=False),
26+
sa.Column('api_user_id', sa.Integer(), nullable=False),
27+
sa.Column('name', sqlmodel.sql.sqltypes.AutoString(), nullable=False),
28+
sa.Column('key_prefix', sqlmodel.sql.sqltypes.AutoString(length=8), nullable=False),
29+
sa.Column('key_hash', sqlmodel.sql.sqltypes.AutoString(), nullable=False),
30+
sa.Column('is_active', sa.Boolean(), nullable=False, server_default=sa.text('true')),
31+
sa.Column('rate_limit_per_minute', sa.Integer(), nullable=False, server_default=sa.text('60')),
32+
sa.Column('rate_window_start', sa.DateTime(timezone=True), nullable=True),
33+
sa.Column('rate_window_count', sa.Integer(), nullable=False, server_default=sa.text('0')),
34+
sa.Column('last_used_at', sa.DateTime(timezone=True), nullable=True),
35+
sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=True),
36+
sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=True),
37+
sa.ForeignKeyConstraint(['api_user_id'], ['api_user.id'], ),
38+
sa.PrimaryKeyConstraint('id'),
39+
sa.UniqueConstraint('key_hash'),
40+
)
41+
op.create_index(op.f('ix_api_key_api_user_id'), 'api_key', ['api_user_id'], unique=False)
42+
op.create_index(op.f('ix_api_key_key_prefix'), 'api_key', ['key_prefix'], unique=False)
43+
44+
45+
def downgrade() -> None:
46+
"""Drop api_key table."""
47+
op.drop_index(op.f('ix_api_key_key_prefix'), table_name='api_key')
48+
op.drop_index(op.f('ix_api_key_api_user_id'), table_name='api_key')
49+
op.drop_table('api_key')

src/ca_biositing/datamodels/ca_biositing/datamodels/models/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from .base import BaseEntity, LookupBase, Aim1RecordBase, Aim2RecordBase
33

44
# Auth
5-
from .auth import ApiUser
5+
from .auth import ApiKey, ApiUser
66

77
# Aim1 Records
88
from .aim1_records import CalorimetryRecord, CompositionalRecord, FtnirRecord, IcpRecord, ProximateRecord, RgbRecord, UltimateRecord, XrdRecord, XrfRecord
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1+
from .api_key import ApiKey
12
from .api_user import ApiUser
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
"""ApiKey model for per-client API key authentication."""
2+
3+
from __future__ import annotations
4+
5+
from datetime import datetime, timezone
6+
from typing import Optional
7+
8+
import sqlalchemy as sa
9+
from sqlmodel import Field, SQLModel
10+
11+
12+
class ApiKey(SQLModel, table=True):
13+
"""Per-client API key for authenticating external callers.
14+
15+
Raw keys are shown exactly once at creation and never stored. Only the
16+
Argon2 hash is persisted. The key_prefix (first 8 chars of the raw key)
17+
is stored in plaintext to allow efficient lookup before hash verification.
18+
19+
Rate limiting uses a DB-based fixed-window counter with SELECT FOR UPDATE
20+
to handle multiple Cloud Run instances without requiring Redis. The window
21+
resets each time 60 seconds have elapsed since rate_window_start, meaning
22+
a burst of up to 2N requests in ~2s is possible at a window boundary —
23+
acceptable for a research API.
24+
"""
25+
26+
__tablename__ = "api_key"
27+
28+
id: Optional[int] = Field(default=None, primary_key=True)
29+
api_user_id: int = Field(foreign_key="api_user.id", nullable=False, index=True)
30+
name: str = Field(nullable=False)
31+
key_prefix: str = Field(nullable=False, index=True)
32+
key_hash: str = Field(nullable=False, unique=True)
33+
is_active: bool = Field(default=True, nullable=False)
34+
rate_limit_per_minute: int = Field(default=60, nullable=False)
35+
# Fixed-window rate limit state. Stored with explicit timezone=True so that
36+
# SQLAlchemy maps these to TIMESTAMPTZ on Postgres, matching the migration.
37+
rate_window_start: Optional[datetime] = Field(
38+
default=None,
39+
sa_column=sa.Column(sa.DateTime(timezone=True), nullable=True),
40+
)
41+
rate_window_count: int = Field(default=0, nullable=False)
42+
last_used_at: Optional[datetime] = Field(
43+
default=None,
44+
sa_column=sa.Column(sa.DateTime(timezone=True), nullable=True),
45+
)
46+
created_at: Optional[datetime] = Field(
47+
default=None,
48+
sa_column_kwargs={"server_default": sa.text("CURRENT_TIMESTAMP")},
49+
)
50+
updated_at: Optional[datetime] = Field(
51+
default=None,
52+
sa_column_kwargs={
53+
"server_default": sa.text("CURRENT_TIMESTAMP"),
54+
"onupdate": lambda: datetime.now(timezone.utc),
55+
},
56+
)

src/ca_biositing/webservice/ca_biositing/webservice/dependencies.py

Lines changed: 88 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,18 @@
88

99
from typing import Annotated, Optional
1010

11-
from fastapi import Depends, HTTPException, Query, Request, status
11+
from fastapi import Depends, Header, HTTPException, Query, Request, status
1212
from fastapi.security import OAuth2PasswordBearer
1313
from sqlmodel import Session, select
1414

1515
from ca_biositing.datamodels.database import get_session
1616
from ca_biositing.datamodels.models import ApiUser
1717
from ca_biositing.webservice.config import config
18-
from ca_biositing.webservice.services.auth_service import decode_access_token
18+
from ca_biositing.webservice.services.auth_service import (
19+
check_and_increment_rate_limit,
20+
decode_access_token,
21+
validate_api_key,
22+
)
1923

2024

2125
# Database session dependency
@@ -38,35 +42,23 @@ def pagination_params(
3842
oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/v1/auth/token", auto_error=False)
3943

4044

41-
def get_current_user(
42-
token: Annotated[Optional[str], Depends(oauth2_scheme)],
43-
request: Request,
44-
session: SessionDep,
45-
) -> ApiUser:
46-
"""Resolve the current authenticated user from Bearer token or HTTP-only cookie.
45+
def _resolve_jwt_user(token: Optional[str], request: Request, session: Session) -> Optional[ApiUser]:
46+
"""Shared helper: resolve a user from a JWT Bearer token or HTTP-only cookie.
4747
48-
Checks the Authorization: Bearer header first; if absent, falls back to the
49-
access_token cookie. Raises 401 if no valid token is found or the user is disabled.
48+
Returns the ApiUser on success, or None if no valid JWT is present.
49+
Raises 401 if a token is present but invalid/expired.
5050
"""
5151
credentials_exception = HTTPException(
5252
status_code=status.HTTP_401_UNAUTHORIZED,
5353
detail="Could not validate credentials",
5454
headers={"WWW-Authenticate": "Bearer"},
5555
)
56-
57-
# Prefer Bearer header; fall back to cookie
5856
resolved_token: Optional[str] = token or request.cookies.get("access_token")
5957
if not resolved_token:
60-
raise credentials_exception
61-
62-
username = decode_access_token(
63-
resolved_token,
64-
config.jwt_secret_key,
65-
config.jwt_algorithm,
66-
)
58+
return None
59+
username = decode_access_token(resolved_token, config.jwt_secret_key, config.jwt_algorithm)
6760
if not username:
6861
raise credentials_exception
69-
7062
user = session.exec(select(ApiUser).where(ApiUser.username == username)).first()
7163
if user is None:
7264
raise credentials_exception
@@ -78,12 +70,86 @@ def get_current_user(
7870
return user
7971

8072

73+
def get_jwt_user(
74+
token: Annotated[Optional[str], Depends(oauth2_scheme)],
75+
request: Request,
76+
session: SessionDep,
77+
) -> ApiUser:
78+
"""JWT-only authentication (Bearer header or HTTP-only cookie).
79+
80+
Does NOT accept X-API-Key. Used for admin endpoints so that a leaked
81+
API key can never access key-management operations.
82+
83+
Raises 401 if no valid JWT credential is present.
84+
"""
85+
user = _resolve_jwt_user(token, request, session)
86+
if user is None:
87+
raise HTTPException(
88+
status_code=status.HTTP_401_UNAUTHORIZED,
89+
detail="Could not validate credentials",
90+
headers={"WWW-Authenticate": "Bearer"},
91+
)
92+
return user
93+
94+
95+
def get_current_user(
96+
token: Annotated[Optional[str], Depends(oauth2_scheme)],
97+
request: Request,
98+
session: SessionDep,
99+
x_api_key: Annotated[Optional[str], Header(alias="X-API-Key")] = None,
100+
) -> ApiUser:
101+
"""Resolve the current authenticated user.
102+
103+
Auth check order:
104+
1. Authorization: Bearer <jwt> header
105+
2. access_token HTTP-only cookie
106+
3. X-API-Key header (per-client API key with rate limiting)
107+
108+
Raises 401 if no valid credential is found or the user is disabled.
109+
Raises 429 if the API key's rate limit is exceeded.
110+
"""
111+
credentials_exception = HTTPException(
112+
status_code=status.HTTP_401_UNAUTHORIZED,
113+
detail="Could not validate credentials",
114+
headers={"WWW-Authenticate": "Bearer"},
115+
)
116+
117+
# --- Path 1 & 2: JWT (Bearer header or cookie) ---
118+
jwt_user = _resolve_jwt_user(token, request, session)
119+
if jwt_user is not None:
120+
return jwt_user
121+
122+
# --- Path 3: X-API-Key header ---
123+
if x_api_key:
124+
api_key = validate_api_key(session, x_api_key)
125+
if not api_key:
126+
raise credentials_exception
127+
user = session.exec(select(ApiUser).where(ApiUser.id == api_key.api_user_id)).first()
128+
if user is None or user.disabled:
129+
raise credentials_exception
130+
if not check_and_increment_rate_limit(session, api_key):
131+
raise HTTPException(
132+
status_code=status.HTTP_429_TOO_MANY_REQUESTS,
133+
detail="Rate limit exceeded",
134+
headers={"Retry-After": "60"},
135+
)
136+
return user
137+
138+
raise credentials_exception
139+
140+
81141
# Typed dependency aliases
82142
CurrentUserDep = Annotated[ApiUser, Depends(get_current_user)]
143+
JWTCurrentUserDep = Annotated[ApiUser, Depends(get_jwt_user)]
83144

84145

85-
def get_current_admin_user(current_user: CurrentUserDep) -> ApiUser:
86-
"""Require the current user to be an admin. Raises 403 otherwise."""
146+
def get_current_admin_user(current_user: JWTCurrentUserDep) -> ApiUser:
147+
"""Require the current user to be an admin, authenticated via JWT only.
148+
149+
API keys are intentionally excluded — a leaked key must not be able to
150+
manage other keys even if the linked account has admin privileges.
151+
Raises 403 if the user is not an admin.
152+
"""
87153
if not current_user.is_admin:
88154
raise HTTPException(
89155
status_code=status.HTTP_403_FORBIDDEN,

0 commit comments

Comments
 (0)