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) <noreply@anthropic.com>
This commit is contained in:
Ullrich Schäfer 2026-05-08 01:59:28 +02:00
parent eee5689a75
commit 067e2ebd0b
No known key found for this signature in database
GPG key ID: A32FF691A0F752D9
7 changed files with 149 additions and 2 deletions

View file

@ -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);
}
});
});

View file

@ -1,8 +1,13 @@
import { getAuthenticatedUser } from "./oauth.server.ts"; import { getAuthenticatedUser } from "./oauth.server.ts";
import { TERMS_VERSION } from "./legal.ts";
import { ERROR_CODES } from "@trails-cool/api"; 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) { export async function requireApiUser(request: Request) {
const user = await getAuthenticatedUser(request); const user = await getAuthenticatedUser(request);
@ -12,6 +17,16 @@ export async function requireApiUser(request: Request) {
{ status: 401 }, { 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; return user;
} }

View file

@ -1,6 +1,8 @@
import { describe, it, expect, vi, beforeEach } from "vitest"; 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 mockGetAuthenticatedUser = vi.fn();
const mockListRoutes = vi.fn(); const mockListRoutes = vi.fn();
const mockCreateRoute = vi.fn(); const mockCreateRoute = vi.fn();

View file

@ -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.

View file

@ -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: <current> }` 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: <message>, code: "TERMS_OUTDATED", currentTermsVersion: <current> }`
- **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

View file

@ -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`.

View file

@ -35,4 +35,5 @@ export const ERROR_CODES = {
CONFLICT: "CONFLICT", CONFLICT: "CONFLICT",
RATE_LIMITED: "RATE_LIMITED", RATE_LIMITED: "RATE_LIMITED",
INTERNAL_ERROR: "INTERNAL_ERROR", INTERNAL_ERROR: "INTERNAL_ERROR",
TERMS_OUTDATED: "TERMS_OUTDATED",
} as const; } as const;