fix(journal): architectural audit omnibus
Addresses 8 issues from the Journal architecture audit: 1. DB indexes on routes.ownerId + activities.ownerId. Listing queries on these tables were full table scans; adds composite indexes matching the order-by columns (updatedAt/startedAt/createdAt). 2. Zod validation on /api/auth/register body. Previously the action destructured request.json() with zero schema validation. 3. N+1 GeoJSON batch fetch collapsed to a single ANY($1::text[]) query in both routes.server and activities.server. 4. Webhook envelope validation in /api/sync/webhook/:provider. 5. AbortSignal.timeout(30s) on all external fetches (Komoot, Wahoo) via a new fetchWithTimeout helper in lib/http.server.ts. 6. .limit(100) on listPublicRoutesForOwner / listPublicActivitiesForOwner. 9. Welcome email moved off fire-and-forget onto a pg-boss job with retryLimit: 3 (send-welcome-email). 10. process.env.ORIGIN ?? "http://localhost:3000" centralized into lib/config.server.ts::getOrigin() across 14 call sites. Issues 7 (centralized apiError/auth guards across 60+ route files) and 8 (split .server.ts boundaries across 20+ route files) intentionally deferred — both are pure refactors that would balloon this PR past reviewability and warrant their own focused PRs. Tests added: - lib/config.server.test.ts (2 cases) - lib/http.server.test.ts (3 cases — timeout abort, success passthrough, caller-signal composition) - routes/api.sync.webhook.$provider.test.ts (6 cases) - routes/api.auth.register.test.ts (7 cases — schema rejection paths + the new welcome-email enqueue assertion) Full repo: pnpm typecheck, pnpm lint, pnpm test all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
a91085ab52
commit
4de6c86d41
28 changed files with 530 additions and 64 deletions
32
apps/journal/app/jobs/send-welcome-email.ts
Normal file
32
apps/journal/app/jobs/send-welcome-email.ts
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
import type { JobDefinition } from "@trails-cool/jobs";
|
||||
import { sendWelcome } from "../lib/email.server.ts";
|
||||
import { logger } from "../lib/logger.server.ts";
|
||||
|
||||
interface WelcomeEmailData {
|
||||
email: string;
|
||||
username: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Queue-backed welcome email send. The old code did
|
||||
* `sendWelcome(...).catch(log)` inline, which silently dropped failures.
|
||||
* pg-boss retries on transient failure (3 attempts) and surfaces persistent
|
||||
* failures via the dead-letter queue / logs.
|
||||
*/
|
||||
export const sendWelcomeEmailJob: JobDefinition = {
|
||||
name: "send-welcome-email",
|
||||
retryLimit: 3,
|
||||
expireInSeconds: 120,
|
||||
async handler(job) {
|
||||
const batch = Array.isArray(job) ? job : [job];
|
||||
for (const item of batch) {
|
||||
const { email, username } = item.data as WelcomeEmailData;
|
||||
try {
|
||||
await sendWelcome(email, username);
|
||||
} catch (err) {
|
||||
logger.error({ err, email }, "send-welcome-email job failed");
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
},
|
||||
};
|
||||
|
|
@ -150,6 +150,7 @@ export async function listActivities(
|
|||
export async function listPublicActivitiesForOwner(
|
||||
ownerId: string,
|
||||
sort: "startedAt" | "addedAt" = "startedAt",
|
||||
limit: number = 100,
|
||||
) {
|
||||
const db = getDb();
|
||||
const order = sort === "addedAt" ? desc(activities.createdAt) : desc(activities.startedAt);
|
||||
|
|
@ -157,7 +158,8 @@ export async function listPublicActivitiesForOwner(
|
|||
.select()
|
||||
.from(activities)
|
||||
.where(and(eq(activities.ownerId, ownerId), eq(activities.visibility, "public")))
|
||||
.orderBy(order);
|
||||
.orderBy(order)
|
||||
.limit(limit);
|
||||
|
||||
const ids = rows.map((r) => r.id);
|
||||
const geojsonMap = ids.length > 0 ? await getSimplifiedActivityGeojsonBatch(ids) : new Map();
|
||||
|
|
@ -288,13 +290,14 @@ async function getSimplifiedActivityGeojsonBatch(ids: string[]): Promise<Map<str
|
|||
if (ids.length === 0) return map;
|
||||
try {
|
||||
const db = getDb();
|
||||
await Promise.all(ids.map(async (id) => {
|
||||
const result = await db.execute(
|
||||
sql`SELECT ST_AsGeoJSON(ST_Simplify(geom, 0.001)) as geojson FROM journal.activities WHERE id = ${id} AND geom IS NOT NULL`,
|
||||
);
|
||||
const row = (result as unknown as Array<{ geojson: string }>)[0];
|
||||
if (row?.geojson) map.set(id, row.geojson);
|
||||
}));
|
||||
const result = await db.execute(
|
||||
sql`SELECT id, ST_AsGeoJSON(ST_Simplify(geom, 0.001)) as geojson
|
||||
FROM journal.activities
|
||||
WHERE id = ANY(${ids}::text[]) AND geom IS NOT NULL`,
|
||||
);
|
||||
for (const row of result as unknown as Array<{ id: string; geojson: string }>) {
|
||||
if (row.geojson) map.set(row.id, row.geojson);
|
||||
}
|
||||
} catch {
|
||||
// Fallback: no geojson
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,8 +1,8 @@
|
|||
import { getOrigin } from "./config.server.ts";
|
||||
|
||||
// Canonical ActivityPub actor IRI for a local user. Used as the key in
|
||||
// `follows.followed_actor_iri` so the column shape is identical for local
|
||||
// and (future) federated follows. Reading from `process.env.ORIGIN` keeps
|
||||
// us aligned with the rest of the auth/federation stack.
|
||||
// and (future) federated follows.
|
||||
export function localActorIri(username: string): string {
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
return `${origin}/users/${username}`;
|
||||
return `${getOrigin()}/users/${username}`;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,12 +12,13 @@ import type {
|
|||
AuthenticatorTransportFuture,
|
||||
} from "@simplewebauthn/types";
|
||||
import { getDb } from "./db.ts";
|
||||
import { getOrigin } from "./config.server.ts";
|
||||
import { users, credentials, magicTokens } from "@trails-cool/db/schema/journal";
|
||||
import type { Visibility } from "@trails-cool/db/schema/journal";
|
||||
|
||||
const RP_NAME = "trails.cool";
|
||||
const RP_ID = process.env.DOMAIN ?? "localhost";
|
||||
const ORIGIN = process.env.ORIGIN ?? `http://localhost:3000`;
|
||||
const ORIGIN = getOrigin();
|
||||
|
||||
// --- Registration ---
|
||||
|
||||
|
|
|
|||
19
apps/journal/app/lib/config.server.test.ts
Normal file
19
apps/journal/app/lib/config.server.test.ts
Normal file
|
|
@ -0,0 +1,19 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
describe("getOrigin", () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
it("returns ORIGIN env when set", async () => {
|
||||
vi.stubEnv("ORIGIN", "https://example.com");
|
||||
const { getOrigin } = await import("./config.server.ts");
|
||||
expect(getOrigin()).toBe("https://example.com");
|
||||
});
|
||||
|
||||
it("falls back to localhost when unset", async () => {
|
||||
delete process.env.ORIGIN;
|
||||
const { getOrigin } = await import("./config.server.ts");
|
||||
expect(getOrigin()).toBe("http://localhost:3000");
|
||||
});
|
||||
});
|
||||
7
apps/journal/app/lib/config.server.ts
Normal file
7
apps/journal/app/lib/config.server.ts
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
// Centralized access to the canonical origin for this Journal instance.
|
||||
// `ORIGIN` is set in production to the public HTTPS URL; in dev it falls
|
||||
// back to http://localhost:3000. Use the helper everywhere so the default
|
||||
// can be changed in one place if needed.
|
||||
export function getOrigin(): string {
|
||||
return process.env.ORIGIN ?? "http://localhost:3000";
|
||||
}
|
||||
|
|
@ -5,6 +5,7 @@
|
|||
// never reads the connected_services credentials JSONB directly.
|
||||
|
||||
import { fitToGpx } from "../../fit.ts";
|
||||
import { fetchWithTimeout } from "../../../http.server.ts";
|
||||
import { importActivity, isAlreadyImported } from "../../../sync/imports.server.ts";
|
||||
import type {
|
||||
CapabilityContext,
|
||||
|
|
@ -39,7 +40,7 @@ async function fetchWahooWorkoutPage(
|
|||
per_page: number;
|
||||
}> {
|
||||
const params = new URLSearchParams({ page: String(page), per_page: "30" });
|
||||
const resp = await fetch(`${WAHOO_API}/v1/workouts?${params}`, {
|
||||
const resp = await fetchWithTimeout(`${WAHOO_API}/v1/workouts?${params}`, {
|
||||
headers: { Authorization: `Bearer ${creds.access_token}` },
|
||||
});
|
||||
if (!resp.ok) throw new Error(`Wahoo list workouts failed: ${resp.status}`);
|
||||
|
|
@ -70,7 +71,7 @@ function toImportable(w: WahooWorkout) {
|
|||
async function downloadFit(fileUrl: string): Promise<Buffer> {
|
||||
// Wahoo CDN URLs are pre-signed; no auth header needed (and adding one
|
||||
// breaks them).
|
||||
const resp = await fetch(fileUrl);
|
||||
const resp = await fetchWithTimeout(fileUrl);
|
||||
if (!resp.ok) throw new Error(`Wahoo file download failed: ${resp.status}`);
|
||||
return Buffer.from(await resp.arrayBuffer());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@
|
|||
// the FIT file (if present) and create an activity.
|
||||
|
||||
import { fitToGpx } from "../../fit.ts";
|
||||
import { fetchWithTimeout } from "../../../http.server.ts";
|
||||
import { isAlreadyImported, importActivity } from "../../../sync/imports.server.ts";
|
||||
import { getServiceByProviderUser, withFreshCredentials } from "../../manager.ts";
|
||||
import type { OAuthCredentials } from "../../types.ts";
|
||||
|
|
@ -53,7 +54,7 @@ export const wahooWebhook: WebhookReceiver = {
|
|||
// handler might make.
|
||||
const buffer = await withFreshCredentials(service.id, async (_creds) => {
|
||||
void (_creds as unknown as OAuthCredentials);
|
||||
const resp = await fetch(event.fileUrl!);
|
||||
const resp = await fetchWithTimeout(event.fileUrl!);
|
||||
if (!resp.ok) throw new Error(`Wahoo file download failed: ${resp.status}`);
|
||||
return Buffer.from(await resp.arrayBuffer());
|
||||
});
|
||||
|
|
|
|||
45
apps/journal/app/lib/http.server.test.ts
Normal file
45
apps/journal/app/lib/http.server.test.ts
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
import { describe, it, expect, vi, afterEach } from "vitest";
|
||||
import { fetchWithTimeout } from "./http.server.ts";
|
||||
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = originalFetch;
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("fetchWithTimeout", () => {
|
||||
it("aborts when the request exceeds the timeout", async () => {
|
||||
// Mock fetch to honor the AbortSignal but never resolve otherwise.
|
||||
globalThis.fetch = vi.fn((_input: RequestInfo | URL, init?: RequestInit) => {
|
||||
return new Promise((_resolve, reject) => {
|
||||
init?.signal?.addEventListener("abort", () => {
|
||||
reject(new DOMException("aborted", "AbortError"));
|
||||
});
|
||||
});
|
||||
}) as typeof fetch;
|
||||
|
||||
await expect(fetchWithTimeout("https://example.test", {}, 25)).rejects.toThrow();
|
||||
});
|
||||
|
||||
it("forwards the response when the call returns in time", async () => {
|
||||
globalThis.fetch = vi.fn(async () => new Response("ok", { status: 200 })) as typeof fetch;
|
||||
const resp = await fetchWithTimeout("https://example.test", {}, 1000);
|
||||
expect(resp.status).toBe(200);
|
||||
expect(await resp.text()).toBe("ok");
|
||||
});
|
||||
|
||||
it("respects a caller-supplied abort signal alongside the timeout", async () => {
|
||||
const controller = new AbortController();
|
||||
globalThis.fetch = vi.fn((_input: RequestInfo | URL, init?: RequestInit) => {
|
||||
return new Promise((_resolve, reject) => {
|
||||
init?.signal?.addEventListener("abort", () => {
|
||||
reject(new DOMException("aborted", "AbortError"));
|
||||
});
|
||||
});
|
||||
}) as typeof fetch;
|
||||
const p = fetchWithTimeout("https://example.test", { signal: controller.signal }, 60_000);
|
||||
controller.abort();
|
||||
await expect(p).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
14
apps/journal/app/lib/http.server.ts
Normal file
14
apps/journal/app/lib/http.server.ts
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
// Default timeout for outbound HTTP calls to third-party providers.
|
||||
// Hung providers must not stall job workers or request handlers.
|
||||
export const DEFAULT_EXTERNAL_FETCH_TIMEOUT_MS = 30_000;
|
||||
|
||||
export function fetchWithTimeout(
|
||||
input: RequestInfo | URL,
|
||||
init: RequestInit = {},
|
||||
timeoutMs: number = DEFAULT_EXTERNAL_FETCH_TIMEOUT_MS,
|
||||
): Promise<Response> {
|
||||
const signal = init.signal
|
||||
? AbortSignal.any([init.signal, AbortSignal.timeout(timeoutMs)])
|
||||
: AbortSignal.timeout(timeoutMs);
|
||||
return fetch(input, { ...init, signal });
|
||||
}
|
||||
|
|
@ -1,10 +1,11 @@
|
|||
import { SignJWT, jwtVerify } from "jose";
|
||||
import { getOrigin } from "./config.server.ts";
|
||||
|
||||
const JWT_SECRET = new TextEncoder().encode(
|
||||
process.env.JWT_SECRET ?? "dev-jwt-secret-change-in-production",
|
||||
);
|
||||
|
||||
const ISSUER = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const ISSUER = getOrigin();
|
||||
|
||||
export async function createRouteToken(routeId: string, permissions: string[] = ["read", "write"]): Promise<string> {
|
||||
return new SignJWT({ route_id: routeId, permissions })
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@
|
|||
//
|
||||
// All network calls use plain fetch; no auth state is cached here.
|
||||
|
||||
import { fetchWithTimeout } from "./http.server.ts";
|
||||
|
||||
const API_BASE = "https://api.komoot.de/v007";
|
||||
const AUTH_BASE = "https://api.komoot.de/v006";
|
||||
|
||||
|
|
@ -50,7 +52,7 @@ export async function fetchPublicProfile(komootUserId: string): Promise<{
|
|||
contentText: string | null;
|
||||
contentLink: string | null;
|
||||
}> {
|
||||
const resp = await fetch(`${API_BASE}/users/${komootUserId}/`);
|
||||
const resp = await fetchWithTimeout(`${API_BASE}/users/${komootUserId}/`);
|
||||
if (!resp.ok) {
|
||||
throw new Error(`Komoot profile fetch failed: ${resp.status}`);
|
||||
}
|
||||
|
|
@ -94,7 +96,7 @@ export async function loginKomoot(
|
|||
password: string,
|
||||
): Promise<{ username: string; basicAuthToken: string }> {
|
||||
const basicAuthToken = Buffer.from(`${email}:${password}`).toString("base64");
|
||||
const resp = await fetch(`${AUTH_BASE}/account/email/${encodeURIComponent(email)}/`, {
|
||||
const resp = await fetchWithTimeout(`${AUTH_BASE}/account/email/${encodeURIComponent(email)}/`, {
|
||||
headers: { Authorization: `Basic ${basicAuthToken}` },
|
||||
});
|
||||
if (!resp.ok) {
|
||||
|
|
@ -158,7 +160,7 @@ export async function fetchKomootTours(
|
|||
if (basicAuthToken) {
|
||||
headers["Authorization"] = `Basic ${basicAuthToken}`;
|
||||
}
|
||||
const resp = await fetch(`${API_BASE}/users/${komootUserId}/tours/?${params}`, { headers });
|
||||
const resp = await fetchWithTimeout(`${API_BASE}/users/${komootUserId}/tours/?${params}`, { headers });
|
||||
if (!resp.ok) {
|
||||
throw new Error(`Komoot tours fetch failed: ${resp.status}`);
|
||||
}
|
||||
|
|
@ -180,7 +182,7 @@ export async function fetchKomootTourGpx(
|
|||
if (basicAuthToken) {
|
||||
headers["Authorization"] = `Basic ${basicAuthToken}`;
|
||||
}
|
||||
const resp = await fetch(`${API_BASE}/tours/${tourId}.gpx`, { headers });
|
||||
const resp = await fetchWithTimeout(`${API_BASE}/tours/${tourId}.gpx`, { headers });
|
||||
if (!resp.ok) {
|
||||
throw new Error(`Komoot GPX fetch failed: ${resp.status}`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -115,13 +115,14 @@ export async function listRoutes(ownerId: string) {
|
|||
* List the *public* routes of a given owner. Used for cross-user listings
|
||||
* (the public profile page); never includes `unlisted` or `private` content.
|
||||
*/
|
||||
export async function listPublicRoutesForOwner(ownerId: string) {
|
||||
export async function listPublicRoutesForOwner(ownerId: string, limit: number = 100) {
|
||||
const db = getDb();
|
||||
const rows = await db
|
||||
.select()
|
||||
.from(routes)
|
||||
.where(and(eq(routes.ownerId, ownerId), eq(routes.visibility, "public")))
|
||||
.orderBy(desc(routes.updatedAt));
|
||||
.orderBy(desc(routes.updatedAt))
|
||||
.limit(limit);
|
||||
|
||||
const ids = rows.map((r) => r.id);
|
||||
const geojsonMap = ids.length > 0 ? await getSimplifiedGeojsonBatch(ids) : new Map();
|
||||
|
|
@ -226,14 +227,14 @@ async function getSimplifiedGeojsonBatch(ids: string[]): Promise<Map<string, str
|
|||
if (ids.length === 0) return map;
|
||||
try {
|
||||
const db = getDb();
|
||||
// Fetch individually — Drizzle's sql template doesn't handle array params well with ANY()
|
||||
await Promise.all(ids.map(async (id) => {
|
||||
const result = await db.execute(
|
||||
sql`SELECT ST_AsGeoJSON(ST_Simplify(geom, 0.001)) as geojson FROM journal.routes WHERE id = ${id} AND geom IS NOT NULL`,
|
||||
);
|
||||
const row = (result as unknown as Array<{ geojson: string }>)[0];
|
||||
if (row?.geojson) map.set(id, row.geojson);
|
||||
}));
|
||||
const result = await db.execute(
|
||||
sql`SELECT id, ST_AsGeoJSON(ST_Simplify(geom, 0.001)) as geojson
|
||||
FROM journal.routes
|
||||
WHERE id = ANY(${ids}::text[]) AND geom IS NOT NULL`,
|
||||
);
|
||||
for (const row of result as unknown as Array<{ id: string; geojson: string }>) {
|
||||
if (row.geojson) map.set(row.id, row.geojson);
|
||||
}
|
||||
} catch {
|
||||
// Fallback: no geojson
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { data } from "react-router";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
import type { Route } from "./+types/api.auth.login";
|
||||
import { startAuthentication, finishAuthentication, createMagicToken, verifyLoginCode } from "~/lib/auth.server";
|
||||
import { completeAuth } from "~/lib/auth/completion.server";
|
||||
|
|
@ -21,7 +22,7 @@ export async function action({ request }: Route.ActionArgs) {
|
|||
|
||||
if (step === "magic-link") {
|
||||
const { token, code: loginCode } = await createMagicToken(email);
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const origin = getOrigin();
|
||||
const link = `${origin}/auth/verify?token=${token}`;
|
||||
// In dev, return the link and code directly
|
||||
if (process.env.NODE_ENV !== "production") {
|
||||
|
|
|
|||
151
apps/journal/app/routes/api.auth.register.test.ts
Normal file
151
apps/journal/app/routes/api.auth.register.test.ts
Normal file
|
|
@ -0,0 +1,151 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
startRegistration: vi.fn(),
|
||||
finishRegistration: vi.fn(),
|
||||
registerWithMagicLink: vi.fn(),
|
||||
addPasskeyStart: vi.fn(),
|
||||
addPasskeyFinish: vi.fn(),
|
||||
sendMagicLink: vi.fn(),
|
||||
enqueueOptional: vi.fn(),
|
||||
completeAuth: vi.fn(),
|
||||
}));
|
||||
const {
|
||||
startRegistration,
|
||||
finishRegistration,
|
||||
registerWithMagicLink,
|
||||
addPasskeyStart,
|
||||
addPasskeyFinish,
|
||||
sendMagicLink,
|
||||
enqueueOptional,
|
||||
completeAuth,
|
||||
} = mocks;
|
||||
|
||||
vi.mock("~/lib/auth.server", () => ({
|
||||
startRegistration: mocks.startRegistration,
|
||||
finishRegistration: mocks.finishRegistration,
|
||||
registerWithMagicLink: mocks.registerWithMagicLink,
|
||||
addPasskeyStart: mocks.addPasskeyStart,
|
||||
addPasskeyFinish: mocks.addPasskeyFinish,
|
||||
}));
|
||||
vi.mock("~/lib/auth/completion.server", () => ({ completeAuth: mocks.completeAuth }));
|
||||
vi.mock("~/lib/email.server", () => ({ sendMagicLink: mocks.sendMagicLink }));
|
||||
vi.mock("~/lib/boss.server", () => ({ enqueueOptional: mocks.enqueueOptional }));
|
||||
|
||||
import { action } from "./api.auth.register.ts";
|
||||
|
||||
function makeRequest(body: unknown): Request {
|
||||
return new Request("http://test.local/api/auth/register", {
|
||||
method: "POST",
|
||||
body: typeof body === "string" ? (body as string) : JSON.stringify(body),
|
||||
headers: { "content-type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
async function callAction(body: unknown): Promise<{ status: number; json: Record<string, unknown> }> {
|
||||
const res = await action({
|
||||
request: makeRequest(body),
|
||||
params: {},
|
||||
context: {} as unknown,
|
||||
} as never);
|
||||
if (res instanceof Response) {
|
||||
return { status: res.status, json: (await res.json()) as Record<string, unknown> };
|
||||
}
|
||||
const d = res as { data: Record<string, unknown>; init?: { status?: number } };
|
||||
return { status: d.init?.status ?? 200, json: d.data };
|
||||
}
|
||||
|
||||
describe("POST /api/auth/register — input validation", () => {
|
||||
beforeEach(() => {
|
||||
startRegistration.mockReset();
|
||||
finishRegistration.mockReset();
|
||||
registerWithMagicLink.mockReset();
|
||||
addPasskeyStart.mockReset();
|
||||
addPasskeyFinish.mockReset();
|
||||
sendMagicLink.mockReset();
|
||||
enqueueOptional.mockReset();
|
||||
completeAuth.mockReset();
|
||||
});
|
||||
|
||||
it("rejects malformed JSON with 400", async () => {
|
||||
const { status, json } = await callAction("not-json{");
|
||||
expect(status).toBe(400);
|
||||
expect(json.error).toMatch(/Invalid JSON/);
|
||||
expect(startRegistration).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects unknown step with 400", async () => {
|
||||
const { status } = await callAction({ step: "bogus" });
|
||||
expect(status).toBe(400);
|
||||
expect(startRegistration).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects non-string email with 400 (zod type check)", async () => {
|
||||
const { status } = await callAction({
|
||||
step: "start",
|
||||
email: 12345,
|
||||
username: "alice",
|
||||
termsAccepted: true,
|
||||
termsVersion: "v1",
|
||||
});
|
||||
expect(status).toBe(400);
|
||||
expect(startRegistration).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects start without termsAccepted with 400", async () => {
|
||||
const { status, json } = await callAction({
|
||||
step: "start",
|
||||
email: "a@b.co",
|
||||
username: "alice",
|
||||
});
|
||||
expect(status).toBe(400);
|
||||
expect(json.error).toMatch(/Terms of Service/);
|
||||
expect(startRegistration).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects start without email with a missing-field error", async () => {
|
||||
const { status, json } = await callAction({
|
||||
step: "start",
|
||||
username: "alice",
|
||||
termsAccepted: true,
|
||||
termsVersion: "v1",
|
||||
});
|
||||
expect(status).toBe(400);
|
||||
expect(json.error).toMatch(/email/i);
|
||||
});
|
||||
|
||||
it("accepts a well-formed start payload and calls downstream", async () => {
|
||||
startRegistration.mockResolvedValue({ options: { foo: 1 }, userId: "u1" });
|
||||
const { status, json } = await callAction({
|
||||
step: "start",
|
||||
email: "a@b.co",
|
||||
username: "alice",
|
||||
termsAccepted: true,
|
||||
termsVersion: "v1",
|
||||
});
|
||||
expect(status).toBe(200);
|
||||
expect(json.step).toBe("challenge");
|
||||
expect(startRegistration).toHaveBeenCalledWith("a@b.co", "alice");
|
||||
});
|
||||
|
||||
it("enqueues welcome email job on finish (no fire-and-forget)", async () => {
|
||||
finishRegistration.mockResolvedValue("new-user-id");
|
||||
completeAuth.mockResolvedValue(new Response(JSON.stringify({ ok: true }), { status: 200 }));
|
||||
const { status } = await callAction({
|
||||
step: "finish",
|
||||
userId: "u1",
|
||||
email: "a@b.co",
|
||||
username: "alice",
|
||||
challenge: "chal",
|
||||
response: { id: "x" },
|
||||
termsAccepted: true,
|
||||
termsVersion: "v1",
|
||||
});
|
||||
expect(status).toBe(200);
|
||||
expect(enqueueOptional).toHaveBeenCalledWith(
|
||||
"send-welcome-email",
|
||||
{ email: "a@b.co", username: "alice" },
|
||||
expect.objectContaining({ source: "register" }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,14 +1,46 @@
|
|||
import { data } from "react-router";
|
||||
import { z } from "zod";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
import type { Route } from "./+types/api.auth.register";
|
||||
import { startRegistration, finishRegistration, addPasskeyStart, addPasskeyFinish, registerWithMagicLink } from "~/lib/auth.server";
|
||||
import { completeAuth } from "~/lib/auth/completion.server";
|
||||
import { sendWelcome, sendMagicLink } from "~/lib/email.server";
|
||||
import { logger } from "~/lib/logger.server";
|
||||
import { sendMagicLink } from "~/lib/email.server";
|
||||
import { enqueueOptional } from "~/lib/boss.server";
|
||||
|
||||
// Permissive schema: the discriminator (`step`) and primitive fields are
|
||||
// validated strictly; nested WebAuthn payloads stay as unknown because their
|
||||
// shape is enforced downstream by @simplewebauthn.
|
||||
const registerBodySchema = z.object({
|
||||
step: z.enum([
|
||||
"start",
|
||||
"finish",
|
||||
"register-magic-link",
|
||||
"add-passkey",
|
||||
"finish-add-passkey",
|
||||
]),
|
||||
email: z.string().email().max(320).optional(),
|
||||
username: z.string().min(1).max(64).optional(),
|
||||
userId: z.string().min(1).max(128).optional(),
|
||||
response: z.unknown().optional(),
|
||||
challenge: z.string().max(4096).optional(),
|
||||
termsAccepted: z.boolean().optional(),
|
||||
termsVersion: z.string().max(64).optional(),
|
||||
returnTo: z.string().max(2048).optional(),
|
||||
});
|
||||
|
||||
export async function action({ request }: Route.ActionArgs) {
|
||||
const body = await request.json();
|
||||
const { step, email, username, response, challenge, userId, termsAccepted, termsVersion, returnTo } = body;
|
||||
const origin = process.env.ORIGIN ?? `http://localhost:3000`;
|
||||
let raw: unknown;
|
||||
try {
|
||||
raw = await request.json();
|
||||
} catch {
|
||||
return data({ error: "Invalid JSON body" }, { status: 400 });
|
||||
}
|
||||
const parsed = registerBodySchema.safeParse(raw);
|
||||
if (!parsed.success) {
|
||||
return data({ error: "Invalid request body" }, { status: 400 });
|
||||
}
|
||||
const { step, email, username, response, challenge, userId, termsAccepted, termsVersion, returnTo } = parsed.data;
|
||||
const origin = getOrigin();
|
||||
|
||||
// Registration steps require terms acceptance + the version the client
|
||||
// agreed to (stored for audit so we can tell which text the user saw).
|
||||
|
|
@ -20,43 +52,77 @@ export async function action({ request }: Route.ActionArgs) {
|
|||
return data({ error: "Terms of Service version missing" }, { status: 400 });
|
||||
}
|
||||
|
||||
function requireString(v: string | undefined, name: string): string {
|
||||
if (typeof v !== "string" || v.length === 0) {
|
||||
throw new Error(`Missing required field: ${name}`);
|
||||
}
|
||||
return v;
|
||||
}
|
||||
|
||||
try {
|
||||
if (step === "start") {
|
||||
const result = await startRegistration(email, username);
|
||||
const result = await startRegistration(
|
||||
requireString(email, "email"),
|
||||
requireString(username, "username"),
|
||||
);
|
||||
return data({ step: "challenge", options: result.options, userId: result.userId });
|
||||
}
|
||||
|
||||
if (step === "finish") {
|
||||
const newUserId = await finishRegistration(userId, email, username, response, challenge, termsVersion);
|
||||
sendWelcome(email, username).catch((err) =>
|
||||
logger.error({ err }, "Failed to send welcome email"),
|
||||
const emailStr = requireString(email, "email");
|
||||
const usernameStr = requireString(username, "username");
|
||||
const newUserId = await finishRegistration(
|
||||
requireString(userId, "userId"),
|
||||
emailStr,
|
||||
usernameStr,
|
||||
// Validated downstream by @simplewebauthn.
|
||||
response as Parameters<typeof finishRegistration>[3],
|
||||
requireString(challenge, "challenge"),
|
||||
requireString(termsVersion, "termsVersion"),
|
||||
);
|
||||
await enqueueOptional(
|
||||
"send-welcome-email",
|
||||
{ email: emailStr, username: usernameStr },
|
||||
{ source: "register" },
|
||||
);
|
||||
return completeAuth({ userId: newUserId, request, returnTo, mode: "json" });
|
||||
}
|
||||
|
||||
if (step === "register-magic-link") {
|
||||
const { token, code } = await registerWithMagicLink(email, username, termsVersion);
|
||||
const emailStr = requireString(email, "email");
|
||||
const usernameStr = requireString(username, "username");
|
||||
const { token, code } = await registerWithMagicLink(
|
||||
emailStr,
|
||||
usernameStr,
|
||||
requireString(termsVersion, "termsVersion"),
|
||||
);
|
||||
const link = `${origin}/auth/verify?token=${token}`;
|
||||
if (process.env.NODE_ENV !== "production") {
|
||||
// Mirror the login endpoint so devs can grab either the link or
|
||||
// the 6-digit code straight from the terminal.
|
||||
console.log(`[Register Magic Link] ${email}: ${link} (code: ${code})`);
|
||||
console.log(`[Register Magic Link] ${emailStr}: ${link} (code: ${code})`);
|
||||
return data({ step: "magic-link-sent", devLink: link, code });
|
||||
}
|
||||
await sendMagicLink(email, link, code);
|
||||
sendWelcome(email, username).catch((err) =>
|
||||
logger.error({ err }, "Failed to send welcome email"),
|
||||
await sendMagicLink(emailStr, link, code);
|
||||
await enqueueOptional(
|
||||
"send-welcome-email",
|
||||
{ email: emailStr, username: usernameStr },
|
||||
{ source: "register" },
|
||||
);
|
||||
return data({ step: "magic-link-sent" });
|
||||
}
|
||||
|
||||
if (step === "add-passkey") {
|
||||
const options = await addPasskeyStart(userId);
|
||||
const options = await addPasskeyStart(requireString(userId, "userId"));
|
||||
return data({ step: "challenge", options });
|
||||
}
|
||||
|
||||
if (step === "finish-add-passkey") {
|
||||
await addPasskeyFinish(userId, response, challenge);
|
||||
await addPasskeyFinish(
|
||||
requireString(userId, "userId"),
|
||||
response as Parameters<typeof addPasskeyFinish>[1],
|
||||
requireString(challenge, "challenge"),
|
||||
);
|
||||
return data({ step: "done" });
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { data } from "react-router";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
import type { Route } from "./+types/api.routes.$id.edit-in-planner";
|
||||
import { getSessionUser } from "~/lib/auth/session.server";
|
||||
import { getRouteWithVersions } from "~/lib/routes.server";
|
||||
|
|
@ -13,7 +14,7 @@ export async function action({ params, request }: Route.ActionArgs) {
|
|||
if (route.ownerId !== user.id) return data({ error: "Not authorized" }, { status: 403 });
|
||||
|
||||
const token = await createRouteToken(params.id);
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const origin = getOrigin();
|
||||
const callbackUrl = `${origin}/api/routes/${params.id}/callback`;
|
||||
const plannerUrl = process.env.PLANNER_URL ?? "http://localhost:3001";
|
||||
const returnUrl = `${origin}/routes/${params.id}`;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { data, redirect } from "react-router";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
import type { Route } from "./+types/api.settings.email";
|
||||
import { initiateEmailChange } from "~/lib/auth.server";
|
||||
import { getSessionUser } from "~/lib/auth/session.server";
|
||||
|
|
@ -12,7 +13,7 @@ export async function action({ request }: Route.ActionArgs) {
|
|||
const newEmail = (formData.get("newEmail") as string)?.trim();
|
||||
if (!newEmail) return data({ error: "Email is required" }, { status: 400 });
|
||||
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const origin = getOrigin();
|
||||
|
||||
try {
|
||||
const token = await initiateEmailChange(user.id, newEmail);
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { redirect, data } from "react-router";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
import type { Route } from "./+types/api.sync.callback.$provider";
|
||||
import { getSessionUser } from "~/lib/auth/session.server";
|
||||
import { getManifest, link } from "~/lib/connected-services";
|
||||
|
|
@ -29,7 +30,7 @@ export async function loader({ params, request }: Route.LoaderArgs) {
|
|||
const code = url.searchParams.get("code");
|
||||
if (!code) return data({ error: "Missing authorization code" }, { status: 400 });
|
||||
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const origin = getOrigin();
|
||||
const redirectUri = `${origin}/api/sync/callback/${params.provider}`;
|
||||
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { redirect, data } from "react-router";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
import type { Route } from "./+types/api.sync.connect.$provider";
|
||||
import { getSessionUser } from "~/lib/auth/session.server";
|
||||
import { getManifest } from "~/lib/connected-services";
|
||||
|
|
@ -13,7 +14,7 @@ export async function loader({ params, request }: Route.LoaderArgs) {
|
|||
return data({ error: "Unknown provider" }, { status: 404 });
|
||||
}
|
||||
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const origin = getOrigin();
|
||||
const redirectUri = `${origin}/api/sync/callback/${params.provider}`;
|
||||
const state = encodeOAuthState({ returnTo: "/settings/connections" });
|
||||
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
// On success, creates or replaces the connected service row in public mode.
|
||||
|
||||
import { data, redirect } from "react-router";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
import type { Route } from "./+types/api.sync.komoot.verify";
|
||||
import { getSessionUser } from "~/lib/auth/session.server";
|
||||
import { parseKomootUserId, verifyKomootOwnership } from "~/lib/komoot.server";
|
||||
|
|
@ -20,7 +21,7 @@ export async function action({ request }: Route.ActionArgs) {
|
|||
return data({ error: "invalid_url" }, { status: 400 });
|
||||
}
|
||||
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const origin = getOrigin();
|
||||
const trailsProfileUrl = `${origin}/users/${user.username}`;
|
||||
|
||||
const verified = await verifyKomootOwnership(komootUserId, trailsProfileUrl);
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { redirect, data } from "react-router";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
import type { Route } from "./+types/api.sync.push.$provider.$routeId";
|
||||
import { getSessionUser } from "~/lib/auth/session.server";
|
||||
import { getManifest } from "~/lib/connected-services";
|
||||
|
|
@ -26,7 +27,7 @@ export async function action({ params, request }: Route.ActionArgs) {
|
|||
if (!manifest.buildAuthUrl) {
|
||||
return redirect(`${returnTo}?push=needs_permission`);
|
||||
}
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const origin = getOrigin();
|
||||
const redirectUri = `${origin}/api/sync/callback/${manifest.id}`;
|
||||
const state = encodeOAuthState({
|
||||
pushAfter: { routeId: params.routeId },
|
||||
|
|
|
|||
93
apps/journal/app/routes/api.sync.webhook.$provider.test.ts
Normal file
93
apps/journal/app/routes/api.sync.webhook.$provider.test.ts
Normal file
|
|
@ -0,0 +1,93 @@
|
|||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
parseWebhook: vi.fn(),
|
||||
handle: vi.fn(),
|
||||
}));
|
||||
const { parseWebhook, handle } = mocks;
|
||||
|
||||
vi.mock("~/lib/connected-services", () => ({
|
||||
getManifest: (id: string) =>
|
||||
id === "test"
|
||||
? {
|
||||
id: "test",
|
||||
webhookReceiver: { parseWebhook: mocks.parseWebhook, handle: mocks.handle },
|
||||
}
|
||||
: null,
|
||||
}));
|
||||
|
||||
import { action } from "./api.sync.webhook.$provider.ts";
|
||||
|
||||
function makeRequest(body: string | object, method = "POST"): Request {
|
||||
const init: RequestInit = { method, headers: { "content-type": "application/json" } };
|
||||
if (method !== "GET" && method !== "HEAD") {
|
||||
init.body = typeof body === "string" ? body : JSON.stringify(body);
|
||||
}
|
||||
return new Request("http://test.local/api/sync/webhook/test", init);
|
||||
}
|
||||
|
||||
describe("POST /api/sync/webhook/:provider", () => {
|
||||
beforeEach(() => {
|
||||
parseWebhook.mockReset();
|
||||
handle.mockReset();
|
||||
parseWebhook.mockReturnValue(null);
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
function statusOf(result: unknown): number {
|
||||
if (result instanceof Response) return result.status;
|
||||
const init = (result as { init?: { status?: number } } | undefined)?.init;
|
||||
return init?.status ?? 200;
|
||||
}
|
||||
|
||||
async function call(body: string | object, opts: { method?: string; provider?: string } = {}) {
|
||||
return action({
|
||||
request: makeRequest(body, opts.method ?? "POST"),
|
||||
params: { provider: opts.provider ?? "test" },
|
||||
context: {} as unknown,
|
||||
} as never);
|
||||
}
|
||||
|
||||
it("returns 405 for non-POST", async () => {
|
||||
const res = await call({}, { method: "GET" });
|
||||
expect(statusOf(res)).toBe(405);
|
||||
});
|
||||
|
||||
it("returns 200 silently for unknown provider", async () => {
|
||||
const res = await call({}, { provider: "no-such-provider" });
|
||||
expect(statusOf(res)).toBe(200);
|
||||
expect(parseWebhook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns 200 silently and does not invoke parseWebhook on malformed JSON", async () => {
|
||||
const res = await call("not-json{");
|
||||
expect(statusOf(res)).toBe(200);
|
||||
expect(parseWebhook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns 200 silently and does not invoke parseWebhook on a non-object body", async () => {
|
||||
const res = await call("42");
|
||||
expect(statusOf(res)).toBe(200);
|
||||
expect(parseWebhook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects when webhook_token does not match", async () => {
|
||||
vi.stubEnv("TEST_WEBHOOK_TOKEN", "expected");
|
||||
const res = await call({ webhook_token: "wrong" });
|
||||
expect(statusOf(res)).toBe(200);
|
||||
expect(parseWebhook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("invokes parseWebhook with a valid object body", async () => {
|
||||
parseWebhook.mockReturnValue({ eventType: "x", providerUserId: "1", workoutId: "9" });
|
||||
handle.mockResolvedValue(undefined);
|
||||
const res = await call({ event_type: "x" });
|
||||
expect(statusOf(res)).toBe(200);
|
||||
expect(parseWebhook).toHaveBeenCalledTimes(1);
|
||||
expect(handle).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,7 +1,13 @@
|
|||
import { data } from "react-router";
|
||||
import { z } from "zod";
|
||||
import type { Route } from "./+types/api.sync.webhook.$provider";
|
||||
import { getManifest } from "~/lib/connected-services";
|
||||
|
||||
// Generic webhook envelope. Provider-specific shape validation happens in
|
||||
// each provider's `parseWebhook`; here we only enforce that the body is a
|
||||
// JSON object so downstream code never crashes on a malformed payload.
|
||||
const webhookEnvelope = z.object({ webhook_token: z.string().optional() }).passthrough();
|
||||
|
||||
export async function action({ params, request }: Route.ActionArgs) {
|
||||
if (request.method !== "POST") {
|
||||
return data({ error: "Method not allowed" }, { status: 405 });
|
||||
|
|
@ -13,14 +19,21 @@ export async function action({ params, request }: Route.ActionArgs) {
|
|||
return data({ ok: true });
|
||||
}
|
||||
|
||||
const body = await request.json();
|
||||
let raw: unknown;
|
||||
try {
|
||||
raw = await request.json();
|
||||
} catch {
|
||||
return data({ ok: true });
|
||||
}
|
||||
const envelope = webhookEnvelope.safeParse(raw);
|
||||
if (!envelope.success) {
|
||||
return data({ ok: true });
|
||||
}
|
||||
const body = envelope.data;
|
||||
|
||||
// Verify webhook token (provider-specific shared secret).
|
||||
const expectedToken = process.env[`${params.provider.toUpperCase()}_WEBHOOK_TOKEN`];
|
||||
if (
|
||||
expectedToken &&
|
||||
(body as { webhook_token?: string }).webhook_token !== expectedToken
|
||||
) {
|
||||
if (expectedToken && body.webhook_token !== expectedToken) {
|
||||
return data({ ok: true });
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { API_VERSION } from "@trails-cool/api";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
|
||||
/**
|
||||
* GET /.well-known/trails-cool
|
||||
|
|
@ -8,7 +9,7 @@ import { API_VERSION } from "@trails-cool/api";
|
|||
*/
|
||||
export function loader() {
|
||||
const domain = process.env.DOMAIN ?? "localhost";
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const origin = getOrigin();
|
||||
|
||||
return Response.json({
|
||||
apiVersion: API_VERSION,
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
|
||||
import { useState } from "react";
|
||||
import { data, redirect, useFetcher } from "react-router";
|
||||
import { getOrigin } from "~/lib/config.server";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import type { Route } from "./+types/settings.connections.komoot";
|
||||
import { getSessionUser } from "~/lib/auth/session.server";
|
||||
|
|
@ -18,7 +19,7 @@ export async function loader({ request }: Route.LoaderArgs) {
|
|||
if (!user) throw redirect("/auth/login");
|
||||
|
||||
const service = await getService(user.id, "komoot");
|
||||
const origin = process.env.ORIGIN ?? "http://localhost:3000";
|
||||
const origin = getOrigin();
|
||||
const trailsProfileUrl = `${origin}/users/${user.username}`;
|
||||
|
||||
return data({
|
||||
|
|
|
|||
|
|
@ -145,8 +145,9 @@ server.listen(port, async () => {
|
|||
const { notificationsPurgeJob } = await import("./app/jobs/notifications-purge.ts");
|
||||
const { komootBulkImportJob } = await import("./app/jobs/komoot-bulk-import.ts");
|
||||
const { importBatchesSweepJob } = await import("./app/jobs/import-batches-sweep.ts");
|
||||
const { sendWelcomeEmailJob } = await import("./app/jobs/send-welcome-email.ts");
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
jobs.push(notificationsFanoutJob, notificationsPurgeJob, komootBulkImportJob as any, importBatchesSweepJob);
|
||||
jobs.push(notificationsFanoutJob, notificationsPurgeJob, komootBulkImportJob as any, importBatchesSweepJob, sendWelcomeEmailJob);
|
||||
|
||||
const boss = createBoss(process.env.DATABASE_URL ?? "postgres://trails:trails@localhost:5432/trails");
|
||||
await startWorker(boss, jobs);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue