diff --git a/README.md b/README.md index 0954594d..0037bd77 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ Documentation for this project is now availiable on it's own page at finished will include full usage information and how-to's. - [Important note on Versioning](#important-note-on-versioning) +- [Breaking Changes](#breaking-changes) - [Changes from version 0.4.x](#changes-from-version-04x) - [Functionality](#functionality) - [Installation](#installation) @@ -50,6 +51,13 @@ previous version. This will be in the form of a `.patch` file which can be applied to their project using the `git apply` command. This will be documented in the release notes. +## Breaking Changes + +There will be breaking changes implemented from time to time, as the template is +still evolving. These may be due to security issues or changes in philosophy. +These can always be found [here](https://api-template.seapagan.net/important/) +on the website. + ## Changes from version 0.4.x Starting from version 0.5.0, the template has been refactored to use SQLAlchemy diff --git a/SECURITY.md b/SECURITY.md index ded5683f..8dfcf5f7 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -4,7 +4,7 @@ | Version | Supported | | ------- | ------------------ | -| >=0.6.0 | :white_check_mark: | +| >=0.7.1 | :white_check_mark: | Until we reach a 1.0 milestone, we will generally only support the latest release of the project. If you are having issues with an older version, please diff --git a/app/managers/api_key.py b/app/managers/api_key.py index 32f5a542..55a1760a 100644 --- a/app/managers/api_key.py +++ b/app/managers/api_key.py @@ -1,6 +1,7 @@ """Define the API Key Manager.""" import hashlib +import hmac import secrets from typing import Optional from uuid import UUID @@ -8,6 +9,7 @@ from fastapi import Depends, HTTPException, Request, status from sqlalchemy.ext.asyncio import AsyncSession +from app.config.settings import get_settings from app.database.db import get_database from app.database.helpers import ( add_new_api_key_, @@ -35,9 +37,19 @@ class ApiKeyManager: KEY_LENGTH = 32 @staticmethod + # codeql[py/weak-sensitive-data-hashing] See comment below def _hash_key(key: str) -> str: - """Hash an API key.""" - return hashlib.sha256(key.encode()).hexdigest() + """Hash an API key using HMAC-SHA256. + + This intentionally uses HMAC-SHA256 rather than a slow, memory-hard + password hash (e.g., bcrypt or argon2) because our API keys are + generated with `secrets.token_urlsafe(32)`, giving ~192 bits of + entropy. They are not human-chosen or guessable, so brute-forcing + them is computationally infeasible. HMAC also prevents + length-extension attacks possible with raw SHA256 hashing. + """ + secret_key = get_settings().secret_key.encode() + return hmac.new(secret_key, key.encode(), hashlib.sha256).hexdigest() @classmethod def generate_key(cls) -> str: diff --git a/docs/important.md b/docs/important.md index e9c9006c..55e6bdc2 100644 --- a/docs/important.md +++ b/docs/important.md @@ -6,7 +6,14 @@ API. ## Breaking Changes in `HEAD` -None. +### Modified the API Key Hashing method + +The API key is **NEVER** stored in the database, however a **hashed** version of +this is so that we can authenticate. Previously it used a plain SHA256 +algorythm, and has now been switched to using `HMAC` in conjunction with SHA256 +instead. This allows using the `SECRET_KEY` already set to make the API keys +more secure. As a result, **any existing API Keys are now invalid and will need to +be deleted and regenerated**. ## Breaking Changes in 0.7.0 @@ -37,7 +44,7 @@ Several function signatures have changed, generally to fix boolean inconsistencies. Boolean parameters should be passed as named parameters instead of positional parameters. This is to make the code more readable and maintainable. The `UserManager.set_ban_status` function is one of these changes -that causes a breaking change. However, this method is only called from and API +that causes a breaking change. However, this method is only called from an API endpoint for the moment, so it should not affect any existing code that depends on it unless you are using it directly in your code. diff --git a/pyproject.toml b/pyproject.toml index f612d2d9..0d03c52a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -180,6 +180,7 @@ convention = "google" "TD003", "FIX002", "RUF012", + "SLF001", # sometimes tests need to access private members ] "app/managers/auth.py" = ["ERA001"] "app/resources/auth.py" = ["ERA001"] diff --git a/tests/unit/test_admin.py b/tests/unit/test_admin.py index 160dc3a3..86540dbb 100644 --- a/tests/unit/test_admin.py +++ b/tests/unit/test_admin.py @@ -1,6 +1,5 @@ """Unit tests for the admin interface.""" -# ruff: noqa: SLF001 import json from datetime import datetime, timedelta, timezone diff --git a/tests/unit/test_api_key_manager.py b/tests/unit/test_api_key_manager.py index 8f2a82b1..07a9cf4b 100644 --- a/tests/unit/test_api_key_manager.py +++ b/tests/unit/test_api_key_manager.py @@ -1,5 +1,7 @@ """Test the API Key Manager functionality.""" +import hashlib +import hmac from uuid import uuid4 import pytest @@ -318,3 +320,47 @@ async def test_api_key_auth_user_not_found_in_db_no_auto_error( auth = ApiKeyAuth(auto_error=False) result = await auth(request=mock_req, db=test_db) assert result is None + + def test_hash_key_uses_hmac(self, mocker) -> None: + """Test that API key hashing uses HMAC-SHA256 with secret key.""" + # Mock the settings + mock_settings = mocker.patch("app.managers.api_key.get_settings") + test_secret = "test_secret_key_12345" # noqa: S105 + mock_settings.return_value.secret_key = test_secret + + test_key = "pk_test_key_123456789" + result = ApiKeyManager._hash_key(test_key) + + # Calculate expected HMAC manually + expected = hmac.new( + test_secret.encode(), test_key.encode(), hashlib.sha256 + ).hexdigest() + + assert result == expected + assert len(result) == 64 # SHA256 hex digest length # noqa: PLR2004 + + def test_hash_key_different_secrets_different_hashes(self, mocker) -> None: + """Test that different secret keys produce different hashes.""" + test_key = "pk_same_key_123" + + # First hash with secret 1 + mock_settings = mocker.patch("app.managers.api_key.get_settings") + mock_settings.return_value.secret_key = "secret1" # noqa: S105 + hash1 = ApiKeyManager._hash_key(test_key) + + # Second hash with secret 2 + mock_settings.return_value.secret_key = "secret2" # noqa: S105 + hash2 = ApiKeyManager._hash_key(test_key) + + assert hash1 != hash2 + + def test_hash_key_deterministic(self, mocker) -> None: + """Test that the same key produces the same hash consistently.""" + mock_settings = mocker.patch("app.managers.api_key.get_settings") + mock_settings.return_value.secret_key = "consistent_secret" # noqa: S105 + + test_key = "pk_deterministic_test" + hash1 = ApiKeyManager._hash_key(test_key) + hash2 = ApiKeyManager._hash_key(test_key) + + assert hash1 == hash2