mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(journey,files): close the cross-trip and role holes on the sharing surface
Files: two :id handlers took the file id from the URL and never resolved it against :tripId. Membership of any trip was enough to read another trip's file links (with the joined reservation titles) or to delete one. Both now resolve the file the way every sibling handler already does. Journey: a contributor with role 'viewer' - blocked from every other write in the domain - could overwrite or delete the Studio book, and could read the share token, which is an anonymous credential that outlives their contributor status. The book writes take canEdit now and the token read takes the owner check its own create and delete already use. Also: the share-link MCP tool reset every flag to permissive defaults when it was called to refresh a link, so a journey deliberately shared without its gallery or map quietly started publishing both; setShareLink dropped newest_first entirely; the public routes ignored the Journey addon switch; and photo uploads committed to storage before the access check, leaking the objects on the 403 path. Trips: DELETE answered 403 to someone with no access at all, which turns sequential ids into an existence oracle. A member who simply lacks trip_delete still gets 403.
This commit is contained in:
Maurice
@@ -31,8 +31,10 @@ export function isVideoExtension(ext: string): boolean {
|
||||
// `allowed_file_types` is `*`, this list is still enforced so the
|
||||
// wildcard doesn't silently admit executables/scripts.
|
||||
export const BLOCKED_EXTENSIONS = [
|
||||
// Server-rendered / scripted content that could XSS a viewer
|
||||
'.svg', '.html', '.htm', '.xml', '.xhtml',
|
||||
// Server-rendered / scripted content that could XSS a viewer. Downloads are
|
||||
// served inline with an extension-derived Content-Type, so every spelling a
|
||||
// browser renders as a document has to be listed, not just the common ones.
|
||||
'.svg', '.svgz', '.html', '.htm', '.shtml', '.shtm', '.xml', '.xhtml', '.xht',
|
||||
// Scripts
|
||||
'.js', '.jsx', '.ts', '.tsx', '.mjs', '.cjs', '.php', '.py', '.rb', '.pl',
|
||||
// Executables
|
||||
|
||||
@@ -279,6 +279,13 @@ export class FilesController {
|
||||
if (!this.files.can('file_edit', trip, user)) {
|
||||
throw new HttpException({ error: 'No permission' }, 403);
|
||||
}
|
||||
// deleteFileLink scopes by (linkId, fileId) only, so the file itself has to
|
||||
// be resolved against :tripId first. Otherwise a member of any trip could
|
||||
// drop a link row belonging to a foreign trip's file.
|
||||
const file = this.files.getFileById(id, tripId);
|
||||
if (!file) {
|
||||
throw new HttpException({ error: 'File not found' }, 404);
|
||||
}
|
||||
this.files.deleteFileLink(linkId, id);
|
||||
return { success: true };
|
||||
}
|
||||
@@ -286,6 +293,10 @@ export class FilesController {
|
||||
@UseGuards(TripAccessGuard)
|
||||
@Get(':id/links')
|
||||
links(@CurrentUser() user: User, @Trip() trip: TripAccess, @Param('tripId') tripId: string, @Param('id') id: string) {
|
||||
const file = this.files.getFileById(id, tripId);
|
||||
if (!file) {
|
||||
throw new HttpException({ error: 'File not found' }, 404);
|
||||
}
|
||||
return { links: this.files.getFileLinks(id) };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,10 +9,11 @@ import { JourneyDomainService } from './journey-domain.service';
|
||||
*
|
||||
* ── Access ───────────────────────────────────────────────────────────────
|
||||
*
|
||||
* A book belongs to its journey, so it inherits the journey's access exactly —
|
||||
* every contributor can open and edit it. There is no separate book permission
|
||||
* and there should not be: a second access model over the same object is how
|
||||
* two rules end up disagreeing about who may do what.
|
||||
* A book belongs to its journey, so it inherits the journey's access exactly:
|
||||
* every contributor can open it, and the ones who may edit the journey may edit
|
||||
* it. There is no separate book permission and there should not be: a second
|
||||
* access model over the same object is how two rules end up disagreeing about
|
||||
* who may do what.
|
||||
*
|
||||
* ── Concurrency ──────────────────────────────────────────────────────────
|
||||
*
|
||||
@@ -37,6 +38,15 @@ export class JourneyBookService {
|
||||
return !!this.journey.canAccessJourney(journeyId, userId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Writing the book is an edit like any other in this domain, so it takes the
|
||||
* same owner-or-editor check the entry and photo writes take. canAccess also
|
||||
* covers role 'viewer', who may read the book but must not overwrite it.
|
||||
*/
|
||||
private canWrite(journeyId: number, userId: number): boolean {
|
||||
return this.journey.canEdit(journeyId, userId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this user may open the journey's book at all.
|
||||
*
|
||||
@@ -115,7 +125,7 @@ export class JourneyBookService {
|
||||
userId: number,
|
||||
input: { title: string; document: unknown; baseVersion?: number },
|
||||
): { record: BookRecord } | { conflict: BookRecord } | null {
|
||||
if (!this.canAccess(journeyId, userId)) return null;
|
||||
if (!this.canWrite(journeyId, userId)) return null;
|
||||
|
||||
const document = JSON.stringify(normalizeBookDocument(input.document));
|
||||
const existing = this.db
|
||||
@@ -160,7 +170,7 @@ export class JourneyBookService {
|
||||
}
|
||||
|
||||
deleteBook(journeyId: number, userId: number): boolean | null {
|
||||
if (!this.canAccess(journeyId, userId)) return null;
|
||||
if (!this.canWrite(journeyId, userId)) return null;
|
||||
const result = this.db
|
||||
.prepare('DELETE FROM journey_books WHERE journey_id = ?')
|
||||
.run(journeyId);
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
import { Controller, Get, HttpException, Param, Res } from '@nestjs/common';
|
||||
import { Controller, Get, HttpException, Param, Res, UseGuards } from '@nestjs/common';
|
||||
import type { Response } from 'express';
|
||||
import path from 'node:path';
|
||||
import { JourneyService } from './journey.service';
|
||||
import { StorageService } from '../storage/storage.service';
|
||||
import { AddonGuard } from '../addons/addon.guard';
|
||||
import { RequireAddon } from '../addons/require-addon.decorator';
|
||||
import { ADDON_IDS } from '../../addons';
|
||||
import { Public } from '../auth/public.decorator';
|
||||
|
||||
/**
|
||||
@@ -10,11 +13,18 @@ import { Public } from '../auth/public.decorator';
|
||||
* proxy for publicly shared journeys.
|
||||
*
|
||||
* Byte-identical to the legacy Express route (server/src/routes/journeyPublic.ts):
|
||||
* NOT behind any guard, every route validates the share token (404 on failure),
|
||||
* no authentication, every route validates the share token (404 on failure),
|
||||
* the unified proxy streams by trek_photo_id and the legacy proxy serves local
|
||||
* files (with the uploads-dir traversal guard) or proxies immich/synology.
|
||||
*
|
||||
* The addon gate is the one guard here, and it has to be: turning the Journey
|
||||
* addon off is meant to take the feature away, and without it already-published
|
||||
* journeys stayed publicly readable while the authenticated surface went dark.
|
||||
* AddonGuard only reads addon state, so it is safe in front of a @Public route.
|
||||
*/
|
||||
@Public('share-token validated: the whole point is a link that works without an account')
|
||||
@UseGuards(AddonGuard)
|
||||
@RequireAddon(ADDON_IDS.JOURNEY, 'Journey')
|
||||
@Controller('api/public/journey')
|
||||
export class JourneyPublicController {
|
||||
constructor(
|
||||
@@ -72,7 +82,9 @@ export class JourneyPublicController {
|
||||
return;
|
||||
}
|
||||
|
||||
const effectiveOwnerId = valid.ownerId || Number(ownerId);
|
||||
// :ownerId stays in the path so old share links keep routing, but it is not
|
||||
// read any more: the owner comes from the row the token resolved to.
|
||||
const effectiveOwnerId = valid.ownerId;
|
||||
const streaming = this.journey.streamProviderAsset(
|
||||
res,
|
||||
provider,
|
||||
|
||||
@@ -44,6 +44,22 @@ export class JourneyShareService {
|
||||
// able to publish the journey or change which screens are shared.
|
||||
if (!this.journey.isOwner(journeyId, createdBy)) return null;
|
||||
|
||||
const existing = this.db.prepare('SELECT token, share_timeline, share_gallery, share_map, newest_first FROM journey_share_tokens WHERE journey_id = ?')
|
||||
.get(journeyId) as { token: string; share_timeline: number; share_gallery: number; share_map: number; newest_first: number } | undefined;
|
||||
|
||||
if (existing) {
|
||||
// An update only changes the flags it was actually given. Falling back to
|
||||
// the create-time defaults here would silently re-publish a gallery or map
|
||||
// the owner had switched off, at the unchanged token.
|
||||
const share_timeline = permissions.share_timeline ?? !!existing.share_timeline;
|
||||
const share_gallery = permissions.share_gallery ?? !!existing.share_gallery;
|
||||
const share_map = permissions.share_map ?? !!existing.share_map;
|
||||
const newest_first = permissions.newest_first ?? !!existing.newest_first;
|
||||
this.db.prepare('UPDATE journey_share_tokens SET share_timeline = ?, share_gallery = ?, share_map = ?, newest_first = ? WHERE journey_id = ?')
|
||||
.run(share_timeline ? 1 : 0, share_gallery ? 1 : 0, share_map ? 1 : 0, newest_first ? 1 : 0, journeyId);
|
||||
return { token: existing.token, created: false };
|
||||
}
|
||||
|
||||
const {
|
||||
share_timeline = true,
|
||||
share_gallery = true,
|
||||
@@ -51,13 +67,6 @@ export class JourneyShareService {
|
||||
newest_first = false,
|
||||
} = permissions;
|
||||
|
||||
const existing = this.db.prepare('SELECT token FROM journey_share_tokens WHERE journey_id = ?').get(journeyId) as { token: string } | undefined;
|
||||
if (existing) {
|
||||
this.db.prepare('UPDATE journey_share_tokens SET share_timeline = ?, share_gallery = ?, share_map = ?, newest_first = ? WHERE journey_id = ?')
|
||||
.run(share_timeline ? 1 : 0, share_gallery ? 1 : 0, share_map ? 1 : 0, newest_first ? 1 : 0, journeyId);
|
||||
return { token: existing.token, created: false };
|
||||
}
|
||||
|
||||
const token = crypto.randomBytes(24).toString('base64url');
|
||||
this.db.prepare('INSERT INTO journey_share_tokens (journey_id, token, created_by, share_timeline, share_gallery, share_map, newest_first) VALUES (?, ?, ?, ?, ?, ?, ?)')
|
||||
.run(journeyId, token, createdBy, share_timeline ? 1 : 0, share_gallery ? 1 : 0, share_map ? 1 : 0, newest_first ? 1 : 0);
|
||||
@@ -109,13 +118,18 @@ export class JourneyShareService {
|
||||
// owner shared the gallery.
|
||||
if (!row.share_gallery) return null;
|
||||
const photo = this.db.prepare(`
|
||||
SELECT tkp.owner_id FROM journey_photos gp
|
||||
SELECT tkp.owner_id, j.user_id AS journey_owner_id
|
||||
FROM journey_photos gp
|
||||
JOIN trek_photos tkp ON tkp.id = gp.photo_id
|
||||
JOIN journeys j ON j.id = gp.journey_id
|
||||
WHERE tkp.asset_id = ? AND gp.journey_id = ?
|
||||
`).get(assetId, row.journey_id) as any;
|
||||
// Only resolve assets that actually belong to this shared journey.
|
||||
if (!photo) return null;
|
||||
return { ownerId: photo.owner_id };
|
||||
// trek_photos.owner_id can be NULL. The journey's owner is the fallback, the
|
||||
// same one the photo proxy uses. Whose provider credentials get tried must
|
||||
// never come from a number an anonymous caller put in the URL.
|
||||
return { ownerId: photo.owner_id || photo.journey_owner_id };
|
||||
}
|
||||
|
||||
getPublicJourney(token: string) {
|
||||
|
||||
@@ -152,6 +152,20 @@ export class JourneyController {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop uploads that were committed before the authorization result was known.
|
||||
*
|
||||
* The commit has to happen first (the Immich mirror reads the final path), so
|
||||
* every refusal after it has to take the bytes back out. Nothing sweeps
|
||||
* orphans, and a loop of rejected POSTs would otherwise just fill the disk.
|
||||
*/
|
||||
private async discardJourneyUploads(files: Express.Multer.File[]): Promise<void> {
|
||||
for (const f of files) {
|
||||
if (f.path) { try { fs.unlinkSync(f.path); } catch { /* best-effort */ } }
|
||||
await this.storage.delete('journey', f.filename).catch(() => {});
|
||||
}
|
||||
}
|
||||
|
||||
// The add call carries only an asset id, so when and where the picture was taken
|
||||
// are fetched from the provider afterwards rather than trusted from the client
|
||||
// (#1614). Detached: a slow or unreachable provider must not hold up the add.
|
||||
@@ -220,10 +234,15 @@ export class JourneyController {
|
||||
// reads each file at its final uploads/journey path.
|
||||
await this.commitJourneyUploads(files);
|
||||
const results: unknown[] = [];
|
||||
const refused: Express.Multer.File[] = [];
|
||||
for (const file of files) {
|
||||
const relativePath = `journey/${file.filename}`;
|
||||
const photo = this.journey.addPhoto(Number(entryId), user.id, relativePath, undefined, body?.caption as string | undefined);
|
||||
if (!photo) continue;
|
||||
if (!photo) {
|
||||
// No row points at this file, so nothing would ever clean it up.
|
||||
refused.push(file);
|
||||
continue;
|
||||
}
|
||||
// Mirror to Immich only when the user explicitly opted in (#730).
|
||||
if (this.journey.immichAutoUploadEnabled(user.id)) {
|
||||
try {
|
||||
@@ -238,6 +257,9 @@ export class JourneyController {
|
||||
}
|
||||
results.push(photo);
|
||||
}
|
||||
if (refused.length) {
|
||||
await this.discardJourneyUploads(refused);
|
||||
}
|
||||
if (!results.length) {
|
||||
throw new HttpException({ error: 'Not allowed' }, 403);
|
||||
}
|
||||
@@ -348,6 +370,7 @@ export class JourneyController {
|
||||
const filePaths = files.map((f) => ({ path: `journey/${f.filename}` }));
|
||||
const photos = this.journey.uploadGalleryPhotos(Number(id), user.id, filePaths);
|
||||
if (!photos.length) {
|
||||
await this.discardJourneyUploads(files);
|
||||
throw new HttpException({ error: 'Not allowed' }, 403);
|
||||
}
|
||||
// An uploaded file carries its own EXIF; reading it is what puts the photo on
|
||||
@@ -465,6 +488,7 @@ export class JourneyController {
|
||||
await this.commitJourneyUploads([file]);
|
||||
const result = this.journey.updateJourney(Number(id), user.id, { cover_image: `journey/${file.filename}` });
|
||||
if (!result) {
|
||||
await this.discardJourneyUploads([file]);
|
||||
throw new HttpException({ error: 'Journey not found' }, 404);
|
||||
}
|
||||
return result;
|
||||
@@ -679,6 +703,7 @@ export class JourneyController {
|
||||
share_timeline: body.share_timeline as boolean | undefined,
|
||||
share_gallery: body.share_gallery as boolean | undefined,
|
||||
share_map: body.share_map as boolean | undefined,
|
||||
newest_first: body.newest_first as boolean | undefined,
|
||||
});
|
||||
if (!result) {
|
||||
throw new HttpException({ error: 'Not allowed' }, 403);
|
||||
|
||||
@@ -408,28 +408,36 @@ export class JourneyMcp {
|
||||
|
||||
@Tool({
|
||||
name: 'get_journey_share_link',
|
||||
description: 'Get the current public share link for a journey. Returns null if none exists.',
|
||||
description: 'Get the current public share link for a journey. Owner only. Returns null if none exists.',
|
||||
inputSchema: { journeyId: z.number().int().positive() },
|
||||
annotations: TOOL_ANNOTATIONS_READONLY,
|
||||
when: journeyAddonOn,
|
||||
access: { group: 'journey', mode: 'share' },
|
||||
})
|
||||
getJourneyShareLink({ journeyId }: { journeyId: number }, ctx: McpContext) {
|
||||
if (!this.journey.canAccessJourney(journeyId, ctx.userId)) return notFound('Journey not found or access denied.');
|
||||
// Owner only, same as the REST route: handing out the token is handing out
|
||||
// the journey.
|
||||
if (!this.journey.isOwner(journeyId, ctx.userId)) return notFound('Journey not found or access denied.');
|
||||
return ok({ shareLink: this.share.getJourneyShareLink(journeyId) });
|
||||
}
|
||||
|
||||
@Tool({
|
||||
name: 'create_journey_share_link',
|
||||
description: 'Create or update the public share link for a journey. Owner only.',
|
||||
inputSchema: { journeyId: z.number().int().positive() },
|
||||
description: 'Create or update the public share link for a journey. Owner only. Flags left out keep their current value on an existing link; a new link defaults to timeline/gallery/map on.',
|
||||
inputSchema: {
|
||||
journeyId: z.number().int().positive(),
|
||||
share_timeline: z.boolean().optional(),
|
||||
share_gallery: z.boolean().optional(),
|
||||
share_map: z.boolean().optional(),
|
||||
newest_first: z.boolean().optional(),
|
||||
},
|
||||
annotations: TOOL_ANNOTATIONS_WRITE,
|
||||
when: journeyAddonOn,
|
||||
access: { group: 'journey', mode: 'share' },
|
||||
})
|
||||
createJourneyShareLink({ journeyId }: { journeyId: number }, ctx: McpContext) {
|
||||
createJourneyShareLink({ journeyId, ...permissions }: { journeyId: number; share_timeline?: boolean; share_gallery?: boolean; share_map?: boolean; newest_first?: boolean }, ctx: McpContext) {
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
const shareLink = this.share.createOrUpdateJourneyShareLink(journeyId, ctx.userId, {});
|
||||
const shareLink = this.share.createOrUpdateJourneyShareLink(journeyId, ctx.userId, permissions);
|
||||
if (!shareLink) return notFound('Journey not found or access denied.');
|
||||
return ok({ shareLink });
|
||||
}
|
||||
|
||||
@@ -78,11 +78,11 @@ export class JourneyService {
|
||||
removeContributor(id: number, userId: number, targetUserId: number) { return this.journey.removeContributor(id, userId, targetUserId); }
|
||||
|
||||
// Share links
|
||||
// Authorization: only someone with access to the journey may read its public
|
||||
// share token — same access model as create/delete here and the
|
||||
// get_journey_share_link MCP tool.
|
||||
// Authorization: the token is the whole credential, so reading it needs the
|
||||
// same owner check create/delete use. A viewer who reads it keeps a working
|
||||
// anonymous link after being removed as a contributor.
|
||||
getJourneyShareLink(id: number, userId: number) {
|
||||
if (!this.journey.canAccessJourney(id, userId)) return null;
|
||||
if (!this.journey.isOwner(id, userId)) return null;
|
||||
return this.share.getJourneyShareLink(id);
|
||||
}
|
||||
createOrUpdateJourneyShareLink(id: number, userId: number, data: Parameters<typeof this.share.createOrUpdateJourneyShareLink>[2]) { return this.share.createOrUpdateJourneyShareLink(id, userId, data); }
|
||||
|
||||
@@ -239,6 +239,13 @@ export class TripsController {
|
||||
if (!owner) {
|
||||
throw new HttpException({ error: 'Trip not found' }, 404);
|
||||
}
|
||||
// Someone with no access at all gets the same 404 as a trip that does not
|
||||
// exist, otherwise the 403 below turns sequential ids into an existence
|
||||
// oracle. Admins are exempt: they may delete trips they are not a member of,
|
||||
// which is what the isAdminDelete branch further down relies on.
|
||||
if (user.role !== 'admin' && !this.trips.canAccessTrip(id, user.id)) {
|
||||
throw new HttpException({ error: 'Trip not found' }, 404);
|
||||
}
|
||||
if (!this.trips.can('trip_delete', user.role, owner.user_id, user.id, owner.user_id !== user.id)) {
|
||||
throw new HttpException({ error: 'No permission to delete this trip' }, 403);
|
||||
}
|
||||
|
||||
@@ -317,6 +317,15 @@ describe('Journey e2e (real auth guard + temp SQLite)', () => {
|
||||
expect(res.body).toEqual({ id: 9 });
|
||||
});
|
||||
|
||||
it('public journey read still answers the addon gate when Journey is off', async () => {
|
||||
// The authenticated surface goes dark with the addon; a published journey
|
||||
// has to go with it, or turning the switch off does not take the feature away.
|
||||
isAddonEnabled.mockReturnValue(false);
|
||||
const res = await request(server).get('/api/public/journey/tok');
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Journey addon is not enabled' });
|
||||
});
|
||||
|
||||
it('the image fileFilter is wired: a disallowed extension is rejected, not stored', async () => {
|
||||
// The multer options are built by MulterModule.registerAsync now, and Nest
|
||||
// injects MULTER_MODULE_OPTIONS with @Optional(): a token that fails to
|
||||
|
||||
@@ -417,6 +417,48 @@ describe('Journey share link', () => {
|
||||
expect(get.body.link.share_map).toBe(false);
|
||||
});
|
||||
|
||||
it('JOURNEY-INT-044 — a contributor cannot read the token, only the owner can', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: helper } = createUser(testDb);
|
||||
const journey = createJourney(testDb, owner.id);
|
||||
addJourneyContributor(testDb, journey.id, helper.id, 'editor');
|
||||
await request(app)
|
||||
.post(`/api/journeys/${journey.id}/share-link`)
|
||||
.set('Cookie', authCookie(owner.id))
|
||||
.send({ share_timeline: true, share_gallery: true, share_map: true });
|
||||
|
||||
// The token is the whole credential and it outlives the contributor's
|
||||
// access, so reading it takes the same owner check create and delete take.
|
||||
const res = await request(app)
|
||||
.get(`/api/journeys/${journey.id}/share-link`)
|
||||
.set('Cookie', authCookie(helper.id));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.link).toBeNull();
|
||||
});
|
||||
|
||||
it('JOURNEY-INT-045 — a flag-only update leaves the other flags alone', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const journey = createJourney(testDb, user.id);
|
||||
await request(app)
|
||||
.post(`/api/journeys/${journey.id}/share-link`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({ share_timeline: true, share_gallery: false, share_map: false, newest_first: true });
|
||||
|
||||
await request(app)
|
||||
.post(`/api/journeys/${journey.id}/share-link`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({ share_timeline: false });
|
||||
|
||||
const get = await request(app)
|
||||
.get(`/api/journeys/${journey.id}/share-link`)
|
||||
.set('Cookie', authCookie(user.id));
|
||||
expect(get.body.link.share_timeline).toBe(false);
|
||||
expect(get.body.link.share_gallery).toBe(false);
|
||||
expect(get.body.link.share_map).toBe(false);
|
||||
expect(get.body.link.newest_first).toBe(true);
|
||||
});
|
||||
|
||||
it('JOURNEY-INT-017 — DELETE /api/journeys/:id/share-link deletes the share link', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const journey = createJourney(testDb, user.id);
|
||||
@@ -1082,7 +1124,7 @@ describe('Journey upload parity', () => {
|
||||
expect(res.body.photos.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('JOURNEY-P05 — non-contributor gallery upload is 403 and files remain on disk (no cleanup today)', async () => {
|
||||
it('JOURNEY-P05 — non-contributor gallery upload is 403 and the bytes are taken back off disk', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: other } = createUser(testDb);
|
||||
const journey = createJourney(testDb, owner.id);
|
||||
@@ -1094,8 +1136,10 @@ describe('Journey upload parity', () => {
|
||||
.attach('photos', FIXTURE_IMG);
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.error).toBe('Not allowed');
|
||||
// The commit runs before the permission answer is known, so the refusal has
|
||||
// to delete the object again. Nothing sweeps orphans.
|
||||
const after = fsMod.existsSync(journeyDir) ? fsMod.readdirSync(journeyDir).length : 0;
|
||||
expect(after).toBe(before + 1);
|
||||
expect(after).toBe(before);
|
||||
});
|
||||
|
||||
it('JOURNEY-P06 — video+poster upload: poster is ALWAYS stored as .jpg (stored-XSS pin)', async () => {
|
||||
|
||||
@@ -527,7 +527,7 @@ describe('Delete trip', () => {
|
||||
expect(getRes.status).toBe(404);
|
||||
});
|
||||
|
||||
it('TRIP-019 — Regular user cannot delete another users trip → 403', async () => {
|
||||
it('TRIP-019 — Regular user cannot delete another users trip → 404', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: other } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id, { title: "Owner's Trip" });
|
||||
@@ -536,8 +536,10 @@ describe('Delete trip', () => {
|
||||
.delete(`/api/trips/${trip.id}`)
|
||||
.set('Cookie', authCookie(other.id));
|
||||
|
||||
// getTripOwner finds the trip (it exists); checkPermission fails for non-members → 403
|
||||
expect(res.status).toBe(403);
|
||||
// 404, not 403: someone with no access at all must not be able to tell an
|
||||
// existing trip from a missing one by walking sequential ids. A member who
|
||||
// simply lacks trip_delete still gets 403 — see the case below.
|
||||
expect(res.status).toBe(404);
|
||||
|
||||
// Trip still exists
|
||||
const tripInDb = testDb.prepare('SELECT id FROM trips WHERE id = ?').get(trip.id);
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
*/
|
||||
import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { db, canAccessTrip } from '../../../src/db/database';
|
||||
import { CAN_ACCESS_TRIP_SQL, buildDbMock, createTestDb } from '../../helpers/test-db';
|
||||
|
||||
function seedUser(username: string): number {
|
||||
return Number(
|
||||
@@ -52,3 +53,34 @@ describe('canAccessTrip', () => {
|
||||
expect(canAccessTrip(tripId, stranger)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The mock every domain test runs against had the pre-#1543 SELECT, so the very
|
||||
* regression the block above pins was invisible to the suites that mock the db.
|
||||
*/
|
||||
describe('the buildDbMock stand-in for canAccessTrip', () => {
|
||||
it('hands back the trip currency, like the real one', () => {
|
||||
const testDb = createTestDb();
|
||||
try {
|
||||
const owner = Number(
|
||||
testDb.prepare("INSERT INTO users (username, email, password_hash, role) VALUES ('m', 'm@example.test', 'x', 'user')")
|
||||
.run().lastInsertRowid,
|
||||
);
|
||||
const tripId = Number(
|
||||
testDb.prepare("INSERT INTO trips (user_id, title, currency) VALUES (?, 'Trip', 'ISK')")
|
||||
.run(owner).lastInsertRowid,
|
||||
);
|
||||
|
||||
expect(buildDbMock(testDb).canAccessTrip(tripId, owner)).toMatchObject({ id: tripId, currency: 'ISK' });
|
||||
} finally {
|
||||
testDb.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('selects the same columns the production statement does', () => {
|
||||
const columns = (sql: string) => sql.replace(/\s+/g, ' ').trim();
|
||||
expect(columns(CAN_ACCESS_TRIP_SQL)).toBe(
|
||||
'SELECT t.id, t.user_id, t.currency FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -441,6 +441,46 @@ describe('journey share-link tools', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('passes the share flags through, and an omitted flag keeps its value', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const journey = await seedJourney(h);
|
||||
await h.client.callTool({
|
||||
name: 'create_journey_share_link',
|
||||
arguments: { journeyId: journey.id, share_gallery: false, newest_first: true },
|
||||
});
|
||||
// Calling it again without the flags must not re-publish the gallery.
|
||||
await h.client.callTool({ name: 'create_journey_share_link', arguments: { journeyId: journey.id } });
|
||||
const read = parseToolResult(await h.client.callTool({
|
||||
name: 'get_journey_share_link', arguments: { journeyId: journey.id },
|
||||
})) as any;
|
||||
expect(read.shareLink.share_gallery).toBe(false);
|
||||
expect(read.shareLink.newest_first).toBe(true);
|
||||
expect(read.shareLink.share_timeline).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('get_journey_share_link is owner-only, like create and delete', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: guest } = createUser(testDb);
|
||||
let journeyId = 0;
|
||||
await withHarness(owner.id, async (h) => {
|
||||
const journey = await seedJourney(h);
|
||||
journeyId = journey.id;
|
||||
await h.client.callTool({ name: 'create_journey_share_link', arguments: { journeyId } });
|
||||
});
|
||||
testDb.prepare(
|
||||
'INSERT INTO journey_contributors (journey_id, user_id, role, added_at) VALUES (?, ?, ?, ?)',
|
||||
).run(journeyId, guest.id, 'viewer', Date.now());
|
||||
await withHarness(guest.id, async (h) => {
|
||||
// The token is the whole credential: a contributor must not be able to
|
||||
// read out a link that keeps working after they are removed.
|
||||
expect((await h.client.callTool({
|
||||
name: 'get_journey_share_link', arguments: { journeyId },
|
||||
})).isError).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('every share tool refuses a foreign journey', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const { journeyId } = foreignJourney();
|
||||
|
||||
@@ -171,12 +171,22 @@ describe('FilesController (parity with the legacy /api/trips/:tripId/files route
|
||||
|
||||
it('DELETE /:id/link/:linkId removes the link; GET /:id/links lists', () => {
|
||||
const deleteFileLink = vi.fn();
|
||||
expect(fc(fsvc({ deleteFileLink } as Partial<FilesService>)).unlink(user, trip, '5', '9', '3')).toEqual({ success: true });
|
||||
expect(fc(fsvc({ getFileById: vi.fn().mockReturnValue({ id: 9 }), deleteFileLink } as Partial<FilesService>)).unlink(user, trip, '5', '9', '3')).toEqual({ success: true });
|
||||
expect(deleteFileLink).toHaveBeenCalledWith('3', '9');
|
||||
const s = fsvc({ getFileLinks: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial<FilesService>);
|
||||
const s = fsvc({ getFileById: vi.fn().mockReturnValue({ id: 9 }), getFileLinks: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial<FilesService>);
|
||||
expect(fc(s).links(user, trip, '5', '9')).toEqual({ links: [{ id: 1 }] });
|
||||
});
|
||||
|
||||
it('the link routes resolve the file against :tripId, so a foreign file is 404', () => {
|
||||
const foreign = () => fsvc({ getFileById: vi.fn().mockReturnValue(undefined), deleteFileLink: vi.fn(), getFileLinks: vi.fn() } as Partial<FilesService>);
|
||||
const unlinkSvc = foreign();
|
||||
expect(thrown(() => fc(unlinkSvc).unlink(user, trip, '5', '9', '3'))).toEqual({ status: 404, body: { error: 'File not found' } });
|
||||
expect(unlinkSvc.deleteFileLink).not.toHaveBeenCalled();
|
||||
const listSvc = foreign();
|
||||
expect(thrown(() => fc(listSvc).links(user, trip, '5', '9'))).toEqual({ status: 404, body: { error: 'File not found' } });
|
||||
expect(listSvc.getFileLinks).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('the trash + link routes all reject without file_delete / file_edit', async () => {
|
||||
const denied = () => fsvc({ can: vi.fn().mockReturnValue(false) });
|
||||
await expect(fc(denied()).permanent(user, trip, '5', '9')).rejects.toMatchObject({ status: 403 });
|
||||
|
||||
@@ -141,10 +141,14 @@ describe('files.constants', () => {
|
||||
});
|
||||
|
||||
it('FILE-SVC-003: blocklist, size caps and the default allowlist are pinned', () => {
|
||||
expect(BLOCKED_EXTENSIONS).toHaveLength(25);
|
||||
expect(BLOCKED_EXTENSIONS).toHaveLength(29);
|
||||
expect(BLOCKED_EXTENSIONS).toContain('.svg');
|
||||
expect(BLOCKED_EXTENSIONS).toContain('.exe');
|
||||
expect(BLOCKED_EXTENSIONS).toContain('.ps1');
|
||||
// The alternative spellings a browser still renders as a document.
|
||||
for (const ext of ['.svgz', '.shtml', '.shtm', '.xht']) {
|
||||
expect(BLOCKED_EXTENSIONS).toContain(ext);
|
||||
}
|
||||
expect(MAX_FILE_SIZE).toBe(50 * 1024 * 1024);
|
||||
expect(MAX_VIDEO_SIZE).toBe(500 * 1024 * 1024);
|
||||
expect(DEFAULT_ALLOWED_EXTENSIONS).toBe('jpg,jpeg,png,gif,webp,heic,pdf,doc,docx,xls,xlsx,txt,csv,pkpass,pkpasses,md,markdown');
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
/**
|
||||
* Storing TREK Studio books (#1973).
|
||||
*
|
||||
* The half worth testing hard is concurrency. Everyone on a journey can edit
|
||||
* The half worth testing hard is concurrency. Every editor on a journey edits
|
||||
* the same book, so "two saves arrive together" is the ordinary case rather
|
||||
* than an edge one, and the failure mode of getting it wrong is somebody's
|
||||
* afternoon disappearing with no error anywhere.
|
||||
@@ -103,6 +103,20 @@ describe('access', () => {
|
||||
expect(books.getBook(journey.id, helper.id)).not.toBeNull();
|
||||
});
|
||||
|
||||
it('lets a viewer read the book but not overwrite or delete it', () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: guest } = createUser(testDb);
|
||||
const journey = createJourney(testDb, owner.id);
|
||||
addJourneyContributor(testDb, journey.id, guest.id, 'viewer');
|
||||
books.saveBook(journey.id, owner.id, { title: 'Iceland', document: doc() });
|
||||
|
||||
expect(books.canOpen(journey.id, guest.id)).toBe(true);
|
||||
expect(books.getBook(journey.id, guest.id)).not.toBeNull();
|
||||
expect(books.saveBook(journey.id, guest.id, { title: 'mine now', document: doc('two') })).toBeNull();
|
||||
expect(books.deleteBook(journey.id, guest.id)).toBeNull();
|
||||
expect(books.getBook(journey.id, owner.id)!.title).toBe('Iceland');
|
||||
});
|
||||
|
||||
it('says nothing about a journey that does not exist', () => {
|
||||
const { user } = createUser(testDb);
|
||||
expect(books.canOpen(999_999, user.id)).toBe(false);
|
||||
|
||||
@@ -148,6 +148,41 @@ describe('createOrUpdateJourneyShareLink', () => {
|
||||
expect(link!.share_map).toBe(false);
|
||||
});
|
||||
|
||||
it('JOURNEY-SHARE-027: an update leaves out flags it was not given', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const journey = createJourney(testDb, user.id);
|
||||
|
||||
svc.createOrUpdateJourneyShareLink(journey.id, user.id, {
|
||||
share_timeline: true,
|
||||
share_gallery: false,
|
||||
share_map: false,
|
||||
newest_first: true,
|
||||
});
|
||||
// A caller that only flips the timeline must not silently re-publish the
|
||||
// gallery and map at the unchanged token.
|
||||
svc.createOrUpdateJourneyShareLink(journey.id, user.id, { share_timeline: false });
|
||||
|
||||
const link = svc.getJourneyShareLink(journey.id);
|
||||
expect(link!.share_timeline).toBe(false);
|
||||
expect(link!.share_gallery).toBe(false);
|
||||
expect(link!.share_map).toBe(false);
|
||||
expect(link!.newest_first).toBe(true);
|
||||
});
|
||||
|
||||
it('JOURNEY-SHARE-028: newest_first survives a flag-only update and is settable', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const journey = createJourney(testDb, user.id);
|
||||
|
||||
svc.createOrUpdateJourneyShareLink(journey.id, user.id, {});
|
||||
expect(svc.getJourneyShareLink(journey.id)!.newest_first).toBe(false);
|
||||
|
||||
svc.createOrUpdateJourneyShareLink(journey.id, user.id, { newest_first: true });
|
||||
expect(svc.getJourneyShareLink(journey.id)!.newest_first).toBe(true);
|
||||
|
||||
svc.createOrUpdateJourneyShareLink(journey.id, user.id, { share_gallery: false });
|
||||
expect(svc.getJourneyShareLink(journey.id)!.newest_first).toBe(true);
|
||||
});
|
||||
|
||||
it('JOURNEY-SHARE-004: different journeys get different tokens', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const j1 = createJourney(testDb, user.id);
|
||||
@@ -323,6 +358,21 @@ describe('validateShareTokenForAsset', () => {
|
||||
expect(svc.validateShareTokenForAsset(token, 'immich-asset-999')).toBeNull();
|
||||
});
|
||||
|
||||
it('JOURNEY-SHARE-029: falls back to the journey owner when the photo has no owner_id', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const journey = createJourney(testDb, user.id);
|
||||
const entry = createJourneyEntry(testDb, journey.id, user.id);
|
||||
insertJourneyPhoto(entry.id, { assetId: 'immich-asset-orphan' });
|
||||
const { token } = svc.createOrUpdateJourneyShareLink(journey.id, user.id, {});
|
||||
|
||||
// Without the fallback the controller filled the gap from the :ownerId path
|
||||
// segment, i.e. an anonymous caller picked whose provider credentials to try.
|
||||
const result = svc.validateShareTokenForAsset(token, 'immich-asset-orphan');
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.ownerId).toBe(user.id);
|
||||
});
|
||||
|
||||
it('JOURNEY-SHARE-015: denies (returns null) when the asset is not part of the shared journey', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const journey = createJourney(testDb, user.id);
|
||||
|
||||
@@ -6,6 +6,9 @@ import { JourneyController } from '../../../src/nest/journey/journey.controller'
|
||||
import { journeyThumbName } from '../../../src/nest/memories/thumbnail.service';
|
||||
import type { PhotoCaptureBackfillService } from '../../../src/nest/memories/photo-capture-backfill.service';
|
||||
import { JourneyPublicController } from '../../../src/nest/journey/journey-public.controller';
|
||||
import { AddonGuard } from '../../../src/nest/addons/addon.guard';
|
||||
import { REQUIRE_ADDON } from '../../../src/nest/addons/require-addon.decorator';
|
||||
import { ADDON_IDS } from '../../../src/addons';
|
||||
import type { JourneyService } from '../../../src/nest/journey/journey.service';
|
||||
import type { JourneyBookService } from '../../../src/nest/journey/journey-book.service';
|
||||
import type { StorageService } from '../../../src/nest/storage/storage.service';
|
||||
@@ -186,10 +189,48 @@ describe('JourneyController', () => {
|
||||
expect(thrown(() => ctl(svc({ updateJourneyPreferences: vi.fn().mockReturnValue(null) } as Partial<JourneyService>)).preferences(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } });
|
||||
expect(ctl(svc({ getJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial<JourneyService>)).getShareLink(user, '9')).toEqual({ link: { token: 'abc' } });
|
||||
expect(thrown(() => ctl(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue(null) } as Partial<JourneyService>)).setShareLink(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } });
|
||||
expect(ctl(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial<JourneyService>)).setShareLink(user, '9', { share_timeline: true })).toEqual({ token: 'abc' });
|
||||
const setLink = vi.fn().mockReturnValue({ token: 'abc' });
|
||||
expect(ctl(svc({ createOrUpdateJourneyShareLink: setLink } as Partial<JourneyService>)).setShareLink(user, '9', { share_timeline: true })).toEqual({ token: 'abc' });
|
||||
// newest_first is part of the shared contract and the public viewer reads
|
||||
// it, so it has to reach the service instead of being dropped here (#1614).
|
||||
expect(setLink).toHaveBeenCalledWith(9, 1, { share_timeline: true, share_gallery: undefined, share_map: undefined, newest_first: undefined });
|
||||
const withOrder = vi.fn().mockReturnValue({ token: 'abc' });
|
||||
ctl(svc({ createOrUpdateJourneyShareLink: withOrder } as Partial<JourneyService>)).setShareLink(user, '9', { newest_first: true });
|
||||
expect(withOrder).toHaveBeenCalledWith(9, 1, { share_timeline: undefined, share_gallery: undefined, share_map: undefined, newest_first: true });
|
||||
expect(thrown(() => ctl(svc({ deleteJourneyShareLink: vi.fn().mockReturnValue(false) } as Partial<JourneyService>)).deleteShareLink(user, '9'))).toEqual({ status: 403, body: { error: 'Not allowed' } });
|
||||
});
|
||||
|
||||
it('takes the committed bytes back out when the upload is refused', async () => {
|
||||
// The commit happens before the authorization result is known (the Immich
|
||||
// mirror reads the final path), so every refusal after it has to delete the
|
||||
// object again. Nothing sweeps orphans.
|
||||
const file = { filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File;
|
||||
await thrownAsync(() => ctl(svc({ addPhoto: vi.fn().mockReturnValue(null) } as Partial<JourneyService>)).uploadEntryPhotos(user, '3', [file], {}));
|
||||
expect(storageStub.delete).toHaveBeenCalledWith('journey', 'a.jpg');
|
||||
|
||||
vi.clearAllMocks();
|
||||
await thrownAsync(() => ctl(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([]) } as Partial<JourneyService>)).uploadGalleryPhotos(user, '3', [{ filename: 'g.jpg' } as Express.Multer.File]));
|
||||
expect(storageStub.delete).toHaveBeenCalledWith('journey', 'g.jpg');
|
||||
|
||||
vi.clearAllMocks();
|
||||
await thrownAsync(() => ctl(svc({ updateJourney: vi.fn().mockReturnValue(null) } as Partial<JourneyService>)).cover(user, '9', { filename: 'c.jpg' } as Express.Multer.File));
|
||||
expect(storageStub.delete).toHaveBeenCalledWith('journey', 'c.jpg');
|
||||
});
|
||||
|
||||
it('drops the files of a partly refused entry batch, keeping the accepted ones', async () => {
|
||||
// A 200 with fewer photos than files still leaves unreferenced objects behind.
|
||||
const addPhoto = vi.fn().mockReturnValueOnce({ id: 5 }).mockReturnValueOnce(null);
|
||||
const immichAutoUploadEnabled = vi.fn().mockReturnValue(false);
|
||||
const files = [
|
||||
{ filename: 'ok.jpg', originalname: 'ok.jpg' } as Express.Multer.File,
|
||||
{ filename: 'no.jpg', originalname: 'no.jpg' } as Express.Multer.File,
|
||||
];
|
||||
const res = await ctl(svc({ addPhoto, immichAutoUploadEnabled } as Partial<JourneyService>)).uploadEntryPhotos(user, '3', files, {});
|
||||
expect(res).toEqual({ photos: [{ id: 5 }] });
|
||||
expect(storageStub.delete).toHaveBeenCalledWith('journey', 'no.jpg');
|
||||
expect(storageStub.delete).not.toHaveBeenCalledWith('journey', 'ok.jpg');
|
||||
});
|
||||
|
||||
it('entry photo upload mirrors to Immich only when opted in', async () => {
|
||||
const addPhoto = vi.fn().mockReturnValue({ id: 5 });
|
||||
const uploadToImmich = vi.fn().mockResolvedValue('immich-1');
|
||||
@@ -376,6 +417,13 @@ describe('JourneyController', () => {
|
||||
});
|
||||
|
||||
describe('JourneyPublicController', () => {
|
||||
it('is addon-gated like the authenticated surface', () => {
|
||||
// Turning the Journey addon off is meant to take the feature away; without
|
||||
// this, already-published journeys stayed publicly readable.
|
||||
expect(Reflect.getMetadata(REQUIRE_ADDON, JourneyPublicController)).toEqual({ addonId: ADDON_IDS.JOURNEY, label: 'Journey' });
|
||||
expect((Reflect.getMetadata('__guards__', JourneyPublicController) ?? [])[0]).toBe(AddonGuard);
|
||||
});
|
||||
|
||||
it('GET /:token 404 / json', () => {
|
||||
expect(thrown(() => new JourneyPublicController(svc({ getPublicJourney: vi.fn().mockReturnValue(null) } as Partial<JourneyService>), storageStub).get('tok'))).toEqual({ status: 404, body: { error: 'Not found' } });
|
||||
expect(new JourneyPublicController(svc({ getPublicJourney: vi.fn().mockReturnValue({ id: 1 }) } as Partial<JourneyService>), storageStub).get('tok')).toEqual({ id: 1 });
|
||||
@@ -435,11 +483,14 @@ describe('JourneyPublicController', () => {
|
||||
expect(json).toHaveBeenCalledWith({ error: 'Provider not supported' });
|
||||
});
|
||||
|
||||
it('legacy photo proxy: falls back to the path ownerId when the token has none', async () => {
|
||||
it('legacy photo proxy: ignores the path ownerId, taking the owner off the token', async () => {
|
||||
// Whose provider credentials get tried is not something an anonymous caller
|
||||
// may choose from the URL. The share service resolves it, falling back to
|
||||
// the journey owner when the photo row carries no owner_id.
|
||||
const streamProviderAsset = vi.fn().mockResolvedValue(undefined);
|
||||
const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 0 }), streamProviderAsset } as Partial<JourneyService>);
|
||||
const s = svc({ validateShareTokenForAsset: vi.fn().mockReturnValue({ ownerId: 3 }), streamProviderAsset } as Partial<JourneyService>);
|
||||
await new JourneyPublicController(s, storageStub).legacyPhoto('tok', 'immich', 'a1', '8', 'original', {} as Response);
|
||||
expect(streamProviderAsset).toHaveBeenCalledWith({}, 'immich', { userId: 8, ownerId: 8, assetId: 'a1' }, 'original');
|
||||
expect(streamProviderAsset).toHaveBeenCalledWith({}, 'immich', { userId: 3, ownerId: 3, assetId: 'a1' }, 'original');
|
||||
});
|
||||
|
||||
it('legacy photo proxy: local provider 404s when the object does not exist', async () => {
|
||||
|
||||
@@ -280,6 +280,20 @@ describe('TripsController (parity with the legacy /api/trips route)', () => {
|
||||
expect(thrown(() => tc(s).remove(user, '9', req))).toEqual({ status: 403, body: { error: 'No permission to delete this trip' } });
|
||||
});
|
||||
|
||||
it('404s for someone with no access, so the 403 is not an existence oracle', () => {
|
||||
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 2 }), canAccessTrip: vi.fn().mockReturnValue(null), remove: vi.fn() } as Partial<TripsService>);
|
||||
expect(thrown(() => tc(s).remove(user, '9', req))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
expect(s.remove).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still lets an admin delete a trip they are not a member of', () => {
|
||||
const admin = { id: 1, role: 'admin', email: 'a@example.test' } as User;
|
||||
const remove = vi.fn().mockReturnValue({ tripId: 9, title: 'T', isAdminDelete: true, ownerEmail: 'owner@x.y' });
|
||||
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 2 }), canAccessTrip: vi.fn().mockReturnValue(null), remove, broadcast: vi.fn() } as Partial<TripsService>);
|
||||
expect(tc(s).remove(admin, '9', req)).toEqual({ success: true });
|
||||
expect(remove).toHaveBeenCalledWith('9', 1, 'admin');
|
||||
});
|
||||
|
||||
it('deletes, audits and broadcasts', () => {
|
||||
const remove = vi.fn().mockReturnValue({ tripId: 9, title: 'T', isAdminDelete: false }); const broadcast = vi.fn();
|
||||
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 1 }), remove, broadcast } as Partial<TripsService>);
|
||||
|
||||
Reference in New Issue
Block a user