feat: Add security hardening, agentic coding infrastructure, and test framework
- Add SECRET_KEY and ENCRYPTION_KEY validation on startup - Implement security headers middleware (X-Frame-Options, CSP, HSTS) - Add CSRF protection middleware - Create comprehensive GitHub issue templates and PR template - Add Makefile with common development tasks - Configure pre-commit hooks (black, ruff, mypy, bandit, detect-secrets) - Create docs/CODING_PATTERNS.md with best practices - Create docs/ERRORS.md documenting all error codes - Add Architecture Decision Records (ADR) for Celery and Fernet encryption - Create CHANGELOG.md for version tracking - Set up pytest test infrastructure with fixtures and factories - Add sample unit tests for security and config validation - Create CI/CD workflows (test, lint, security) - Add comprehensive TODO.md with milestones and progress tracking Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
@@ -92,6 +92,50 @@ class Settings(BaseSettings):
|
||||
if isinstance(v, str):
|
||||
return [i.strip() for i in v.split(",")]
|
||||
return v
|
||||
|
||||
@field_validator("SECRET_KEY")
|
||||
@classmethod
|
||||
def validate_secret_key(cls, v: str) -> str:
|
||||
"""Validate that SECRET_KEY is changed from default and is secure"""
|
||||
default_keys = [
|
||||
"change-this-to-a-secure-random-secret-key-in-production",
|
||||
"secret",
|
||||
"secret-key",
|
||||
"secretkey",
|
||||
]
|
||||
if v.lower() in default_keys:
|
||||
raise ValueError(
|
||||
"SECRET_KEY must be changed from default value! "
|
||||
"Generate a secure key with: python -c 'import secrets; print(secrets.token_urlsafe(32))'"
|
||||
)
|
||||
if len(v) < 32:
|
||||
raise ValueError(
|
||||
f"SECRET_KEY must be at least 32 characters long (current: {len(v)}). "
|
||||
"Generate a secure key with: python -c 'import secrets; print(secrets.token_urlsafe(32))'"
|
||||
)
|
||||
return v
|
||||
|
||||
@field_validator("ENCRYPTION_KEY")
|
||||
@classmethod
|
||||
def validate_encryption_key(cls, v: str) -> str:
|
||||
"""Validate that ENCRYPTION_KEY is changed from default and is secure"""
|
||||
default_keys = [
|
||||
"change-this-to-a-secure-encryption-key",
|
||||
"encryption",
|
||||
"encryption-key",
|
||||
"encryptionkey",
|
||||
]
|
||||
if v.lower() in default_keys:
|
||||
raise ValueError(
|
||||
"ENCRYPTION_KEY must be changed from default value! "
|
||||
"Generate a secure key with: python -c 'import secrets; print(secrets.token_urlsafe(32))'"
|
||||
)
|
||||
if len(v) < 32:
|
||||
raise ValueError(
|
||||
f"ENCRYPTION_KEY must be at least 32 characters long (current: {len(v)}). "
|
||||
"Generate a secure key with: python -c 'import secrets; print(secrets.token_urlsafe(32))'"
|
||||
)
|
||||
return v
|
||||
|
||||
|
||||
# Global settings instance
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
"""
|
||||
Security middleware for adding security headers and CSRF protection.
|
||||
"""
|
||||
from fastapi import Request, Response
|
||||
from starlette.middleware.base import BaseHTTPMiddleware
|
||||
from starlette.types import ASGIApp
|
||||
import secrets
|
||||
|
||||
|
||||
class SecurityHeadersMiddleware(BaseHTTPMiddleware):
|
||||
"""Add security headers to all responses"""
|
||||
|
||||
async def dispatch(self, request: Request, call_next) -> Response:
|
||||
response = await call_next(request)
|
||||
|
||||
# Prevent clickjacking
|
||||
response.headers["X-Frame-Options"] = "DENY"
|
||||
|
||||
# Prevent MIME type sniffing
|
||||
response.headers["X-Content-Type-Options"] = "nosniff"
|
||||
|
||||
# Enable XSS protection (for older browsers)
|
||||
response.headers["X-XSS-Protection"] = "1; mode=block"
|
||||
|
||||
# Strict Transport Security (HTTPS only)
|
||||
# Note: Only enable in production with HTTPS
|
||||
if not request.url.hostname in ["localhost", "127.0.0.1"]:
|
||||
response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains"
|
||||
|
||||
# Content Security Policy (adjust based on frontend needs)
|
||||
csp = (
|
||||
"default-src 'self'; "
|
||||
"script-src 'self' 'unsafe-inline' https://js.stripe.com; "
|
||||
"style-src 'self' 'unsafe-inline'; "
|
||||
"img-src 'self' data: https:; "
|
||||
"font-src 'self' data:; "
|
||||
"connect-src 'self' https://api.stripe.com; "
|
||||
"frame-src https://js.stripe.com;"
|
||||
)
|
||||
response.headers["Content-Security-Policy"] = csp
|
||||
|
||||
# Referrer Policy
|
||||
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
|
||||
|
||||
# Permissions Policy (formerly Feature Policy)
|
||||
response.headers["Permissions-Policy"] = (
|
||||
"geolocation=(), microphone=(), camera=()"
|
||||
)
|
||||
|
||||
return response
|
||||
|
||||
|
||||
class CSRFProtectionMiddleware(BaseHTTPMiddleware):
|
||||
"""
|
||||
Basic CSRF protection for state-changing operations.
|
||||
For API-only applications, this is less critical but still good practice.
|
||||
"""
|
||||
|
||||
def __init__(self, app: ASGIApp, exempt_paths: list = None):
|
||||
super().__init__(app)
|
||||
self.exempt_paths = exempt_paths or [
|
||||
"/api/v1/auth/login",
|
||||
"/api/v1/auth/register",
|
||||
"/api/v1/auth/google",
|
||||
"/docs",
|
||||
"/openapi.json",
|
||||
"/health",
|
||||
]
|
||||
|
||||
async def dispatch(self, request: Request, call_next) -> Response:
|
||||
# Skip CSRF check for safe methods
|
||||
if request.method in ["GET", "HEAD", "OPTIONS"]:
|
||||
return await call_next(request)
|
||||
|
||||
# Skip CSRF check for exempt paths
|
||||
if any(request.url.path.startswith(path) for path in self.exempt_paths):
|
||||
return await call_next(request)
|
||||
|
||||
# For API endpoints using JWT, the token itself provides CSRF protection
|
||||
# This is because attackers can't access the token stored in httpOnly cookies
|
||||
# or local storage from a different origin
|
||||
|
||||
# If implementing cookie-based sessions, would check CSRF token here:
|
||||
# csrf_token = request.headers.get("X-CSRF-Token")
|
||||
# if not csrf_token or not self._validate_csrf_token(csrf_token):
|
||||
# return JSONResponse(
|
||||
# status_code=403,
|
||||
# content={"detail": "CSRF token missing or invalid"}
|
||||
# )
|
||||
|
||||
response = await call_next(request)
|
||||
return response
|
||||
|
||||
@staticmethod
|
||||
def _generate_csrf_token() -> str:
|
||||
"""Generate a secure CSRF token"""
|
||||
return secrets.token_urlsafe(32)
|
||||
|
||||
@staticmethod
|
||||
def _validate_csrf_token(token: str) -> bool:
|
||||
"""Validate CSRF token (implement actual validation logic)"""
|
||||
# In a real implementation, compare against stored token
|
||||
return len(token) == 43 # token_urlsafe(32) produces 43 chars
|
||||
@@ -8,6 +8,7 @@ from fastapi.responses import JSONResponse
|
||||
import logging
|
||||
|
||||
from app.core.config import settings
|
||||
from app.core.middleware import SecurityHeadersMiddleware, CSRFProtectionMiddleware
|
||||
from app.api.v1.api import api_router
|
||||
|
||||
# Configure logging
|
||||
@@ -31,6 +32,10 @@ def create_application() -> FastAPI:
|
||||
openapi_url="/api/openapi.json"
|
||||
)
|
||||
|
||||
# Security middleware (add before CORS)
|
||||
app.add_middleware(SecurityHeadersMiddleware)
|
||||
app.add_middleware(CSRFProtectionMiddleware)
|
||||
|
||||
# CORS middleware
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
[pytest]
|
||||
asyncio_mode = auto
|
||||
testpaths = tests
|
||||
python_files = test_*.py
|
||||
python_classes = Test*
|
||||
python_functions = test_*
|
||||
addopts =
|
||||
-v
|
||||
--strict-markers
|
||||
--tb=short
|
||||
--cov=app
|
||||
--cov-report=term-missing
|
||||
--cov-report=html
|
||||
--cov-branch
|
||||
markers =
|
||||
unit: Unit tests
|
||||
integration: Integration tests
|
||||
e2e: End-to-end tests
|
||||
slow: Slow running tests
|
||||
|
||||
[coverage:run]
|
||||
source = app
|
||||
omit =
|
||||
*/tests/*
|
||||
*/migrations/*
|
||||
*/__pycache__/*
|
||||
*/venv/*
|
||||
|
||||
[coverage:report]
|
||||
precision = 2
|
||||
show_missing = True
|
||||
skip_covered = False
|
||||
|
||||
[coverage:html]
|
||||
directory = htmlcov
|
||||
@@ -0,0 +1,192 @@
|
||||
"""
|
||||
Test configuration and fixtures.
|
||||
"""
|
||||
import pytest
|
||||
import asyncio
|
||||
from typing import AsyncGenerator, Generator
|
||||
from httpx import AsyncClient
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine, async_sessionmaker
|
||||
from sqlalchemy.pool import NullPool
|
||||
|
||||
from app.main import app
|
||||
from app.core.database import Base, get_db
|
||||
from app.core.config import settings
|
||||
from app.models.database_models import User
|
||||
from app.core.security import get_password_hash, create_access_token
|
||||
|
||||
# Test database URL (use different database for tests)
|
||||
TEST_DATABASE_URL = settings.DATABASE_URL.replace("/pop3_forwarder", "/pop3_forwarder_test")
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
def event_loop() -> Generator:
|
||||
"""Create event loop for async tests"""
|
||||
loop = asyncio.get_event_loop_policy().new_event_loop()
|
||||
yield loop
|
||||
loop.close()
|
||||
|
||||
|
||||
@pytest.fixture(scope="function")
|
||||
async def db_engine():
|
||||
"""Create test database engine"""
|
||||
engine = create_async_engine(
|
||||
TEST_DATABASE_URL,
|
||||
poolclass=NullPool,
|
||||
echo=False,
|
||||
)
|
||||
|
||||
# Create tables
|
||||
async with engine.begin() as conn:
|
||||
await conn.run_sync(Base.metadata.drop_all)
|
||||
await conn.run_sync(Base.metadata.create_all)
|
||||
|
||||
yield engine
|
||||
|
||||
# Drop tables
|
||||
async with engine.begin() as conn:
|
||||
await conn.run_sync(Base.metadata.drop_all)
|
||||
|
||||
await engine.dispose()
|
||||
|
||||
|
||||
@pytest.fixture(scope="function")
|
||||
async def db_session(db_engine) -> AsyncGenerator[AsyncSession, None]:
|
||||
"""Create test database session"""
|
||||
async_session_maker = async_sessionmaker(
|
||||
db_engine,
|
||||
class_=AsyncSession,
|
||||
expire_on_commit=False,
|
||||
)
|
||||
|
||||
async with async_session_maker() as session:
|
||||
yield session
|
||||
|
||||
|
||||
@pytest.fixture(scope="function")
|
||||
async def client(db_session: AsyncSession) -> AsyncGenerator[AsyncClient, None]:
|
||||
"""Create test client with database session override"""
|
||||
|
||||
async def override_get_db():
|
||||
yield db_session
|
||||
|
||||
app.dependency_overrides[get_db] = override_get_db
|
||||
|
||||
async with AsyncClient(app=app, base_url="http://test") as client:
|
||||
yield client
|
||||
|
||||
app.dependency_overrides.clear()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def test_user(db_session: AsyncSession) -> User:
|
||||
"""Create a test user"""
|
||||
user = User(
|
||||
email="test@example.com",
|
||||
hashed_password=get_password_hash("testpassword123"),
|
||||
is_active=True,
|
||||
is_verified=True,
|
||||
)
|
||||
db_session.add(user)
|
||||
await db_session.commit()
|
||||
await db_session.refresh(user)
|
||||
return user
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def test_admin_user(db_session: AsyncSession) -> User:
|
||||
"""Create a test admin user"""
|
||||
user = User(
|
||||
email="admin@example.com",
|
||||
hashed_password=get_password_hash("adminpassword123"),
|
||||
is_active=True,
|
||||
is_verified=True,
|
||||
is_admin=True,
|
||||
)
|
||||
db_session.add(user)
|
||||
await db_session.commit()
|
||||
await db_session.refresh(user)
|
||||
return user
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def auth_headers(test_user: User) -> dict:
|
||||
"""Generate authentication headers for test user"""
|
||||
access_token = create_access_token(data={"sub": test_user.email})
|
||||
return {"Authorization": f"Bearer {access_token}"}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def admin_auth_headers(test_admin_user: User) -> dict:
|
||||
"""Generate authentication headers for admin user"""
|
||||
access_token = create_access_token(data={"sub": test_admin_user.email})
|
||||
return {"Authorization": f"Bearer {access_token}"}
|
||||
|
||||
|
||||
# Factory fixtures for creating test data
|
||||
|
||||
@pytest.fixture
|
||||
def user_factory(db_session: AsyncSession):
|
||||
"""Factory for creating test users"""
|
||||
async def _create_user(
|
||||
email: str = None,
|
||||
password: str = "testpassword123",
|
||||
is_active: bool = True,
|
||||
is_verified: bool = True,
|
||||
is_admin: bool = False,
|
||||
) -> User:
|
||||
if email is None:
|
||||
import uuid
|
||||
email = f"test-{uuid.uuid4()}@example.com"
|
||||
|
||||
user = User(
|
||||
email=email,
|
||||
hashed_password=get_password_hash(password),
|
||||
is_active=is_active,
|
||||
is_verified=is_verified,
|
||||
is_admin=is_admin,
|
||||
)
|
||||
db_session.add(user)
|
||||
await db_session.commit()
|
||||
await db_session.refresh(user)
|
||||
return user
|
||||
|
||||
return _create_user
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mail_account_factory(db_session: AsyncSession):
|
||||
"""Factory for creating test mail accounts"""
|
||||
from app.models.database_models import MailAccount
|
||||
from app.core.security import encrypt_password
|
||||
|
||||
async def _create_mail_account(
|
||||
user_id: int,
|
||||
host: str = "pop.example.com",
|
||||
port: int = 995,
|
||||
username: str = None,
|
||||
password: str = "mailpassword",
|
||||
protocol: str = "pop3",
|
||||
use_ssl: bool = True,
|
||||
) -> MailAccount:
|
||||
if username is None:
|
||||
import uuid
|
||||
username = f"test-{uuid.uuid4()}@example.com"
|
||||
|
||||
encrypted_password = encrypt_password(password, user_id)
|
||||
|
||||
account = MailAccount(
|
||||
user_id=user_id,
|
||||
host=host,
|
||||
port=port,
|
||||
username=username,
|
||||
encrypted_password=encrypted_password,
|
||||
protocol=protocol,
|
||||
use_ssl=use_ssl,
|
||||
is_active=True,
|
||||
)
|
||||
db_session.add(account)
|
||||
await db_session.commit()
|
||||
await db_session.refresh(account)
|
||||
return account
|
||||
|
||||
return _create_mail_account
|
||||
@@ -0,0 +1,60 @@
|
||||
"""
|
||||
Unit tests for configuration module.
|
||||
"""
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
from app.core.config import Settings
|
||||
|
||||
|
||||
class TestConfigValidation:
|
||||
"""Test configuration validation"""
|
||||
|
||||
def test_default_secret_key_rejected(self):
|
||||
"""Test that default SECRET_KEY is rejected"""
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
Settings(
|
||||
SECRET_KEY="change-this-to-a-secure-random-secret-key-in-production",
|
||||
ENCRYPTION_KEY="this-is-a-secure-32-character-key-for-testing",
|
||||
)
|
||||
|
||||
assert "SECRET_KEY must be changed from default" in str(exc_info.value)
|
||||
|
||||
def test_short_secret_key_rejected(self):
|
||||
"""Test that short SECRET_KEY is rejected"""
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
Settings(
|
||||
SECRET_KEY="short",
|
||||
ENCRYPTION_KEY="this-is-a-secure-32-character-key-for-testing",
|
||||
)
|
||||
|
||||
assert "at least 32 characters" in str(exc_info.value)
|
||||
|
||||
def test_default_encryption_key_rejected(self):
|
||||
"""Test that default ENCRYPTION_KEY is rejected"""
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
Settings(
|
||||
SECRET_KEY="this-is-a-secure-32-character-key-for-testing",
|
||||
ENCRYPTION_KEY="change-this-to-a-secure-encryption-key",
|
||||
)
|
||||
|
||||
assert "ENCRYPTION_KEY must be changed from default" in str(exc_info.value)
|
||||
|
||||
def test_short_encryption_key_rejected(self):
|
||||
"""Test that short ENCRYPTION_KEY is rejected"""
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
Settings(
|
||||
SECRET_KEY="this-is-a-secure-32-character-key-for-testing",
|
||||
ENCRYPTION_KEY="short",
|
||||
)
|
||||
|
||||
assert "at least 32 characters" in str(exc_info.value)
|
||||
|
||||
def test_valid_keys_accepted(self):
|
||||
"""Test that valid keys are accepted"""
|
||||
settings = Settings(
|
||||
SECRET_KEY="this-is-a-secure-32-character-key-for-testing-secret",
|
||||
ENCRYPTION_KEY="this-is-a-secure-32-character-key-for-encryption",
|
||||
)
|
||||
|
||||
assert settings.SECRET_KEY == "this-is-a-secure-32-character-key-for-testing-secret"
|
||||
assert settings.ENCRYPTION_KEY == "this-is-a-secure-32-character-key-for-encryption"
|
||||
@@ -0,0 +1,103 @@
|
||||
"""
|
||||
Unit tests for security module.
|
||||
"""
|
||||
import pytest
|
||||
from app.core.security import (
|
||||
get_password_hash,
|
||||
verify_password,
|
||||
create_access_token,
|
||||
encrypt_password,
|
||||
decrypt_password,
|
||||
)
|
||||
|
||||
|
||||
class TestPasswordHashing:
|
||||
"""Test password hashing and verification"""
|
||||
|
||||
def test_hash_password(self):
|
||||
"""Test password hashing"""
|
||||
password = "securepassword123"
|
||||
hashed = get_password_hash(password)
|
||||
|
||||
assert hashed != password
|
||||
assert len(hashed) > 50
|
||||
assert hashed.startswith("$2b$")
|
||||
|
||||
def test_verify_password_success(self):
|
||||
"""Test password verification with correct password"""
|
||||
password = "securepassword123"
|
||||
hashed = get_password_hash(password)
|
||||
|
||||
assert verify_password(password, hashed) is True
|
||||
|
||||
def test_verify_password_failure(self):
|
||||
"""Test password verification with wrong password"""
|
||||
password = "securepassword123"
|
||||
wrong_password = "wrongpassword"
|
||||
hashed = get_password_hash(password)
|
||||
|
||||
assert verify_password(wrong_password, hashed) is False
|
||||
|
||||
|
||||
class TestJWT:
|
||||
"""Test JWT token creation and validation"""
|
||||
|
||||
def test_create_access_token(self):
|
||||
"""Test access token creation"""
|
||||
data = {"sub": "test@example.com"}
|
||||
token = create_access_token(data)
|
||||
|
||||
assert isinstance(token, str)
|
||||
assert len(token) > 50
|
||||
assert token.count('.') == 2 # JWT has 3 parts
|
||||
|
||||
|
||||
class TestEncryption:
|
||||
"""Test credential encryption/decryption"""
|
||||
|
||||
def test_encrypt_password(self):
|
||||
"""Test password encryption"""
|
||||
password = "mailpassword123"
|
||||
user_id = 1
|
||||
|
||||
encrypted = encrypt_password(password, user_id)
|
||||
|
||||
assert encrypted != password
|
||||
assert len(encrypted) > 50
|
||||
|
||||
def test_decrypt_password(self):
|
||||
"""Test password decryption"""
|
||||
password = "mailpassword123"
|
||||
user_id = 1
|
||||
|
||||
encrypted = encrypt_password(password, user_id)
|
||||
decrypted = decrypt_password(encrypted, user_id)
|
||||
|
||||
assert decrypted == password
|
||||
|
||||
def test_encryption_with_different_user_ids(self):
|
||||
"""Test that encryption produces different results for different users"""
|
||||
password = "mailpassword123"
|
||||
user_id_1 = 1
|
||||
user_id_2 = 2
|
||||
|
||||
encrypted_1 = encrypt_password(password, user_id_1)
|
||||
encrypted_2 = encrypt_password(password, user_id_2)
|
||||
|
||||
# Different users should produce different encrypted values
|
||||
assert encrypted_1 != encrypted_2
|
||||
|
||||
# But decryption should work correctly for each
|
||||
assert decrypt_password(encrypted_1, user_id_1) == password
|
||||
assert decrypt_password(encrypted_2, user_id_2) == password
|
||||
|
||||
def test_decrypt_with_wrong_user_id_fails(self):
|
||||
"""Test that decryption fails with wrong user ID"""
|
||||
password = "mailpassword123"
|
||||
user_id = 1
|
||||
wrong_user_id = 2
|
||||
|
||||
encrypted = encrypt_password(password, user_id)
|
||||
|
||||
with pytest.raises(Exception):
|
||||
decrypt_password(encrypted, wrong_user_id)
|
||||
Reference in New Issue
Block a user