You cannot select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
226 lines
10 KiB
Markdown
226 lines
10 KiB
Markdown
# Phase 1 Code Review & Final Summary
|
|
|
|
**Date**: 2026-03-21
|
|
**Reviewer**: Tech Lead (Claude)
|
|
|
|
---
|
|
|
|
## Build & Test Status
|
|
|
|
| Component | Status | Details |
|
|
|---|---|---|
|
|
| Backend compilation | PASS | `cargo check` — 0 errors, 10 warnings (unused code for future phases) |
|
|
| Backend unit tests | PASS | 47/47 passed |
|
|
| Backend integration tests | SKIP | Require Postgres (gracefully skip without `TEST_DATABASE_URL`) |
|
|
| Frontend TypeScript | PASS | `tsc --noEmit` — 0 errors (after .ts -> .tsx fix for i18n) |
|
|
| Frontend unit tests | PASS | 26/26 passed (4 test files) |
|
|
| Frontend build | PASS | Vite production build — 58KB main bundle + code-split pages |
|
|
|
|
---
|
|
|
|
## Files Created
|
|
|
|
### Backend (`backend/`) — 30 Rust files + 4 migrations
|
|
|
|
```
|
|
backend/
|
|
├── Cargo.toml
|
|
├── Dockerfile
|
|
├── migrations/
|
|
│ ├── 20260321000001_create_users.sql
|
|
│ ├── 20260321000002_create_sessions.sql
|
|
│ ├── 20260321000003_create_magic_tokens.sql
|
|
│ └── 20260321000004_create_settings.sql
|
|
├── src/
|
|
│ ├── main.rs # Entry point, CLI, server startup
|
|
│ ├── lib.rs # Library crate for integration tests
|
|
│ ├── cli.rs # Clap subcommands (serve, create-admin)
|
|
│ ├── config.rs # Typed config from env vars + validation
|
|
│ ├── app_state.rs # Shared state (pool, config, http_client, rate_limiter)
|
|
│ ├── errors.rs # AppError enum -> HTTP responses
|
|
│ ├── router.rs # Route definitions, middleware stack, SPA fallback
|
|
│ ├── db/
|
|
│ │ ├── mod.rs
|
|
│ │ ├── users.rs # User CRUD queries
|
|
│ │ ├── sessions.rs # Session CRUD queries
|
|
│ │ ├── magic_links.rs # Magic link token queries
|
|
│ │ └── settings.rs # Settings upsert/get queries
|
|
│ ├── handlers/
|
|
│ │ ├── mod.rs
|
|
│ │ ├── auth.rs # Auth endpoints (register, login, verify, logout, me)
|
|
│ │ ├── settings.rs # Settings GET/PUT
|
|
│ │ └── health.rs # Health check
|
|
│ ├── middleware/
|
|
│ │ ├── mod.rs
|
|
│ │ ├── auth.rs # Session extraction, AuthUser/AdminUser extractors
|
|
│ │ └── csrf.rs # X-Requested-With header check
|
|
│ ├── models/
|
|
│ │ ├── mod.rs
|
|
│ │ ├── user.rs # User model + UserRole enum
|
|
│ │ ├── session.rs # Session model
|
|
│ │ ├── magic_link.rs # MagicLinkToken model
|
|
│ │ └── settings.rs # UserSettings model + validation
|
|
│ ├── services/
|
|
│ │ ├── mod.rs
|
|
│ │ ├── auth.rs # Token lifecycle, session creation
|
|
│ │ ├── email.rs # Resend API integration
|
|
│ │ ├── turnstile.rs # Cloudflare Turnstile verification
|
|
│ │ └── rate_limiter.rs # In-memory sliding window rate limiter
|
|
│ └── util/
|
|
│ ├── mod.rs
|
|
│ └── token.rs # Token generation + SHA-256 hashing
|
|
└── tests/
|
|
├── common/
|
|
│ └── mod.rs # TestApp harness (creates temp DB, runs migrations)
|
|
├── api_auth_test.rs # 13 auth flow tests
|
|
├── api_csrf_test.rs # 4 CSRF protection tests
|
|
├── api_health_test.rs # 1 health check test
|
|
└── api_settings_test.rs # 11 settings CRUD tests
|
|
```
|
|
|
|
### Frontend (`frontend/`) — 34 files
|
|
|
|
```
|
|
frontend/
|
|
├── package.json
|
|
├── vite.config.ts
|
|
├── tsconfig.json
|
|
├── index.html
|
|
├── src/
|
|
│ ├── index.tsx # SolidJS render entry
|
|
│ ├── index.css # Tailwind import
|
|
│ ├── App.tsx # Router, layouts, route guards
|
|
│ ├── types.ts # All TypeScript interfaces + DEFAULT_SETTINGS
|
|
│ ├── env.d.ts # Vite env type declarations
|
|
│ ├── api/
|
|
│ │ ├── client.ts # Fetch wrapper (CSRF, credentials, 401 redirect)
|
|
│ │ ├── auth.ts # Auth API functions
|
|
│ │ └── settings.ts # Settings API functions
|
|
│ ├── contexts/
|
|
│ │ └── AuthContext.tsx # Auth provider (signals, session check)
|
|
│ ├── i18n/
|
|
│ │ ├── fr.ts # French translations (all strings)
|
|
│ │ └── index.tsx # i18n provider + t() helper
|
|
│ ├── pages/
|
|
│ │ ├── Login.tsx # Magic link request + Turnstile
|
|
│ │ ├── Register.tsx # Sign-up + Turnstile
|
|
│ │ ├── AuthVerify.tsx # Token verification page
|
|
│ │ ├── Settings.tsx # Full settings form (categories, theme, etc.)
|
|
│ │ └── Home.tsx # Placeholder with empty state
|
|
│ ├── components/
|
|
│ │ ├── Layout.tsx # App shell with navbar
|
|
│ │ ├── Navbar.tsx # Navigation + active route indicator
|
|
│ │ ├── MobileMenu.tsx # Hamburger menu (new feature)
|
|
│ │ ├── ErrorBoundary.tsx # Top-level error boundary
|
|
│ │ ├── Turnstile.tsx # Cloudflare widget wrapper
|
|
│ │ └── ui/
|
|
│ │ ├── Button.tsx # Reusable button (primary/secondary/danger)
|
|
│ │ ├── LoadingSpinner.tsx # Centered spinner
|
|
│ │ └── Toast.tsx # Toast notification system
|
|
│ └── __tests__/
|
|
│ ├── setup.ts
|
|
│ ├── api-client.test.ts # 7 tests
|
|
│ ├── i18n.test.ts # 7 tests
|
|
│ ├── auth-context.test.tsx # 3 tests
|
|
│ └── settings-validation.test.ts # 9 tests
|
|
```
|
|
|
|
### Infrastructure (project root)
|
|
|
|
```
|
|
docker-compose.yml # App + Postgres services
|
|
.env.example # All required env vars documented
|
|
```
|
|
|
|
---
|
|
|
|
## Architecture Compliance
|
|
|
|
| Decision | Backend | Frontend |
|
|
|---|---|---|
|
|
| Rust + Axum | PASS | N/A |
|
|
| Postgres (sqlx) | PASS | N/A |
|
|
| SolidJS | N/A | PASS |
|
|
| Tailwind CSS v4 | N/A | PASS |
|
|
| Email + magic link auth | PASS | PASS |
|
|
| Cloudflare Turnstile | PASS | PASS |
|
|
| 30-day sessions | PASS | PASS (relies on backend) |
|
|
| CSRF: X-Requested-With | PASS | PASS |
|
|
| CLI admin creation | PASS | N/A |
|
|
| Docker deployment | PASS | N/A |
|
|
| i18n-ready (French) | N/A | PARTIAL (3 hardcoded strings) |
|
|
| No Google dependencies | PASS | PASS |
|
|
| Idiomatic Rust | PASS | N/A |
|
|
|
|
---
|
|
|
|
## Issues Found
|
|
|
|
### Critical (fix before production)
|
|
|
|
| # | Issue | File | Description |
|
|
|---|---|---|---|
|
|
| C1 | CORS fallback to `*` | `backend/src/router.rs` | If APP_URL is malformed, CORS silently allows all origins. Should fail startup instead. |
|
|
| C2 | No request body size limit | `backend/src/router.rs` | No DefaultBodyLimit configured — DoS via large request bodies. |
|
|
|
|
### High Priority
|
|
|
|
| # | Issue | File | Description |
|
|
|---|---|---|---|
|
|
| H1 | Email validation too permissive | `backend/src/handlers/auth.rs` | Only checks `contains('@')` — accepts malformed emails like `@x` or `a@@b`. Use email validation crate. |
|
|
| H2 | 3 hardcoded French strings | Frontend (Toast, MobileMenu, Home) | `"Fermer"`, `"Menu"`, `"Disponible prochainement"` bypass i18n. |
|
|
| H3 | Auth token in URL query | `frontend/src/api/auth.ts` | Magic link token appears in browser history/logs. Consider POST body instead. |
|
|
|
|
### Medium Priority
|
|
|
|
| # | Issue | File | Description |
|
|
|---|---|---|---|
|
|
| M1 | Turnstile polling has no timeout | `frontend/src/components/Turnstile.tsx` | Polls indefinitely if script fails to load. Add 5s timeout. |
|
|
| M2 | Settings validation uses byte length | `backend/src/models/settings.rs` | `.len()` counts bytes, not characters. Multi-byte UTF-8 strings fail early. Use `.chars().count()`. |
|
|
| M3 | DELETE method in CORS but no DELETE endpoints | `backend/src/router.rs` | Minor; remove until needed. |
|
|
| M4 | Settings page hardcoded fallback values | `frontend/src/pages/Settings.tsx` | `parseInt(...) \|\| 7` instead of using DEFAULT_SETTINGS constants. |
|
|
|
|
### Low Priority (suggestions)
|
|
|
|
- Add audit logging for failed magic link attempts
|
|
- Add Subresource Integrity (SRI) for Turnstile script in index.html
|
|
- Add AbortSignal to API calls for cleanup
|
|
- Document that in-memory rate limiter is single-instance only
|
|
- Remove unnecessary timing delay in login handler (Turnstile latency already mitigates timing attacks)
|
|
|
|
---
|
|
|
|
## What's Good
|
|
|
|
### Backend
|
|
- **Zero unwrap() in production code** — proper error propagation throughout
|
|
- **All SQL queries parameterized** — no SQL injection risk
|
|
- **Token security**: 32-byte random tokens, SHA-256 hashed, never stored raw
|
|
- **Anti-enumeration**: register/login return identical responses regardless of email existence
|
|
- **Secure cookies**: HttpOnly + SameSite=Lax + Secure (HTTPS)
|
|
- **Clean 3-layer architecture**: handlers -> services -> db
|
|
- **Comprehensive security headers**: CSP, HSTS, X-Frame-Options, etc.
|
|
- **47 unit tests** covering config, tokens, validation, errors
|
|
- **29 integration tests** covering auth flow, CSRF, settings CRUD, data isolation
|
|
|
|
### Frontend
|
|
- **Correct SolidJS idioms**: createSignal, Show, For, onMount, onCleanup — no React patterns leaked
|
|
- **CSRF header on all requests** via API client wrapper
|
|
- **No sensitive data in localStorage** — relies on secure session cookies
|
|
- **i18n architecture** with typed translation keys (89% coverage)
|
|
- **Mobile hamburger menu** (fixing a gap in the original app)
|
|
- **Active route indicator** (fixing another gap)
|
|
- **Code-split pages** via lazy() imports
|
|
- **26 unit tests** covering API client, i18n, auth context, settings validation
|
|
- **58KB production bundle** (well-optimized)
|
|
|
|
---
|
|
|
|
## Recommended Next Steps
|
|
|
|
1. **Fix C1 + C2** (CORS + body limit) — 15 minutes
|
|
2. **Fix H1** (email validation) — add `email_address` crate — 20 minutes
|
|
3. **Fix H2** (i18n strings) — add missing translation keys — 10 minutes
|
|
4. **Set up Postgres for integration tests** and run full test suite
|
|
5. **Proceed to Phase 2** (Sources CRUD + Scraper) per the roadmap
|