Skip to content

Commit b58beb8

Browse files
fix(plugin-auth)!: an allowed IP range that names no range is refused at save and never matches at the gate (#30717)
Clause-②: no (narrowing) Metadata-protocol: none Skills-docs: updated here — `content/docs/permissions/authentication.mdx` (the `allowed_ip_ranges` row, and the first two rules under "Saving, clearing and lockout recovery") and `docs/HARDENING.md` (the Network row); `skills/**` teaches no `allowed_ip_ranges` ## 维护者速读 - 改了什么:`auth.allowed_ip_ranges`(IP 白名单)的每一项只剩一种读法:一个地址,或「地址 / 前缀长度」,前缀只能是十进制数字且落在本地址族范围内(IPv4 为 0 到 32,IPv6 为 0 到 128)。设置值本身必须是文本(每行一项或逗号分隔),列表、数字、对象等非文本值不构成任何列表。登录网关、设置保存检查、部署代码里声明的列表三处共用这一读法。 - 为什么改:读不懂的条目或取值应当在保存时就被拒收,在网关处不放行任何地址;三处此前没有共用一条严格的规则。 - 风险与回滚:写法规范的条目含义不变。含不规范条目的部署:在设置里保存会被拒(400),代码里声明会在启动时报错,已存的或环境变量里的不规范条目不再放行任何人,并在启动时以 error 日志点名该条目。已存的非文本取值不再放行任何地址,启动时以 error 日志说明并给出修法。回滚即 revert 本 PR。 - 先看:`packages/plugins/plugin-auth/src/auth-manager.ts`(`parseIpRange` 与 `ipMatchesRange`),`allowed-ip-ranges.ts`(`readAllowedIpRanges`;保存检查先判取值形式,再判条目),`auth-plugin.ts`(设置绑定时点名不规范条目)。 - 两分钟验证:`pnpm --filter @objectstack/plugin-auth exec vitest run src/allowed-ip-ranges.pin.test.ts`,预期全部通过。 - 需要你做的:无,席位落地。 ## What changes An allowed IP range entry now has one reading, `parseIpRange` in `auth-manager.ts`, shared by every door that judges or saves the list. An entry is an IPv4 or IPv6 address, or an address, `/` and a prefix length written in decimal digits: one or two digits, from 0 to 32, for an IPv4 base (an IPv4-mapped base included), and one to three digits, from 0 to 128, for an IPv6 base. Blanks around the entry are not part of it. Any other entry names no range. Every well-formed entry keeps its meaning. The setting holds its entries as text, one per line or comma-separated, and `readAllowedIpRanges` in `allowed-ip-ranges.ts` is the one reading of a whole value: a value that is not text (a list, a number, an object) names no list. - **Sign-in gate** (`ipMatchesRange`, `AuthManager.isClientIpAllowed`): an entry that names no range matches no address, its own base address included, so a list holding one admits exactly what its other entries admit. - **Settings save** (`allowedIpRangesWritePredicate`): a value that is not text is refused first, with `400 SETTINGS_VALIDATION`, `invalid_value` at `allowed_ip_ranges`, `constraint: { type: 'textarea' }` and a message in the caller's language. A list holding an entry that names no range is refused with `400 SETTINGS_VALIDATION`, `invalid_value` at `allowed_ip_ranges`, the entries in `constraint.invalidEntries`, and a message in the caller's language (every language on the platform locale table) naming them. The value and its entries are judged before the saving address, on every write: the console's `PUT /api/settings/auth` and in-process `SettingsService` writes alike. - **Declared list** (`AuthManager` constructor, `assertAllowedIpRanges`): an `allowedIpRanges` from the plugin options or an `auth:configure` handler that holds one, or is not an array, is refused at construction, naming each entry. - **Applied list** (`bindAuthSettings`): a stored list or an `OS_AUTH_ALLOWED_IP_RANGES` value holding one is applied (the entry matches nothing), and the binding logs one `error` per distinct list naming the entry and the fix. A stored value that is not text is applied as a list that matches no address, so the gate admits nobody while it is in force, and the binding logs one `error` per distinct value naming the fix. - **`AuthManager.applyConfigPatch`**: its docblock states that an entry naming no range is applied, not refused, matches no address, and is reported by the settings binding rather than by the method. - **Docs**: the authentication guide's `allowed_ip_ranges` row and its saving rules, and the hardening guide's Network row. ## Tests - `packages/plugins/plugin-auth/src/auth-manager.test.ts`, in the existing `IP allow-list (ADR-0069 D5)` describe: - `ipMatchesRange` matches no address through an IPv4 entry whose prefix is not 1-2 decimal digits from 0 to 32, nor through an IPv6 entry whose prefix is not 1-3 decimal digits from 0 to 128, nor through an entry that is not an address (each judged against the entry's own base address, an address inside the range it would name, and one outside). - Well-formed controls keep their reading: IPv4 and IPv6 CIDR, `/32`, `/128`, bare IPv4 and IPv6 addresses, `0.0.0.0/0`, `::/0`, an IPv4-mapped base, a zone identifier, surrounding blanks. - `isClientIpAllowed` admits no client through an unreadable entry, and still admits through the readable ones in the same list. - The constructor refuses a declared `allowedIpRanges` holding an unreadable entry or a non-string, naming each one, and accepts a well-formed list. - The former case "a prefix the IPv6 family does not read leaves the base address alone" is replaced by the rule above: such an entry now matches nothing. - `packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts`, a new top-level describe on the existing harness (the real `AuthPlugin`, the real `SettingsService` with the shipped auth manifest, the real settings routes): - The console save (`PUT /api/settings/auth`) refuses a list holding an unreadable entry, alone or beside an entry matching the saving address: `400`, `error.code` `SETTINGS_VALIDATION`, field `allowed_ip_ranges` with `code` `invalid_value` and `constraint` `{ invalidEntries: [...] }`, the message naming the entry, nothing stored. - The entry refusal comes first, whatever address the save comes from (including an address the server cannot establish), with the same `400`, `error.code` and field `code` as the first case. - An in-process `SettingsService.set` is refused with the same `SETTINGS_VALIDATION` / `invalid_value` / `constraint`. - A declared `allowedIpRanges` holding one refuses the boot, naming it. - A list stored before the save check, and an `OS_AUTH_ALLOWED_IP_RANGES` value, holding one: the gate admits nobody through it, the readable entry still admits, and the boot logs at `error` naming it. - Preservation: a list of well-formed IPv4, IPv6, `/32`, `/128`, bare-address and two-digit-prefix entries saves and gates as before. - `packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts`, a second new top-level describe on the same harness, for a value that is not text (`['10.1.2.3']`, a two-entry list, `[]`, a number, `true`, an object): - The console save refuses it, saved from an address it spells: `400`, `error.code` `SETTINGS_VALIDATION`, field `allowed_ip_ranges` with `code` `invalid_value` and `constraint` `{ type: 'textarea' }`, nothing stored. The same holds whatever address the save comes from. - An in-process `SettingsService.set` of it is refused with the same `SETTINGS_VALIDATION` / `invalid_value` / `constraint`. - Stored before the save check (a list, a number, an object): the gate admits no client, the boot logs at `error`, and a save of the list as text replaces it and gates as written. - No dogfood pin: no dogfood harness covers `allowed_ip_ranges` (no hit for `allowed_ip_ranges`, `allowedIpRanges`, `isClientIpAllowed` or `IP_NOT_ALLOWED` under `packages/qa`). ## Acceptance notes - **`/0` is kept.** `0.0.0.0/0` admits every IPv4 address and `::/0` every IPv6 address, as before; each still refuses the other family and a request whose address cannot be established, so it is not the same as an empty list. - **Bits past the prefix are not refused.** `203.0.113.5/24` keeps reading as `203.0.113.0/24` (at `/0` too). Refusing them would change the meaning of entries that save today. - **Leading zeros inside the digit count read decimal, as before** (`/08` is `/8`; `/032` is `/32` on an IPv6 base). A three-digit IPv4 prefix is refused. - **An IPv4-mapped base takes an IPv4 prefix, as before** (`::ffff:203.0.113.0/24`). An IPv6-sized prefix on it (`/120`) is refused. - **`applyConfigPatch({ allowedIpRanges })` does not refuse.** There a refusal would keep the standing list in force instead of the one the settings binding applies, and the standing list can admit addresses the stored one does not. The entry matches nothing at the gate instead, and the binding names it. The declared list (plugin options, `auth:configure`) is refused at construction. The method's docblock now says so, and that it logs nothing itself. - **The value's form is the textarea's.** The manifest declares `allowed_ip_ranges` as a `textarea`, so the save accepts text or a clear (`null`) and nothing else, an empty list `[]` included: no other form is read anywhere. The refusal's `constraint` is `{ type: 'textarea' }`, the specifier type the value must take, spelled as the service's own format refusals spell it. A stored value of another form is applied closed rather than read as entries, because no door declares how such a value reads. `OS_AUTH_ALLOWED_IP_RANGES` cannot produce one: the service reads it as text, since the key declares no default to coerce by. - **The save doors, enumerated.** The settings `PUT /api/settings/auth` and every in-process `SettingsService.set`/`setMany` reach the write predicate. `sys_setting` and `sys_platform_setting` are read-only on the data API (`apiMethods: ['get', 'list']`), so no other API writes the row. `OS_AUTH_ALLOWED_IP_RANGES` is read, not saved: the gate and the binding's log hold for it. No spec or stack key carries the list (`packages/spec/src` names `allowed_ip_ranges` only in the `server.trustProxy` description); a host's `allowedIpRanges` reaches `AuthManager` only through its constructor. - **The matcher's other callers pass constants** (the plain-HTTP private/loopback list and `isLoopbackIssuer`'s `127.0.0.0/8`), all well-formed, and read as before. - `ipMatchesRange` stays the one published name; `parseIpRange`, `unreadableIpRangeEntries`, `assertAllowedIpRanges` and `readAllowedIpRanges` are module helpers, not added to the package entry. - **The merges of `main`.** The branch merged `origin/main` `4564d6fa5b`. The one conflict, in the settings binding of `auth-plugin.ts`, was resolved by keeping both sides: `main`'s restore of a cleared setting to the deployment's declared value through the shared restore arms, and this change's reading and report of the applied value. - It then merged `origin/main` `9b24f3ca87`, which had moved the refusal copy of `allowed-ip-ranges.ts` to every language on the platform locale table and made the lock-out recovery four steps. Three conflicts, each resolved by keeping both sides: the copy table carries this change's two refusals (a value that is not text, an entry that names no range) in all eleven languages, which `main`'s `settings-copy-locale.test.ts` holds sentence by sentence; the pin file keeps both new top-level describes; the hardening guide's Network row keeps the IPv4 or IPv6 wording and `main`'s recovery. The changeset's lock-out step now clears the stored value, since a clear is the one write an env override lets through. - `test:repo` was not run locally: no test in that project changed. CI's Test Core runs it. ## Verification Readings at head `03655cf8e1` (the branch merged with `origin/main` `9b24f3ca87`, then the changeset's lock-out step) unless another tree is named. <details><summary>Premise: the new pins on the trees before them</summary> - Entry rule: tests committed first (`da71a1fb9a`), source still at `b3d2f4bdb7`: `vitest run src/allowed-ip-ranges.pin.test.ts src/auth-manager.test.ts -t allow` gave `Tests 11 failed | 21 passed`. Every new refusal pin was red there; every well-formed control and the preservation save were green. - Value rule: tests committed first (`d28f95ba40`), source still at the merge `2a06416210`: both files gave `Tests 4 failed | 347 passed (351)`. The four not-text pins were red; the ordering case with its added `error.code` and field `code` assertions was green, as it needs no code change. </details> <details><summary>Package tests and typecheck</summary> - `pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2` (the whole package, under the verify lock): `Test Files 228 passed (228)`, `Tests 4105 passed | 10 skipped (4115)`, exit 0. The files include `settings-copy-locale.test.ts`, which holds every sentence of the refusal copy, this change's two included, in every language on the platform locale table. - `pnpm --filter @objectstack/plugin-auth typecheck`: exit 0, `check:test-typecheck: OK` (10 files, 88 errors, 23 pinned signatures held; the ledger is unchanged). - Consumers, downstream direction: the ten packages that depend on `@objectstack/plugin-auth` directly (`cli`, `client`, `organizations`, `plugin-dev`, `dogfood`, `http-conformance`, `runtime`, `service-automation`, `service-sms`, `verify`), `turbo run typecheck --only` over their built closure: `Tasks: 10 successful, 10 total`, exit 0. The package entry (`index.ts`) is not in the diff, so nothing further downstream sees a type move. - The one locked hold that ran these, the gates below that need a full build, and the dogfood battery: `VERDICT command-exit 0`. </details> <details><summary>Ablation: each refusal, on committed files</summary> Run at `4b2c6130c0`, before the second merge of `main`. The merge left `auth-manager.ts` and `auth-plugin.ts` byte-identical, and in `allowed-ip-ranges.ts` changed only the copy table and the predicate's copy lookup, not the conditions the legs mutate. `node scripts/ablation-replace.mjs` in wrap mode, one locked run of both test files per leg. Each mutation landed (anchor count 1 to 0, blob changed) and the tool restored it (blob equal to HEAD, `git diff HEAD` empty). These tests import `src/` directly, so no `dist/` leg applies. | Leg | Mutation | Tests red | |---|---|---| | A | the prefix-digit patterns accept any text | 9 | | B | the prefix-range bound removed | 5 | | C | the blank-inside-an-entry refusal removed | 1 | | D | the save predicate's entry refusal never fires | 3 | | E | the constructor's `assertAllowedIpRanges` call removed | 2 | | F | the settings binding's report removed | 3 | | G | the save predicate's not-text refusal never fires | 3 | | H | a stored value that is not text read as no list | 1 | | I | the binding's not-text report never logs | 1 | After the nine legs the tree had 0 changed paths. </details> <details><summary>Gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 95 commands (8 paths against merge base `9b24f3ca8`). All 95 ran at this head, each with stdin closed, and each exited 0. Ten first refused for want of built output in this worktree (nine with exit 3, `check:docs` with exit 1 on a missing generated schema tree); they were run again after the closure build, or after `check:type-check-debt`'s own full build for `check:skill-examples` and `check:dual-build-cjs-loads`, and the code recorded is that run's. `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran`: `95 derived, 95 run, 0 NOT-MEASURED, 0 UNRUN`. </details> <details><summary>Dogfood battery</summary> `pnpm --filter @objectstack/dogfood test` at this head, under the verify lock, no shard, after `turbo run build --filter=@objectstack/dogfood` with the consumers above (`Tasks: 69 successful, 69 total`): `Test Files 423 passed | 3 skipped (426)`, `Tests 3532 passed | 41 skipped (3573)`, exit 0. </details> <details><summary>CI at this head</summary> 50 check-runs, read once their runs completed: 38 success, 12 skipped, none failed. Required contexts: `Lint & Repo Gates`, `TypeScript Type Check`, `Test Core` (8 of 8 shards), `Build Core`, `Governed Surface Queue Guard`, `Test UI` and `Live E2E UI` success; `Lint UI`, `Type Check UI` and `Build & E2E UI` skipped (the paths select no UI lane). </details> <details><summary>NOT MEASURED</summary> - `test:repo` locally: no test in that project changed. CI's `Test Core` runs it, green above. - The CI jobs the gate derivation names outside its list: the dogfood shards (merge queue only), Dogfood Verify CLI and Temporal Conformance (`main`'s push battery only), Build Docs, and the type-check lanes. CI's own, not run here. - The ablation at this head: its readings are the ones at `4b2c6130c0` above. - `origin/main` past `9b24f3ca87`: 24 commits at reading time, none touching `packages/plugins/plugin-auth`. GitHub reads the PR `mergeable: clean`, so it was not merged again. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01L6bhgVfjk2i8T2D4RmxsKg <!-- signed: security#1 · os-tesla · session_01L9qibvynRbCtMndW8EHNGc --> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent adb2b89 commit b58beb8

8 files changed

Lines changed: 631 additions & 59 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
fix(plugin-auth)!: an allowed IP range that names no range is refused at save and never matches at the gate
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: none
9+
Skills-docs: updated here — `content/docs/permissions/authentication.mdx` (the `allowed_ip_ranges` row, and the first two rules under "Saving, clearing and lockout recovery") and `docs/HARDENING.md` (the Network row); `skills/**` teaches no `allowed_ip_ranges`
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) no metadata an author writes changes and no stored row is rewritten: a value of the `auth.allowed_ip_ranges` setting that is not text, or an entry of it or of the auth plugin's `allowedIpRanges` option that names no range, is refused where it is saved or declared, and matches no address at the sign-in gate -->
12+
13+
**BREAKING** for a deployment whose `auth.allowed_ip_ranges` (ADR-0069 D5), `OS_AUTH_ALLOWED_IP_RANGES` or declared `allowedIpRanges` holds an entry that is not an IP address or a CIDR range, or whose stored `auth.allowed_ip_ranges` value is not text.
14+
15+
The setting holds its entries as text, one per line or comma-separated; a value that is not text (a list, a number, an object) names no list. An entry is an IPv4 or IPv6 address, or an address, `/` and a prefix length written in decimal digits: one or two digits, from 0 to 32, for an IPv4 base (an IPv4-mapped one included), and one to three digits, from 0 to 128, for an IPv6 base. Every such entry keeps its meaning. Any other entry names no range, and one reading of that rule now serves every door that judges or saves the list:
16+
17+
- **The sign-in gate.** An entry that names no range matches no address, its own base address included, so a list holding one admits exactly what its other entries admit. `ipMatchesRange` answers `false` for it.
18+
- **The settings save.** A value that is not a list of entries written as text is refused with `400 SETTINGS_VALIDATION`, `invalid_value` at `allowed_ip_ranges` and `constraint: { type: 'textarea' }`. A list holding an entry that names no range is refused the same way, the entries in `constraint.invalidEntries`, with a message in the caller's language that names them. Both are judged before the saving address, on every write: the console's `PUT /api/settings/auth` and writes made inside the server alike. Saving the value already in force again is not judged, as before.
19+
- **The declared list.** An `allowedIpRanges` passed to `AuthPlugin` or `AuthManager`, or set by an `auth:configure` handler, that holds one, or that is not an array, is refused at construction with an error naming each such entry.
20+
- **A list already stored, or pinned by `OS_AUTH_ALLOWED_IP_RANGES`.** It is applied, the entry matches no address, and the settings binding logs one `error` that names the entry and the fix. A stored value that is not text admits no address while it is in force, and the binding logs one `error` that says so and names the fix.
21+
- **Who moves.** A deployment with such an entry corrects it to an address, or an address and a prefix length (`203.0.113.0/24`, `2001:db8::/32`), or removes it. A deployment whose stored value is not text saves the list again as text, or clears it. Locked out meanwhile, start the server with `OS_AUTH_ALLOWED_IP_RANGES` set to an empty value, sign in, clear the stored value (`PUT /api/settings/auth` with `allowed_ip_ranges: null`, the one write the override lets through), remove the variable and restart, then save the corrected list.

