BizzleBot
f718a76153
Features: - Copy Previous Week (entries duplication) - Bulk Approve/Reject (admin workflow) - Overtime tracking (employee + admin views) - DayCard component with auto-save - PDF generation, email notifications - React + Tailwind frontend, Prisma + PostgreSQL backend - Docker deployment (3 containers)
244 lines
13 KiB
Markdown
244 lines
13 KiB
Markdown
# Coastal Timesheet v2 — Backend API Audit Report
|
||
|
||
**Auditor:** ScriptForge ⚡ (Expert Panel)
|
||
**Date:** 2026-02-15
|
||
**Backend:** Node.js + Express + Prisma + PostgreSQL
|
||
**Container:** `coastal-backend` on port 127.0.0.1:3001
|
||
|
||
---
|
||
|
||
## Executive Summary
|
||
|
||
The backend API is **solid and well-engineered**. All core CRUD endpoints work correctly, authentication/authorization is properly enforced, Zod validation catches malformed input, and the locked-timesheet guard prevents edits to submitted/approved timesheets. The seed file is clean — no phantom entries. There are a few medium-priority issues (no future-date guard, no daily hour cap, JWT dev fallback secrets in code) and a couple of informational notes.
|
||
|
||
**Overall Grade: B+** — Production-worthy with minor hardening needed.
|
||
|
||
---
|
||
|
||
## Test Results Summary
|
||
|
||
| # | Test | Endpoint | Result | Status |
|
||
|---|------|----------|--------|--------|
|
||
| 1 | Health check | `GET /api/health` | `{"status":"ok"}` | ✅ PASS |
|
||
| 2 | Admin login | `POST /api/auth/login` | Returns JWT + user object | ✅ PASS |
|
||
| 3 | Get entries (locked week) | `GET /api/entries?week=2026-02-10` | 200, 2 entries, `isLocked:true` | ✅ PASS |
|
||
| 4 | Create entry (locked week) | `POST /api/entries` | 403 — correctly blocked | ✅ PASS |
|
||
| 5 | Create entry (open week) | `POST /api/entries` | 201 — entry created | ✅ PASS |
|
||
| 6 | Update entry | `PUT /api/entries/:id` | 200 — hours & description updated | ✅ PASS |
|
||
| 7 | Delete entry | `DELETE /api/entries/:id` | 200 — entry removed | ✅ PASS |
|
||
| 8 | Delete non-existent | `DELETE /api/entries/:id` | 404 — `Entry not found` | ✅ PASS |
|
||
| 9 | List timesheets (admin) | `GET /api/admin/timesheets` | 200, pagination working | ✅ PASS |
|
||
| 10 | Filter timesheets | `GET /api/admin/timesheets?status=submitted` | 200, empty (none submitted) | ✅ PASS |
|
||
| 11 | Reports (no filter) | `GET /api/admin/reports` | 200, summary + byUser + byHomeowner | ✅ PASS |
|
||
| 12 | Reports (date range) | `GET /api/admin/reports?from=...&to=...` | 200, filtered correctly | ✅ PASS |
|
||
| 13 | Timesheet detail | `GET /api/admin/timesheets/:id` | 200, full entry breakdown | ✅ PASS |
|
||
| 14 | Bad validation (all fields) | `POST /api/entries` bad data | 400, 4 error details | ✅ PASS |
|
||
| 15 | No auth token | `GET /api/entries` | 401 — `Access token required` | ✅ PASS |
|
||
| 16 | Invalid auth token | `GET /api/entries` | 401 — `Invalid access token` | ✅ PASS |
|
||
| 17 | Non-existent entry update | `PUT /api/entries/00000...` | 404 — `Entry not found` | ✅ PASS |
|
||
| 18 | Zero hours | `POST /api/entries` hours=0 | 400 — `Hours must be positive` | ✅ PASS |
|
||
| 19 | Over 24 hours | `POST /api/entries` hours=25 | 400 — `Hours cannot exceed 24` | ✅ PASS |
|
||
| 20 | Edit approved entry | `PUT /api/entries/:id` | 403 — correctly blocked | ✅ PASS |
|
||
| 21 | Delete approved entry | `DELETE /api/entries/:id` | 403 — correctly blocked | ✅ PASS |
|
||
| 22 | Auth me | `GET /api/auth/me` | 200, user profile | ✅ PASS |
|
||
| 23 | Timesheet view | `GET /api/timesheets?week=...` | 200, status + entries | ✅ PASS |
|
||
| 24 | Timesheet history | `GET /api/timesheets/history` | 200, list with approver | ✅ PASS |
|
||
| 25 | Multiple entries per day | `POST /api/entries` ×2 | 201 both, confirmed on GET | ✅ PASS |
|
||
| 26 | Future date accepted | `POST /api/entries` date=2026-12-25 | 201 — **no guard** | ⚠️ WARNING |
|
||
| 27 | 48h on same day | `POST /api/entries` 24h ×2 | Both created — **no cap** | ⚠️ WARNING |
|
||
| 28 | Inactive homeowner | `POST /api/entries` | 400 — `Invalid or inactive homeowner` | ✅ PASS |
|
||
| 29 | Empty body POST | `POST /api/entries` `{}` | 400, all fields required | ✅ PASS |
|
||
| 30 | Empty body PUT | `PUT /api/entries/:id` `{}` | 200, no-op update (returned unchanged) | ✅ PASS |
|
||
| 31 | SQL injection in query | `?week=2026-02-10';DROP...` | 400 — regex validation blocked | ✅ PASS |
|
||
| 32 | Refresh token flow | `POST /api/auth/refresh` | 200, new token pair | ✅ PASS |
|
||
| 33 | PDF generation | `GET /api/admin/reports/pdf` | 200, valid PDF (4090 bytes) | ✅ PASS |
|
||
| 34 | PDF missing params | `GET /api/admin/reports/pdf` no query | 400 — proper error | ✅ PASS |
|
||
| 35 | Submit timesheet | `POST /api/timesheets/submit` | 200, status=submitted, entries linked | ✅ PASS |
|
||
| 36 | Edit after submit | `PUT /api/entries/:id` | 403 — blocked | ✅ PASS |
|
||
| 37 | Edit after rejection | `PUT /api/entries/:id` | 200 — unlocked correctly | ✅ PASS |
|
||
| 38 | CORS (evil origin) | `Origin: http://evil.com` | 403 — blocked | ✅ PASS |
|
||
| 39 | Week boundary (Sunday input) | `?week=2026-02-15` (Sunday) | weekStart=2026-02-09 (correct Monday) | ✅ PASS |
|
||
| 40 | Homeowners list | `GET /api/homeowners` | 200, 47 homeowners | ✅ PASS |
|
||
| 41 | Admin users list | `GET /api/admin/users` | 200, 2 users | ✅ PASS |
|
||
|
||
**Pass: 37/41 | Warnings: 2 | Informational: 2**
|
||
|
||
---
|
||
|
||
## Seed.js Analysis — No Phantom Entries
|
||
|
||
**File:** `backend/prisma/seed.js`
|
||
|
||
The seed script creates **only**:
|
||
1. One admin user (`admin@coastal.com`, role `super_admin`)
|
||
2. 47 homeowners (using `upsert` — idempotent)
|
||
|
||
**It does NOT create any `TimeEntry`, `Timesheet`, or `TimesheetEntry` records.**
|
||
|
||
The 2 entries found in the database (date `2026-02-09` "Monday work" and `2026-02-10` "Tuesday work") were created **after seeding** — their `createdAt` timestamps (`2026-02-15T19:17:44.997Z`) are well after seed time, and they were created programmatically (likely during prior testing/demo setup). They are not phantom entries from the seed.
|
||
|
||
**Verdict: Seed is clean. ✅**
|
||
|
||
---
|
||
|
||
## Issues Found
|
||
|
||
### 🟡 MEDIUM: No Future Date Validation (entries.js)
|
||
|
||
**Impact:** Users can create timesheet entries for any date in the future (tested: 2026-12-25, 10 months out). This allows pre-filling timesheets for work not yet performed.
|
||
|
||
**Location:** `POST /api/entries` and `PUT /api/entries/:id` in `backend/src/routes/entries.js`
|
||
|
||
**Recommendation:** Add server-side validation in `createEntrySchema`:
|
||
```js
|
||
// In validation.js — add to createEntrySchema
|
||
date: z.string()
|
||
.regex(/^\d{4}-\d{2}-\d{2}$/, 'Date must be YYYY-MM-DD')
|
||
.refine(val => new Date(val + 'T00:00:00Z') <= new Date(), {
|
||
message: 'Cannot create entries for future dates',
|
||
}),
|
||
```
|
||
Or allow up to end of current week only.
|
||
|
||
---
|
||
|
||
### 🟡 MEDIUM: No Per-Day Total Hour Cap (entries.js)
|
||
|
||
**Impact:** A user can add unlimited entries per day with up to 24 hours each. Two entries at 24h each = 48h on one day. No server-side aggregate check.
|
||
|
||
**Location:** `POST /api/entries` in `backend/src/routes/entries.js`
|
||
|
||
**Recommendation:** Before creating an entry, sum existing hours for that user + date. Reject if `existingHours + newEntry.hoursWorked > 24`:
|
||
```js
|
||
const existing = await req.prisma.timeEntry.aggregate({
|
||
where: { userId: req.user.id, date: new Date(date + 'T00:00:00Z') },
|
||
_sum: { hoursWorked: true },
|
||
});
|
||
const totalForDay = parseFloat(existing._sum.hoursWorked || 0) + hoursWorked;
|
||
if (totalForDay > 24) {
|
||
return res.status(400).json({
|
||
error: `Total hours for this day would be ${totalForDay}. Maximum is 24.`,
|
||
});
|
||
}
|
||
```
|
||
|
||
---
|
||
|
||
### 🟢 LOW: JWT Dev Fallback Secrets in Code (auth.js)
|
||
|
||
**Impact:** If `JWT_SECRET` / `JWT_REFRESH_SECRET` env vars are missing, the code falls back to hardcoded strings (`'dev-jwt-secret-change-me'`). In production Docker, these ARE set via env vars (confirmed), so this is not an active risk — but the fallback should ideally throw an error in production instead of silently using a weak secret.
|
||
|
||
**Location:** `backend/src/middleware/auth.js`, lines 3-4
|
||
|
||
**Current:**
|
||
```js
|
||
const JWT_SECRET = process.env.JWT_SECRET || 'dev-jwt-secret-change-me';
|
||
```
|
||
|
||
**Recommended:**
|
||
```js
|
||
const JWT_SECRET = process.env.JWT_SECRET || (() => {
|
||
if (process.env.NODE_ENV === 'production') throw new Error('JWT_SECRET required');
|
||
return 'dev-jwt-secret-change-me';
|
||
})();
|
||
```
|
||
|
||
---
|
||
|
||
### 🟢 LOW: Refresh Token Reuse Detection Race Condition
|
||
|
||
**Observation:** The auth code implements refresh token rotation (good), but rapid successive calls can bypass the reuse detection. This is a known limitation of simple token rotation and not a critical flaw — in practice, an attacker would need to intercept the token in the exact window.
|
||
|
||
**Mitigation (if desired):** Store a token family ID and invalidate all tokens in the family on reuse detection.
|
||
|
||
---
|
||
|
||
### ℹ️ INFO: `PUT /api/entries/:id` with Empty Body Returns 200
|
||
|
||
Sending `{}` to the update endpoint returns 200 with the entry unchanged. This is technically correct (no fields to update → no-op) but could confuse clients expecting a 400. The `updateEntrySchema` has all fields optional, which is the intended design for partial updates.
|
||
|
||
**Verdict:** Acceptable behavior. No action needed.
|
||
|
||
---
|
||
|
||
### ℹ️ INFO: Timesheet History `totalHours` is Always `null`
|
||
|
||
**Location:** `GET /api/timesheets/history` in `backend/src/routes/timesheets.js`
|
||
|
||
The history response includes `totalHours: null` with a comment `// Could aggregate if needed`. The frontend may need this for displaying summaries.
|
||
|
||
**Recommendation:** Either aggregate hours or remove the field to avoid confusion:
|
||
```js
|
||
// Option A: populate it
|
||
const entryLinks = await prisma.timesheetEntry.findMany({
|
||
where: { timesheetId: ts.id },
|
||
include: { timeEntry: { select: { hoursWorked: true } } },
|
||
});
|
||
const totalHours = entryLinks.reduce((s, l) => s + parseFloat(l.timeEntry.hoursWorked), 0);
|
||
```
|
||
|
||
---
|
||
|
||
## What's Working Well
|
||
|
||
1. **Zod Validation** — All input is validated with clear error messages. Regex on dates, UUID on IDs, ranges on hours. SQL injection is impossible through validated paths.
|
||
|
||
2. **Timesheet Locking** — The submit → lock → reject → unlock flow works perfectly. Entries in submitted/approved weeks are properly guarded on all mutation endpoints (POST, PUT, DELETE).
|
||
|
||
3. **Authentication & Authorization** — Bearer token auth with proper 401/403 distinctions. Admin routes require `admin` or `super_admin` role. Token refresh with rotation is implemented.
|
||
|
||
4. **CORS** — Properly configured allowlist. `Origin: evil.com` gets 403. Curl (no origin) is allowed for API tooling.
|
||
|
||
5. **Week Calculation** — `getWeekMonday()` correctly uses UTC throughout. Sunday input (2026-02-15) correctly resolves to Monday (2026-02-09). No timezone drift bugs in the backend.
|
||
|
||
6. **Multiple Entries Per Day** — Fully supported. No unique constraint on (userId, date, homeownerId), allowing multiple jobs per homeowner or multiple homeowners per day.
|
||
|
||
7. **Inactive Homeowner Guard** — Creating entries for deactivated homeowners is properly rejected with 400.
|
||
|
||
8. **PDF Generation** — Works end-to-end. Returns valid PDF with correct content-type headers.
|
||
|
||
9. **Pagination** — Admin timesheets endpoint supports `page` + `limit` with `total` and `totalPages` in response.
|
||
|
||
10. **Helmet + Security Headers** — `X-Frame-Options`, `X-Content-Type-Options`, `X-XSS-Protection` all set via nginx.
|
||
|
||
---
|
||
|
||
## Architecture Notes
|
||
|
||
| Layer | Tech | Notes |
|
||
|-------|------|-------|
|
||
| Runtime | Node.js v22.22 | Inside Docker |
|
||
| Framework | Express.js | With helmet, cors, rate-limit |
|
||
| ORM | Prisma | PostgreSQL 16 Alpine |
|
||
| Validation | Zod | Middleware pattern (`validateBody`, `validateQuery`) |
|
||
| Auth | JWT (HS256) | 15m access + 7d refresh, rotation |
|
||
| Rate Limit | express-rate-limit | 5 attempts / 15 min on auth |
|
||
| Proxy | Nginx | `/api/*` → backend, `/*` → frontend SPA |
|
||
| PDF | Custom (pdf.js) | Generates valid PDF documents |
|
||
|
||
---
|
||
|
||
## Relationship to AUDIT-TASK.md Bugs
|
||
|
||
### Bug 1 (Timezone wrong day names) — **Backend is NOT affected**
|
||
The backend uses `'T00:00:00Z'` (UTC) consistently in `getWeekMonday()` and all date construction. The timezone bug is **frontend-only** (using `new Date("YYYY-MM-DD")` which parses as UTC midnight, then `getDay()` in local timezone).
|
||
|
||
### Bug 2 (Pre-filled entries can't be edited) — **Backend works correctly**
|
||
- Unlocked entries: PUT/DELETE both work ✅
|
||
- Locked entries: PUT/DELETE properly return 403 ✅
|
||
- After rejection: entries become editable again ✅
|
||
|
||
The "can't edit" issue is likely **frontend** — the `isLocked` / `disabled` logic in `DayCard.jsx` or `Timesheet.jsx` may not be checking the right condition, or may not be updating state after rejection.
|
||
|
||
### Bug 3 (Week label mismatch) — **Backend returns correct data**
|
||
`weekStart: 2026-02-09` (Monday) and `weekEnd: 2026-02-15` (Sunday) are correct. The mismatch is a **frontend rendering issue** in `WeekNavigator.jsx`.
|
||
|
||
---
|
||
|
||
## Recommended Priority Actions
|
||
|
||
1. **Add future-date guard** — 30 min fix in `validation.js` (🟡 MEDIUM)
|
||
2. **Add per-day hour cap** — 1 hour fix in `entries.js` (🟡 MEDIUM)
|
||
3. **Populate `totalHours` in history** — 30 min fix in `timesheets.js` (ℹ️ LOW)
|
||
4. **Fail-fast on missing JWT secrets in production** — 10 min fix in `auth.js` (🟢 LOW)
|