Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/node/api-core/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@saga-ed/soa-api-core",
"version": "1.1.4",
"version": "1.2.0",
"private": false,
"type": "module",
"main": "dist/rest-controller.js",
Expand Down Expand Up @@ -68,6 +68,7 @@
"dependencies": {
"@apollo/server": "^4.10.0",
"@graphql-tools/schema": "^10.0.0",
"@saga-ed/soa-api-util": "workspace:*",
"cors": "^2.8.5",
"express": "^4.18.2",
"fast-glob": "^3.3.3",
Expand Down
46 changes: 46 additions & 0 deletions packages/node/api-core/src/__tests__/express-server.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { TestSector } from './test-sector.js';
import { fetch } from 'undici';
import { useContainer, createExpressServer } from 'routing-controllers';
import { ExpressServer } from '../express-server.js';
import { DATADOG_RUM_TRACING_HEADERS } from '@saga-ed/soa-api-util';
import { JsonController, Get, Post, Body } from 'routing-controllers';

function getRandomPort() {
Expand Down Expand Up @@ -183,6 +184,7 @@ describe('ExpressServer (integration)', () => {
async function withCorsServer(
domains: string[] | undefined,
fn: (port: number, warnings: string[]) => Promise<void>,
extraConfig: Record<string, unknown> = {},
) {
const container = new Container();
const warnings: string[] = [];
Expand All @@ -199,6 +201,7 @@ describe('ExpressServer (integration)', () => {
logLevel: 'info' as const,
name: 'CORS Test Server',
...(domains !== undefined ? { corsAllowedDomains: domains } : {}),
...extraConfig,
};

container.bind('ExpressServerConfig').toConstantValue(config);
Expand Down Expand Up @@ -279,6 +282,49 @@ describe('ExpressServer (integration)', () => {
expect(res.headers.get('access-control-allow-methods')).toContain('POST');
});
});

describe('allowed headers (Datadog RUM)', () => {
it('default allow-headers include the baseline + every Datadog RUM tracing header', async () => {
await withCorsServer(['saga.org'], async (port) => {
const res = await fetch(`http://localhost:${port}/simple/`, {
method: 'OPTIONS',
headers: {
Origin: 'https://coach.saga.org',
'Access-Control-Request-Method': 'GET',
'Access-Control-Request-Headers': 'x-datadog-trace-id',
},
});
const allow = res.headers.get('access-control-allow-headers') ?? '';
expect(allow).toContain('Content-Type');
expect(allow).toContain('Authorization');
for (const header of DATADOG_RUM_TRACING_HEADERS) {
expect(allow).toContain(header);
}
});
});

it('config.allowedHeaders overrides the default (replaces, not merges)', async () => {
await withCorsServer(
['saga.org'],
async (port) => {
const res = await fetch(`http://localhost:${port}/simple/`, {
method: 'OPTIONS',
headers: {
Origin: 'https://coach.saga.org',
'Access-Control-Request-Method': 'GET',
'Access-Control-Request-Headers': 'x-custom-header',
},
});
const allow = res.headers.get('access-control-allow-headers') ?? '';
expect(allow).toContain('X-Custom-Header');
// A caller-supplied list fully replaces the default — Datadog headers
// are not auto-merged, so a service overriding must spread them itself.
expect(allow).not.toContain('x-datadog-trace-id');
},
{ allowedHeaders: ['Content-Type', 'X-Custom-Header'] },
);
});
});
});

it('should work without basePath using ExpressServer (backward compatibility)', async () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/node/api-core/src/express-server-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ export const ExpressServerSchema = z.object({
corsAllowedDomains: z
.array(z.string())
.optional(),
// CORS `Access-Control-Allow-Headers`. When omitted, defaults to the baseline
// request headers plus the Datadog RUM tracing headers (see ExpressServer).
// Set this to override — spread `DATADOG_RUM_TRACING_HEADERS` from
// `@saga-ed/soa-api-util` to keep Datadog browser RUM working.
allowedHeaders: z
.array(z.string())
.optional(),
});

export type ExpressServerConfig = z.infer<typeof ExpressServerSchema>;
15 changes: 14 additions & 1 deletion packages/node/api-core/src/express-server.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,23 @@
import express, { Application } from 'express';
import cors from 'cors';
import { injectable, inject } from 'inversify';
import { DATADOG_RUM_TRACING_HEADERS } from '@saga-ed/soa-api-util';
import type { ExpressServerConfig } from './express-server-schema.js';
import type { ILogger } from '@saga-ed/soa-logger';
import { useContainer, useExpressServer, getMetadataArgsStorage } from 'routing-controllers';
import { Container } from 'inversify';
import { SectorsController } from './sectors-controller.js';

// Default CORS `Access-Control-Allow-Headers`: the baseline request headers
// every Saga API accepts, plus the Datadog browser-RUM distributed-tracing
// headers. Once a frontend enables RUM, those headers ride on every cross-origin
// request and the browser fails the preflight if any one is missing — so
// allowing them by default keeps RUM-instrumented frontends working with no
// per-service config. Services needing extra headers set `allowedHeaders` in
// their ExpressServerConfig (spread `DATADOG_RUM_TRACING_HEADERS` to keep RUM
// working). See hipponot/iac#358.
const DEFAULT_ALLOWED_HEADERS: string[] = ['Content-Type', 'Authorization', ...DATADOG_RUM_TRACING_HEADERS];

@injectable()
export class ExpressServer {
private readonly app: Application;
Expand All @@ -29,12 +40,14 @@ export class ExpressServer {
// any exact match or subdomain of a listed domain is allowed.
// When omitted/empty, defaults to origin: true (reflect any origin) for backward compatibility.
// credentials is always true — Saga apps use cross-origin cookie auth.
// allowedHeaders defaults to DEFAULT_ALLOWED_HEADERS (baseline + Datadog RUM);
// services override via the `allowedHeaders` config field.
const domains = this.config.corsAllowedDomains ?? [];

const corsOptions: cors.CorsOptions = {
credentials: true,
methods: ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'],
allowedHeaders: ['Content-Type', 'Authorization'],
allowedHeaders: this.config.allowedHeaders ?? DEFAULT_ALLOWED_HEADERS,
};

if (domains.length > 0) {
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading