Skip to content

Commit 0df5a16

Browse files
committed
fix: security hardening for API key generation (Priya's review)
3 issues found during frontend planning: 1. Tier self-escalation: users could set tier='enterprise' in the request body. Fixed: tier locked to auth.tier, removed from request. 2. No key limit: unlimited key generation. Fixed: max 5 active keys per user, returns 403 with clear message. 3. generate_key returned only the raw key string, no ID for frontend list. Fixed: returns dict with key, id, name, tier. Also: generate route now returns 'id' so frontend can show the key in the list immediately without refetching. Tests updated: mock returns dict, CreateAPIKeyRequest no longer accepts tier. 392 tests pass.
1 parent e51a33d commit 0df5a16

3 files changed

Lines changed: 57 additions & 21 deletions

File tree

backend/routes/api_keys.py

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
router = APIRouter(prefix="", tags=["API Keys"])
1616

1717

18+
MAX_KEYS_PER_USER = 5
19+
20+
1821
class CreateAPIKeyRequest(BaseModel):
1922
name: str
20-
tier: str = "free"
2123

2224

2325
@router.get("/metrics")
@@ -46,37 +48,54 @@ async def generate_api_key(
4648
auth: AuthContext = Depends(require_auth)
4749
):
4850
"""Generate a new API key."""
49-
set_operation_context("generate_api_key", user_id=auth.user_id, tier=request.tier)
50-
add_breadcrumb("API key generation requested", category="api_keys", tier=request.tier)
51+
if not auth.user_id:
52+
raise HTTPException(status_code=401, detail="User ID required")
53+
54+
set_operation_context("generate_api_key", user_id=auth.user_id, tier=auth.tier)
55+
add_breadcrumb("API key generation requested", category="api_keys", tier=auth.tier)
5156

5257
logger.info(
5358
"API key generation requested",
5459
user_id=auth.user_id,
5560
key_name=request.name,
56-
tier=request.tier
61+
tier=auth.tier
5762
)
5863

64+
# Tier is locked to the user's auth tier (no self-escalation)
65+
tier = auth.tier or "free"
66+
67+
# Enforce key limit per user
68+
key_count = api_key_manager.count_keys(auth.user_id)
69+
if key_count >= MAX_KEYS_PER_USER:
70+
raise HTTPException(
71+
status_code=403,
72+
detail=f"Maximum {MAX_KEYS_PER_USER} active API keys allowed. Revoke an existing key first."
73+
)
74+
5975
try:
60-
with track_time("generate_api_key", tier=request.tier):
61-
new_key = api_key_manager.generate_key(
76+
with track_time("generate_api_key", tier=tier):
77+
result = api_key_manager.generate_key(
6278
name=request.name,
63-
tier=request.tier,
79+
tier=tier,
6480
user_id=auth.user_id
6581
)
6682

6783
logger.info(
6884
"API key generated successfully",
6985
user_id=auth.user_id,
7086
key_name=request.name,
71-
tier=request.tier
87+
tier=tier
7288
)
7389

7490
return {
75-
"api_key": new_key,
76-
"tier": request.tier,
91+
"api_key": result["key"],
92+
"id": result["id"],
93+
"tier": tier,
7794
"name": request.name,
7895
"message": "Save this key securely - it won't be shown again"
7996
}
97+
except HTTPException:
98+
raise
8099
except Exception as e:
81100
logger.error(
82101
"API key generation failed",

backend/services/rate_limiter.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -177,21 +177,32 @@ class APIKeyManager:
177177
def __init__(self, supabase_client):
178178
self.db = supabase_client
179179

180-
def generate_key(self, name: str, tier: str = 'free', user_id: Optional[str] = None) -> str:
181-
"""Generate a new API key"""
182-
# Generate secure random key
180+
def generate_key(self, name: str, tier: str = 'free', user_id: Optional[str] = None) -> Dict:
181+
"""Generate a new API key. Returns dict with raw key + metadata."""
183182
key = f"ci_{secrets.token_urlsafe(32)}"
184-
185-
# Store in database
186-
self.db.table("api_keys").insert({
183+
184+
result = self.db.table("api_keys").insert({
187185
"key_hash": hashlib.sha256(key.encode()).hexdigest(),
188186
"name": name,
189187
"tier": tier,
190188
"user_id": user_id,
191189
"created_at": datetime.utcnow().isoformat()
192190
}).execute()
193-
194-
return key
191+
192+
row = result.data[0] if result.data else {}
193+
return {
194+
"key": key,
195+
"id": row.get("id"),
196+
"name": name,
197+
"tier": tier,
198+
}
199+
200+
def count_keys(self, user_id: str) -> int:
201+
"""Count active keys for a user."""
202+
result = self.db.table("api_keys").select(
203+
"id", count="exact"
204+
).eq("user_id", user_id).eq("active", True).execute()
205+
return result.count or 0
195206

196207
def verify_key(self, api_key: str) -> Optional[Dict]:
197208
"""Verify API key and return metadata"""

backend/tests/test_observability_routes.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,13 @@ def mock_dependencies(self):
355355
patch("routes.api_keys.rate_limiter") as mock_limiter, \
356356
patch("routes.api_keys.metrics") as mock_metrics:
357357

358-
mock_manager.generate_key = MagicMock(return_value="sk-test-key-123")
358+
mock_manager.generate_key = MagicMock(return_value={
359+
"key": "ci_test-key-123",
360+
"id": "test-uuid",
361+
"name": "test",
362+
"tier": "free",
363+
})
364+
mock_manager.count_keys = MagicMock(return_value=0)
359365
mock_limiter.get_usage = MagicMock(return_value={"minute": 5, "hour": 50})
360366
mock_metrics.get_metrics = MagicMock(return_value={"searches": 100})
361367

@@ -371,7 +377,7 @@ async def test_generate_key_logs_request(self, mock_observability, mock_dependen
371377
from routes.api_keys import generate_api_key, CreateAPIKeyRequest
372378
from middleware.auth import AuthContext
373379

374-
request = CreateAPIKeyRequest(name="Production Key", tier="pro")
380+
request = CreateAPIKeyRequest(name="Production Key")
375381
auth = AuthContext(user_id="user-123", email="test@test.com", tier="pro")
376382

377383
await generate_api_key(request, auth)
@@ -387,7 +393,7 @@ async def test_generate_key_sets_context_with_tier(self, mock_observability, moc
387393
from routes.api_keys import generate_api_key, CreateAPIKeyRequest
388394
from middleware.auth import AuthContext
389395

390-
request = CreateAPIKeyRequest(name="Key", tier="enterprise")
396+
request = CreateAPIKeyRequest(name="Key")
391397
auth = AuthContext(user_id="user", email="test@test.com", tier="enterprise")
392398

393399
await generate_api_key(request, auth)

0 commit comments

Comments
 (0)