diff --git a/server/src/nest/public-api/public-api.controller.ts b/server/src/nest/public-api/public-api.controller.ts index 48775b9af..c0a7db22e 100644 --- a/server/src/nest/public-api/public-api.controller.ts +++ b/server/src/nest/public-api/public-api.controller.ts @@ -12,6 +12,7 @@ import { ApiTokenGuard } from './api-token.guard'; import { PublicApiService } from './public-api.service'; import { enforcePublicApiRateLimit, + grantedScopes, narrowToGrant, requireScope, requireUserId, @@ -98,7 +99,10 @@ export class PublicApiController { } const allowed = narrowToGrant(asked, req); - const trip = this.api.getTrip(tripId, requireUserId(req), allowed); + // The grant goes in as well as the narrowed include: days are implied by any + // section that hangs off them, and the implied block must still be measured + // against what the key may read. + const trip = this.api.getTrip(tripId, requireUserId(req), allowed, grantedScopes(req)); if (!trip) { throw new HttpException({ error: 'Trip not found' }, 404); } diff --git a/server/src/nest/public-api/public-api.service.ts b/server/src/nest/public-api/public-api.service.ts index 7af29291f..6281926d5 100644 --- a/server/src/nest/public-api/public-api.service.ts +++ b/server/src/nest/public-api/public-api.service.ts @@ -61,7 +61,7 @@ export class PublicApiService { * turns both into the same 404, so the endpoint cannot be used to probe which * trip ids exist. */ - getTrip(tripId: number, userId: number, include: PublicApiInclude[]): PublicApiTrip | null { + getTrip(tripId: number, userId: number, include: PublicApiInclude[], granted: readonly string[] = include): PublicApiTrip | null { if (!this.db.canAccessTrip(tripId, userId)) return null; const row = this.db.get( `SELECT id, title, description, start_date, end_date, currency, is_archived, updated_at @@ -74,8 +74,13 @@ export class PublicApiService { // Places, notes and reservations hang off days, so asking for one of them // and not for `days` used to return the trip and nothing else — silently, // which is the worst way to answer. Days are implied instead. + // + // Implied, not granted: `granted` is what the key may read at all, and a key + // narrowed to places must not get the day's title and free-text notes back + // through the container those places arrive in. The shell it does get is the + // join key the children are useless without. if (DAY_SCOPED.some((section) => include.includes(section))) { - trip.days = this.buildDays(tripId, include); + trip.days = this.buildDays(tripId, include, granted.includes('days')); } if (include.includes('places')) { trip.unplanned_places = this.buildUnplannedPlaces(tripId); @@ -100,7 +105,7 @@ export class PublicApiService { * rather than queried per day — a two-week trip would otherwise cost 42 round * trips for the same rows. */ - private buildDays(tripId: number, include: PublicApiInclude[]): PublicApiDay[] { + private buildDays(tripId: number, include: PublicApiInclude[], dayFields: boolean): PublicApiDay[] { const days = this.db.all( `SELECT id, day_number, date, title, notes FROM days WHERE trip_id = ? ORDER BY day_number ASC`, @@ -117,8 +122,8 @@ export class PublicApiService { return days.map((day) => ({ date: day.date, day_number: day.day_number, - title: day.title ?? null, - notes: day.notes ?? null, + title: dayFields ? day.title ?? null : null, + notes: dayFields ? day.notes ?? null : null, places: placesByDay.get(day.id) ?? [], day_notes: notesByDay.get(day.id) ?? [], reservations: reservationsByDay.get(day.id) ?? [], diff --git a/server/tests/e2e/public-api.e2e.test.ts b/server/tests/e2e/public-api.e2e.test.ts index b7321bd2e..e8d7de6e3 100644 --- a/server/tests/e2e/public-api.e2e.test.ts +++ b/server/tests/e2e/public-api.e2e.test.ts @@ -126,6 +126,8 @@ const MCP_TOKEN = 'trek_' + 'c'.repeat(48); /** Narrowed keys, all Ada's, so only the grant differs between them. */ const TRIPS_ONLY_TOKEN = 'trek_' + 'd'.repeat(48); const TRIPS_DAYS_TOKEN = 'trek_' + 'e'.repeat(48); +/** Places without days: the grant that used to get the day spine anyway. */ +const TRIPS_PLACES_TOKEN = 'trek_' + '1'.repeat(48); /** api_scopes that cannot be parsed — the fall-back-to-everything case. */ const BROKEN_SCOPES_TOKEN = 'trek_' + 'f'.repeat(48); @@ -178,6 +180,7 @@ describe('Public API v1 e2e (real guard + real SQL)', () => { // it, so if narrowing ever leaks into the default those tests fail first. seedLimitedToken(1, TRIPS_ONLY_TOKEN, JSON.stringify(['trips'])); seedLimitedToken(1, TRIPS_DAYS_TOKEN, JSON.stringify(['trips', 'days', 'notes'])); + seedLimitedToken(1, TRIPS_PLACES_TOKEN, JSON.stringify(['trips', 'places'])); seedLimitedToken(1, BROKEN_SCOPES_TOKEN, '{"not":"a list"'); // Ada owns trip 1; Bob owns trip 2; trip 3 is Bob's but Ada is a member. @@ -188,7 +191,7 @@ describe('Public API v1 e2e (real guard + real SQL)', () => { db.prepare("INSERT INTO trips (id, user_id, title, start_date) VALUES (3, 2, 'Shared', '2026-08-01')").run(); db.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (3, 1)').run(); - db.prepare("INSERT INTO days (id, trip_id, day_number, date, title) VALUES (1, 1, 1, '2026-06-14', 'Ankunft')").run(); + db.prepare("INSERT INTO days (id, trip_id, day_number, date, title, notes) VALUES (1, 1, 1, '2026-06-14', 'Ankunft', 'Schlüssel beim Nachbarn abholen')").run(); db.prepare("INSERT INTO days (id, trip_id, day_number, date) VALUES (2, 1, 2, '2026-06-15')").run(); db.prepare("INSERT INTO categories (id, name) VALUES (1, 'Museum')").run(); db.prepare( @@ -553,6 +556,35 @@ describe('Public API v1 e2e (real guard + real SQL)', () => { expect(JSON.stringify(res.body)).not.toContain('Uffizien'); }); + it('PUBAPI-SCOPE-008b: a grant without `days` gets the day shell, not the day', async () => { + // Places hang off days, so the day container is implied and has to be. It is + // the join key, though, not a way back to the section the key was narrowed to + // exclude: the day's title and its own free-text notes stay behind. + const res = await get('/api/v1/trips/1?include=places', TRIPS_PLACES_TOKEN); + expect(res.status).toBe(200); + expect(res.body.days).toHaveLength(2); + expect(res.body.days[0]).toMatchObject({ date: '2026-06-14', title: null, notes: null }); + expect(res.body.days[0].places.map((pl: { name: string }) => pl.name)).toEqual(['Uffizien', 'Ponte Vecchio']); + const serialised = JSON.stringify(res.body); + expect(serialised).not.toContain('Ankunft'); + expect(serialised).not.toContain('Nachbarn'); + }); + + it('PUBAPI-SCOPE-008c: the same key with no include at all is narrowed the same way', async () => { + // The route that made it a bypass rather than an oddity: `include` absent + // means every section, and narrowToGrant reduces it to ['places'], which + // implies days all over again. + const res = await get('/api/v1/trips/1', TRIPS_PLACES_TOKEN); + expect(res.status).toBe(200); + expect(res.body.days[0]).toMatchObject({ title: null, notes: null }); + expect(JSON.stringify(res.body)).not.toContain('Nachbarn'); + }); + + it('PUBAPI-SCOPE-008d: a key that was granted days still reads both fields', async () => { + const res = await get('/api/v1/trips/1', TRIPS_DAYS_TOKEN); + expect(res.body.days[0]).toMatchObject({ title: 'Ankunft', notes: 'Schlüssel beim Nachbarn abholen' }); + }); + it('PUBAPI-SCOPE-009: an include the key does cover still answers', async () => { const res = await get('/api/v1/trips/1?include=days,notes', TRIPS_DAYS_TOKEN); expect(res.status).toBe(200); diff --git a/server/tests/unit/nest/public-api-scopes.test.ts b/server/tests/unit/nest/public-api-scopes.test.ts index 5e4746e96..8074c3dee 100644 --- a/server/tests/unit/nest/public-api-scopes.test.ts +++ b/server/tests/unit/nest/public-api-scopes.test.ts @@ -273,7 +273,7 @@ describe('PublicApiController — what a narrowed key reaches', () => { expect(ctl.listTrips(req(ALL))).toEqual({ trips: [TRIP] }); expect(ctl.listBucketList(req(ALL))).toEqual({ items: [] }); expect(ctl.getTrip(req(ALL), '12', undefined)).toEqual(TRIP); - expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES]); + expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES], [...PUBLIC_API_SCOPES]); }); it('PUBAPI-SCOPE-U021: a trips-only key reads the trip list', () => { @@ -305,20 +305,21 @@ describe('PublicApiController — what a narrowed key reaches', () => { const getTrip = vi.fn().mockReturnValue(TRIP); apiController({ getTrip }).getTrip(req(limited('trips', 'days', 'notes')), '12', undefined); // `include` absent means "everything"; refusing a key for wanting sections it - // never named would make a narrow key unable to read a trip at all. - expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes']); + // never named would make a narrow key unable to read a trip at all. The grant + // rides along so the service can tell an implied day block from a granted one. + expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes'], ['trips', 'days', 'notes']); }); it('PUBAPI-SCOPE-U025: a trips-only key gets the summary and nothing that hangs off it', () => { const getTrip = vi.fn().mockReturnValue(TRIP); apiController({ getTrip }).getTrip(req(limited('trips')), '12', undefined); - expect(getTrip).toHaveBeenCalledWith(12, 7, []); + expect(getTrip).toHaveBeenCalledWith(12, 7, [], ['trips']); }); it('PUBAPI-SCOPE-U026: an empty include is treated like an absent one, and narrowed too', () => { const getTrip = vi.fn().mockReturnValue(TRIP); apiController({ getTrip }).getTrip(req(limited('trips', 'days')), '12', ' '); - expect(getTrip).toHaveBeenCalledWith(12, 7, ['days']); + expect(getTrip).toHaveBeenCalledWith(12, 7, ['days'], ['trips', 'days']); }); it('PUBAPI-SCOPE-U027: a section named explicitly and not granted is a 403, not a silent drop', () => { @@ -340,7 +341,7 @@ describe('PublicApiController — what a narrowed key reaches', () => { it('PUBAPI-SCOPE-U028: an include the key covers is passed through untouched', () => { const getTrip = vi.fn().mockReturnValue(TRIP); apiController({ getTrip }).getTrip(req(limited('trips', 'days', 'notes')), '12', 'days, notes'); - expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes']); + expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes'], ['trips', 'days', 'notes']); }); it('PUBAPI-SCOPE-U029: a bad id is still a 400 — the scope check does not swallow it', () => { diff --git a/server/tests/unit/nest/public-api.controller.test.ts b/server/tests/unit/nest/public-api.controller.test.ts index afa1cc6ce..e3cd60040 100644 --- a/server/tests/unit/nest/public-api.controller.test.ts +++ b/server/tests/unit/nest/public-api.controller.test.ts @@ -112,19 +112,19 @@ describe('PublicApiController', () => { const getTrip = vi.fn().mockReturnValue(TRIP); const res = makeController({ getTrip }).getTrip(req(7), '12', undefined); expect(res).toEqual(TRIP); - expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES]); + expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES], expect.arrayContaining(['days'])); }); it('treats an empty include the same as an absent one', () => { const getTrip = vi.fn().mockReturnValue(TRIP); makeController({ getTrip }).getTrip(req(7), '12', ' '); - expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES]); + expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES], expect.arrayContaining(['days'])); }); it('narrows to the requested sections and tolerates spacing', () => { const getTrip = vi.fn().mockReturnValue(TRIP); makeController({ getTrip }).getTrip(req(7), '12', 'days, notes'); - expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes']); + expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes'], expect.arrayContaining(['days'])); }); /**