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
2 changes: 2 additions & 0 deletions .env.dfe.example
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ DFE_AUTH_MODE=
# serves JWKS at this path -- see dfe-engine docs/LOCAL-DEV.md.
DFE_ENGINE_JWKS_URL=http://localhost:8003/.well-known/jwks.json
DFE_ENGINE_ISSUER=https://dfe.local/api
# Milliseconds any request to the engine may take before it is abandoned.
# DFE_ENGINE_TIMEOUT_MS=5000

# Optional: auto-provision a ClickHouse connection on team creation
# (array of {name, host, username, password}; omit to skip).
Expand Down
1 change: 1 addition & 0 deletions docs/architecture/oidc-authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ invite step in DFE mode.
| `DFE_AUTH_MODE` | unset | `oidc-proxy`, `header-dev`, or unset for stock upstream. Any other value stops the API |
| `DFE_ENGINE_JWKS_URL` | - | engine JWKS URL, and `oidc-proxy` mode will not start without it |
| `DFE_ENGINE_ISSUER` | unset | enforced as the `iss` claim when set |
| `DFE_ENGINE_TIMEOUT_MS` | `5000` | how long any request to the engine may take before it is abandoned |
| `DFE_AUTH_HEADER_EMAIL` | `x-forwarded-email` | `header-dev` only, which will not start with `NODE_ENV=production` |
| `DFE_AUTH_HEADER_GROUPS` | `x-forwarded-groups` | `header-dev` only |
| `DFE_AUTH_DEFAULT_TEAM` | unset | the engine service identity's team, and the `header-dev` team when no group header is present |
Expand Down
24 changes: 24 additions & 0 deletions packages/api/src/dfe/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
const KEYS: readonly string[] = [
'DFE_AUTH_MODE',
'DFE_ENGINE_JWKS_URL',
'DFE_ENGINE_TIMEOUT_MS',
'NODE_ENV',
];
type Env = {
DFE_AUTH_MODE?: string;
DFE_ENGINE_JWKS_URL?: string;
DFE_ENGINE_TIMEOUT_MS?: string;
NODE_ENV?: string;
};

Expand Down Expand Up @@ -107,3 +109,25 @@ describe('header-dev in production', () => {
).not.toThrow();
});
});

describe('DFE_ENGINE_TIMEOUT_MS', () => {
test.each([
[undefined, 5000],
['', 5000],
['250', 250],
['15000', 15000],
])('%j reads as %d ms', (value, ms) => {
expect(
loadConfigWith({ DFE_ENGINE_TIMEOUT_MS: value })?.DFE_ENGINE_TIMEOUT_MS,
).toBe(ms);
});

test.each(['0', '-5', '1.5', 'soon', '5s', ' '])(
'refuses to start on %j',
value => {
expect(() => loadConfigWith({ DFE_ENGINE_TIMEOUT_MS: value })).toThrow(
/DFE_ENGINE_TIMEOUT_MS/,
);
},
);
});
10 changes: 10 additions & 0 deletions packages/api/src/dfe/__tests__/engine-session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ jest.mock('@/dfe/controllers/org-connection', () => ({
engineOrigin: jest.fn(() => 'http://engine.test:8000'),
}));

import { DFE_ENGINE_TIMEOUT_MS } from '@/dfe/config';
import {
clearEngineSessionCache,
resolveEngineSession,
Expand Down Expand Up @@ -84,6 +85,15 @@ describe('the decision', () => {
);
});

test('bounds the engine call by the configured timeout', async () => {
const timeout = jest.spyOn(AbortSignal, 'timeout');
grants(['sre']);

await resolveEngineSession('tok-a', FAR);

expect(timeout).toHaveBeenCalledWith(DFE_ENGINE_TIMEOUT_MS);
});

