diff --git a/server/src/db/repositories/BudgetItems.repository.ts b/server/src/db/repositories/BudgetItems.repository.ts index 8f97fb915..b620a8702 100644 --- a/server/src/db/repositories/BudgetItems.repository.ts +++ b/server/src/db/repositories/BudgetItems.repository.ts @@ -248,6 +248,15 @@ export class BudgetItemsRepository extends TrekRepository { return await this.kysely().selectFrom('budget_items').selectAll().where('trip_id', '=', trip_id as number).execute(); } + /** + * `share.service.ts:391` SH14 (`getSharedTripData`'s share_budget read) — + * `SELECT * FROM budget_items WHERE trip_id = ? ORDER BY category ASC` + * (unlike BG72's `listAllForTrip` above, which has no `ORDER BY`). + */ + async listPublicForShare(trip_id: number | string): Promise { + return await this.kysely().selectFrom('budget_items').selectAll().where('trip_id', '=', trip_id as number).orderBy('category', 'asc').execute(); + } + /** * BG71 (`getPerPersonSummary`) — the correlated-subquery-inside-`SUM` * aggregate (T6, per the plan's own ruling): `SELECT bm.user_id, diff --git a/server/src/db/repositories/Categories.repository.ts b/server/src/db/repositories/Categories.repository.ts index 13cc11377..c2ac06d2a 100644 --- a/server/src/db/repositories/Categories.repository.ts +++ b/server/src/db/repositories/Categories.repository.ts @@ -21,6 +21,18 @@ export class CategoriesRepository extends TrekRepository { return categories.map((category) => toRow(category) as CategoryRow); } + /** + * `share.service.ts:396` SH15 (`getSharedTripData`) — `SELECT * FROM + * categories`, no `ORDER BY` (unlike `list()` above, which adds `ORDER BY + * name ASC` for the authenticated `/api/categories` list — the legacy + * public-share statement never sorted). Unscoped by trip on purpose: "a + * shared global pool", the same statement the authed list reads from. + */ + async listAllUnordered(): Promise { + const categories = await this.find({}); + return categories.map((category) => toRow(category) as CategoryRow); + } + /** * PL33 (`PlacesService.importKmlPlaces`) — `SELECT id, name FROM * categories`, the id/name-only projection the KML importer folds into diff --git a/server/src/db/repositories/CollabMessages.repository.ts b/server/src/db/repositories/CollabMessages.repository.ts index 4ec5b1a45..fc74dc207 100644 --- a/server/src/db/repositories/CollabMessages.repository.ts +++ b/server/src/db/repositories/CollabMessages.repository.ts @@ -24,6 +24,12 @@ export interface CollabMessageJoinRow extends CollabMessageRow { reply_username: string | null; } +/** {@link CollabMessagesRepository.listPublicForShare}'s projection (SH16) — `m.*, u.username, u.avatar` only. */ +export interface SharePublicCollabMessageRow extends CollabMessageRow { + username: string; + avatar: string | null; +} + interface CollabMessagesKyselyDB { collab_messages: CollabMessageRow; users: { id: number; username: string; avatar: string | null }; @@ -117,6 +123,27 @@ export class CollabMessagesRepository extends TrekRepository { return await this.joinedQuery().where('m.id', '=', id).executeTakeFirst(); } + /** + * `share.service.ts:400` SH16 (`getSharedTripData`'s share_collab read) — + * `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`. A narrower projection than {@link joinedQuery} above (no + * `reply_text`/`reply_username` — a public share viewer never resolves a + * reply thread through this read) and a different order/shape than + * `listForTrip` (ascending, no cursor, no `LIMIT`). + */ + async listPublicForShare(trip_id: number): Promise { + return await this.kysely() + .selectFrom('collab_messages as m') + .innerJoin('users as u', 'u.id', 'm.user_id') + .selectAll('m') + .select(['u.username', 'u.avatar']) + .where('m.trip_id', '=', trip_id) + .where('m.deleted', '=', 0) + .orderBy('m.created_at', 'asc') + .execute(); + } + /** * CB2/CB51 (`reactMessage`'s existence guard, `deleteMessage`'s * trip-scoping + owner-check read) — `SELECT * FROM collab_messages WHERE diff --git a/server/src/db/repositories/CollectionPlaces.repository.ts b/server/src/db/repositories/CollectionPlaces.repository.ts index 4131e392f..23d0b762b 100644 --- a/server/src/db/repositories/CollectionPlaces.repository.ts +++ b/server/src/db/repositories/CollectionPlaces.repository.ts @@ -418,6 +418,39 @@ export class CollectionPlacesRepository extends TrekRepository .executeTakeFirst(); return Number(result.numInsertedOrUpdatedRows ?? 0); } + + // --------------------------------------------------------------------- + // Plan 3h Task 6 (survivors — `places.service.ts` SV-PI2, `place-photo- + // cache.service.ts` SV-PP6, `place-image.ts` SV-PLACEIMG-2) — additive. + // --------------------------------------------------------------------- + + /** + * SV-PI2 (`places.service.ts::reclaimPlaceImage`) / SV-PLACEIMG-2 + * (`CollectionsService`'s own private `reclaimPlaceImage`, replacing + * `place-image.ts`'s free function) — `SELECT 1 FROM collection_places + * WHERE image_url = ? LIMIT 1`. Trip-agnostic, same shape as + * `PlacesRepository.existsByImageUrl` for its own table. + */ + async existsByImageUrl(imageUrl: string): Promise { + const row = await this.db_().selectFrom('collection_places').select('id').where('image_url', '=', imageUrl).limit(1).executeTakeFirst(); + return !!row; + } + + /** + * SV-PP6 (`place-photo-cache.service.ts::isReferenced`) — `SELECT 1 FROM + * collection_places WHERE google_place_id = ? OR image_url = ? LIMIT 1`, + * the same shape `PlacesRepository.existsByGoogleIdOrImageUrl` uses for + * its own table. + */ + async existsByGoogleIdOrImageUrl(googlePlaceId: string, imageUrl: string): Promise { + const row = await this.db_() + .selectFrom('collection_places') + .select('id') + .where((eb) => eb.or([eb('google_place_id', '=', googlePlaceId), eb('image_url', '=', imageUrl)])) + .limit(1) + .executeTakeFirst(); + return !!row; + } } /** {@link CollectionPlacesRepository.findForCopy}'s narrow projection (CL62). */ diff --git a/server/src/db/repositories/DayAssignments.repository.ts b/server/src/db/repositories/DayAssignments.repository.ts index d7eb81790..36b92d7f0 100644 --- a/server/src/db/repositories/DayAssignments.repository.ts +++ b/server/src/db/repositories/DayAssignments.repository.ts @@ -63,6 +63,16 @@ export interface AssignmentWithPlaceRow extends DayAssignmentRow { category_icon: string | null; } +/** + * {@link DayAssignmentsRepository.listPublicForShare}'s projection (SH9) — + * {@link AssignmentWithPlaceRow} minus the six columns a public share link + * must not leak. + */ +export type SharePublicAssignmentRow = Omit< + AssignmentWithPlaceRow, + 'google_place_id' | 'google_ftid' | 'osm_id' | 'amap_poi_id' | 'stop_type' | 'fill_percent' +>; + /** * `day_assignments`'s shape as DY24's `em.getKysely()` call needs it (an * explicit `TDB` type argument, per `WebauthnChallengesRepository @@ -308,6 +318,53 @@ export class DayAssignmentsRepository extends TrekRepository { .execute('all', false); } + /** + * `share.service.ts:288` SH9 (`getSharedTripData`'s share_map assignment + * read) — the SAME projection {@link assignmentWithPlaceSelect} builds, + * minus six internal-id/routing columns a public share link must never + * leak (`google_place_id`, `google_ftid`, `osm_id`, `amap_poi_id`, + * `stop_type`, `fill_percent`) — never widen this list to match the + * private `listWithPlaceAndCategory` projection above. + */ + private publicAssignmentSelect(platform: Platform) { + return [ + 'da.*', + 'p.id as place_id', + 'p.name as place_name', + 'p.description as place_description', + 'p.lat', + 'p.lng', + 'p.address', + columnRef(platform, 'p.category_id'), + 'p.price', + 'p.currency as place_currency', + coalesce(platform, 'da.assignment_time', 'p.place_time').as('place_time'), + coalesce(platform, 'da.assignment_end_time', 'p.end_time').as('end_time'), + 'p.duration_minutes', + 'p.notes as place_notes', + 'p.image_url', + 'p.transport_mode', + 'p.website', + 'p.phone', + 'c.name as category_name', + 'c.color as category_color', + 'c.icon as category_icon', + ] as const; + } + + /** SH9 — same WHERE/ORDER BY shape as {@link listWithPlaceAndCategory}, narrower projection. */ + async listPublicForShare(day_ids: number[]): Promise { + if (day_ids.length === 0) return []; + const platform = this.getEntityManager().getPlatform(); + return await this.qb('da') + .join('da.place', 'p') + .leftJoin('p.category', 'c') + .select(this.publicAssignmentSelect(platform)) + .where({ 'da.day': { $in: day_ids } }) + .orderBy({ 'da.order_index': 'asc', 'da.created_at': 'asc' }) + .execute('all', false); + } + /** * DY24 (`days.service.ts::resyncAccommodationDays`'s `moveStayStop`, * inventory §18.2) — a correlated scalar subquery in an UPDATE SET clause diff --git a/server/src/db/repositories/PackingItems.repository.ts b/server/src/db/repositories/PackingItems.repository.ts index 11097c2e6..1deb5d916 100644 --- a/server/src/db/repositories/PackingItems.repository.ts +++ b/server/src/db/repositories/PackingItems.repository.ts @@ -117,6 +117,25 @@ export class PackingItemsRepository extends TrekRepository { return await this.db().selectFrom('packing_items').selectAll().where('trip_id', '=', trip_id as number).orderBy('sort_order', 'asc').orderBy('created_at', 'asc').execute(); } + /** + * `share.service.ts:386` SH13 (`getSharedTripData`'s share_packing read) + * — **SECURITY-CRITICAL (#858)**: `SELECT * FROM packing_items WHERE + * trip_id = ? AND is_private = 0 ORDER BY sort_order ASC` — a public + * viewer is neither owner nor recipient, so only Common items may + * surface. `ORDER BY sort_order ASC` only (unlike PK4's `listForTrip`, + * which also orders by `created_at` — the legacy public statement never + * added that second key). + */ + async listPublicForShare(trip_id: number | string): Promise { + return await this.db() + .selectFrom('packing_items') + .selectAll() + .where('trip_id', '=', trip_id as number) + .where('is_private', '=', 0) + .orderBy('sort_order', 'asc') + .execute(); + } + /** * PK5 (`listItems`'s viewer-filtered branch, T5) — `SELECT * FROM * packing_items WHERE trip_id = ? AND (${VISIBLE_TO_ACTOR}) ORDER BY diff --git a/server/src/db/repositories/Places.repository.ts b/server/src/db/repositories/Places.repository.ts index aba1dcbae..4b00fbd5d 100644 --- a/server/src/db/repositories/Places.repository.ts +++ b/server/src/db/repositories/Places.repository.ts @@ -53,6 +53,28 @@ export interface PlaceWithCategoryRow extends PlaceRow { category_icon: string | null; } +/** + * {@link PlacesRepository.listPublicForShare}'s projection (SH12) — the + * `PUBLIC_PLACE_COLUMNS` allow-list plus the category join, picked out of + * {@link PlaceWithCategoryRow} rather than re-derived (never independently + * widen this list — see that method's docstring). + */ +export type SharePublicPlaceRow = Pick< + PlaceWithCategoryRow, + | 'id' | 'trip_id' | 'name' | 'description' | 'lat' | 'lng' | 'address' | 'category_id' | 'price' | 'currency' + | 'place_time' | 'end_time' | 'duration_minutes' | 'notes' | 'image_url' | 'website' | 'phone' | 'transport_mode' + | 'created_at' | 'updated_at' | 'category_name' | 'category_color' | 'category_icon' +>; + +interface SharePublicPlaceKyselyDB { + places: Pick; + categories: { id: number; name: string; color: string | null; icon: string | null }; +} + interface PlaceRatingRow { user_id: number; username: string; @@ -756,6 +778,58 @@ export class PlacesRepository extends TrekRepository { .execute('all', false); } + // --------------------------------------------------------------------------- + // Plan 3h Task 6 (`share.service.ts`) — additive. + // --------------------------------------------------------------------------- + + /** + * `share.service.ts:340` SH12 (`getSharedTripData`'s share_map place-pool + * read) — **SECURITY-CRITICAL (#2320)**: a named 20-column allow-list, + * never `p.*`. Withheld on purpose (matching `PUBLIC_PLACE_COLUMNS`'s own + * doc comment in `share.service.ts`): the owner's booking-status columns + * (`reservation_status`/`reservation_notes`/`reservation_datetime`), every + * routing/dedup id (`google_place_id`, `google_ftid`, `osm_id`, + * `amap_poi_id`), `route_geometry`/`route_color`/`stop_type`/ + * `fill_percent`, and `source`. Kysely, not `qb()`+`columnRef`: this read + * joins `p.category` under alias `c`, and both `trip_id`/`category_id` are + * `persist(false)` mirror columns — the exact trap + * `DayAssignmentsRepository.assignmentWithPlaceSelect`'s docstring + * documents. `ORDER BY p.created_at DESC`, matching the legacy statement. + */ + async listPublicForShare(trip_id: number | string): Promise { + return await this.kysely() + .selectFrom('places as p') + .leftJoin('categories as c', 'c.id', 'p.category_id') + .select([ + 'p.id', 'p.trip_id', 'p.name', 'p.description', 'p.lat', 'p.lng', 'p.address', 'p.category_id', + 'p.price', 'p.currency', 'p.place_time', 'p.end_time', 'p.duration_minutes', 'p.notes', + 'p.image_url', 'p.website', 'p.phone', 'p.transport_mode', 'p.created_at', 'p.updated_at', + 'c.name as category_name', 'c.color as category_color', 'c.icon as category_icon', + ]) + .where('p.trip_id', '=', trip_id as number) + .orderBy('p.created_at', 'desc') + .execute(); + } + + /** + * `share.service.ts:461` SH18 (`getSharedPlacePhotoKey`) — + * **SECURITY-CRITICAL**: the anonymous photo-serving ownership gate. + * `SELECT 1 FROM places WHERE trip_id = ? AND image_url = ?` — both must + * match, proving the requested proxy URL belongs to a place on the SAME + * trip the share token covers, never just any place anywhere with that + * `image_url`. + */ + async existsByTripAndImageUrl(trip_id: number | string, imageUrl: string): Promise { + const row = await this.kysely<{ places: { id: number; trip_id: number; image_url: string | null } }>() + .selectFrom('places') + .select('id') + .where('trip_id', '=', trip_id as number) + .where('image_url', '=', imageUrl) + .limit(1) + .executeTakeFirst(); + return !!row; + } + // --------------------------------------------------------------------------- // Plan 3c Task 8 (`TripsService.copy`) — additive. // --------------------------------------------------------------------------- diff --git a/server/src/db/repositories/ShareTokens.repository.ts b/server/src/db/repositories/ShareTokens.repository.ts index e02c067f7..ebf7d9113 100644 --- a/server/src/db/repositories/ShareTokens.repository.ts +++ b/server/src/db/repositories/ShareTokens.repository.ts @@ -1,4 +1,160 @@ import type { ShareTokens } from '../entities/ShareTokens.entity'; +import { currentTimestampKysely } from '../dialect/sql-functions'; import { TrekRepository } from './_shared/trek-repository'; -export class ShareTokensRepository extends TrekRepository {} +/** + * `share_tokens` — public trip-share links (Plan 3h Task 6). `trip_id` and + * `created_by` are `persist(false)` shadow mirrors of the `trip`/`createdByRef` + * relations (`ShareTokens.entity.ts`), so every READ here goes through + * `this.db_()` (Kysely), never a bare `qb().select(['trip_id', ...])` — the + * `CollectionPlacesRepository`/`DayAssignmentsRepository.assignmentWithPlaceSelect` + * class-docstring trap applies here too. WRITES go through the MikroORM + * entity API (`insert`/`nativeUpdate`/`nativeDelete`), using the entity's own + * relation property names (`trip`/`createdByRef`) for the two FK columns — + * the `DocumentConnectionsRepository.insertConnection` precedent. + */ +export interface ShareTokenRow { + id: number; + trip_id: number; + token: string; + created_by: number; + share_map: number | null; + share_bookings: number | null; + share_packing: number | null; + share_budget: number | null; + share_collab: number | null; + created_at: string | null; + expires_at: string | null; +} + +interface ShareTokensKyselyDB { + share_tokens: ShareTokenRow; +} + +export class ShareTokensRepository extends TrekRepository { + private db_() { + return this.kysely(); + } + + /** + * SH1 (`createOrUpdate`, TX) — `SELECT token FROM share_tokens WHERE + * trip_id = ?`. + */ + async findTokenByTrip(trip_id: number | string): Promise<{ token: string } | undefined> { + return await this.db_().selectFrom('share_tokens').select('token').where('trip_id', '=', trip_id as number).executeTakeFirst(); + } + + /** + * SH2 (`createOrUpdate`, TX, existing row) — `UPDATE share_tokens SET + * share_map=?, share_bookings=?, share_packing=?, share_budget=?, + * share_collab=?, expires_at=? WHERE trip_id=?`. + */ + async updateFlagsByTrip(trip_id: number | string, data: { + share_map: number; + share_bookings: number; + share_packing: number; + share_budget: number; + share_collab: number; + expires_at: string; + }): Promise { + await this.nativeUpdate({ trip: trip_id as number }, data); + } + + /** + * SH3 (`createOrUpdate`, TX, new row) — `INSERT INTO share_tokens + * (trip_id, token, created_by, share_map, share_bookings, share_packing, + * share_budget, share_collab, expires_at) VALUES (...)`. `token` is + * generated by the caller (`crypto.randomBytes(24)`), passed straight + * through. + */ + async insertNew(data: { + trip_id: number | string; + token: string; + created_by: number; + share_map: number; + share_bookings: number; + share_packing: number; + share_budget: number; + share_collab: number; + expires_at: string; + }): Promise { + await this.insert({ + trip: data.trip_id as number, + token: data.token, + createdByRef: data.created_by, + share_map: data.share_map, + share_bookings: data.share_bookings, + share_packing: data.share_packing, + share_budget: data.share_budget, + share_collab: data.share_collab, + expires_at: data.expires_at, + }); + } + + /** SH4 (`get`) — `SELECT * FROM share_tokens WHERE trip_id = ?`. */ + async findRawByTrip(trip_id: number | string): Promise { + return await this.db_().selectFrom('share_tokens').selectAll().where('trip_id', '=', trip_id as number).executeTakeFirst(); + } + + /** + * SH5 (`remove`) — `DELETE FROM share_tokens WHERE trip_id = ?`. + * **DISTINCT from `deleteByCreator` (UC6)** — a different predicate on the + * same table, never conflated into one method with an optional parameter. + */ + async deleteByTrip(trip_id: number | string): Promise { + await this.nativeDelete({ trip: trip_id as number }); + } + + /** + * The `token = ?` and `(expires_at IS NULL OR expires_at > )` + * predicate SH6, SH17 and `platform.routes.ts`'s R1-converted pre-init + * `servePhoto` fallback ALL share — built exactly once here, never + * re-derived at any of the three call sites (R5). `token` is compared + * with a plain `=` — no `LIKE`, no case-folding, ever (R5's own + * "the opposite of rule 18" framing). `currentTimestampKysely` (not a + * hand-written `datetime('now')` literal): SQLite's bare `CURRENT_TIMESTAMP` + * keyword renders in the SAME `YYYY-MM-DD HH:MM:SS` UTC text `datetime('now')` + * does, so reusing the existing helper is byte-equivalent to the legacy + * predicate, not an approximation of it. + */ + private validTokenQuery(token: string) { + const platform = this.getEntityManager().getPlatform(); + return this.db_() + .selectFrom('share_tokens') + .where('token', '=', token) + .where((eb) => eb.or([eb('expires_at', 'is', null), eb('expires_at', '>', currentTimestampKysely(platform))])); + } + + /** SH6 (`getSharedTripData`) — the full row, valid-token predicate above. */ + async findValidByToken(token: string): Promise { + return await this.validTokenQuery(token).selectAll().executeTakeFirst(); + } + + /** + * SH17 (`getSharedPlacePhotoKey`) — `trip_id, share_map` only, SAME + * valid-token predicate as SH6/R1. + */ + async findTripAndShareMapByToken(token: string): Promise<{ trip_id: number; share_map: number | null } | undefined> { + return await this.validTokenQuery(token).select(['trip_id', 'share_map']).executeTakeFirst(); + } + + /** + * R1 (`platform.routes.ts`'s pre-init `servePhoto` share-token fallback, + * SV-PLATFORM1) — `trip_id` only, the SAME valid-token predicate SH6/SH17 + * use, built once here and reused, not re-derived at the pre-init call + * site. + */ + async findTripIdByToken(token: string): Promise<{ trip_id: number } | undefined> { + return await this.validTokenQuery(token).select(['trip_id']).executeTakeFirst(); + } + + /** + * UC6 (`user-cleanup.service.ts`'s GDPR erasure) — `DELETE FROM + * share_tokens WHERE created_by = ?`. **DISTINCT from `deleteByTrip` + * (SH5)** — deletes every link a user CREATED, across every trip, + * regardless of who owns those trips, not the links on one trip. + */ + async deleteByCreator(userId: number): Promise { + await this.nativeDelete({ createdByRef: userId }); + } +} diff --git a/server/src/db/repositories/Trips.repository.ts b/server/src/db/repositories/Trips.repository.ts index dca0c7a2c..e92c29428 100644 --- a/server/src/db/repositories/Trips.repository.ts +++ b/server/src/db/repositories/Trips.repository.ts @@ -695,6 +695,22 @@ export class TripsRepository extends TrekRepository { return row ?? null; } + /** + * `share.service.ts:264` SH7 (`getSharedTripData`) — `SELECT id, title, + * description, start_date, end_date, cover_image, currency FROM trips + * WHERE id = ?`. A different 7-column public projection from + * `TripSummaryProjectionRow`'s 8 above (`cover_image` in, `is_archived`/ + * `updated_at` out — a public share-link viewer never sees either). No + * access check here, matching the legacy statement — the caller's own + * `verifyTripAccess`/token validity gates this read. + */ + async findPublicForShare(id: number | string): Promise { + return await this.qb('t') + .select(['t.id', 't.title', 't.description', 't.start_date', 't.end_date', 't.cover_image', 't.currency']) + .where('t.id = ?', [id]) + .execute('get', false); + } + /** * `PublicApiService.buildTravellers`'s statement — the owner row (a * literal `1 AS is_owner`) `UNION ALL` every member row (`0 AS is_owner`), @@ -921,6 +937,17 @@ export interface TripSummaryProjectionRow { updated_at: string | null; } +/** {@link TripsRepository.findPublicForShare}'s projection (SH7). */ +export interface TripPublicShareRow { + id: number; + title: string; + description: string | null; + start_date: string | null; + end_date: string | null; + cover_image: string | null; + currency: string | null; +} + /** `PublicApiService.buildTravellers`'s output row. */ export interface TravellerUsernameRow { username: string; diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index 7300991d6..c1abc137f 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -14,6 +14,7 @@ import { JourneyShareTokens } from '../../db/entities/JourneyShareTokens.entity' import { Journeys } from '../../db/entities/Journeys.entity'; import { JourneyEntries } from '../../db/entities/JourneyEntries.entity'; import { JourneyContributors } from '../../db/entities/JourneyContributors.entity'; +import { ShareTokens } from '../../db/entities/ShareTokens.entity'; import { TokensModule } from '../tokens/tokens.module'; import { AuthPublicController } from './auth-public.controller'; import { AuthController } from './auth.controller'; @@ -92,8 +93,10 @@ import { AllowedFileTypesModule } from '../files/allowed-file-types.module'; // 3g Task 4's own addition — `UserCleanupService.cleanupUserReferences`'s // UC7-10 GDPR-erasure deletes, reached the same cross-domain way as // McpTokens/OauthTokens above (owned by `nest/journey`, registered here - // for THIS module's own `@InjectRepository` params). - MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems, JourneyShareTokens, Journeys, JourneyEntries, JourneyContributors])], + // for THIS module's own `@InjectRepository` params). ShareTokens: Plan + // 3h Task 6's own addition, the SAME cross-domain shape — UC6's + // `share_tokens` erasure delete (owned by `nest/share`). + MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials, WebauthnChallenges, InviteTokens, McpTokens, OauthTokens, PasswordResetTokens, BudgetItems, JourneyShareTokens, Journeys, JourneyEntries, JourneyContributors, ShareTokens])], controllers: [AuthPublicController, AuthController, PasskeyController], providers: [AuthService, UserProfileService, RegistrationInvitesService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp], exports: [AuthService, RegistrationInvitesService, PasskeyService, UserCleanupService], diff --git a/server/src/nest/auth/user-cleanup.service.ts b/server/src/nest/auth/user-cleanup.service.ts index 0dfe5c649..93ddaea54 100644 --- a/server/src/nest/auth/user-cleanup.service.ts +++ b/server/src/nest/auth/user-cleanup.service.ts @@ -20,6 +20,8 @@ import { JourneyEntries } from '../../db/entities/JourneyEntries.entity'; import type { JourneyEntriesRepository } from '../../db/repositories/JourneyEntries.repository'; import { JourneyContributors } from '../../db/entities/JourneyContributors.entity'; import type { JourneyContributorsRepository } from '../../db/repositories/JourneyContributors.repository'; +import { ShareTokens } from '../../db/entities/ShareTokens.entity'; +import type { ShareTokensRepository } from '../../db/repositories/ShareTokens.repository'; /** * Account erasure — everything that has to happen around `DELETE FROM users` @@ -43,12 +45,12 @@ export class UserCleanupService { // Plan 3e Task 2 (budget) — additive, UC5 only. @InjectRepository(BudgetItems) private readonly budgetItemsRepo: BudgetItemsRepository, // Plan 3g Task 4 (UC7-10) — the genuinely-journey GDPR-erasure deletes. - // UC6 (`share_tokens`, `nest/share`, Plan 3h) is NOT one of these — see - // `cleanupUserReferences`'s own doc comment. @InjectRepository(JourneyShareTokens) private readonly journeyShareTokensRepo: JourneyShareTokensRepository, @InjectRepository(Journeys) private readonly journeysRepo: JourneysRepository, @InjectRepository(JourneyEntries) private readonly journeyEntriesRepo: JourneyEntriesRepository, @InjectRepository(JourneyContributors) private readonly journeyContributorsRepo: JourneyContributorsRepository, + // Plan 3h Task 6 (UC6, R10) — the genuinely-share GDPR-erasure delete. + @InjectRepository(ShareTokens) private readonly shareTokensRepo: ShareTokensRepository, ) {} /** @@ -102,15 +104,14 @@ export class UserCleanupService { * this plan, so UC4 stays a raw `DatabaseService` call (Plan 3b Task 5 * ruling; inventory §6) — UC5 is already converted (`budgetItemsRepo * .clearPaidByUser`). `share_tokens` (UC6) belongs to `nest/share` - * (trip-level share links, Plan 3h) — a genuinely different table from - * the four below despite sharing this method and this erasure - * transaction, so it ALSO stays a raw `DatabaseService` call, but for a - * different reason than UC4 (not this plan's table at all, not merely - * "another plan hasn't converted it yet"). `journey_share_tokens`/ + * (trip-level share links) and converts here (Plan 3h Task 6, R10) onto + * `ShareTokensRepository.deleteByCreator` — **DISTINCT from + * `ShareService`'s own `remove()`**, which deletes `WHERE trip_id = ?`, a + * different predicate on the same table. `journey_share_tokens`/ * `journeys`/`journey_entries`/`journey_contributors` (UC7-10, - * `nest/journey-share`/`nest/journey-domain`, Plan 3g) ARE this plan's - * own and are converted (Plan 3g Task 4) onto their owning repositories. - * Verbatim otherwise: same statement order, same bounded 4-table + * `nest/journey-share`/`nest/journey-domain`, Plan 3g) were converted + * earlier (Plan 3g Task 4) onto their owning repositories. Verbatim + * otherwise: same statement order, same bounded 5-table * share_tokens/journey_share_tokens/journeys/journey_entries/ * journey_contributors sequence, `this.budget.removeUserFromBudgetItems` * unchanged. @@ -119,7 +120,7 @@ export class UserCleanupService { this.db.run('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?', userId); // UC4 — Plan 3c await this.budgetItemsRepo.clearPaidByUser(userId); // UC5 — Plan 3e Task 2, converted. await this.budget.removeUserFromBudgetItems(userId); - this.db.run('DELETE FROM share_tokens WHERE created_by = ?', userId); // UC6 — Plan 3h + await this.shareTokensRepo.deleteByCreator(userId); // UC6 — converted (Plan 3h Task 6) await this.journeyShareTokensRepo.deleteByCreatedBy(userId); // UC7 — converted (Plan 3g Task 4) // Owned journeys cascade-delete their entries/contributors/share_tokens/photos via journey_id FKs await this.journeysRepo.deleteOwnedByUser(userId); // UC8 — converted (Plan 3g Task 4) diff --git a/server/src/nest/collections/collections.service.ts b/server/src/nest/collections/collections.service.ts index 44fa499d2..a280e0adc 100644 --- a/server/src/nest/collections/collections.service.ts +++ b/server/src/nest/collections/collections.service.ts @@ -3,7 +3,7 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import { PermissionsService } from '../permissions/permissions.service'; import { RealtimeService } from '../realtime/realtime.service'; -import { reclaimPlaceImage } from '../places/place-image'; +import { isUploadedPlaceImage } from '../places/place-image'; import { StorageService } from '../storage/storage.service'; import { Collections } from '../../db/entities/Collections.entity'; import { CollectionsRepository, type CollectionPlaceRow } from '../../db/repositories/Collections.repository'; @@ -945,7 +945,7 @@ export class CollectionsService { }); if (body.image_url !== undefined && prevImage !== (body.image_url ?? null)) { - await reclaimPlaceImage(this.storage, prevImage); + await this.reclaimPlaceImage(prevImage); } await this.notifyCollectionUsers(currentCollection, socketId, 'collections:updated'); @@ -978,12 +978,35 @@ export class CollectionsService { return this.getPlaceById(placeId); } + /** + * Delete a custom saved-place-image object once nothing references it any + * more (Plan 3h Task 6, replacing `place-image.ts`'s free function — see + * that file's own doc comment). A trip place and a collection saved-place + * can share the same uploaded file — save-to-collection and copy-to-trip + * copy `image_url` by reference — so this ref-counts across both tables + * before deleting, mirroring `PlacesService.reclaimPlaceImage`'s shape + * exactly: `places` first via `PlacesRepository.existsByImageUrl` + * (`tripPlaces`, already injected), `collection_places` second via + * `CollectionPlacesRepository.existsByImageUrl` (`collectionPlaces`, + * already injected), JS-level `||` short-circuit preserved by early + * return. `path.basename()` keeps the storage name confined to the + * 'places' category. Best-effort: never throws. + */ + private async reclaimPlaceImage(url: string | null | undefined): Promise { + if (!isUploadedPlaceImage(url)) return; + if (await this.tripPlaces.existsByImageUrl(url)) return; + if (await this.collectionPlaces.existsByImageUrl(url)) return; + await this.storage.delete('places', path.basename(url)).catch(() => { + /* best-effort */ + }); + } + async deletePlace(userId: number, placeId: number, socketId?: string): Promise { const collectionId = await this.collectionIdOfPlace(placeId); await this.assertCanDelete(userId, collectionId); const image = (await this.collectionPlaces.imageUrl(placeId)) ?? null; await this.collectionPlaces.deleteById(placeId); // CASCADE drops tags. NO photo-cache reclaim. - await reclaimPlaceImage(this.storage, image); + await this.reclaimPlaceImage(image); await this.notifyCollectionUsers(collectionId, socketId, 'collections:updated'); } @@ -1007,7 +1030,7 @@ export class CollectionsService { deleted.push(id); } }); - for (const image of images) await reclaimPlaceImage(this.storage, image); + for (const image of images) await this.reclaimPlaceImage(image); for (const cid of touched) await this.notifyCollectionUsers(cid, socketId, 'collections:updated'); return deleted; } @@ -1101,7 +1124,7 @@ export class CollectionsService { await this.assertCanEdit(userId, collectionId); const prev = (await this.collectionPlaces.imageUrl(placeId)) ?? null; await this.collectionPlaces.setImageUrl(placeId, imageUrl); - if (prev !== imageUrl) await reclaimPlaceImage(this.storage, prev); + if (prev !== imageUrl) await this.reclaimPlaceImage(prev); await this.notifyCollectionUsers(collectionId, socketId, 'collections:updated'); return this.getPlaceById(placeId); } diff --git a/server/src/nest/place-photos/place-photo-cache.service.ts b/server/src/nest/place-photos/place-photo-cache.service.ts index c9a6d44db..c6de12ec0 100644 --- a/server/src/nest/place-photos/place-photo-cache.service.ts +++ b/server/src/nest/place-photos/place-photo-cache.service.ts @@ -9,6 +9,8 @@ import { GooglePlacePhotoMeta } from '../../db/entities/GooglePlacePhotoMeta.ent import type { GooglePlacePhotoMetaRepository } from '../../db/repositories/GooglePlacePhotoMeta.repository'; import { Places } from '../../db/entities/Places.entity'; import type { PlacesRepository } from '../../db/repositories/Places.repository'; +import { CollectionPlaces } from '../../db/entities/CollectionPlaces.entity'; +import type { CollectionPlacesRepository } from '../../db/repositories/CollectionPlaces.repository'; // How long a "no photo for this place" answer stays remembered. Nothing about it // changes until a photo appears upstream, so it is worth keeping: without it every @@ -52,10 +54,14 @@ interface CachedPhoto { * layouts (unset: uploads/photos/google; set: that dir, bare keys), so the * cache itself is mode-agnostic. * - * `DatabaseService` stays injected (Plan 3c Task 1, PP6 ruling option 2) - * purely for the `collection_places` half of `isReferenced` — that table is - * Plan 3h's, so it stays a raw statement with a `// Plan 3h` comment until - * then. Every other statement in this file is repository-backed. + * `DatabaseService` was injected (Plan 3c Task 1, PP6 ruling option 2) purely + * for the `collection_places` half of `isReferenced`; Plan 3h Task 6 converts + * that half onto `CollectionPlacesRepository.existsByGoogleIdOrImageUrl`, so + * every statement in this file is now repository-backed — `DatabaseService` + * itself is left injected (out of this task's one-line-swap scope: several + * other domains' test files hand-construct this class positionally) but is + * now unused in the body; flagged for whoever next touches this file's + * constructor to drop. */ @Injectable() export class PlacePhotoCacheService { @@ -71,6 +77,8 @@ export class PlacePhotoCacheService { private readonly storage: StorageService, @InjectRepository(GooglePlacePhotoMeta) private readonly meta: GooglePlacePhotoMetaRepository, @InjectRepository(Places) private readonly places: PlacesRepository, + // Plan 3h Task 6 (survivors) — additive, SV-PP6's `isReferenced` only. + @InjectRepository(CollectionPlaces) private readonly collectionPlaces: CollectionPlacesRepository, ) {} private fileName(placeId: string): string { @@ -215,17 +223,14 @@ export class PlacePhotoCacheService { * statement spans `places` (this plan's) and `collection_places` (Plan 3h's), * so it is split into two existence checks, evaluated in the SAME order the * legacy `UNION ALL … LIMIT 1` would short-circuit in — the `collection_places` - * half only runs when the `places` half comes back false. + * half only runs when the `places` half comes back false. Plan 3h Task 6 + * converts the second half onto `CollectionPlacesRepository + * .existsByGoogleIdOrImageUrl`, closing out the carve-out. */ private async isReferenced(placeId: string): Promise { const proxyUrl = this.proxyUrl(placeId); if (await this.places.existsByGoogleIdOrImageUrl(placeId, proxyUrl)) return true; - // Plan 3h: collection_places is not yet repository-backed. - const row = this.db.get( - `SELECT 1 FROM collection_places WHERE google_place_id = ? OR image_url = ? LIMIT 1`, - placeId, proxyUrl, - ); - return !!row; + return await this.collectionPlaces.existsByGoogleIdOrImageUrl(placeId, proxyUrl); } private async deleteEntry(placeId: string): Promise { diff --git a/server/src/nest/place-photos/place-photos.module.ts b/server/src/nest/place-photos/place-photos.module.ts index 98786769e..bd1e52631 100644 --- a/server/src/nest/place-photos/place-photos.module.ts +++ b/server/src/nest/place-photos/place-photos.module.ts @@ -7,6 +7,7 @@ import { SchedulingModule } from '../scheduling/scheduling.module'; import { StorageModule } from '../storage/storage.module'; import { GooglePlacePhotoMeta } from '../../db/entities/GooglePlacePhotoMeta.entity'; import { Places } from '../../db/entities/Places.entity'; +import { CollectionPlaces } from '../../db/entities/CollectionPlaces.entity'; /** The marker-photo cache. No controller of its own — maps serves the bytes, * places and share read through it, and PlacePhotoCacheJob sweeps it nightly. @@ -17,9 +18,10 @@ import { Places } from '../../db/entities/Places.entity'; * * MikroOrmModule.forFeature registers GooglePlacePhotoMetaRepository/ * PlacesRepository for PlacePhotoCacheService's @InjectRepository - * constructor (Plan 3c Task 1). */ + * constructor (Plan 3c Task 1); Plan 3h Task 6 adds CollectionPlaces for + * SV-PP6's `isReferenced` second existence check. */ @Module({ - imports: [AppConfigModule, SchedulingModule, StorageModule, MikroOrmModule.forFeature([GooglePlacePhotoMeta, Places])], + imports: [AppConfigModule, SchedulingModule, StorageModule, MikroOrmModule.forFeature([GooglePlacePhotoMeta, Places, CollectionPlaces])], providers: [PlacePhotoCacheService, PlacePhotoCacheJob], exports: [PlacePhotoCacheService], }) diff --git a/server/src/nest/places/place-image.ts b/server/src/nest/places/place-image.ts index 1c46ff73a..f2ba2942b 100644 --- a/server/src/nest/places/place-image.ts +++ b/server/src/nest/places/place-image.ts @@ -1,7 +1,3 @@ -import path from 'node:path'; -import { db } from '../../db/database'; -import type { StorageService } from '../storage/storage.service'; - const URL_PREFIX = '/uploads/places/'; export function placeImageUrl(filename: string): string { @@ -12,39 +8,18 @@ export function isUploadedPlaceImage(url: string | null | undefined): url is str return typeof url === 'string' && url.startsWith(URL_PREFIX); } -/** - * Delete a custom place-image object once nothing references it any more. A trip - * place and a collection saved-place can share the same uploaded file — save-to- - * collection and copy-to-trip copy image_url by reference — so we ref-count across - * both tables before deleting. basename() keeps the storage name confined to the - * 'places' category, mirroring tripService.deleteOldCover. Best-effort: never - * throws (central key validation rejects a hostile stored value; the catch - * swallows it exactly like the old unlink guard). - * - * Plan 3c Task 4: `PlacesService` no longer calls this free function — it has - * its own converted private `reclaimPlaceImage` method (`places.service.ts`), - * which does the same job through `PlacesRepository.existsByImageUrl` for - * PI1 and one raw `// Plan 3h` statement for PI2. This copy, and the raw `db` - * proxy import it still needs, stay here UNCHANGED because - * `collections.service.ts` (`src/nest/collections/`, Plan 3h's file — the - * `collection_places` table is Plan 3h's) also imports and calls this exact - * function on four of its own call sites, verified by grep - * (`grep -rn "from '.*place-image'" server/src`) — contradicting the - * inventory's stated "every caller is this service" premise - * (`plan3c-sql-inventory.md` §8d). Converting that caller means wiring a - * `PlacesRepository` injection into `CollectionsModule`, a change to an - * unowned domain's DI graph out of this task's file ownership and test - * budget — flagged for whoever next owns `src/nest/collections/**` (Plan 3h) - * to finish, the same `// Plan 3h` carve-out pattern PI2/PP6 already use in - * this cluster. - */ -export async function reclaimPlaceImage(storage: StorageService, url: string | null | undefined): Promise { - if (!isUploadedPlaceImage(url)) return; - const referenced = - db.prepare('SELECT 1 FROM places WHERE image_url = ? LIMIT 1').get(url) || - db.prepare('SELECT 1 FROM collection_places WHERE image_url = ? LIMIT 1').get(url); - if (referenced) return; - await storage.delete('places', path.basename(url)).catch(() => { - /* best-effort */ - }); -} +// `reclaimPlaceImage` (the free function this file used to export) is gone +// (Plan 3h Task 6, closing out a 3-plan-old carve-out): `PlacesService` had +// its own converted private `reclaimPlaceImage` since Plan 3c Task 4 +// (`places.service.ts`, PI1/PI2 through `PlacesRepository`/ +// `CollectionPlacesRepository.existsByImageUrl`); `collections.service.ts`'s +// four call sites (`updatePlace`, `deletePlace`, `deletePlacesMany`, +// `setPlaceImage`) now call a mirroring private `CollectionsService +// .reclaimPlaceImage` method instead, through `PlacesRepository +// .existsByImageUrl` (already injected there, `tripPlaces`) and +// `CollectionPlacesRepository.existsByImageUrl` (already injected, +// `collectionPlaces`) — a whole-tree grep for `from '.*place-image'` (run +// before this deletion) confirmed zero remaining importers of this function; +// `placeImageUrl`/`isUploadedPlaceImage` above still have real importers +// (`collections.controller.ts`, `places.controller.ts`, `places.service.ts`) +// and stay. diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index aa3a65eb2..3ea9dd0da 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -8,6 +8,7 @@ import { TripMembers } from '../../db/entities/TripMembers.entity'; import { DayAssignments } from '../../db/entities/DayAssignments.entity'; import { Trips } from '../../db/entities/Trips.entity'; import { Categories } from '../../db/entities/Categories.entity'; +import { CollectionPlaces } from '../../db/entities/CollectionPlaces.entity'; import { JourneyDomainModule } from '../journey/journey-domain.module'; import { PlacesController } from './places.controller'; import { PlacesService } from './places.service'; @@ -63,8 +64,13 @@ import { MAX_PLACE_IMAGE_SIZE } from '../common/place-image-upload'; // `AssignmentsDomainModule`'s own `MikroOrmModule.forFeature(...)` // precedent: a `forFeature` registration only reaches providers declared // in the SAME module. Plan 3c Task 5 adds `CategoriesRepository` for - // PL33's `importKmlPlaces` folder → category lookup. - MikroOrmModule.forFeature([Places, Tags, PlaceRatings, TripMembers, DayAssignments, Trips, Categories, BudgetItems]), + // PL33's `importKmlPlaces` folder → category lookup. Plan 3h Task 6 adds + // `CollectionPlaces` for SV-PI2 (`reclaimPlaceImage`'s second existence + // check) — a read-only cross-domain repository reference, not a + // `CollectionsModule` import, so it creates no cycle (`CollectionsModule` + // does not import `PlacesModule` either — both sides reach the other's + // table only through `forFeature`). + MikroOrmModule.forFeature([Places, Tags, PlaceRatings, TripMembers, DayAssignments, Trips, Categories, BudgetItems, CollectionPlaces]), ], controllers: [PlacesController], providers: [PlacesService, PlacesMcp, PlacesRpc], diff --git a/server/src/nest/places/places.service.ts b/server/src/nest/places/places.service.ts index ae10b31cd..19a9bf920 100644 --- a/server/src/nest/places/places.service.ts +++ b/server/src/nest/places/places.service.ts @@ -49,6 +49,8 @@ import type { CategoriesRepository } from '../../db/repositories/Categories.repo import type { TripsRepository } from '../../db/repositories/Trips.repository'; import { BudgetItems } from '../../db/entities/BudgetItems.entity'; import type { BudgetItemsRepository } from '../../db/repositories/BudgetItems.repository'; +import { CollectionPlaces } from '../../db/entities/CollectionPlaces.entity'; +import type { CollectionPlacesRepository } from '../../db/repositories/CollectionPlaces.repository'; /** Rows a place delete took down with the nights booked there, for the caller to announce. */ export interface CancelledStays { @@ -167,6 +169,8 @@ export class PlacesService { @InjectRepository(Trips) private readonly tripsRepo: TripsRepository, // Plan 3e Task 2 (budget) — additive, PL15/PL18/PL22 only. @InjectRepository(BudgetItems) private readonly budgetItemsRepo: BudgetItemsRepository, + // Plan 3h Task 6 (survivors) — additive, SV-PI2's `reclaimPlaceImage` only. + @InjectRepository(CollectionPlaces) private readonly collectionPlacesRepo: CollectionPlacesRepository, ) {} /** @@ -234,9 +238,10 @@ export class PlacesService { * * PI1 (`SELECT 1 FROM places WHERE image_url = ? LIMIT 1`) through * `PlacesRepository.existsByImageUrl`; PI2 (`SELECT 1 FROM collection_places - * WHERE image_url = ? LIMIT 1`, Plan 3h's table) stays one raw statement, - * evaluated only when PI1 is false — reproducing the legacy `UNION ALL … - * LIMIT 1`'s short-circuit by evaluation order, the same pattern + * WHERE image_url = ? LIMIT 1`) through `CollectionPlacesRepository + * .existsByImageUrl` (Plan 3h Task 6, closing out the carve-out), evaluated + * only when PI1 is false — reproducing the legacy `UNION ALL … LIMIT 1`'s + * short-circuit by evaluation order, the same pattern * `PlacePhotoCacheService.isReferenced` uses for the same two tables * (Plan 3c Task 1's PP6 ruling). `path.basename()` keeps the storage name * confined to the 'places' category. Best-effort: never throws (central @@ -246,9 +251,8 @@ export class PlacesService { private async reclaimPlaceImage(url: string | null | undefined): Promise { if (!isUploadedPlaceImage(url)) return; if (await this.placesRepo.existsByImageUrl(url)) return; - // Plan 3h: collection_places is not yet repository-backed. - const referenced = this.dbs.get('SELECT 1 FROM collection_places WHERE image_url = ? LIMIT 1', url); - if (referenced) return; + // PI2 (Plan 3h Task 6) — `CollectionPlacesRepository.existsByImageUrl`. + if (await this.collectionPlacesRepo.existsByImageUrl(url)) return; await this.storage.delete('places', path.basename(url)).catch(() => { /* best-effort */ }); diff --git a/server/src/nest/platform/platform.routes.ts b/server/src/nest/platform/platform.routes.ts index c79f971c9..fc87c8ee1 100644 --- a/server/src/nest/platform/platform.routes.ts +++ b/server/src/nest/platform/platform.routes.ts @@ -10,6 +10,8 @@ import { StorageService } from '../storage/storage.service'; import { StorageInvalidKeyError, StorageNotFoundError, type StorageCategory } from '../storage/storage.types'; import { Users } from '../../db/entities/Users.entity'; import type { UsersRepository } from '../../db/repositories/Users.repository'; +import { ShareTokens } from '../../db/entities/ShareTokens.entity'; +import type { ShareTokensRepository } from '../../db/repositories/ShareTokens.repository'; // Platform / transport routes extracted verbatim from createApp() (app.ts) so they can be // mounted on either the legacy Express app or the NestJS Express instance (strangler A6/A8). @@ -74,7 +76,7 @@ export function storageStaticHandler(storage: StorageService, category: StorageC }; } -async function servePhoto(storage: StorageService, req: Request, res: Response, users: UsersRepository): Promise { +async function servePhoto(storage: StorageService, req: Request, res: Response, users: UsersRepository, shareTokens: ShareTokensRepository): Promise { const safeName = path.basename(req.params.filename); // Parity: after basename(), the old resolve()+startsWith guard could only // fire when the remaining segment was '..' — keep that exact 403. @@ -116,9 +118,7 @@ async function servePhoto(storage: StorageService, req: Request, res: Response, res.status(401).send('Authentication required'); return; } - const share = db - .prepare("SELECT trip_id FROM share_tokens WHERE token = ? AND (expires_at IS NULL OR expires_at > datetime('now'))") - .get(rawToken) as { trip_id: number } | undefined; + const share = await shareTokens.findTripIdByToken(rawToken); if (!share || share.trip_id !== photo.trip_id) { res.status(401).send('Authentication required'); return; @@ -136,17 +136,28 @@ async function servePhoto(storage: StorageService, req: Request, res: Response, * per-request EntityManager fork, forever, not just during a boot window. * `verifyJwtAndLoadUser` (`jwt-verify.ts`, JV1) now reads `users` through * `UsersRepository` (Plan 3b Task 1) rather than the legacy `db` proxy — the - * exact case Task 0's wrap was built for. `orm.em.getRepository(Users)` is - * resolved inside the arrow passed to `withRequestContext` below: `orm.em` - * is the global, context-resolving EntityManager, and MikroORM's repository - * object holds a reference to that same proxy rather than a snapshot, so a - * query issued through it later resolves whatever `AsyncLocalStorage` - * context is active AT QUERY TIME — not at the moment `getRepository()` was - * called. What actually gates this is `withRequestContext` wrapping the - * whole `servePhoto(...)` call (query included): removing the wrapper - * entirely reproduces Task 0's `ValidationError: Using global EntityManager - * instance...` (verified directly — mutation-proof in `task-1-report.md`), - * exactly the failure `PHOTOCTX-002`/`SEAM-002` guard against. + * exact case Task 0's wrap was built for. The anonymous share-token fallback + * (used when no JWT is present) now reads `share_tokens` through + * `ShareTokensRepository.findTripIdByToken` the SAME way (Plan 3h Task 6, R1 + * — the plan's single highest-severity finding: this branch ran unconverted, + * against the legacy `db` proxy, for three plans after the JWT half above + * was fixed). Both `orm.em.getRepository(Users)` and + * `orm.em.getRepository(ShareTokens)` are resolved inside the arrow passed + * to `withRequestContext` below: `orm.em` is the global, context-resolving + * EntityManager, and MikroORM's repository object holds a reference to that + * same proxy rather than a snapshot, so a query issued through it later + * resolves whatever `AsyncLocalStorage` context is active AT QUERY TIME — + * not at the moment `getRepository()` was called. What actually gates this + * is `withRequestContext` wrapping the whole `servePhoto(...)` call (both + * repository reads included): removing the wrapper entirely reproduces Task + * 0's `ValidationError: Using global EntityManager instance...` (verified + * directly — mutation-proof in `task-1-report.md`), exactly the failure + * `PHOTOCTX-002`/`SEAM-002` guard against. The sibling `photos` table read + * (`db.prepare('SELECT trip_id FROM photos ...')`, a few lines into + * `servePhoto` below) stays on the legacy `db` proxy — a different domain's + * table, not this plan's — but its `trip_id` MATCH against the converted + * `share` read is the handler's whole authorization and is untouched: only + * the `share_tokens` lookup itself moved, not the comparison built on it. * * Optional at the type level only, the same shape `TrekWsAdapter` uses for * its own D6 wrapper (`src/nest/realtime/trek-ws.adapter.ts`): `bootstrap.ts` @@ -191,7 +202,7 @@ export function applyPlatformUploads(app: express.Application, storage: StorageS next(new Error('applyPlatformUploads: no MikroORM available to build a request context for /uploads/photos/*')); return; } - return withRequestContext(orm, () => servePhoto(storage, req, res, orm.em.getRepository(Users))).catch(next); + return withRequestContext(orm, () => servePhoto(storage, req, res, orm.em.getRepository(Users), orm.em.getRepository(ShareTokens))).catch(next); }); // Block direct access to /uploads/files diff --git a/server/src/nest/share/share.module.ts b/server/src/nest/share/share.module.ts index 106676590..236b7af29 100644 --- a/server/src/nest/share/share.module.ts +++ b/server/src/nest/share/share.module.ts @@ -11,16 +11,31 @@ import { StorageModule } from '../storage/storage.module'; import { AuthModule } from '../auth/auth.module'; import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; import { Reservations } from '../../db/entities/Reservations.entity'; +import { ShareTokens } from '../../db/entities/ShareTokens.entity'; +import { Trips } from '../../db/entities/Trips.entity'; +import { Days } from '../../db/entities/Days.entity'; +import { DayAssignments } from '../../db/entities/DayAssignments.entity'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import { Places } from '../../db/entities/Places.entity'; +import { PackingItems } from '../../db/entities/PackingItems.entity'; +import { BudgetItems } from '../../db/entities/BudgetItems.entity'; +import { Categories } from '../../db/entities/Categories.entity'; +import { CollabMessages } from '../../db/entities/CollabMessages.entity'; -// `MikroOrmModule.forFeature([Reservations])` registers `ReservationsRepository` -// for `ShareService`'s `@InjectRepository` constructor param (Plan 3d Task 4 — -// its four reads on `reservations`/`reservation_endpoints`/ -// `reservation_day_positions`/`day_accommodations` convert onto the same -// repository `ReservationsModule` owns, without importing that module itself: -// this file stays a leaf the way `PublicApiModule`'s own `Trips`-only -// `forFeature` already does for the same reason). +// `MikroOrmModule.forFeature([Reservations, ...])` registers every repository +// `ShareService`'s `@InjectRepository` constructor params need (Plan 3d Task 4 +// already registered `Reservations`; Plan 3h Task 6 adds `ShareTokens` — this +// file's own table — plus the nine cross-domain tables its public-share reads +// (SH7-9,11-16) touch, without importing each owning module: this file stays a +// leaf the way `PublicApiModule`'s own `Trips`-only `forFeature` already does +// for the same reason). @Module({ - imports: [McpSharedModule, SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule, PlacePhotosModule, StorageModule, MikroOrmModule.forFeature([Reservations])], + imports: [ + McpSharedModule, SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule, PlacePhotosModule, StorageModule, + MikroOrmModule.forFeature([ + Reservations, ShareTokens, Trips, Days, DayAssignments, DayNotes, Places, PackingItems, BudgetItems, Categories, CollabMessages, + ]), + ], controllers: [TripShareController, SharedController], providers: [ShareService, ShareMcp], }) diff --git a/server/src/nest/share/share.service.ts b/server/src/nest/share/share.service.ts index 68d9d7721..d4122dc51 100644 --- a/server/src/nest/share/share.service.ts +++ b/server/src/nest/share/share.service.ts @@ -1,8 +1,6 @@ import { Injectable } from '@nestjs/common'; import crypto from 'crypto'; import { InjectRepository } from '@mikro-orm/nestjs'; -import { DatabaseService } from '../database/database.service'; -import type { TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import { QueryHelpersService } from '../query-helpers/query-helpers.service'; import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; @@ -11,6 +9,26 @@ import { UnitOfWork } from '../database/unit-of-work'; import type { User } from '../../types'; import { Reservations } from '../../db/entities/Reservations.entity'; import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; +import { ShareTokens } from '../../db/entities/ShareTokens.entity'; +import type { ShareTokensRepository } from '../../db/repositories/ShareTokens.repository'; +import { Trips } from '../../db/entities/Trips.entity'; +import type { TripsRepository, TripAccess } from '../../db/repositories/Trips.repository'; +import { Days } from '../../db/entities/Days.entity'; +import type { DaysRepository } from '../../db/repositories/Days.repository'; +import { DayAssignments } from '../../db/entities/DayAssignments.entity'; +import type { DayAssignmentsRepository } from '../../db/repositories/DayAssignments.repository'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import type { DayNotesRepository } from '../../db/repositories/DayNotes.repository'; +import { Places } from '../../db/entities/Places.entity'; +import type { PlacesRepository } from '../../db/repositories/Places.repository'; +import { PackingItems } from '../../db/entities/PackingItems.entity'; +import type { PackingItemsRepository } from '../../db/repositories/PackingItems.repository'; +import { BudgetItems } from '../../db/entities/BudgetItems.entity'; +import type { BudgetItemsRepository } from '../../db/repositories/BudgetItems.repository'; +import { Categories } from '../../db/entities/Categories.entity'; +import type { CategoriesRepository } from '../../db/repositories/Categories.repository'; +import { CollabMessages } from '../../db/entities/CollabMessages.entity'; +import type { CollabMessagesRepository } from '../../db/repositories/CollabMessages.repository'; type Trip = TripAccess; @@ -50,9 +68,10 @@ export interface ShareTokenInfo { } /** - * Public share links — the legacy shareService SQL folded in over the injected - * DatabaseService. Trip access and the 'share_manage' permission gate - * create/delete; the shared read is public. + * Public share links — `share_tokens` and its cross-domain public reads + * through repositories (Plan 3h Task 6; 3d Task 4 already converted the + * reservations-related reads). Trip access and the 'share_manage' permission + * gate create/delete; the shared read is public. */ /** * What a place shows the public: where it is, what it is, how to reach it. @@ -61,12 +80,12 @@ export interface ShareTokenInfo { * Google identifiers, the routing bookkeeping and the fill figures a road trip * keeps for itself. `notes` stays — it is the note somebody wrote to be read * on the plan, and the plan is what a link shares. + * + * The exact column list now lives as {@link PlacesRepository.listPublicForShare}'s + * own docstring (Plan 3h Task 6) — this file no longer spells the SQL, so + * keeping a second copy of the allow-list here would drift from the one the + * query actually runs. */ -const PUBLIC_PLACE_COLUMNS = [ - 'id', 'trip_id', 'name', 'description', 'lat', 'lng', 'address', 'category_id', 'price', 'currency', - 'place_time', 'end_time', 'duration_minutes', 'notes', 'image_url', 'website', 'phone', - 'transport_mode', 'created_at', 'updated_at', -].map(c => `p.${c}`).join(', '); // What a booking/stay shows the public — never the confirmation number, the // import trail, or who is travelling — now lives as the exact column list in @@ -145,17 +164,27 @@ export function publicHttpUrl(value: unknown): string | null { @Injectable() export class ShareService { constructor( - private readonly dbs: DatabaseService, private readonly settings: SettingsService, private readonly permissions: PermissionsService, private readonly queryHelpers: QueryHelpersService, private readonly photoCache: PlacePhotoCacheService, private readonly uow: UnitOfWork, @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, + @InjectRepository(ShareTokens) private readonly shareTokens: ShareTokensRepository, + @InjectRepository(Trips) private readonly trips: TripsRepository, + @InjectRepository(Days) private readonly days: DaysRepository, + @InjectRepository(DayAssignments) private readonly dayAssignments: DayAssignmentsRepository, + @InjectRepository(DayNotes) private readonly dayNotes: DayNotesRepository, + @InjectRepository(Places) private readonly places: PlacesRepository, + @InjectRepository(PackingItems) private readonly packingItems: PackingItemsRepository, + @InjectRepository(BudgetItems) private readonly budgetItems: BudgetItemsRepository, + @InjectRepository(Categories) private readonly categories: CategoriesRepository, + @InjectRepository(CollabMessages) private readonly collabMessages: CollabMessagesRepository, ) {} - async verifyTripAccess(tripId: string, userId: number) { - return await this.dbs.canAccessTrip(tripId, userId); + /** SH-AP1 — the same `TripsRepository.findAccessible` every other access-primitive call in this plan reuses. */ + async verifyTripAccess(tripId: string, userId: number): Promise { + return await this.trips.findAccessible(tripId, userId); } async canManage(trip: Trip, user: User): Promise { @@ -182,20 +211,31 @@ export class ShareService { const expiresAt = new Date(Date.now() + 90 * 24 * 60 * 60 * 1000).toISOString(); return await this.uow.transactional(async () => { - const existing = this.dbs.get<{ token: string }>('SELECT token FROM share_tokens WHERE trip_id = ?', tripId); + const existing = await this.shareTokens.findTokenByTrip(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, - ); + await this.shareTokens.updateFlagsByTrip(tripId, { + share_map: share_map ? 1 : 0, + share_bookings: share_bookings ? 1 : 0, + share_packing: share_packing ? 1 : 0, + share_budget: share_budget ? 1 : 0, + share_collab: share_collab ? 1 : 0, + expires_at: expiresAt, + }); 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, - ); + await this.shareTokens.insertNew({ + trip_id: tripId, + token, + created_by: userId, + share_map: share_map ? 1 : 0, + share_bookings: share_bookings ? 1 : 0, + share_packing: share_packing ? 1 : 0, + share_budget: share_budget ? 1 : 0, + share_collab: share_collab ? 1 : 0, + expires_at: expiresAt, + }); return { token, created: true }; }); } @@ -204,11 +244,11 @@ export class ShareService { * Returns share token info for a trip, or null if no share link exists. */ async get(tripId: string): Promise { - const row = this.dbs.get('SELECT * FROM share_tokens WHERE trip_id = ?', tripId); + const row = await this.shareTokens.findRawByTrip(tripId); if (!row) return null; return { token: row.token, - created_at: row.created_at, + created_at: row.created_at ?? '', share_map: !!row.share_map, share_bookings: !!row.share_bookings, share_packing: !!row.share_packing, @@ -221,7 +261,7 @@ export class ShareService { * Deletes the share token for a trip. */ async remove(tripId: string): Promise { - this.dbs.run('DELETE FROM share_tokens WHERE trip_id = ?', tripId); + await this.shareTokens.deleteByTrip(tripId); } /** @@ -251,19 +291,13 @@ export class ShareService { } async getSharedTripData(token: string): Promise | null> { - const shareRow = this.dbs.get( - "SELECT * FROM share_tokens WHERE token = ? AND (expires_at IS NULL OR expires_at > datetime('now'))", - token, - ); + const shareRow = await this.shareTokens.findValidByToken(token); if (!shareRow) return null; const tripId = shareRow.trip_id; // Trip - const trip = this.dbs.get( - 'SELECT id, title, description, start_date, end_date, cover_image, currency FROM trips WHERE id = ?', - tripId, - ); + const trip = await this.trips.findPublicForShare(tripId); if (!trip) return null; const permissions = { @@ -280,31 +314,17 @@ export class ShareService { 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); + days = await this.days.listByTrip(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, - p.website, p.phone, - 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 allAssignments = await this.dayAssignments.listPublicForShare(dayIds); - const placeIds = [...new Set(allAssignments.map((a: any) => a.place_id))]; + const placeIds = [...new Set(allAssignments.map((a) => a.place_id))]; const tagsByPlace = await this.queryHelpers.loadTagsByPlaceIds(placeIds, { compact: true }); const byDay: Record = {}; - for (const a of allAssignments as any[]) { + for (const a of allAssignments) { 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, @@ -325,9 +345,9 @@ export class ShareService { } 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 allNotes = await this.dayNotes.listByDayIds(dayIds); const notesByDay: Record = {}; - for (const n of allNotes as any[]) { + for (const n of allNotes) { if (!notesByDay[n.day_id]) notesByDay[n.day_id] = []; notesByDay[n.day_id].push(n); } @@ -337,11 +357,7 @@ export class ShareService { // Named columns, not p.*: the pool used to travel whole, which put the // owner's booking notes on the place, the Google ids and the import // bookkeeping in front of anybody holding the link (#2320). - places = this.dbs.all(` - SELECT ${PUBLIC_PLACE_COLUMNS}, 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) => ({ + places = (await this.places.listPublicForShare(tripId)).map((p) => ({ ...p, image_url: rewritePlacePhotoUrl(p.image_url, token), website: publicHttpUrl(p.website), @@ -383,21 +399,21 @@ export class ShareService { // 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) + ? await this.packingItems.listPublicForShare(tripId) : []; // Budget const budget = permissions.share_budget - ? this.dbs.all('SELECT * FROM budget_items WHERE trip_id = ? ORDER BY category ASC', tripId) + ? await this.budgetItems.listPublicForShare(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'); + const categories = await this.categories.listAllUnordered(); // 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) + ? await this.collabMessages.listPublicForShare(tripId) : []; // Display currency the share owner sees in their Costs view. A public viewer has @@ -447,10 +463,7 @@ export class ShareService { * mirroring the authenticated bytes endpoint. */ async getSharedPlacePhotoKey(token: string, placeId: string): Promise { - const shareRow = this.dbs.get<{ trip_id: string; share_map: number }>( - "SELECT trip_id, share_map FROM share_tokens WHERE token = ? AND (expires_at IS NULL OR expires_at > datetime('now'))", - token, - ); + const shareRow = await this.shareTokens.findTripAndShareMapByToken(token); if (!shareRow) return null; // Place photos belong to the map/itinerary section — withhold them when the // owner disabled the map, matching getSharedTripData which no longer returns @@ -458,8 +471,8 @@ export class ShareService { if (!shareRow.share_map) return null; const expectedUrl = `${PLACE_PHOTO_PROXY_PREFIX}${encodeURIComponent(placeId)}/bytes`; - const place = this.dbs.get('SELECT 1 FROM places WHERE trip_id = ? AND image_url = ?', shareRow.trip_id, expectedUrl); - if (!place) return null; + const exists = await this.places.existsByTripAndImageUrl(shareRow.trip_id, expectedUrl); + if (!exists) return null; return this.photoCache.serveKey(placeId); } diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 7eaf43322..6875040e2 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -138,6 +138,7 @@ import { import { createTestOrm } from './test-orm'; import { createTestTripFilesRepo, createTestFileLinksRepo, createTestBudgetItemsRepo } from './files-repos'; import { budgetRepoArgs } from './budget-repos'; +import { createTestShareTokensRepo, shareServiceRepoArgs } from './share-repos'; import { createTestCollabMessageReactionsRepo, createTestCollabNotesRepo, createTestCollabPollsRepo, createTestCollabPollVotesRepo, createTestCollabLinksRepo, createTestCollabMessagesRepo, @@ -215,7 +216,7 @@ export async function createMcpTestRegistry(): Promise { permissionsService, new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), new WebauthnConfigService(appSettings), - new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection)), + new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection), await createTestShareTokensRepo(dbService.connection)), new MailerService(usersRepo, settingsRepo, appSettings), new EphemeralTokenService(), new AllowedFileTypesService(appSettings), await createTestUnitOfWork(dbService.connection), @@ -251,7 +252,7 @@ export async function createMcpTestRegistry(): Promise { ); // Exactly one instance, shared by maps, places and share: its stampede guard // and its on-disk set only work if all three readers see the same maps. - const placePhotoCache = new PlacePhotoCacheService(dbService, makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(dbService.connection), await createTestPlacesRepo(dbService.connection)); + const placePhotoCache = new PlacePhotoCacheService(dbService, makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(dbService.connection), await createTestPlacesRepo(dbService.connection), await createTestCollectionPlacesRepo(dbService.connection)); const mapsService = new MapsService(placePhotoCache, appSettings, usersRepo, await createTestPlaceDetailsCacheRepo(dbService.connection), await createTestPlacesRepo(dbService.connection)); const journeyDomain = new JourneyDomainService( dbService, realtimeService, new TrekPhotoRegistrationService(mcpOrm.repo(TrekPhotos), mcpOrm.repo(TripPhotos), await createTestJourneyPhotosRepo(dbService.connection), dbService), await createTestUnitOfWork(dbService.connection), @@ -310,10 +311,11 @@ export async function createMcpTestRegistry(): Promise { await createTestCategoriesRepo(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestBudgetItemsRepo(dbService.connection), + await createTestCollectionPlacesRepo(dbService.connection), ); // Built after it: a hotel booking writes the stay's day stop through this one. const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(dbService.connection), await createTestReservationEndpointsRepo(dbService.connection), await createTestReservationTravelersRepo(dbService.connection)), accommodationsService, await createTestUnitOfWork(dbService.connection), await createTestReservationsRepo(dbService.connection), await createTestReservationEndpointsRepo(dbService.connection), await createTestReservationTravelersRepo(dbService.connection), await createTestReservationDayPositionsRepo(dbService.connection), await createTestDayAccommodationsRepo(dbService.connection), await createTestDaysRepo(dbService.connection), await createTestPlacesRepo(dbService.connection), await createTestDayAssignmentsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), await createTestUsersRepo(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestBudgetItemsRepo(dbService.connection)); - const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection)), permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), usersRepo); + const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo, await createTestBudgetItemsRepo(dbService.connection), await createTestJourneyShareTokensRepo(dbService.connection), await createTestJourneysRepo(dbService.connection), await createTestJourneyEntriesRepo(dbService.connection), await createTestJourneyContributorsRepo(dbService.connection), await createTestShareTokensRepo(dbService.connection)), permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), usersRepo); const tripsService = new TripsService( dbService, reservationsService, @@ -431,7 +433,7 @@ export async function createMcpTestRegistry(): Promise { ), new TripsMcp(tripsService, todoService, collabService, authService, calendarService, membersService, readModelService, addonsService, guards), new TripPromptsMcp(tripsService, readModelService, packingService, addonsService), - new ShareMcp(new ShareService(dbService, new SettingsService(await createTestUnitOfWork(dbService.connection), appSettings, await createTestSettingsRepo(dbService.connection)), permissionsService, queryHelpersService, placePhotoCache, await createTestUnitOfWork(dbService.connection), await createTestReservationsRepo(dbService.connection)), authService, guards), + new ShareMcp(new ShareService(new SettingsService(await createTestUnitOfWork(dbService.connection), appSettings, await createTestSettingsRepo(dbService.connection)), permissionsService, queryHelpersService, placePhotoCache, await createTestUnitOfWork(dbService.connection), ...(await shareServiceRepoArgs(dbService.connection))), authService, guards), new FeedsMcp(new FeedsService(await createTestTripsRepo(dbService.connection), usersRepo, calendarService), dbService, new RuntimeEnvService(), guards), new TripInviteMcp(new TripInviteService(dbService, permissionsService, new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), await createTestUnitOfWork(dbService.connection)), dbService, new RuntimeEnvService(), guards, new AuditService(auditLogRepo, usersRepo)), new MapsMcp(mapsService), diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index f98320fe0..1b902c251 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -94,6 +94,7 @@ import { AuditLog } from '../../src/db/entities/AuditLog.entity'; import { Users } from '../../src/db/entities/Users.entity'; import { createTestTripFilesRepo, createTestFileLinksRepo, createTestBudgetItemsRepo } from './files-repos'; import { budgetRepoArgs } from './budget-repos'; +import { createTestShareTokensRepo } from './share-repos'; import { createTestCollabMessageReactionsRepo, createTestCollabNotesRepo, createTestCollabPollsRepo, createTestCollabPollVotesRepo, createTestCollabLinksRepo, createTestCollabMessagesRepo, @@ -188,7 +189,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< await createTestReservationEndpointsRepo(dbs.connection), await createTestDayAccommodationsRepo(dbs.connection), ); - const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(dbs.connection), await createTestPlacesRepo(dbs.connection)); + const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(dbs.connection), await createTestPlacesRepo(dbs.connection), await createTestCollectionPlacesRepo(dbs.connection)); const unsplash = new UnsplashService(appSettings, usersRepo, new RuntimeEnvService(), generalStorage); const journey = new JourneyDomainService( dbs, realtime, new TrekPhotoRegistrationService((await sharedTestOrm(dbs.connection)).repo(TrekPhotos), (await sharedTestOrm(dbs.connection)).repo(TripPhotos), await createTestJourneyPhotosRepo(dbs.connection), dbs), await createTestUnitOfWork(dbs.connection), @@ -263,11 +264,12 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< await createTestCategoriesRepo(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), + await createTestCollectionPlacesRepo(dbs.connection), ); // After accommodations: a hotel booking writes the stay's day stop through it. const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(dbs.connection), await createTestReservationEndpointsRepo(dbs.connection), await createTestReservationTravelersRepo(dbs.connection)), accommodations, await createTestUnitOfWork(dbs.connection), await createTestReservationsRepo(dbs.connection), await createTestReservationEndpointsRepo(dbs.connection), await createTestReservationTravelersRepo(dbs.connection), await createTestReservationDayPositionsRepo(dbs.connection), await createTestDayAccommodationsRepo(dbs.connection), await createTestDaysRepo(dbs.connection), await createTestPlacesRepo(dbs.connection), await createTestDayAssignmentsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection)); const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash, generalStorage, await createTestUnitOfWork(dbs.connection), (await sharedTestOrm(dbs.connection)).em); - const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), usersRepo, await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection)), permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), usersRepo); + const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), usersRepo, await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestShareTokensRepo(dbs.connection)), permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), usersRepo); const guards = new PluginGuards(dbs, permissions, addons); const registry = createTestPluginRegistry([ diff --git a/server/tests/helpers/share-repos.ts b/server/tests/helpers/share-repos.ts new file mode 100644 index 000000000..b82dd63cf --- /dev/null +++ b/server/tests/helpers/share-repos.ts @@ -0,0 +1,67 @@ +import type Database from 'better-sqlite3'; +import { + sharedTestOrm, createTestReservationsRepo, createTestTripsRepo, createTestDaysRepo, + createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestPlacesRepo, createTestCategoriesRepo, +} from './test-uow'; +import { createTestBudgetItemsRepo } from './files-repos'; +import { createTestPackingItemsRepo } from './packing-repos'; +import { createTestCollabMessagesRepo } from './collab-repos'; +import { ShareTokens } from '../../src/db/entities/ShareTokens.entity'; +import type { ShareTokensRepository } from '../../src/db/repositories/ShareTokens.repository'; +import type { ReservationsRepository } from '../../src/db/repositories/Reservations.repository'; +import type { TripsRepository } from '../../src/db/repositories/Trips.repository'; +import type { DaysRepository } from '../../src/db/repositories/Days.repository'; +import type { DayAssignmentsRepository } from '../../src/db/repositories/DayAssignments.repository'; +import type { DayNotesRepository } from '../../src/db/repositories/DayNotes.repository'; +import type { PlacesRepository } from '../../src/db/repositories/Places.repository'; +import type { PackingItemsRepository } from '../../src/db/repositories/PackingItems.repository'; +import type { BudgetItemsRepository } from '../../src/db/repositories/BudgetItems.repository'; +import type { CategoriesRepository } from '../../src/db/repositories/Categories.repository'; +import type { CollabMessagesRepository } from '../../src/db/repositories/CollabMessages.repository'; + +/** + * Plan 3h Task 6 (`ShareService`/`UserCleanupService`'s UC6) test-only + * repository factories, bound to a suite's own better-sqlite3 handle via the + * SAME memoised `sharedTestOrm` `test-uow.ts` exports — required so a + * repository built here and a `UnitOfWork` built from `createTestUnitOfWork` + * (that file) resolve the IDENTICAL context-resolving `EntityManager` (the + * `budget-repos.ts`/`collab-repos.ts` precedent this file copies verbatim). + * + * `ShareTokensRepository` is the only genuinely NEW factory here — every + * other repository `ShareService`'s constructor now needs already has one + * (`test-uow.ts`, `files-repos.ts`, `packing-repos.ts`, `collab-repos.ts`), + * reused directly, never re-derived. + */ +export function createTestShareTokensRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(ShareTokens)); +} + +/** + * The 10 repository arguments `ShareService`'s constructor appends after + * `uow` (`reservationsRepo`, `shareTokens`, `trips`, `days`, `dayAssignments`, + * `dayNotes`, `places`, `packingItems`, `budgetItems`, `categories`, + * `collabMessages` — 11 total, `reservationsRepo` already had its own 3d + * factory), built over the SAME connection in one call — every hand-built + * `new ShareService(...)` test call site spreads this + * (`...(await shareServiceRepoArgs(conn))`) instead of repeating all 11 + * factories, the `budgetRepoArgs` precedent. + */ +export async function shareServiceRepoArgs(db: Database.Database): Promise<[ + ReservationsRepository, ShareTokensRepository, TripsRepository, DaysRepository, DayAssignmentsRepository, + DayNotesRepository, PlacesRepository, PackingItemsRepository, BudgetItemsRepository, CategoriesRepository, + CollabMessagesRepository, +]> { + return [ + await createTestReservationsRepo(db), + await createTestShareTokensRepo(db), + await createTestTripsRepo(db), + await createTestDaysRepo(db), + await createTestDayAssignmentsRepo(db), + await createTestDayNotesRepo(db), + await createTestPlacesRepo(db), + await createTestPackingItemsRepo(db), + await createTestBudgetItemsRepo(db), + await createTestCategoriesRepo(db), + await createTestCollabMessagesRepo(db), + ]; +} diff --git a/server/tests/integration/share.test.ts b/server/tests/integration/share.test.ts index 01532c9b5..b73015d25 100644 --- a/server/tests/integration/share.test.ts +++ b/server/tests/integration/share.test.ts @@ -37,6 +37,7 @@ import { DEFAULT_UPLOADS_ROOT, GLOBAL_TEMP_DIR } from '../../src/nest/storage/st import { createTestOrm } from '../helpers/test-orm'; import { GooglePlacePhotoMeta } from '../../src/db/entities/GooglePlacePhotoMeta.entity'; import { Places } from '../../src/db/entities/Places.entity'; +import { CollectionPlaces } from '../../src/db/entities/CollectionPlaces.entity'; // A real instance over the same connection the app uses — these cases write a // cache entry and then read it back through the HTTP route, so the stub @@ -66,7 +67,7 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); const t = await createTestOrm(sharedDb, { allowGlobalContext: true }); - placePhotoCache = new PlacePhotoCacheService(new DatabaseService(sharedDb), testStorage, t.repo(GooglePlacePhotoMeta), t.repo(Places)); + placePhotoCache = new PlacePhotoCacheService(new DatabaseService(sharedDb), testStorage, t.repo(GooglePlacePhotoMeta), t.repo(Places), t.repo(CollectionPlaces)); }); beforeEach(() => { diff --git a/server/tests/integration/uploads-photo-request-context.test.ts b/server/tests/integration/uploads-photo-request-context.test.ts index 793d5f716..4c944ad14 100644 --- a/server/tests/integration/uploads-photo-request-context.test.ts +++ b/server/tests/integration/uploads-photo-request-context.test.ts @@ -50,7 +50,7 @@ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi. import { db as testDb } from '../../src/db/database'; import { buildApp } from '../../src/bootstrap'; import { resetTestDb } from '../helpers/test-db'; -import { createUser } from '../helpers/factories'; +import { createUser, createTrip } from '../helpers/factories'; import { generateToken } from '../helpers/auth'; import { applyPlatformUploads } from '../../src/nest/platform/platform.routes'; import { StorageService } from '../../src/nest/storage/storage.service'; @@ -123,6 +123,44 @@ describe('GET /uploads/photos/:filename — request context (Plan 3b Task 0, D6) spy.mockRestore(); } }); + + // R1 (Plan 3h Task 6): the anonymous share-token FALLBACK branch of the + // SAME pre-init handler — the one PHOTOCTX-001/002 above never exercised + // (they only drove the JWT half) — now reads `share_tokens` through + // `ShareTokensRepository.findTripIdByToken` inside the SAME `withRequestContext` + // wrap, the plan's single highest-severity finding (an unconverted `db` + // proxy read had run outside Nest's per-request EntityManager fork, + // permanently, since Plan 3b's own D6 wrap covered only the JWT branch). + // These two cases drive the REAL pre-init route, anonymously, through the + // real compiled `buildApp()` boot above — not a wrapper mock. + it('PHOTOCTX-004 (R1): a valid share token serves the SAME photo bytes as the JWT path, byte-identical', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + testDb.prepare('INSERT INTO photos (trip_id, filename, original_name) VALUES (?, ?, ?)').run(trip.id, photoName, photoName); + testDb.prepare('INSERT INTO share_tokens (trip_id, token, created_by) VALUES (?, ?, ?)').run(trip.id, 'ratchet-valid-token', user.id); + + const res = await request(app).get(`/uploads/photos/${photoName}?token=ratchet-valid-token`); + + expect(res.status).toBe(200); + expect(Buffer.from(res.body as Buffer)).toEqual(photoBytes); + }); + + it('PHOTOCTX-005 (R1): an invalid/unknown share token answers the legacy 401 — never `cannotUseGlobalContext`', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + testDb.prepare('INSERT INTO photos (trip_id, filename, original_name) VALUES (?, ?, ?)').run(trip.id, photoName, photoName); + // No matching share_tokens row for this token at all. + + const res = await request(app).get(`/uploads/photos/${photoName}?token=ratchet-unknown-token`); + + expect(res.status).toBe(401); + expect(res.status).not.toBe(500); + // The explicit R1 assertion (rule 13(b)'s shape): the pre-init handler + // never throws `cannotUseGlobalContext` for this branch — the wrap + // covers the ShareTokensRepository read exactly as it covers the + // already-converted Users read. + expect(JSON.stringify(res.body ?? res.text ?? '')).not.toContain('cannotUseGlobalContext'); + }); }); describe('applyPlatformUploads without an ORM — fail-closed (Plan 3b Task 0, D6; mutation proof for PHOTOCTX-001/002)', () => { diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index d833ec95c..bc12195f0 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -97,6 +97,7 @@ import { createTestSettingsRepo, } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; @@ -118,7 +119,7 @@ let svc: AdminService; beforeAll(async () => { webauthn = new WebauthnConfigService(await createTestAppSettingsRepo(dbs.connection)); permissions = new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)); - userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection)); + userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestShareTokensRepo(dbs.connection)); auth = new AuthService( permissions, new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), webauthn, userCleanup, new MailerService(await createTestUsersRepo(dbs.connection), await createTestSettingsRepo(dbs.connection), await createTestAppSettingsRepo(dbs.connection)), new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(dbs.connection)), await createTestUnitOfWork(dbs.connection), await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestInviteTokensRepo(dbs.connection), await createTestMcpTokensRepo(dbs.connection), diff --git a/server/tests/unit/nest/admin.version-notification.test.ts b/server/tests/unit/nest/admin.version-notification.test.ts index 2b94f1eec..80dd93c8d 100644 --- a/server/tests/unit/nest/admin.version-notification.test.ts +++ b/server/tests/unit/nest/admin.version-notification.test.ts @@ -75,6 +75,7 @@ import { createTestSettingsRepo, } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; @@ -96,7 +97,7 @@ let svc: AdminService; beforeAll(async () => { webauthn = new WebauthnConfigService(await createTestAppSettingsRepo(dbs.connection)); permissions = new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)); - userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection)); + userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection), ...(await budgetRepoArgs(dbs.connection))), await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), await createTestShareTokensRepo(dbs.connection)); auth = new AuthService( permissions, new TripMembershipService(await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection)), webauthn, userCleanup, new MailerService(await createTestUsersRepo(dbs.connection), await createTestSettingsRepo(dbs.connection), await createTestAppSettingsRepo(dbs.connection)), new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(dbs.connection)), await createTestUnitOfWork(dbs.connection), await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection), await createTestInviteTokensRepo(dbs.connection), await createTestMcpTokensRepo(dbs.connection), diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index e3d1f9411..acd99acd3 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -95,6 +95,7 @@ import { createTestPasswordResetTokensRepo, } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; @@ -118,7 +119,7 @@ beforeAll(async () => { new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), membershipStub, new WebauthnConfigService(await createTestAppSettingsRepo(testDb)), - new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), mailerStub, new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)), await createTestUnitOfWork(testDb), diff --git a/server/tests/unit/nest/budget.service.db.test.ts b/server/tests/unit/nest/budget.service.db.test.ts index 0f2873760..c86ea5374 100644 --- a/server/tests/unit/nest/budget.service.db.test.ts +++ b/server/tests/unit/nest/budget.service.db.test.ts @@ -74,6 +74,7 @@ import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpe import { notificationsStub } from '../../helpers/notifications'; import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm, createTestTripsRepo, createTestTripMembersRepo } from '../../helpers/test-uow'; import { budgetRepoArgs, createTestBudgetItemMembersRepo, createTestBudgetItemPayersRepo, createTestBudgetSettlementsRepo } from '../../helpers/budget-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; @@ -120,7 +121,7 @@ beforeAll(async () => { membersSvc = new TripMembersService( dbs(), budget, - new UserCleanupService(dbs(), budget, await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb)), + new UserCleanupService(dbs(), budget, await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), diff --git a/server/tests/unit/nest/collections.service.test.ts b/server/tests/unit/nest/collections.service.test.ts index 0c68aff23..83bea83e2 100644 --- a/server/tests/unit/nest/collections.service.test.ts +++ b/server/tests/unit/nest/collections.service.test.ts @@ -104,6 +104,7 @@ beforeAll(async () => { makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(testDb), await createTestPlacesRepo(testDb), + await createTestCollectionPlacesRepo(testDb), ); }); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index 09b6f282b..da95e5c5b 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -119,6 +119,7 @@ import { createTestTripMembersRepo, } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; @@ -141,7 +142,7 @@ beforeAll(async () => { new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), membership, new WebauthnConfigService(await createTestAppSettingsRepo(testDb)), - new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), mailerStub, new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)), await createTestUnitOfWork(testDb), diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index b79a55224..7d6ec3ba7 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -103,6 +103,7 @@ import { createTestTripMembersRepo, } from '../../helpers/test-uow'; import { budgetRepoArgs } from '../../helpers/budget-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; @@ -134,7 +135,7 @@ beforeAll(async () => { new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new TripMembershipService(await createTestTripsRepo(testDb), await createTestTripMembersRepo(testDb)), new WebauthnConfigService(await createTestAppSettingsRepo(testDb)), - new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))), await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb)), mailerStub, new EphemeralTokenService(), new AllowedFileTypesService(await createTestAppSettingsRepo(testDb)), await createTestUnitOfWork(testDb), diff --git a/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts index bc1ae8568..ffec6465b 100644 --- a/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts +++ b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts @@ -11,15 +11,17 @@ * pinned by the storage-registry tests; here the two prefixes prove the cache * itself is mode-agnostic. * - * Rebuilt on GooglePlacePhotoMetaRepository/PlacesRepository (Plan 3c Task 1): - * the legacy version cast a hand-rolled 3-table SQLite fixture to - * `DatabaseService`; that cannot survive the service now calling two - * repositories for its owned reads/writes. The same minimal 3-table fixture - * stays (every column these repositories touch is already in it — the R8 - * `createSnapshotTestDb()` migration only applies once a converted read - * needs a column the hand-rolled DDL omits, which is not the case here), but - * it is now wired through `createTestOrm()` and `DatabaseService` is kept - * ONLY for the one raw `collection_places` statement (PP6's Plan 3h half). + * Rebuilt on GooglePlacePhotoMetaRepository/PlacesRepository (Plan 3c Task 1) + * and, since Plan 3h Task 6, CollectionPlacesRepository too (PP6's + * `collection_places` half, closing out the Plan 3h carve-out): the legacy + * version cast a hand-rolled 3-table SQLite fixture to `DatabaseService`; + * that cannot survive the service now calling three repositories for its + * owned reads/writes. The same minimal 3-table fixture stays (every column + * these repositories touch is already in it — the R8 `createSnapshotTestDb()` + * migration only applies once a converted read needs a column the hand-rolled + * DDL omits, which is not the case here), wired through `createTestOrm()`. + * `DatabaseService` itself is left injected (constructor-ripple scope, see + * the class's own docstring) but is now unused in the body. */ import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; import path from 'node:path'; @@ -63,6 +65,8 @@ import { GooglePlacePhotoMeta } from '../../../../src/db/entities/GooglePlacePho import type { GooglePlacePhotoMetaRepository } from '../../../../src/db/repositories/GooglePlacePhotoMeta.repository'; import { Places } from '../../../../src/db/entities/Places.entity'; import type { PlacesRepository } from '../../../../src/db/repositories/Places.repository'; +import { CollectionPlaces } from '../../../../src/db/entities/CollectionPlaces.entity'; +import type { CollectionPlacesRepository } from '../../../../src/db/repositories/CollectionPlaces.repository'; import { makeStorageFixture, type StorageFixture } from '../../../helpers/storage-fixture'; async function makeJpeg(width: number, height: number): Promise { @@ -77,6 +81,7 @@ function nameFor(placeId: string): string { let t: TestOrm; let metaRepo: GooglePlacePhotoMetaRepository; let placesRepo: PlacesRepository; +let collectionPlacesRepo: CollectionPlacesRepository; // Task 1 fix review L3: this is the one suite in this commit that opts out of // the request-context ratchet (`allowGlobalContext: true`, disabling @@ -100,6 +105,7 @@ beforeAll(async () => { t = await createTestOrm(testDb, { allowGlobalContext: true }); metaRepo = t.repo(GooglePlacePhotoMeta); placesRepo = t.repo(Places); + collectionPlacesRepo = t.repo(CollectionPlaces); }); afterAll(async () => { await t.close(); }); @@ -119,7 +125,7 @@ describe.each([ beforeAll(() => { fx = makeStorageFixture(keyPrefix); - cache = new PlacePhotoCacheService(new DatabaseService(testDb as never), fx.storage, metaRepo, placesRepo); + cache = new PlacePhotoCacheService(new DatabaseService(testDb as never), fx.storage, metaRepo, placesRepo, collectionPlacesRepo); }); beforeEach(() => { @@ -256,11 +262,11 @@ describe.each([ expect(fs.existsSync(filePathFor(id))).toBe(true); }); - // Plan 3c Task 1 PP6 ruling: the `collection_places` half stays raw - // (Plan 3h owns that table) and only runs when the `places` half comes - // back false — this is the direct proof that half is still wired, - // through the SAME `isReferenced` call as the two above. - it('PPC-016: keeps an entry referenced only through collection_places, proving the Plan 3h raw fallback still fires', async () => { + // Plan 3c Task 1 PP6 ruling, converted onto CollectionPlacesRepository by + // Plan 3h Task 6: the `collection_places` half only runs when the + // `places` half comes back false — this is the direct proof that half + // is still wired, through the SAME `isReferenced` call as the two above. + it('PPC-016: keeps an entry referenced only through collection_places, proving the repository-backed fallback still fires', async () => { const id = 'coll-only'; await cache.put(id, await makeJpeg(50, 50), null); testDb.prepare('INSERT INTO collection_places (google_place_id) VALUES (?)').run(id); diff --git a/server/tests/unit/nest/places.directions.test.ts b/server/tests/unit/nest/places.directions.test.ts index 63665516e..cd12b6d7f 100644 --- a/server/tests/unit/nest/places.directions.test.ts +++ b/server/tests/unit/nest/places.directions.test.ts @@ -58,6 +58,7 @@ import { TripPhotos } from '../../../src/db/entities/TripPhotos.entity'; import { makeStorageFixture } from '../../helpers/storage-fixture'; import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, createTestPlacesRepo, createTestTripMembersRepo, createTestDayAssignmentsRepo, createTestCategoriesRepo, createTestTripsRepo, sharedTestOrm } from '../../helpers/test-uow'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestCollectionPlacesRepo } from '../../helpers/test-uow'; import { createTestJourneysRepo, createTestJourneyContributorsRepo, createTestJourneyTripsRepo, createTestJourneyEntriesRepo, createTestJourneyPhotosRepo, createTestJourneyEntryPhotosRepo, @@ -113,6 +114,7 @@ async function svc(searchNominatim: MapsService['searchNominatim']): Promise { expect(res.statusCode).toBe(401); }); + // R1/R5 (Plan 3h Task 6): the share-token half of this handler now reads + // through `ShareTokensRepository.findTripIdByToken` — `orm.em.getRepository + // (ShareTokens)` inside the SAME `withRequestContext` wrap `applyPlatformUploads` + // already uses for the JWT half — so these cases seed REAL rows in + // `uploadsTestDb` (bound to `uploadsOrm.orm`) rather than mocking + // `db.prepare`'s return value the way the (still-unconverted) sibling + // `photos` read below continues to. `photoStmt` stays a `db.prepare` mock + // (untouched domain) throughout. + function insertShareToken(tripId: number, userId: number, token: string, expiresAt: string | null = null) { + uploadsTestDb.prepare('INSERT INTO share_tokens (trip_id, token, created_by, expires_at) VALUES (?, ?, ?, ?)') + .run(tripId, token, userId, expiresAt); + } + it('401 when a share token does not cover the photo trip', async () => { + const { user } = createUser(uploadsTestDb); + const photoTrip = createTrip(uploadsTestDb, user.id); + const otherTrip = createTrip(uploadsTestDb, user.id); + insertShareToken(otherTrip.id, user.id, 'share-mismatch'); h.exists.mockResolvedValue(true); h.verifyJwtAndLoadUser.mockReturnValue(null); - const photoStmt = { get: vi.fn().mockReturnValue({ trip_id: 7 }) }; - const shareStmt = { get: vi.fn().mockReturnValue({ trip_id: 8 }) }; - h.dbPrepare.mockImplementationOnce(() => photoStmt).mockImplementationOnce(() => shareStmt); + h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); const res = makeRes(); - await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share1' } }, res, next); + await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-mismatch' } }, res, next); expect(res.statusCode).toBe(401); }); - it('401 when there is no matching share token at all', async () => { + it('R5: 401 when there is no matching share token at all (unknown)', async () => { + const { user } = createUser(uploadsTestDb); + const photoTrip = createTrip(uploadsTestDb, user.id); h.exists.mockResolvedValue(true); h.verifyJwtAndLoadUser.mockReturnValue(null); - const photoStmt = { get: vi.fn().mockReturnValue({ trip_id: 7 }) }; - const shareStmt = { get: vi.fn().mockReturnValue(undefined) }; - h.dbPrepare.mockImplementationOnce(() => photoStmt).mockImplementationOnce(() => shareStmt); + h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); const res = makeRes(); - await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share1' } }, res, next); + await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'never-issued' } }, res, next); expect(res.statusCode).toBe(401); }); - it('serves the file when the share token covers the photo trip', async () => { + it('R5: 401 when the token is revoked (deleted, not merely unknown)', async () => { + const { user } = createUser(uploadsTestDb); + const photoTrip = createTrip(uploadsTestDb, user.id); + insertShareToken(photoTrip.id, user.id, 'share-revoked'); + uploadsTestDb.prepare('DELETE FROM share_tokens WHERE token = ?').run('share-revoked'); + h.exists.mockResolvedValue(true); + h.verifyJwtAndLoadUser.mockReturnValue(null); + h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + const res = makeRes(); + await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-revoked' } }, res, next); + expect(res.statusCode).toBe(401); + }); + + it('R5: 401 when the token is expired', async () => { + const { user } = createUser(uploadsTestDb); + const photoTrip = createTrip(uploadsTestDb, user.id); + insertShareToken(photoTrip.id, user.id, 'share-expired', '2020-01-01 00:00:00'); + h.exists.mockResolvedValue(true); + h.verifyJwtAndLoadUser.mockReturnValue(null); + h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + const res = makeRes(); + await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-expired' } }, res, next); + expect(res.statusCode).toBe(401); + }); + + it('R5: 401 for a wrong-case token — no case-folding is introduced', async () => { + const { user } = createUser(uploadsTestDb); + const photoTrip = createTrip(uploadsTestDb, user.id); + insertShareToken(photoTrip.id, user.id, 'share-CaseSensitive'); + h.exists.mockResolvedValue(true); + h.verifyJwtAndLoadUser.mockReturnValue(null); + h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + const res = makeRes(); + await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-casesensitive' } }, res, next); + expect(res.statusCode).toBe(401); + }); + + it('R5: 401 for a token with an embedded NUL byte', async () => { + const { user } = createUser(uploadsTestDb); + const photoTrip = createTrip(uploadsTestDb, user.id); + insertShareToken(photoTrip.id, user.id, 'share-nul'); + h.exists.mockResolvedValue(true); + h.verifyJwtAndLoadUser.mockReturnValue(null); + h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); + const res = makeRes(); + await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'share-nul\0extra' } }, res, next); + expect(res.statusCode).toBe(401); + }); + + it('serves the file when the share token covers the photo trip (R1 valid-token case)', async () => { + const { user } = createUser(uploadsTestDb); + const photoTrip = createTrip(uploadsTestDb, user.id); + insertShareToken(photoTrip.id, user.id, 'share-valid'); h.exists.mockResolvedValue(true); h.sendToResponse.mockResolvedValue(undefined); h.verifyJwtAndLoadUser.mockReturnValue(null); - const photoStmt = { get: vi.fn().mockReturnValue({ trip_id: 7 }) }; - const shareStmt = { get: vi.fn().mockReturnValue({ trip_id: 7 }) }; - h.dbPrepare.mockImplementationOnce(() => photoStmt).mockImplementationOnce(() => shareStmt); + h.dbPrepare.mockReturnValue({ get: vi.fn().mockReturnValue({ trip_id: photoTrip.id }) }); const res = makeRes(); await photoHandler()( - { params: { filename: 'a.jpg' }, headers: { authorization: 'Bearer share1' }, query: {} }, + { params: { filename: 'a.jpg' }, headers: { authorization: 'Bearer share-valid' }, query: {} }, res, next, ); diff --git a/server/tests/unit/nest/share.service.test.ts b/server/tests/unit/nest/share.service.test.ts index 059c601ff..09e1274e0 100644 --- a/server/tests/unit/nest/share.service.test.ts +++ b/server/tests/unit/nest/share.service.test.ts @@ -54,13 +54,13 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, addTripMember, createDay, createPlace, createDayAssignment, } from '../../helpers/factories'; -import { DatabaseService } from '../../../src/nest/database/database.service'; import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { ShareService, publicReservationMetadata } from '../../../src/nest/share/share.service'; import { SettingsService } from '../../../src/nest/settings/settings.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import type { User } from '../../../src/types'; -import { sharedTestOrm, createTestUnitOfWork, createTestAppSettingsRepo, createTestSettingsRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, createTestReservationsRepo } from '../../helpers/test-uow'; +import { sharedTestOrm, createTestUnitOfWork, createTestAppSettingsRepo, createTestSettingsRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo } from '../../helpers/test-uow'; +import { shareServiceRepoArgs } from '../../helpers/share-repos'; import type { TestOrm } from '../../helpers/test-orm'; let svc: ShareService; @@ -73,15 +73,13 @@ beforeAll(async () => { createTables(testDb); runMigrations(testDb); t = await sharedTestOrm(testDb); - const dbs = new DatabaseService(testDb, t.em); svc = new ShareService( - dbs, new SettingsService(await createTestUnitOfWork(testDb), await createTestAppSettingsRepo(testDb), await createTestSettingsRepo(testDb)), permissionsStub, - new QueryHelpersService(await createTestTagsRepo(dbs.connection), await createTestPlaceRatingsRepo(dbs.connection), await createTestAssignmentParticipantsRepo(dbs.connection)), + new QueryHelpersService(await createTestTagsRepo(testDb), await createTestPlaceRatingsRepo(testDb), await createTestAssignmentParticipantsRepo(testDb)), photoCacheStub, await createTestUnitOfWork(testDb), - await createTestReservationsRepo(testDb), + ...(await shareServiceRepoArgs(testDb)), ); }); @@ -249,6 +247,34 @@ describe('getSharedTripData', () => { expect(await svc.getSharedTripData(token)).not.toBeNull(); }); + // R5's six-case matrix (valid, revoked, unknown, wrong-case, NUL-byte, + // expired) — SHARE-SVC-010/011 above already pin unknown/expired; the + // remaining four are named separately here, per the plan's "a failure in + // one branch must not be masked by another's passing case" discipline. + it('SHARE-SVC-039 (R5, valid): a freshly created token returns the shared payload', async () => { + const { token } = await seedSharedTrip(); + expect(await svc.getSharedTripData(token)).not.toBeNull(); + }); + + it('SHARE-SVC-040 (R5, revoked): a deleted token returns null — distinct from merely unknown', async () => { + const { trip, token } = await seedSharedTrip(); + expect(await svc.getSharedTripData(token)).not.toBeNull(); + testDb.prepare('DELETE FROM share_tokens WHERE trip_id = ?').run(trip.id); + expect(await svc.getSharedTripData(token)).toBeNull(); + }); + + it('SHARE-SVC-041 (R5, wrong-case): a differently-cased token never matches — no case-folding introduced (mutation proof: a COLLATE NOCASE companion would flip this red)', async () => { + const { token } = await seedSharedTrip(); + const flipped = token === token.toUpperCase() ? token.toLowerCase() : token.toUpperCase(); + expect(flipped).not.toBe(token); + expect(await svc.getSharedTripData(flipped)).toBeNull(); + }); + + it('SHARE-SVC-042 (R5, NUL-byte): a token with an embedded NUL byte never matches', async () => { + const { token } = await seedSharedTrip(); + expect(await svc.getSharedTripData(`${token}\0x`)).toBeNull(); + }); + it('SHARE-SVC-012: returns null when the trip row is gone', async () => { const { trip, token } = await seedSharedTrip(); // The token normally cascades away with its trip; orphan it deliberately @@ -557,6 +583,45 @@ describe('getSharedTripData redaction (#2320)', () => { expect(JSON.stringify(data)).not.toContain('ChIJ123'); }); + it('SHARE-SVC-046 (SH12 full-key parity): every allow-listed column of a fully-populated place reaches the public payload, nothing else does', async () => { + const { trip, token } = await seedSharedTrip(); + const place = createPlace(testDb, trip.id, { name: 'Fully Populated' }); + testDb.prepare(`UPDATE places SET + description = 'A description', address = '1 Rue Test', price = 12.5, currency = 'EUR', + place_time = '09:00', end_time = '10:00', duration_minutes = 60, notes = 'A public note', + image_url = '/uploads/pic.jpg', website = 'https://example.com', phone = '+33 1 23', + transport_mode = 'walk', + reservation_status = 'confirmed', reservation_notes = 'Private booking note', reservation_datetime = '2026-01-01T09:00', + google_place_id = 'ChIJ-private', google_ftid = 'ftid-private', osm_id = 'osm-private', amap_poi_id = 'amap-private', + route_geometry = '{"type":"LineString"}', route_color = '#ff0000', stop_type = 'hotel', fill_percent = 42, + source = 'import' + WHERE id = ?`).run(place.id); + + const data = (await svc.getSharedTripData(token))!; + const pool = (data.places as any[])[0]; + + // The full allow-list (20 `places` columns + the 3 flat category-join + // columns), and nothing more. + expect(Object.keys(pool).sort()).toEqual([ + 'address', 'category_color', 'category_icon', 'category_id', 'category_name', 'created_at', 'currency', + 'description', 'duration_minutes', 'end_time', 'id', 'image_url', 'lat', 'lng', 'name', 'notes', 'phone', + 'place_time', 'price', 'transport_mode', 'trip_id', 'updated_at', 'website', + ]); + expect(pool.notes).toBe('A public note'); + expect(pool.address).toBe('1 Rue Test'); + expect(pool.duration_minutes).toBe(60); + + // Every non-allow-listed column, whole-payload-wide. + const whole = JSON.stringify(data); + for (const secret of [ + 'Private booking note', '2026-01-01T09:00', + 'ChIJ-private', 'ftid-private', 'osm-private', 'amap-private', + 'LineString', '#ff0000', + ]) { + expect(whole, secret).not.toContain(secret); + } + }); + it('SHARE-SVC-037: a stay carries its desk times and note, not its confirmation', async () => { const { trip, token } = await seedSharedTrip(); const place = createPlace(testDb, trip.id, { name: 'Hotel' }); @@ -592,6 +657,32 @@ describe('getSharedPlacePhotoKey', () => { expect(serveKey).not.toHaveBeenCalled(); }); + // R5's six-case matrix, this method's own named cases (SHARE-SVC-022 above + // already pins unknown/expired — same predicate, same {@link + // ShareTokensRepository.findTripAndShareMapByToken}, proven separately here + // since a regression in this method's own call site would not otherwise be + // caught). + it('SHARE-SVC-043 (R5, revoked): a deleted token returns null — distinct from merely unknown', async () => { + const { trip, token } = await seedSharedTrip(); + const place = createPlace(testDb, trip.id); + testDb.prepare('UPDATE places SET image_url = ? WHERE id = ?').run('/api/maps/place-photo/ChIJabc/bytes', place.id); + expect(await svc.getSharedPlacePhotoKey(token, 'ChIJabc')).not.toBeNull(); + testDb.prepare('DELETE FROM share_tokens WHERE trip_id = ?').run(trip.id); + expect(await svc.getSharedPlacePhotoKey(token, 'ChIJabc')).toBeNull(); + }); + + it('SHARE-SVC-044 (R5, wrong-case): a differently-cased token never matches (mutation proof: a COLLATE NOCASE companion would flip this red)', async () => { + const { token } = await seedSharedTrip(); + const flipped = token === token.toUpperCase() ? token.toLowerCase() : token.toUpperCase(); + expect(flipped).not.toBe(token); + expect(await svc.getSharedPlacePhotoKey(flipped, 'ChIJabc')).toBeNull(); + }); + + it('SHARE-SVC-045 (R5, NUL-byte): a token with an embedded NUL byte never matches', async () => { + const { token } = await seedSharedTrip(); + expect(await svc.getSharedPlacePhotoKey(`${token}\0x`, 'ChIJabc')).toBeNull(); + }); + it('SHARE-SVC-023: returns null when the owner disabled the map section', async () => { const { trip, token } = await seedSharedTrip({ share_map: false }); const place = createPlace(testDb, trip.id); diff --git a/server/tests/unit/nest/trip-members.service.test.ts b/server/tests/unit/nest/trip-members.service.test.ts index d8ecc3dc5..f2ec2d27a 100644 --- a/server/tests/unit/nest/trip-members.service.test.ts +++ b/server/tests/unit/nest/trip-members.service.test.ts @@ -72,6 +72,7 @@ import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; import type { EntityManager } from '@mikro-orm/core'; import type { TripsRepository } from '../../../src/db/repositories/Trips.repository'; import type { UsersRepository } from '../../../src/db/repositories/Users.repository'; @@ -104,7 +105,7 @@ beforeAll(async () => { budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection), ...(await budgetRepoArgs(dbs().connection))); roster = new TripMembersService( dbs(), budgetSvc, - new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), usersRepo, await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection)), + new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), usersRepo, await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestShareTokensRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(notifySend), await createTestUnitOfWork(dbs().connection), tripsRepo, tripMembersRepo, usersRepo, diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts index 316fb9c9f..fa8882a0b 100644 --- a/server/tests/unit/nest/trip-read-model.service.test.ts +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -109,6 +109,8 @@ import { createTestJourneyPhotosRepo, createTestJourneyEntryPhotosRepo, } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; +import { createTestCollectionPlacesRepo } from '../../helpers/test-uow'; // Real sibling services over the same in-memory DB — the aggregation runs the // actual SQL of every domain it fans out to, so a shape change downstream shows @@ -135,6 +137,7 @@ beforeAll(async () => { makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(dbs().connection), await createTestPlacesRepo(dbs().connection), + await createTestCollectionPlacesRepo(dbs().connection), ); budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection), ...(await budgetRepoArgs(dbs().connection))); daysSvc = new DaysService( @@ -172,8 +175,9 @@ beforeAll(async () => { await createTestCategoriesRepo(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), + await createTestCollectionPlacesRepo(dbs().connection), ); - membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); + membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestShareTokensRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); }); const buildReadModel = async (tripsRepo: TripsRepository, roster: TripMembersService = membersSvc) => diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 12fb0bc6f..6e08df2ba 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -126,6 +126,8 @@ import { createTestJourneyPhotosRepo, createTestJourneyEntryPhotosRepo, } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; +import { createTestCollectionPlacesRepo } from '../../helpers/test-uow'; // Real sibling services over the same in-memory DB — updateTrip's date-shift // resyncs and the summary/bundle aggregation run their actual SQL. @@ -176,6 +178,7 @@ beforeAll(async () => { makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(dbs().connection), await createTestPlacesRepo(dbs().connection), + await createTestCollectionPlacesRepo(dbs().connection), ); budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection), ...(await budgetRepoArgs(dbs().connection))); daysSvc = new DaysService( @@ -217,6 +220,7 @@ beforeAll(async () => { await createTestCategoriesRepo(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), + await createTestCollectionPlacesRepo(dbs().connection), ); svc = new TripsService( dbs(), @@ -239,7 +243,7 @@ beforeAll(async () => { await createTestUnitOfWork(dbs().connection), (await sharedTestOrm(testDb)).em, ); - membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); + membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestBudgetItemsRepo(dbs().connection), await createTestJourneyShareTokensRepo(dbs().connection), await createTestJourneysRepo(dbs().connection), await createTestJourneyEntriesRepo(dbs().connection), await createTestJourneyContributorsRepo(dbs().connection), await createTestShareTokensRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection)); readModelSvc = new TripReadModelService( await createTestTripsRepo(dbs().connection), membersSvc, daysSvc, accommodationsSvc, budgetSvc, new PackingService( diff --git a/server/tests/unit/nest/user-cleanup.service.test.ts b/server/tests/unit/nest/user-cleanup.service.test.ts index 20a82bee0..1f8902655 100644 --- a/server/tests/unit/nest/user-cleanup.service.test.ts +++ b/server/tests/unit/nest/user-cleanup.service.test.ts @@ -59,6 +59,7 @@ import { budgetRepoArgs } from '../../helpers/budget-repos'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; import { createTestJourneysRepo, createTestJourneyEntriesRepo, createTestJourneyContributorsRepo } from '../../helpers/journey-repos'; import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos'; +import { createTestShareTokensRepo } from '../../helpers/share-repos'; const dbs = new DatabaseService(testDb); @@ -80,6 +81,8 @@ beforeAll(async () => { // Plan 3g Task 4 constructor-ripple: UC7-10's repositories. await createTestJourneyShareTokensRepo(dbs.connection), await createTestJourneysRepo(dbs.connection), await createTestJourneyEntriesRepo(dbs.connection), await createTestJourneyContributorsRepo(dbs.connection), + // Plan 3h Task 6 constructor-ripple: UC6's repository. + await createTestShareTokensRepo(dbs.connection), ); }); @@ -182,6 +185,10 @@ describe('erasePluginUserData', () => { new DatabaseService(slim), budget, await createTestUnitOfWork(slim), await createTestUsersRepo(slim), {} as unknown as BudgetItemsRepository, {} as unknown as JourneyShareTokensRepository, {} as unknown as JourneysRepository, {} as unknown as JourneyEntriesRepository, {} as unknown as JourneyContributorsRepository, + // Plan 3h Task 6: a real repository (never a stub cast — `erasePluginUserData` + // never touches it, but MikroORM's entity metadata does not require the + // physical table to exist to construct the repository object itself). + await createTestShareTokensRepo(slim), ); await expect(slimSvc.erasePluginUserData(1)).resolves.toBeUndefined(); @@ -222,7 +229,7 @@ describe('deleteUserCompletely', () => { expect(testDb.prepare('SELECT COUNT(*) AS c FROM journey_entries').get()).toEqual({ c: 0 }); }); - it('USER-CLEANUP-010: GDPR erasure — UC7-10 remove every journey-table row the departing user reaches, and only those', async () => { + it('USER-CLEANUP-010: GDPR erasure — UC6-10 remove every journey/share-table row the departing user reaches, and only those', async () => { const { user: victim } = createUser(testDb, { username: 'gdpr-victim' }); const { user: second } = createUser(testDb, { username: 'gdpr-second' }); const { user: third } = createUser(testDb, { username: 'gdpr-third' }); @@ -246,6 +253,19 @@ describe('deleteUserCompletely', () => { .run(thirdJourney, victim.id); testDb.prepare("INSERT INTO journey_share_tokens (journey_id, token, created_by) VALUES (?, 'third-tok', ?)").run(thirdJourney, victim.id); + // R10/UC6, full-key: `share_tokens` (trip-level, `nest/share`) — a + // GENUINELY DIFFERENT table from `journey_share_tokens` above, on the + // SAME fixture per the ruling's own instruction. Victim creates a share + // link on a trip they OWN, and — `created_by` need not equal the trip's + // owner — a SECOND share link on a trip owned by `second` (not the + // victim). A THIRD share link, on that same foreign trip but created by + // `third` (not the victim), is the control row UC6 must NOT touch. + const ownTrip = createTrip(testDb, victim.id); + const foreignTrip = createTrip(testDb, second.id); + testDb.prepare("INSERT INTO share_tokens (trip_id, token, created_by) VALUES (?, 'own-trip-tok', ?)").run(ownTrip.id, victim.id); + testDb.prepare("INSERT INTO share_tokens (trip_id, token, created_by) VALUES (?, 'foreign-trip-tok', ?)").run(foreignTrip.id, victim.id); + testDb.prepare("INSERT INTO share_tokens (trip_id, token, created_by) VALUES (?, 'control-tok', ?)").run(foreignTrip.id, third.id); + await svc.deleteUserCompletely(victim.id); // Everything the departing user owned is gone (UC8 + cascade). @@ -260,6 +280,14 @@ describe('deleteUserCompletely', () => { // Nothing extra removed: the other two users' own journeys survive. expect(testDb.prepare('SELECT id FROM journeys WHERE id = ?').get(secondJourney)).toBeDefined(); expect(testDb.prepare('SELECT id FROM journeys WHERE id = ?').get(thirdJourney)).toBeDefined(); + + // UC6/R10: every `share_tokens` row `created_by` the departing user is + // gone, on BOTH their own trip and a trip they don't own — and the + // control row (same foreign trip, created by someone else) survives + // untouched. `ownTrip` itself cascades away with the user's cleanup + // elsewhere (not this test's concern); the assertion is on `created_by`. + expect(testDb.prepare('SELECT id FROM share_tokens WHERE created_by = ?').get(victim.id)).toBeUndefined(); + expect(testDb.prepare('SELECT token FROM share_tokens WHERE trip_id = ?').get(foreignTrip.id)).toEqual({ token: 'control-tok' }); }); it('USER-CLEANUP-008: re-derives the expense divisor before the member rows cascade away', async () => { @@ -295,6 +323,7 @@ describe('deleteUserCompletely', () => { dbs, budget, await createTestUnitOfWork(testDb), usersRepo, await createTestBudgetItemsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), + await createTestShareTokensRepo(testDb), ).deleteUserCompletely(victim.id)).rejects.toThrow('boom'); expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(victim.id)).toBeDefined(); diff --git a/server/tests/unit/services/conflictUpdate.test.ts b/server/tests/unit/services/conflictUpdate.test.ts index a2f277e6b..da2dde6cf 100644 --- a/server/tests/unit/services/conflictUpdate.test.ts +++ b/server/tests/unit/services/conflictUpdate.test.ts @@ -73,6 +73,7 @@ import { sharedTestOrm, } from '../../helpers/test-uow'; import { createTestBudgetItemsRepo } from '../../helpers/files-repos'; +import { createTestCollectionPlacesRepo } from '../../helpers/test-uow'; import { createTestJourneysRepo, createTestJourneyContributorsRepo, createTestJourneyTripsRepo, createTestJourneyEntriesRepo, createTestJourneyPhotosRepo, createTestJourneyEntryPhotosRepo, @@ -104,6 +105,7 @@ beforeAll(async () => { makeStorageFixture('photos/google/').storage, await createTestGooglePlacePhotoMetaRepo(dbs.connection), await createTestPlacesRepo(dbs.connection), + await createTestCollectionPlacesRepo(dbs.connection), ); // Plan 3c Task 0b: `dbs` is constructed at module load, before any // `beforeAll` can resolve a real `EntityManager` — the four @@ -155,6 +157,7 @@ beforeAll(async () => { await createTestCategoriesRepo(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestBudgetItemsRepo(dbs.connection), + await createTestCollectionPlacesRepo(dbs.connection), ); });