diff --git a/fluxer_api/src/api/app/MiddlewarePipeline.ts b/fluxer_api/src/api/app/MiddlewarePipeline.ts index 742657d21..51a8e115f 100644 --- a/fluxer_api/src/api/app/MiddlewarePipeline.ts +++ b/fluxer_api/src/api/app/MiddlewarePipeline.ts @@ -91,11 +91,7 @@ export function configureMiddleware(routes: HonoApp, options: MiddlewarePipeline } routes.use(TorExitMiddleware); routes.use(AuditLogMiddleware); - routes.use( - RequireClientIpMiddleware({ - requiredHeaders: [resolvedHeader], - }), - ); + routes.use(RequireClientIpMiddleware()); routes.use(ServiceMiddleware); routes.use(UserMiddleware); routes.use(ContentFilterMiddleware); diff --git a/fluxer_api/src/api/middleware/RequireClientIpMiddleware.ts b/fluxer_api/src/api/middleware/RequireClientIpMiddleware.ts index 0afc9ffa4..9408a7de5 100644 --- a/fluxer_api/src/api/middleware/RequireClientIpMiddleware.ts +++ b/fluxer_api/src/api/middleware/RequireClientIpMiddleware.ts @@ -2,16 +2,15 @@ import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes'; import {ForbiddenError} from '@fluxer/errors/src/domains/core/ForbiddenError'; -import {resolveClientIpHeaderName} from '@fluxer/ip_utils/src/ClientIp'; import {createMiddleware} from 'hono/factory'; import {Config} from '../Config'; import {Logger} from '../Logger'; import type {HonoEnv} from '../types/HonoEnv'; +import {getRequestClientIp} from '../utils/RequestClientIp'; import {stripApiPrefix} from '../utils/RequestPathUtils'; interface RequireClientIpOptions { exemptPaths?: Array; - requiredHeaders?: Array; } const defaultExemptPaths: Array = [ @@ -22,10 +21,7 @@ const defaultExemptPaths: Array = [ '/connections/bluesky/jwks.json', ]; -export function RequireClientIpMiddleware({ - exemptPaths = defaultExemptPaths, - requiredHeaders = [resolveClientIpHeaderName(Config.proxy.client_ip_header)], -}: RequireClientIpOptions = {}) { +export function RequireClientIpMiddleware({exemptPaths = defaultExemptPaths}: RequireClientIpOptions = {}) { return createMiddleware(async (ctx, next) => { if (Config.dev.testModeEnabled) { await next(); @@ -36,12 +32,8 @@ export function RequireClientIpMiddleware({ await next(); return; } - const hasRequiredHeader = requiredHeaders.some((header) => { - const value = ctx.req.header(header); - return value != null && value.trim() !== ''; - }); - if (!hasRequiredHeader) { - Logger.warn({path}, 'Rejected request without required proxy headers'); + if (getRequestClientIp(ctx) === null) { + Logger.warn({path}, 'Rejected request without a resolvable client IP'); throw new ForbiddenError({code: APIErrorCodes.FORBIDDEN}); } await next(); diff --git a/fluxer_api/src/api/middleware/tests/RequireClientIpMiddleware.test.ts b/fluxer_api/src/api/middleware/tests/RequireClientIpMiddleware.test.ts new file mode 100644 index 000000000..2c3a80904 --- /dev/null +++ b/fluxer_api/src/api/middleware/tests/RequireClientIpMiddleware.test.ts @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import {AppErrorHandler} from '@fluxer/errors/src/domains/core/ErrorHandlers'; +import {Hono} from 'hono'; +import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import {Config} from '../../Config'; +import type {HonoEnv} from '../../types/HonoEnv'; +import type {ClientIpResolution} from '../../utils/RequestClientIp'; +import {RequireClientIpMiddleware} from '../RequireClientIpMiddleware'; + +interface Harness { + request: (headers: Record) => Promise; + resolutions: Array; +} + +function createHarness(path = 'http://localhost/v1/messages'): Harness { + const resolutions: Array = []; + const app = new Hono(); + app.use(RequireClientIpMiddleware()); + app.get('/v1/messages', (ctx) => { + resolutions.push(ctx.get('clientIpResolution')); + return ctx.text('ok'); + }); + app.get('/_health', (ctx) => ctx.text('OK')); + app.onError(AppErrorHandler); + return { + request: async (headers) => app.request(path, {headers}), + resolutions, + }; +} + +describe('RequireClientIpMiddleware', () => { + let previousTestModeEnabled: boolean; + let previousTrustClientIpHeader: boolean; + + beforeEach(() => { + previousTestModeEnabled = Config.dev.testModeEnabled; + previousTrustClientIpHeader = Config.proxy.trust_client_ip_header; + Config.dev.testModeEnabled = false; + Config.proxy.trust_client_ip_header = true; + }); + + afterEach(() => { + Config.dev.testModeEnabled = previousTestModeEnabled; + Config.proxy.trust_client_ip_header = previousTrustClientIpHeader; + }); + + it('rejects an unparsable client ip header with 403 FORBIDDEN', async () => { + const harness = createHarness(); + const response = await harness.request({'x-forwarded-for': 'not-an-ip'}); + expect(response.status).toBe(403); + expect(await response.json()).toMatchObject({code: 'FORBIDDEN'}); + }); + + it('rejects a request when the header is not trusted with 403', async () => { + Config.proxy.trust_client_ip_header = false; + const harness = createHarness(); + const response = await harness.request({'x-forwarded-for': '203.0.113.10'}); + expect(response.status).toBe(403); + expect(await response.json()).toMatchObject({code: 'FORBIDDEN'}); + }); + + it('rejects a request with no client ip header with 403 FORBIDDEN', async () => { + const harness = createHarness(); + const response = await harness.request({}); + expect(response.status).toBe(403); + expect(await response.json()).toMatchObject({code: 'FORBIDDEN'}); + }); + + it('passes a parsable header and caches the resolution', async () => { + const harness = createHarness(); + const response = await harness.request({'x-forwarded-for': '203.0.113.10, 10.0.0.1'}); + expect(response.status).toBe(200); + expect(harness.resolutions[0]?.ip).toBe('203.0.113.10'); + }); + + it('leaves exempt paths alone', async () => { + const harness = createHarness('http://localhost/_health'); + const response = await harness.request({}); + expect(response.status).toBe(200); + }); + + it('passes every request through in test mode', async () => { + Config.dev.testModeEnabled = true; + const harness = createHarness(); + const response = await harness.request({'x-forwarded-for': 'not-an-ip'}); + expect(response.status).toBe(200); + }); +}); diff --git a/fluxer_api/src/api/utils/tests/RequestClientIp.test.ts b/fluxer_api/src/api/utils/tests/RequestClientIp.test.ts index a5a66f030..985d566b4 100644 --- a/fluxer_api/src/api/utils/tests/RequestClientIp.test.ts +++ b/fluxer_api/src/api/utils/tests/RequestClientIp.test.ts @@ -1,5 +1,6 @@ // SPDX-License-Identifier: AGPL-3.0-or-later +import {AppErrorHandler} from '@fluxer/errors/src/domains/core/ErrorHandlers'; import {MissingClientIpError} from '@fluxer/ip_utils/src/ClientIp'; import {Hono} from 'hono'; import {describe, expect, it} from 'vitest'; @@ -125,4 +126,15 @@ describe('RequestClientIp', () => { await app.request('http://localhost/v1/messages', {headers: {'x-forwarded-for': '203.0.113.10'}}); expect(seen).toEqual(['203.0.113.10', null, '203.0.113.10']); }); + it('a missing client ip surfaces as 403 FORBIDDEN through AppErrorHandler', async () => { + const app = new Hono(); + app.get('/v1/messages', (ctx) => { + requireRequestClientIp(ctx); + return ctx.text('ok'); + }); + app.onError(AppErrorHandler); + const response = await app.request('http://localhost/v1/messages'); + expect(response.status).toBe(403); + expect(await response.json()).toMatchObject({code: 'FORBIDDEN'}); + }); }); diff --git a/packages/ip_utils/src/ClientIp.ts b/packages/ip_utils/src/ClientIp.ts index e5d3fb21e..b7c3871df 100644 --- a/packages/ip_utils/src/ClientIp.ts +++ b/packages/ip_utils/src/ClientIp.ts @@ -21,6 +21,9 @@ interface HeadersLike { } export class MissingClientIpError extends Error { + readonly code = 'FORBIDDEN'; + readonly status = 403; + constructor() { super('Client IP header is required'); this.name = 'MissingClientIpError';