mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 05:26:47 +00:00
b3a085df3f
The process-state ports were meant to let a store shared between processes replace the in-memory one, but they could not. The readers outside the container (the broadcast functions, the backup restore's permissions-cache flush, the pending-code sweep, hand-built consumers) went straight to the module-level in-memory instances, so an overridden provider was used by the gateway and the services while the broadcasts and the restore flush kept using the old store. And every port was synchronous, while a store in the database or on the network is async, so plugging one in would have changed every signature anyway. RateLimitStore, PendingCodeStore, OidcFlowStore and PermissionsCacheStore now return promises, and RateLimitService, OauthService, OidcService, PermissionsService and their callers await them. The in-memory stores still decide synchronously, so who gets a 429, a code or a cached level is unchanged. RoomRegistry stays synchronous: membership holds live sockets and broadcast() is a synchronous call every domain makes, so several processes need a fan-out in front of broadcast(), not a different place for the sets. The process-wide ports get a ProcessStoreSlot. PermissionsModule, OauthModule and RealtimeGatewayModule install the store the container resolved and release it on shutdown, and the free functions, the sweep and hand-built consumers read the slot, so overriding the provider changes every reader. Tests provide fake stores and check that the broadcasts, the restore flush and the sweep use them. Two callers needed more than an await. The transit MCP tools counted in a RateLimitService of their own that no provider could reach; they now take the one RateLimitModule provides (their mcp_transit_* buckets never overlap the REST ones). The link preview claimed its in-flight slot only after the budget check, which now yields, so simultaneous askers would each have been charged; it now claims the slot first, and an asker that joined someone out of budget asks again on its own budget. The unused putPendingCode/takePendingCode helpers are gone.
186 lines
7.6 KiB
TypeScript
186 lines
7.6 KiB
TypeScript
/**
|
||
* PublicApiController — request shaping for /api/v1.
|
||
*
|
||
* The controller owns three decisions and the tests below pin all three: how a
|
||
* trip id is parsed, how `include` is validated, and what a caller is told about a
|
||
* trip they may not read (nothing that distinguishes it from one that does not
|
||
* exist).
|
||
*/
|
||
import { describe, it, expect, vi } from 'vitest';
|
||
import { HttpException } from '@nestjs/common';
|
||
import type { Request } from 'express';
|
||
import { PUBLIC_API_INCLUDES, PUBLIC_API_SCOPES, type PublicApiGrant } 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,
|
||
title: 'Toskana',
|
||
description: null,
|
||
start_date: '2026-06-14',
|
||
end_date: '2026-06-22',
|
||
currency: 'EUR',
|
||
archived: false,
|
||
updated_at: '2026-06-01 10:00:00',
|
||
};
|
||
|
||
const ITEM = {
|
||
name: 'Hokkaido',
|
||
lat: 43.06,
|
||
lng: 141.35,
|
||
country_code: 'JP',
|
||
notes: null,
|
||
target_date: null,
|
||
};
|
||
|
||
/** The limiter always allows unless a test says otherwise. */
|
||
function makeController(svc: Partial<PublicApiService>, allow = true) {
|
||
const rl = { check: vi.fn().mockResolvedValue(allow) } as unknown as RateLimitService;
|
||
return new PublicApiController(svc as PublicApiService, rl);
|
||
}
|
||
|
||
/**
|
||
* The guard leaves two things behind (#2279): who is calling, and what their key
|
||
* may read. Every request here carries the full grant, because these cases are
|
||
* about shaping; the narrowed ones live in public-api-scopes.test.ts.
|
||
*/
|
||
const FULL_GRANT: PublicApiGrant = { mode: 'all', scopes: [...PUBLIC_API_SCOPES] };
|
||
const req = (userId = 7) => ({ user: { id: userId }, apiToken: FULL_GRANT }) as Request;
|
||
/** A request the guard left no user on — the shape the controller must refuse. */
|
||
const reqWithoutUser = () => ({ apiToken: FULL_GRANT }) as Request;
|
||
|
||
async function thrownAsync(fn: () => Promise<unknown>): Promise<{ status: number; body: unknown }> {
|
||
try {
|
||
await fn();
|
||
} catch (err) {
|
||
expect(err).toBeInstanceOf(HttpException);
|
||
const e = err as HttpException;
|
||
return { status: e.getStatus(), body: e.getResponse() };
|
||
}
|
||
throw new Error('expected the handler to throw');
|
||
}
|
||
|
||
describe('PublicApiController', () => {
|
||
describe('GET /api/v1/bucket-list', () => {
|
||
it('returns the caller’s wishlist and passes the id from the guard, never the query', async () => {
|
||
const listBucketList = vi.fn().mockReturnValue([ITEM]);
|
||
expect(await makeController({ listBucketList }).listBucketList(req(7))).toEqual({ items: [ITEM] });
|
||
expect(listBucketList).toHaveBeenCalledWith(7);
|
||
});
|
||
|
||
it('401s when the guard left no user behind', async () => {
|
||
const listBucketList = vi.fn();
|
||
expect(await thrownAsync(() => makeController({ listBucketList }).listBucketList(reqWithoutUser()))).toEqual({
|
||
status: 401,
|
||
body: { error: 'API token required', code: 'API_TOKEN_REQUIRED' },
|
||
});
|
||
expect(listBucketList).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it('counts against the same rate budget as everything else', async () => {
|
||
const listBucketList = vi.fn();
|
||
expect((await thrownAsync(() => makeController({ listBucketList }, false).listBucketList(req(7)))).status).toBe(429);
|
||
expect(listBucketList).not.toHaveBeenCalled();
|
||
});
|
||
});
|
||
|
||
describe('GET /api/v1/trips', () => {
|
||
it('returns the accessible trips for the token owner', async () => {
|
||
const listTrips = vi.fn().mockReturnValue([TRIP]);
|
||
expect(await makeController({ listTrips }).listTrips(req(7))).toEqual({ trips: [TRIP] });
|
||
expect(listTrips).toHaveBeenCalledWith(7);
|
||
});
|
||
|
||
it('returns an empty list rather than 404 when the user has no trips', async () => {
|
||
const listTrips = vi.fn().mockReturnValue([]);
|
||
expect(await makeController({ listTrips }).listTrips(req(7))).toEqual({ trips: [] });
|
||
});
|
||
|
||
it('401s if the guard was somehow bypassed and no user is attached', async () => {
|
||
const listTrips = vi.fn();
|
||
expect(await thrownAsync(() => makeController({ listTrips }).listTrips(reqWithoutUser()))).toEqual({
|
||
status: 401,
|
||
body: { error: 'API token required', code: 'API_TOKEN_REQUIRED' },
|
||
});
|
||
expect(listTrips).not.toHaveBeenCalled();
|
||
});
|
||
});
|
||
|
||
describe('GET /api/v1/trips/:id', () => {
|
||
it('passes the parsed id and user through, and defaults include to every section', async () => {
|
||
const getTrip = vi.fn().mockReturnValue(TRIP);
|
||
const res = await makeController({ getTrip }).getTrip(req(7), '12', undefined);
|
||
expect(res).toEqual(TRIP);
|
||
expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES], expect.arrayContaining(['days']));
|
||
});
|
||
|
||
it('treats an empty include the same as an absent one', async () => {
|
||
const getTrip = vi.fn().mockReturnValue(TRIP);
|
||
await makeController({ getTrip }).getTrip(req(7), '12', ' ');
|
||
expect(getTrip).toHaveBeenCalledWith(12, 7, [...PUBLIC_API_INCLUDES], expect.arrayContaining(['days']));
|
||
});
|
||
|
||
it('narrows to the requested sections and tolerates spacing', async () => {
|
||
const getTrip = vi.fn().mockReturnValue(TRIP);
|
||
await makeController({ getTrip }).getTrip(req(7), '12', 'days, notes');
|
||
expect(getTrip).toHaveBeenCalledWith(12, 7, ['days', 'notes'], expect.arrayContaining(['days']));
|
||
});
|
||
|
||
/**
|
||
* The one that matters: a trip belonging to someone else answers exactly like a
|
||
* trip that was never created. Anything else turns the endpoint into a way to
|
||
* count another user's trips.
|
||
*/
|
||
it('404s for a trip the caller may not read, with no hint that it exists', async () => {
|
||
const getTrip = vi.fn().mockReturnValue(null);
|
||
expect(await thrownAsync(() => makeController({ getTrip }).getTrip(req(7), '999', undefined))).toEqual({
|
||
status: 404,
|
||
body: { error: 'Trip not found' },
|
||
});
|
||
});
|
||
|
||
it.each([
|
||
['a non-numeric id', 'abc'],
|
||
['a numeric prefix', '12abc'],
|
||
['a negative id', '-1'],
|
||
['zero', '0'],
|
||
['a float', '1.5'],
|
||
['an empty id', ''],
|
||
['whitespace', ' 12 '],
|
||
['a sql fragment', "1 OR 1=1"],
|
||
['an id past the safe integer range', '9007199254740993'],
|
||
])('400s on %s without touching the service', async (_label, raw) => {
|
||
const getTrip = vi.fn();
|
||
expect((await thrownAsync(() => makeController({ getTrip }).getTrip(req(7), raw, undefined))).status).toBe(400);
|
||
expect(getTrip).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it.each([
|
||
['an unknown section', 'days,everything'],
|
||
['a typo', 'dayz'],
|
||
['only commas', ',,,'],
|
||
])('400s on %s rather than silently dropping it', async (_label, include) => {
|
||
const getTrip = vi.fn();
|
||
const res = await thrownAsync(() => makeController({ getTrip }).getTrip(req(7), '12', include));
|
||
expect(res.status).toBe(400);
|
||
expect(getTrip).not.toHaveBeenCalled();
|
||
});
|
||
});
|
||
|
||
describe('rate limiting', () => {
|
||
it('429s the list once the budget is spent, without reaching the service', async () => {
|
||
const listTrips = vi.fn();
|
||
const res = await thrownAsync(() => 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', async () => {
|
||
const getTrip = vi.fn();
|
||
expect((await thrownAsync(() => makeController({ getTrip }, false).getTrip(req(7), '12', undefined))).status).toBe(429);
|
||
expect(getTrip).not.toHaveBeenCalled();
|
||
});
|
||
});
|
||
});
|