From 7a1dca378fc28959cb88bcc8061b0607782632d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Wed, 10 Jun 2026 02:35:00 +0200 Subject: [PATCH] journal: branded ownership loading for routes and activities Ownership was checked ad hoc: some handlers loaded-and-compared ownerId, some lib mutators enforced it in WHERE clauses and silently no-op'd for non-owners, and nothing tied the two together. Two real authorization bugs hid in the gaps: linkActivityToRoute ignored its ownerId parameter entirely (any logged-in user could relink any activity), and createRouteFromActivity loaded any activity without an ownership check (a non-owner could copy a private activity's GPX into their own route). PUT /api/v1/routes/:id also returned ok:true for non-owners without updating anything. ownership.server.ts is now the single enforcement point: - loadOwnedRoute / loadOwnedActivity (non-throwing, for callers with their own error vocabulary) and requireOwnedRoute / requireOwnedActivity (throwing data() 404/403 for web handlers; 404 by default so guessed ids don't leak existence) - the returned entities carry an Owned<> brand; mutators (updateRoute, deleteRoute, deleteActivity, updateActivityVisibility, linkActivityToRoute, createRouteFromActivity) now require an OwnedRef, so skipping the check is a compile error - vouchOwnership is the explicit, greppable escape hatch for the one non-session authorization path (the Planner JWT callback) - WHERE ownerId clauses stay in the mutators as defense in depth Co-Authored-By: Claude Fable 5 --- apps/journal/app/lib/activities.server.ts | 24 +++-- .../connected-services/push-action.server.ts | 10 +- apps/journal/app/lib/ownership.server.test.ts | 91 +++++++++++++++++++ apps/journal/app/lib/ownership.server.ts | 88 ++++++++++++++++++ apps/journal/app/lib/routes.server.ts | 14 +-- .../app/routes/activities.$id.server.ts | 22 +++-- .../app/routes/api.routes.$id.callback.ts | 7 +- .../routes/api.routes.$id.edit-in-planner.ts | 6 +- .../app/routes/api.v1.activities.$id.ts | 6 +- apps/journal/app/routes/api.v1.routes.$id.ts | 12 ++- apps/journal/app/routes/api.v1.routes.test.ts | 5 +- .../app/routes/routes.$id.edit.server.ts | 12 +-- apps/journal/app/routes/routes.$id.server.ts | 7 +- 13 files changed, 256 insertions(+), 48 deletions(-) create mode 100644 apps/journal/app/lib/ownership.server.test.ts create mode 100644 apps/journal/app/lib/ownership.server.ts diff --git a/apps/journal/app/lib/activities.server.ts b/apps/journal/app/lib/activities.server.ts index 6ad22b7..6e99d15 100644 --- a/apps/journal/app/lib/activities.server.ts +++ b/apps/journal/app/lib/activities.server.ts @@ -7,6 +7,7 @@ import type { Visibility } from "@trails-cool/db/schema/journal"; import { validateGpx, writeGeom } from "./gpx-save.server.ts"; import type { GpxData } from "./gpx-save.server.ts"; import { enqueueOptional } from "./boss.server.ts"; +import type { OwnedRef } from "./ownership.server.ts"; import { enqueueActivityDeliveries, visibilityTransitionAction, @@ -25,10 +26,10 @@ export interface ActivityInput { } export async function updateActivityVisibility( - id: string, - ownerId: string, + ownedActivity: OwnedRef, visibility: Visibility, ): Promise { + const { id, ownerId } = ownedActivity; const db = getDb(); // Read the previous visibility first: the federation action depends on // the *transition*, not the new value alone (a gratuitous Delete @@ -130,8 +131,11 @@ export async function getActivity(id: string) { return { ...activity, geojson, importSource }; } -export async function deleteActivity(id: string, ownerId: string): Promise { +export async function deleteActivity(ownedActivity: OwnedRef): Promise { + const { id, ownerId } = ownedActivity; const db = getDb(); + // The WHERE ownerId clause stays as defense in depth even though the + // OwnedRef brand already proves ownership. const [activity] = await db.select({ id: activities.id, visibility: activities.visibility }).from(activities) .where(and(eq(activities.id, id), eq(activities.ownerId, ownerId))); if (!activity) return false; @@ -311,17 +315,21 @@ export async function listRecentPublicActivities(limit: number = 20) { return rows.map((r) => ({ ...r, geojson: geojsonMap.get(r.id) ?? null })); } -export async function linkActivityToRoute(activityId: string, routeId: string, _ownerId: string) { +export async function linkActivityToRoute(ownedActivity: OwnedRef, ownedRoute: OwnedRef) { const db = getDb(); await db .update(activities) - .set({ routeId }) - .where(eq(activities.id, activityId)); + .set({ routeId: ownedRoute.id }) + .where(and(eq(activities.id, ownedActivity.id), eq(activities.ownerId, ownedActivity.ownerId))); } -export async function createRouteFromActivity(activityId: string, ownerId: string): Promise { +export async function createRouteFromActivity(ownedActivity: OwnedRef): Promise { + const { id: activityId, ownerId } = ownedActivity; const db = getDb(); - const [activity] = await db.select().from(activities).where(eq(activities.id, activityId)); + const [activity] = await db + .select() + .from(activities) + .where(and(eq(activities.id, activityId), eq(activities.ownerId, ownerId))); if (!activity?.gpx) return null; const parsed = await validateGpx(activity.gpx); diff --git a/apps/journal/app/lib/connected-services/push-action.server.ts b/apps/journal/app/lib/connected-services/push-action.server.ts index a42039f..eb17cd0 100644 --- a/apps/journal/app/lib/connected-services/push-action.server.ts +++ b/apps/journal/app/lib/connected-services/push-action.server.ts @@ -10,7 +10,7 @@ import { desc, eq } from "drizzle-orm"; import { parseGpxAsync } from "@trails-cool/gpx"; import { routeVersions } from "@trails-cool/db/schema/journal"; import { getDb } from "../db.ts"; -import { getRoute } from "../routes.server.ts"; +import { loadOwnedRoute } from "../ownership.server.ts"; import { ConnectionNotActiveError, NeedsRelinkError, @@ -49,9 +49,11 @@ export async function pushRouteToProvider( if (!manifest) return { status: "not_found" }; if (!manifest.routePusher) return { status: "unsupported_provider" }; - const route = await getRoute(routeId); - if (!route) return { status: "not_found" }; - if (route.ownerId !== userId) return { status: "not_owner" }; + const loaded = await loadOwnedRoute(routeId, userId); + if (!loaded.ok) { + return { status: loaded.reason === "not_found" ? "not_found" : "not_owner" }; + } + const route = loaded.entity; const service = await getService(userId, providerId); if (!service) return { status: "no_connection" }; diff --git a/apps/journal/app/lib/ownership.server.test.ts b/apps/journal/app/lib/ownership.server.test.ts new file mode 100644 index 0000000..3104d71 --- /dev/null +++ b/apps/journal/app/lib/ownership.server.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const getRoute = vi.fn(); +vi.mock("./routes.server.ts", () => ({ + getRoute: (...args: unknown[]) => getRoute(...args), +})); + +const getActivity = vi.fn(); +vi.mock("./activities.server.ts", () => ({ + getActivity: (...args: unknown[]) => getActivity(...args), +})); + +import { + loadOwnedRoute, + loadOwnedActivity, + requireOwnedRoute, + requireOwnedActivity, + vouchOwnership, +} from "./ownership.server.ts"; + +const USER = "user-1"; + +describe("loadOwnedRoute / loadOwnedActivity", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns not_found for a missing route", async () => { + getRoute.mockResolvedValue(null); + expect(await loadOwnedRoute("r1", USER)).toEqual({ ok: false, reason: "not_found" }); + }); + + it("returns not_owner for someone else's route", async () => { + getRoute.mockResolvedValue({ id: "r1", ownerId: "someone-else" }); + expect(await loadOwnedRoute("r1", USER)).toEqual({ ok: false, reason: "not_owner" }); + }); + + it("returns the entity when owned", async () => { + const route = { id: "r1", ownerId: USER, name: "Tour" }; + getRoute.mockResolvedValue(route); + const result = await loadOwnedRoute("r1", USER); + expect(result.ok).toBe(true); + if (result.ok) expect(result.entity).toBe(route); + }); + + it("treats remote-ingested activities (ownerId null) as not owned", async () => { + getActivity.mockResolvedValue({ id: "a1", ownerId: null }); + expect(await loadOwnedActivity("a1", USER)).toEqual({ ok: false, reason: "not_owner" }); + }); +}); + +describe("requireOwnedRoute / requireOwnedActivity", () => { + beforeEach(() => vi.clearAllMocks()); + + async function statusOf(promise: Promise): Promise { + try { + await promise; + throw new Error("expected a throw"); + } catch (thrown) { + // react-router data() throws a Response-like with init status + return (thrown as { init?: { status: number }; status?: number }).init?.status + ?? (thrown as { status: number }).status; + } + } + + it("throws 404 for missing entities", async () => { + getRoute.mockResolvedValue(null); + expect(await statusOf(requireOwnedRoute("r1", USER))).toBe(404); + }); + + it("throws 404 for not-owner by default (no existence leak)", async () => { + getRoute.mockResolvedValue({ id: "r1", ownerId: "someone-else" }); + expect(await statusOf(requireOwnedRoute("r1", USER))).toBe(404); + }); + + it("throws 403 for not-owner when the surface opts in", async () => { + getActivity.mockResolvedValue({ id: "a1", ownerId: "someone-else" }); + expect(await statusOf(requireOwnedActivity("a1", USER, { notOwnerStatus: 403 }))).toBe(403); + }); + + it("returns the entity when owned", async () => { + const activity = { id: "a1", ownerId: USER }; + getActivity.mockResolvedValue(activity); + expect(await requireOwnedActivity("a1", USER)).toBe(activity); + }); +}); + +describe("vouchOwnership", () => { + it("passes the entity through unchanged (brand is type-level)", () => { + const route = { id: "r1", ownerId: "u9" }; + expect(vouchOwnership(route)).toBe(route); + }); +}); diff --git a/apps/journal/app/lib/ownership.server.ts b/apps/journal/app/lib/ownership.server.ts new file mode 100644 index 0000000..aa5f7ba --- /dev/null +++ b/apps/journal/app/lib/ownership.server.ts @@ -0,0 +1,88 @@ +// The single enforcement point for "does this user own this entity". +// +// Mutators in routes.server.ts / activities.server.ts take an `OwnedRef` +// — a branded {id, ownerId} that can only be produced here (or by the +// explicit `vouchOwnership` escape hatch). A handler that forgets the +// ownership check no longer compiles, instead of silently no-op'ing or +// leaking another user's data. + +import { data } from "react-router"; +import { getRoute } from "./routes.server.ts"; +import { getActivity } from "./activities.server.ts"; + +declare const ownedBrand: unique symbol; + +/** An entity verified to belong to the acting user (or an equivalent grant). */ +export type Owned = T & { readonly [ownedBrand]: true }; + +/** The minimal shape mutators need; any Owned entity satisfies it. */ +export type OwnedRef = Owned<{ id: string; ownerId: string }>; + +export type OwnershipFailure = "not_found" | "not_owner"; + +type LoadResult = { ok: true; entity: Owned } | { ok: false; reason: OwnershipFailure }; + +function check( + entity: T | null, + userId: string, +): LoadResult { + if (!entity) return { ok: false, reason: "not_found" }; + // ownerId === null covers remote-ingested content, which nobody owns locally. + if (entity.ownerId === null || entity.ownerId !== userId) { + return { ok: false, reason: "not_owner" }; + } + return { ok: true, entity: entity as Owned }; +} + +/** Non-throwing core — for callers with their own error vocabulary + * (discriminated results, apiError JSON shapes). */ +export async function loadOwnedRoute(routeId: string, userId: string) { + return check(await getRoute(routeId), userId); +} + +export async function loadOwnedActivity(activityId: string, userId: string) { + return check(await getActivity(activityId), userId); +} + +interface RequireOptions { + /** + * Status for the not-owner case. Defaults to 404 so a guessed id + * doesn't leak that the entity exists; pass 403 on surfaces where + * the entity is already known to the viewer (e.g. the edit page). + */ + notOwnerStatus?: 403 | 404; +} + +function throwFor(reason: OwnershipFailure, label: string, opts?: RequireOptions): never { + if (reason === "not_owner" && (opts?.notOwnerStatus ?? 404) === 403) { + throw data({ error: "Not authorized" }, { status: 403 }); + } + throw data({ error: `${label} not found` }, { status: 404 }); +} + +/** Throwing wrapper for web loaders/actions: 404 / 403 as `data()`. */ +export async function requireOwnedRoute(routeId: string, userId: string, opts?: RequireOptions) { + const result = await loadOwnedRoute(routeId, userId); + if (!result.ok) throwFor(result.reason, "Route", opts); + return result.entity; +} + +export async function requireOwnedActivity( + activityId: string, + userId: string, + opts?: RequireOptions, +) { + const result = await loadOwnedActivity(activityId, userId); + if (!result.ok) throwFor(result.reason, "Activity", opts); + return result.entity; +} + +/** + * Escape hatch for callers whose authorization does not come from a + * session — today only the Planner JWT callback, where a single-use + * token scoped to the route stands in for the owner. Every use should + * be greppable and justified by a comment at the call site. + */ +export function vouchOwnership(entity: T): Owned { + return entity as Owned; +} diff --git a/apps/journal/app/lib/routes.server.ts b/apps/journal/app/lib/routes.server.ts index d50e54e..664fe9a 100644 --- a/apps/journal/app/lib/routes.server.ts +++ b/apps/journal/app/lib/routes.server.ts @@ -6,6 +6,7 @@ import type { Visibility } from "@trails-cool/db/schema/journal"; import { sql } from "drizzle-orm"; import { validateGpx, writeGeom } from "./gpx-save.server.ts"; import type { GpxData } from "./gpx-save.server.ts"; +import type { OwnedRef } from "./ownership.server.ts"; export interface RouteInput { name: string; @@ -130,11 +131,8 @@ export async function listPublicRoutesForOwner(ownerId: string, limit: number = return rows.map((r) => ({ ...r, geojson: geojsonMap.get(r.id) ?? null })); } -export async function updateRoute( - id: string, - ownerId: string, - input: Partial, -) { +export async function updateRoute(route: OwnedRef, input: Partial) { + const { id, ownerId } = route; const db = getDb(); let parsed: GpxData | null = null; @@ -188,11 +186,13 @@ export async function updateRoute( }); } -export async function deleteRoute(id: string, ownerId: string) { +export async function deleteRoute(route: OwnedRef) { const db = getDb(); + // The WHERE ownerId clause stays as defense in depth even though the + // OwnedRef brand already proves ownership. const result = await db .delete(routes) - .where(and(eq(routes.id, id), eq(routes.ownerId, ownerId))) + .where(and(eq(routes.id, route.id), eq(routes.ownerId, route.ownerId))) .returning({ id: routes.id }); return result.length > 0; } diff --git a/apps/journal/app/routes/activities.$id.server.ts b/apps/journal/app/routes/activities.$id.server.ts index d75261a..dbca57c 100644 --- a/apps/journal/app/routes/activities.$id.server.ts +++ b/apps/journal/app/routes/activities.$id.server.ts @@ -12,6 +12,7 @@ import { } from "~/lib/activities.server"; import { deleteImportByActivity } from "~/lib/sync/imports.server"; import { listRoutes } from "~/lib/routes.server"; +import { requireOwnedActivity, requireOwnedRoute } from "~/lib/ownership.server"; import type { Visibility } from "@trails-cool/db/schema/journal"; const VISIBILITY_VALUES = new Set(["private", "unlisted", "public"]); @@ -65,24 +66,27 @@ export async function activityDetailAction(request: Request, id: string | undefi const intent = formData.get("intent"); if (intent === "link-route") { - const routeId = formData.get("routeId") as string; - if (routeId) { - await linkActivityToRoute(activityId, routeId, user.id); + const activity = await requireOwnedActivity(activityId, user.id); + const routeIdRaw = formData.get("routeId") as string; + if (routeIdRaw) { + const route = await requireOwnedRoute(routeIdRaw, user.id); + await linkActivityToRoute(activity, route); } return redirect(`/activities/${activityId}`); } if (intent === "create-route") { - const routeId = await createRouteFromActivity(activityId, user.id); + const activity = await requireOwnedActivity(activityId, user.id); + const routeId = await createRouteFromActivity(activity); if (routeId) return redirect(`/routes/${routeId}`); return data({ error: "No GPX data to create route from" }, { status: 400 }); } if (intent === "delete") { + const activity = await requireOwnedActivity(activityId, user.id); await deleteImportByActivity(activityId); - const deleted = await deleteActivity(activityId, user.id); - if (deleted) return redirect("/activities"); - return data({ error: "Activity not found" }, { status: 404 }); + await deleteActivity(activity); + return redirect("/activities"); } if (intent === "set-visibility") { @@ -90,8 +94,8 @@ export async function activityDetailAction(request: Request, id: string | undefi if (!raw || !VISIBILITY_VALUES.has(raw as Visibility)) { return data({ error: "Invalid visibility" }, { status: 400 }); } - const ok = await updateActivityVisibility(activityId, user.id, raw as Visibility); - if (!ok) return data({ error: "Activity not found" }, { status: 404 }); + const activity = await requireOwnedActivity(activityId, user.id); + await updateActivityVisibility(activity, raw as Visibility); return redirect(`/activities/${activityId}`); } diff --git a/apps/journal/app/routes/api.routes.$id.callback.ts b/apps/journal/app/routes/api.routes.$id.callback.ts index 2bc4f8c..98f814c 100644 --- a/apps/journal/app/routes/api.routes.$id.callback.ts +++ b/apps/journal/app/routes/api.routes.$id.callback.ts @@ -2,6 +2,7 @@ import { data } from "react-router"; import type { Route } from "./+types/api.routes.$id.callback"; import { verifyRouteToken } from "~/lib/jwt.server"; import { updateRoute, getRoute } from "~/lib/routes.server"; +import { vouchOwnership } from "~/lib/ownership.server"; import { GpxValidationError } from "~/lib/gpx-save.server"; const PLANNER_ORIGIN = process.env.PLANNER_URL ?? "http://localhost:3001"; @@ -61,8 +62,10 @@ export async function action({ params, request }: Route.ActionArgs) { return data({ error: "Missing GPX data" }, { status: 400, headers: corsHeaders() }); } - // Update route with new GPX (creates new version) - await updateRoute(params.id, route.ownerId, { gpx }); + // Update route with new GPX (creates new version). Authorization + // here comes from the verified single-use route token, not a + // session — vouchOwnership marks that explicitly. + await updateRoute(vouchOwnership(route), { gpx }); return data({ success: true, routeId: params.id }, { headers: corsHeaders() }); } catch (e) { diff --git a/apps/journal/app/routes/api.routes.$id.edit-in-planner.ts b/apps/journal/app/routes/api.routes.$id.edit-in-planner.ts index d694e53..ad41964 100644 --- a/apps/journal/app/routes/api.routes.$id.edit-in-planner.ts +++ b/apps/journal/app/routes/api.routes.$id.edit-in-planner.ts @@ -2,16 +2,14 @@ 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"; +import { requireOwnedRoute } from "~/lib/ownership.server"; import { createRouteToken } from "~/lib/jwt.server"; export async function action({ params, request }: Route.ActionArgs) { const user = await getSessionUser(request); if (!user) return data({ error: "Not authenticated" }, { status: 401 }); - const route = await getRouteWithVersions(params.id); - if (!route) return data({ error: "Route not found" }, { status: 404 }); - if (route.ownerId !== user.id) return data({ error: "Not authorized" }, { status: 403 }); + const route = await requireOwnedRoute(params.id, user.id, { notOwnerStatus: 403 }); const token = await createRouteToken(params.id); const origin = getOrigin(); diff --git a/apps/journal/app/routes/api.v1.activities.$id.ts b/apps/journal/app/routes/api.v1.activities.$id.ts index 82af35e..41c36c1 100644 --- a/apps/journal/app/routes/api.v1.activities.$id.ts +++ b/apps/journal/app/routes/api.v1.activities.$id.ts @@ -1,6 +1,7 @@ import type { Route } from "./+types/api.v1.activities.$id"; import { requireApiUser, apiError } from "~/lib/api-guard.server"; import { getActivity, deleteActivity } from "~/lib/activities.server"; +import { loadOwnedActivity } from "~/lib/ownership.server"; import { ERROR_CODES } from "@trails-cool/api"; /** GET /api/v1/activities/:id — full activity detail */ @@ -33,9 +34,10 @@ export async function action({ request, params }: Route.ActionArgs) { if (request.method !== "DELETE") return new Response(null, { status: 405 }); const user = await requireApiUser(request); - const deleted = await deleteActivity(params.id, user.id); - if (!deleted) { + const result = await loadOwnedActivity(params.id, user.id); + if (!result.ok) { return apiError(404, ERROR_CODES.NOT_FOUND, "Activity not found"); } + await deleteActivity(result.entity); return new Response(null, { status: 204 }); } diff --git a/apps/journal/app/routes/api.v1.routes.$id.ts b/apps/journal/app/routes/api.v1.routes.$id.ts index 86e3690..c2c41a1 100644 --- a/apps/journal/app/routes/api.v1.routes.$id.ts +++ b/apps/journal/app/routes/api.v1.routes.$id.ts @@ -1,6 +1,7 @@ import type { Route } from "./+types/api.v1.routes.$id"; import { requireApiUser, apiError } from "~/lib/api-guard.server"; import { getRouteWithVersions, updateRoute, deleteRoute } from "~/lib/routes.server"; +import { loadOwnedRoute } from "~/lib/ownership.server"; import { UpdateRouteRequestSchema, ERROR_CODES, zodIssuesToFieldErrors } from "@trails-cool/api"; /** GET /api/v1/routes/:id — full route detail */ @@ -48,15 +49,20 @@ export async function action({ request, params }: Route.ActionArgs) { zodIssuesToFieldErrors(parsed.error)); } - await updateRoute(params.id, user.id, parsed.data); + const result = await loadOwnedRoute(params.id, user.id); + if (!result.ok) { + return apiError(404, ERROR_CODES.NOT_FOUND, "Route not found"); + } + await updateRoute(result.entity, parsed.data); return Response.json({ ok: true }); } if (request.method === "DELETE") { - const deleted = await deleteRoute(params.id, user.id); - if (!deleted) { + const result = await loadOwnedRoute(params.id, user.id); + if (!result.ok) { return apiError(404, ERROR_CODES.NOT_FOUND, "Route not found"); } + await deleteRoute(result.entity); return new Response(null, { status: 204 }); } diff --git a/apps/journal/app/routes/api.v1.routes.test.ts b/apps/journal/app/routes/api.v1.routes.test.ts index b3c6afb..88f88ac 100644 --- a/apps/journal/app/routes/api.v1.routes.test.ts +++ b/apps/journal/app/routes/api.v1.routes.test.ts @@ -7,6 +7,7 @@ const mockGetAuthenticatedUser = vi.fn(); const mockListRoutes = vi.fn(); const mockCreateRoute = vi.fn(); const mockGetRouteWithVersions = vi.fn(); +const mockGetRoute = vi.fn(); const mockDeleteRoute = vi.fn(); vi.mock("~/lib/db", () => ({ getDb: vi.fn() })); @@ -19,6 +20,7 @@ vi.mock("~/lib/routes.server", () => ({ listRoutes: mockListRoutes, createRoute: mockCreateRoute, getRouteWithVersions: mockGetRouteWithVersions, + getRoute: mockGetRoute, updateRoute: vi.fn(), deleteRoute: mockDeleteRoute, })); @@ -146,6 +148,7 @@ describe("GET /api/v1/routes/:id", () => { describe("DELETE /api/v1/routes/:id", () => { it("returns 204 on success", async () => { + mockGetRoute.mockResolvedValue({ id: "r1", ownerId: "user-1" }); mockDeleteRoute.mockResolvedValue(true); const { action } = await import("./api.v1.routes.$id.ts"); const resp = await action(routeArgs( @@ -156,7 +159,7 @@ describe("DELETE /api/v1/routes/:id", () => { }); it("returns 404 if not found", async () => { - mockDeleteRoute.mockResolvedValue(false); + mockGetRoute.mockResolvedValue(null); const { action } = await import("./api.v1.routes.$id.ts"); const resp = await action(routeArgs( authRequest("/api/v1/routes/missing", { method: "DELETE" }), { id: "missing" }, "api/v1/routes/:id", diff --git a/apps/journal/app/routes/routes.$id.edit.server.ts b/apps/journal/app/routes/routes.$id.edit.server.ts index b3ed830..379438c 100644 --- a/apps/journal/app/routes/routes.$id.edit.server.ts +++ b/apps/journal/app/routes/routes.$id.edit.server.ts @@ -1,8 +1,9 @@ // Server-only loader/action for /routes/:id/edit. See `home.server.ts`. -import { data, redirect } from "react-router"; +import { redirect } from "react-router"; import { requireSessionUser } from "~/lib/auth/session.server"; -import { getRoute, updateRoute } from "~/lib/routes.server"; +import { updateRoute } from "~/lib/routes.server"; +import { requireOwnedRoute } from "~/lib/ownership.server"; import type { Visibility } from "@trails-cool/db/schema/journal"; const VISIBILITY_VALUES = new Set(["private", "unlisted", "public"]); @@ -10,9 +11,7 @@ const VISIBILITY_VALUES = new Set(["private", "unlisted", "public"]) export async function loadRouteEdit(request: Request, id: string | undefined) { const user = await requireSessionUser(request); - const route = await getRoute(id ?? ""); - if (!route) throw data({ error: "Route not found" }, { status: 404 }); - if (route.ownerId !== user.id) throw data({ error: "Not authorized" }, { status: 403 }); + const route = await requireOwnedRoute(id ?? "", user.id, { notOwnerStatus: 403 }); return { route: { @@ -44,6 +43,7 @@ export async function routeEditAction(request: Request, id: string | undefined) input.visibility = visibilityRaw as Visibility; } - await updateRoute(routeId, user.id, input); + const route = await requireOwnedRoute(routeId, user.id, { notOwnerStatus: 403 }); + await updateRoute(route, input); return redirect(`/routes/${routeId}`); } diff --git a/apps/journal/app/routes/routes.$id.server.ts b/apps/journal/app/routes/routes.$id.server.ts index f4fe947..acb0534 100644 --- a/apps/journal/app/routes/routes.$id.server.ts +++ b/apps/journal/app/routes/routes.$id.server.ts @@ -5,6 +5,7 @@ import { and, eq } from "drizzle-orm"; import { canView } from "~/lib/auth.server"; import { getSessionUser, requireSessionUser } from "~/lib/auth/session.server"; import { getRoute, getRouteWithVersions, deleteRoute, updateRoute } from "~/lib/routes.server"; +import { requireOwnedRoute } from "~/lib/ownership.server"; import { getDb } from "~/lib/db"; import { syncPushes } from "@trails-cool/db/schema/journal"; import { getService } from "~/lib/connected-services"; @@ -146,7 +147,8 @@ export async function routeDetailAction(request: Request, id: string | undefined const intent = formData.get("intent"); if (intent === "delete") { - await deleteRoute(routeId, user.id); + const route = await requireOwnedRoute(routeId, user.id); + await deleteRoute(route); return redirect("/routes"); } @@ -162,7 +164,8 @@ export async function routeDetailAction(request: Request, id: string | undefined input.gpx = await gpxFile.text(); } - await updateRoute(routeId, user.id, input as { name?: string; description?: string; gpx?: string }); + const route = await requireOwnedRoute(routeId, user.id); + await updateRoute(route, input as { name?: string; description?: string; gpx?: string }); return redirect(`/routes/${routeId}`); }