‎content/docs/permissions/authentication.mdx‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,7 @@ often before the account-level lock engages.
619619

620620
| Setting | Effect |
621621
|---|---|
622-
| `allowed_ip_ranges` | Restrict authenticated access to one or more IPv4 CIDR ranges (or exact IPs). The address judged is the one the server establishes, by the same rule as the inbound rate limiter: `X-Forwarded-For` / `X-Real-IP` only when the stack declares `server.trustProxy`, the connection's peer address otherwise. Behind a reverse proxy, declare `server.trustProxy`, or the allow-list judges the proxy's own address. A request whose address cannot be established is refused with `403 IP_NOT_ALLOWED`, like one outside the ranges. `last_login_ip` records the same address. A save that leaves out your own address is refused, and clearing the field applies at once: see [Saving, clearing and lockout recovery](#saving-clearing-and-lockout-recovery). |
622+
| `allowed_ip_ranges` | Restrict authenticated access to one or more CIDR ranges or exact addresses, IPv4 or IPv6 (`203.0.113.0/24`, `2001:db8::/32`, `192.0.2.7`). An entry that is neither is refused when you save it. The address judged is the one the server establishes, by the same rule as the inbound rate limiter: `X-Forwarded-For` / `X-Real-IP` only when the stack declares `server.trustProxy`, the connection's peer address otherwise. Behind a reverse proxy, declare `server.trustProxy`, or the allow-list judges the proxy's own address. A request whose address cannot be established is refused with `403 IP_NOT_ALLOWED`, like one outside the ranges. `last_login_ip` records the same address. A save that leaves out your own address is refused, and clearing the field applies at once: see [Saving, clearing and lockout recovery](#saving-clearing-and-lockout-recovery). |
623623

624624
> **How enforcement works.** Password expiry (an administrator-forced password
625625
> change included) and enforced MFA share a single *session-validation gate*.
@@ -661,6 +661,25 @@ often before the account-level lock engages.
661661

662662
These rules apply to `allowed_ip_ranges`.
663663

664+
- **An entry the server cannot read is refused.** Each entry is an IPv4 or IPv6
665+
address, or an address followed by `/` and a prefix length written in decimal
666+
digits: one or two digits, from 0 to 32, for IPv4, and one to three digits, from
667+
0 to 128, for IPv6. Any other entry is refused with `400 SETTINGS_VALIDATION`,
668+
whatever address you save from, and nothing is stored. The field error names the
669+
entries, also listed in `constraint.invalidEntries`. This check runs before the
670+
address check below, on every write, including writes made inside the server. An
671+
`allowedIpRanges` list the deployment declares in its own auth configuration is
672+
held to the same rule and refused at startup. An entry the server cannot read
673+
matches no address at sign-in. If a stored list or `OS_AUTH_ALLOWED_IP_RANGES`
674+
already holds one, the server logs it at startup with the entry named, and only
675+
the list's other entries admit anyone.
676+
- **A value that is not text is refused.** The field holds its entries as text, one
677+
per line or comma-separated. A value written in another form, such as a JSON list,
678+
a number or an object, names no list: through the API or by a write inside the
679+
server, it is refused the same way, with `constraint: { type: 'textarea' }`, and
680+
nothing is stored. A value of another
681+
form stored before this check admits no address at sign-in, and the server logs
682+
it at startup. Save the list as text, or clear it.
664683
- **A save that would lock you out is refused.** When you save the field, the server
665684
checks the new list against the address your own request comes from. It finds that
666685
address by the same rule as the gate (`server.trustProxy` decides whether the
@@ -675,7 +694,7 @@ These rules apply to `allowed_ip_ranges`.
675694
If the server cannot establish your address at all, any non-empty list is refused,
676695
because the gate would refuse you too. Only a change is checked: saving the list
677696
already in force again is not refused. Writes made inside the server (boot, seed,
678-
migrations) carry no request and are not checked.
697+
migrations) carry no request, so no address is checked for them.
679698
- **Setup shows your address before you save.** Under the field, **Setup →
680699
Authentication** shows the address the server judges for your request, read from
681700
`GET /api/v1/auth/admin/client-address` (platform administrators only), which

‎docs/HARDENING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ production deployment, enable at least:
241241
| Anti-abuse | `lockout_threshold: 5`, `lockout_duration_minutes: 15`, `rate_limit_max` / `rate_limit_window_seconds` |
242242
| MFA | `mfa_required: true` (or per-org `require_mfa`, which needs two-factor enrollment on), `mfa_grace_period_days: 7`; neither saves while `OS_AUTH_TWO_FACTOR=false` |
243243
| Sessions | `session_idle_timeout_minutes: 30–60`, `session_absolute_max_hours: 8–12`, `max_concurrent_sessions_per_user` |
244-
| Network | `allowed_ip_ranges` (IPv4 CIDR) for tenant- or office-scoped access; locked out, restart with `OS_AUTH_ALLOWED_IP_RANGES=` (empty), sign in, clear the stored list (`PUT /api/settings/auth` with `allowed_ip_ranges: null`), then remove the variable |
244+
| Network | `allowed_ip_ranges` (IPv4 or IPv6 CIDR, or exact addresses) for tenant- or office-scoped access; locked out, restart with `OS_AUTH_ALLOWED_IP_RANGES=` (empty), sign in, clear the stored list (`PUT /api/settings/auth` with `allowed_ip_ranges: null`), then remove the variable |
245245

246246
Password expiry and enforced MFA share a *session-validation gate*: a
247247
non-compliant session can authenticate but is blocked from data access (auth +

‎packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts‎

Lines changed: 153 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
* `vitest.config.ts`), so the routes and the service are the checkout's.
3535
*/
3636

37-
import { afterEach, describe, expect, it, vi } from 'vitest';
37+
import { afterEach, describe, expect, it, vi, type Mock } from 'vitest';
3838
import { Hono } from 'hono';
3939
import type { IHttpRequest, IHttpResponse, IHttpServer, RouteHandler } from '@objectstack/spec/contracts';
4040
import {
@@ -73,6 +73,7 @@ const PLATFORM_ADMIN_SESSION = { user: { id: 'usr_admin', email: 'admin@example.
7373
interface Rig {
7474
svc: SettingsService;
7575
manager: AuthManager;
76+
logger: { info: Mock; error: Mock; warn: Mock; debug: Mock };
7677
save(body: Record<string, unknown>, transport?: Transport): Promise<{ status: number; body: any }>;
7778
/** `GET /api/v1/auth/admin/client-address` on the plugin's real auth routes. */
7879
read(transport?: Transport, opts?: { session?: unknown; query?: string }):
@@ -153,7 +154,7 @@ async function boot(
153154
getApi.mockRestore();
154155
}
155156
};
156-
return { svc, manager, save, read };
157+
return { svc, manager, logger: ctx.logger, save, read };
157158
}
158159

159160
const stored = async (svc: SettingsService) => (await svc.get('auth', 'allowed_ip_ranges', {})).value ?? null;
@@ -390,6 +391,156 @@ describe('#30398 the address read serves the address the save check judges', ()
390391
});
391392
});
392393

394+
describe('an allowed IP range whose prefix is not decimal digits within its family is refused at every save door and never matches at the gate', () => {
395+
// An entry is an address, or an address, `/` and a prefix length of one or
396+
// two decimal digits from 0 to 32 (IPv4) or one to three from 0 to 128
397+
// (IPv6). Each entry here is neither. The saving address, 10.1.2.3, is one
398+
// the list's other entry matches, so only the entry itself can refuse it.
399+
const UNREADABLE = ['10.0.0.0/', '10.0.0.0/ 24', '10.0.0.0/0x18', '10.0.0.0/33', '10.0.0.0/8/8', '2001:db8::/', '2001:db8::/129', 'office'];
400+
const SAVER = { remoteAddress: '10.1.2.3' };
401+
402+
it('the console save refuses a list holding one, names it, and stores nothing', async () => {
403+
const { svc, save } = await boot();
404+
for (const entry of UNREADABLE) {
405+
for (const list of [entry, `10.1.2.3\n${entry}`]) {
406+
const res = await save({ allowed_ip_ranges: list }, SAVER);
407+
expect(res.status, JSON.stringify(list)).toBe(400);
408+
expect(res.body.error.code).toBe('SETTINGS_VALIDATION');
409+
const field = res.body.error.details.fields.find((f: any) => f.field === 'allowed_ip_ranges');
410+
expect(field.code).toBe('invalid_value');
411+
expect(field.constraint).toEqual({ invalidEntries: [entry] });
412+
expect(field.message).toContain(entry);
413+
expect(await stored(svc)).toBeNull();
414+
}
415+
}
416+
});
417+
418+
it('the console save names the unreadable entry first, whatever address it is saved from', async () => {
419+
const { save } = await boot();
420+
for (const transport of [{}, { remoteAddress: '198.51.100.1' }]) {
421+
const res = await save({ allowed_ip_ranges: '203.0.113.0/24, 10.0.0.0/, 2001:db8::/0x20' }, transport);
422+
expect(res.status).toBe(400);
423+
expect(res.body.error.code).toBe('SETTINGS_VALIDATION');
424+
expect(res.body.error.details.fields[0].field).toBe('allowed_ip_ranges');
425+
expect(res.body.error.details.fields[0].code).toBe('invalid_value');
426+
expect(res.body.error.details.fields[0].constraint).toEqual({ invalidEntries: ['10.0.0.0/', '2001:db8::/0x20'] });
427+
}
428+
});
429+
430+
it('an in-process write of a list holding one is refused too', async () => {
431+
const { svc } = await boot();
432+
const err: any = await svc.set('auth', 'allowed_ip_ranges', '10.0.0.0/', {}).then(() => null, (e) => e);
433+
expect(err?.code).toBe('SETTINGS_VALIDATION');
434+
expect(err.fields).toEqual([
435+
expect.objectContaining({ field: 'allowed_ip_ranges', code: 'invalid_value', constraint: { invalidEntries: ['10.0.0.0/'] } }),
436+
]);
437+
expect(await stored(svc)).toBeNull();
438+
});
439+
440+
it("the deployment's own declared allowedIpRanges holding one refuses the boot, naming it", async () => {
441+
await expect(boot({ plugin: { allowedIpRanges: ['10.0.0.0/8', '10.0.0.0/'] } })).rejects.toThrow(
442+
/allowedIpRanges.*"10\.0\.0\.0\/"/,
443+
);
444+
});
445+
446+
it('a list stored before the save check admits no client through the unreadable entry, and the boot names it', async () => {
447+
const svc = new SettingsService({ env: {} });
448+
svc.registerManifest(authManifest);
449+
// Written before the plugin registers its save check: the shape of a list stored by an earlier release.
450+
await svc.set('auth', 'allowed_ip_ranges', '10.0.0.0/\n192.0.2.7', {});
451+
const { manager, logger } = await boot({ svc });
452+
expect(manager.isClientIpAllowed('10.1.2.3')).toBe(false);
453+
expect(manager.isClientIpAllowed('8.8.8.8')).toBe(false);
454+
expect(manager.isClientIpAllowed('192.0.2.7')).toBe(true);
455+
expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('"10.0.0.0/"'));
456+
});
457+
458+
it('OS_AUTH_ALLOWED_IP_RANGES holding one admits no client through it, and the boot names it', async () => {
459+
const { manager, logger } = await boot({ env: { OS_AUTH_ALLOWED_IP_RANGES: '10.0.0.0/, 192.0.2.7' } });
460+
expect(manager.isClientIpAllowed('10.1.2.3')).toBe(false);
461+
expect(manager.isClientIpAllowed('192.0.2.7')).toBe(true);
462+
expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('"10.0.0.0/"'));
463+
});
464+
465+
it('a list of well-formed entries saves and gates as before (preservation)', async () => {
466+
const list = '10.0.0.0/8\n10.0.0.0/08, 192.0.2.7/32\n2001:db8::/32\n::1/128\n203.0.113.5\n2001:db8::1';
467+
const { svc, manager, logger, save } = await boot();
468+
const res = await save({ allowed_ip_ranges: list }, SAVER);
469+
expect(res.status, JSON.stringify(res.body)).toBe(200);
470+
expect(await stored(svc)).toBe(list);
471+
await vi.waitFor(() => expect(manager.isClientIpAllowed('198.51.100.1')).toBe(false));
472+
expect(manager.isClientIpAllowed('10.200.0.1')).toBe(true);
473+
expect(manager.isClientIpAllowed('192.0.2.7')).toBe(true);
474+
expect(manager.isClientIpAllowed('192.0.2.8')).toBe(false);
475+
expect(manager.isClientIpAllowed('2001:db8:ffff::1')).toBe(true);
476+
expect(manager.isClientIpAllowed('::1')).toBe(true);
477+
expect(manager.isClientIpAllowed('203.0.113.5')).toBe(true);
478+
expect(logger.error).not.toHaveBeenCalledWith(expect.stringContaining('allowed_ip_ranges'));
479+
});
480+
});
481+
482+
describe('an allowed_ip_ranges value that is not text is refused at every save door, and where it is stored the gate admits nobody through it', () => {
483+
// The setting is a textarea: its value is text, one entry per line or
484+
// comma-separated. Each value here is not text. The saving address,
485+
// 10.1.2.3, is one each of them spells, so only the value's form can refuse it.
486+
const NOT_TEXT: unknown[] = [['10.1.2.3'], ['10.0.0.0/8', '10.1.2.3'], [], 167838211, true, { ranges: '10.1.2.3' }];
487+
const SAVER = { remoteAddress: '10.1.2.3' };
488+
489+
it('the console save refuses it and stores nothing', async () => {
490+
const { svc, save } = await boot();
491+
for (const value of NOT_TEXT) {
492+
const res = await save({ allowed_ip_ranges: value }, SAVER);
493+
expect(res.status, JSON.stringify(value)).toBe(400);
494+
expect(res.body.error.code).toBe('SETTINGS_VALIDATION');
495+
const field = res.body.error.details.fields.find((f: any) => f.field === 'allowed_ip_ranges');
496+
expect(field.code).toBe('invalid_value');
497+
expect(field.constraint).toEqual({ type: 'textarea' });
498+
expect(field.message).toContain('not text');
499+
expect(await stored(svc)).toBeNull();
500+
}
501+
});
502+
503+
it('it is refused whatever address it is saved from', async () => {
504+
const { save } = await boot();
505+
for (const transport of [{}, { remoteAddress: '198.51.100.1' }]) {
506+
const res = await save({ allowed_ip_ranges: ['203.0.113.0/24'] }, transport);
507+
expect(res.status).toBe(400);
508+
expect(res.body.error.code).toBe('SETTINGS_VALIDATION');
509+
expect(res.body.error.details.fields[0].code).toBe('invalid_value');
510+
expect(res.body.error.details.fields[0].constraint).toEqual({ type: 'textarea' });
511+
}
512+
});
513+
514+
it('an in-process write of it is refused too', async () => {
515+
const { svc } = await boot();
516+
for (const value of NOT_TEXT) {
517+
const err: any = await svc.set('auth', 'allowed_ip_ranges', value, {}).then(() => null, (e) => e);
518+
expect(err?.code, JSON.stringify(value)).toBe('SETTINGS_VALIDATION');
519+
expect(err.fields).toEqual([
520+
expect.objectContaining({ field: 'allowed_ip_ranges', code: 'invalid_value', constraint: { type: 'textarea' } }),
521+
]);
522+
expect(await stored(svc)).toBeNull();
523+
}
524+
});
525+
526+
it('a value stored before the save check admits no client, the boot names it, and saving the list as text replaces it', async () => {
527+
for (const value of [['10.1.2.3'], 167838211, { ranges: '10.1.2.3' }]) {
528+
const svc = new SettingsService({ env: {} });
529+
svc.registerManifest(authManifest);
530+
// Written before the plugin registers its save check: the shape of a value stored by an earlier release.
531+
await svc.set('auth', 'allowed_ip_ranges', value, {});
532+
const { manager, logger, save } = await boot({ svc });
533+
expect(manager.isClientIpAllowed('10.1.2.3'), JSON.stringify(value)).toBe(false);
534+
expect(manager.isClientIpAllowed('8.8.8.8')).toBe(false);
535+
expect(logger.error).toHaveBeenCalledWith(expect.stringMatching(/allowed_ip_ranges.*not text/));
536+
const res = await save({ allowed_ip_ranges: '10.1.2.3' }, SAVER);
537+
expect(res.status, JSON.stringify(res.body)).toBe(200);
538+
await vi.waitFor(() => expect(manager.isClientIpAllowed('10.1.2.3')).toBe(true));
539+
expect(manager.isClientIpAllowed('8.8.8.8')).toBe(false);
540+
}
541+
});
542+
});
543+
393544
describe('#30385 the env recovery clears the stored list, so lifting the override does not lock the caller out again', () => {
394545
it('set the override empty, sign in, clear the stored list through the settings door, remove the override: the caller is admitted', async () => {
395546
const env: Record<string, string | undefined> = {};

0 commit comments

Comments
 (0)