Skip to content

Commit 8abf3f0

Browse files
Fase 4 & 5 Compleet: Optimalisatie, Security & Cleanup.
- Password hashing verplaatst naar threadpool (non-blocking). - API Keys gemigreerd naar SHA256 (fix voor Bcrypt limiet). - Pydantic V2 modernisatie (model_config, model_dump). - SQLAlchemy 2.0 modernisatie (timezone-aware datetime). - Tests: 100% pass, 0 warnings, >80% coverage. - Services gebruiken nu dependency injection.
1 parent bdf2bb5 commit 8abf3f0

File tree

24 files changed

+359
-108
lines changed

24 files changed

+359
-108
lines changed

app/api/deps.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,5 +107,15 @@ async def get_current_client(
107107
return db_key.client
108108

109109

110+
def get_content_service() -> "ContentService":
111+
"""
112+
Dependency to get content service instance.
113+
In a real app with more complex deps, this could initialize the service.
114+
"""
115+
from app.services.content import ContentService, content_service as _content_service
116+
return _content_service
117+
118+
110119
CurrentUser = Annotated[User, Depends(get_current_user)]
111120
CurrentClient = Annotated[Client, Depends(get_current_client)]
121+

app/api/v1/admin/auth.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ async def login_access_token(
2323
OAuth2 compatible token login, get an access token for future requests.
2424
"""
2525
user = await user_repo.get_by_email(session, email=form_data.username)
26-
if not user or not security.verify_password(form_data.password, user.hashed_password):
26+
if not user or not await security.verify_password(form_data.password, user.hashed_password):
2727
raise HTTPException(
2828
status_code=status.HTTP_400_BAD_REQUEST, detail="Incorrect email or password"
2929
)

app/api/v1/content/guides.py

Lines changed: 73 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,44 +3,93 @@
33

44
from fastapi import APIRouter, Depends, HTTPException, status
55

6-
from app.api.deps import SessionDep, CurrentClient
7-
from app.schemas.guide import GuideCreate, GuideDetailResponse, GuideListResponse, GuideUpdate
8-
from app.services.content import content_service
6+
from app.api.deps import CurrentClient, SessionDep, get_content_service
7+
from app.schemas.guide import (
8+
GuideCreate,
9+
GuideDetailResponse,
10+
GuideListResponse,
11+
GuideUpdate,
12+
)
13+
from app.services.content import ContentService
914

1015
router = APIRouter()
1116

1217

1318
@router.get("/", response_model=list[GuideListResponse])
1419
async def read_guides(
15-
session: SessionDep,
20+
session: SessionDep,
1621
current_client: CurrentClient,
17-
topic_id: UUID,
18-
skip: int = 0,
19-
limit: int = 100
22+
skip: int = 0,
23+
limit: int = 100,
24+
service: ContentService = Depends(get_content_service),
2025
) -> list[GuideListResponse]:
21-
"""Retrieve guides for a specific topic."""
22-
return await content_service.get_guides(session, current_client=current_client, topic_id=topic_id, skip=skip, limit=limit)
26+
"""
27+
Retrieve guides (Global + Private).
28+
"""
29+
return await service.get_guides(
30+
session, current_client=current_client, skip=skip, limit=limit
31+
)
32+
33+
34+
@router.post("/", response_model=GuideListResponse)
35+
async def create_guide(
36+
*,
37+
session: SessionDep,
38+
current_client: CurrentClient,
39+
guide_in: GuideCreate,
40+
service: ContentService = Depends(get_content_service),
41+
) -> GuideListResponse:
42+
"""
43+
Create a new guide.
44+
"""
45+
return await service.create_guide(
46+
session, current_client=current_client, guide_in=guide_in
47+
)
2348

2449

2550
@router.get("/{guide_id}", response_model=GuideDetailResponse)
2651
async def read_guide(
27-
session: SessionDep,
52+
*,
53+
session: SessionDep,
2854
current_client: CurrentClient,
29-
guide_id: UUID
55+
guide_id: UUID,
56+
service: ContentService = Depends(get_content_service),
3057
) -> GuideDetailResponse:
31-
"""Get a specific guide by ID."""
32-
return await content_service.get_guide(session, current_client=current_client, guide_id=guide_id)
33-
58+
"""
59+
Get guide by ID.
60+
"""
61+
guide = await service.get_guide(
62+
session, current_client=current_client, guide_id=guide_id
63+
)
64+
if not guide:
65+
raise HTTPException(
66+
status_code=status.HTTP_404_NOT_FOUND,
67+
detail="Guide not found or access denied",
68+
)
69+
return guide
3470

35-
@router.post("/", response_model=GuideDetailResponse, status_code=status.HTTP_201_CREATED)
36-
async def create_guide(session: SessionDep, guide_in: GuideCreate) -> GuideDetailResponse:
37-
"""Create a new guide."""
38-
return await content_service.create_guide(session, guide_in)
3971

40-
41-
@router.put("/{guide_id}", response_model=GuideDetailResponse)
72+
@router.put("/{guide_id}", response_model=GuideListResponse)
4273
async def update_guide(
43-
session: SessionDep, guide_id: UUID, guide_in: GuideUpdate
44-
) -> GuideDetailResponse:
45-
"""Update a guide."""
46-
return await content_service.update_guide(session, guide_id, guide_in)
74+
*,
75+
session: SessionDep,
76+
current_client: CurrentClient,
77+
guide_id: UUID,
78+
guide_in: GuideUpdate,
79+
service: ContentService = Depends(get_content_service),
80+
) -> GuideListResponse:
81+
"""
82+
Update a guide.
83+
"""
84+
guide = await service.update_guide(
85+
session,
86+
current_client=current_client,
87+
guide_id=guide_id,
88+
guide_in=guide_in,
89+
)
90+
if not guide:
91+
raise HTTPException(
92+
status_code=status.HTTP_404_NOT_FOUND,
93+
detail="Guide not found or access denied",
94+
)
95+
return guide

app/api/v1/content/topics.py

Lines changed: 67 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,42 +3,86 @@
33

44
from fastapi import APIRouter, Depends, HTTPException, status
55

6-
from app.api.deps import SessionDep, CurrentClient
6+
from app.api.deps import CurrentClient, SessionDep, get_content_service
77
from app.schemas.topic import TopicCreate, TopicDetail, TopicResponse, TopicUpdate
8-
from app.services.content import content_service
8+
from app.services.content import ContentService
99

1010
router = APIRouter()
1111

1212

1313
@router.get("/", response_model=list[TopicResponse])
1414
async def read_topics(
15-
session: SessionDep,
15+
session: SessionDep,
1616
current_client: CurrentClient,
17-
skip: int = 0,
18-
limit: int = 100
17+
skip: int = 0,
18+
limit: int = 100,
19+
service: ContentService = Depends(get_content_service),
1920
) -> list[TopicResponse]:
20-
"""Retrieve topics (Global + Private)."""
21-
return await content_service.get_topics(session, current_client=current_client, skip=skip, limit=limit)
21+
"""
22+
Retrieve topics.
23+
Clients only see Global topics or their own Private topics.
24+
"""
25+
return await service.get_topics(
26+
session, current_client=current_client, skip=skip, limit=limit
27+
)
2228

2329

24-
@router.get("/{topic_id}", response_model=TopicDetail)
25-
async def read_topic(
26-
session: SessionDep,
30+
@router.post("/", response_model=TopicResponse)
31+
async def create_topic(
32+
*,
33+
session: SessionDep,
2734
current_client: CurrentClient,
28-
topic_id: UUID
29-
) -> TopicDetail:
30-
"""Get a specific topic by ID."""
31-
return await content_service.get_topic(session, current_client=current_client, topic_id=topic_id)
35+
topic_in: TopicCreate,
36+
service: ContentService = Depends(get_content_service),
37+
) -> TopicResponse:
38+
"""
39+
Create a new topic.
40+
"""
41+
return await service.create_topic(
42+
session, current_client=current_client, topic_in=topic_in
43+
)
3244

3345

34-
# Admin routes (TODO: Add Auth dependency later for protection)
35-
@router.post("/", response_model=TopicResponse, status_code=status.HTTP_201_CREATED)
36-
async def create_topic(session: SessionDep, topic_in: TopicCreate) -> TopicResponse:
37-
"""Create a new topic."""
38-
return await content_service.create_topic(session, topic_in)
46+
@router.put("/{topic_id}", response_model=TopicResponse)
47+
async def update_topic(
48+
*,
49+
session: SessionDep,
50+
current_client: CurrentClient,
51+
topic_id: UUID,
52+
topic_in: TopicUpdate,
53+
service: ContentService = Depends(get_content_service),
54+
) -> TopicResponse:
55+
"""
56+
Update a topic.
57+
"""
58+
topic = await service.update_topic(
59+
session, current_client=current_client, topic_id=topic_id, topic_in=topic_in
60+
)
61+
if not topic:
62+
raise HTTPException(
63+
status_code=status.HTTP_404_NOT_FOUND,
64+
detail="Topic not found or access denied",
65+
)
66+
return topic
3967

4068

41-
@router.put("/{topic_id}", response_model=TopicResponse)
42-
async def update_topic(session: SessionDep, topic_id: UUID, topic_in: TopicUpdate) -> TopicResponse:
43-
"""Update a topic."""
44-
return await content_service.update_topic(session, topic_id, topic_in)
69+
@router.get("/{topic_id}", response_model=TopicDetail)
70+
async def read_topic(
71+
*,
72+
session: SessionDep,
73+
current_client: CurrentClient,
74+
topic_id: UUID,
75+
service: ContentService = Depends(get_content_service),
76+
) -> TopicDetail:
77+
"""
78+
Get topic by ID.
79+
"""
80+
topic = await service.get_topic(
81+
session, current_client=current_client, topic_id=topic_id
82+
)
83+
if not topic:
84+
raise HTTPException(
85+
status_code=status.HTTP_404_NOT_FOUND,
86+
detail="Topic not found or access denied",
87+
)
88+
return topic

app/core/api_key.py

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,8 @@
1-
"""API Key generation and validation for B2B client authentication."""
1+
import hashlib
22
import secrets
33

4-
from passlib.context import CryptContext
5-
64
from app.core.config import settings
75

8-
# API Key hashing context (using bcrypt for consistency)
9-
api_key_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
10-
116

127
def generate_api_key(prefix: str | None = None) -> tuple[str, str]:
138
"""
@@ -21,16 +16,17 @@ def generate_api_key(prefix: str | None = None) -> tuple[str, str]:
2116
2217
Example:
2318
plain_key = "keng_live_abc123..."
24-
hashed_key = "$2b$12$..."
19+
hashed_key = "e3b0c44298..." (SHA256)
2520
"""
2621
if prefix is None:
2722
prefix = settings.api_key_prefix
2823

2924
random_bytes = secrets.token_urlsafe(32)
3025
plain_key = f"{prefix}_live_{random_bytes}"
3126

32-
# Hash the key for storage
33-
hashed_key = str(api_key_context.hash(plain_key))
27+
# Hash the key for storage using SHA256
28+
# API keys are high entropy, so salt/slow-hash is less critical than for passwords
29+
hashed_key = hashlib.sha256(plain_key.encode()).hexdigest()
3430

3531
return plain_key, hashed_key
3632

@@ -47,7 +43,8 @@ def verify_api_key(plain_key: str, hashed_key: str) -> bool:
4743
True if valid, False otherwise
4844
"""
4945
try:
50-
return bool(api_key_context.verify(plain_key, hashed_key))
46+
compare_hash = hashlib.sha256(plain_key.encode()).hexdigest()
47+
return secrets.compare_digest(compare_hash, hashed_key)
5148
except Exception:
5249
return False
5350

@@ -70,4 +67,4 @@ def generate_api_key_hash(plain_key: str) -> str:
7067
Returns:
7168
Hashed version
7269
"""
73-
return str(api_key_context.hash(plain_key))
70+
return hashlib.sha256(plain_key.encode()).hexdigest()

app/core/config.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""Core configuration using Pydantic Settings."""
22

3-
from pydantic import Field, field_validator
3+
from pydantic import Field, ValidationInfo, field_validator
44
from pydantic_settings import BaseSettings, SettingsConfigDict
55

66

@@ -72,6 +72,19 @@ def parse_cors_origins(cls, v: str) -> list[str]:
7272
"""Parse comma-separated CORS origins."""
7373
return [origin.strip() for origin in v.split(",")]
7474

75+
@field_validator("jwt_secret_key", "api_key_secret", mode="after")
76+
@classmethod
77+
def validate_production_secrets(cls, v: str, info: ValidationInfo) -> str:
78+
"""Ensure default secrets are not used in production."""
79+
if info.data.get("environment") == "production":
80+
# Check if value contains default insecure strings
81+
if "insecure" in v:
82+
name = info.field_name
83+
raise ValueError(
84+
f"Production configuration error: {name} must be changed from default insecure value."
85+
)
86+
return v
87+
7588
@property
7689
def is_production(self) -> bool:
7790
"""Check if running in production."""

app/core/security.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from datetime import UTC, datetime, timedelta
33
from typing import Any, cast
44

5+
from fastapi.concurrency import run_in_threadpool
56
from jose import JWTError, jwt
67
from passlib.context import CryptContext
78

@@ -11,14 +12,14 @@
1112
pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
1213

1314

14-
def get_password_hash(password: str) -> str:
15-
"""Hash a password using bcrypt."""
16-
return str(pwd_context.hash(password))
15+
async def get_password_hash(password: str) -> str:
16+
"""Hash a password using bcrypt (non-blocking)."""
17+
return await run_in_threadpool(pwd_context.hash, password)
1718

1819

19-
def verify_password(plain_password: str, hashed_password: str) -> bool:
20-
"""Verify a password against its hash."""
21-
return bool(pwd_context.verify(plain_password, hashed_password))
20+
async def verify_password(plain_password: str, hashed_password: str) -> bool:
21+
"""Verify a password against its hash (non-blocking)."""
22+
return await run_in_threadpool(pwd_context.verify, plain_password, hashed_password)
2223

2324

2425
def create_access_token(data: dict[str, Any], expires_delta: timedelta | None = None) -> str:

app/db/init_db.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ async def create_initial_admin(session: AsyncSession) -> None:
3030
# Create admin user
3131
admin = User(
3232
email=settings.admin_email,
33-
hashed_password=get_password_hash(settings.admin_password),
33+
hashed_password=await get_password_hash(settings.admin_password),
3434
is_active=True,
3535
is_superuser=True,
3636
)

app/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ async def custom_exception_handler(_request: Request, exc: BaseAPIError) -> JSON
6565
code=exc.__class__.__name__.replace("Exception", "").upper(),
6666
message=exc.message,
6767
details=exc.details,
68-
).dict(),
68+
).model_dump(),
6969
)
7070

7171

app/models/api_key.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""API Key model for B2B authentication."""
22
import uuid
3-
from datetime import UTC, datetime
3+
from datetime import datetime, timezone
44
from typing import TYPE_CHECKING
55
from uuid import UUID
66

@@ -38,7 +38,7 @@ class APIKey(Base):
3838
) # Optional name ("Production Key", etc.)
3939
is_active: Mapped[bool] = mapped_column(Boolean, default=True, nullable=False, index=True)
4040
last_used_at: Mapped[datetime | None] = mapped_column(DateTime, nullable=True)
41-
created_at: Mapped[datetime] = mapped_column(DateTime, server_default=func.now())
41+
created_at: Mapped[datetime] = mapped_column(DateTime, default=lambda: datetime.now(timezone.utc))
4242
expires_at: Mapped[datetime | None] = mapped_column(
4343
DateTime, nullable=True
4444
) # None = no expiration

0 commit comments

Comments
 (0)