diff --git a/server/src/mcp/tools/trips.ts b/server/src/mcp/tools/trips.ts index 2a80d589c..71e947156 100644 --- a/server/src/mcp/tools/trips.ts +++ b/server/src/mcp/tools/trips.ts @@ -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 }); } diff --git a/server/src/nest/share/share.service.ts b/server/src/nest/share/share.service.ts index 04f275988..3940e5bcd 100644 --- a/server/src/nest/share/share.service.ts +++ b/server/src/nest/share/share.service.ts @@ -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 | null { const shareRow = this.dbs.get( @@ -141,100 +149,6 @@ export class ShareService { ); if (!trip) return null; - // Days with assignments - const days = this.dbs.all('SELECT * FROM days WHERE trip_id = ? ORDER BY day_number ASC', tripId); - const dayIds = days.map(d => d.id); - - let assignments: Record = {}; - let dayNotes: Record = {}; - if (dayIds.length > 0) { - const ph = dayIds.map(() => '?').join(','); - const allAssignments = this.dbs.all(` - 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 = {}; - 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(`SELECT * FROM day_notes WHERE day_id IN (${ph}) ORDER BY sort_order ASC, created_at ASC`, ...dayIds); - const notesByDay: Record = {}; - 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(` - 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('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>(); - 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 = {}; + let dayNotes: Record = {}; + let places: any[] = []; + if (permissions.share_map) { + days = this.dbs.all('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(` + 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 = {}; + 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(`SELECT * FROM day_notes WHERE day_id IN (${ph}) ORDER BY sort_order ASC, created_at ASC`, ...dayIds); + const notesByDay: Record = {}; + 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(` + 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>(); + 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('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, }; } diff --git a/server/tests/unit/nest/share.service.test.ts b/server/tests/unit/nest/share.service.test.ts index ba5a140e2..41e03af4a 100644 --- a/server/tests/unit/nest/share.service.test.ts +++ b/server/tests/unit/nest/share.service.test.ts @@ -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); }); });