Merge pull request #366 from trails-cool/fix/mobile-terms-gate
fix: enforce Terms gate on bearer-token API requests
This commit is contained in:
commit
1a0a212139
8 changed files with 165 additions and 2 deletions
70
apps/journal/app/lib/api-guard.server.test.ts
Normal file
70
apps/journal/app/lib/api-guard.server.test.ts
Normal 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);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
@ -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
|
||||
|
|
@ -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.
|
||||
- [x] 3.2 Open draft PR against `main` (#366).
|
||||
|
|
@ -59,3 +59,19 @@ Logged-in users whose stored `terms_version` does not match the currently-publis
|
|||
#### Scenario: returnTo is restricted to same-origin paths
|
||||
- **WHEN** a `returnTo` value is not a same-origin absolute path (missing leading `/`, or starting with `//`)
|
||||
- **THEN** the server redirects to `/` instead, preventing open-redirect abuse
|
||||
|
||||
### 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
|
||||
|
|
|
|||
|
|
@ -35,4 +35,5 @@ export const ERROR_CODES = {
|
|||
CONFLICT: "CONFLICT",
|
||||
RATE_LIMITED: "RATE_LIMITED",
|
||||
INTERNAL_ERROR: "INTERNAL_ERROR",
|
||||
TERMS_OUTDATED: "TERMS_OUTDATED",
|
||||
} as const;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue