Files
classroomio/.cursor/plans/public_api_tests_89047418.plan.md
T

220 lines
14 KiB
Markdown
Raw Normal View History

---
name: Public API Tests
overview: Add a Vitest suite for `/public-api/v1` with explicit tests that Bearer API keys, scopes, and org context are enforced, plus strict JSON assertions on success and error responses. DB and external I/O stay mocked; one small Hono app uses real middleware with a mocked auth service to prove the full chain.
todos:
- id: vitest-config
content: Update vitest.config.ts with @api path alias
status: pending
- id: courses-routes
content: Create courses route tests with strict response body assertions
status: pending
- id: audience-routes
content: Create audience route tests with strict response body assertions
status: pending
- id: auth-chain
content: Add public-api-auth-chain tests (real middleware + mocked authenticate + mocked services)
status: pending
- id: automation-key-mw
content: Create automation-key middleware tests (Bearer + touch + JSON error shape)
status: pending
- id: automation-scopes-mw
content: Create automation-key-scopes tests (403 without public_api:*, JSON shape)
status: pending
- id: course-service
content: Create v1 course service unit tests (returns + mock call args)
status: pending
- id: audience-service
content: Create v1 audience service unit tests (returns + mock call args)
status: pending
isProject: false
---
# Public API Test Suite
## Architecture
Three layers of tests, each mocking only the layer below it.
```mermaid
flowchart TD
RouteTests["Route Tests\n(mock middleware + services)"]
MiddlewareTests["Middleware Tests\n(mock DB/auth-key service)"]
ServiceTests["Service Tests\n(mock DB queries)"]
RouteTests --> MiddlewareTests
RouteTests --> ServiceTests
MiddlewareTests --> ServiceTests
```
## Coverage vs codebase (audit)
In this repository, **public API** means the org automation key surface at `/public-api/v1` ([apps/api/src/app.ts](apps/api/src/app.ts)). Nothing else is mounted there; [apps/api/src/routes/v1/index.ts](apps/api/src/routes/v1/index.ts) only wires `/audience` and `/courses`. Other Hono routes (for example `/organization`, `/course`) use session or dashboard flows and are out of scope for this plan.
Full endpoint inventory the plan must exercise (paths relative to `/public-api/v1`):
- `GET /courses`, `POST /courses`
- `GET /courses/:courseId/students`, `GET /courses/:courseId/export`, `GET /courses/:courseId/structure`, `PUT /courses/:courseId/structure`
- `GET /courses/:courseId`, `PUT /courses/:courseId`, `DELETE /courses/:courseId`
- `GET /audience`, `POST /audience`, `POST /audience/assign-courses`
- `GET /audience/:memberId`, `PUT /audience/:memberId`, `DELETE /audience/:memberId`
Validation for these lives under [packages/utils/src/validation/public-api/](packages/utils/src/validation/public-api/) (`course.ts`, `audience.ts` only). If new routers are added under `v1/`, extend this inventory and add matching test files.
## Auth, API keys, and middleware enforcement
Public endpoints must never rely on “trust the client.” Tests must prove the following, aligned with [apps/api/src/middlewares/automation-key.ts](apps/api/src/middlewares/automation-key.ts) and [apps/api/src/middlewares/automation-key-scopes.ts](apps/api/src/middlewares/automation-key-scopes.ts).
**Bearer and API key validation (`automationKeyMiddleware`)**
- Missing `Authorization` → **401**, body `success: false`, stable `code` (e.g. `UNAUTHORIZED`).
- Header present but not `Bearer <token>` → **401** (same shape).
- Empty or whitespace-only token after `Bearer` → **401** (authenticate fails).
- `authenticateOrganizationApiKeyService` throws (invalid / unknown key) → **401**, message surfaced from middleware catch path.
- Successful auth → `c.set('orgId', …)`, `c.set('actorId', …)`, `c.set('automationKey', …)`; assert `touchOrganizationApiKeyLastUsedService` is invoked with the resolved key (proves real keys go through the “touch” path). Expiry is enforced inside `authenticateOrganizationApiKeyService` ([apps/api/src/services/organization/automation-key.ts](apps/api/src/services/organization/automation-key.ts)) — add a mock case where the service throws for an expired key and expect **401**.
**Scope enforcement (`automationKeyScopesMiddleware(['public_api:*'])`)**
- No `automationKey` in context (simulating a bug or bypass) → **401**.
- Key present but `scopes` omit `public_api:*` (e.g. only MCP scopes) → **403**, body includes `code: FORBIDDEN` and a clear error string.
- Key includes `public_api:*` → `next()` runs and the handler can execute.
**Full chain (no mocked middleware on the router)**
Route files that mock middleware cannot prove enforcement by themselves. Add **[apps/api/src/routes/v1/__tests__/public-api-auth-chain.test.ts](apps/api/src/routes/v1/__tests__/public-api-auth-chain.test.ts)**:
- Build a small Hono app that mounts a real v1 router (e.g. `v1CoursesRouter` under `/courses`) with **unmocked** `automationKeyMiddleware` and `automationKeyScopesMiddleware`.
- Mock only `@api/services/organization/automation-key` (`authenticateOrganizationApiKeyService`, `touchOrganizationApiKeyLastUsedService`) and the v1 service module.
- Cases: no `Authorization` → 401; `Bearer` + key that authenticates to a record **without** `public_api:*` → 403; `Bearer` + key with `public_api:*` → 200 and JSON matches the mocked service return (see next section).
- Optionally repeat one path on `v1AudienceRouter` so audience is covered by the same guarantees.
**Service-level actor rules**
Several handlers require `actorId` from the key’s `createdByProfileId`. With middleware mocked to `actorId: null`, assert **401** and `AppError` / `handleError` shape for: `POST /courses`, `PUT /courses/:courseId/structure`, `POST /audience`, `PUT /audience/:memberId`. This documents that automation keys without a creator profile cannot perform those mutations.
## Response contracts (data must match expectations)
Do not only assert HTTP status. Parse `await response.json()` and validate **shape and values**.
**Success responses**
- Every happy path: `expect(body.success).toBe(true)`.
- `GET/POST` course list/detail: `data` must **deep-equal** (or `toMatchObject` with explicit fields) the object/array returned by the mocked service — proves the handler forwards the service result, not an accidental empty object or wrong key.
- `GET /audience`: assert `data`, `pagination`, and `query` exist and match the mocked `listAudienceService` return (same values the service produced).
- `POST` create paths: assert **201** where applicable and `data` matches the created entity mock.
- Where services return nested structures (course export / structure), assert at least stable top-level keys and one nested field so refactors do not silently change the contract.
**Error responses**
- Middleware errors: `success: false`, string `error`, string `code` — match actual payloads from middleware (401/403).
- `handleError` / `AppError` paths: `success: false`, `error` message, `code` matching `ErrorCodes.*`, optional `field` when the route sets it. Assert status equals `AppError.statusCode` (404, 401, etc.).
- Validation failures (Zod / hono-openapi): assert **4xx** and that the body documents validation failure in the form the stack returns (status + JSON); do not hand-wave as “400” without reading the response.
**Service unit tests**
- When a function maps or merges arguments (e.g. `createPublicApiCourseService` adds `organizationId`), assert the **exact** arguments passed into the downstream mock (`toHaveBeenCalledWith`).
## Files to Change
### 1. `[apps/api/vitest.config.ts](apps/api/vitest.config.ts)`
Add `resolve.alias` so Vitest resolves `@api/*` without needing `tsc-alias` at runtime:
```ts
import { resolve } from 'path';
export default defineConfig({
resolve: { alias: { '@api': resolve(__dirname, 'src') } },
test: { globals: true, environment: 'node', ... }
});
```
(`@cio/*` packages already resolve from `node_modules` at test time.)
### 2. Route tests (mocked middleware — fast handler + validation coverage)
These files focus on **validators**, **handler wiring**, and **error mapping** with middleware stubbed so `orgId` / `actorId` are deterministic.
`**[apps/api/src/routes/v1/__tests__/courses.test.ts](apps/api/src/routes/v1/__tests__/courses.test.ts)**`
- `vi.mock` both middleware modules to inject `orgId`/`actorId` and call `next()`.
- `vi.mock('@api/services/v1/course')` — all service functions as `vi.fn()`.
- One `describe` block per endpoint. For **each** success case: set the mock return to a **concrete** object/array, perform the request, `const body = await res.json()`, then `expect(body).toEqual({ success: true, data: … })` (or `toMatchObject` only where the payload is huge and you document which fields are guaranteed).
- Cases (plus response assertions as above):
- `GET /` — 200; service throws `AppError` → assert status + `{ success: false, code, error }`
- `POST /` — 201 + body matches mock; invalid body → validation status + body; middleware mock with `actorId` undefined/null → 401 + error contract
- `GET /:courseId` — 200 + `data` equals `getCourseService` mock; non-UUID param → 4xx; `AppError` 404 → 404 + `COURSE_NOT_FOUND` (or whatever the service throws)
- `PUT /:courseId`, `DELETE /:courseId` — same pattern
- `GET …/students`, `export`, `structure` — 200 + `data` equals mock; structure route shares export service — assert same mock used if both tested
- `PUT …/structure` — 200 + merged result shape; no `actorId` → 401; invalid JSON → validation error
- Use `v1CoursesRouter.request('/path', { headers, body })`.
`**[apps/api/src/routes/v1/__tests__/audience.test.ts](apps/api/src/routes/v1/__tests__/audience.test.ts)**`
- Same middleware mocks as courses; mock `@api/services/v1/audience`.
- Assert full list payload: `success`, `data`, `pagination`, `query` **equal** mocked `listAudienceService` output fields.
- Other endpoints: same success/error JSON rules as courses.
### 2b. Auth chain tests (real middleware)
`**[apps/api/src/routes/v1/__tests__/public-api-auth-chain.test.ts](apps/api/src/routes/v1/__tests__/public-api-auth-chain.test.ts)**`
- Do **not** mock `automationKeyMiddleware` / `automationKeyScopesMiddleware`.
- Mock `authenticateOrganizationApiKeyService` to return a full key-shaped object (`organizationId`, `createdByProfileId`, `scopes`, `id`, etc. — whatever the middleware reads).
- Mount router under a prefix so paths match production (`/courses` + `GET /` → list).
- Prove **401** without header, **403** when `scopes` is `[]` or MCP-only, **200** when `scopes` includes `public_api:*` and `data` matches mocked `listCoursesService` (or one audience list call).
### 3. Middleware tests
`**[apps/api/src/middlewares/__tests__/automation-key.test.ts](apps/api/src/middlewares/__tests__/automation-key.test.ts)**`
- `vi.mock('@api/services/organization/automation-key')` — mock `authenticateOrganizationApiKeyService` and `touchOrganizationApiKeyLastUsedService`.
- Create a minimal Hono app that mounts the real `automationKeyMiddleware` + a `GET /` handler that returns context values.
- Cases (each: `await res.json()` and assert `success`, `code`, `error` where applicable):
- No `Authorization` header → 401 + `success: false` + `code: UNAUTHORIZED`
- Header not `Bearer …` → 401 + same JSON shape
- `authenticateOrganizationApiKeyService` throws (invalid or expired key; service throws before returning) → 401 + body matches middleware catch behavior
- Valid key → 200; response reflects `orgId`, `actorId`, `automationKey` from context (echo in test handler)
- `touchOrganizationApiKeyLastUsedService` called exactly once with the authenticated key
`**[apps/api/src/middlewares/__tests__/automation-key-scopes.test.ts](apps/api/src/middlewares/__tests__/automation-key-scopes.test.ts)**`
- No DB mocking needed; test `organizationApiKeyHasScopes` logic via the scope middleware itself.
- Cases (parse JSON on 401/403):
- `automationKey` not set in context → 401 + `success: false`
- Key with `scopes: []` or without `public_api:*` → 403 + `code: FORBIDDEN` + error message string
- Key with `public_api:*` → handler runs; response body proves `next()` was reached
### 4. Service unit tests
For every exported function, assert **return value** equals the expected object (or key fields) after the mocked dependency returns — not only “did not throw.” Use `expect(fn).toHaveBeenCalledWith(...)` for argument contracts.
`**[apps/api/src/services/v1/__tests__/course.test.ts](apps/api/src/services/v1/__tests__/course.test.ts)**`
Mock: `@cio/db/queries/tag` (`getCourseOrganizationId`), `@api/services/organization` (`getOrganizationCourses`), `@api/services/course/course` (`getCourse`, `updateCourse`, `deleteCourse`, `createCourse`), `@api/services/course/people` (`listCourseMembers`), course-import services.
- `assertCourseBelongsToOrg` helper: course not found → throws `AppError(404, COURSE_NOT_FOUND)`; org mismatch → same
- `listCoursesService` — returns `items` from `getOrganizationCourses`
- `getCourseService` — happy path; unauthorized org
- `listCourseStudentsService` — filters by `ROLE.STUDENT`
- `exportCourseService` — delegates to `getCourseImportStructureService`
- `createPublicApiCourseService` — no `actorId` → 401; calls `createCourse` with `organizationId` merged in
- `updatePublicApiCourseService` — calls `updateCourseService`
- `deletePublicApiCourseService` — calls `deleteCourseService`
- `updatePublicApiCourseStructureService` — no `actorId` → 401; creates draft then publishes
`**[apps/api/src/services/v1/__tests__/audience.test.ts](apps/api/src/services/v1/__tests__/audience.test.ts)**`
Mock: `@api/services/organization` (`getOrgAudience`, `getOrgAudienceMember`, `removeAudienceMember`), `@api/services/organization/audience` (`assignAudienceToCourses`, `importAudienceMembers`, `updatePendingAudienceMemberEmail`).
- `listAudienceService` — delegates to `getOrgAudience`
- `createAudienceService` — no `actorId` → 401; maps payload fields correctly
- `getAudienceMemberService` — passes through
- `assignAudienceCoursesService` — passes through
- `removeAudienceService` — passes through; `actorId` null defaults to `undefined`
- `updateAudienceService` — no `actorId` → 401; calls update then re-fetches member
## Total: 8 new files, 1 updated file