Skip to content

Commit a1eba39

Browse files
committed
fix: review findings -- unreachable user check, UUID validation, key preview suffix
1. auth.py: 'no linked user' check moved INTO _validate_api_key() where it can actually fire. Previously it was in _authenticate's error block which only ran when _validate_api_key returned None (meaning the key wasn't found at all, so user_id was never checked). 2. api_keys.py: key_id parameter changed from str to UUID. FastAPI now returns 422 for malformed UUIDs before hitting the DB. Added return type annotations (Dict[str, Any]) to list and revoke routes. 3. rate_limiter.py: key_preview now uses persisted key_suffix (last 8 chars of raw key) instead of SHA-256 hash suffix. Users see 'ci_...xYz12345' matching their actual key, not hash gibberish. 4. 004_api_keys.sql: added key_suffix column. Skipped: async conversion of service methods (Supabase client is sync; all other APIKeyManager methods are sync, would be inconsistent). Dev: run ALTER TABLE api_keys ADD COLUMN key_suffix text; in Supabase. 392 tests pass.
1 parent 0df5a16 commit a1eba39

4 files changed

Lines changed: 24 additions & 9 deletions

File tree

backend/middleware/auth.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,21 @@ def _validate_api_key(token: str) -> Optional[AuthContext]:
133133
except Exception:
134134
pass # Non-critical; don't fail auth over timestamp update
135135

136+
# Reject keys with no linked user at the auth layer
137+
# so routes get a clear 401 instead of silently passing with user_id=None
138+
if not key_data.get("user_id"):
139+
raise HTTPException(
140+
status_code=401,
141+
detail="API key has no linked user. Contact admin."
142+
)
143+
136144
return AuthContext(
137145
api_key_name=key_data.get("name"),
138146
user_id=key_data.get("user_id"),
139147
tier=key_data.get("tier", "free")
140148
)
149+
except HTTPException:
150+
raise
141151
except Exception:
142152
return None
143153

@@ -173,8 +183,6 @@ def _authenticate(token: str) -> AuthContext:
173183
detail = "API key not found. Check that the key is correct."
174184
elif not result.data[0].get("active"):
175185
detail = "API key has been revoked."
176-
elif not result.data[0].get("user_id"):
177-
detail = "API key has no linked user. Contact admin."
178186
else:
179187
detail = "API key validation failed."
180188
except Exception:

backend/routes/api_keys.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
"""API key management and metrics routes."""
2+
from typing import Any, Dict
3+
from uuid import UUID
4+
25
from fastapi import APIRouter, HTTPException, Depends
36
from pydantic import BaseModel
47

@@ -115,7 +118,7 @@ async def generate_api_key(
115118
@router.get("/keys")
116119
async def list_api_keys(
117120
auth: AuthContext = Depends(require_auth)
118-
):
121+
) -> Dict[str, Any]:
119122
"""List all API keys for the authenticated user."""
120123
if not auth.user_id:
121124
raise HTTPException(status_code=401, detail="User ID required")
@@ -131,15 +134,15 @@ async def list_api_keys(
131134

132135
@router.delete("/keys/{key_id}")
133136
async def revoke_api_key(
134-
key_id: str,
137+
key_id: UUID,
135138
auth: AuthContext = Depends(require_auth)
136-
):
139+
) -> Dict[str, Any]:
137140
"""Revoke an API key by ID. Soft-deletes (sets active=false)."""
138141
if not auth.user_id:
139142
raise HTTPException(status_code=401, detail="User ID required")
140143

141144
try:
142-
success = api_key_manager.revoke_key_by_id(key_id, auth.user_id)
145+
success = api_key_manager.revoke_key_by_id(str(key_id), auth.user_id)
143146
if not success:
144147
raise HTTPException(
145148
status_code=404,

backend/services/rate_limiter.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,12 @@ def __init__(self, supabase_client):
180180
def generate_key(self, name: str, tier: str = 'free', user_id: Optional[str] = None) -> Dict:
181181
"""Generate a new API key. Returns dict with raw key + metadata."""
182182
key = f"ci_{secrets.token_urlsafe(32)}"
183+
# Persist last 8 chars of raw key for display (ci_...xYz12345)
184+
suffix = key[-8:]
183185

184186
result = self.db.table("api_keys").insert({
185187
"key_hash": hashlib.sha256(key.encode()).hexdigest(),
188+
"key_suffix": suffix,
186189
"name": name,
187190
"tier": tier,
188191
"user_id": user_id,
@@ -230,18 +233,18 @@ def revoke_key_by_id(self, key_id: str, user_id: str) -> bool:
230233
def list_keys(self, user_id: str) -> list:
231234
"""List all API keys for a user. Returns masked keys (no raw values)."""
232235
result = self.db.table("api_keys").select(
233-
"id, name, tier, active, created_at, last_used_at, key_hash"
236+
"id, name, tier, active, created_at, last_used_at, key_suffix"
234237
).eq("user_id", user_id).order("created_at", desc=True).execute()
235238
keys = []
236239
for row in result.data:
237-
h = row.get("key_hash", "")
240+
suffix = row.get("key_suffix", "")
238241
keys.append({
239242
"id": row["id"],
240243
"name": row["name"],
241244
"tier": row["tier"],
242245
"active": row["active"],
243246
"created_at": row["created_at"],
244247
"last_used_at": row.get("last_used_at"),
245-
"key_preview": f"ci_...{h[-8:]}" if h else "ci_...",
248+
"key_preview": f"ci_...{suffix}" if suffix else "ci_...",
246249
})
247250
return keys

supabase/migrations/004_api_keys.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ CREATE TABLE IF NOT EXISTS api_keys (
1313
user_id uuid REFERENCES auth.users(id) ON DELETE CASCADE,
1414
name text NOT NULL,
1515
key_hash text NOT NULL UNIQUE,
16+
key_suffix text, -- last 8 chars of raw key for masked display (ci_...xYz12345)
1617
tier text DEFAULT 'free' CHECK (tier IN ('free', 'pro', 'enterprise')),
1718
active boolean DEFAULT true,
1819
created_at timestamptz DEFAULT now(),

0 commit comments

Comments
 (0)