From e4d2e13c9ffd3b4183a3391a42ba4c4a59da112a Mon Sep 17 00:00:00 2001 From: Scott Duensing Date: Thu, 18 Jun 2026 17:22:33 -0500 Subject: [PATCH] Fixes. --- README.md | 38 ++++++--- docs/architecture.md | 21 ++++- docs/configuration.md | 43 +++++++--- docs/ldap.md | 60 ++++++++++++-- docs/oidc.md | 27 +++++-- docs/operations.md | 46 ++++++++++- docs/saml.md | 6 +- docs/security.md | 57 +++++++++---- docs/testing.md | 57 +++++++++---- index.js | 20 +++-- lib/adminUi.js | 113 +++++++++++++++++++++++++- lib/constants.js | 19 +++++ lib/crypto.js | 33 +++++++- lib/keys.js | 16 ++++ lib/ldap/server.js | 48 ++++++----- lib/ldap/settings.js | 131 ++++++++++++++++++++++++++++++ lib/oidc/provider.js | 25 ++++-- test/e2e.js | 8 +- test/jwksConsistencyGate.js | 156 ++++++++++++++++++++++++++++++++++++ test/keyRotationGate.js | 35 ++++++-- 20 files changed, 839 insertions(+), 120 deletions(-) create mode 100644 lib/ldap/settings.js create mode 100644 test/jwksConsistencyGate.js diff --git a/README.md b/README.md index 056dc47..36b0a72 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,8 @@ model: All three are multi-tenant aware: each Saltcorn tenant gets its own issuer, asymmetric signing key, and SAML certificate, with the per-tenant private key -sealed at rest (AES-256-GCM) under a key derived from `SALTCORN_SESSION_SECRET`. +sealed at rest (AES-256-GCM) under a key derived from Saltcorn's session secret +(resolved from the installation -- env var, `~/.config/.saltcorn`, or DB config). - Package name: `saltcorn-idp` - Version: `0.0.1` @@ -32,7 +33,7 @@ work is hardening, not new protocols. | Protocol | Library | Public base path | Status | |----------|---------|------------------|--------| | OIDC / OAuth2 (auth-code + PKCE) | `oidc-provider` `^9.8.4` | `/idp` | Built; covered by `test/e2e.js` and `test/mtGate.js` | -| LDAP with groups (LDAPS only) | `ldapjs` `3.0.7` | LDAPS listener (opt-in via env) | Built; covered by `test/ldapGate.js` and `test/ldapMultiTenantGate.js` | +| LDAP with groups (LDAPS only) | `ldapjs` `3.0.7` | LDAPS listener (enabled from `/admin/idp/ldap`) | Built; covered by `test/ldapGate.js` and `test/ldapMultiTenantGate.js` | | SAML 2.0 (SSO + SLO) | `samlify` `2.13.1` | `/idp/saml` | Built; covered by `test/samlGate.js` | | Multi-tenant (per-tenant issuer/keys/cert) | n/a | per-tenant schema | Built; verified on Postgres | | Admin UI (clients, groups, SAML SPs, LDAP service) | n/a | `/admin/idp` | Built (CSRF-protected, role-gated) | @@ -95,8 +96,11 @@ cd /home/scott/claude/saltcorn On first load per tenant, `onLoad` (see `index.js`) creates the `_idp_*` tables, bootstraps the RSA signing key, ensures the SAML certificate, registers -the `/idp/` CSRF bypass, and starts the LDAP listener if -`SALTCORN_IDP_LDAP_PORT` is set. +the `/idp/` CSRF bypass, and starts the in-process LDAP listener when it is +enabled. The single, instance-global LDAP listener is configured from the admin +panel at `/admin/idp/ldap` on the public (root) site (see `lib/ldap/settings.js`, +`resolveRuntime`); `SALTCORN_IDP_LDAP_PORT` / `SALTCORN_IDP_LDAP_HOST` remain +optional per-setting overrides (and the env port also still enables LDAP). For the Postgres multi-tenant instance, register the plugin into each tenant schema (tenants must already exist and the plugin must be installed into the PG @@ -120,6 +124,8 @@ npm run test:mt # multi-tenant OIDC (test/mtGate.js) -- needs :3002 (Pos node test/ldapGate.js # LDAP (needs :3000 and LDAPS :1636) node test/samlGate.js # SAML 2.0 (needs :3000) node test/ldapMultiTenantGate.js # multi-tenant LDAP (needs :3002 and LDAPS :1637; self-skips if port unreachable) +node test/keyRotationGate.js # key rotation invariants (8 checks; pins one worker via keep-alive) +node test/jwksConsistencyGate.js # post-rotation JWKS convergence across all cluster workers ``` `npm test` and `npm run test:mt` are the only scripts defined in @@ -131,9 +137,22 @@ node test/ldapMultiTenantGate.js # multi-tenant LDAP (needs :3002 and LDAPS :1 | Variable | Required | Default | Purpose | |----------|----------|---------|---------| -| `SALTCORN_SESSION_SECRET` | Yes | n/a (falls back to Saltcorn `session_secret` config) | Derives the at-rest KEK (AES-256-GCM) that seals private keys, SAML keys, and client/LDAP secrets, and the oidc-provider cookie keys. Rotating it invalidates all sealed data. | -| `SALTCORN_IDP_LDAP_PORT` | No | unset (LDAP disabled) | Enables the LDAPS listener on this port. Each dev instance uses its own port to avoid collisions. | -| `SALTCORN_IDP_LDAP_HOST` | No | `127.0.0.1` | Interface the LDAPS listener binds to. Loopback by default; set to `0.0.0.0` to expose LDAP on the network. | +| `SALTCORN_SESSION_SECRET` | No | Resolved from the Saltcorn installation (connection object / `~/.config/.saltcorn` config file / DB config) | Derives the at-rest KEK (AES-256-GCM) that seals private keys, SAML keys, and client/LDAP secrets, and the oidc-provider cookie keys. The plugin reads the secret from Saltcorn itself, so no separate env var is needed; setting it still works and takes priority. Rotating the secret invalidates all sealed data. | +| `SALTCORN_IDP_LDAP_PORT` | No | from `/admin/idp/ldap` (LDAP disabled until enabled there) | Optional override for the LDAPS listener port; when set it also enables LDAP. The listener is normally configured from the admin panel; each dev instance can set its own port to avoid collisions. | +| `SALTCORN_IDP_LDAP_HOST` | No | from `/admin/idp/ldap` (default `127.0.0.1`) | Optional override for the interface the LDAPS listener binds to. Loopback by default; set to `0.0.0.0` to expose LDAP on the network (the panel warns on a non-loopback host). | + +The in-process LDAPS listener is normally enabled and addressed from the admin +panel at `/admin/idp/ldap` on the public (root) site -- root-tenant + admin only +(tenant admins see it read-only). There is one listener per Saltcorn instance +shared by all tenants, so its enable/host/port live in the root tenant's config +(`idp_ldap_enabled` / `idp_ldap_host` / `idp_ldap_port`). The env vars above are +optional per-setting overrides that win when present (env beats config); they are +never required, and a plain install can enable LDAP from the panel alone. Listener +changes apply on restart: the binding process records `idp_ldap_applied` and the +panel shows a "restart Saltcorn to apply" reminder until the running listener +matches the desired settings. `resolveRuntime` (`lib/ldap/settings.js`) applies +this precedence; `startLdap` (`lib/ldap/server.js`) and the self-heal watchdog +(`index.js`) consult it instead of reading `process.env` directly. OIDC issuer derivation also honors Saltcorn's global `base_url` config. In any multi-tenant or proxied deployment, set `base_url`; the request-host fallback is @@ -152,8 +171,9 @@ states are `active`, `retiring`, and `retired` (see `lib/constants.js`). | `/idp/saml` | SAML metadata / SSO / SLO / IdP-init | Exempt | | `/admin/idp` | Admin UI (browser) | Protected; gated to role_id 1 | -LDAP is not an HTTP path; it is a separate LDAPS listener enabled by -`SALTCORN_IDP_LDAP_PORT`. +LDAP is not an HTTP path; it is a separate LDAPS listener configured from +`/admin/idp/ldap` (with `SALTCORN_IDP_LDAP_PORT` / `SALTCORN_IDP_LDAP_HOST` as +optional overrides). --- diff --git a/docs/architecture.md b/docs/architecture.md index c0afbd4..d56dfb5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -66,9 +66,13 @@ tenant. The steps, in order: line is logged. Both log lines include `env_id` and signing `kid`. 6. `ensureCsrfBypass()` -- register the CSRF exemption for the `/idp/` namespace (see below). -7. `startLdap()` (`lib/ldap/server.js`) -- start the LDAPS listener. This only - binds if `SALTCORN_IDP_LDAP_PORT` is set; a module-level guard ensures the - listener starts once per process despite `onLoad` running per tenant. +7. `startLdap()` (`lib/ldap/server.js`) -- start the LDAPS listener. It binds + per the resolved listener settings (admin-panel config on the public site, + with `SALTCORN_IDP_LDAP_PORT` / `SALTCORN_IDP_LDAP_HOST` as optional + overrides) computed by `lib/ldap/settings.js` `resolveRuntime()`; a + module-level guard ensures the listener starts once per process despite + `onLoad` running per tenant. See [`./ldap.md`](./ldap.md) and + [`./operations.md`](./operations.md) for the listener configuration. A failure in the bootstrap steps (1-6) is logged and re-thrown so it is visible -- but only **after** step 7 is attempted. `startLdap()` runs in its own @@ -146,6 +150,7 @@ lib/ interactions.js Login + consent interaction handlers ldap/ server.js LDAPS listener startup + bind retry + settings.js Instance-global listener config (resolveRuntime: env over panel) bind.js Bind handler (user + service account) search.js Search handler (people + groupOfNames) dn.js DN construction + RFC 4514 escaping @@ -223,6 +228,16 @@ for non-GET/HEAD requests calls `restreamBody()` to rebuild a readable stream carrying the body that Saltcorn's parser already drained (using `req.rawBody` when available, otherwise re-encoding `req.body` as JSON or urlencoded). +Each cluster worker caches its own `oidc-provider` instance (with the JWKS +baked in), so the per-worker cache must not go stale after an admin key +rotation. `getProviderEntry` keys its cache on `(issuer, fingerprint)` where +`fingerprint` is `keys.signingSetFingerprint()` -- the ACTIVE-then-RETIRING +`kid`s read fresh from the DB. When the fingerprint changes, the worker +rebuilds the provider on its next request, so every worker converges on the new +active key and JWKS without a restart (otherwise only the worker that handled +the rotate POST would serve the fresh JWKS, and a relying party load-balanced +to a stale worker could fail to verify `id_token`s signed with the new key). + ### Admin pages (`/admin/idp`, CSRF-protected) Defined in `lib/adminUi.js`. These are server-rendered HTML pages and form diff --git a/docs/configuration.md b/docs/configuration.md index 85ebb62..b21d7fe 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -16,23 +16,35 @@ overview), [`./oidc.md`](./oidc.md), [`./ldap.md`](./ldap.md), ## Environment variables -The plugin itself reads only the three variables below. They are read in -`lib/crypto.js` (`SALTCORN_SESSION_SECRET`) and `lib/ldap/server.js` / -`lib/constants.js` (the two LDAP variables). +The plugin reads only the three variables below, and all three are optional. +They are read in `lib/crypto.js` (`SALTCORN_SESSION_SECRET`) and +`lib/ldap/settings.js` (`resolveRuntime`, the two LDAP variables). The LDAP +listener's enable/host/port are configured from the admin panel (see the LDAP +listener section below); the two `SALTCORN_IDP_LDAP_*` env vars are now optional +per-setting overrides of that config, never the only way to enable LDAP. | Variable | Required | Default | Effect | |----------|----------|---------|--------| -| `SALTCORN_SESSION_SECRET` | Yes (see note) | None. Falls back to Saltcorn's `session_secret` config when unset; if neither is available, deriving the KEK throws `"saltcorn-idp: SALTCORN_SESSION_SECRET not available; cannot derive KEK"`. | Source material for two derivations: (1) the at-rest KEK (AES-256-GCM, via HKDF-SHA256) that seals RSA signing keys, the SAML private key, OIDC client secrets, and the LDAP service-account password; (2) the deterministic oidc-provider cookie-signing keys (so interaction/session cookies survive a restart). Rotating this value re-derives the KEK and invalidates all previously sealed data. | -| `SALTCORN_IDP_LDAP_PORT` | No | Unset. When unset, `startLdap()` returns early and no LDAP listener is started. | Enables the in-process LDAPS listener and sets its TCP port. Parsed with `parseInt(..., 10)`; a non-finite value is ignored (no listener). The listener binds only in the cluster primary process. Constant name: `LDAP_PORT_ENV`. | -| `SALTCORN_IDP_LDAP_HOST` | No | `127.0.0.1` (constant `LDAP_DEFAULT_HOST`). An empty or whitespace-only value also falls back to the default. | Interface the LDAPS listener binds to. Loopback by default so LDAP is not network-exposed; set to `0.0.0.0` to serve LDAP clients on other hosts. Binding beyond loopback logs a network-exposure warning. Constant name: `LDAP_HOST_ENV`. | +| `SALTCORN_SESSION_SECRET` | No (see note) | None as an env var. The plugin instead resolves the secret from the Saltcorn installation in the same precedence order Saltcorn uses: env var, then `db.connectObj.session_secret` (which already merges the env var and the `~/.config/.saltcorn` config file), then the config file read directly, then the `session_secret` DB config. Only if none of those yield a value does deriving the KEK throw `"saltcorn-idp: session_secret not available ...; cannot derive KEK"`. | Source material for two derivations: (1) the at-rest KEK (AES-256-GCM, via HKDF-SHA256) that seals RSA signing keys, the SAML private key, OIDC client secrets, and the LDAP service-account password; (2) the deterministic oidc-provider cookie-signing keys (so interaction/session cookies survive a restart). Rotating this value re-derives the KEK and invalidates all previously sealed data. | +| `SALTCORN_IDP_LDAP_PORT` | No (optional override) | Unset. When unset, the port comes from the admin-panel config key `idp_ldap_port`; the listener is disabled unless it is enabled with a valid port (see the LDAP listener section). | Optional override of the panel-configured listener port; when set it wins per-setting and its presence also enables LDAP. Resolved by `resolveRuntime` (`lib/ldap/settings.js`), which `startLdap()` and the self-heal watchdog consult instead of reading `process.env` directly. Parsed as an integer in the range 1-65535. The listener binds only in the cluster primary process. Constant name: `LDAP_PORT_ENV`. | +| `SALTCORN_IDP_LDAP_HOST` | No (optional override) | When unset, the host comes from the admin-panel config key `idp_ldap_host`, which defaults to `127.0.0.1` (constant `LDAP_DEFAULT_HOST`). An empty or whitespace-only value also falls back to the default. | Optional override of the panel-configured bind interface; when set it wins per-setting. Loopback by default so LDAP is not network-exposed; set to `0.0.0.0` to serve LDAP clients on other hosts. Binding beyond loopback logs a network-exposure warning. Resolved by `resolveRuntime` (`lib/ldap/settings.js`). Constant name: `LDAP_HOST_ENV`. | Notes: -- `SALTCORN_SESSION_SECRET` "required" means the plugin cannot seal/unseal data - without it. In a normal Saltcorn deployment the `session_secret` config - provides the fallback, so the explicit environment variable is required only - where that config is not present (for example, code paths outside a request - context). +- `SALTCORN_SESSION_SECRET` is **not** required as an environment variable. The + plugin draws the secret from the Saltcorn installation (the resolved + connection object, the `~/.config/.saltcorn` config file, or the DB config), + so a standard Saltcorn deployment configured through the config file needs no + extra env var for the plugin. Setting the env var still works and takes + priority; the plugin only fails to seal/unseal if none of those sources yield + a secret. +- The `SALTCORN_IDP_LDAP_PORT` / `SALTCORN_IDP_LDAP_HOST` env vars are optional + per-setting overrides of the admin-panel LDAP listener config, not the enabling + mechanism. The listener is normally enabled and configured at `/admin/idp/ldap` + on the public (root) site; an env var, when present, simply wins for its + setting (and a set port also still enables LDAP). The dev instances' `env.sh` + files still set `SALTCORN_IDP_LDAP_PORT` (MAIN `1636`, Postgres `1637`); those + remain valid as overrides. - `SALTCORN_JWT_SECRET` is **not** read by this plugin. It appears in the Postgres dev instance's `env.sh` but is consumed by Saltcorn core, not by `saltcorn-idp`. (Verified: no reference to `SALTCORN_JWT_SECRET` exists in the @@ -49,8 +61,12 @@ These are Saltcorn config values (read/written through `getState().getConfig` / | Config key | Effect | |------------|--------| | `base_url` | Preferred source for OIDC issuer and SAML entityID derivation (`lib/oidc/discovery.js`). When unset, the issuer is derived from the request `Host` header and a warning is logged: `"base_url not set; deriving issuer from request Host ... Set base_url to prevent Host-header issuer poisoning."` Set `base_url` in any multi-tenant or proxied deployment. | -| `session_secret` | Fallback source for the KEK when `SALTCORN_SESSION_SECRET` is unset (`lib/crypto.js`). | +| `session_secret` | Primary source for the KEK. Resolved via `db.connectObj.session_secret` and the `~/.config/.saltcorn` config file (and DB config), so the plugin needs no `SALTCORN_SESSION_SECRET` env var (`lib/crypto.js`). | | `disable_csrf_routes` | The plugin appends `/idp/` to this list during `onLoad` so the machine-driven `/idp` endpoints are CSRF-exempt (`index.js`). Admin pages under `/admin/idp` remain CSRF-protected. | +| `idp_ldap_enabled` | Whether the in-process LDAPS listener is enabled. There is one listener per Saltcorn instance shared by all tenants, so this key is instance-global and lives only in the ROOT/public tenant's `_sc_config`. Set from the public-site admin panel at `/admin/idp/ldap`. `resolveRuntime` (`lib/ldap/settings.js`) treats LDAP as enabled when this is set with a valid port, or when the `SALTCORN_IDP_LDAP_PORT` override is present. | +| `idp_ldap_host` | Bind interface for the listener (instance-global, root-tenant `_sc_config`). Defaults to `127.0.0.1`; overridden by the `SALTCORN_IDP_LDAP_HOST` env var when set. A non-loopback host raises a network-exposure warning in the panel. | +| `idp_ldap_port` | TCP port for the listener (instance-global, root-tenant `_sc_config`), validated to the range 1-65535. Overridden by the `SALTCORN_IDP_LDAP_PORT` env var when set. | +| `idp_ldap_applied` | Restart marker recording the `{enabled, host, port}` actually in effect on the running listener, written by the binding process to root config. The admin panel compares it against the resolved desired settings to drive a "restart Saltcorn to apply" reminder banner; LDAP listener changes apply on restart, and the banner self-clears once applied matches desired. | --- @@ -293,7 +309,8 @@ CSRF-protected browser forms (a hidden `_csrf` field is rendered via | `/admin/idp/saml-sps` | GET | List SAML service providers and the registration form. | | `/admin/idp/saml-sps/create` | POST | Register a SAML SP (rejects empty entityID or empty ACS list). | | `/admin/idp/saml-sps/delete` | POST | Delete a SAML SP by `entity_id`. | -| `/admin/idp/ldap` | GET | Show the configured LDAP service DN and set/clear forms (password never displayed). | +| `/admin/idp/ldap` | GET | Show the configured LDAP service DN and set/clear forms (password never displayed), plus the in-process LDAPS listener config (enable/host/port) and the "restart Saltcorn to apply" banner. The listener config is editable only by a root-tenant admin on the public site; tenant admins see it read-only (one listener per instance). | +| `/admin/idp/ldap/settings` | POST | Save the instance-global listener config (`idp_ldap_enabled` / `idp_ldap_host` / `idp_ldap_port`) to the root tenant's `_sc_config`. Root-tenant + admin only, CSRF-protected; port validated to 1-65535; a non-loopback host raises a network-exposure warning. Applies on restart. | | `/admin/idp/ldap/service` | POST | Set the LDAP service-account DN and password (password sealed at rest). | | `/admin/idp/ldap/service/clear` | POST | Clear the LDAP service account. | diff --git a/docs/ldap.md b/docs/ldap.md index 7734891..bc740f4 100644 --- a/docs/ldap.md +++ b/docs/ldap.md @@ -28,14 +28,55 @@ lib/ldap/harden.js per-IP failed-bind lockout ## Enabling LDAP -LDAP is off unless the port environment variable is set. The listener is -started from the plugin's `onLoad` hook (`startLdap` in `lib/ldap/server.js`), -and only when `SALTCORN_IDP_LDAP_PORT` holds a parseable integer. +LDAP is configured from the admin panel. There is exactly one in-process LDAPS +listener per Saltcorn instance, shared by all tenants, so its enable/host/port +are instance-global settings rather than per-tenant ones: they live in the +ROOT (public) tenant's `_sc_config` under the keys `idp_ldap_enabled`, +`idp_ldap_host`, and `idp_ldap_port`. The listener is started from the plugin's +`onLoad` hook (`startLdap` in `lib/ldap/server.js`), which consults +`resolveRuntime` (`lib/ldap/settings.js`) to decide whether to bind and on +which host/port. + +### Admin panel configuration + +Open `/admin/idp/ldap` on the PUBLIC (root) site to configure the listener: + +- **Enable** -- turns the listener on (`idp_ldap_enabled`). +- **Host** -- the interface the listener binds to (`idp_ldap_host`). Defaults + to loopback (`127.0.0.1`); a non-loopback host raises a network-exposure + warning (see below). +- **Port** -- the LDAPS port (`idp_ldap_port`), validated to the range + 1-65535. + +These settings are root-tenant + admin only. A tenant admin viewing the page +sees them read-only, since the single listener is instance-global and not +something an individual tenant can repoint. The save is CSRF-protected. + +Changes apply on restart. The binding process records the settings it actually +brought up in the root config as `idp_ldap_applied = { enabled, host, port }`, +and the panel compares those applied values against the resolved desired +settings; when they differ it shows a "restart Saltcorn to apply" reminder +banner. The banner self-clears once a restart has brought the applied values in +line with the desired ones. + +### Environment-variable overrides + +`SALTCORN_IDP_LDAP_PORT` and `SALTCORN_IDP_LDAP_HOST` are optional overrides, +never required. `resolveRuntime` applies env over config on a per-setting basis: +when an env value is present it wins for that setting; otherwise the root config +value is used. Presence of the env port also still enables LDAP (so a port-only +override is sufficient to bring the listener up). When neither the panel nor the +env enables it -- or no valid port resolves -- LDAP stays off, with the host +defaulting to loopback. | Variable | Constant | Default | Purpose | |----------|----------|---------|---------| -| `SALTCORN_IDP_LDAP_PORT` | `LDAP_PORT_ENV` | unset (disabled) | Port for the LDAPS listener. Unset or non-numeric means LDAP does not start. Each dev instance uses its own port so they do not collide on one. | -| `SALTCORN_IDP_LDAP_HOST` | `LDAP_HOST_ENV` | `127.0.0.1` (`LDAP_DEFAULT_HOST`) | Interface the listener binds to. An empty or whitespace-only value falls back to the default. | +| `SALTCORN_IDP_LDAP_PORT` | `LDAP_PORT_ENV` | unset (config wins) | Optional override of the LDAPS port. When set it wins and also enables LDAP; when unset the panel's `idp_ldap_port` / `idp_ldap_enabled` decide. Each dev instance uses its own port so they do not collide on one. | +| `SALTCORN_IDP_LDAP_HOST` | `LDAP_HOST_ENV` | `127.0.0.1` (`LDAP_DEFAULT_HOST`) | Optional override of the interface the listener binds to. An empty or whitespace-only value falls back to the config value, then to the default. | + +The dev `env.sh` files still set `SALTCORN_IDP_LDAP_PORT` (MAIN 1636, PG 1637); +those now act as overrides and remain valid -- they are no longer the only way +to enable LDAP. The transport is LDAPS only. `ldapjs` has no StartTLS support, so TLS is required from the first byte; there is no plaintext LDAP on the wire. For @@ -46,7 +87,8 @@ certificate. ### Bind interface and network-exposure warning The default bind host is loopback (`127.0.0.1`), so LDAP is not reachable from -the network unless an operator explicitly opts in (for example by setting +the network unless an operator explicitly opts in (for example by setting the +host to `0.0.0.0` on the admin panel, or overriding it with `SALTCORN_IDP_LDAP_HOST=0.0.0.0`). On a successful listen the server logs: ``` @@ -318,8 +360,10 @@ is needed. `ldapjs` is pinned (the caret dropped) rather than forked; see ## Example: bind and search over loopback -With `SALTCORN_IDP_LDAP_PORT=1636` set on a single-tenant instance (the default -host is loopback), a user can bind and search with OpenLDAP's client tools. The +With the listener enabled on port 1636 -- either from the admin panel at +`/admin/idp/ldap` or via the `SALTCORN_IDP_LDAP_PORT=1636` override -- on a +single-tenant instance (the default host is loopback), a user can bind and +search with OpenLDAP's client tools. The listener is LDAPS only, so use the `ldaps://` scheme. The development certificate is self-signed, so a client will need to disable certificate verification (here via `LDAPTLS_REQCERT=never`). diff --git a/docs/oidc.md b/docs/oidc.md index 13fb7e3..7ef236b 100644 --- a/docs/oidc.md +++ b/docs/oidc.md @@ -240,12 +240,27 @@ Keys live in `_idp_keys` (one per tenant) with a `status` of `active`, key object; `getActivePrivateJwk()` exports that as a private JWK for the Provider config. +Each cluster worker caches its own `oidc-provider` instance with the JWKS baked +in, so a key rotation on one worker would otherwise leave the other workers +serving a stale JWKS missing the new active key. To make a rotation converge +across all workers, the per-worker provider cache is invalidated by a +SIGNING-SET FINGERPRINT: `keys.signingSetFingerprint()` reads the current +`active`-then-`retiring` kids fresh from the DB, and `getProviderEntry` +(`lib/oidc/provider.js`) keys its cache on `(issuer, fingerprint)`, rebuilding +the Provider when the fingerprint no longer matches. After an admin key +rotation, every worker therefore converges on the new active key / JWKS on its +next request -- a relying party load-balanced to any worker verifies +`id_token`s signed with the new key. + Each public/private JWK carries `kid`, `alg`, and `use: "sig"` (set in -`lib/crypto.js`). The private key is sealed under a KEK derived from -`SALTCORN_SESSION_SECRET`; rotating that secret invalidates all sealed material -(KEK derivation and the schema-qualified DDL are documented in -[`./configuration.md`](./configuration.md); the at-rest sealing rationale is in -[`./security.md`](./security.md)). +`lib/crypto.js`). The private key is sealed under a KEK derived from the +session_secret resolved from the Saltcorn installation (the +`SALTCORN_SESSION_SECRET` env var is an optional override, otherwise +`getSessionSecret` in `lib/crypto.js` reads Saltcorn's `connectObj.session_secret`, +the `~/.config/.saltcorn` config file, or DB config); rotating that secret +invalidates all sealed material (KEK derivation and the schema-qualified DDL are +documented in [`./configuration.md`](./configuration.md); the at-rest sealing +rationale is in [`./security.md`](./security.md)). The code does not include an automated rotation routine; the `retiring` / `retire_after` machinery and `getJwks()` publishing retiring keys are the @@ -435,6 +450,6 @@ deployments get fully isolated issuers, JWKS, and client registries. See | Variable | Used for | |----------|----------| -| `SALTCORN_SESSION_SECRET` | Derives the at-rest KEK (sealing private keys and client secrets) and the deterministic `oidc-provider` cookie-signing keys. Required; see the "Environment variables" and "Crypto byte sizes" sections of [`./configuration.md`](./configuration.md). | +| `SALTCORN_SESSION_SECRET` | Optional override for the at-rest KEK secret (sealing private keys and client secrets) and the deterministic `oidc-provider` cookie-signing keys. NOT required: when unset, the secret is resolved from the Saltcorn installation (its `connectObj.session_secret`, the `~/.config/.saltcorn` config file, or DB config), so a plain Saltcorn install configured via the config file needs no extra env var. The env var wins when present. See the "Environment variables" and "Crypto byte sizes" sections of [`./configuration.md`](./configuration.md). | Saltcorn's `base_url` config (not an env var) drives the issuer; see above. diff --git a/docs/operations.md b/docs/operations.md index 31fa5b8..2f91fb7 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -48,7 +48,12 @@ prepend for the in-tree CLI. | `SQLITE_FILEPATH` | `.dev-state/saltcorn.sqlite` | Forces the SQLite backend; DB file location | | `SALTCORN_FILE_STORE` | `.dev-state/files` | Uploaded-file directory | | `SALTCORN_SESSION_SECRET` | `32552b95...410151` | Session/cookie key; also the IdP KEK + oidc cookie-key source | -| `SALTCORN_IDP_LDAP_PORT` | `1636` | Enables the LDAPS listener on 1636. Only MAIN sets this | +| `SALTCORN_IDP_LDAP_PORT` | `1636` | Override for the LDAPS listener port (and, by its presence, enables LDAP on 1636). Only MAIN sets this | + +LDAP is now configured from the public-site admin panel (see +[Configuring the LDAP listener](#configuring-the-ldap-listener) below); these +`SALTCORN_IDP_LDAP_PORT` / `SALTCORN_IDP_LDAP_HOST` env vars are now optional +per-setting overrides, not the only way to enable LDAP. **TEST -- `.dev-state-test/env.sh`** @@ -59,7 +64,8 @@ prepend for the in-tree CLI. | `SALTCORN_SESSION_SECRET` | `cde4d5ce...86265` | Different secret from MAIN | | `SALTCORN_PORT` | `3001` | HTTP listen port (passed via `saltcorn serve -p "$SALTCORN_PORT"`) | -TEST does **not** set `SALTCORN_IDP_LDAP_PORT`, so it runs no LDAP listener. +TEST does **not** set the `SALTCORN_IDP_LDAP_PORT` override and does not enable +LDAP from the admin panel, so it runs no LDAP listener. **PG -- `.dev-state-pg/env.sh`** @@ -70,7 +76,7 @@ TEST does **not** set `SALTCORN_IDP_LDAP_PORT`, so it runs no LDAP listener. | `PGDATABASE` | `saltcorn_idp` | DB name (must already exist) | | `PGPASSWORD` | `peer` | Dummy value; peer auth ignores it, but Saltcorn only selects Postgres when user+password+database are all set (`connect.ts getConnectObject`) | | `SALTCORN_MULTI_TENANT` | `true` | Enables schema-per-tenant (Postgres only) | -| `SALTCORN_IDP_LDAP_PORT` | `1637` | LDAPS listener on 1637 (distinct from MAIN's 1636) | +| `SALTCORN_IDP_LDAP_PORT` | `1637` | Override for the LDAPS listener port, distinct from MAIN's 1636 (and, by its presence, enables LDAP on 1637) | | `SALTCORN_FILE_STORE` | `.dev-state-pg/files` | File store | | `SALTCORN_SESSION_SECRET` | `3ca4fab8...41a` | Different secret from MAIN/TEST | | `SALTCORN_JWT_SECRET` | `d379db38...f158f` | JWT signing secret | @@ -244,6 +250,40 @@ For LDAP, the tenant is encoded in the bind/search DN as an extra `uid=admin@t1.local,ou=people,dc=t1,dc=saltcorn,dc=local`. Single-tenant (SQLite) uses the base DN with no tenant component. See [LDAP](./ldap.md). +## Configuring the LDAP listener + +The in-process LDAPS listener's enable flag, host, and port are configured from +the admin panel on the **public (root) site** at `/admin/idp/ldap`. There is +exactly **one** listener per Saltcorn instance, shared by all tenants, so these +settings are instance-global: they live in the **root/public tenant's** +`_sc_config` under the keys `idp_ldap_enabled`, `idp_ldap_host`, and +`idp_ldap_port`. Tenant admins see them read-only; only a root-tenant admin can +change them. + +`SALTCORN_IDP_LDAP_PORT` and `SALTCORN_IDP_LDAP_HOST` are optional per-setting +overrides -- when set, the env value wins for that setting, and the presence of +the env port also still enables LDAP. They are never required. The dev `env.sh` +files (MAIN 1636, PG 1637) set the port env var, so it acts as the override +there; the admin panel is the supported way to enable/configure LDAP otherwise. + +`resolveRuntime()` (`lib/ldap/settings.js`) computes the effective runtime +settings with this precedence: env over config, defaulting the host to loopback +(`127.0.0.1`), and treating LDAP as disabled unless it is both enabled and has a +valid port. `startLdap` (`lib/ldap/server.js`) and the self-heal watchdog +(`index.js`) consult `resolveRuntime()` instead of reading `process.env` +directly. + +**Changes apply on restart.** The binding process records the settings it +actually applied in the root config as `idp_ldap_applied` +(`{ enabled, host, port }`). The panel compares that against the resolved +desired settings and shows a "restart Saltcorn to apply" reminder banner when +they differ; the banner self-clears after a restart re-applies the settings. + +Security: the panel is root-tenant + admin gated and its save is CSRF-protected; +the port is validated to the 1-65535 range; the host defaults to loopback +(`127.0.0.1`), and a network-exposure warning is shown when the host is set to a +non-loopback address. + ## Known issues ### Intermittent PG LDAP bind flake on :1637 diff --git a/docs/saml.md b/docs/saml.md index 722cb15..105e6e5 100644 --- a/docs/saml.md +++ b/docs/saml.md @@ -285,9 +285,9 @@ DDL: `idp/lib/schema.js` (`createIdpSaml`). `onLoad` hook (per tenant). It is idempotent -- if the `id = "default"` row exists it returns immediately. Otherwise it generates a 2048-bit RSA, SHA-256 self-signed certificate via `selfsigned`, **seals the private key** with - `idpCrypto.sealText(pems.private)` (AES-256-GCM under a KEK derived from - `SALTCORN_SESSION_SECRET`; see [`./security.md`](./security.md)), and inserts the - row. + `idpCrypto.sealText(pems.private)` (AES-256-GCM under a KEK derived from the + Saltcorn session secret, resolved from the installation; see + [`./security.md`](./security.md)), and inserts the row. - **Retrieval:** `getSamlCert()` (`idp.js`) selects the row and unseals the private key with `idpCrypto.openText(...)`, returning `{ cert, key }`. - **Per-tenant isolation:** the constructed IdP entity is cached per issuer in an diff --git a/docs/security.md b/docs/security.md index 29ce823..ee757a7 100644 --- a/docs/security.md +++ b/docs/security.md @@ -21,7 +21,7 @@ surfaces, each with a distinct trust boundary: |---------|-----------|--------------------------|------------------| | OIDC / OAuth2 | Host HTTP(S) stack | Browser session (interactive) + per-client credentials/PKCE | Relying parties are explicitly registered; redirect URIs are allow-listed | | SAML 2.0 | Host HTTP(S) stack | Browser session; per-SP registry + optional AuthnRequest signature | Service providers are explicitly registered; ACS URLs are allow-listed | -| LDAP(S) | Dedicated LDAPS listener | LDAP simple bind (user bcrypt or service account) | Listener is loopback-bound by default; outside Saltcorn's web-login throttling | +| LDAP(S) | Dedicated LDAPS listener | LDAP simple bind (user bcrypt or service account) | Listener defaults to loopback (host/port/enable set in the root admin panel); outside Saltcorn's web-login throttling | Adversary capabilities assumed in scope: @@ -62,10 +62,14 @@ plugin's even though both derive from the same session secret `sealText`/`openText` because Saltcorn's SQLite layer JSON-stringifies values and cannot hold raw Buffers (`crypto.js`, `crypto.js`). -The KEK source resolves in this order (`crypto.js`): the -`SALTCORN_SESSION_SECRET` environment variable, then Saltcorn's `session_secret` -config, otherwise a hard error (`SALTCORN_SESSION_SECRET not available; cannot -derive KEK`). The KEK is cached per process and keyed by a SHA-256 hash of the +The KEK source resolves in this order (`crypto.js`), mirroring Saltcorn's own +`db/connect.ts` precedence so the plugin needs no env var of its own: the +`SALTCORN_SESSION_SECRET` environment variable, then Saltcorn's resolved +connection object (`db.connectObj.session_secret`, which already merges the env +var and the `~/.config/.saltcorn` config file), then the config file read +directly, then the `session_secret` DB config, otherwise a hard error +(`session_secret not available ...; cannot derive KEK`). The KEK is cached per +process and keyed by a SHA-256 hash of the secret, so a rotated secret never keeps serving a stale KEK -- if the secret changes the KEK is re-derived (`crypto.js`). @@ -297,14 +301,36 @@ replicated across a cluster. ### LDAP loopback-default bind and exposure warning The LDAPS listener binds loopback by default and is LDAPS-only (no plaintext, no -StartTLS); it binds only in the cluster primary process. +StartTLS); it binds only in the cluster primary process. There is ONE listener +per Saltcorn instance shared by all tenants, so its enable/host/port are +instance-global and configured from the admin panel on the PUBLIC (root) site at +`/admin/idp/ldap` -- root-tenant + admin only (tenant admins see these settings +read-only). The values persist in the root/public tenant's `_sc_config` under +`idp_ldap_enabled` / `idp_ldap_host` / `idp_ldap_port`. -| Setting | Default | Env var | Source | -|---------|---------|---------|--------| -| Bind host | `127.0.0.1` | `SALTCORN_IDP_LDAP_HOST` | [`lib/constants.js`](../lib/constants.js) | -| Listener enabled / port | (disabled unless set) | `SALTCORN_IDP_LDAP_PORT` | [`lib/constants.js`](../lib/constants.js) | +| Setting | Default | Config source | Env override | Source | +|---------|---------|---------------|--------------|--------| +| Bind host | `127.0.0.1` (loopback) | `idp_ldap_host` (root `_sc_config`) | `SALTCORN_IDP_LDAP_HOST` | [`lib/constants.js`](../lib/constants.js) | +| Listener enabled | off until enabled in the panel (or an env port is set) | `idp_ldap_enabled` (root `_sc_config`) | -- | [`lib/ldap/settings.js`](../lib/ldap/settings.js) | +| Listener port | 1-65535, validated | `idp_ldap_port` (root `_sc_config`) | `SALTCORN_IDP_LDAP_PORT` | [`lib/ldap/settings.js`](../lib/ldap/settings.js) | -If the operator binds to a non-loopback host, the server logs an explicit +The env vars `SALTCORN_IDP_LDAP_PORT` and `SALTCORN_IDP_LDAP_HOST` are now +OPTIONAL per-setting overrides -- never required. `resolveRuntime()` +([`lib/ldap/settings.js`](../lib/ldap/settings.js)) applies env over config, +defaults the host to loopback, and reports the listener disabled unless something +enables it with a valid port (an env port also still enables LDAP for +backward compatibility). `startLdap` ([`lib/ldap/server.js`](../lib/ldap/server.js)) +and the self-heal watchdog ([`index.js`](../index.js)) consult `resolveRuntime()` +rather than reading `process.env` directly. The save path is root-tenant + admin +gated, CSRF-protected, and validates the port range (1-65535). + +Changes apply on RESTART. The binding process records +`idp_ldap_applied = {enabled, host, port}` in the root config, and the panel +shows a "restart Saltcorn to apply" reminder banner -- driven by comparing the +applied snapshot against the resolved desired settings, and self-clearing once +the restart picks up the change. + +If the resolved host is a non-loopback host, the server logs an explicit exposure warning at startup ([`lib/ldap/server.js`](../lib/ldap/server.js)): @@ -382,10 +408,11 @@ decisions are recorded in [`../VENDORING.md`](../VENDORING.md). The plugin defaults to the least-exposed posture, requiring an explicit operator opt-in to widen it: -- LDAP is disabled unless `SALTCORN_IDP_LDAP_PORT` is set, and binds only to - loopback (`127.0.0.1`) unless `SALTCORN_IDP_LDAP_HOST` is changed; a - non-loopback bind logs an explicit network-exposure warning - ([`lib/ldap/server.js`](../lib/ldap/server.js)). +- LDAP is disabled until enabled in the root admin panel (`/admin/idp/ldap`, + root-tenant + admin only) or an `SALTCORN_IDP_LDAP_PORT` override is set, and + binds only to loopback (`127.0.0.1`) unless the host is changed in the panel + or via `SALTCORN_IDP_LDAP_HOST`; a non-loopback bind logs an explicit + network-exposure warning ([`lib/ldap/server.js`](../lib/ldap/server.js)). - LDAP is LDAPS-only -- no plaintext and no StartTLS (VENDORING.md). - SAML and OIDC only issue assertions/tokens to explicitly registered SPs/clients with allow-listed ACS/redirect URIs. diff --git a/docs/testing.md b/docs/testing.md index a4e5e06..701b209 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1,7 +1,7 @@ # Testing -The `saltcorn-idp` test suite is a set of five end-to-end "gates", one per -protocol surface. Each gate is a standalone Node script under `test/` that +The `saltcorn-idp` test suite is a set of nine end-to-end "gates", most of them +one per protocol surface. Each gate is a standalone Node script under `test/` that drives a live, already-running Saltcorn instance over the wire (HTTP, LDAPS, or SAML) and asserts on real responses. There is no mocking: a gate is a black box client, so a passing gate means the deployed plugin actually behaves. @@ -29,15 +29,19 @@ The gates assume the local dev instances described in `mtGate.js` also delete their clients again at the end; `samlGate.js` leaves its registered SP in place (the next run's delete-then-create absorbs it). -## The five gates +## The nine gates | Gate file | Covers | Prerequisites (instance / port) | Backend | Self-skip when port unreachable | Run command | |-----------|--------|---------------------------------|---------|---------------------------------|-------------| -| `test/e2e.js` | OIDC: discovery + JWKS on both instances; client registration via admin UI; authorization-code + PKCE + consent; token + `id_token` RS256 verify; userinfo; groups claim (`role:admin` + custom `group:` claim); confidential-client one-time secret | MAIN `:3000` and TEST `:3001` both up | SQLite | No -- throws (exit 2) if an instance is down | `node test/e2e.js` (or `npm test`) | +| `test/e2e.js` | OIDC: discovery + JWKS on both instances (Phase 0 asserts `>= 1` RSA key, tolerating a retiring key during the grace window); client registration via admin UI; authorization-code + PKCE + consent; token + `id_token` RS256 verify; userinfo; groups claim (`role:admin` + custom `group:` claim); confidential-client one-time secret | MAIN `:3000` and TEST `:3001` both up | SQLite | No -- throws (exit 2) if an instance is down | `node test/e2e.js` (or `npm test`) | +| `test/jwksConsistencyGate.js` | Cross-worker JWKS consistency after rotation: rotate the signing key via the admin UI, then fetch `/idp/jwks` over many fresh connections and assert the new active kid appears in ALL responses and every cluster worker serves an identical key set (regression guard for a worker serving a stale JWKS keyed only by issuer; each worker's `oidc-provider` cache is now keyed on `(issuer, keys.signingSetFingerprint())` and rebuilds on mismatch) | MAIN `:3000` up with multiple cluster workers | SQLite | No -- throws (exit 2) if MAIN is down | `node test/jwksConsistencyGate.js` | | `test/ldapGate.js` | LDAP (Phase 4): LDAPS simple bind vs bcrypt hash; authenticated user search (`mail`, `memberOf`); wrong-password + anonymous-search rejection; case-insensitive attribute selection; `groupOfNames` entries; filter-depth rejection; per-message inbound byte cap (DoS); configurable service account (search-then-bind); RFC 4514 DN escaping + `uidFromDn` unescape (unit) | MAIN `:3000` up AND its LDAPS listener on `:1636` | SQLite | No -- if LDAPS is down the bind checks fail (exit 1) rather than self-skip | `node test/ldapGate.js` | | `test/samlGate.js` | SAML (Phase 5): IdP metadata; SP-initiated SSO (signed assertion + real `AuthnStatement`); SP registry + ACS allow-list (unregistered SP and out-of-allow-list ACS rejected, 403); DTD/ENTITY (XXE) rejection (400); decompression-bomb rejection (400); empty-ACS SP refused at registration; signed-SP AuthnRequest accepted + unsigned rejected (403) for a cert-registered SP; IdP-initiated SSO (no `InResponseTo`); Single Logout incl. no-session and wrong-NameID rejection (403) (LAST -- destroys the session) | MAIN `:3000` up | SQLite | No -- throws (exit 2) if MAIN is down | `node test/samlGate.js` | +| `test/keyRotationGate.js` | Key-rotation lifecycle invariants: an admin rotation mints a NEW active kid and KEEPS the prior key in JWKS (status `retiring`) for the grace window; the rotate endpoint is admin-gated; assertions are invariant-based (not absolute key counts) and all requests are pinned to one cluster worker via a keep-alive agent | MAIN `:3000` up | SQLite | No -- throws (exit 2) if MAIN is down | `node test/keyRotationGate.js` | +| `test/adminGate.js` | Admin defense-in-depth: `javascript:`/`data:` `redirect_uri` and out-of-scheme ACS rejected; an expired SP signing cert rejected at registration; per-session admin-mutation rate-limit returns HTTP 429 over the cap (this check runs LAST as it throttles the session) | MAIN `:3000` up | SQLite | No -- throws (exit 2) if MAIN is down | `node test/adminGate.js` | | `test/mtGate.js` | Multi-tenant OIDC: distinct per-tenant issuers + signing keys; full auth-code + PKCE flow on `t1`; cross-tenant isolation -- a `t1` code, access token, and `id_token` are all REJECTED at `t2` (per-tenant store / Provider / key); positive control that the `t1` token still works at `t1` | PG `:3002` up, tenants `t1` + `t2` present with the plugin installed | Postgres | No explicit skip -- throws (exit 2) if PG/tenants are unavailable | `node test/mtGate.js` (or `npm run test:mt`) | | `test/ldapMultiTenantGate.js` | Multi-tenant LDAP: tenant-in-DN binding (`dc=,dc=saltcorn,dc=local`); correct-tenant bind + search; same uid under another tenant fails; cross-tenant search denied; unknown tenant denied; tenant-scoped `groupOfNames` member DNs. Bootstraps each tenant admin over HTTP first | PG `:3002` up AND its LDAPS listener on `:1637` | Postgres | Yes -- if `127.0.0.1:1637` is not reachable it prints `SKIP` and exits 0 (no-op on SQLite-only setups) | `node test/ldapMultiTenantGate.js` | +| `test/samlMultiTenantGate.js` | Multi-tenant SAML isolation: `t1` and `t2` serve DISTINCT signing certs + issuers in their IdP metadata; a `t1` SP-initiated Response embeds `t1`'s cert and validates against `t1`'s metadata-derived IdP but is REJECTED against `t2`'s | PG `:3002` up, tenants `t1` + `t2` with the plugin installed per tenant | Postgres | Yes -- prints `SKIP` and exits 0 if `:3002` is unreachable | `node test/samlMultiTenantGate.js` | ### Expected pass counts @@ -46,14 +50,19 @@ When run against a correctly configured instance, each gate currently reports: | Gate | Expected passes | |------|-----------------| | `test/e2e.js` | 28 | +| `test/jwksConsistencyGate.js` | 7 | | `test/ldapGate.js` | 22 | | `test/samlGate.js` | 28 | +| `test/keyRotationGate.js` | 8 | +| `test/adminGate.js` | 7 | | `test/mtGate.js` | 15 | | `test/ldapMultiTenantGate.js` | 8 | +| `test/samlMultiTenantGate.js` | 10 | +| **Total** | **133** | -A gate that self-skips (only `ldapMultiTenantGate.js` does this) reports -`0 passed, 0 failed (skipped)` and exits 0; that is not a failure, but it also -does not contribute its 8 passes. +A gate that self-skips (`ldapMultiTenantGate.js` and `samlMultiTenantGate.js` do +this) reports `0 passed, 0 failed (skipped)` and exits 0; that is not a failure, +but it also does not contribute its passes to the total. ### Ordering and shared state @@ -82,8 +91,9 @@ From `package.json`: | `npm test` | `node test/e2e.js` | OIDC e2e gate (MAIN + TEST, SQLite) | | `npm run test:mt` | `node test/mtGate.js` | multi-tenant OIDC gate (PG) | -The three remaining gates (`ldapGate.js`, `samlGate.js`, -`ldapMultiTenantGate.js`) have no npm script; run them directly with `node` as +The remaining seven gates (`ldapGate.js`, `samlGate.js`, `keyRotationGate.js`, +`jwksConsistencyGate.js`, `adminGate.js`, `ldapMultiTenantGate.js`, +`samlMultiTenantGate.js`) have no npm script; run them directly with `node` as shown in the table above. ## Running the full suite @@ -93,7 +103,8 @@ servers you start determines which gates can pass. See [operations.md](./operations.md) for how to start each instance and install the plugin. -1. **SQLite gates (`e2e`, `ldapGate`, `samlGate`).** Start MAIN and TEST: +1. **SQLite gates (`e2e`, `ldapGate`, `samlGate`, `keyRotationGate`, + `jwksConsistencyGate`, `adminGate`).** Start MAIN and TEST: ```bash ./startServer.sh # MAIN -> :3000 (LDAPS :1636) @@ -101,21 +112,34 @@ plugin. ``` - `e2e.js` needs both MAIN and TEST (Phase 0 checks discovery/JWKS on each). - - `ldapGate.js` needs MAIN plus its LDAPS listener on `:1636` (MAIN's - `env.sh` sets `SALTCORN_IDP_LDAP_PORT=1636`). + - `ldapGate.js` needs MAIN plus its LDAPS listener on `:1636`. The listener + host/port/enable are configured from the admin panel (root tenant) at + `/admin/idp/ldap`; MAIN's `env.sh` sets `SALTCORN_IDP_LDAP_PORT=1636` as a + dev override (env wins per-setting, and the env port also enables LDAP), so + the dev instance comes up on `:1636` without touching the panel. - `samlGate.js` needs MAIN only. + - `keyRotationGate.js` needs MAIN only. + - `jwksConsistencyGate.js` needs MAIN only, and is meaningful only when MAIN + runs more than one cluster worker (it rotates the signing key and asserts + every worker converges on the new JWKS). + - `adminGate.js` needs MAIN only; run it LAST, because its rate-limit check + deliberately throttles the admin session (a 429), which would interfere with + other admin-driven gates run after it in the same session. Then: ```bash - npm test # e2e + npm test # e2e node test/ldapGate.js node test/samlGate.js + node test/keyRotationGate.js + node test/jwksConsistencyGate.js + node test/adminGate.js # LAST -- throttles the admin session ``` -2. **Postgres gates (`mtGate`, `ldapMultiTenantGate`).** Start PG and make sure - tenants `t1` and `t2` exist with the plugin installed per tenant (see - operations.md, "PG (multi-tenant): per-tenant install"): +2. **Postgres gates (`mtGate`, `ldapMultiTenantGate`, `samlMultiTenantGate`).** + Start PG and make sure tenants `t1` and `t2` exist with the plugin installed + per tenant (see operations.md, "PG (multi-tenant): per-tenant install"): ```bash ./startServerPg.sh # PG -> :3002 (LDAPS :1637) @@ -126,6 +150,7 @@ plugin. ```bash npm run test:mt # mtGate (HTTP :3002) node test/ldapMultiTenantGate.js # PG LDAPS :1637 + node test/samlMultiTenantGate.js # PG SAML isolation (HTTP :3002) ``` `ldapMultiTenantGate.js` probes `127.0.0.1:1637` first and self-skips diff --git a/index.js b/index.js index 5e354f1..e81fb7c 100644 --- a/index.js +++ b/index.js @@ -9,12 +9,13 @@ const cluster = require("cluster"); const net = require("net"); -const { PLUGIN_NAME, PLUGIN_VERSION, IDP_BASE_PATH, LDAP_PORT_ENV, LDAP_HOST_ENV, LDAP_DEFAULT_HOST } = require("./lib/constants"); +const { PLUGIN_NAME, PLUGIN_VERSION, IDP_BASE_PATH, LDAP_DEFAULT_HOST } = require("./lib/constants"); const { createAllTables } = require("./lib/schema"); const { initEnvIfMissing, markBootstrapped } = require("./lib/env"); const { ensureActiveKey } = require("./lib/keys"); const { routes } = require("./lib/routes"); const { startLdap, isListening } = require("./lib/ldap/server"); +const ldapSettings = require("./lib/ldap/settings"); const { ensureSamlCert } = require("./lib/saml/idp"); // Self-heal delay for the intermittent unbound-LDAP heisenbug. ROOT CAUSE @@ -107,6 +108,15 @@ const onLoad = async (cfg) => { // the bind -- that was the suspected cause of the intermittent unbound-:1637. // Starts only if SALTCORN_IDP_LDAP_PORT is set; the module guard binds once per // process despite per-tenant onLoad calls; retry + loud warning live in server.js. + // Resolve the effective LDAP settings (public-site config, with env override) + // once, for both the bind and the self-heal watchdog below. + let ldapRuntime = null; + try { + ldapRuntime = await ldapSettings.resolveRuntime(); + } catch (e) { + // eslint-disable-next-line no-console + console.error(`[${PLUGIN_NAME}] resolving LDAP settings failed:`, e); + } try { await startLdap(); } catch (e) { @@ -117,12 +127,12 @@ const onLoad = async (cfg) => { // that has LDAP configured (NOT just the primary -- the captured root cause is // that the primary never runs onLoad on the flaky boot). One-shot, staggered by // worker id so the first worker binds and the rest see the port already up. - if (!ldapWatchdogArmed && process.env[LDAP_PORT_ENV]) { + if (!ldapWatchdogArmed && ldapRuntime && ldapRuntime.enabled) { ldapWatchdogArmed = true; - const port = parseInt(process.env[LDAP_PORT_ENV], 10); - const hostEnv = (process.env[LDAP_HOST_ENV] || "").trim(); + const port = ldapRuntime.port; + const host = ldapRuntime.host; // Probe loopback when bound to a wildcard/loopback interface. - const probeHost = (!hostEnv || hostEnv === "0.0.0.0" || hostEnv === "::") ? LDAP_DEFAULT_HOST : hostEnv; + const probeHost = (!host || host === "0.0.0.0" || host === "::") ? LDAP_DEFAULT_HOST : host; const workerId = (cluster.worker && cluster.worker.id) || 0; const delay = LDAP_WATCHDOG_MS + workerId * LDAP_WATCHDOG_STAGGER_MS; const timer = setTimeout(async () => { diff --git a/lib/adminUi.js b/lib/adminUi.js index c8ab771..abe5d28 100644 --- a/lib/adminUi.js +++ b/lib/adminUi.js @@ -10,6 +10,8 @@ const groups = require("./groups"); const clients = require("./clients"); const samlSps = require("./saml/sps"); const serviceAccount = require("./ldap/serviceAccount"); +const ldapSettings = require("./ldap/settings"); +const db = require("@saltcorn/data/db"); const User = require("@saltcorn/data/models/user"); const { escapeHtml } = require("./web"); @@ -471,6 +473,78 @@ const deleteSamlSpHandler = async (req, res) => { }; +// The LDAP listener is process-global (one per instance), so its host/port/enable +// settings are editable only on the root/public site; tenant admins see them +// read-only. isRootTenant() is true on a single-tenant (SQLite) deployment. +const isRootTenant = () => { + if (!db.is_it_multi_tenant || !db.is_it_multi_tenant()) { + return true; + } + const def = String((db.connectObj && db.connectObj.default_schema) || "public").toLowerCase(); + const cur = String((db.getTenantSchema && db.getTenantSchema()) || "").toLowerCase(); + return cur === def; +}; + + +// Whether the running listener (applied) differs from the resolved desired +// settings (runtime) -- i.e. a restart is needed for an edit to take effect. +const ldapAppliedDiffers = (applied, runtime) => { + const a = applied || { enabled: false }; + if (!a.enabled && !runtime.enabled) { + return false; + } + if (!!a.enabled !== !!runtime.enabled) { + return true; + } + return a.host !== runtime.host || a.port !== runtime.port; +}; + + +const LDAP_SETTINGS_ERRORS = { + port: "Port must be an integer between 1 and 65535.", + host: "Bind host contains invalid characters.", + noport: "Enabling the LDAP listener requires a valid port." +}; + + +const ldapListenerSection = async (req) => { + const runtime = await ldapSettings.resolveRuntime(); + const applied = await ldapSettings.getApplied(); + const effective = runtime.enabled ? `${escapeHtml(runtime.host)}:${escapeHtml(String(runtime.port))}` : "disabled"; + const running = (applied && applied.enabled) ? `${escapeHtml(applied.host)}:${escapeHtml(String(applied.port))}` : "disabled"; + const statusTable = ` + + +
Currently running${running}
Effective after restart${effective}
`; + if (!isRootTenant()) { + return `

