Skip to content

Commit 7c28e02

Browse files
committed
feat: implement HMAC-SHA256 for API key hashing and add corresponding tests
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
1 parent 00b0c8a commit 7c28e02

4 files changed

Lines changed: 60 additions & 3 deletions

File tree

app/managers/api_key.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
"""Define the API Key Manager."""
22

33
import hashlib
4+
import hmac
45
import secrets
56
from typing import Optional
67
from uuid import UUID
78

89
from fastapi import Depends, HTTPException, Request, status
910
from sqlalchemy.ext.asyncio import AsyncSession
1011

12+
from app.config.settings import get_settings
1113
from app.database.db import get_database
1214
from app.database.helpers import (
1315
add_new_api_key_,
@@ -36,8 +38,17 @@ class ApiKeyManager:
3638

3739
@staticmethod
3840
def _hash_key(key: str) -> str:
39-
"""Hash an API key."""
40-
return hashlib.sha256(key.encode()).hexdigest()
41+
"""Hash an API key using HMAC-SHA256.
42+
43+
This intentionally uses HMAC-SHA256 rather than a slow, memory-hard
44+
password hash (e.g., bcrypt or argon2) because our API keys are
45+
generated with `secrets.token_urlsafe(32)`, giving ~192 bits of
46+
entropy. They are not human-chosen or guessable, so brute-forcing
47+
them is computationally infeasible. HMAC also prevents
48+
length-extension attacks possible with raw SHA256 hashing.
49+
"""
50+
secret_key = get_settings().secret_key.encode()
51+
return hmac.new(secret_key, key.encode(), hashlib.sha256).hexdigest()
4152

4253
@classmethod
4354
def generate_key(cls) -> str:

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ convention = "google"
180180
"TD003",
181181
"FIX002",
182182
"RUF012",
183+
"SLF001", # sometimes tests need to access private members
183184
]
184185
"app/managers/auth.py" = ["ERA001"]
185186
"app/resources/auth.py" = ["ERA001"]

tests/unit/test_admin.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
"""Unit tests for the admin interface."""
22

3-
# ruff: noqa: SLF001
43
import json
54
from datetime import datetime, timedelta, timezone
65

tests/unit/test_api_key_manager.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Test the API Key Manager functionality."""
22

3+
import hashlib
4+
import hmac
35
from uuid import uuid4
46

57
import pytest
@@ -318,3 +320,47 @@ async def test_api_key_auth_user_not_found_in_db_no_auto_error(
318320
auth = ApiKeyAuth(auto_error=False)
319321
result = await auth(request=mock_req, db=test_db)
320322
assert result is None
323+
324+
def test_hash_key_uses_hmac(self, mocker) -> None:
325+
"""Test that API key hashing uses HMAC-SHA256 with secret key."""
326+
# Mock the settings
327+
mock_settings = mocker.patch("app.managers.api_key.get_settings")
328+
test_secret = "test_secret_key_12345" # noqa: S105
329+
mock_settings.return_value.secret_key = test_secret
330+
331+
test_key = "pk_test_key_123456789"
332+
result = ApiKeyManager._hash_key(test_key)
333+
334+
# Calculate expected HMAC manually
335+
expected = hmac.new(
336+
test_secret.encode(), test_key.encode(), hashlib.sha256
337+
).hexdigest()
338+
339+
assert result == expected
340+
assert len(result) == 64 # SHA256 hex digest length # noqa: PLR2004
341+
342+
def test_hash_key_different_secrets_different_hashes(self, mocker) -> None:
343+
"""Test that different secret keys produce different hashes."""
344+
test_key = "pk_same_key_123"
345+
346+
# First hash with secret 1
347+
mock_settings = mocker.patch("app.managers.api_key.get_settings")
348+
mock_settings.return_value.secret_key = "secret1" # noqa: S105
349+
hash1 = ApiKeyManager._hash_key(test_key)
350+
351+
# Second hash with secret 2
352+
mock_settings.return_value.secret_key = "secret2" # noqa: S105
353+
hash2 = ApiKeyManager._hash_key(test_key)
354+
355+
assert hash1 != hash2
356+
357+
def test_hash_key_deterministic(self, mocker) -> None:
358+
"""Test that the same key produces the same hash consistently."""
359+
mock_settings = mocker.patch("app.managers.api_key.get_settings")
360+
mock_settings.return_value.secret_key = "consistent_secret" # noqa: S105
361+
362+
test_key = "pk_deterministic_test"
363+
hash1 = ApiKeyManager._hash_key(test_key)
364+
hash2 = ApiKeyManager._hash_key(test_key)
365+
366+
assert hash1 == hash2

0 commit comments

Comments
 (0)