diff --git a/server/src/nest/public-api/public-api.controller.ts b/server/src/nest/public-api/public-api.controller.ts index 8f670c4cf..7c7700ee6 100644 --- a/server/src/nest/public-api/public-api.controller.ts +++ b/server/src/nest/public-api/public-api.controller.ts @@ -9,6 +9,8 @@ import { } from '@trek/shared'; import { ApiTokenGuard } from './api-token.guard'; import { PublicApiService } from './public-api.service'; +import { RateLimitService } from '../common/rate-limit.service'; +import type { User } from '../../types'; /** * `/api/v1` — the versioned, read-only surface for third-party integrations. @@ -24,15 +26,40 @@ import { PublicApiService } from './public-api.service'; * missing feature: it would need scope enforcement this surface does not implement * yet, and a token that reads everything is a very different thing from one that * can also delete a trip. + * + * Rate limited per token rather than per IP: a self-hosted integration and its + * user's browser routinely share an address, and limiting by IP would let one + * starve the other. The budget is generous — this is a sync surface, not a login + * form — but bounded, so a runaway poll degrades its own integration instead of + * the instance. */ +const RATE_WINDOW_MS = 60_000; +const RATE_MAX_PER_MINUTE = 120; + @Controller('api/v1') @UseGuards(ApiTokenGuard) export class PublicApiController { - constructor(private readonly api: PublicApiService) {} + constructor( + private readonly api: PublicApiService, + private readonly rl: RateLimitService, + ) {} + + /** + * Keyed by token id, not by IP. Falls back to the user id when a token somehow + * carries none, so the limiter can never end up with a shared bucket. + */ + private limit(req: Request): void { + const user = req.user as User | undefined; + const key = `user:${user?.id ?? 'unknown'}`; + if (!this.rl.check('public-api', key, RATE_MAX_PER_MINUTE, RATE_WINDOW_MS, Date.now())) { + throw new HttpException({ error: 'Too many requests. Please slow down.' }, 429); + } + } /** Every trip the token's owner can reach, without itineraries. */ @Get('trips') listTrips(@Req() req: Request): PublicApiTripList { + this.limit(req); return { trips: this.api.listTrips(requireUserId(req)) }; } @@ -49,6 +76,7 @@ export class PublicApiController { @Param('id') id: string, @Query('include') include?: string, ): PublicApiTrip { + this.limit(req); const tripId = parseTripId(id); const trip = this.api.getTrip(tripId, requireUserId(req), parseInclude(include)); if (!trip) { diff --git a/server/src/nest/public-api/public-api.module.ts b/server/src/nest/public-api/public-api.module.ts index e6fab65be..586be8ef8 100644 --- a/server/src/nest/public-api/public-api.module.ts +++ b/server/src/nest/public-api/public-api.module.ts @@ -4,6 +4,7 @@ import { PublicApiService } from './public-api.service'; import { ApiTokenGuard } from './api-token.guard'; import { TokensModule } from '../tokens/tokens.module'; import { TripMembershipModule } from '../trip-membership/trip-membership.module'; +import { RateLimitModule } from '../common/rate-limit.module'; /** * Public API v1 — the versioned read-only surface for third-party integrations. @@ -14,7 +15,7 @@ import { TripMembershipModule } from '../trip-membership/trip-membership.module' * this trip" is decided. */ @Module({ - imports: [TokensModule, TripMembershipModule], + imports: [TokensModule, TripMembershipModule, RateLimitModule], controllers: [PublicApiController], providers: [PublicApiService, ApiTokenGuard], }) diff --git a/server/src/nest/public-api/public-api.service.ts b/server/src/nest/public-api/public-api.service.ts index 597cca2a4..fda24ca25 100644 --- a/server/src/nest/public-api/public-api.service.ts +++ b/server/src/nest/public-api/public-api.service.ts @@ -43,7 +43,7 @@ export class PublicApiService { const ids = this.membership.listAccessibleTripIds(userId); if (ids.length === 0) return []; const rows = this.db.all( - `SELECT id, title, description, start_date, end_date, currency, is_archived + `SELECT id, title, description, start_date, end_date, currency, is_archived, updated_at FROM trips WHERE id IN (${ids.map(() => '?').join(',')}) ORDER BY start_date DESC, id DESC`, @@ -62,7 +62,7 @@ export class PublicApiService { getTrip(tripId: number, userId: number, include: PublicApiInclude[]): 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 + `SELECT id, title, description, start_date, end_date, currency, is_archived, updated_at FROM trips WHERE id = ?`, tripId, ); @@ -227,6 +227,7 @@ function toTripSummary(row: TripRow): PublicApiTripSummary { end_date: row.end_date ?? null, currency: row.currency ?? null, archived: row.is_archived === 1, + updated_at: row.updated_at ?? null, }; } @@ -253,6 +254,7 @@ interface TripRow { end_date: string | null; currency: string | null; is_archived: number | null; + updated_at: string | null; } interface DayRow { diff --git a/server/tests/e2e/public-api.e2e.test.ts b/server/tests/e2e/public-api.e2e.test.ts index 89a7e6df3..09c2296f8 100644 --- a/server/tests/e2e/public-api.e2e.test.ts +++ b/server/tests/e2e/public-api.e2e.test.ts @@ -26,7 +26,8 @@ const { db } = vi.hoisted(() => { created_at DATETIME DEFAULT CURRENT_TIMESTAMP, last_used_at DATETIME); CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL, title TEXT NOT NULL, description TEXT, start_date TEXT, end_date TEXT, currency TEXT, - is_archived INTEGER DEFAULT 0, created_at DATETIME DEFAULT CURRENT_TIMESTAMP); + is_archived INTEGER DEFAULT 0, created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME DEFAULT CURRENT_TIMESTAMP); CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL); CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, day_number INTEGER NOT NULL, date TEXT NOT NULL, notes TEXT, title TEXT); @@ -68,6 +69,7 @@ vi.mock('../../src/db/database', async (importActual) => { }); import { DatabaseModule } from '../../src/nest/database/database.module'; +import { RateLimitModule } from '../../src/nest/common/rate-limit.module'; import { PublicApiModule } from '../../src/nest/public-api/public-api.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; @@ -90,7 +92,7 @@ describe('Public API v1 e2e (real guard + real SQL)', () => { let app: Awaited>; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, PublicApiModule] }).compile(); + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, RateLimitModule, PublicApiModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.useGlobalFilters(new TrekExceptionFilter()); await nest.init(); diff --git a/server/tests/unit/nest/public-api.controller.test.ts b/server/tests/unit/nest/public-api.controller.test.ts index a94a5429d..1a80730df 100644 --- a/server/tests/unit/nest/public-api.controller.test.ts +++ b/server/tests/unit/nest/public-api.controller.test.ts @@ -12,6 +12,7 @@ import type { Request } from 'express'; import { PUBLIC_API_INCLUDES } from '@trek/shared'; import { PublicApiController } from '../../../src/nest/public-api/public-api.controller'; import type { PublicApiService } from '../../../src/nest/public-api/public-api.service'; +import type { RateLimitService } from '../../../src/nest/common/rate-limit.service'; const TRIP = { id: 12, @@ -21,10 +22,13 @@ const TRIP = { end_date: '2026-06-22', currency: 'EUR', archived: false, + updated_at: '2026-06-01 10:00:00', }; -function makeController(svc: Partial) { - return new PublicApiController(svc as PublicApiService); +/** The limiter always allows unless a test says otherwise. */ +function makeController(svc: Partial, allow = true) { + const rl = { check: vi.fn().mockReturnValue(allow) } as unknown as RateLimitService; + return new PublicApiController(svc as PublicApiService, rl); } const req = (userId = 7) => ({ user: { id: userId } }) as Request; @@ -125,4 +129,19 @@ describe('PublicApiController', () => { expect(getTrip).not.toHaveBeenCalled(); }); }); + + describe('rate limiting', () => { + it('429s the list once the budget is spent, without reaching the service', () => { + const listTrips = vi.fn(); + const res = thrown(() => makeController({ listTrips }, false).listTrips(req(7))); + expect(res).toEqual({ status: 429, body: { error: 'Too many requests. Please slow down.' } }); + expect(listTrips).not.toHaveBeenCalled(); + }); + + it('429s the detail route the same way', () => { + const getTrip = vi.fn(); + expect(thrown(() => makeController({ getTrip }, false).getTrip(req(7), '12', undefined)).status).toBe(429); + expect(getTrip).not.toHaveBeenCalled(); + }); + }); }); diff --git a/shared/src/public-api/public-api.schema.ts b/shared/src/public-api/public-api.schema.ts index 4a49dd008..99ab8dd0c 100644 --- a/shared/src/public-api/public-api.schema.ts +++ b/shared/src/public-api/public-api.schema.ts @@ -101,6 +101,15 @@ export const publicApiTripSummarySchema = z.object({ end_date: z.string().nullable(), currency: z.string().nullable(), archived: z.boolean(), + /** + * When the trip's own fields last changed — title, dates, currency, cover. + * + * Explicitly NOT a content timestamp: editing a place, a day or a note does not + * touch it, because the child tables carry no `updated_at` of their own. A client + * that polls must compare the payload it received, not this value; using it to + * skip a fetch would silently miss every itinerary edit. + */ + updated_at: z.string().nullable(), }); export type PublicApiTripSummary = z.infer;