test('names the team after the ClickHouse identity the engine hands the caller', async () => {
grants(['acme-analysts'], ORG);

Expand Down
17 changes: 17 additions & 0 deletions packages/api/src/dfe/__tests__/org-connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@ jest.mock('@/controllers/sources', () => ({

jest.mock('@/dfe/config', () => ({
DFE_ENGINE_JWKS_URL: 'http://engine.test:8000/.well-known/jwks.json',
get DFE_ENGINE_TIMEOUT_MS() {
return mockEngineTimeoutMs;
},
}));

let mockEngineTimeoutMs = 5000;

jest.mock('@/tasks/provisionDashboards', () => ({
syncDashboards: jest.fn(),
}));
Expand Down Expand Up @@ -263,6 +268,18 @@ describe('ensureOrgConnection - the team identity', () => {
expect(mockCreateConn).not.toHaveBeenCalled();
});

test('bounds the engine read by the configured timeout', async () => {
mockEngineTimeoutMs = 4321;
const timeout = jest.spyOn(AbortSignal, 'timeout');
mockConns.mockResolvedValue([]);
(global.fetch as jest.Mock).mockResolvedValue({ ok: false, status: 403 });

await ensureOrgConnection('tok', 'team-1', ORG);

expect(timeout).toHaveBeenCalledWith(4321);
mockEngineTimeoutMs = 5000;
});

test.each([500, 502, 503])(
'an engine %i is unavailable, not a refusal',
async status => {
Expand Down
10 changes: 10 additions & 0 deletions packages/api/src/dfe/__tests__/query-export.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import request from 'supertest';
import { getConnectionById } from '@/controllers/connection';
import { getSource } from '@/controllers/sources';
import { makeDocument } from '@/dfe/__tests__/doubles';
import { DFE_ENGINE_TIMEOUT_MS } from '@/dfe/config';
import { extractToken } from '@/dfe/middleware/jwt-verify';
import router, { MAX_EXPORT_SQL_BODY_CHARS } from '@/dfe/routers/query-export';
import { getNonNullUserWithTeam } from '@/middleware/auth';
Expand Down Expand Up @@ -134,6 +135,15 @@ describe('POST /dfe/create-rule status mapping', () => {
expect(res.body.detail).toBe('rule:write required');
});

it('bounds the engine call by the configured timeout', async () => {
const timeout = jest.spyOn(AbortSignal, 'timeout');
engineReplies(200, { id: 'rule-1' });

await createRule().send(RULE_BODY);

expect(timeout).toHaveBeenCalledWith(DFE_ENGINE_TIMEOUT_MS);
});

it('forwards the created rule on success', async () => {
engineReplies(200, { id: 'rule-1', display_name: 'noisy logins' });

Expand Down
20 changes: 20 additions & 0 deletions packages/api/src/dfe/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,26 @@ export const DFE_ENGINE_JWKS_URL = env.DFE_ENGINE_JWKS_URL as

export const DFE_ENGINE_ISSUER = env.DFE_ENGINE_ISSUER as string | undefined;

// How long a request to the engine may take before it is abandoned, so a slow
// or unreachable control plane fails the request instead of hanging the client.
// Every DFE call to the engine reads this one value.
const DEFAULT_ENGINE_TIMEOUT_MS = 5000;

function engineTimeoutMs(raw: string | undefined): number {
if (raw === undefined || raw === '') {
return DEFAULT_ENGINE_TIMEOUT_MS;
}
const ms = Number(raw);
if (!Number.isInteger(ms) || ms <= 0) {
throw new Error(
`DFE_ENGINE_TIMEOUT_MS=${JSON.stringify(raw)} is not a whole number of milliseconds above zero.`,
);
}
return ms;
}

export const DFE_ENGINE_TIMEOUT_MS = engineTimeoutMs(env.DFE_ENGINE_TIMEOUT_MS);

export const isDfeEnabled =
DFE_AUTH_MODE === 'oidc-proxy' || DFE_AUTH_MODE === 'header-dev';

Expand Down
7 changes: 3 additions & 4 deletions packages/api/src/dfe/controllers/engine-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ import { createHash } from 'node:crypto';

import { z } from 'zod';

import { DFE_ENGINE_TIMEOUT_MS } from '@/dfe/config';
import { engineOrigin } from '@/dfe/controllers/org-connection';
import logger from '@/utils/logger';

const ENGINE_TIMEOUT_MS = 5000;

// A revoked group, a changed role or a deleted account reaches HyperDX within this window.
const CACHE_TTL_MS = 30_000;

Expand Down Expand Up @@ -90,7 +89,7 @@ async function identityFromConnection(
): Promise<string | EngineAnswer> {
const resp = await fetch(`${origin}/api/v1/hyperdx/connection`, {
headers: { Authorization: `Bearer ${token}` },
signal: AbortSignal.timeout(ENGINE_TIMEOUT_MS),
signal: AbortSignal.timeout(DFE_ENGINE_TIMEOUT_MS),
});
if (resp.status === 401) {
return refused(401, 'engine_refused');
Expand Down Expand Up @@ -122,7 +121,7 @@ async function askEngine(token: string): Promise<EngineAnswer> {
try {
const resp = await fetch(`${origin}/api/v1/auth/me`, {
headers: { Authorization: `Bearer ${token}` },
signal: AbortSignal.timeout(ENGINE_TIMEOUT_MS),
signal: AbortSignal.timeout(DFE_ENGINE_TIMEOUT_MS),
});
if (resp.status === 401 || resp.status === 403) {
return refused(resp.status === 401 ? 401 : 403, 'engine_refused');
Expand Down
4 changes: 1 addition & 3 deletions packages/api/src/dfe/controllers/org-connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ const OrgConnectionSchema = z.object({

type OrgConnection = z.infer<typeof OrgConnectionSchema>;

const ENGINE_TIMEOUT_MS = 5000;

// The engine API shares an origin with its JWKS endpoint, so we derive the base
// URL from DFE_ENGINE_JWKS_URL rather than carrying a second env var.
// Exported as the single definition of the engine origin so DFE routers reuse it
Expand Down Expand Up @@ -97,7 +95,7 @@ async function fetchOrgConnection(
try {
const resp = await fetch(`${origin}/api/v1/hyperdx/connection`, {
headers: { Authorization: `Bearer ${token}` },
signal: AbortSignal.timeout(ENGINE_TIMEOUT_MS),
signal: AbortSignal.timeout(dfeConfig.DFE_ENGINE_TIMEOUT_MS),
});
if (resp.status === 401 || resp.status === 403) {
logger.warn(
Expand Down
7 changes: 2 additions & 5 deletions packages/api/src/dfe/routers/query-export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { validateRequest } from 'zod-express-middleware';

import { getConnectionById } from '@/controllers/connection';
import { getSource } from '@/controllers/sources';
import { DFE_ENGINE_TIMEOUT_MS } from '@/dfe/config';
import { engineOrigin } from '@/dfe/controllers/org-connection';
import { requireNonSimpleRequest } from '@/dfe/middleware/cross-site';
import { extractToken } from '@/dfe/middleware/jwt-verify';
Expand Down Expand Up @@ -48,10 +49,6 @@ const createRuleBodySchema = z.object({
source: z.string().optional(),
});

// Bound the wait on the engine so a slow/unreachable control plane fails the
// request instead of hanging the client. Matches org-connection's ceiling.
const ENGINE_TIMEOUT_MS = 5000;

// renderChartConfig's expression regexes run in time quadratic in their input
// and the API accepts 32 MB bodies, so the export body is capped at a size whose
// worst case holds the event loop, and every team with it, under half a second.
Expand Down Expand Up @@ -230,7 +227,7 @@ router.post(
hunt_name,
source,
}),
signal: AbortSignal.timeout(ENGINE_TIMEOUT_MS),
signal: AbortSignal.timeout(DFE_ENGINE_TIMEOUT_MS),
});

// Forward the engine's status and JSON body verbatim so RBAC failures
Expand Down
Loading