24 KiB
OIDC Identity Provider - Projekt-Analyse
Datum: 2025-11-27 Status: Produktionsbereit (mit Verbesserungspotenzial)
Executive Summary
Das Projekt ist ein funktionsfähiger OIDC Identity Provider, erfolgreich deployed und getestet. Allerdings weicht die Architektur massiv von modernen Python Best Practices ab (Python Quick Start Guide).
Gesamtbewertung: 5/10
Funktional: ✅ Gut (8/10) Architektur: ❌ Mangelhaft (3/10) Wartbarkeit: ⚠️ Problematisch (4/10) Skalierbarkeit: ❌ Sehr eingeschränkt (2/10)
Kritische Probleme
🔴 KRITISCH: Monolithische Struktur
Problem 1: Alles in einer Datei (948 Zeilen!)
oidc_server.py - 948 Zeilen
Guide-Empfehlung: Max 300-500 Zeilen pro Modul
Ist-Zustand: 948 Zeilen in EINER Datei
Überschreitung: 189% - 89% zu groß!
Was ist drin:
- 23 Flask-Routen (API-Endpoints)
- Business Logic direkt in Endpoints
- Database Queries direkt in Endpoints
- Template-Rendering
- JWT-Verarbeitung
- Session-Management
- Error Handling
- Keine Layer-Trennung
Das ist wie: Ein Restaurant, wo der Koch gleichzeitig Kellner, Kassierer, Geschäftsführer und Buchhalter ist.
🔴 KRITISCH: Keine Layer-Architektur
Der Guide fordert 4 Layer:
Guide-Anforderung:
┌─────────────────────────────────────┐
│ 1. API LAYER (endpoints) │ ← Thin (5-10 lines)
├─────────────────────────────────────┤
│ 2. SERVICE LAYER │ ← Business logic
├─────────────────────────────────────┤
│ 3. REPOSITORY LAYER │ ← Database ops
├─────────────────────────────────────┤
│ 4. MODEL LAYER │ ← Data structures
└─────────────────────────────────────┘
Ist-Zustand:
┌─────────────────────────────────────┐
│ Alles in oidc_server.py │ ← 948 Zeilen
│ + models.py │ ← 329 Zeilen
└─────────────────────────────────────┘
Fehlende Layer:
- ❌ Kein Service Layer
- ❌ Kein Repository Layer
- ❌ Keine Dependency Injection
- ❌ Keine Trennung von Verantwortlichkeiten
🔴 KRITISCH: Endpoints mit Business Logic
Beispiel 1: /admin/user/create (66 Zeilen!)
@app.route('/admin/user/create', methods=['GET', 'POST'])
@admin_required
def admin_create_user():
# GET-Logic
if request.method == 'GET':
return render_template_string(...)
# POST-Logic mit:
# - Form-Validierung (Business Logic!)
# - Password-Hashing (Business Logic!)
# - Database Insert (Database Operation!)
# - Permission-Parsing (Business Logic!)
# - Error Handling
# - Audit Logging
# - Session Management
# = 66 ZEILEN!
Guide-Anforderung: Max 10 Zeilen, nur Service-Aufruf!
# So sollte es sein:
@router.post("/users", response_model=UserResponse)
async def create_user(
user_in: UserCreate,
service: Annotated[UserService, Depends(get_user_service)],
) -> UserResponse:
return await service.register_user(user_in) # 1 Zeile!
Weitere Beispiele:
/register- 52 Zeilen (sollte: 10)/login- 68 Zeilen (sollte: 10)/token- 85 Zeilen (sollte: 10)/authorize- 72 Zeilen (sollte: 10)
Struktur-Vergleich: Ist vs. Soll
Aktuelle Struktur (Ist)
wlkns_auth/
├── oidc_server.py # 948 Zeilen - ALLES drin! ❌
├── models.py # 329 Zeilen - OK ✅
├── config.py # 157 Zeilen - OK ✅
├── templates.py # 305 Zeilen - Templates als Strings ⚠️
├── admin_templates.py # 697 Zeilen - Templates als Strings ⚠️
├── test_client.py # Test-Tool
├── migrations/ # Alembic migrations ✅
├── static/ # CSS files ✅
└── instance/ # JWT keys ✅
Probleme:
- ❌ Alles in
oidc_server.py - ❌ Keine Service-Layer
- ❌ Keine Repository-Layer
- ❌ Templates als Python-Strings (sollten HTML-Files sein)
- ❌ Keine Test-Struktur
- ❌ Keine Schemas (Pydantic)
Empfohlene Struktur (Soll)
wlkns_auth/
├── app/
│ ├── __init__.py
│ ├── main.py # Flask app (50 lines)
│ ├── config.py # Settings ✅ (bereits gut)
│ ├── dependencies.py # DI container (NEW)
│ │
│ ├── api/
│ │ └── v1/
│ │ ├── endpoints/
│ │ │ ├── auth.py # Login, register (10-15 lines each)
│ │ │ ├── users.py # User CRUD (10-15 lines each)
│ │ │ ├── admin.py # Admin panel (10-15 lines each)
│ │ │ └── oidc.py # OIDC endpoints (10-15 lines each)
│ │ └── router.py # Route aggregation
│ │
│ ├── services/ # NEW - Business Logic HERE
│ │ ├── user_service.py # User registration, profile
│ │ ├── auth_service.py # Login, password management
│ │ ├── oidc_service.py # OIDC flow logic
│ │ ├── admin_service.py # Admin operations
│ │ └── token_service.py # JWT generation/validation
│ │
│ ├── repositories/ # NEW - Database Operations HERE
│ │ ├── base_repository.py # Generic CRUD
│ │ ├── user_repository.py # User DB operations
│ │ ├── client_repository.py # Client DB operations
│ │ └── token_repository.py # Token DB operations
│ │
│ ├── models/ # SQLAlchemy models
│ │ ├── user.py # ✅ (from current models.py)
│ │ ├── client.py
│ │ ├── token.py
│ │ └── audit_log.py
│ │
│ ├── schemas/ # NEW - Pydantic Schemas
│ │ ├── user.py # UserCreate, UserResponse
│ │ ├── auth.py # LoginRequest, TokenResponse
│ │ ├── oidc.py # OIDCRequest, OIDCResponse
│ │ └── admin.py # Admin schemas
│ │
│ ├── core/
│ │ ├── database.py # DB setup
│ │ ├── security.py # Password hashing, JWT
│ │ └── logging_config.py # Logging setup
│ │
│ ├── templates/ # HTML files (not strings!)
│ │ ├── base.html
│ │ ├── login.html
│ │ ├── register.html
│ │ └── admin/
│ │ ├── dashboard.html
│ │ └── users.html
│ │
│ ├── exceptions.py # Custom exceptions
│ └── utils/
│
├── tests/ # NEW - Test structure
│ ├── conftest.py
│ ├── test_api/
│ │ ├── test_auth.py
│ │ ├── test_users.py
│ │ └── test_oidc.py
│ └── test_services/
│ ├── test_user_service.py
│ └── test_auth_service.py
│
├── static/ # ✅ Already exists
├── instance/ # ✅ Already exists
├── migrations/ # ✅ Already exists
├── .env
├── requirements.txt
└── README.md
Vorteile:
- ✅ Klare Trennung der Verantwortlichkeiten
- ✅ Jedes Modul < 300 Zeilen
- ✅ Einfach zu testen (Mocks für Services/Repos)
- ✅ Skalierbar (neue Features = neue Service)
- ✅ Wartbar (Bug? → Klare Stelle!)
- ✅ Team-fähig (mehrere Entwickler parallel)
Code-Analyse: Konkrete Beispiele
Beispiel 1: User Registration
Aktuell (68 Zeilen in /register):
@app.route('/register', methods=['GET', 'POST'])
@limiter.limit("5 per minute")
def register():
if request.method == 'GET':
return render_template_string(REGISTER_TEMPLATE, error=None, success=None)
username = request.form.get('username')
email = request.form.get('email')
name = request.form.get('name')
password = request.form.get('password')
password2 = request.form.get('password2')
# Validation (Business Logic!)
if not all([username, email, name, password, password2]):
return render_template_string(REGISTER_TEMPLATE, error="All fields required", success=None)
if password != password2:
return render_template_string(REGISTER_TEMPLATE, error="Passwords don't match", success=None)
# Check existence (Database Query!)
existing_user = User.query.filter_by(username=username).first()
if existing_user:
return render_template_string(REGISTER_TEMPLATE, error="Username exists", success=None)
existing_email = User.query.filter_by(email=email).first()
if existing_email:
return render_template_string(REGISTER_TEMPLATE, error="Email exists", success=None)
# Create user (Database Operation!)
new_user = User(
username=username,
email=email,
name=name,
preferred_username=username,
)
new_user.set_password(password)
db.session.add(new_user)
db.session.commit()
# Audit log (More Business Logic!)
AuditLog.log(
action='user_registered',
username=username,
user_id=new_user.id,
ip_address=request.remote_addr,
user_agent=request.headers.get('User-Agent')
)
return render_template_string(REGISTER_TEMPLATE, error=None, success="Registration successful")
Probleme:
- ❌ 68 Zeilen (sollte: 10)
- ❌ Business Logic im Endpoint
- ❌ Database Queries im Endpoint
- ❌ Keine Type Hints
- ❌ Nicht testbar (kein Mock möglich)
- ❌ Schwer zu warten
So sollte es sein (Guide-konform):
# app/api/v1/endpoints/auth.py
from fastapi import APIRouter, Depends, status
from typing import Annotated
from app.schemas.auth import UserRegistration, UserResponse
from app.services.user_service import UserService
from app.dependencies import get_user_service
router = APIRouter()
@router.post("/register", response_model=UserResponse, status_code=status.HTTP_201_CREATED)
async def register_user(
registration: UserRegistration,
service: Annotated[UserService, Depends(get_user_service)],
) -> UserResponse:
"""Register new user - endpoint is THIN."""
return await service.register_user(registration)
# ← 10 Zeilen, Business Logic in Service!
# app/services/user_service.py
class UserService:
def __init__(self, user_repo: UserRepository, audit_service: AuditService):
self.user_repo = user_repo
self.audit_service = audit_service
async def register_user(self, data: UserRegistration) -> User:
"""Register new user - ALL business logic here."""
# Business Rule 1: Check uniqueness
if await self.user_repo.get_by_username(data.username):
raise ConflictError("Username already exists")
if await self.user_repo.get_by_email(data.email):
raise ConflictError("Email already exists")
# Business Rule 2: Hash password
hashed_password = get_password_hash(data.password)
# Create user via repository
user = await self.user_repo.create(
username=data.username,
email=data.email,
name=data.name,
hashed_password=hashed_password,
)
# Business Rule 3: Log registration
await self.audit_service.log_user_registration(user)
return user
# app/repositories/user_repository.py
class UserRepository:
def __init__(self, db: Session):
self.db = db
async def get_by_username(self, username: str) -> Optional[User]:
"""Get user by username - ONLY database operation."""
return self.db.query(User).filter(User.username == username).first()
async def get_by_email(self, email: str) -> Optional[User]:
"""Get user by email - ONLY database operation."""
return self.db.query(User).filter(User.email == email).first()
async def create(self, username: str, email: str, name: str, hashed_password: str) -> User:
"""Create new user - ONLY database operation."""
user = User(username=username, email=email, name=name, hashed_password=hashed_password)
self.db.add(user)
self.db.flush()
return user
Vorteile:
- ✅ Endpoint: 10 Zeilen (wie gefordert)
- ✅ Business Logic isoliert im Service
- ✅ Database Queries isoliert im Repository
- ✅ Type Hints überall
- ✅ Einfach testbar mit Mocks
- ✅ Service kann von mehreren Endpoints genutzt werden
Beispiel 2: Token Endpoint
Aktuell (85 Zeilen!):
@app.route('/token', methods=['POST'])
@limiter.limit("10 per minute")
def token():
# Parameter extraction
grant_type = request.form.get('grant_type')
code = request.form.get('code')
redirect_uri = request.form.get('redirect_uri')
client_id = request.form.get('client_id')
client_secret = request.form.get('client_secret')
# Validation (20 lines of if statements)
# Database queries (multiple!)
# Token generation (JWT logic)
# Response building
# Error handling
# 85 LINES TOTAL!
Sollte sein:
# app/api/v1/endpoints/oidc.py (10 lines)
@router.post("/token")
async def exchange_token(
request: TokenRequest,
service: Annotated[OIDCService, Depends(get_oidc_service)],
) -> TokenResponse:
return await service.exchange_authorization_code(request)
# app/services/oidc_service.py (Business Logic)
class OIDCService:
async def exchange_authorization_code(self, request: TokenRequest) -> TokenResponse:
# Validation
# Code verification
# Token generation
# All business rules here
Type Hints & Validation
❌ Aktueller Zustand: Keine Type Hints
# Keine Ahnung, was zurückgegeben wird
def admin_create_user():
# ...
return render_template_string(...) # Was für ein Typ?
# Keine Ahnung, was Parameter sind
def authorize():
# request.args.get() → str? None? List?
client_id = request.args.get('client_id')
✅ Sollte sein: Type Hints + Pydantic
# app/schemas/auth.py
from pydantic import BaseModel, EmailStr, Field
class UserRegistration(BaseModel):
username: str = Field(..., min_length=3, max_length=50)
email: EmailStr
password: str = Field(..., min_length=8)
name: str = Field(..., min_length=1)
class UserResponse(BaseModel):
id: int
username: str
email: str
name: str
is_active: bool
created_at: datetime
model_config = ConfigDict(from_attributes=True)
Vorteile:
- ✅ Automatische Validierung
- ✅ Klare API-Dokumentation
- ✅ Type Safety (IDE Support)
- ✅ Keine manuellen Checks
Testing
❌ Aktueller Zustand: Nicht testbar
# Wie testet man das?
@app.route('/register', methods=['GET', 'POST'])
def register():
# 68 Zeilen mit:
# - Flask Request (global!)
# - Database Queries (direkt!)
# - Session (global!)
# - Unmöglich zu mocken!
Test-Code wäre:
def test_register():
# Braucht:
# - Flask app context
# - Database setup
# - Request mocking
# - Session mocking
# - Kompliziert und langsam!
✅ Sollte sein: Unit Tests für Services
# tests/test_services/test_user_service.py
async def test_register_user_success(user_service, mock_user_repo):
# Arrange
mock_user_repo.get_by_username = AsyncMock(return_value=None)
mock_user_repo.create = AsyncMock(return_value=Mock(id=1))
data = UserRegistration(username="test", email="test@test.com", password="pass123")
# Act
result = await user_service.register_user(data)
# Assert
assert result.id == 1
mock_user_repo.create.assert_called_once()
# Schnell, isoliert, klar!
Templates
❌ Aktueller Zustand: Python Strings
# templates.py (305 Zeilen)
# admin_templates.py (697 Zeilen)
LOGIN_TEMPLATE = """
<!DOCTYPE html>
<html>
<head>...</head>
<body>
<!-- 200 Zeilen HTML als Python String! -->
</body>
</html>
"""
Probleme:
- ❌ Keine Syntax-Highlighting
- ❌ Keine Auto-Completion
- ❌ Schwer zu debuggen
- ❌ Keine Template-Vererbung
- ❌ Keine IDE-Unterstützung
- ❌ 1002 Zeilen nur für Templates!
✅ Sollte sein: Template-Dateien
app/templates/
├── base.html # Base layout mit Theme-Toggle
├── auth/
│ ├── login.html # Extends base.html
│ └── register.html # Extends base.html
└── admin/
├── base_admin.html # Admin-specific base
├── dashboard.html # Extends base_admin.html
└── users.html # Extends base_admin.html
<!-- app/templates/base.html -->
<!DOCTYPE html>
<html>
<head>
<link rel="stylesheet" href="{{ url_for('static', filename='styles.css') }}">
{% block head %}{% endblock %}
</head>
<body>
{% block content %}{% endblock %}
</body>
</html>
<!-- app/templates/auth/login.html -->
{% extends "base.html" %}
{% block content %}
<div class="container">
<h1>Login</h1>
<!-- Form hier -->
</div>
{% endblock %}
Vorteile:
- ✅ Syntax-Highlighting
- ✅ Template-Vererbung (DRY)
- ✅ IDE-Support
- ✅ Getrennt von Python-Code
Security & Best Practices
✅ Gut gemacht:
-
Password Hashing:
- ✅ bcrypt verwendet (korrekt)
- ✅ Salting automatisch
-
Rate Limiting:
- ✅ Flask-Limiter eingebunden
- ✅ Auf kritischen Endpoints
-
Audit Logging:
- ✅ Security Events werden geloggt
- ✅ IP-Adresse und User-Agent
-
JWT Signing:
- ✅ RS256 (asymmetrisch)
- ✅ Keys in Files (nicht hardcoded)
-
Environment Config:
- ✅
.envFiles - ✅ Secrets nicht im Code
- ✅
⚠️ Verbesserungsbedarf:
-
Keine strukturierte Logging:
# Aktuell: print-ähnlich # Sollte: Loguru mit strukturiertem Logging logger.info("User registered", extra={"user_id": user.id, "username": user.username}) -
Keine Custom Exceptions:
# Aktuell: HTTP Exception direkt # Sollte: AppException Hierarchy raise NotFoundError("User", user_id) -
Keine Request ID Tracking:
# Sollte: Request-ID in jedem Log logger.info("Request started", extra={"request_id": uuid.uuid4()}) -
Keine Input Validation (Pydantic):
# Aktuell: Manuelle Checks if not username: return error # Sollte: Pydantic automatisch class UserCreate(BaseModel): username: str = Field(..., min_length=3)
Migrations & Database
✅ Gut gemacht:
migrations/
├── env.py
└── versions/
├── 8ee9394b7cd5_initial_migration.py
└── d0b3ddd682f3_add_client_model.py
- ✅ Alembic korrekt eingerichtet
- ✅ Migrationen vorhanden
- ✅ Models mit Relationships
⚠️ Models.py sollte aufgeteilt werden:
Aktuell: models.py (329 Zeilen)
# models.py
class User(db.Model): pass
class Client(db.Model): pass
class AuthorizationCode(db.Model): pass
class AccessToken(db.Model): pass
class AuditLog(db.Model): pass
Sollte:
app/models/
├── __init__.py
├── user.py # User model (80 lines)
├── client.py # Client model (60 lines)
├── token.py # Token models (100 lines)
└── audit_log.py # AuditLog model (50 lines)
Performance & Skalierung
⚠️ Potenzielle Probleme:
-
N+1 Query Problem:
# Keine Eager Loading erkennbar users = User.query.all() for user in users: print(user.audit_logs) # Separate query! -
Keine Query Optimization:
# Sollte: .options(joinedload(User.permissions)) -
Keine Connection Pooling Config:
# config.py sollte haben: engine = create_engine( DATABASE_URL, pool_size=10, max_overflow=20, pool_pre_ping=True )
Docker & Deployment
✅ Gut gemacht:
# Dockerfile
- ✅ Multi-stage Build Pattern
- ✅ Non-root User (UID 1000)
- ✅ Health Check
- ✅ Gunicorn WSGI Server
# docker-compose.prod.yml
- ✅ PostgreSQL mit Health Check
- ✅ Named Volumes (Persistence)
- ✅ Only localhost exposure
- ✅ Log rotation
⚠️ Verbesserungspotential:
-
Keine Environment-specific Dockerfiles:
- Sollte:
Dockerfile.dev,Dockerfile.prod
- Sollte:
-
Keine Docker Secrets:
- Sollte: Docker Secrets für Produktion
-
Kein Redis für Rate Limiting:
- Aktuell: In-Memory (geht bei Restart verloren)
- Sollte: Redis für Produktion
Metrics & Monitoring
❌ Fehlt komplett:
-
Keine Metriken:
- Kein Prometheus Exporter
- Keine Request-Latency Tracking
- Keine Error-Rate Metrics
-
Keine Monitoring-Integration:
- Kein Grafana Dashboard
- Keine Alerts
-
Keine Tracing:
- Kein OpenTelemetry
- Keine Request-Flow-Verfolgung
Zusammenfassung: Was muss geändert werden?
🔴 Kritisch (Blocker für Wachstum)
-
Refactoring in 4-Layer-Architektur
- Aufwand: 40-60 Stunden
- Priorität: HOCH
- Impact: Wartbarkeit, Testbarkeit, Skalierbarkeit
-
Service Layer einführen
- Aufwand: 20-30 Stunden
- Priorität: HOCH
- Impact: Business Logic isoliert
-
Repository Layer einführen
- Aufwand: 10-15 Stunden
- Priorität: HOCH
- Impact: Database Queries isoliert
🟡 Wichtig (Sollte gemacht werden)
-
Pydantic Schemas
- Aufwand: 8-10 Stunden
- Priorität: MITTEL
- Impact: Validation, Type Safety
-
Templates zu HTML-Files
- Aufwand: 4-6 Stunden
- Priorität: MITTEL
- Impact: Maintainability
-
Test Suite aufbauen
- Aufwand: 15-20 Stunden
- Priorität: MITTEL
- Impact: Confidence bei Changes
🟢 Nice-to-Have (Später)
-
Structured Logging (Loguru)
- Aufwand: 4-6 Stunden
- Priorität: NIEDRIG
- Impact: Debugging
-
Monitoring & Metrics
- Aufwand: 8-12 Stunden
- Priorität: NIEDRIG
- Impact: Observability
Migrations-Plan
Phase 1: Vorbereitung (2-3 Tage)
- Neue Ordnerstruktur erstellen
- Dependencies installieren (Pydantic, etc.)
- Test-Setup vorbereiten
Phase 2: Layer-Trennung (1-2 Wochen)
- Models aufteilen →
app/models/ - Schemas erstellen →
app/schemas/ - Repositories erstellen →
app/repositories/ - Services erstellen →
app/services/
Phase 3: Endpoints anpassen (1 Woche)
- Endpoints refactoren (thin!)
- Dependency Injection einführen
- Route-Struktur neu aufbauen
Phase 4: Templates & Tests (1 Woche)
- Templates zu HTML-Files
- Unit Tests für Services
- Integration Tests für API
Gesamt: 3-4 Wochen Vollzeit-Arbeit
Empfehlung
Kurz-Fristig (Diese Woche):
- ✅ Deployment funktioniert - Lassen!
- ⚠️ Admin-Passwort ändern - Sofort!
- ⚠️ Backup-Strategie - Einrichten!
Mittel-Fristig (Nächste 2 Wochen):
- 🔴 Service Layer - Starten!
- 🔴 Repository Layer - Starten!
- 🟡 Pydantic Schemas - Parallel!
Lang-Fristig (Nächste 4 Wochen):
- 🔴 Komplette Refactoring - Planen!
- 🟡 Test Suite - Aufbauen!
- 🟡 Templates - Auslagern!
Fazit
Das Projekt ist funktional gut, aber architektonisch ein Monolith.
Für ein Homelab-Projekt ist es ausreichend. Für ein Team-Projekt oder kommerzielle Nutzung ist dringend Refactoring nötig.
Der Code funktioniert - aber er ist nicht wartbar, testbar oder skalierbar.
Nächste Schritte:
- Diese Analyse mit dem Team besprechen
- Entscheiden: Behalten oder Refactoren?
- Wenn Refactoren: Migrations-Plan umsetzen
- Wenn Behalten: Regelmäßige Code Reviews einführen
Ende der Analyse