Files
oicd/.archive/PROJECT_ANALYSIS.md
2025-11-30 00:07:24 +01:00

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:

  1. ❌ Alles in oidc_server.py
  2. ❌ Keine Service-Layer
  3. ❌ Keine Repository-Layer
  4. ❌ Templates als Python-Strings (sollten HTML-Files sein)
  5. ❌ Keine Test-Struktur
  6. ❌ 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:

  1. Password Hashing:

    • ✅ bcrypt verwendet (korrekt)
    • ✅ Salting automatisch
  2. Rate Limiting:

    • ✅ Flask-Limiter eingebunden
    • ✅ Auf kritischen Endpoints
  3. Audit Logging:

    • ✅ Security Events werden geloggt
    • ✅ IP-Adresse und User-Agent
  4. JWT Signing:

    • ✅ RS256 (asymmetrisch)
    • ✅ Keys in Files (nicht hardcoded)
  5. Environment Config:

    • ✅ .env Files
    • ✅ Secrets nicht im Code

⚠️ Verbesserungsbedarf:

  1. Keine strukturierte Logging:

    # Aktuell: print-ähnlich
    # Sollte: Loguru mit strukturiertem Logging
    logger.info("User registered", extra={"user_id": user.id, "username": user.username})
    
  2. Keine Custom Exceptions:

    # Aktuell: HTTP Exception direkt
    # Sollte: AppException Hierarchy
    raise NotFoundError("User", user_id)
    
  3. Keine Request ID Tracking:

    # Sollte: Request-ID in jedem Log
    logger.info("Request started", extra={"request_id": uuid.uuid4()})
    
  4. 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:

  1. N+1 Query Problem:

    # Keine Eager Loading erkennbar
    users = User.query.all()
    for user in users:
        print(user.audit_logs)  # Separate query!
    
  2. Keine Query Optimization:

    # Sollte: .options(joinedload(User.permissions))
    
  3. 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:

  1. Keine Environment-specific Dockerfiles:

    • Sollte: Dockerfile.dev, Dockerfile.prod
  2. Keine Docker Secrets:

    • Sollte: Docker Secrets für Produktion
  3. Kein Redis für Rate Limiting:

    • Aktuell: In-Memory (geht bei Restart verloren)
    • Sollte: Redis für Produktion

Metrics & Monitoring

❌ Fehlt komplett:

  1. Keine Metriken:

    • Kein Prometheus Exporter
    • Keine Request-Latency Tracking
    • Keine Error-Rate Metrics
  2. Keine Monitoring-Integration:

    • Kein Grafana Dashboard
    • Keine Alerts
  3. Keine Tracing:

    • Kein OpenTelemetry
    • Keine Request-Flow-Verfolgung

Zusammenfassung: Was muss geändert werden?

🔴 Kritisch (Blocker für Wachstum)

  1. Refactoring in 4-Layer-Architektur

    • Aufwand: 40-60 Stunden
    • Priorität: HOCH
    • Impact: Wartbarkeit, Testbarkeit, Skalierbarkeit
  2. Service Layer einführen

    • Aufwand: 20-30 Stunden
    • Priorität: HOCH
    • Impact: Business Logic isoliert
  3. Repository Layer einführen

    • Aufwand: 10-15 Stunden
    • Priorität: HOCH
    • Impact: Database Queries isoliert

🟡 Wichtig (Sollte gemacht werden)

  1. Pydantic Schemas

    • Aufwand: 8-10 Stunden
    • Priorität: MITTEL
    • Impact: Validation, Type Safety
  2. Templates zu HTML-Files

    • Aufwand: 4-6 Stunden
    • Priorität: MITTEL
    • Impact: Maintainability
  3. Test Suite aufbauen

    • Aufwand: 15-20 Stunden
    • Priorität: MITTEL
    • Impact: Confidence bei Changes

🟢 Nice-to-Have (Später)

  1. Structured Logging (Loguru)

    • Aufwand: 4-6 Stunden
    • Priorität: NIEDRIG
    • Impact: Debugging
  2. Monitoring & Metrics

    • Aufwand: 8-12 Stunden
    • Priorität: NIEDRIG
    • Impact: Observability

Migrations-Plan

Phase 1: Vorbereitung (2-3 Tage)

  1. Neue Ordnerstruktur erstellen
  2. Dependencies installieren (Pydantic, etc.)
  3. Test-Setup vorbereiten

Phase 2: Layer-Trennung (1-2 Wochen)

  1. Models aufteilen → app/models/
  2. Schemas erstellen → app/schemas/
  3. Repositories erstellen → app/repositories/
  4. Services erstellen → app/services/

Phase 3: Endpoints anpassen (1 Woche)

  1. Endpoints refactoren (thin!)
  2. Dependency Injection einführen
  3. Route-Struktur neu aufbauen

Phase 4: Templates & Tests (1 Woche)

  1. Templates zu HTML-Files
  2. Unit Tests für Services
  3. Integration Tests für API

Gesamt: 3-4 Wochen Vollzeit-Arbeit


Empfehlung

Kurz-Fristig (Diese Woche):

  1. ✅ Deployment funktioniert - Lassen!
  2. ⚠️ Admin-Passwort ändern - Sofort!
  3. ⚠️ Backup-Strategie - Einrichten!

Mittel-Fristig (Nächste 2 Wochen):

  1. 🔴 Service Layer - Starten!
  2. 🔴 Repository Layer - Starten!
  3. 🟡 Pydantic Schemas - Parallel!

Lang-Fristig (Nächste 4 Wochen):

  1. 🔴 Komplette Refactoring - Planen!
  2. 🟡 Test Suite - Aufbauen!
  3. 🟡 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:

  1. Diese Analyse mit dem Team besprechen
  2. Entscheiden: Behalten oder Refactoren?
  3. Wenn Refactoren: Migrations-Plan umsetzen
  4. Wenn Behalten: Regelmäßige Code Reviews einführen

Ende der Analyse