LDAP listener

+

One LDAPS listener serves all tenants on this Saltcorn instance; it is configured on the public site (read-only here).

+ ${statusTable}`; + } + const stored = await ldapSettings.getStoredConfig(); + const errCode = String((req.query && req.query.err) || ""); + const errBanner = (errCode && LDAP_SETTINGS_ERRORS[errCode]) + ? `

${escapeHtml(LDAP_SETTINGS_ERRORS[errCode])}

` : ""; + const restartBanner = ldapAppliedDiffers(applied, runtime) + ? `

⚠ LDAP settings changed — restart Saltcorn for them to take effect.

` : ""; + const envNote = (runtime.portFromEnv || runtime.hostFromEnv) + ? `

An LDAP environment variable (${escapeHtml(constants.LDAP_PORT_ENV)} / ${escapeHtml(constants.LDAP_HOST_ENV)}) is set and currently OVERRIDES the values below until it is removed.

` : ""; + const exposureWarn = (stored.host && !ldapSettings.isLoopbackHost(stored.host)) + ? `

Warning: bind host ${escapeHtml(stored.host)} is beyond loopback — the listener will be reachable from the network. Ensure this is intended and firewalled.

` : ""; + return `

LDAP listener (this server)

+

One LDAPS listener serves all tenants on this Saltcorn instance. Changes apply on the next Saltcorn restart.

