From 067e2ebd0b14a2897a74c56868bf9134803e0fb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Fri, 8 May 2026 01:59:28 +0200 Subject: [PATCH] fix: enforce Terms gate on bearer-token API requests Mobile API requests authenticated via OAuth2 bearer tokens bypassed the Terms gate that the root loader applies to web cookie sessions. Extend requireApiUser to compare the user's termsVersion with TERMS_VERSION and return a structured 403 { code: "TERMS_OUTDATED", currentTermsVersion } on mismatch so mobile clients can surface their own re-acceptance UI. Spec delta on journal-auth captures the new requirement. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/journal/app/lib/api-guard.server.test.ts | 70 +++++++++++++++++++ apps/journal/app/lib/api-guard.server.ts | 17 ++++- apps/journal/app/routes/api.v1.routes.test.ts | 4 +- .../changes/mobile-terms-gate/proposal.md | 29 ++++++++ .../specs/journal-auth/spec.md | 17 +++++ openspec/changes/mobile-terms-gate/tasks.md | 13 ++++ packages/api/src/errors.ts | 1 + 7 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 apps/journal/app/lib/api-guard.server.test.ts create mode 100644 openspec/changes/mobile-terms-gate/proposal.md create mode 100644 openspec/changes/mobile-terms-gate/specs/journal-auth/spec.md create mode 100644 openspec/changes/mobile-terms-gate/tasks.md 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;