fix landing page
This commit is contained in:
278
CLAUDE.md
278
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
|
||||
<script type="module">
|
||||
import api from '/js/api.js';
|
||||
import { initNav } from '/js/nav.js';
|
||||
### Email Testing
|
||||
With `SMTP_ENABLED=false`, check email log:
|
||||
```bash
|
||||
tail -f /tmp/parking-emails.log
|
||||
```
|
||||
|
||||
document.addEventListener('DOMContentLoaded', async () => {
|
||||
await api.checkAuth(true); // Redirect to login if not auth'd
|
||||
initNav();
|
||||
// Page-specific logic
|
||||
});
|
||||
</script>
|
||||
### 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)
|
||||
|
||||
Reference in New Issue
Block a user