k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(server): harden share links — transactional writes, TTL renewal on update, scoped shared queries

Browse Source
This commit is contained in:
jubnl
2026-07-26 21:25:47 +02:00
parent d572b1ec8d
commit c9a9df5310
3 changed files with 149 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/mcp/tools/trips.ts
+3 -5
View File
@@ -367,12 +367,10 @@ export function registerTripTools(server: McpServer, userId: number, scopes: str
if (isDemoUser(userId)) return demoDenied();
if (!canAccessTrip(tripId, userId)) return noAccess();
if (!hasTripPermission('share_manage', tripId, userId)) return permissionDenied();
// The zod .default()s above fill omitted flags, and ShareService applies
// the same defaults again for undefined — no re-defaulting needed here.
const { token, created } = createOrUpdateShareLink(String(tripId), userId, {
share_map: share_map ?? true,
share_bookings: share_bookings ?? true,
share_packing: share_packing ?? false,
share_budget: share_budget ?? false,
share_collab: share_collab ?? false,
share_map, share_bookings, share_packing, share_budget, share_collab,
});
return ok({ token, created });
}
server/src/nest/share/share.service.ts
+137 -128
View File
@@ -47,8 +47,8 @@ export interface ShareTokenInfo {
/**
* Public share links — the legacy shareService SQL folded in over the injected
* DatabaseService (byte-identical statements and coercions). Trip access and
* the 'share_manage' permission gate create/delete; the shared read is public.
* DatabaseService. Trip access and the 'share_manage' permission gate
* create/delete; the shared read is public.
*/
@Injectable()
export class ShareService {
@@ -65,6 +65,11 @@ export class ShareService {
/**
* Creates a new share link or updates the permissions on an existing one.
* Returns an object with the token string and whether it was newly created.
*
* Share links carry a 90-day TTL; updating an existing link renews it, so a
* link the owner is actively managing never expires under them. Rows created
* before the expires_at migration keep NULL until touched and remain valid
* indefinitely; an explicit update moves them onto the TTL.
*/
createOrUpdate(tripId: string, userId: number, permissions: SharePermissions): { token: string; created: boolean } {
const {
@@ -75,26 +80,24 @@ export class ShareService {
share_collab = false,
} = permissions;
const existing = this.dbs.get<{ token: string }>('SELECT token FROM share_tokens WHERE trip_id = ?', tripId);
if (existing) {
this.dbs.run(
'UPDATE share_tokens SET share_map = ?, share_bookings = ?, share_packing = ?, share_budget = ?, share_collab = ? WHERE trip_id = ?',
share_map ? 1 : 0, share_bookings ? 1 : 0, share_packing ? 1 : 0, share_budget ? 1 : 0, share_collab ? 1 : 0, tripId,
);
return { token: existing.token, created: false };
}
// New share links default to a 90-day TTL. Existing tokens that were
// created before the expires_at migration keep NULL here and remain
// valid indefinitely until the owner rotates them; that preserves
// behaviour for anyone who's already sharing a link.
const token = crypto.randomBytes(24).toString('base64url');
const expiresAt = new Date(Date.now() + 90 * 24 * 60 * 60 * 1000).toISOString();
this.dbs.run(
'INSERT INTO share_tokens (trip_id, token, created_by, share_map, share_bookings, share_packing, share_budget, share_collab, expires_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)',
tripId, token, userId, share_map ? 1 : 0, share_bookings ? 1 : 0, share_packing ? 1 : 0, share_budget ? 1 : 0, share_collab ? 1 : 0, expiresAt,
);
return { token, created: true };
return this.dbs.transaction(() => {
const existing = this.dbs.get<{ token: string }>('SELECT token FROM share_tokens WHERE trip_id = ?', tripId);
if (existing) {
this.dbs.run(
'UPDATE share_tokens SET share_map = ?, share_bookings = ?, share_packing = ?, share_budget = ?, share_collab = ?, expires_at = ? WHERE trip_id = ?',
share_map ? 1 : 0, share_bookings ? 1 : 0, share_packing ? 1 : 0, share_budget ? 1 : 0, share_collab ? 1 : 0, expiresAt, tripId,
);
return { token: existing.token, created: false };
}
const token = crypto.randomBytes(24).toString('base64url');
this.dbs.run(
'INSERT INTO share_tokens (trip_id, token, created_by, share_map, share_bookings, share_packing, share_budget, share_collab, expires_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)',
tripId, token, userId, share_map ? 1 : 0, share_bookings ? 1 : 0, share_packing ? 1 : 0, share_budget ? 1 : 0, share_collab ? 1 : 0, expiresAt,
);
return { token, created: true };
});
}
/**
@@ -124,6 +127,11 @@ export class ShareService {
/**
* Loads the full public trip data for a share token, filtered by the token's
* permission flags. Returns null if the token is invalid or the trip is gone.
*
* Every share flag is honoured server-side — the client gates these too, but
* it must not rely on that (mirrors journeyShareService). A withheld section
* is never even queried. share_map covers the whole itinerary: days, their
* assignments/notes, and the place list with coordinates, addresses and notes.
*/
getSharedTripData(token: string): Record<string, any> | null {
const shareRow = this.dbs.get<any>(
@@ -141,100 +149,6 @@ export class ShareService {
);
if (!trip) return null;
// Days with assignments
const days = this.dbs.all<any>('SELECT * FROM days WHERE trip_id = ? ORDER BY day_number ASC', tripId);
const dayIds = days.map(d => d.id);
let assignments: Record<number, any[]> = {};
let dayNotes: Record<number, any[]> = {};
if (dayIds.length > 0) {
const ph = dayIds.map(() => '?').join(',');
const allAssignments = this.dbs.all<any>(`
SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description,
p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency,
COALESCE(da.assignment_time, p.place_time) as place_time,
COALESCE(da.assignment_end_time, p.end_time) as end_time,
p.duration_minutes, p.notes as place_notes, p.image_url, p.transport_mode,
c.name as category_name, c.color as category_color, c.icon as category_icon
FROM day_assignments da
JOIN places p ON da.place_id = p.id
LEFT JOIN categories c ON p.category_id = c.id
WHERE da.day_id IN (${ph})
ORDER BY da.order_index ASC, da.created_at ASC
`, ...dayIds);
const placeIds = [...new Set(allAssignments.map((a: any) => a.place_id))];
const tagsByPlace = loadTagsByPlaceIds(placeIds, { compact: true });
const byDay: Record<number, any[]> = {};
for (const a of allAssignments as any[]) {
if (!byDay[a.day_id]) byDay[a.day_id] = [];
byDay[a.day_id].push({
id: a.id, day_id: a.day_id, order_index: a.order_index, notes: a.notes,
place: {
id: a.place_id, name: a.place_name, description: a.place_description,
lat: a.lat, lng: a.lng, address: a.address, category_id: a.category_id,
price: a.price, place_time: a.place_time, end_time: a.end_time,
image_url: rewritePlacePhotoUrl(a.image_url, token), transport_mode: a.transport_mode,
category: a.category_id ? { id: a.category_id, name: a.category_name, color: a.category_color, icon: a.category_icon } : null,
tags: tagsByPlace[a.place_id] || [],
}
});
}
assignments = byDay;
const allNotes = this.dbs.all<any>(`SELECT * FROM day_notes WHERE day_id IN (${ph}) ORDER BY sort_order ASC, created_at ASC`, ...dayIds);
const notesByDay: Record<number, any[]> = {};
for (const n of allNotes as any[]) {
if (!notesByDay[n.day_id]) notesByDay[n.day_id] = [];
notesByDay[n.day_id].push(n);
}
dayNotes = notesByDay;
}
// Places
const places = this.dbs.all<any>(`
SELECT p.*, c.name as category_name, c.color as category_color, c.icon as category_icon
FROM places p LEFT JOIN categories c ON p.category_id = c.id
WHERE p.trip_id = ? ORDER BY p.created_at DESC
`, tripId).map((p) => ({ ...p, image_url: rewritePlacePhotoUrl(p.image_url, token) }));
// Reservations — include per-day positions so the client can render the same order as the planner
const reservations = this.dbs.all<any>('SELECT * FROM reservations WHERE trip_id = ? ORDER BY reservation_time ASC', tripId);
const dayPositions = this.dbs.all<{ reservation_id: number; day_id: number; position: number }>(`
SELECT rdp.reservation_id, rdp.day_id, rdp.position
FROM reservation_day_positions rdp
JOIN reservations r ON rdp.reservation_id = r.id
WHERE r.trip_id = ?
`, tripId);
const posMap = new Map<number, Record<number, number>>();
for (const dp of dayPositions) {
if (!posMap.has(dp.reservation_id)) posMap.set(dp.reservation_id, {});
posMap.get(dp.reservation_id)![dp.day_id] = dp.position;
}
for (const r of reservations) {
r.day_positions = posMap.get(r.id) || null;
}
// Accommodations
const accommodations = this.dbs.all(`
SELECT a.*, p.name as place_name, p.address as place_address, p.lat as place_lat, p.lng as place_lng
FROM day_accommodations a JOIN places p ON a.place_id = p.id
WHERE a.trip_id = ?
`, tripId);
// Packing — a public viewer is neither owner nor recipient, so only Common items
// may surface; never a co-member's private/personal packing items (#858).
const packing = this.dbs.all('SELECT * FROM packing_items WHERE trip_id = ? AND is_private = 0 ORDER BY sort_order ASC', tripId);
// Budget
const budget = this.dbs.all('SELECT * FROM budget_items WHERE trip_id = ? ORDER BY category ASC', tripId);
// Categories
const categories = this.dbs.all('SELECT * FROM categories');
const permissions = {
share_map: !!shareRow.share_map,
share_bookings: !!shareRow.share_bookings,
@@ -243,6 +157,109 @@ export class ShareService {
share_collab: !!shareRow.share_collab,
};
// Itinerary — days with assignments/notes, and the place pool
let days: any[] = [];
let assignments: Record<number, any[]> = {};
let dayNotes: Record<number, any[]> = {};
let places: any[] = [];
if (permissions.share_map) {
days = this.dbs.all<any>('SELECT * FROM days WHERE trip_id = ? ORDER BY day_number ASC', tripId);
const dayIds = days.map(d => d.id);
if (dayIds.length > 0) {
const ph = dayIds.map(() => '?').join(',');
const allAssignments = this.dbs.all<any>(`
SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description,
p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency,
COALESCE(da.assignment_time, p.place_time) as place_time,
COALESCE(da.assignment_end_time, p.end_time) as end_time,
p.duration_minutes, p.notes as place_notes, p.image_url, p.transport_mode,
c.name as category_name, c.color as category_color, c.icon as category_icon
FROM day_assignments da
JOIN places p ON da.place_id = p.id
LEFT JOIN categories c ON p.category_id = c.id
WHERE da.day_id IN (${ph})
ORDER BY da.order_index ASC, da.created_at ASC
`, ...dayIds);
const placeIds = [...new Set(allAssignments.map((a: any) => a.place_id))];
const tagsByPlace = loadTagsByPlaceIds(placeIds, { compact: true });
const byDay: Record<number, any[]> = {};
for (const a of allAssignments as any[]) {
if (!byDay[a.day_id]) byDay[a.day_id] = [];
byDay[a.day_id].push({
id: a.id, day_id: a.day_id, order_index: a.order_index, notes: a.notes,
place: {
id: a.place_id, name: a.place_name, description: a.place_description,
lat: a.lat, lng: a.lng, address: a.address, category_id: a.category_id,
price: a.price, place_time: a.place_time, end_time: a.end_time,
image_url: rewritePlacePhotoUrl(a.image_url, token), transport_mode: a.transport_mode,
category: a.category_id ? { id: a.category_id, name: a.category_name, color: a.category_color, icon: a.category_icon } : null,
tags: tagsByPlace[a.place_id] ?? [],
}
});
}
assignments = byDay;
const allNotes = this.dbs.all<any>(`SELECT * FROM day_notes WHERE day_id IN (${ph}) ORDER BY sort_order ASC, created_at ASC`, ...dayIds);
const notesByDay: Record<number, any[]> = {};
for (const n of allNotes as any[]) {
if (!notesByDay[n.day_id]) notesByDay[n.day_id] = [];
notesByDay[n.day_id].push(n);
}
dayNotes = notesByDay;
}
places = this.dbs.all<any>(`
SELECT p.*, c.name as category_name, c.color as category_color, c.icon as category_icon
FROM places p LEFT JOIN categories c ON p.category_id = c.id
WHERE p.trip_id = ? ORDER BY p.created_at DESC
`, tripId).map((p) => ({ ...p, image_url: rewritePlacePhotoUrl(p.image_url, token) }));
}
// Bookings — reservations carry per-day positions so the client can render
// the same order as the planner
let reservations: any[] = [];
let accommodations: unknown[] = [];
if (permissions.share_bookings) {
const dayPositions = this.dbs.all<{ reservation_id: number; day_id: number; position: number }>(`
SELECT rdp.reservation_id, rdp.day_id, rdp.position
FROM reservation_day_positions rdp
JOIN reservations r ON rdp.reservation_id = r.id
WHERE r.trip_id = ?
`, tripId);
const posMap = new Map<number, Record<number, number>>();
for (const dp of dayPositions) {
if (!posMap.has(dp.reservation_id)) posMap.set(dp.reservation_id, {});
posMap.get(dp.reservation_id)![dp.day_id] = dp.position;
}
reservations = this.dbs.all<any>('SELECT * FROM reservations WHERE trip_id = ? ORDER BY reservation_time ASC', tripId)
.map((r) => ({ ...r, day_positions: posMap.get(r.id) ?? null }));
accommodations = this.dbs.all(`
SELECT a.*, p.name as place_name, p.address as place_address, p.lat as place_lat, p.lng as place_lng
FROM day_accommodations a JOIN places p ON a.place_id = p.id
WHERE a.trip_id = ?
`, tripId);
}
// Packing — a public viewer is neither owner nor recipient, so only Common items
// may surface; never a co-member's private/personal packing items (#858).
const packing = permissions.share_packing
? this.dbs.all('SELECT * FROM packing_items WHERE trip_id = ? AND is_private = 0 ORDER BY sort_order ASC', tripId)
: [];
// Budget
const budget = permissions.share_budget
? this.dbs.all('SELECT * FROM budget_items WHERE trip_id = ? ORDER BY category ASC', tripId)
: [];
// Categories are a shared global pool (the authed /api/categories list is
// equally unscoped), so the public payload returns them all too.
const categories = this.dbs.all('SELECT * FROM categories');
// Collab messages (only if owner chose to share)
const collabMessages = permissions.share_collab
? this.dbs.all('SELECT m.*, u.username, u.avatar FROM collab_messages m JOIN users u ON m.user_id = u.id WHERE m.trip_id = ? AND m.deleted = 0 ORDER BY m.created_at', tripId)
@@ -254,7 +271,8 @@ export class ShareService {
// payload and used by the client to convert every expense — otherwise guests
// fall back to the trip's base currency and see the wrong totals (#1361).
// getUserSettings merges admin defaults under the user's own settings, so this
// honours per-user → admin-default; we then fall back to trip currency → EUR.
// honours per-user → admin-default; we then fall back to trip currency → EUR
// (`||` on purpose: an empty-string trip currency also falls back).
let baseCurrency = (trip as { currency?: string }).currency || 'EUR';
if (shareRow.created_by != null) {
const ownerDefault = getUserSettings(shareRow.created_by)['default_currency'];
@@ -263,20 +281,11 @@ export class ShareService {
}
}
// Honour every share flag server-side — the client gates these too, but it must
// not rely on that (mirrors journeyShareService). share_map covers the whole
// itinerary: days, their assignments/notes, and the place list with coordinates,
// addresses and notes. Withhold it when the owner disabled the map.
return {
trip, baseCurrency, categories, permissions,
days: permissions.share_map ? days : [],
assignments: permissions.share_map ? assignments : {},
dayNotes: permissions.share_map ? dayNotes : {},
places: permissions.share_map ? places : [],
reservations: permissions.share_bookings ? reservations : [],
accommodations: permissions.share_bookings ? accommodations : [],
packing: permissions.share_packing ? packing : [],
budget: permissions.share_budget ? budget : [],
days, assignments, dayNotes, places,
reservations, accommodations,
packing, budget,
collab: collabMessages,
};
}
server/tests/unit/nest/share.service.test.ts
+9 -3
View File
@@ -155,17 +155,23 @@ describe('createOrUpdate', () => {
expect(shareRow(trip.id).share_budget).toBe(1);
});
it('SHARE-SVC-006: the update path re-applies the defaults for omitted flags and never touches expires_at', () => {
it('SHARE-SVC-006: the update path re-applies the defaults for omitted flags and renews the 90-day expiry', () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
svc.createOrUpdate(String(trip.id), user.id, { share_packing: true });
testDb.prepare('UPDATE share_tokens SET expires_at = ? WHERE trip_id = ?').run('2099-01-01T00:00:00.000Z', trip.id);
// Simulate a legacy pre-TTL row (NULL expiry): an explicit update moves it
// onto the 90-day clock.
testDb.prepare('UPDATE share_tokens SET expires_at = NULL WHERE trip_id = ?').run(trip.id);
const before = Date.now();
svc.createOrUpdate(String(trip.id), user.id, {});
const row = shareRow(trip.id);
// Omitted share_packing fell back to its default (off) — destructuring
// defaults apply on every call, not only on create.
expect(row.share_packing).toBe(0);
expect(row.expires_at).toBe('2099-01-01T00:00:00.000Z');
const ninetyDays = 90 * 24 * 60 * 60 * 1000;
const expires = new Date(row.expires_at).getTime();
expect(expires).toBeGreaterThanOrEqual(before + ninetyDays - 5000);
expect(expires).toBeLessThanOrEqual(Date.now() + ninetyDays + 5000);
});
});