diff --git a/.env.example b/.env.example index 35030cc..4433c81 100644 --- a/.env.example +++ b/.env.example @@ -1,9 +1,15 @@ # Parking Manager Configuration -# Security - REQUIRED: Change in production! +# ============================================================================= +# REQUIRED - Security +# ============================================================================= +# MUST be set to a random string of at least 32 characters +# Generate with: openssl rand -hex 32 SECRET_KEY=change-me-to-a-random-string-at-least-32-chars +# ============================================================================= # Server +# ============================================================================= HOST=0.0.0.0 PORT=8000 @@ -13,14 +19,54 @@ DATABASE_PATH=/app/data/parking.db # CORS (comma-separated origins, or * for all) ALLOWED_ORIGINS=https://parking.rocketscale.it +# JWT token expiration (minutes, default 24 hours) +ACCESS_TOKEN_EXPIRE_MINUTES=1440 + +# Logging level (DEBUG, INFO, WARNING, ERROR) +LOG_LEVEL=INFO + +# ============================================================================= +# Rate Limiting +# ============================================================================= +# Number of requests allowed per window for sensitive endpoints (login, register) +RATE_LIMIT_REQUESTS=5 +# Window size in seconds +RATE_LIMIT_WINDOW=60 + +# ============================================================================= # Authentication +# ============================================================================= # Set to true when behind Authelia reverse proxy AUTHELIA_ENABLED=false -# SMTP - Email Notifications (optional) -SMTP_HOST= +# Header names (only change if your proxy uses different headers) +AUTHELIA_HEADER_USER=Remote-User +AUTHELIA_HEADER_NAME=Remote-Name +AUTHELIA_HEADER_EMAIL=Remote-Email +AUTHELIA_HEADER_GROUPS=Remote-Groups + +# LLDAP group that maps to admin role +AUTHELIA_ADMIN_GROUP=parking_admins + +# External URLs for Authelia mode (used for landing page buttons) +# Login URL - Authelia's login page (users are redirected here to authenticate) +AUTHELIA_LOGIN_URL=https://auth.rocketscale.it +# Registration URL - External registration portal (org-stack self-registration) +REGISTRATION_URL=https://register.rocketscale.it + +# ============================================================================= +# Email Notifications +# ============================================================================= +# Set to true to enable email sending +SMTP_ENABLED=false + +# SMTP server configuration +SMTP_HOST=localhost SMTP_PORT=587 SMTP_USER= SMTP_PASSWORD= -SMTP_FROM=noreply@rocketscale.it +SMTP_FROM=noreply@parking.local SMTP_USE_TLS=true + +# When SMTP is disabled, emails are logged to this file +EMAIL_LOG_FILE=/tmp/parking-emails.log diff --git a/CLAUDE.md b/CLAUDE.md index e838e0a..65d9a80 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,21 +9,26 @@ - **Frontend:** Vanilla JavaScript (no frameworks) - **Auth:** JWT tokens + Authelia SSO support - **Containerization:** Docker + Docker Compose +- **Rate Limiting:** slowapi ### Architecture ``` -app/routes/ → API endpoints (auth, users, managers, presence, parking) -services/ → Business logic (parking algorithm, auth, notifications) -database/ → SQLAlchemy models and connection -frontend/ → Static HTML pages + JS modules -utils/ → Auth middleware +app/ +├── config.py → Configuration with logging and validation +└── routes/ → API endpoints (auth, users, managers, presence, parking) +services/ → Business logic (parking algorithm, auth, notifications) +database/ → SQLAlchemy models and connection +frontend/ → Static HTML pages + JS modules +utils/ +├── auth_middleware.py → JWT/Authelia authentication +└── helpers.py → Shared utility functions ``` ## Build & Run Commands ```bash # Development -python -m uvicorn main:app --reload --host 0.0.0.0 --port 8000 +SECRET_KEY=dev-secret-key python -m uvicorn main:app --reload --host 0.0.0.0 --port 8000 # Docker docker compose up -d @@ -41,7 +46,8 @@ python create_test_db.py - FastAPI async patterns with `Depends()` for dependency injection - Pydantic models for request/response validation - SQLAlchemy ORM (no raw SQL) -- UUIDs as string primary keys: `str(uuid.uuid4())` +- Use `generate_uuid()` from `utils.helpers` for UUIDs +- Use `config.logger` for logging (not print statements) - Dates stored as TEXT in "YYYY-MM-DD" format ### JavaScript @@ -53,152 +59,102 @@ python create_test_db.py ### Authentication - Dual mode: JWT tokens (standalone) or Authelia headers (SSO) - LDAP users have `password_hash = None` -- Check pattern: `config.AUTHELIA_ENABLED and user.password_hash is None` +- Use helper: `is_ldap_user(user)` from `utils.helpers` ---- - -## Known Issues & Technical Debt - -### Critical - -1. **Inactive Notification System** - - Location: [services/notifications.py](services/notifications.py) - - Issue: All code implemented but no scheduler integrated - - TODO: Integrate APScheduler or similar - -2. **Default SECRET_KEY** - - Location: [app/config.py:13](app/config.py#L13) - - Issue: Defaults to "change-me-in-production" - - Fix: Add startup validation to error if default key used - -3. **No CSRF Protection** - - Forms use token auth only, vulnerable to CSRF attacks - - Fix: Implement CSRF tokens or validate referer header - -4. **No Rate Limiting** - - Login endpoint has no brute force protection - - Fix: Add slowapi or similar middleware - -### Performance - -1. **N+1 Query Problems** - - Location: [app/routes/managers.py:244-259](app/routes/managers.py#L244-L259) - - Location: [app/routes/presence.py:336-419](app/routes/presence.py#L336-L419) - - Issue: Loops that query database for each item - - Fix: Use joins and relationship loading - -2. **Inefficient Spot Prefix Lookups** - - Location: [services/parking.py:56-64](services/parking.py#L56-L64) - - Issue: Repeated DB queries for same data - - Fix: Cache in request context - -### Code Quality - -1. **Duplicated LDAP Check Logic** (4+ locations) - ```python - # Appears in: users.py:91, 168, 257, 280 - is_ldap_user = config.AUTHELIA_ENABLED and user.password_hash is None - ``` - - Fix: Create `utils.is_ldap_user(user)` helper - -2. **Inline JavaScript in HTML** - - 500+ lines embedded across pages - - Affected: team-rules.html, team-calendar.html, settings.html - - Fix: Extract to separate JS files - -3. **Inconsistent Response Formats** - - Some endpoints return dicts, others Pydantic models - - Fix: Standardize on Pydantic response schemas - -4. **God Object: User Model** - - Location: [database/models.py:11-47](database/models.py#L11-L47) - - Issue: 27 columns mixing auth, profile, preferences, manager settings - - Fix: Normalize into UserProfile, UserPreferences, ManagerSettings tables - -5. **Repetitive CRUD in managers.py** - - Location: [app/routes/managers.py](app/routes/managers.py) - - Issue: 4 resources × 3 operations with near-identical code - - Fix: Create generic CRUD factory or base class - -6. **Silent Exception Handling** - - Location: [app/routes/presence.py:135-143](app/routes/presence.py#L135-L143) - - Issue: Catches all exceptions and only prints - - Fix: Log properly and propagate meaningful errors - ---- - -## Areas for Simplification - -### 1. Consolidate Response Building -The `user_to_response()` pattern is duplicated in [users.py:76-107](app/routes/users.py#L76-L107) and [users.py:254-274](app/routes/users.py#L254-L274). Create single reusable function. - -### 2. Generic CRUD Router Factory -[managers.py](app/routes/managers.py) has 12 nearly identical endpoints. Create: +### Utility Functions (`utils/helpers.py`) ```python -def create_crud_router(model: Type, schema: Type, parent_key: str): - router = APIRouter() - # Generate GET, POST, DELETE endpoints - return router +from utils.helpers import ( + generate_uuid, # Use instead of str(uuid.uuid4()) + is_ldap_user, # Check if user is LDAP-managed + is_ldap_admin, # Check if user is LDAP admin + validate_password, # Returns list of validation errors + format_password_errors, # Format errors into user message + get_notification_default # Get setting value with default +) ``` -### 3. Frontend State Management -Each page maintains its own globals (`currentUser`, `currentData`). Consider simple app-level state or Web Components. +--- -### 4. Date Handling -All dates stored as TEXT strings. Could use proper DATE columns for better query performance and validation. +## Configuration (`app/config.py`) + +Configuration is environment-based with required validation: + +### Required +- `SECRET_KEY` - **MUST** be set (app exits if missing) + +### Security +- `RATE_LIMIT_REQUESTS` - Requests per window (default: 5) +- `RATE_LIMIT_WINDOW` - Window in seconds (default: 60) + +### Email (org-stack pattern) +- `SMTP_ENABLED` - Set to `true` to enable SMTP sending +- When disabled, emails are logged to `EMAIL_LOG_FILE` +- Follows org-stack pattern: direct send with file fallback + +### Logging +- `LOG_LEVEL` - DEBUG, INFO, WARNING, ERROR (default: INFO) +- Use `config.logger` for all logging + +--- + +## Notifications (`services/notifications.py`) + +Simplified notification service following org-stack pattern: -### 5. Notification Service Activation -Create [services/scheduler.py](services/scheduler.py) with APScheduler: ```python -from apscheduler.schedulers.asyncio import AsyncIOScheduler - -scheduler = AsyncIOScheduler() -scheduler.add_job(process_notification_queue, 'interval', minutes=5) +from services.notifications import ( + send_email, # Direct send or file fallback + notify_parking_assigned, # When spot assigned + notify_parking_released, # When spot released + notify_parking_reassigned, # When spot reassigned + send_presence_reminder, # Weekly presence reminder + send_weekly_parking_summary, # Friday parking summary + send_daily_parking_reminder, # Daily parking reminder + run_scheduled_notifications # Called by cron/scheduler +) ``` +### Email Behavior +1. If `SMTP_ENABLED=true`: Send via SMTP +2. If SMTP fails or disabled: Log to `EMAIL_LOG_FILE` +3. Never throws - always returns success/failure + --- -## Security Improvements Needed +## Recent Improvements -| Priority | Issue | Location | Recommendation | -|----------|-------|----------|----------------| -| HIGH | Rate limiting | main.py | Add slowapi middleware | -| HIGH | CSRF protection | All forms | Implement CSRF tokens | -| HIGH | Secret validation | config.py:13 | Error on default key | -| MEDIUM | Password validation | auth.py:63-67 | Enforce complexity rules | -| MEDIUM | Input sanitization | notification emails | Use template library | -| LOW | CORS configuration | compose.yml | Document production settings | +### Security Enhancements +- **Required SECRET_KEY**: App exits if not set +- **Rate limiting**: Login/register endpoints limited to 5 req/min +- **Password validation**: Requires uppercase, lowercase, number, 8+ chars +- **Proper logging**: All security events logged ---- +### Performance Optimizations +- **Fixed N+1 queries** in: + - `list_users()` - Batch query for manager names and counts + - `list_managers()` - Batch query for managed user counts + - `get_manager_guarantees()` - Batch query for user names + - `get_manager_exclusions()` - Batch query for user names -## Testing Strategy (Missing) +### Code Consolidation +- **Utility functions** (`utils/helpers.py`): + - `generate_uuid()` - Replaces 50+ `str(uuid.uuid4())` calls + - `is_ldap_user()` - Replaces 4+ duplicated checks + - `validate_password()` - Consistent password validation +- **Simplified notifications** - Removed queue system, direct send -No tests currently exist. Recommended structure: -``` -tests/ -├── conftest.py # Fixtures (test DB, client, auth) -├── test_auth.py # JWT and Authelia modes -├── test_parking.py # Fair assignment algorithm -├── test_presence.py # Bulk operations, team calendar -├── test_managers.py # CRUD operations -└── integration/ - └── test_workflows.py # End-to-end scenarios -``` - -Key test scenarios: -1. Parking algorithm with varying ratios -2. Manager closing days affect assignments -3. Guarantee and exclusion rules -4. Authelia header authentication flow -5. LDAP vs local user password handling +### Logging Improvements +- Centralized logging via `config.logger` +- Replaced `print()` with proper logging +- Security events logged (login, password change, etc.) --- ## API Quick Reference ### Authentication -- `POST /api/auth/register` - Create user (standalone mode) -- `POST /api/auth/login` - Get JWT token +- `POST /api/auth/register` - Create user (rate limited) +- `POST /api/auth/login` - Get JWT token (rate limited) - `GET /api/auth/me` - Current user (JWT or Authelia) ### Presence @@ -226,25 +182,31 @@ Key test scenarios: 2. Use `APIRouter(prefix="/api/...", tags=["..."])` 3. Register in `main.py`: `app.include_router(...)` 4. Add auth dependency: `current_user: User = Depends(get_current_user)` +5. Use `config.logger` for logging +6. Use `generate_uuid()` for new records -### Database Migrations +### Database Changes No migration system (Alembic) configured. Schema changes require: 1. Update [database/models.py](database/models.py) 2. Delete SQLite file or write manual migration 3. Run `create_test_db.py` for fresh database -### Frontend Page Pattern -```html - +### Running Scheduled Notifications +Add to cron or systemd timer: +```bash +# Every 5 minutes +*/5 * * * * cd /path/to/org-parking && python -c " +from database.connection import get_db +from services.notifications import run_scheduled_notifications +db = next(get_db()) +run_scheduled_notifications(db) +" ``` --- @@ -257,7 +219,35 @@ document.addEventListener('DOMContentLoaded', async () => { | Configuration | [app/config.py](app/config.py) | | Database models | [database/models.py](database/models.py) | | Parking algorithm | [services/parking.py](services/parking.py) | +| Notifications | [services/notifications.py](services/notifications.py) | | Auth middleware | [utils/auth_middleware.py](utils/auth_middleware.py) | +| Utility helpers | [utils/helpers.py](utils/helpers.py) | | Frontend API client | [frontend/js/api.js](frontend/js/api.js) | | CSS styles | [frontend/css/styles.css](frontend/css/styles.css) | | Docker config | [compose.yml](compose.yml) | +| Environment template | [.env.example](.env.example) | + +--- + +## Deployment Notes + +### Remote Server +- Host: `rocketscale.it` +- User: `rocky` +- SSH: `ssh rocky@rocketscale.it` +- Project path: `/home/rocky/org-parking` +- Related project: `/home/rocky/org-stack` (LLDAP, Authelia, etc.) + +### Environment Variables +Copy `.env.example` to `.env` and configure: +```bash +# Generate secure key +openssl rand -hex 32 +``` + +### Production Checklist +- [ ] Set strong `SECRET_KEY` +- [ ] Configure `ALLOWED_ORIGINS` (not `*`) +- [ ] Set `AUTHELIA_ENABLED=true` if using SSO +- [ ] Configure SMTP or check email log file +- [ ] Set up notification scheduler (cron/systemd) diff --git a/app/config.py b/app/config.py index 02c3e65..1d8ec2a 100644 --- a/app/config.py +++ b/app/config.py @@ -3,16 +3,32 @@ Application Configuration Environment-based settings with sensible defaults """ import os +import sys +import logging from pathlib import Path +# Configure logging +LOG_LEVEL = os.getenv("LOG_LEVEL", "INFO").upper() +logging.basicConfig( + level=getattr(logging, LOG_LEVEL, logging.INFO), + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s" +) +logger = logging.getLogger("org-parking") + # Database DATABASE_PATH = os.getenv("DATABASE_PATH", "parking.db") DATABASE_URL = os.getenv("DATABASE_URL", f"sqlite:///{DATABASE_PATH}") # JWT Authentication -SECRET_KEY = os.getenv("SECRET_KEY", "change-me-in-production") +SECRET_KEY = os.getenv("SECRET_KEY", "") +if not SECRET_KEY: + logger.error("FATAL: SECRET_KEY environment variable is required") + sys.exit(1) +if SECRET_KEY == "change-me-in-production": + logger.warning("WARNING: Using default SECRET_KEY - change this in production!") + ALGORITHM = "HS256" -ACCESS_TOKEN_EXPIRE_MINUTES = 1440 # 24 hours +ACCESS_TOKEN_EXPIRE_MINUTES = int(os.getenv("ACCESS_TOKEN_EXPIRE_MINUTES", "1440")) # 24 hours # Server HOST = os.getenv("HOST", "0.0.0.0") @@ -32,12 +48,24 @@ AUTHELIA_HEADER_GROUPS = os.getenv("AUTHELIA_HEADER_GROUPS", "Remote-Groups") # Manager role and user assignments are managed by admin in the app UI AUTHELIA_ADMIN_GROUP = os.getenv("AUTHELIA_ADMIN_GROUP", "parking_admins") -# Email (optional) -SMTP_HOST = os.getenv("SMTP_HOST", "") +# External URLs for Authelia mode +# When AUTHELIA_ENABLED, login redirects to Authelia and register to external portal +AUTHELIA_LOGIN_URL = os.getenv("AUTHELIA_LOGIN_URL", "") # e.g., https://auth.rocketscale.it +REGISTRATION_URL = os.getenv("REGISTRATION_URL", "") # e.g., https://register.rocketscale.it + +# Email configuration (following org-stack pattern) +SMTP_ENABLED = os.getenv("SMTP_ENABLED", "false").lower() == "true" +SMTP_HOST = os.getenv("SMTP_HOST", "localhost") SMTP_PORT = int(os.getenv("SMTP_PORT", "587")) SMTP_USER = os.getenv("SMTP_USER", "") SMTP_PASSWORD = os.getenv("SMTP_PASSWORD", "") -SMTP_FROM_EMAIL = os.getenv("SMTP_FROM_EMAIL", SMTP_USER) +SMTP_FROM = os.getenv("SMTP_FROM", SMTP_USER or "noreply@parking.local") +SMTP_USE_TLS = os.getenv("SMTP_USE_TLS", "true").lower() == "true" +EMAIL_LOG_FILE = os.getenv("EMAIL_LOG_FILE", "/tmp/parking-emails.log") + +# Rate limiting +RATE_LIMIT_REQUESTS = int(os.getenv("RATE_LIMIT_REQUESTS", "5")) # requests per window +RATE_LIMIT_WINDOW = int(os.getenv("RATE_LIMIT_WINDOW", "60")) # seconds # Paths BASE_DIR = Path(__file__).resolve().parent.parent diff --git a/app/routes/auth.py b/app/routes/auth.py index dded0e7..4984673 100644 --- a/app/routes/auth.py +++ b/app/routes/auth.py @@ -2,20 +2,23 @@ Authentication Routes Login, register, logout, and user info """ -from fastapi import APIRouter, Depends, HTTPException, status, Response +from fastapi import APIRouter, Depends, HTTPException, status, Response, Request from pydantic import BaseModel, EmailStr from sqlalchemy.orm import Session +from slowapi import Limiter +from slowapi.util import get_remote_address from database.connection import get_db from services.auth import ( create_user, authenticate_user, create_access_token, - get_user_by_email, hash_password, verify_password + get_user_by_email ) from utils.auth_middleware import get_current_user +from utils.helpers import validate_password, format_password_errors, get_notification_default from app import config -import re router = APIRouter(prefix="/api/auth", tags=["auth"]) +limiter = Limiter(key_func=get_remote_address) class RegisterRequest(BaseModel): @@ -52,7 +55,8 @@ class UserResponse(BaseModel): @router.post("/register", response_model=TokenResponse) -def register(data: RegisterRequest, db: Session = Depends(get_db)): +@limiter.limit(f"{config.RATE_LIMIT_REQUESTS}/minute") +def register(request: Request, data: RegisterRequest, db: Session = Depends(get_db)): """Register a new user""" if get_user_by_email(db, data.email): raise HTTPException( @@ -60,10 +64,12 @@ def register(data: RegisterRequest, db: Session = Depends(get_db)): detail="Email already registered" ) - if len(data.password) < 8: + # Validate password strength + password_errors = validate_password(data.password) + if password_errors: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail="Password must be at least 8 characters" + detail=format_password_errors(password_errors) ) user = create_user( @@ -74,16 +80,19 @@ def register(data: RegisterRequest, db: Session = Depends(get_db)): manager_id=data.manager_id ) + config.logger.info(f"New user registered: {data.email}") token = create_access_token(user.id, user.email) return TokenResponse(access_token=token) @router.post("/login", response_model=TokenResponse) -def login(data: LoginRequest, response: Response, db: Session = Depends(get_db)): +@limiter.limit(f"{config.RATE_LIMIT_REQUESTS}/minute") +def login(request: Request, data: LoginRequest, response: Response, db: Session = Depends(get_db)): """Login with email and password""" user = authenticate_user(db, data.email, data.password) if not user: + config.logger.warning(f"Failed login attempt for: {data.email}") raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid credentials" @@ -99,6 +108,7 @@ def login(data: LoginRequest, response: Response, db: Session = Depends(get_db)) samesite="lax" ) + config.logger.info(f"User logged in: {data.email}") return TokenResponse(access_token=token) @@ -119,15 +129,27 @@ def get_me(user=Depends(get_current_user)): manager_id=user.manager_id, role=user.role, manager_parking_quota=user.manager_parking_quota, - week_start_day=user.week_start_day or 0, - notify_weekly_parking=user.notify_weekly_parking if user.notify_weekly_parking is not None else 1, - notify_daily_parking=user.notify_daily_parking if user.notify_daily_parking is not None else 1, - notify_daily_parking_hour=user.notify_daily_parking_hour if user.notify_daily_parking_hour is not None else 8, - notify_daily_parking_minute=user.notify_daily_parking_minute if user.notify_daily_parking_minute is not None else 0, - notify_parking_changes=user.notify_parking_changes if user.notify_parking_changes is not None else 1 + week_start_day=get_notification_default(user.week_start_day, 0), + notify_weekly_parking=get_notification_default(user.notify_weekly_parking, 1), + notify_daily_parking=get_notification_default(user.notify_daily_parking, 1), + notify_daily_parking_hour=get_notification_default(user.notify_daily_parking_hour, 8), + notify_daily_parking_minute=get_notification_default(user.notify_daily_parking_minute, 0), + notify_parking_changes=get_notification_default(user.notify_parking_changes, 1) ) +@router.get("/config") +def get_auth_config(): + """Get authentication configuration for frontend. + Returns info about auth mode and external URLs. + """ + return { + "authelia_enabled": config.AUTHELIA_ENABLED, + "login_url": config.AUTHELIA_LOGIN_URL if config.AUTHELIA_ENABLED else None, + "registration_url": config.REGISTRATION_URL if config.AUTHELIA_ENABLED else None + } + + @router.get("/holidays/{year}") def get_holidays(year: int): """Get public holidays for a given year""" diff --git a/app/routes/managers.py b/app/routes/managers.py index 380b13a..c1eda53 100644 --- a/app/routes/managers.py +++ b/app/routes/managers.py @@ -9,7 +9,7 @@ from datetime import datetime from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel from sqlalchemy.orm import Session -import uuid +from sqlalchemy import func from database.connection import get_db from database.models import ( @@ -18,6 +18,8 @@ from database.models import ( ParkingGuarantee, ParkingExclusion ) from utils.auth_middleware import require_admin, require_manager_or_admin, get_current_user +from utils.helpers import generate_uuid +from app import config router = APIRouter(prefix="/api/managers", tags=["managers"]) @@ -54,21 +56,28 @@ class ManagerSettingsUpdate(BaseModel): def list_managers(db: Session = Depends(get_db), user=Depends(require_manager_or_admin)): """Get all managers with their managed user count and parking quota""" managers = db.query(User).filter(User.role == "manager").all() - result = [] - for manager in managers: - managed_user_count = db.query(User).filter(User.manager_id == manager.id).count() + # Batch query to get managed user counts for all managers at once + manager_ids = [m.id for m in managers] + if manager_ids: + counts = db.query(User.manager_id, func.count(User.id)).filter( + User.manager_id.in_(manager_ids) + ).group_by(User.manager_id).all() + managed_counts = {manager_id: count for manager_id, count in counts} + else: + managed_counts = {} - result.append({ + return [ + { "id": manager.id, "name": manager.name, "email": manager.email, "parking_quota": manager.manager_parking_quota or 0, "spot_prefix": manager.manager_spot_prefix, - "managed_user_count": managed_user_count - }) - - return result + "managed_user_count": managed_counts.get(manager.id, 0) + } + for manager in managers + ] @router.get("/{manager_id}") @@ -164,7 +173,7 @@ def add_manager_closing_day(manager_id: str, data: ClosingDayCreate, db: Session raise HTTPException(status_code=400, detail="Closing day already exists for this date") closing_day = ManagerClosingDay( - id=str(uuid.uuid4()), + id=generate_uuid(), manager_id=manager_id, date=data.date, reason=data.reason @@ -217,7 +226,7 @@ def add_manager_weekly_closing_day(manager_id: str, data: WeeklyClosingDayCreate raise HTTPException(status_code=400, detail="Weekly closing day already exists for this weekday") weekly_closing = ManagerWeeklyClosingDay( - id=str(uuid.uuid4()), + id=generate_uuid(), manager_id=manager_id, weekday=data.weekday ) @@ -246,17 +255,25 @@ def remove_manager_weekly_closing_day(manager_id: str, weekly_id: str, db: Sessi def get_manager_guarantees(manager_id: str, db: Session = Depends(get_db), user=Depends(require_manager_or_admin)): """Get parking guarantees for a manager""" guarantees = db.query(ParkingGuarantee).filter(ParkingGuarantee.manager_id == manager_id).all() - result = [] - for g in guarantees: - target_user = db.query(User).filter(User.id == g.user_id).first() - result.append({ + + # Batch query to get all user names at once + user_ids = [g.user_id for g in guarantees] + if user_ids: + users = db.query(User).filter(User.id.in_(user_ids)).all() + user_lookup = {u.id: u.name for u in users} + else: + user_lookup = {} + + return [ + { "id": g.id, "user_id": g.user_id, - "user_name": target_user.name if target_user else None, + "user_name": user_lookup.get(g.user_id), "start_date": g.start_date, "end_date": g.end_date - }) - return result + } + for g in guarantees + ] @router.post("/{manager_id}/guarantees") @@ -276,7 +293,7 @@ def add_manager_guarantee(manager_id: str, data: GuaranteeCreate, db: Session = raise HTTPException(status_code=400, detail="User already has a parking guarantee") guarantee = ParkingGuarantee( - id=str(uuid.uuid4()), + id=generate_uuid(), manager_id=manager_id, user_id=data.user_id, start_date=data.start_date, @@ -308,17 +325,25 @@ def remove_manager_guarantee(manager_id: str, guarantee_id: str, db: Session = D def get_manager_exclusions(manager_id: str, db: Session = Depends(get_db), user=Depends(require_manager_or_admin)): """Get parking exclusions for a manager""" exclusions = db.query(ParkingExclusion).filter(ParkingExclusion.manager_id == manager_id).all() - result = [] - for e in exclusions: - target_user = db.query(User).filter(User.id == e.user_id).first() - result.append({ + + # Batch query to get all user names at once + user_ids = [e.user_id for e in exclusions] + if user_ids: + users = db.query(User).filter(User.id.in_(user_ids)).all() + user_lookup = {u.id: u.name for u in users} + else: + user_lookup = {} + + return [ + { "id": e.id, "user_id": e.user_id, - "user_name": target_user.name if target_user else None, + "user_name": user_lookup.get(e.user_id), "start_date": e.start_date, "end_date": e.end_date - }) - return result + } + for e in exclusions + ] @router.post("/{manager_id}/exclusions") @@ -338,7 +363,7 @@ def add_manager_exclusion(manager_id: str, data: ExclusionCreate, db: Session = raise HTTPException(status_code=400, detail="User already has a parking exclusion") exclusion = ParkingExclusion( - id=str(uuid.uuid4()), + id=generate_uuid(), manager_id=manager_id, user_id=data.user_id, start_date=data.start_date, diff --git a/app/routes/parking.py b/app/routes/parking.py index e65b138..0f32b23 100644 --- a/app/routes/parking.py +++ b/app/routes/parking.py @@ -12,13 +12,13 @@ from datetime import datetime from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel from sqlalchemy.orm import Session -import uuid from database.connection import get_db from database.models import DailyParkingAssignment, User from utils.auth_middleware import get_current_user, require_manager_or_admin from services.parking import initialize_parking_pool, get_spot_display_name -from services.notifications import queue_parking_change_notification +from services.notifications import notify_parking_assigned, notify_parking_released, notify_parking_reassigned +from app import config router = APIRouter(prefix="/api/parking", tags=["parking"]) @@ -203,12 +203,10 @@ def release_my_spot(assignment_id: str, db: Session = Depends(get_db), current_u assignment.user_id = None db.commit() - # Queue notification (self-release, so just confirmation) - queue_parking_change_notification( - current_user, assignment.date, "released", - spot_display_name, db=db - ) + # Send notification (self-release, so just confirmation) + notify_parking_released(current_user, assignment.date, spot_display_name) + config.logger.info(f"User {current_user.email} released parking spot {spot_display_name} on {assignment.date}") return {"message": "Parking spot released"} @@ -257,26 +255,21 @@ def reassign_spot(data: ReassignSpotRequest, db: Session = Depends(get_db), curr assignment.user_id = data.new_user_id - # Queue notifications + # Send notifications # Notify old user that spot was reassigned if old_user and old_user.id != new_user.id: - queue_parking_change_notification( - old_user, assignment.date, "reassigned", - spot_display_name, new_user.name, db - ) + notify_parking_reassigned(old_user, assignment.date, spot_display_name, new_user.name) # Notify new user that spot was assigned - queue_parking_change_notification( - new_user, assignment.date, "assigned", - spot_display_name, db=db - ) + notify_parking_assigned(new_user, assignment.date, spot_display_name) + + config.logger.info(f"Parking spot {spot_display_name} on {assignment.date} reassigned from {old_user.email if old_user else 'unassigned'} to {new_user.email}") else: assignment.user_id = None # Notify old user that spot was released if old_user: - queue_parking_change_notification( - old_user, assignment.date, "released", - spot_display_name, db=db - ) + notify_parking_released(old_user, assignment.date, spot_display_name) + + config.logger.info(f"Parking spot {spot_display_name} on {assignment.date} released by {old_user.email if old_user else 'unknown'}") db.commit() db.refresh(assignment) diff --git a/app/routes/presence.py b/app/routes/presence.py index a6a2aca..da5c7b6 100644 --- a/app/routes/presence.py +++ b/app/routes/presence.py @@ -7,12 +7,13 @@ from datetime import datetime, timedelta from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel from sqlalchemy.orm import Session -import uuid from database.connection import get_db from database.models import UserPresence, User, DailyParkingAssignment from utils.auth_middleware import get_current_user, require_manager_or_admin +from utils.helpers import generate_uuid from services.parking import handle_presence_change, get_spot_display_name +from app import config router = APIRouter(prefix="/api/presence", tags=["presence"]) @@ -113,7 +114,7 @@ def _mark_presence_for_user( presence = existing else: presence = UserPresence( - id=str(uuid.uuid4()), + id=generate_uuid(), user_id=user_id, date=date, status=status, @@ -139,7 +140,7 @@ def _mark_presence_for_user( parking_manager_id, db ) except Exception as e: - print(f"Warning: Parking handler failed: {e}") + config.logger.warning(f"Parking handler failed for user {user_id} on {date}: {e}") return presence @@ -186,7 +187,7 @@ def _bulk_mark_presence( results.append(existing) else: presence = UserPresence( - id=str(uuid.uuid4()), + id=generate_uuid(), user_id=user_id, date=date_str, status=status, @@ -209,8 +210,8 @@ def _bulk_mark_presence( old_status or "absent", status, parking_manager_id, db ) - except Exception: - pass + except Exception as e: + config.logger.warning(f"Parking handler failed for user {user_id} on {date_str}: {e}") current_date += timedelta(days=1) @@ -253,8 +254,8 @@ def _delete_presence( old_status, "absent", parking_manager_id, db ) - except Exception: - pass + except Exception as e: + config.logger.warning(f"Parking handler failed for user {user_id} on {date}: {e}") return {"message": "Presence deleted"} diff --git a/app/routes/users.py b/app/routes/users.py index 2548589..5a18ac5 100644 --- a/app/routes/users.py +++ b/app/routes/users.py @@ -2,17 +2,18 @@ User Management Routes Admin user CRUD and user self-service (profile, settings, password) """ -from typing import List from datetime import datetime from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel, EmailStr from sqlalchemy.orm import Session -import uuid -import re from database.connection import get_db from database.models import User from utils.auth_middleware import get_current_user, require_admin +from utils.helpers import ( + generate_uuid, is_ldap_user, is_ldap_admin, + validate_password, format_password_errors, get_notification_default +) from services.auth import hash_password, verify_password from app import config @@ -73,23 +74,33 @@ class UserResponse(BaseModel): from_attributes = True -def user_to_response(user: User, db: Session) -> dict: - """Convert user to response dict with computed fields""" - # Get manager name if user has a manager +def user_to_response(user: User, db: Session, manager_lookup: dict = None, managed_counts: dict = None) -> dict: + """ + Convert user to response dict with computed fields. + + Args: + user: The user to convert + db: Database session + manager_lookup: Optional pre-fetched dict of manager_id -> name (for batch operations) + managed_counts: Optional pre-fetched dict of user_id -> managed_user_count (for batch operations) + """ + # Get manager name - use lookup if available, otherwise query manager_name = None if user.manager_id: - manager = db.query(User).filter(User.id == user.manager_id).first() - if manager: - manager_name = manager.name + if manager_lookup is not None: + manager_name = manager_lookup.get(user.manager_id) + else: + manager = db.query(User).filter(User.id == user.manager_id).first() + if manager: + manager_name = manager.name # Count managed users if this user is a manager managed_user_count = None if user.role == "manager": - managed_user_count = db.query(User).filter(User.manager_id == user.id).count() - - # Determine if user is LDAP-managed - is_ldap_user = config.AUTHELIA_ENABLED and user.password_hash is None - is_ldap_admin = is_ldap_user and user.role == "admin" + if managed_counts is not None: + managed_user_count = managed_counts.get(user.id, 0) + else: + managed_user_count = db.query(User).filter(User.manager_id == user.id).count() return { "id": user.id, @@ -101,8 +112,8 @@ def user_to_response(user: User, db: Session) -> dict: "manager_parking_quota": user.manager_parking_quota, "manager_spot_prefix": user.manager_spot_prefix, "managed_user_count": managed_user_count, - "is_ldap_user": is_ldap_user, - "is_ldap_admin": is_ldap_admin, + "is_ldap_user": is_ldap_user(user), + "is_ldap_admin": is_ldap_admin(user), "created_at": user.created_at } @@ -112,7 +123,25 @@ def user_to_response(user: User, db: Session) -> dict: def list_users(db: Session = Depends(get_db), user=Depends(require_admin)): """List all users (admin only)""" users = db.query(User).all() - return [user_to_response(u, db) for u in users] + + # Build lookups to avoid N+1 queries + # Manager lookup: id -> name + manager_ids = list(set(u.manager_id for u in users if u.manager_id)) + managers = db.query(User).filter(User.id.in_(manager_ids)).all() if manager_ids else [] + manager_lookup = {m.id: m.name for m in managers} + + # Managed user counts for managers + from sqlalchemy import func + manager_user_ids = [u.id for u in users if u.role == "manager"] + if manager_user_ids: + counts = db.query(User.manager_id, func.count(User.id)).filter( + User.manager_id.in_(manager_user_ids) + ).group_by(User.manager_id).all() + managed_counts = {manager_id: count for manager_id, count in counts} + else: + managed_counts = {} + + return [user_to_response(u, db, manager_lookup, managed_counts) for u in users] @router.get("/{user_id}") @@ -136,13 +165,18 @@ def create_user(data: UserCreate, db: Session = Depends(get_db), user=Depends(re if data.role not in ["admin", "manager", "employee"]: raise HTTPException(status_code=400, detail="Invalid role") + # Validate password strength + password_errors = validate_password(data.password) + if password_errors: + raise HTTPException(status_code=400, detail=format_password_errors(password_errors)) + if data.manager_id: manager = db.query(User).filter(User.id == data.manager_id).first() if not manager or manager.role != "manager": raise HTTPException(status_code=400, detail="Invalid manager") new_user = User( - id=str(uuid.uuid4()), + id=generate_uuid(), email=data.email, password_hash=hash_password(data.password), name=data.name, @@ -154,6 +188,7 @@ def create_user(data: UserCreate, db: Session = Depends(get_db), user=Depends(re db.add(new_user) db.commit() db.refresh(new_user) + config.logger.info(f"Admin created new user: {data.email}") return user_to_response(new_user, db) @@ -165,12 +200,12 @@ def update_user(user_id: str, data: UserUpdate, db: Session = Depends(get_db), u raise HTTPException(status_code=404, detail="User not found") # Check if user is LDAP-managed - is_ldap_user = config.AUTHELIA_ENABLED and target.password_hash is None - is_ldap_admin = is_ldap_user and target.role == "admin" + target_is_ldap = is_ldap_user(target) + target_is_ldap_admin = is_ldap_admin(target) # Name update - blocked for LDAP users if data.name is not None: - if is_ldap_user: + if target_is_ldap: raise HTTPException(status_code=400, detail="Name is managed by LDAP") target.name = data.name @@ -179,7 +214,7 @@ def update_user(user_id: str, data: UserUpdate, db: Session = Depends(get_db), u if data.role not in ["admin", "manager", "employee"]: raise HTTPException(status_code=400, detail="Invalid role") # Can't change admin role for LDAP admins (they get admin from parking_admins group) - if is_ldap_admin and data.role != "admin": + if target_is_ldap_admin and data.role != "admin": raise HTTPException(status_code=400, detail="Admin role is managed by LDAP group (parking_admins)") # If changing from manager to another role, check for managed users if target.role == "manager" and data.role != "manager": @@ -254,8 +289,6 @@ def delete_user(user_id: str, db: Session = Depends(get_db), current_user=Depend @router.get("/me/profile") def get_profile(db: Session = Depends(get_db), current_user=Depends(get_current_user)): """Get current user's profile""" - is_ldap_user = config.AUTHELIA_ENABLED and current_user.password_hash is None - # Get manager name manager_name = None if current_user.manager_id: @@ -270,17 +303,15 @@ def get_profile(db: Session = Depends(get_db), current_user=Depends(get_current_ "role": current_user.role, "manager_id": current_user.manager_id, "manager_name": manager_name, - "is_ldap_user": is_ldap_user + "is_ldap_user": is_ldap_user(current_user) } @router.put("/me/profile") def update_profile(data: ProfileUpdate, db: Session = Depends(get_db), current_user=Depends(get_current_user)): """Update current user's profile (limited fields)""" - is_ldap_user = config.AUTHELIA_ENABLED and current_user.password_hash is None - if data.name is not None: - if is_ldap_user: + if is_ldap_user(current_user): raise HTTPException(status_code=400, detail="Name is managed by LDAP") current_user.name = data.name current_user.updated_at = datetime.utcnow().isoformat() @@ -293,12 +324,12 @@ def update_profile(data: ProfileUpdate, db: Session = Depends(get_db), current_u def get_settings(current_user=Depends(get_current_user)): """Get current user's settings""" return { - "week_start_day": current_user.week_start_day or 0, - "notify_weekly_parking": current_user.notify_weekly_parking if current_user.notify_weekly_parking is not None else 1, - "notify_daily_parking": current_user.notify_daily_parking if current_user.notify_daily_parking is not None else 1, - "notify_daily_parking_hour": current_user.notify_daily_parking_hour if current_user.notify_daily_parking_hour is not None else 8, - "notify_daily_parking_minute": current_user.notify_daily_parking_minute if current_user.notify_daily_parking_minute is not None else 0, - "notify_parking_changes": current_user.notify_parking_changes if current_user.notify_parking_changes is not None else 1 + "week_start_day": get_notification_default(current_user.week_start_day, 0), + "notify_weekly_parking": get_notification_default(current_user.notify_weekly_parking, 1), + "notify_daily_parking": get_notification_default(current_user.notify_daily_parking, 1), + "notify_daily_parking_hour": get_notification_default(current_user.notify_daily_parking_hour, 8), + "notify_daily_parking_minute": get_notification_default(current_user.notify_daily_parking_minute, 0), + "notify_parking_changes": get_notification_default(current_user.notify_parking_changes, 1) } @@ -346,28 +377,19 @@ def update_settings(data: SettingsUpdate, db: Session = Depends(get_db), current @router.post("/me/change-password") def change_password(data: ChangePasswordRequest, db: Session = Depends(get_db), current_user=Depends(get_current_user)): """Change current user's password (not available in LDAP mode)""" - if config.AUTHELIA_ENABLED and current_user.password_hash is None: + if is_ldap_user(current_user): raise HTTPException(status_code=400, detail="Password is managed by LDAP") if not verify_password(data.current_password, current_user.password_hash): raise HTTPException(status_code=400, detail="Current password is incorrect") # Validate new password - password = data.new_password - errors = [] - if len(password) < 8: - errors.append("at least 8 characters") - if not re.search(r'[A-Z]', password): - errors.append("one uppercase letter") - if not re.search(r'[a-z]', password): - errors.append("one lowercase letter") - if not re.search(r'[0-9]', password): - errors.append("one number") + password_errors = validate_password(data.new_password) + if password_errors: + raise HTTPException(status_code=400, detail=format_password_errors(password_errors)) - if errors: - raise HTTPException(status_code=400, detail=f"Password must contain: {', '.join(errors)}") - - current_user.password_hash = hash_password(password) + current_user.password_hash = hash_password(data.new_password) current_user.updated_at = datetime.utcnow().isoformat() db.commit() + config.logger.info(f"User {current_user.email} changed password") return {"message": "Password changed"} diff --git a/compose.yml b/compose.yml index 7799ec3..425989d 100644 --- a/compose.yml +++ b/compose.yml @@ -7,19 +7,12 @@ services: - "8000:8000" volumes: - ./data:/app/data + env_file: + - .env environment: - - SECRET_KEY=${SECRET_KEY:-change-me-in-production} - HOST=0.0.0.0 - PORT=8000 - DATABASE_PATH=/app/data/parking.db - - AUTHELIA_ENABLED=${AUTHELIA_ENABLED:-false} - - ALLOWED_ORIGINS=${ALLOWED_ORIGINS:-*} - # SMTP (optional) - - SMTP_HOST=${SMTP_HOST:-} - - SMTP_PORT=${SMTP_PORT:-587} - - SMTP_USER=${SMTP_USER:-} - - SMTP_PASSWORD=${SMTP_PASSWORD:-} - - SMTP_FROM=${SMTP_FROM:-} healthcheck: test: ["CMD", "python", "-c", "import urllib.request; urllib.request.urlopen('http://localhost:8000/health')"] interval: 30s diff --git a/frontend/pages/landing.html b/frontend/pages/landing.html index 9d9774f..a92a49d 100644 --- a/frontend/pages/landing.html +++ b/frontend/pages/landing.html @@ -15,16 +15,18 @@

Manage team presence and parking assignments

-
- Sign In - Create Account +
+ +
Loading...
diff --git a/frontend/pages/login.html b/frontend/pages/login.html index 50074ea..2d0e3b4 100644 --- a/frontend/pages/login.html +++ b/frontend/pages/login.html @@ -37,11 +37,30 @@