diff --git a/apps/journal/app/lib/api-guard.server.test.ts b/apps/journal/app/lib/api-guard.server.test.ts new file mode 100644 index 0000000..695086b --- /dev/null +++ b/apps/journal/app/lib/api-guard.server.test.ts @@ -0,0 +1,70 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { TERMS_VERSION } from "./legal"; + +const mockGetAuthenticatedUser = vi.fn(); + +vi.mock("./oauth.server.ts", () => ({ + getAuthenticatedUser: mockGetAuthenticatedUser, +})); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("requireApiUser", () => { + it("returns 401 when unauthenticated", async () => { + mockGetAuthenticatedUser.mockResolvedValue(null); + const { requireApiUser } = await import("./api-guard.server.ts"); + + try { + await requireApiUser(new Request("http://localhost/api/v1/routes")); + expect.fail("should throw"); + } catch (err) { + expect(err).toBeInstanceOf(Response); + expect((err as Response).status).toBe(401); + const body = await (err as Response).json(); + expect(body.code).toBe("UNAUTHORIZED"); + } + }); + + it("returns the user when termsVersion matches", async () => { + const user = { id: "u1", termsVersion: TERMS_VERSION }; + mockGetAuthenticatedUser.mockResolvedValue(user); + const { requireApiUser } = await import("./api-guard.server.ts"); + + const result = await requireApiUser(new Request("http://localhost/api/v1/routes")); + expect(result).toBe(user); + }); + + it("returns 403 TERMS_OUTDATED when termsVersion is stale", async () => { + mockGetAuthenticatedUser.mockResolvedValue({ id: "u1", termsVersion: "2020-01-01" }); + const { requireApiUser } = await import("./api-guard.server.ts"); + + try { + await requireApiUser(new Request("http://localhost/api/v1/routes")); + expect.fail("should throw"); + } catch (err) { + expect(err).toBeInstanceOf(Response); + expect((err as Response).status).toBe(403); + const body = await (err as Response).json(); + expect(body.code).toBe("TERMS_OUTDATED"); + expect(body.currentTermsVersion).toBe(TERMS_VERSION); + } + }); + + it("returns 403 TERMS_OUTDATED when termsVersion is null", async () => { + mockGetAuthenticatedUser.mockResolvedValue({ id: "u1", termsVersion: null }); + const { requireApiUser } = await import("./api-guard.server.ts"); + + try { + await requireApiUser(new Request("http://localhost/api/v1/routes")); + expect.fail("should throw"); + } catch (err) { + expect(err).toBeInstanceOf(Response); + expect((err as Response).status).toBe(403); + const body = await (err as Response).json(); + expect(body.code).toBe("TERMS_OUTDATED"); + expect(body.currentTermsVersion).toBe(TERMS_VERSION); + } + }); +}); diff --git a/apps/journal/app/lib/api-guard.server.ts b/apps/journal/app/lib/api-guard.server.ts index bf480b2..18c6be9 100644 --- a/apps/journal/app/lib/api-guard.server.ts +++ b/apps/journal/app/lib/api-guard.server.ts @@ -1,8 +1,13 @@ import { getAuthenticatedUser } from "./oauth.server.ts"; +import { TERMS_VERSION } from "./legal.ts"; import { ERROR_CODES } from "@trails-cool/api"; /** - * Require authentication for an API route. Returns the user or throws a 401 Response. + * Require authentication for an API route. Returns the user or throws a + * Response: 401 if unauthenticated, 403 with `TERMS_OUTDATED` if the user's + * stored `terms_version` is missing or stale relative to the current + * `TERMS_VERSION`. Mirrors the cookie-session terms gate enforced by the + * root loader, so bearer-token API traffic can't bypass it. */ export async function requireApiUser(request: Request) { const user = await getAuthenticatedUser(request); @@ -12,6 +17,16 @@ export async function requireApiUser(request: Request) { { status: 401 }, ); } + if (user.termsVersion !== TERMS_VERSION) { + throw Response.json( + { + error: "Terms of Service have been updated and must be re-accepted", + code: ERROR_CODES.TERMS_OUTDATED, + currentTermsVersion: TERMS_VERSION, + }, + { status: 403 }, + ); + } return user; } diff --git a/apps/journal/app/routes/api.v1.routes.test.ts b/apps/journal/app/routes/api.v1.routes.test.ts index a8c4664..b3c6afb 100644 --- a/apps/journal/app/routes/api.v1.routes.test.ts +++ b/apps/journal/app/routes/api.v1.routes.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -const mockUser = { id: "user-1", email: "test@test.com", username: "test", domain: "localhost", displayName: null, bio: null, createdAt: new Date() }; +import { TERMS_VERSION } from "~/lib/legal"; + +const mockUser = { id: "user-1", email: "test@test.com", username: "test", domain: "localhost", displayName: null, bio: null, createdAt: new Date(), termsVersion: TERMS_VERSION }; const mockGetAuthenticatedUser = vi.fn(); const mockListRoutes = vi.fn(); const mockCreateRoute = vi.fn(); diff --git a/openspec/changes/mobile-terms-gate/proposal.md b/openspec/changes/mobile-terms-gate/proposal.md new file mode 100644 index 0000000..b6e49e7 --- /dev/null +++ b/openspec/changes/mobile-terms-gate/proposal.md @@ -0,0 +1,29 @@ +## Why + +Mobile API requests authenticated via OAuth2 bearer tokens currently bypass the Terms gate. Web users with a stale `users.terms_version` are redirected to `/auth/accept-terms` by the journal root loader (`apps/journal/app/root.tsx:58`), but mobile clients hitting `/api/v1/*` with the same stale version get a normal 200 response. The Terms gate (defined in `journal-auth/spec.md`) is meant to apply to **all** authenticated requests, not only cookie-session traffic. + +## What Changes + +- Extend the bearer-token API chokepoint (`requireApiUser` in `apps/journal/app/lib/api-guard.server.ts`) to compare the authenticated user's `termsVersion` against the current `TERMS_VERSION`. On mismatch, return a structured **HTTP 403** with `{ error, code: "TERMS_OUTDATED", currentTermsVersion }` so the mobile app can surface its own UI (or open the web Terms page in a webview). +- Add `TERMS_OUTDATED` to `packages/api/src/errors.ts` `ERROR_CODES` so clients have a stable code to switch on. +- Spec delta on `journal-auth`: add a requirement that the Terms gate also applies to bearer-token API auth, with the structured-error contract as a scenario. + +Web cookie-session behaviour is unchanged (still a redirect via the root loader). Only the API chokepoint changes. + +## Capabilities + +### New Capabilities + +(none) + +### Modified Capabilities + +- `journal-auth`: adds a requirement that the Terms-version gate applies to bearer-token API requests, returning HTTP 403 with a structured `terms_outdated` payload instead of a redirect. + +## Impact + +- **Code**: one function (`requireApiUser`) gains a terms check; one constant added to `@trails-cool/api`. +- **Clients**: the mobile app needs to handle `403 { code: "TERMS_OUTDATED" }` (out of scope for this change — tracked separately on the mobile side). +- **Tests**: unit test for `requireApiUser` covering stale / current / null `termsVersion`. +- **Specs**: 1 spec delta on `journal-auth` (added requirement). +- **Out of scope**: mobile client UI for the new error; any change to web cookie-session behaviour. diff --git a/openspec/changes/mobile-terms-gate/specs/journal-auth/spec.md b/openspec/changes/mobile-terms-gate/specs/journal-auth/spec.md new file mode 100644 index 0000000..7458598 --- /dev/null +++ b/openspec/changes/mobile-terms-gate/specs/journal-auth/spec.md @@ -0,0 +1,17 @@ +## ADDED Requirements + +### Requirement: Terms gate applies to bearer-token API requests +Authenticated API requests carrying an OAuth2 bearer token SHALL be subject to the same Terms-version gate as cookie-session requests. When the authenticated user's stored `terms_version` is NULL or differs from the current `TERMS_VERSION`, the API chokepoint (`requireApiUser`) SHALL reject the request with HTTP 403 and a structured JSON body `{ error, code: "TERMS_OUTDATED", currentTermsVersion: }` instead of returning the resource. Cookie-session web traffic continues to be handled by the root-loader redirect to `/auth/accept-terms`; only the API path uses the structured 403. + +#### Scenario: Stale bearer token receives structured 403 +- **WHEN** a request to `/api/v1/*` arrives with a valid bearer token whose user has `terms_version` NULL or different from the current `TERMS_VERSION` +- **THEN** the server responds with HTTP 403 and JSON `{ error: , code: "TERMS_OUTDATED", currentTermsVersion: }` +- **AND** the requested resource is not returned + +#### Scenario: Up-to-date bearer token is allowed through +- **WHEN** a request to `/api/v1/*` arrives with a valid bearer token whose user's `terms_version` matches the current `TERMS_VERSION` +- **THEN** the request proceeds normally + +#### Scenario: Anonymous API request still 401 +- **WHEN** a request to `/api/v1/*` arrives without a bearer token (or with an invalid one) +- **THEN** the server responds with HTTP 401 `UNAUTHORIZED` as before — the Terms check only applies after authentication succeeds diff --git a/openspec/changes/mobile-terms-gate/tasks.md b/openspec/changes/mobile-terms-gate/tasks.md new file mode 100644 index 0000000..f8328ab --- /dev/null +++ b/openspec/changes/mobile-terms-gate/tasks.md @@ -0,0 +1,13 @@ +## 1. Implementation + +- [x] 1.1 Add `TERMS_OUTDATED: "TERMS_OUTDATED"` to `ERROR_CODES` in `packages/api/src/errors.ts`. +- [x] 1.2 Extend `requireApiUser` in `apps/journal/app/lib/api-guard.server.ts` to compare the authenticated user's `termsVersion` to `TERMS_VERSION` (from `~/lib/legal`). On mismatch (or NULL), throw a `Response` with status 403 and JSON body `{ error, code: ERROR_CODES.TERMS_OUTDATED, currentTermsVersion: TERMS_VERSION }`. + +## 2. Tests + +- [x] 2.1 Add `apps/journal/app/lib/api-guard.server.test.ts` covering: stale `termsVersion` → 403 with `TERMS_OUTDATED` + `currentTermsVersion`; matching `termsVersion` → returns user; null `termsVersion` → 403; no auth → 401 (unchanged). + +## 3. Verification + +- [x] 3.1 Run `pnpm typecheck && pnpm lint && pnpm test` — all green. +- [ ] 3.2 Open draft PR against `main`. diff --git a/packages/api/src/errors.ts b/packages/api/src/errors.ts index 381dd95..5152b95 100644 --- a/packages/api/src/errors.ts +++ b/packages/api/src/errors.ts @@ -35,4 +35,5 @@ export const ERROR_CODES = { CONFLICT: "CONFLICT", RATE_LIMITED: "RATE_LIMITED", INTERNAL_ERROR: "INTERNAL_ERROR", + TERMS_OUTDATED: "TERMS_OUTDATED", } as const;