diff --git a/.changeset/trust-proxy-client-address.md b/.changeset/trust-proxy-client-address.md deleted file mode 100644 index c959341..0000000 --- a/.changeset/trust-proxy-client-address.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -'seamless-auth-api': patch ---- - -Add `TRUST_PROXY` so the client address can be read from `X-Forwarded-For`. - -Express leaves `trust proxy` off by default, so behind a load balancer `req.ip` resolved to the -balancer rather than the caller. Every client shared one bucket in the global rate limiter and the -slow-down middleware, and `express-rate-limit` logged an `ERR_ERL_UNEXPECTED_X_FORWARDED_FOR` -validation error on each request. Set `TRUST_PROXY` to the number of proxies in front of the server -(`1` behind a single load balancer) to restore per-client limiting. It stays unset by default -because a directly reachable server that trusts the header lets a client forge its own address. diff --git a/.env.example b/.env.example index 2cd43c1..868bd99 100644 --- a/.env.example +++ b/.env.example @@ -4,10 +4,6 @@ NODE_ENV=development # SERVER HOST=0.0.0.0 PORT=5312 -# Number of proxies in front of this server, so the client address can be read from -# X-Forwarded-For. Set it behind a load balancer, otherwise the rate limiters treat every -# client as a single caller. Leave it unset when the server is reachable directly. -# TRUST_PROXY=1 # APPLICATION APP_NAME=Seamless Auth Example diff --git a/docs/configuration.md b/docs/configuration.md index e100634..8ac62d7 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -50,18 +50,17 @@ first boot. ### Application -| Variable | Required | Default | Seeds `system_config` | Notes | -| ----------------- | -------- | ------------- | --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `NODE_ENV` | No | `development` | No | `production` enables stricter checks (JWKS, secrets). | -| `HOST` | No | `0.0.0.0` | No | Bind address. | -| `PORT` | No | `5312` | No | Listen port. | -| `TRUST_PROXY` | No | unset | No | Express `trust proxy` value, used to read the client address from `X-Forwarded-For`. Set it to the number of proxies in front of the server (`1` behind a single load balancer); `loopback` and a comma-separated IP/CIDR allowlist also work. Without it the rate limiters bucket every client behind the proxy together. Leave it unset when the server is reachable directly, or a client can forge the header and pick its own bucket. | -| `APP_NAME` | Yes | - | `app_name` | Min 3 characters. | -| `APP_ID` | Yes | - | No | Stable app identifier. | -| `APP_ORIGINS` | Yes | - | No | CORS allowlist for callers of this API (comma-separated). Distinct from WebAuthn `ORIGINS`. | -| `ISSUER` | Yes | - | No | JWT `iss` and issuer URL. | -| `DEFAULT_ROLES` | Yes | - | `default_roles` | Roles for new users (comma-separated). | -| `AVAILABLE_ROLES` | Yes | - | `available_roles` | Roles permitted in the system (comma-separated). Assigning a role that is not listed is rejected. Include `admin:read` and `admin:write` to offer scoped admin. See [Scoped Admin Roles](./admin-operations.md#scoped-admin-roles). | +| Variable | Required | Default | Seeds `system_config` | Notes | +| ----------------- | -------- | ------------- | --------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `NODE_ENV` | No | `development` | No | `production` enables stricter checks (JWKS, secrets). | +| `HOST` | No | `0.0.0.0` | No | Bind address. | +| `PORT` | No | `5312` | No | Listen port. | +| `APP_NAME` | Yes | - | `app_name` | Min 3 characters. | +| `APP_ID` | Yes | - | No | Stable app identifier. | +| `APP_ORIGINS` | Yes | - | No | CORS allowlist for callers of this API (comma-separated). Distinct from WebAuthn `ORIGINS`. | +| `ISSUER` | Yes | - | No | JWT `iss` and issuer URL. | +| `DEFAULT_ROLES` | Yes | - | `default_roles` | Roles for new users (comma-separated). | +| `AVAILABLE_ROLES` | Yes | - | `available_roles` | Roles permitted in the system (comma-separated). Assigning a role that is not listed is rejected. Include `admin:read` and `admin:write` to offer scoped admin. See [Scoped Admin Roles](./admin-operations.md#scoped-admin-roles). | ### Auth and tokens diff --git a/src/app.ts b/src/app.ts index 6e65f1e..7a0277d 100644 --- a/src/app.ts +++ b/src/app.ts @@ -22,16 +22,6 @@ import getLogger from './utils/logger.js'; const logger = getLogger('app'); const app = express(); -// Express ignores X-Forwarded-For unless it is told how far to trust it, so behind a load -// balancer every request looks like it came from the balancer and the rate limiters bucket -// all clients together. Opt in per deployment: direct-to-internet installs must leave this -// unset, otherwise a client could forge the header and choose its own rate-limit bucket. -const trustProxy = process.env.TRUST_PROXY; -if (trustProxy) { - const hops = Number(trustProxy); - app.set('trust proxy', Number.isNaN(hops) ? trustProxy : hops); -} - const rawOrigin = process.env.APP_ORIGINS!.split(','); const corsOptions: CorsOptions = { diff --git a/tests/unit/app.spec.ts b/tests/unit/app.spec.ts index b8d51a8..96aa93d 100644 --- a/tests/unit/app.spec.ts +++ b/tests/unit/app.spec.ts @@ -1,5 +1,5 @@ import request from 'supertest'; -import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; +import { beforeAll, describe, expect, it } from 'vitest'; import app, { createApp } from '../../src/app.js'; import { AuthEventService } from '../../src/services/authEventService.js'; @@ -84,46 +84,3 @@ describe('createApp error handling', () => { expect(AuthEventService.requestSuspicious).toHaveBeenCalled(); }); }); - -describe('TRUST_PROXY', () => { - // The setting is applied while the module body runs, so each case needs a fresh import. - async function loadApp(trustProxy: string) { - vi.resetModules(); - vi.stubEnv('TRUST_PROXY', trustProxy); - - const { default: freshApp } = await import('../../src/app.js'); - return freshApp; - } - - afterEach(() => { - vi.unstubAllEnvs(); - vi.resetModules(); - }); - - it('leaves X-Forwarded-For untrusted when unset', async () => { - const freshApp = await loadApp(''); - - expect(freshApp.get('trust proxy')).toBe(false); - }); - - it('resolves the client address through the given number of hops', async () => { - const freshApp = await loadApp('1'); - freshApp.get('/__test_client_ip', (req, res) => { - res.json({ ip: req.ip }); - }); - - expect(freshApp.get('trust proxy')).toBe(1); - - const res = await request(freshApp) - .get('/__test_client_ip') - .set('X-Forwarded-For', '203.0.113.7'); - - expect(res.body.ip).toBe('203.0.113.7'); - }); - - it('passes a non-numeric setting through to Express', async () => { - const freshApp = await loadApp('loopback'); - - expect(freshApp.get('trust proxy')).toBe('loopback'); - }); -});