+ ${errBanner}${restartBanner}${envNote} + ${statusTable} +
${csrfField(req)} +

+

Bind host

+

Port

+ ${exposureWarn} + +
`; +}; + + const ldapServicePage = async (req, res) => { if (!isAdmin(req)) { res.status(403).type("text/plain").send("admin only"); @@ -478,7 +552,9 @@ const ldapServicePage = async (req, res) => { } try { const dn = await serviceAccount.getServiceDn(); + const listener = await ldapListenerSection(req); const body = ` + ${listener}

LDAP service account

A service DN + password for the search-then-bind flow (an application binds as this DN, searches for a user, then re-binds as that user to validate the password). The password is sealed at rest and never displayed.

Configured service DN${dn ? `${escapeHtml(dn)}` : '(none)'}
@@ -521,6 +597,40 @@ const clearLdapServiceHandler = async (req, res) => { }; +const setLdapSettingsHandler = async (req, res) => { + if (!requireAdmin(req, res)) { + return; + } + // The listener is process-global; only the public/root site may change it. + if (!isRootTenant()) { + res.status(403).type("text/plain").send("LDAP listener settings are managed on the public site"); + return; + } + const enabled = !!(req.body && (req.body.enabled === "on" || req.body.enabled === "true" || req.body.enabled === true)); + const host = String((req.body && req.body.host) || "").trim(); + const portRaw = String((req.body && req.body.port) || "").trim(); + if (host && !/^[A-Za-z0-9_.:-]+$/.test(host)) { + res.redirect(constants.ADMIN_BASE_PATH + "/ldap?err=host"); + return; + } + let port = null; + if (portRaw !== "") { + const n = parseInt(portRaw, 10); + if (!ldapSettings.validatePort(n)) { + res.redirect(constants.ADMIN_BASE_PATH + "/ldap?err=port"); + return; + } + port = n; + } + if (enabled && port === null) { + res.redirect(constants.ADMIN_BASE_PATH + "/ldap?err=noport"); + return; + } + await ldapSettings.setStoredConfig({ enabled: enabled, host: host, port: port }); + res.redirect(constants.ADMIN_BASE_PATH + "/ldap"); +}; + + const adminRoutes = [ { url: constants.ADMIN_BASE_PATH, method: "get", callback: dashboard }, { url: constants.ADMIN_BASE_PATH + "/", method: "get", callback: dashboard }, @@ -538,7 +648,8 @@ const adminRoutes = [ { url: constants.ADMIN_BASE_PATH + "/saml-sps/delete", method: "post", callback: deleteSamlSpHandler }, { url: constants.ADMIN_BASE_PATH + "/ldap", method: "get", callback: ldapServicePage }, { url: constants.ADMIN_BASE_PATH + "/ldap/service", method: "post", callback: setLdapServiceHandler }, - { url: constants.ADMIN_BASE_PATH + "/ldap/service/clear", method: "post", callback: clearLdapServiceHandler } + { url: constants.ADMIN_BASE_PATH + "/ldap/service/clear", method: "post", callback: clearLdapServiceHandler }, + { url: constants.ADMIN_BASE_PATH + "/ldap/settings", method: "post", callback: setLdapSettingsHandler } ]; diff --git a/lib/constants.js b/lib/constants.js index 398abe9..4daab1d 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -83,6 +83,18 @@ const LDAP_PORT_ENV = "SALTCORN_IDP_LDAP_PORT"; const LDAP_HOST_ENV = "SALTCORN_IDP_LDAP_HOST"; const LDAP_DEFAULT_HOST = "127.0.0.1"; +// Listener settings stored in the ROOT (public) tenant's _sc_config and edited +// from the admin panel on the public site. The env vars above, when set, override +// these but are never required. CFG_LDAP_APPLIED records what the running listener +// bound with so the panel can show a "restart to apply" reminder. +const CFG_LDAP_ENABLED = "idp_ldap_enabled"; +const CFG_LDAP_HOST = "idp_ldap_host"; +const CFG_LDAP_PORT = "idp_ldap_port"; +const CFG_LDAP_APPLIED = "idp_ldap_applied"; +const LDAP_PORT_MIN = 1; +const LDAP_PORT_MAX = 65535; +const LDAP_PRIVILEGED_PORT_MAX = 1023; + // LDAP DoS guards: cap total inbound bytes per connection (the BER parser has no // size limit) and the search-filter nesting depth (the filter parser recurses // without a depth bound). A multi-tenant deployment encodes the tenant as an @@ -162,6 +174,13 @@ module.exports = { LDAP_PORT_ENV, LDAP_HOST_ENV, LDAP_DEFAULT_HOST, + CFG_LDAP_ENABLED, + CFG_LDAP_HOST, + CFG_LDAP_PORT, + CFG_LDAP_APPLIED, + LDAP_PORT_MIN, + LDAP_PORT_MAX, + LDAP_PRIVILEGED_PORT_MAX, LDAP_MAX_MSG_BYTES, LDAP_MAX_FILTER_DEPTH, TABLE_LDAP_SERVICE, diff --git a/lib/crypto.js b/lib/crypto.js index 66b8615..f743ec7 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -29,11 +29,42 @@ let cachedKek = null; let cachedSecretHash = null; +// Resolve the session secret from the Saltcorn installation, mirroring the +// precedence Saltcorn itself uses in db/connect.ts (env var -> ~/.config/.saltcorn +// -> generated). We deliberately lean on Saltcorn's own resolution rather than +// requiring SALTCORN_SESSION_SECRET to be exported into the plugin's process +// environment, so the plugin installs cleanly on an instance configured purely +// through the config file. const getSessionSecret = () => { + // 1. Explicit environment override (also what Saltcorn checks first). const fromEnv = process.env.SALTCORN_SESSION_SECRET; if (fromEnv && fromEnv.length > 0) { return fromEnv; } + // 2. Saltcorn's resolved connection object. session_secret here has already + // been merged from the env var and the .saltcorn config file (and is the + // exact value Saltcorn signs its own session cookies with), so this is the + // single source of truth whenever the db module is initialised. + try { + const db = require("@saltcorn/data/db"); + if (db.connectObj && db.connectObj.session_secret) { + return db.connectObj.session_secret; + } + } catch (e) { + // db module not loadable in this context; fall through + } + // 3. Read the Saltcorn config file (~/.config/.saltcorn) directly, in case + // the db module is not yet initialised when we are called. + try { + const { getConfigFile } = require("@saltcorn/data/db/connect"); + const cfg = getConfigFile(); + if (cfg && cfg.session_secret) { + return cfg.session_secret; + } + } catch (e) { + // connect module not available; fall through + } + // 4. DB-stored config value, if any. try { const { getState } = require("@saltcorn/data/db/state"); const v = getState().getConfig("session_secret"); @@ -43,7 +74,7 @@ const getSessionSecret = () => { } catch (e) { // getState not available (e.g. outside a request context); fall through } - throw new Error("saltcorn-idp: SALTCORN_SESSION_SECRET not available; cannot derive KEK"); + throw new Error("saltcorn-idp: session_secret not available from environment, the Saltcorn config file, or DB config; cannot derive KEK"); }; diff --git a/lib/keys.js b/lib/keys.js index 2587cc8..49301bd 100644 --- a/lib/keys.js +++ b/lib/keys.js @@ -117,6 +117,21 @@ const getSigningPrivateJwks = async () => { }; +// Cheap signature of the PUBLISHED signing set (ACTIVE first, then RETIRING), +// read FRESH from the DB (loadKeys goes direct to _sc_config, never a per-worker +// cache -- see configStore.js). The cached oidc-provider bakes this exact set in +// at build time, so when it changes -- a rotation, or a retire-sweep dropping a +// key -- the fingerprint changes. provider.js compares it per request and rebuilds +// on mismatch, which is how EVERY worker (not just the one that handled the +// rotation) converges on the new signing set / JWKS in a clustered, single-node +// deployment where the rotating worker can only clear its own in-process cache. +const signingSetFingerprint = async () => { + const keys = await loadKeys(); + const ordered = byStatus(keys, constants.KEY_STATUS.ACTIVE).concat(byStatus(keys, constants.KEY_STATUS.RETIRING)); + return ordered.map((k) => k.kid).join(","); +}; + + // Flip any RETIRING keys whose grace window has elapsed to RETIRED, dropping // them from JWKS. Idempotent. A null retire_after never expires. const retireExpiredKeys = async () => { @@ -170,6 +185,7 @@ module.exports = { getActiveSigningKey, getActivePrivateJwk, getSigningPrivateJwks, + signingSetFingerprint, rotateActiveKey, retireExpiredKeys }; diff --git a/lib/ldap/server.js b/lib/ldap/server.js index 31f3a87..ccf127c 100644 --- a/lib/ldap/server.js +++ b/lib/ldap/server.js @@ -18,8 +18,9 @@ const selfsigned = require("selfsigned"); const constants = require("../constants"); const vendor = require("./vendor"); -const bind = require("./bind"); -const search = require("./search"); +const bind = require("./bind"); +const search = require("./search"); +const settings = require("./settings"); let started = false; let listening = false; @@ -74,6 +75,12 @@ const listenWithRetry = (server, host, port) => { }); // eslint-disable-next-line no-console console.log("[saltcorn-idp] LDAPS listening on ldaps://" + host + ":" + port + " base " + constants.LDAP_BASE_DN + " (tenant = dc=,)"); + // Record what we actually bound with so the admin panel can tell whether a + // later settings change still needs a restart to take effect. + settings.setApplied({ enabled: true, host: host, port: port }).catch((e) => { + // eslint-disable-next-line no-console + console.error("[saltcorn-idp] failed to record LDAP applied state:", e && e.message); + }); if (!isLoopbackHost(host)) { // eslint-disable-next-line no-console console.warn("[saltcorn-idp] NOTE: LDAP is bound to " + host + " (beyond loopback) -- it is reachable from the network; ensure this is intended and firewalled appropriately."); @@ -112,31 +119,32 @@ const listenWithRetry = (server, host, port) => { const startLdap = async (opts) => { const force = !!(opts && opts.force); - // Diagnostic line for the intermittent unbound-:1637 heisenbug: capture pid, - // cluster role, the once-per-process started/listening flags, and the port env - // at a normal log level so the next occurrence is recorded (the symptom is a - // silent no-bind: no "LDAPS listening" line, no EADDRINUSE/retry). - // ROOT CAUSE (captured 2026-06-01): on a flaky PG boot EVERY startLdap call is - // isPrimary=false -- the cluster PRIMARY never runs onLoad/startLdap, so nobody - // binds. The self-heal watchdog (index.js) therefore force-binds from whatever - // process notices the port is unbound; force=true bypasses the isPrimary gate. - // eslint-disable-next-line no-console - console.log("[saltcorn-idp] startLdap: pid=" + process.pid + " isPrimary=" + isPrimary() + " force=" + force + " started=" + started + " listening=" + listening + " " + constants.LDAP_PORT_ENV + "=" + (process.env[constants.LDAP_PORT_ENV] || "")); if (started) { return; } - const portStr = process.env[constants.LDAP_PORT_ENV]; - if (!portStr) { + // Settings come from the public-site config (root tenant); the + // SALTCORN_IDP_LDAP_PORT/HOST env vars, when set, override them but are never + // required. resolveRuntime applies that precedence. + const runtime = await settings.resolveRuntime(); + // Diagnostic line for the intermittent unbound-:1637 heisenbug (captured + // 2026-06-01: on a flaky PG boot EVERY startLdap call is isPrimary=false, so the + // cluster PRIMARY never binds and the index.js watchdog force-binds from whatever + // process notices). force=true bypasses the isPrimary gate. + // eslint-disable-next-line no-console + console.log("[saltcorn-idp] startLdap: pid=" + process.pid + " isPrimary=" + isPrimary() + " force=" + force + " started=" + started + " listening=" + listening + " enabled=" + runtime.enabled + " host=" + runtime.host + " port=" + runtime.port + (runtime.portFromEnv ? " (port from env)" : "")); + if (!runtime.enabled) { + // LDAP off (no config and no env override). Record the applied state so the + // admin panel's restart reminder is accurate, then do nothing. + await settings.setApplied({ enabled: false }); return; } - const port = parseInt(portStr, 10); - if (!Number.isFinite(port)) { + const port = runtime.port; + const host = runtime.host; + if (!settings.validatePort(port)) { + // eslint-disable-next-line no-console + console.error("[saltcorn-idp] LDAP enabled but port " + port + " is out of range (" + constants.LDAP_PORT_MIN + "-" + constants.LDAP_PORT_MAX + "); not binding"); return; } - // Bind interface is configurable; default to loopback so LDAP is not exposed - // unless an operator opts in. An empty/whitespace value falls back to default. - const hostEnv = (process.env[constants.LDAP_HOST_ENV] || "").trim(); - const host = hostEnv || constants.LDAP_DEFAULT_HOST; // Bind only in the cluster primary; forked workers return silently -- UNLESS // forced by the watchdog (the primary never bound, so a worker heals it; the // EADDRINUSE retry in listenWithRetry arbitrates if two workers race). diff --git a/lib/ldap/settings.js b/lib/ldap/settings.js new file mode 100644 index 0000000..8605ea1 --- /dev/null +++ b/lib/ldap/settings.js @@ -0,0 +1,131 @@ +// Settings for the process-global LDAPS listener (enable / bind host / port). +// +// There is ONE listener per Saltcorn instance, shared by all tenants (see +// lib/ldap/tenant.js), so its settings are INSTANCE-global, not per-tenant: they +// live in the ROOT (public) tenant's _sc_config and are edited from the admin +// panel on the public site. The SALTCORN_IDP_LDAP_PORT / SALTCORN_IDP_LDAP_HOST +// environment variables, WHEN SET, override the stored value (an ops/container +// escape-hatch and backward-compat), but they are never required: a fresh +// instance is configured entirely from the panel. +// +// resolveRuntime() is what the bind path (server.js) and the self-heal watchdog +// (index.js) consult. getApplied()/setApplied() record what the running listener +// actually bound with, so the panel can show a "restart to apply" reminder. + +const db = require("@saltcorn/data/db"); + +const constants = require("../constants"); +const { readKey, writeKey } = require("../configStore"); + + +const rootSchema = () => String((db.connectObj && db.connectObj.default_schema) || "public").toLowerCase(); + + +// Run fn in the ROOT/public tenant context so listener config is global to the +// instance regardless of which tenant's request/onLoad we are serving. A +// single-tenant (SQLite) deployment has only one context, so run directly. +const inRoot = (fn) => { + if (db.is_it_multi_tenant && db.is_it_multi_tenant()) { + return db.runWithTenant(rootSchema(), fn); + } + return fn(); +}; + + +const parseEnvPort = (raw) => { + if (raw === undefined || raw === null || String(raw).trim() === "") { + return null; + } + const n = parseInt(String(raw), 10); + return Number.isFinite(n) ? n : null; +}; + + +const isLoopbackHost = (host) => { + const h = String(host || "").trim().toLowerCase(); + return h === "127.0.0.1" || h === "::1" || h === "localhost"; +}; + + +const validatePort = (n) => { + return Number.isInteger(n) && n >= constants.LDAP_PORT_MIN && n <= constants.LDAP_PORT_MAX; +}; + + +// The values stored in the public-site config (no env applied). +const getStoredConfig = async () => { + return inRoot(async () => { + const enabled = await readKey(constants.CFG_LDAP_ENABLED); + const host = await readKey(constants.CFG_LDAP_HOST); + const port = await readKey(constants.CFG_LDAP_PORT); + return { + enabled: enabled === true, + host: typeof host === "string" ? host : "", + port: Number.isFinite(port) ? port : null + }; + }); +}; + + +const setStoredConfig = async (cfg) => { + await inRoot(async () => { + await writeKey(constants.CFG_LDAP_ENABLED, cfg.enabled === true); + await writeKey(constants.CFG_LDAP_HOST, typeof cfg.host === "string" ? cfg.host.trim() : ""); + await writeKey(constants.CFG_LDAP_PORT, Number.isFinite(cfg.port) ? cfg.port : null); + }); +}; + + +// Effective runtime settings: env overrides stored config PER SETTING (set => +// wins), but env is never required. The listener runs iff enabled with a valid port. +const resolveRuntime = async () => { + const stored = await getStoredConfig(); + const envPort = parseEnvPort(process.env[constants.LDAP_PORT_ENV]); + const envHost = (process.env[constants.LDAP_HOST_ENV] || "").trim(); + const port = envPort !== null ? envPort : stored.port; + const host = envHost || stored.host || constants.LDAP_DEFAULT_HOST; + // Presence of an env port preserves the legacy "env enables LDAP" behaviour. + const enabled = envPort !== null ? true : stored.enabled; + return { + enabled: enabled && Number.isFinite(port), + host: host, + port: Number.isFinite(port) ? port : null, + portFromEnv: envPort !== null, + hostFromEnv: !!envHost + }; +}; + + +// What the running listener actually bound with (or { enabled: false } when off). +// The admin panel compares this against resolveRuntime() to decide whether a +// settings change still needs a restart to take effect. +const getApplied = async () => { + const a = await inRoot(() => readKey(constants.CFG_LDAP_APPLIED)); + return a && typeof a === "object" ? a : null; +}; + + +const setApplied = async (applied) => { + const norm = { + enabled: applied.enabled === true, + host: applied.enabled === true ? applied.host : null, + port: applied.enabled === true ? applied.port : null + }; + // Avoid write churn (every onLoad/worker would otherwise rewrite it). + const cur = await getApplied(); + if (cur && cur.enabled === norm.enabled && cur.host === norm.host && cur.port === norm.port) { + return; + } + await inRoot(() => writeKey(constants.CFG_LDAP_APPLIED, norm)); +}; + + +module.exports = { + getStoredConfig, + setStoredConfig, + resolveRuntime, + getApplied, + setApplied, + isLoopbackHost, + validatePort +}; diff --git a/lib/oidc/provider.js b/lib/oidc/provider.js index 6ef6235..bb4541b 100644 --- a/lib/oidc/provider.js +++ b/lib/oidc/provider.js @@ -97,22 +97,31 @@ const buildProvider = async (issuer) => { const getProviderEntry = async (req) => { const issuer = issuerForReq(req); + // The per-worker cache is invalidated by the current signing-set fingerprint + // (read fresh from the DB), not just by issuer. In a clustered single-node + // deployment a rotation on ONE worker updates the shared DB but can only clear + // its OWN in-process cache; every other worker would keep serving the stale + // baked-in JWKS until restarted. Comparing the fingerprint here makes each + // worker rebuild on its next request, so all workers converge on the rotated + // signing key and JWKS. loadKeys() is a direct _sc_config read, so the + // fingerprint reflects writes from any worker immediately. + const fingerprint = await keys.signingSetFingerprint(); let entry = providersByIssuer.get(issuer); - if (!entry) { + if (!entry || entry.fingerprint !== fingerprint) { const provider = await buildProvider(issuer); - entry = { provider: provider, handler: provider.callback() }; + entry = { provider: provider, handler: provider.callback(), fingerprint: fingerprint }; providersByIssuer.set(issuer, entry); } return entry; }; -// Drop the cached Provider(s) so the next request rebuilds with the current -// active signing key. Each Provider is constructed with the active private JWK -// baked in (jwks config), so after a key rotation the cached instances would -// keep signing id_tokens with the OLD key and serving the OLD JWKS; clearing the -// cache forces a rebuild. The key material itself lives in the DB, so a rebuilt -// Provider picks up the freshly-rotated key. Called by keys.rotateActiveKey(). +// Drop this worker's cached Provider(s) so its next request rebuilds with the +// current active signing key. Called by keys.rotateActiveKey() as a local +// fast-path: the rotating worker rebuilds immediately rather than waiting for the +// fingerprint comparison in getProviderEntry to notice the change. OTHER workers +// are handled by that fingerprint check (this clear only affects the calling +// process), so cross-worker convergence does not depend on this call. const clearProviderCache = () => { providersByIssuer.clear(); }; diff --git a/test/e2e.js b/test/e2e.js index 03d84c1..9dda30e 100644 --- a/test/e2e.js +++ b/test/e2e.js @@ -228,9 +228,11 @@ const run = async () => { const disc = await getJson(port, "/idp/.well-known/openid-configuration"); ok(disc.issuer === "http://localhost:" + port + "/idp", label + " issuer = " + disc.issuer); const jwks = await getJson(port, "/idp/jwks"); - const key = jwks.keys && jwks.keys[0]; - ok(jwks.keys && jwks.keys.length === 1 && key.kty === "RSA", label + " JWKS: one RSA key"); - ok(key && !key.d && !key.p && !key.q, label + " JWKS public-only"); + const jkeys = (jwks.keys) || []; + // >= 1 (not == 1): a prior key rotation legitimately keeps the rotated-out + // key in JWKS as RETIRING during its grace window, so the count is not fixed. + ok(jkeys.length >= 1 && jkeys.every((k) => k.kty === "RSA"), label + " JWKS: >= 1 RSA key (" + jkeys.length + ")"); + ok(jkeys.every((k) => !k.d && !k.p && !k.q), label + " JWKS public-only (all keys)"); } section("Phase 1: client registration + auth-code + PKCE + consent"); diff --git a/test/jwksConsistencyGate.js b/test/jwksConsistencyGate.js new file mode 100644 index 0000000..297effb --- /dev/null +++ b/test/jwksConsistencyGate.js @@ -0,0 +1,156 @@ +// Cross-worker JWKS consistency gate (MAIN :3000). +// +// MAIN runs a multi-worker cluster, and each worker caches its OWN oidc-provider +// with the JWKS baked in (lib/oidc/provider.js). A key rotation must become visible +// in the JWKS served by EVERY worker -- not only the one that handled the rotate +// POST -- otherwise a relying party load-balanced to a stale worker cannot verify +// id_tokens signed with the new active key. This gate guards the fingerprint-based +// provider-cache invalidation that makes every worker converge after a rotation. +// +// It rotates once over a PINNED connection (one worker), then fetches /idp/jwks +// many times over FRESH connections so the cluster spreads them across workers, and +// asserts the new active kid (and the retained rotated-out kid) appears in EVERY +// response and that all workers serve an identical set. +// +// Run after a coherent boot: node idp/test/jwksConsistencyGate.js (MAIN up; admin@local). + +const http = require("http"); + +const PORT = 3000; +const ADMIN_EMAIL = "admin@local"; +const ADMIN_PW = "AdminP@ss1"; +const SPREAD_REQUESTS = 40; // fresh connections -- enough to hit every cluster worker + +const jar = {}; +let pass = 0; +let fail = 0; + + +const ok = (cond, msg) => { + if (cond) { + pass++; + console.log(" PASS " + msg); + } else { + fail++; + console.log(" FAIL " + msg); + } +}; + + +// Auth + rotate are pinned to ONE worker (keep-alive) so the session and CSRF token +// stay consistent. The JWKS reads below deliberately use fresh connections to fan +// out across the cluster. +const pinAgent = new http.Agent({ keepAlive: true, maxSockets: 1 }); + + +const storeCookies = (headers) => { + const sc = headers["set-cookie"]; + if (!sc) { + return; + } + for (const line of sc) { + const pair = line.split(";")[0]; + const eq = pair.indexOf("="); + if (eq > 0) { + jar[pair.slice(0, eq).trim()] = pair.slice(eq + 1).trim(); + } + } +}; + + +const request = (method, path, opts) => { + const options = opts || {}; + return new Promise((resolve, reject) => { + const headers = Object.assign({}, options.headers || {}); + if (!options.noCookies && Object.keys(jar).length > 0) { + headers["Cookie"] = Object.keys(jar).map((k) => k + "=" + jar[k]).join("; "); + } + let data = null; + if (options.body) { + data = new URLSearchParams(options.body).toString(); + headers["Content-Type"] = "application/x-www-form-urlencoded"; + headers["Content-Length"] = Buffer.byteLength(data); + } + // fresh -> a brand-new connection per request (the cluster round-robins it + // to some worker); otherwise reuse the pinned keep-alive connection. + const agent = options.fresh ? false : pinAgent; + const r = http.request({ host: "localhost", port: PORT, method: method, path: path, headers: headers, agent: agent }, (resp) => { + storeCookies(resp.headers); + let body = ""; + resp.on("data", (c) => { body += c; }); + resp.on("end", () => resolve({ status: resp.statusCode, headers: resp.headers, body: body })); + }); + r.on("error", reject); + if (data !== null) { + r.write(data); + } + r.end(); + }); +}; + + +const csrfOf = (h) => (h.match(/name="_csrf" value="([^"]+)"/) || [])[1] || ""; +const activeKidFrom = (html) => (html.match(/Active signing key \(kid\)<\/th>([^<]+)<\/code>/) || [])[1] || ""; + + +const jwksKids = async () => { + const r = await request("GET", "/idp/jwks", { fresh: true }); + try { + return (JSON.parse(r.body).keys || []).map((k) => k.kid).filter(Boolean); + } catch (e) { + return []; + } +}; + + +const run = async () => { + const lp = await request("GET", "/auth/login"); + await request("POST", "/auth/login", { body: { email: ADMIN_EMAIL, password: ADMIN_PW, _csrf: csrfOf(lp.body) } }); + ok(/true/.test((await request("GET", "/auth/authenticated")).body), "admin session authenticated"); + + const adminBefore = await request("GET", "/admin/idp"); + const beforeActive = activeKidFrom(adminBefore.body); + ok(!!beforeActive, "read pre-rotation active kid (" + beforeActive + ")"); + + const rot = await request("POST", "/admin/idp/rotate-key", { body: { _csrf: csrfOf(adminBefore.body) } }); + ok(rot.status === 302, "admin rotate-key returned 302 (HTTP " + rot.status + ")"); + + const afterActive = activeKidFrom((await request("GET", "/admin/idp")).body); + ok(!!afterActive && afterActive !== beforeActive, "rotation minted a new active kid (" + beforeActive + " -> " + afterActive + ")"); + + // Fan out JWKS reads across workers. EVERY response must carry the new active + // kid (so no worker serves a stale JWKS) and the rotated-out kid (grace window), + // and all responses must be the identical set. + let withNew = 0; + let withOld = 0; + const seenSets = new Set(); + for (let i = 0; i < SPREAD_REQUESTS; i++) { + const kids = await jwksKids(); + seenSets.add(kids.slice().sort().join(",")); + if (kids.includes(afterActive)) { + withNew++; + } + if (kids.includes(beforeActive)) { + withOld++; + } + } + ok(withNew === SPREAD_REQUESTS, "new active kid in ALL " + SPREAD_REQUESTS + " JWKS responses across workers (" + withNew + "/" + SPREAD_REQUESTS + ")"); + ok(withOld === SPREAD_REQUESTS, "rotated-out kid retained in ALL JWKS responses for the grace window (" + withOld + "/" + SPREAD_REQUESTS + ")"); + ok(seenSets.size === 1, "every worker served an IDENTICAL JWKS set (distinct sets seen: " + seenSets.size + ")"); + + console.log("\nRESULT: " + pass + " passed, " + fail + " failed"); + process.exit(fail ? 1 : 0); +}; + + +const main = async () => { + try { + await run(); + } catch (e) { + console.error("JWKS CONSISTENCY GATE ERROR:", e); + process.exit(2); + } +}; + + +main(); diff --git a/test/keyRotationGate.js b/test/keyRotationGate.js index 437fdac..d19cb62 100644 --- a/test/keyRotationGate.js +++ b/test/keyRotationGate.js @@ -14,6 +14,13 @@ const jar = {}; let pass = 0; let fail = 0; +// MAIN runs a multi-worker cluster, and idp signing keys live in per-worker +// config cache (single-node SQLite => no cross-worker config propagation). So +// /admin/idp and /idp/jwks can be served by DIFFERENT workers with divergent key +// views, which would make rotation assertions race. Pin every request in this gate +// to ONE worker by funnelling them over a single keep-alive connection. +const agent = new http.Agent({ keepAlive: true, maxSockets: 1 }); + const ok = (cond, msg) => { if (cond) { @@ -54,7 +61,7 @@ const request = (method, path, opts) => { headers["Content-Type"] = "application/x-www-form-urlencoded"; headers["Content-Length"] = Buffer.byteLength(data); } - const r = http.request({ host: "localhost", port: PORT, method: method, path: path, headers: headers }, (resp) => { + const r = http.request({ host: "localhost", port: PORT, method: method, path: path, headers: headers, agent: agent }, (resp) => { storeCookies(resp.headers); let body = ""; resp.on("data", (c) => { body += c; }); @@ -70,6 +77,10 @@ const request = (method, path, opts) => { const csrfOf = (h) => (h.match(/name="_csrf" value="([^"]+)"/) || [])[1] || ""; +// The admin dashboard renders the ACTIVE signing kid (adminUi.js). Parse it so the +// gate can assert rotation invariants (which key is active / retained) rather than +// fragile absolute JWKS counts. +const activeKidFrom = (html) => (html.match(/Active signing key \(kid\)<\/th>([^<]+)<\/code>/) || [])[1] || ""; const jwksKids = async () => { const r = await request("GET", "/idp/jwks"); let keys = []; @@ -92,22 +103,34 @@ const run = async () => { await request("POST", "/auth/login", { body: { email: ADMIN_EMAIL, password: ADMIN_PW, _csrf: csrfOf(lp.body) } }); ok(/true/.test((await request("GET", "/auth/authenticated")).body), "admin session authenticated"); + // Capture the ACTIVE kid before rotating. Visiting /admin/idp also opportunistically + // reaps RETIRING keys whose grace window has elapsed (adminUi.js), so the ABSOLUTE + // JWKS count is not stable across runs against a long-lived instance -- assert the + // rotation INVARIANTS instead of count deltas. + const adminBefore = await request("GET", "/admin/idp"); + const beforeActive = activeKidFrom(adminBefore.body); const before = await jwksKids(); ok(before.length >= 1, "JWKS serves at least one signing key before rotation (" + before.length + ")"); + ok(!!beforeActive && before.includes(beforeActive), "pre-rotation active kid is published in JWKS (" + beforeActive + ")"); - const csrf = csrfOf((await request("GET", "/admin/idp")).body); - const rot = await request("POST", "/admin/idp/rotate-key", { body: { _csrf: csrf } }); + const rot = await request("POST", "/admin/idp/rotate-key", { body: { _csrf: csrfOf(adminBefore.body) } }); ok(rot.status === 302, "admin rotate-key returned 302 (HTTP " + rot.status + ")"); const after = await jwksKids(); + const afterActive = activeKidFrom((await request("GET", "/admin/idp")).body); const beforeSet = new Set(before); const afterSet = new Set(after); const newKids = after.filter((k) => !beforeSet.has(k)); - const retained = before.every((k) => afterSet.has(k)); - ok(retained, "ALL pre-rotation kids remain in JWKS (rotated-out key kept as RETIRING for the grace window)"); + // Invariants that hold regardless of how many already-expired keys were reaped: + // * rotation mints exactly one brand-new kid, which becomes the active key; + // * the rotated-out (previously active) key is RETAINED in JWKS for its grace + // window (it is freshly demoted to RETIRING, so it cannot be reaped this pass). ok(newKids.length === 1, "exactly ONE new kid appeared in JWKS after rotation (" + newKids.join(",") + ")"); - ok(after.length === before.length + 1, "JWKS grew by exactly one key (" + before.length + " -> " + after.length + ")"); + ok(!!afterActive && afterActive !== beforeActive && newKids[0] === afterActive, + "rotation promoted a brand-new active kid (" + beforeActive + " -> " + afterActive + ")"); + ok(afterSet.has(beforeActive), + "rotated-out (previously active) key RETAINED in JWKS for the grace window (" + beforeActive + ")"); console.log("\nRESULT: " + pass + " passed, " + fail + " failed"); process.exit(fail ? 1 : 0);