Skip to content

Commit 3b7b640

Browse files
authored
Authentication: Replace encryption with hashing and add permission-based authorization (#396)
* Draft one: modify model, added crud, route and fix deps, migration * pre commit * Add permission management for API key routes and user context dependency * Refactor API key management: replace verify_api_key function with APIKeyManager class, add delete API key route, and update related CRUD operations * Enhance API key management: add support for old key format in APIKeyManager, update key prefix extraction logic, and adjust seed data creation for API keys. * Fix API key generation: ensure exact lengths for prefix and secret key, and update comments for clarity. * Add migration script for API keys: convert encrypted keys to hashed format and update database schema * precommit * Refactor API key handling in tests to use AuthContext - Removed direct usage of APIKeyPublic in tests and replaced with AuthContext for better encapsulation of authentication details. - Updated test cases across various modules (e.g., test_api_key, test_assistants, test_creds, etc.) to utilize the new AuthContext structure. - Simplified the retrieval of API keys and user context in test setups, enhancing readability and maintainability. - Removed redundant API key creation logic from test utilities and centralized it within the AuthContext. - Ensured all tests still pass with the new structure, maintaining functionality while improving code organization. * Fix API key route prefix and improve formatting in auth context functions * move APIKeyManager to security module and clean up imports in CRUD operations * user context handling to use AuthContext, updating dependencies and verification logic for API key authentication * rename AuthContext to TestAuthContext for consistency in test files * API key creation to include user and project ID parameters, enhancing user-specific key management. * enhance test coverage for API key CRUD and Routes operations * Add tests for API key manager * authentication context functions for consistency in test files * replace APIKeyCrud with create_test_api_key for consistency in test setup * Add tests for get_user_context * Add tests for permission checks and permission enum functionality * pre commt * Fix read_one method docstring and update logging for API key creation * Update API key deletion to set updated_at timestamp and modify user_id field in AuthContext * Fix import statement for get_project_by_id in APIKeyCrud * pre commit * fix migration * precommit * Refactor API key handling and improve documentation - Updated downgrade function to clarify snapshot restoration. - Changed conditional logic in get_current_user function for clarity. - Enhanced create_api_key_route response with security message about raw API key. - Updated APIKeyManager docstring - removed unnecessary comments * Update APIKeyManager docstring and enhance seed data comment for clarity * fix migration head * precomit * Refactor API key migration to use context manager for session handling and improve code clarity * precommit * remove id's from authcontext * Remove user_id from AuthContext instantiation in get_auth_context function * Update user authentication to check for active status before returning user object * precommit
1 parent 692b3ea commit 3b7b640

35 files changed

+1878
-638
lines changed
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
"""
2+
Migration script to convert encrypted API keys to hashed format.
3+
4+
This script:
5+
1. Decrypts existing API keys from the old encrypted format
6+
2. Extracts the prefix and secret from the decrypted keys
7+
3. Hashes the secret using bcrypt
8+
4. Generates UUID4 for the new primary key
9+
5. Stores the prefix, hash, and UUID in the new format for backward compatibility
10+
11+
The format is: "ApiKey {12-char-prefix}{31-char-secret}" (total 43 chars)
12+
"""
13+
14+
import logging
15+
import uuid
16+
from sqlalchemy.orm import Session
17+
from sqlalchemy import text
18+
from passlib.context import CryptContext
19+
20+
from app.core.security import decrypt_api_key
21+
22+
logger = logging.getLogger(__name__)
23+
24+
# Use the same hash algorithm as APIKeyManager
25+
pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
26+
27+
# Old format constants
28+
OLD_PREFIX_NAME = "ApiKey "
29+
OLD_PREFIX_LENGTH = 12
30+
OLD_SECRET_LENGTH = 31
31+
OLD_KEY_LENGTH = 43 # Total: 12 + 31
32+
33+
34+
def migrate_api_keys(session: Session, generate_uuid: bool = False) -> None:
35+
"""
36+
Migrate all existing API keys from encrypted format to hashed format.
37+
38+
This function:
39+
1. Fetches all API keys with the old 'key' column
40+
2. Decrypts each key
41+
3. Extracts prefix and secret
42+
4. Hashes the secret
43+
5. Generates UUID4 for new_id column if generate_uuid is True
44+
6. Updates key_prefix, key_hash, and optionally new_id columns
45+
46+
Args:
47+
session: SQLAlchemy database session
48+
generate_uuid: Whether to generate and set UUID for new_id column
49+
"""
50+
logger.info(
51+
"[migrate_api_keys] Starting API key migration from encrypted to hashed format"
52+
)
53+
54+
try:
55+
# Fetch all API keys that have the old 'key' column
56+
result = session.execute(
57+
text("SELECT id, key FROM apikey WHERE key IS NOT NULL")
58+
)
59+
api_keys = result.fetchall()
60+
61+
if not api_keys:
62+
logger.info("[migrate_api_keys] No API keys found to migrate")
63+
return
64+
65+
logger.info(f"[migrate_api_keys] Found {len(api_keys)} API keys to migrate")
66+
67+
migrated_count = 0
68+
failed_count = 0
69+
70+
for row in api_keys:
71+
key_id = row[0]
72+
encrypted_key = row[1]
73+
74+
try:
75+
# Decrypt the API key
76+
decrypted_key = decrypt_api_key(encrypted_key)
77+
78+
# Validate format
79+
if not decrypted_key.startswith(OLD_PREFIX_NAME):
80+
logger.error(
81+
f"[migrate_api_keys] Invalid key format for ID {key_id}: "
82+
f"does not start with '{OLD_PREFIX_NAME}'"
83+
)
84+
failed_count += 1
85+
continue
86+
87+
# Extract the key part (after "ApiKey ")
88+
key_part = decrypted_key[len(OLD_PREFIX_NAME) :]
89+
90+
if len(key_part) != OLD_KEY_LENGTH:
91+
logger.error(
92+
f"[migrate_api_keys] Invalid key length for ID {key_id}: "
93+
f"expected {OLD_KEY_LENGTH}, got {len(key_part)}"
94+
)
95+
failed_count += 1
96+
continue
97+
98+
# Extract prefix and secret
99+
key_prefix = key_part[:OLD_PREFIX_LENGTH]
100+
secret_key = key_part[OLD_PREFIX_LENGTH:]
101+
102+
# Hash the secret
103+
key_hash = pwd_context.hash(secret_key)
104+
105+
# Generate UUID if requested
106+
if generate_uuid:
107+
new_uuid = uuid.uuid4()
108+
# Update the record with prefix, hash, and UUID
109+
session.execute(
110+
text(
111+
"UPDATE apikey SET key_prefix = :prefix, key_hash = :hash, new_id = :new_id "
112+
"WHERE id = :id"
113+
),
114+
{
115+
"prefix": key_prefix,
116+
"hash": key_hash,
117+
"new_id": new_uuid,
118+
"id": key_id,
119+
},
120+
)
121+
else:
122+
# Update the record with prefix and hash only
123+
session.execute(
124+
text(
125+
"UPDATE apikey SET key_prefix = :prefix, key_hash = :hash "
126+
"WHERE id = :id"
127+
),
128+
{"prefix": key_prefix, "hash": key_hash, "id": key_id},
129+
)
130+
131+
migrated_count += 1
132+
logger.info(
133+
f"[migrate_api_keys] Successfully migrated key ID {key_id} "
134+
f"with prefix {key_prefix[:4]}..."
135+
)
136+
137+
except Exception as e:
138+
logger.error(
139+
f"[migrate_api_keys] Failed to migrate key ID {key_id}: {str(e)}",
140+
exc_info=True,
141+
)
142+
failed_count += 1
143+
continue
144+
145+
logger.info(
146+
f"[migrate_api_keys] Migration completed: "
147+
f"{migrated_count} successful, {failed_count} failed"
148+
)
149+
150+
except Exception as e:
151+
logger.error(
152+
f"[migrate_api_keys] Fatal error during migration: {str(e)}", exc_info=True
153+
)
154+
raise
155+
156+
157+
def verify_migration(session: Session) -> bool:
158+
"""
159+
Verify that all API keys have been migrated successfully.
160+
161+
Args:
162+
session: SQLAlchemy database session
163+
164+
Returns:
165+
bool: True if all keys are migrated, False otherwise
166+
"""
167+
try:
168+
# Check for any keys with NULL key_prefix or key_hash
169+
result = session.execute(
170+
text(
171+
"SELECT COUNT(*) FROM apikey "
172+
"WHERE key_prefix IS NULL OR key_hash IS NULL"
173+
)
174+
)
175+
null_count = result.scalar()
176+
177+
if null_count > 0:
178+
logger.warning(
179+
f"[verify_migration] Found {null_count} API keys with NULL "
180+
"key_prefix or key_hash"
181+
)
182+
return False
183+
184+
# Check total count
185+
result = session.execute(text("SELECT COUNT(*) FROM apikey"))
186+
total_count = result.scalar()
187+
188+
logger.info(
189+
f"[verify_migration] All {total_count} API keys have been "
190+
"successfully migrated"
191+
)
192+
return True
193+
194+
except Exception as e:
195+
logger.error(
196+
f"[verify_migration] Error verifying migration: {str(e)}", exc_info=True
197+
)
198+
return False
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
"""Refactor API key table
2+
3+
Revision ID: e7c68e43ce6f
4+
Revises: 27c271ab6dd0
5+
Create Date: 2025-10-16 13:06:51.777671
6+
7+
"""
8+
from alembic import op
9+
import sqlalchemy as sa
10+
import sqlmodel.sql.sqltypes
11+
from sqlalchemy.orm import Session
12+
from app.alembic.migrate_api_key import migrate_api_keys, verify_migration
13+
14+
15+
# revision identifiers, used by Alembic.
16+
revision = "e7c68e43ce6f"
17+
down_revision = "27c271ab6dd0"
18+
branch_labels = None
19+
depends_on = None
20+
21+
22+
def upgrade():
23+
# ### commands auto generated by Alembic - please adjust! ###
24+
# Step 1: Add new columns as nullable to allow migration
25+
op.add_column(
26+
"apikey",
27+
sa.Column("key_prefix", sqlmodel.sql.sqltypes.AutoString(), nullable=True),
28+
)
29+
op.add_column(
30+
"apikey",
31+
sa.Column("key_hash", sqlmodel.sql.sqltypes.AutoString(), nullable=True),
32+
)
33+
34+
# Step 2: Add UUID column before migration
35+
op.add_column("apikey", sa.Column("new_id", sa.Uuid(), nullable=True))
36+
37+
# Step 3: Migrate existing encrypted keys to the new hashed format and generate UUIDs
38+
bind = op.get_bind()
39+
with Session(bind=bind) as session:
40+
migrate_api_keys(session, generate_uuid=True)
41+
42+
# Step 4: Verify migration was successful
43+
if not verify_migration(session):
44+
raise Exception(
45+
"API key migration verification failed. Please check the logs."
46+
)
47+
48+
session.flush()
49+
50+
# Step 5: Make the columns non-nullable after migration
51+
op.alter_column("apikey", "key_prefix", nullable=False)
52+
op.alter_column("apikey", "key_hash", nullable=False)
53+
54+
# Step 6: Replace old PK with UUID-based PK
55+
op.drop_constraint("apikey_pkey", "apikey", type_="primary")
56+
op.drop_column("apikey", "id")
57+
op.alter_column("apikey", "new_id", new_column_name="id", nullable=False)
58+
op.create_primary_key("apikey_pkey", "apikey", ["id"])
59+
60+
# Step 7: Update indexes and drop old key column
61+
op.drop_index("ix_apikey_key", table_name="apikey")
62+
op.create_index(op.f("ix_apikey_key_prefix"), "apikey", ["key_prefix"], unique=True)
63+
op.drop_column("apikey", "key")
64+
# ### end Alembic commands ###
65+
66+
67+
def downgrade():
68+
# instead of downgrade, will take a db snapshot and restore from that if needed
69+
pass

0 commit comments

Comments
 (0)