Implements Phase 2 infrastructure for participant registration and authentication: Database Models: - Add Participant model with exchange scoping and soft deletes - Add MagicToken model for passwordless authentication - Add participants relationship to Exchange model - Include proper indexes and foreign key constraints Migration Infrastructure: - Generate Alembic migration for new models - Create entrypoint.sh script for automatic migrations on container startup - Update Containerfile to use entrypoint script and include uv binary - Remove db.create_all() in favor of migration-based schema management This establishes the foundation for implementing stories 4.1-4.3, 5.1-5.3, and 10.1. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
934 lines
32 KiB
Markdown
934 lines
32 KiB
Markdown
# Phase 2 Implementation Decisions
|
|
|
|
**Version**: 0.2.0
|
|
**Date**: 2025-12-22
|
|
**Status**: Architectural Guidance
|
|
|
|
## Overview
|
|
|
|
This document provides architectural answers to implementation questions raised during Phase 2 design review. These decisions are based on existing codebase patterns, Flask/Python best practices, and the project's design principles (simplicity, self-host friendly, security).
|
|
|
|
---
|
|
|
|
## 1. Database Schema Decisions
|
|
|
|
### 1.1 MagicToken Foreign Key Constraints
|
|
|
|
**Question**: How to enforce that magic_link tokens have participant_id/exchange_id NOT NULL, while password_reset tokens have them NULL?
|
|
|
|
**Answer**: **Option A - Nullable columns with application-level validation**
|
|
|
|
**Rationale**:
|
|
- SQLite has limited CHECK constraint support for complex conditions
|
|
- Application-level validation is already the project pattern (see Exchange state transitions in `/home/phil/Projects/sneaky-klaus/src/models/exchange.py`)
|
|
- Simpler migration path (no separate tables)
|
|
- Token cleanup job handles both types uniformly
|
|
|
|
**Implementation**:
|
|
```python
|
|
class MagicToken(db.Model):
|
|
# Columns are nullable
|
|
participant_id: Mapped[int | None] = mapped_column(Integer, ForeignKey('participant.id'), nullable=True)
|
|
exchange_id: Mapped[int | None] = mapped_column(Integer, ForeignKey('exchange.id'), nullable=True)
|
|
|
|
def validate(self):
|
|
"""Validate foreign key requirements based on token type."""
|
|
if self.token_type == 'magic_link':
|
|
if not self.participant_id or not self.exchange_id:
|
|
raise ValueError("Magic link tokens require participant_id and exchange_id")
|
|
elif self.token_type == 'password_reset':
|
|
if self.participant_id or self.exchange_id:
|
|
raise ValueError("Password reset tokens cannot have participant_id or exchange_id")
|
|
```
|
|
|
|
---
|
|
|
|
### 1.2 Participant Email Normalization
|
|
|
|
**Question**: Should emails be normalized (lowercased) before storage?
|
|
|
|
**Answer**: **Yes - always normalize to lowercase before storage**
|
|
|
|
**Rationale**:
|
|
- Existing admin login pattern already does this: `/home/phil/Projects/sneaky-klaus/src/routes/admin.py` line 40 `email = form.email.data.lower()`
|
|
- Consistency across admin and participant authentication
|
|
- Prevents duplicate registrations with different casing (Alice@example.com vs alice@example.com)
|
|
- Email comparison is case-insensitive per RFC 5321
|
|
|
|
**Implementation Pattern**:
|
|
```python
|
|
# During participant registration
|
|
email = form.email.data.lower().strip()
|
|
participant = Participant(
|
|
email=email,
|
|
# ...
|
|
)
|
|
```
|
|
|
|
Apply normalization at these points:
|
|
- Participant registration form processing
|
|
- Request magic link form processing
|
|
- Any email lookup query
|
|
|
|
---
|
|
|
|
### 1.3 Session Table Management
|
|
|
|
**Question**: Confirm we're using Flask-Session's built-in table, not a custom Session model.
|
|
|
|
**Answer**: **Correct - use Flask-Session's managed table**
|
|
|
|
**Rationale**:
|
|
- Current implementation already uses Flask-Session with SQLAlchemy backend: `/home/phil/Projects/sneaky-klaus/src/app.py` lines 54-56
|
|
- Flask-Session automatically creates and manages the `sessions` table
|
|
- No custom Session model needed
|
|
- v0.2.0 data model document explicitly states: "Session storage is managed by Flask-Session, which creates and manages its own session table."
|
|
|
|
**No action needed** - existing implementation is correct.
|
|
|
|
---
|
|
|
|
## 2. Authentication & Session Decisions
|
|
|
|
### 2.1 Session Scoping Implementation
|
|
|
|
**Question**: When clicking magic link for different exchange, should we always replace, show warning, or support concurrent sessions?
|
|
|
|
**Answer**: **Option A - Always replace (as designed)**
|
|
|
|
**Rationale**:
|
|
- ADR-0003 (Participant Session Scoping) explicitly documents this behavior
|
|
- Simplicity: no complex multi-session management
|
|
- Security: clear isolation between exchanges
|
|
- Existing admin session pattern: `/home/phil/Projects/sneaky-klaus/src/routes/admin.py` line 59 `session.clear()` before creating new session
|
|
|
|
**Participant session key naming**: Use `user_id` and `user_type` to match existing pattern from ADR-0002.
|
|
|
|
**Implementation**:
|
|
```python
|
|
def create_participant_session(participant_id: int, exchange_id: int):
|
|
"""Create Flask session for participant."""
|
|
session.clear() # Always replace existing session
|
|
session['user_id'] = participant_id
|
|
session['user_type'] = 'participant'
|
|
session['exchange_id'] = exchange_id
|
|
session.permanent = True
|
|
```
|
|
|
|
**Do NOT use different key names** - admin uses `admin_id` for historical reasons (legacy pattern), but participants should use the standardized `user_id` + `user_type` pattern from ADR-0002.
|
|
|
|
---
|
|
|
|
### 2.2 Session Duration and Sliding Window
|
|
|
|
**Question**: Does Flask-Session automatically implement sliding window? Should admin and participant have different durations?
|
|
|
|
**Answer**: **Flask-Session implements sliding window automatically; use same 7-day duration for both**
|
|
|
|
**Rationale**:
|
|
- Configuration already sets `SESSION_REFRESH_EACH_REQUEST = True` in `/home/phil/Projects/sneaky-klaus/src/config.py` (implied by defaults)
|
|
- Flask-Session extends session on each request when `session.permanent = True`
|
|
- ADR-0002 specifies 7-day sliding window for both admin and participants
|
|
- Existing admin login sets variable duration (7 days default, 30 days with remember_me) - but Phase 2 design specifies 7 days for participants
|
|
|
|
**Implementation**:
|
|
- Admin sessions: Keep existing pattern (7 days default, 30 days with remember_me)
|
|
- Participant sessions: 7 days, no remember_me option
|
|
|
|
```python
|
|
# In config.py - already correct
|
|
PERMANENT_SESSION_LIFETIME = timedelta(days=7)
|
|
|
|
# Participant session creation
|
|
session.permanent = True # Enables sliding window
|
|
# No need to override PERMANENT_SESSION_LIFETIME for participants
|
|
```
|
|
|
|
---
|
|
|
|
## 3. Email Integration Decisions
|
|
|
|
### 3.1 Resend SDK Integration
|
|
|
|
**Question**: Should we create EmailService class? Where to store templates? Use Jinja2?
|
|
|
|
**Answer**: **Create `src/services/email.py` with EmailService class; store templates in `templates/emails/`; yes, use Jinja2**
|
|
|
|
**Rationale**:
|
|
- Service layer pattern separates concerns
|
|
- Existing project structure has `/home/phil/Projects/sneaky-klaus/src/utils/` for utilities, but email is a service (external integration)
|
|
- Jinja2 already used for web templates, reuse for email
|
|
- Phase 2 design specifies template paths: `templates/emails/participant/registration_confirmation.html`
|
|
|
|
**Directory Structure**:
|
|
```
|
|
src/
|
|
services/
|
|
__init__.py
|
|
email.py # EmailService class
|
|
templates/
|
|
emails/
|
|
participant/
|
|
registration_confirmation.html
|
|
registration_confirmation.txt # Plain text fallback
|
|
magic_link.html
|
|
magic_link.txt
|
|
admin/
|
|
password_reset.html
|
|
password_reset.txt
|
|
```
|
|
|
|
**Implementation**:
|
|
```python
|
|
# src/services/email.py
|
|
from flask import render_template
|
|
import resend
|
|
|
|
class EmailService:
|
|
def __init__(self, api_key: str):
|
|
resend.api_key = api_key
|
|
|
|
def send_registration_confirmation(self, participant_id: int, token: str):
|
|
# Load participant and exchange
|
|
# Render template
|
|
html = render_template('emails/participant/registration_confirmation.html', **variables)
|
|
text = render_template('emails/participant/registration_confirmation.txt', **variables)
|
|
# Send via Resend
|
|
```
|
|
|
|
---
|
|
|
|
### 3.2 Development Mode Email Logging
|
|
|
|
**Question**: Should dev mode check app.config['DEBUG'], FLASK_ENV, or both? Mock Resend API or attempt to send?
|
|
|
|
**Answer**: **Check FLASK_ENV only (not DEBUG); attempt to send, with logging as fallback**
|
|
|
|
**Rationale**:
|
|
- ADR-0004 (Development Mode Email Logging) explicitly specifies checking `FLASK_ENV`
|
|
- DEBUG can be True in testing environments where we don't want dev mode logging
|
|
- Attempting to send email (when configured) tests the real integration path
|
|
- Logging provides fallback when Resend not configured
|
|
|
|
**Implementation**:
|
|
```python
|
|
import os
|
|
|
|
def should_log_dev_mode_links() -> bool:
|
|
"""Check if development mode email logging is enabled.
|
|
|
|
CRITICAL: This must NEVER be True in production.
|
|
"""
|
|
env = os.environ.get('FLASK_ENV', '').lower()
|
|
return env == 'development'
|
|
|
|
def send_registration_confirmation(participant_id: int, token: str):
|
|
# Build magic link URL
|
|
magic_link_url = url_for('...')
|
|
|
|
# Development mode: log the link
|
|
if should_log_dev_mode_links():
|
|
logger.info(f"DEV MODE: Magic link generated for participant {participant.email}")
|
|
logger.info(f"DEV MODE: Full magic link URL: {magic_link_url}")
|
|
|
|
# Attempt to send email (even in dev mode, if configured)
|
|
try:
|
|
resend.Emails.send({...})
|
|
except Exception as e:
|
|
if should_log_dev_mode_links():
|
|
logger.info("Email service unavailable, but link logged for DEV MODE testing")
|
|
else:
|
|
raise # In production, we need to know if email failed
|
|
```
|
|
|
|
---
|
|
|
|
## 4. Magic Token Decisions
|
|
|
|
### 4.1 Token Cleanup Job
|
|
|
|
**Question**: Should token cleanup be implemented in Phase 2 with APScheduler, or use simpler on-demand cleanup?
|
|
|
|
**Answer**: **Use on-demand cleanup for Phase 2; defer APScheduler to Phase 5**
|
|
|
|
**Rationale**:
|
|
- Simplicity first (core principle)
|
|
- Self-hosted deployment avoids unnecessary background processes
|
|
- Phase 2 design document doesn't require background jobs
|
|
- v0.1.0 data model mentions hourly cleanup, but Phase 5 (Background Jobs) is the proper place
|
|
- On-demand cleanup is sufficient for token table (tokens auto-expire, cleanup is optimization not requirement)
|
|
|
|
**Implementation**:
|
|
```python
|
|
# Call during token validation
|
|
def validate_magic_token(token: str) -> TokenValidationResult:
|
|
# Before validation, opportunistically clean up expired tokens
|
|
cleanup_expired_tokens()
|
|
|
|
# Then validate token
|
|
...
|
|
|
|
def cleanup_expired_tokens():
|
|
"""Remove expired tokens (called opportunistically)."""
|
|
MagicToken.query.filter(
|
|
MagicToken.expires_at < datetime.utcnow()
|
|
).delete()
|
|
db.session.commit()
|
|
```
|
|
|
|
**Defer to Phase 5**: Proper scheduled cleanup with APScheduler or similar.
|
|
|
|
---
|
|
|
|
### 4.2 Token Hash Algorithm
|
|
|
|
**Question**: Use hashlib.sha256() directly, or different library?
|
|
|
|
**Answer**: **Use hashlib.sha256() directly**
|
|
|
|
**Rationale**:
|
|
- Standard library, no external dependencies
|
|
- Adequate security for token hashing (not password hashing)
|
|
- Phase 2 design explicitly specifies SHA-256: line 318 "SHA-256 hash of token"
|
|
- Existing project uses bcrypt for passwords (different use case)
|
|
|
|
**Implementation**:
|
|
```python
|
|
import hashlib
|
|
import secrets
|
|
|
|
def generate_magic_token(participant_id: int, exchange_id: int) -> str:
|
|
# Generate token
|
|
token = secrets.token_urlsafe(32) # 32 bytes
|
|
|
|
# Hash for storage
|
|
token_hash = hashlib.sha256(token.encode()).hexdigest()
|
|
|
|
# Store hash, return original
|
|
magic_token = MagicToken(token_hash=token_hash, ...)
|
|
db.session.add(magic_token)
|
|
|
|
return token # Send in email
|
|
```
|
|
|
|
---
|
|
|
|
## 5. Form Validation Decisions
|
|
|
|
### 5.1 Participant Registration Form
|
|
|
|
**Question**: Should reminder_enabled be BooleanField with default=True? How to handle checkbox initial state?
|
|
|
|
**Answer**: **Use BooleanField with default=True**
|
|
|
|
**Rationale**:
|
|
- WTForms BooleanField is designed for checkboxes
|
|
- Default True matches data model: "BOOLEAN NOT NULL, DEFAULT TRUE"
|
|
- Opt-out pattern (checked by default) is better UX for beneficial features
|
|
|
|
**Implementation**:
|
|
```python
|
|
# src/forms/participant.py
|
|
from wtforms import BooleanField, StringField, TextAreaField
|
|
from wtforms.validators import DataRequired, Email, Length
|
|
|
|
class ParticipantRegistrationForm(FlaskForm):
|
|
name = StringField('Name', validators=[DataRequired(), Length(max=255)])
|
|
email = StringField('Email', validators=[DataRequired(), Email(), Length(max=255)])
|
|
gift_ideas = TextAreaField('Gift Ideas', validators=[Length(max=10000)])
|
|
reminder_enabled = BooleanField('Send me reminders', default=True)
|
|
```
|
|
|
|
**Template**:
|
|
```html
|
|
<input type="checkbox" name="reminder_enabled" id="reminder_enabled"
|
|
{% if form.reminder_enabled.data %}checked{% endif %}>
|
|
```
|
|
|
|
---
|
|
|
|
### 5.2 Gift Ideas Field Validation
|
|
|
|
**Question**: Max 10,000 chars - implement client-side counter, server-side truncation, or just server-side validation with error?
|
|
|
|
**Answer**: **Server-side validation with error + optional client-side counter (progressive enhancement)**
|
|
|
|
**Rationale**:
|
|
- Security: Never trust client-side validation
|
|
- UX: Client-side counter is helpful but optional
|
|
- Existing form pattern: server-side validation with flash messages (see `/home/phil/Projects/sneaky-klaus/src/routes/admin.py`)
|
|
- No truncation: explicit error better than silent data loss
|
|
|
|
**Implementation**:
|
|
```python
|
|
# Server-side (required)
|
|
class ParticipantRegistrationForm(FlaskForm):
|
|
gift_ideas = TextAreaField('Gift Ideas', validators=[Length(max=10000)])
|
|
|
|
# Client-side (optional, progressive enhancement)
|
|
# In template:
|
|
<textarea name="gift_ideas" maxlength="10000"></textarea>
|
|
<small id="char-count">0 / 10,000 characters</small>
|
|
|
|
<script>
|
|
// Optional character counter
|
|
document.querySelector('textarea[name="gift_ideas"]').addEventListener('input', (e) => {
|
|
document.getElementById('char-count').textContent =
|
|
`${e.target.value.length} / 10,000 characters`;
|
|
});
|
|
</script>
|
|
```
|
|
|
|
---
|
|
|
|
## 6. Route Organization Decisions
|
|
|
|
### 6.1 Blueprint Structure
|
|
|
|
**Question**: Current admin login at /admin/login, design shows /auth/admin/login. Which structure?
|
|
|
|
**Answer**: **Option B - Keep admin auth in admin_bp, create participant routes in participant_bp**
|
|
|
|
**Rationale**:
|
|
- Existing admin routes already in admin_bp with `/admin` prefix
|
|
- Breaking change to move admin auth would require updating tests, templates, and documentation
|
|
- Participant routes already have participant_bp: `/home/phil/Projects/sneaky-klaus/src/routes/participant.py`
|
|
- URL structure less important than consistency and minimal changes
|
|
|
|
**Blueprint Organization**:
|
|
- `admin_bp` (/admin/*): Admin login, logout, dashboard, exchanges
|
|
- `participant_bp` (no prefix): Participant registration, magic link auth, dashboard
|
|
- `setup_bp` (/setup): Initial setup (existing)
|
|
|
|
**Participant Routes**:
|
|
```python
|
|
# src/routes/participant.py
|
|
participant_bp = Blueprint('participant', __name__, url_prefix='')
|
|
|
|
@participant_bp.route('/exchange/<slug>/register', methods=['GET', 'POST'])
|
|
def register(slug):
|
|
"""Participant registration"""
|
|
|
|
@participant_bp.route('/exchange/<slug>/request-access', methods=['POST'])
|
|
def request_access(slug):
|
|
"""Request magic link for existing participant"""
|
|
|
|
@participant_bp.route('/auth/magic/<token>')
|
|
def magic_link_auth(token):
|
|
"""Validate magic link and create session"""
|
|
|
|
@participant_bp.route('/participant/dashboard')
|
|
@participant_required
|
|
def dashboard():
|
|
"""Participant dashboard"""
|
|
```
|
|
|
|
**Do NOT create separate auth_bp** - would fragment routes unnecessarily.
|
|
|
|
---
|
|
|
|
### 6.2 Public Routes
|
|
|
|
**Question**: Should we create public_bp, add to participant_bp, or register directly on app?
|
|
|
|
**Answer**: **Add to participant_bp (which has no prefix)**
|
|
|
|
**Rationale**:
|
|
- Public routes (landing page, registration) are participant-facing
|
|
- participant_bp already has no prefix (empty string)
|
|
- No need for additional blueprint
|
|
- Simpler organization
|
|
|
|
**Implementation**:
|
|
```python
|
|
# src/routes/participant.py
|
|
participant_bp = Blueprint('participant', __name__, url_prefix='')
|
|
|
|
# Public routes (no auth required)
|
|
@participant_bp.route('/')
|
|
def landing():
|
|
"""Public landing page (optional)"""
|
|
|
|
@participant_bp.route('/exchange/<slug>/register', methods=['GET', 'POST'])
|
|
def register(slug):
|
|
"""Public registration page (no auth required)"""
|
|
|
|
# Protected routes (auth required)
|
|
@participant_bp.route('/participant/dashboard')
|
|
@participant_required
|
|
def dashboard():
|
|
"""Protected dashboard"""
|
|
```
|
|
|
|
---
|
|
|
|
## 7. Error Handling Decisions
|
|
|
|
### 7.1 Rate Limit Exceptions
|
|
|
|
**Question**: Define in new src/exceptions.py? Inherit from werkzeug or custom? Flash+render or exception-based?
|
|
|
|
**Answer**: **Create src/exceptions.py; inherit from werkzeug.exceptions.HTTPException; use exception-based handling**
|
|
|
|
**Rationale**:
|
|
- Werkzeug exceptions integrate with Flask error handlers
|
|
- Existing codebase uses exception-based handling (see 404, 403, 500 handlers in `/home/phil/Projects/sneaky-klaus/src/app.py`)
|
|
- Centralized exception definitions improve maintainability
|
|
- Allows custom error handlers per exception type
|
|
|
|
**Implementation**:
|
|
```python
|
|
# src/exceptions.py
|
|
from werkzeug.exceptions import HTTPException
|
|
|
|
class RateLimitExceeded(HTTPException):
|
|
"""Raised when rate limit is exceeded."""
|
|
code = 429
|
|
description = "Too many requests. Please try again later."
|
|
|
|
class EmailAlreadyRegistered(HTTPException):
|
|
"""Raised when participant email already exists in exchange."""
|
|
code = 400
|
|
description = "This email is already registered for this exchange."
|
|
|
|
class RegistrationClosed(HTTPException):
|
|
"""Raised when registration is not open."""
|
|
code = 400
|
|
description = "Registration is not currently open for this exchange."
|
|
|
|
# src/app.py - error handlers
|
|
@app.errorhandler(RateLimitExceeded)
|
|
def handle_rate_limit(error):
|
|
flash(error.description, "error")
|
|
return redirect(request.referrer or '/'), 429
|
|
|
|
@app.errorhandler(EmailAlreadyRegistered)
|
|
def handle_email_exists(error):
|
|
flash(error.description, "error")
|
|
return render_template('participant/register.html', show_request_access=True), 400
|
|
```
|
|
|
|
---
|
|
|
|
## 8. Testing Infrastructure Decisions
|
|
|
|
### 8.1 Email Testing
|
|
|
|
**Question**: Use mock email backend, TestEmailService, or configure test email backend in TestConfig?
|
|
|
|
**Answer**: **Use unittest.mock to patch EmailService in tests**
|
|
|
|
**Rationale**:
|
|
- Existing test pattern: `/home/phil/Projects/sneaky-klaus/tests/conftest.py` uses fixtures
|
|
- Python unittest.mock is standard library
|
|
- Allows inspection of email calls (recipients, content, etc.)
|
|
- No test-specific email backend needed
|
|
|
|
**Implementation**:
|
|
```python
|
|
# tests/conftest.py
|
|
from unittest.mock import Mock, patch
|
|
|
|
@pytest.fixture
|
|
def mock_email_service():
|
|
"""Mock email service for testing."""
|
|
with patch('src.services.email.EmailService') as mock:
|
|
mock_instance = Mock()
|
|
mock.return_value = mock_instance
|
|
yield mock_instance
|
|
|
|
# tests/integration/test_participant_registration.py
|
|
def test_registration_sends_email(client, db, mock_email_service):
|
|
"""Test that registration sends confirmation email."""
|
|
response = client.post('/exchange/test-slug/register', data={
|
|
'name': 'Alice',
|
|
'email': 'alice@example.com',
|
|
'gift_ideas': 'Books',
|
|
'csrf_token': get_csrf_token()
|
|
})
|
|
|
|
# Verify email was sent
|
|
mock_email_service.send_registration_confirmation.assert_called_once()
|
|
call_args = mock_email_service.send_registration_confirmation.call_args
|
|
assert call_args[0][0] > 0 # participant_id
|
|
assert len(call_args[0][1]) == 43 # token length
|
|
```
|
|
|
|
---
|
|
|
|
### 8.2 Magic Link Testing
|
|
|
|
**Question**: Parse from mock email content, direct API to generate tokens, or both?
|
|
|
|
**Answer**: **Use direct API to generate tokens in tests; optionally verify email content in integration tests**
|
|
|
|
**Rationale**:
|
|
- Unit tests should test token generation/validation directly
|
|
- Integration tests can verify full flow (including email)
|
|
- Parsing email content is brittle and slow
|
|
- Direct API access is faster and more reliable
|
|
|
|
**Implementation**:
|
|
```python
|
|
# Unit test - direct token generation
|
|
def test_magic_token_validation(db):
|
|
"""Test token validation."""
|
|
participant = create_test_participant()
|
|
|
|
# Generate token directly
|
|
from src.services.token import generate_magic_token
|
|
token = generate_magic_token(participant.id, participant.exchange_id)
|
|
|
|
# Validate token
|
|
result = validate_magic_token(token)
|
|
assert result.valid is True
|
|
|
|
# Integration test - full flow with email
|
|
def test_registration_flow_with_magic_link(client, db, mock_email_service):
|
|
"""Test complete registration flow."""
|
|
# Register participant
|
|
response = client.post('/exchange/test-slug/register', data={...})
|
|
|
|
# Extract token from mock call (not from email content)
|
|
token = mock_email_service.send_registration_confirmation.call_args[0][1]
|
|
|
|
# Use magic link
|
|
response = client.get(f'/auth/magic/{token}')
|
|
assert response.status_code == 302
|
|
```
|
|
|
|
---
|
|
|
|
## 9. Migration Decisions
|
|
|
|
### 9.1 Alembic Migrations
|
|
|
|
**Question**: Create Alembic migrations for Phase 2 models, or continue with db.create_all()?
|
|
|
|
**Answer**: **Use Alembic migrations for Phase 2 and all future schema changes**
|
|
|
|
**Rationale**:
|
|
- Alembic infrastructure already exists in the codebase (alembic.ini, migrations/ directory)
|
|
- Initial migration already created for Admin and Exchange models
|
|
- Production-ready approach from the start avoids migration debt
|
|
- Enables safe, versioned schema evolution
|
|
- Supports rollback if needed
|
|
- Critical for any deployment with persistent data
|
|
|
|
**Implementation Approach**:
|
|
|
|
**For new Phase 2 models** (Participant, MagicToken):
|
|
```bash
|
|
# Generate migration for new models
|
|
uv run alembic revision --autogenerate -m "Add Participant and MagicToken models"
|
|
|
|
# Review the generated migration file
|
|
# Edit if needed to ensure correctness
|
|
|
|
# Apply migration
|
|
uv run alembic upgrade head
|
|
```
|
|
|
|
**Migration Naming Conventions**:
|
|
- Use descriptive, imperative messages: "Add Participant model", "Add email index to Participant"
|
|
- Keep messages under 80 characters
|
|
- Use lowercase except for model/table names
|
|
|
|
**Testing Migrations**:
|
|
1. Test upgrade path: `alembic upgrade head`
|
|
2. Test downgrade path: `alembic downgrade -1` then `alembic upgrade head`
|
|
3. Verify database schema matches models
|
|
4. Run application tests after migration
|
|
|
|
**Handling Existing Databases**:
|
|
If a database was created with db.create_all() before migrations:
|
|
```bash
|
|
# Option 1: Stamp existing database with current migration
|
|
uv run alembic stamp head
|
|
|
|
# Option 2: Recreate database from migrations (dev only)
|
|
rm data/sneaky-klaus.db
|
|
uv run alembic upgrade head
|
|
```
|
|
|
|
**Migration Workflow**:
|
|
1. Create/modify SQLAlchemy models
|
|
2. Generate migration: `alembic revision --autogenerate -m "description"`
|
|
3. Review and test migration file
|
|
4. Apply migration: `alembic upgrade head`
|
|
5. Commit migration file with model changes
|
|
|
|
**Never use db.create_all() in production** - it does not handle schema changes safely.
|
|
|
|
### 9.2 Automatic Migrations for Deployments
|
|
|
|
**Question**: When self-hosted users pull a new container image with schema changes, how are migrations applied?
|
|
|
|
**Answer**: **Automatic migrations via container entrypoint script**
|
|
|
|
**Rationale**:
|
|
- Self-hosted users should not need to manually run migration commands
|
|
- Container startup is the natural point to ensure schema is up-to-date
|
|
- Prevents application from starting with incompatible schema
|
|
- Provides clear error visibility if migrations fail
|
|
|
|
**Implementation**:
|
|
|
|
Create `entrypoint.sh` in the project root:
|
|
```bash
|
|
#!/bin/bash
|
|
set -e # Exit on any error
|
|
|
|
echo "Running database migrations..."
|
|
if uv run alembic upgrade head; then
|
|
echo "Database migrations completed successfully"
|
|
else
|
|
echo "ERROR: Database migration failed!"
|
|
echo "Please check the logs above for details."
|
|
exit 1
|
|
fi
|
|
|
|
echo "Starting application server..."
|
|
exec gunicorn --bind 0.0.0.0:8000 --workers 2 --threads 4 main:app
|
|
```
|
|
|
|
Update `Containerfile`:
|
|
```dockerfile
|
|
# ... existing content ...
|
|
|
|
# Copy entrypoint script
|
|
COPY entrypoint.sh /app/entrypoint.sh
|
|
RUN chmod +x /app/entrypoint.sh
|
|
|
|
# ... existing content ...
|
|
|
|
# Change CMD to use entrypoint
|
|
CMD ["/app/entrypoint.sh"]
|
|
```
|
|
|
|
**Behavior**:
|
|
- **First run**: Creates database and runs all migrations
|
|
- **Updates**: Applies only new migrations incrementally
|
|
- **Failures**: Container exits with error, preventing app startup with wrong schema
|
|
- **Logging**: Migration output visible in container logs
|
|
|
|
**Remove db.create_all() from Application**:
|
|
|
|
The `db.create_all()` call in `src/app.py` (line 77) must be removed because:
|
|
- It conflicts with migration-based schema management
|
|
- Cannot handle schema changes (only creates missing tables)
|
|
- ADR-0005 explicitly requires migrations for production
|
|
|
|
Update `src/app.py`:
|
|
```python
|
|
# Import models to ensure they're registered with SQLAlchemy
|
|
with app.app_context():
|
|
from src.models import Admin, Exchange, RateLimit # noqa: F401
|
|
# REMOVED: db.create_all() - schema managed by Alembic migrations
|
|
```
|
|
|
|
**Development Workflow**:
|
|
Developers continue to run migrations manually for explicit control:
|
|
```bash
|
|
uv run alembic upgrade head
|
|
```
|
|
|
|
See ADR-0005 (Database Migrations) section "Automatic Migrations for Self-Hosted Deployments" for full rationale and alternative approaches considered.
|
|
|
|
---
|
|
|
|
## 10. Implementation Order Decisions
|
|
|
|
### 10.1 Story Implementation Sequence
|
|
|
|
**Question**: Bottom-up (models → services → routes), story-by-story, or vertical slice?
|
|
|
|
**Answer**: **Vertical slice (one complete story at a time)**
|
|
|
|
**Rationale**:
|
|
- TDD-friendly: write test for story, implement minimum code to pass
|
|
- Delivers working features incrementally
|
|
- Easier to review and merge
|
|
- Matches project workflow (feature branches)
|
|
- Each story is independently testable
|
|
|
|
**Recommended Order**:
|
|
1. Story 3.1: Participant Registration (includes models, forms, routes, email service)
|
|
2. Story 3.2: Participant Authentication (magic links)
|
|
3. Story 3.3: Participant Dashboard
|
|
4. Story 3.4: Update Gift Ideas
|
|
5. Story 3.5: Withdraw from Exchange
|
|
|
|
**Within each story**:
|
|
1. Write integration test (story acceptance criteria)
|
|
2. Create models (if needed)
|
|
3. Create forms (if needed)
|
|
4. Create services (if needed)
|
|
5. Create routes
|
|
6. Create templates
|
|
7. Run tests, iterate until passing
|
|
|
|
---
|
|
|
|
### 10.2 Decorator Implementation
|
|
|
|
**Question**: Add to existing auth.py or create separate participant_decorators.py?
|
|
|
|
**Answer**: **Add to existing src/decorators/auth.py**
|
|
|
|
**Rationale**:
|
|
- Single source of truth for authentication decorators
|
|
- admin_required already exists there: `/home/phil/Projects/sneaky-klaus/src/decorators/auth.py`
|
|
- Decorators are small, related functionality
|
|
- No need to split until file becomes unwieldy (>300 lines)
|
|
|
|
**Implementation**:
|
|
```python
|
|
# src/decorators/auth.py
|
|
|
|
def admin_required(f):
|
|
"""Existing admin decorator"""
|
|
...
|
|
|
|
def participant_required(f):
|
|
"""Decorator to require participant authentication."""
|
|
@wraps(f)
|
|
def decorated_function(*args, **kwargs):
|
|
if 'user_id' not in session or session.get('user_type') != 'participant':
|
|
flash("You must be logged in to access this page.", "error")
|
|
return redirect(url_for('participant.landing'))
|
|
|
|
# Load participant
|
|
from src.models import Participant
|
|
participant = Participant.query.get(session['user_id'])
|
|
if not participant or participant.withdrawn_at is not None:
|
|
session.clear()
|
|
flash("Your session is invalid.", "error")
|
|
return redirect(url_for('participant.landing'))
|
|
|
|
g.participant = participant
|
|
g.exchange_id = session.get('exchange_id')
|
|
return f(*args, **kwargs)
|
|
return decorated_function
|
|
|
|
def exchange_access_required(f):
|
|
"""Decorator to validate participant access to specific exchange.
|
|
Must be used after @participant_required.
|
|
"""
|
|
@wraps(f)
|
|
def decorated_function(*args, **kwargs):
|
|
exchange_id = kwargs.get('exchange_id')
|
|
if not exchange_id:
|
|
abort(400)
|
|
|
|
if g.participant.exchange_id != exchange_id:
|
|
flash("You don't have access to this exchange.", "error")
|
|
return redirect(url_for('participant.dashboard'))
|
|
|
|
return f(*args, **kwargs)
|
|
return decorated_function
|
|
```
|
|
|
|
---
|
|
|
|
## 11. Development Mode Decisions
|
|
|
|
### 11.1 Dev Mode Testing
|
|
|
|
**Question**: Provide dev-only endpoint /dev/recent-links, or strictly logs-only? Show dev mode indicator in UI?
|
|
|
|
**Answer**: **Logs-only for Phase 2; no dev endpoints or UI indicators**
|
|
|
|
**Rationale**:
|
|
- ADR-0004 specifies logs-only approach
|
|
- Dev endpoints add complexity and security risk (even in dev mode)
|
|
- Logs are sufficient for QA workflow: `podman logs sneaky-klaus | grep "DEV MODE"`
|
|
- UI indicators add no value (developers know what environment they're in)
|
|
|
|
**Implementation**: Follow ADR-0004 exactly - log magic links to application logs when `FLASK_ENV=development`.
|
|
|
|
**Future enhancement** (if QA requests): Could add `/dev/magic-links` endpoint in Phase 8+, but require it to be explicitly enabled with additional env var (defense in depth).
|
|
|
|
---
|
|
|
|
## Summary: Questions Requiring User Input
|
|
|
|
The following questions genuinely require user preference and should be escalated:
|
|
|
|
**NONE** - All questions have clear architectural answers based on existing patterns, best practices, or design decisions.
|
|
|
|
---
|
|
|
|
## Implementation Checklist for Developer
|
|
|
|
Based on these decisions, the developer should:
|
|
|
|
1. **Database Models & Migrations**:
|
|
- [ ] Create Participant model with email normalization in setter
|
|
- [ ] Create MagicToken model with application-level validation in validate() method
|
|
- [ ] Generate Alembic migration for new models: `alembic revision --autogenerate -m "Add Participant and MagicToken models"`
|
|
- [ ] Review and test the generated migration
|
|
- [ ] Apply migration: `alembic upgrade head`
|
|
- [ ] Verify Flask-Session is properly configured (already done)
|
|
- [ ] Remove `db.create_all()` from `src/app.py` (replaced by migrations)
|
|
|
|
2. **Services**:
|
|
- [ ] Create src/services/email.py with EmailService class
|
|
- [ ] Create src/services/token.py with generate_magic_token() and validate_magic_token()
|
|
- [ ] Implement on-demand token cleanup in validate function
|
|
|
|
3. **Forms**:
|
|
- [ ] Create src/forms/participant.py with registration and request access forms
|
|
- [ ] Use BooleanField for reminder_enabled with default=True
|
|
- [ ] Use Length(max=10000) validator for gift_ideas
|
|
|
|
4. **Exceptions**:
|
|
- [ ] Create src/exceptions.py with RateLimitExceeded, EmailAlreadyRegistered, RegistrationClosed
|
|
- [ ] Register error handlers in src/app.py
|
|
|
|
5. **Routes**:
|
|
- [ ] Implement participant registration in participant_bp
|
|
- [ ] Implement magic link authentication in participant_bp
|
|
- [ ] Implement participant dashboard in participant_bp
|
|
- [ ] Keep admin auth in admin_bp (no changes)
|
|
|
|
6. **Decorators**:
|
|
- [ ] Add participant_required to src/decorators/auth.py
|
|
- [ ] Add exchange_access_required to src/decorators/auth.py
|
|
|
|
7. **Templates**:
|
|
- [ ] Create templates/emails/participant/ directory with email templates
|
|
- [ ] Create templates/participant/ directory with web templates
|
|
|
|
8. **Session Management**:
|
|
- [ ] Implement create_participant_session() using session.clear() pattern
|
|
- [ ] Use user_id + user_type + exchange_id session structure
|
|
- [ ] Set session.permanent = True (7-day sliding window)
|
|
|
|
9. **Development Mode**:
|
|
- [ ] Implement should_log_dev_mode_links() checking FLASK_ENV
|
|
- [ ] Log magic links in EmailService when dev mode enabled
|
|
|
|
10. **Testing**:
|
|
- [ ] Use unittest.mock to patch EmailService in tests
|
|
- [ ] Write vertical slice tests (one complete story at a time)
|
|
- [ ] Test token generation/validation directly (not via email parsing)
|
|
|
|
11. **Container & Deployment**:
|
|
- [ ] Create `entrypoint.sh` script to run migrations before starting app
|
|
- [ ] Make entrypoint script executable in Containerfile
|
|
- [ ] Update Containerfile CMD to use entrypoint script
|
|
- [ ] Test container startup with fresh database (first-run scenario)
|
|
- [ ] Test container update with existing database (migration scenario)
|
|
|
|
---
|
|
|
|
## References
|
|
|
|
- [ADR-0002: Authentication Strategy](/home/phil/Projects/sneaky-klaus/docs/decisions/0002-authentication-strategy.md)
|
|
- [ADR-0003: Participant Session Scoping](/home/phil/Projects/sneaky-klaus/docs/decisions/0003-participant-session-scoping.md)
|
|
- [ADR-0004: Development Mode Email Logging](/home/phil/Projects/sneaky-klaus/docs/decisions/0004-dev-mode-email-logging.md)
|
|
- [Phase 2 Design: Participant Authentication](/home/phil/Projects/sneaky-klaus/docs/designs/v0.2.0/components/participant-auth.md)
|
|
- [Data Model v0.2.0](/home/phil/Projects/sneaky-klaus/docs/designs/v0.2.0/data-model.md)
|
|
- [Project Overview](/home/phil/Projects/sneaky-klaus/docs/PROJECT_OVERVIEW.md)
|