From c8404e588b228e657d2f0fcaef05af6a5ac66bde Mon Sep 17 00:00:00 2001 From: rodrigobnogueira Date: Mon, 20 Jul 2026 12:12:46 -0300 Subject: [PATCH] feat(core): per-dimension identity normalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A case-insensitive login keyed on an un-normalized dimension is a lockout BYPASS: `Alice`, `alice`, and `alice ` hash to three different counters, so an attacker exceeds the limit by varying case or whitespace. The docs already warned about this; now the library provides the fix. - interfaces.ts: `Normalize` type + optional `normalize?` on LockoutPolicy — a per-dimension `Record string>` (mirrors django-axes's per-field username callable; you normalize `username`, not `ip`). - key.ts: apply the normalizer inside deriveKeys, before hashing — so it covers check / record / reset uniformly (all route through deriveKeys). - manager.ts: thread it through; validate at construction (each entry must be a function — fail loud, not on the hot auth path). - Tests: key.spec (case/whitespace variants collapse to one key; only listed dimensions normalized; combination-key per-dimension) + manager.spec (end-to-end counter collapse; reset by any spelling; non-function rejected) + adapter (normalize threads through forRoot). 100% core coverage; the normalizer-application mutant is test-killed (hand-verified). - Docs: upgraded the existing "normalize it yourself" security note to the configurable option + the api-reference policy table. Backward compatible. --- CHANGELOG.md | 13 ++++++ packages/core/README.md | 15 +++++-- packages/core/interfaces.ts | 15 +++++++ packages/core/key.ts | 12 +++++- packages/core/manager.ts | 28 ++++++++++-- packages/core/test/key.spec.ts | 45 +++++++++++++++++++ packages/core/test/manager.spec.ts | 48 +++++++++++++++++++++ packages/nestjs/test/lockout.module.spec.ts | 18 ++++++++ website/docs/api-reference.md | 9 ++-- 9 files changed, 192 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 943df01..a1636c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ This project follows semantic versioning for the published packages. Sample, documentation, and CI-only changes may remain unreleased until the next package release is useful for users. +## Unreleased + +### `@authlock/core` + +- **Added per-dimension identity `normalize`** — a `Record + string>` on the policy, applied to each identity value before key derivation + on every path (check / record / reset). Without it, `Alice`, `alice`, and + `alice ` hash to three different counters, so a case-insensitive login could + be brute-forced past the limit by varying case or whitespace — a bypass the + docs previously only warned about. Normalizers are validated at construction + (each entry must be a function). Fully backward compatible: omit `normalize` + and behaviour is unchanged. + ## 0.3.1 `@nest-native/lockout` only (`@authlock/core` unchanged at 0.3.0). diff --git a/packages/core/README.md b/packages/core/README.md index 13307dc..d354b74 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -110,9 +110,18 @@ how long any single run can keep an identity locked, at `windowMs`. single-target brute force even when the IP is spoofable. - **Don't `whitelist` on a spoofable dimension.** A whitelist keyed on an IP an attacker can forge is a bypass; key it on something they can't control. -- **Normalize identity dimensions.** `Alice` and `alice` (and unicode variants) - hash to different counters. If your auth is case-insensitive, lowercase / - canonicalize the username before you pass it, or the limit is per-spelling. +- **Normalize identity dimensions.** `Alice`, `alice`, and `alice ` hash to + three different counters, so on a case-insensitive login an attacker bypasses + the limit by varying case or whitespace. Configure a per-dimension + `normalize` map (applied before hashing, on every path — check / record / + reset) — or normalize in your own extractor: + ```ts + new LockoutManager({ + parameters: [['username'], ['ip']], + normalize: { username: (v) => v.trim().toLowerCase() }, // ip left verbatim + // ... + }); + ``` - **Bound store growth.** Every distinct identity creates a record, so a flood of fabricated usernames/IPs grows the store. Schedule `pruneExpired()` (it drops records past their window, capping the store to identities active within diff --git a/packages/core/interfaces.ts b/packages/core/interfaces.ts index 89b716b..f6d848c 100644 --- a/packages/core/interfaces.ts +++ b/packages/core/interfaces.ts @@ -21,6 +21,16 @@ export interface Identifiers { */ export type LockoutParameter = ReadonlyArray; +/** + * Per-dimension value normalizers, applied to each identity value BEFORE it is + * hashed into a key. This is a security control, not a convenience: without it, + * `Alice`, `alice`, and `alice ` derive three different counters, so an attacker + * bypasses the limit by varying case or whitespace on a case-insensitive login. + * Normalize the dimensions your auth treats as equal (e.g. lowercase + trim the + * username), and leave the rest (an IP needs no normalization) unlisted. + */ +export type Normalize = Readonly string>>; + /** A persisted failure counter for one resolved key. */ export interface FailureRecord { /** Canonical, collision-resistant key derived from (parameter, values). */ @@ -98,6 +108,11 @@ export interface LockoutPolicy { tiers?: readonly CooloffTier[]; /** Which dimension combinations are evaluated; a lock trips if ANY of them does. */ parameters: readonly LockoutParameter[]; + /** + * Per-dimension value normalizers applied before key derivation — the + * defence against case/whitespace lockout bypass. See {@link Normalize}. + */ + normalize?: Normalize; /** Identities for which locking is skipped entirely (never counted or locked). */ whitelist?: (id: Identifiers) => boolean | Promise; /** Clear a key's failures on a successful login. Defaults to `true`. */ diff --git a/packages/core/key.ts b/packages/core/key.ts index 1083286..dcd4775 100644 --- a/packages/core/key.ts +++ b/packages/core/key.ts @@ -1,6 +1,6 @@ import { createHash } from 'node:crypto'; -import type { Identifiers, LockoutParameter } from './interfaces'; +import type { Identifiers, LockoutParameter, Normalize } from './interfaces'; /** A configured parameter paired with the concrete key it resolves to. */ export interface DerivedKey { @@ -22,10 +22,17 @@ export interface DerivedKey { * than concatenated so a credential-bearing dimension never appears verbatim in * a store, and so arbitrary user input can never collide with or forge another * key by embedding the separator. + * + * A per-dimension `normalize` map (if given) is applied to each value before it + * is hashed, so equal-by-policy identities (`Alice` vs `alice`) collapse to one + * counter — the defence against case/whitespace lockout bypass. It is applied + * uniformly on every path (check / record / reset) because all of them route + * through here. */ export function deriveKeys( id: Identifiers, parameters: readonly LockoutParameter[], + normalize?: Normalize, ): DerivedKey[] { const derived: DerivedKey[] = []; for (const parameter of parameters) { @@ -37,7 +44,8 @@ export function deriveKeys( applies = false; break; } - pairs.push([dimension, value]); + const normalizer = normalize?.[dimension]; + pairs.push([dimension, normalizer ? normalizer(value) : value]); } if (!applies) { continue; diff --git a/packages/core/manager.ts b/packages/core/manager.ts index d0a7ec2..08a2254 100644 --- a/packages/core/manager.ts +++ b/packages/core/manager.ts @@ -15,6 +15,7 @@ import type { LockoutManagerOptions, LockoutParameter, LockoutStore, + Normalize, } from './interfaces'; /** @@ -63,6 +64,24 @@ function validateTiers(cooloffMs: number, tiers?: readonly CooloffTier[]): void } } +/** + * Reject a `normalize` map whose entries are not functions — a non-function + * normalizer would throw (or silently no-op) inside key derivation, on the hot + * auth path, long after startup. Fail loud instead. + */ +function validateNormalize(normalize?: Normalize): void { + if (normalize === undefined) { + return; + } + for (const [dimension, fn] of Object.entries(normalize)) { + if (typeof fn !== 'function') { + throw new TypeError( + `LockoutManager: normalize['${dimension}'] must be a function.`, + ); + } + } +} + function notLocked(): LockoutDecision { return { locked: false, retryAfterMs: null }; } @@ -112,6 +131,7 @@ function mostRestrictive( export class LockoutManager { private readonly store: LockoutStore; private readonly parameters: readonly LockoutParameter[]; + private readonly normalize?: Normalize; private readonly limit: number; private readonly cooloffMs: number; private readonly windowMs: number; @@ -135,8 +155,10 @@ export class LockoutManager { throw new TypeError('LockoutManager: `cooloffMs` must be greater than 0.'); } validateTiers(options.cooloffMs, options.tiers); + validateNormalize(options.normalize); this.store = options.store; this.parameters = options.parameters; + this.normalize = options.normalize; this.limit = options.limit; this.cooloffMs = options.cooloffMs; this.windowMs = effectiveWindowMs( @@ -169,7 +191,7 @@ export class LockoutManager { } const now = this.now(); let decision = notLocked(); - for (const { parameter, key } of deriveKeys(id, this.parameters)) { + for (const { parameter, key } of deriveKeys(id, this.parameters, this.normalize)) { const evaluation = await this.readKey(key, now); decision = mostRestrictive(decision, toDecision(evaluation, parameter)); } @@ -187,7 +209,7 @@ export class LockoutManager { } const now = this.now(); let decision = notLocked(); - for (const { parameter, key } of deriveKeys(id, this.parameters)) { + for (const { parameter, key } of deriveKeys(id, this.parameters, this.normalize)) { const keyDecision = await this.applyFailure(id, parameter, key, now); decision = mostRestrictive(decision, keyDecision); } @@ -219,7 +241,7 @@ export class LockoutManager { } private async clearKeys(id: Identifiers): Promise { - for (const { key } of deriveKeys(id, this.parameters)) { + for (const { key } of deriveKeys(id, this.parameters, this.normalize)) { try { await this.store.clear(key); } catch (error) { diff --git a/packages/core/test/key.spec.ts b/packages/core/test/key.spec.ts index ea51b03..fd1ace3 100644 --- a/packages/core/test/key.spec.ts +++ b/packages/core/test/key.spec.ts @@ -71,4 +71,49 @@ describe('deriveKeys', () => { assert.notEqual(a, b); assert.notEqual(a, c); }); + + describe('normalize', () => { + const lower = { username: (v: string) => v.trim().toLowerCase() }; + + it('collapses case/whitespace variants to a single key (the bypass defence)', () => { + const canonical = deriveKeys({ username: 'alice' }, [['username']], lower)[0].key; + for (const variant of ['Alice', 'ALICE', ' alice ', 'aLiCe']) { + const [derived] = deriveKeys({ username: variant }, [['username']], lower); + assert.equal(derived.key, canonical, `${variant} must normalize to alice`); + } + // And it equals hashing the normalized value directly. + assert.equal(canonical, expectedKey([['username', 'alice']])); + }); + + it('only normalizes listed dimensions, leaving others verbatim', () => { + const [byUser, byIp] = deriveKeys( + { username: 'BOB', ip: '1.2.3.4' }, + [['username'], ['ip']], + lower, + ); + assert.equal(byUser.key, expectedKey([['username', 'bob']])); + assert.equal(byIp.key, expectedKey([['ip', '1.2.3.4']]), 'ip untouched'); + }); + + it('normalizes each dimension of a combination parameter independently', () => { + const [derived] = deriveKeys( + { username: 'CAROL', ip: '1.2.3.4' }, + [['username', 'ip']], + { username: (v: string) => v.toLowerCase() }, + ); + assert.equal( + derived.key, + expectedKey([ + ['username', 'carol'], + ['ip', '1.2.3.4'], + ]), + ); + }); + + it('is a no-op when no normalizer is given for a dimension', () => { + const withEmptyMap = deriveKeys({ username: 'Dave' }, [['username']], {})[0].key; + const without = deriveKeys({ username: 'Dave' }, [['username']])[0].key; + assert.equal(withEmptyMap, without); + }); + }); }); diff --git a/packages/core/test/manager.spec.ts b/packages/core/test/manager.spec.ts index eadab6e..b8237af 100644 --- a/packages/core/test/manager.spec.ts +++ b/packages/core/test/manager.spec.ts @@ -276,6 +276,54 @@ describe('LockoutManager — configuration validation', () => { ]), ); }); + + it('rejects a normalize entry that is not a function', () => { + assert.throws( + () => + new LockoutManager({ + store, + limit: 2, + cooloffMs: 1000, + parameters: [['username']], + // eslint-disable-next-line @typescript-eslint/no-explicit-any + normalize: { username: 'nope' as any }, + }), + /normalize\['username'\] must be a function/, + ); + }); +}); + +describe('LockoutManager — identity normalization', () => { + it('counts case/whitespace variants against ONE counter (bypass defence)', async () => { + const manager = new LockoutManager({ + store: new InMemoryLockoutStore(), + limit: 3, + cooloffMs: 1000, + parameters: [['username']], + normalize: { username: (v) => v.trim().toLowerCase() }, + }); + // Three "different" spellings of the same user must share the counter and + // trip the lock — without normalization each spelling would be limit-1. + assert.equal((await manager.recordFailure({ username: 'Alice' })).locked, false); + assert.equal((await manager.recordFailure({ username: 'ALICE ' })).locked, false); + assert.equal((await manager.recordFailure({ username: ' alice' })).locked, true); + // And check() sees the lock under any spelling. + assert.equal((await manager.check({ username: 'aLiCe' })).locked, true); + }); + + it('normalizes the reset path too (unlock by any spelling)', async () => { + const manager = new LockoutManager({ + store: new InMemoryLockoutStore(), + limit: 2, + cooloffMs: 1000, + parameters: [['username']], + normalize: { username: (v) => v.toLowerCase() }, + }); + await manager.recordFailure({ username: 'Bob' }); + assert.equal((await manager.recordFailure({ username: 'bob' })).locked, true); + await manager.reset({ username: 'BOB' }); // different spelling still unlocks + assert.equal((await manager.check({ username: 'bob' })).locked, false); + }); }); describe('LockoutManager — multi-key evaluation', () => { diff --git a/packages/nestjs/test/lockout.module.spec.ts b/packages/nestjs/test/lockout.module.spec.ts index a7e6a83..8289dec 100644 --- a/packages/nestjs/test/lockout.module.spec.ts +++ b/packages/nestjs/test/lockout.module.spec.ts @@ -42,6 +42,24 @@ describe('LockoutModule.forRoot', () => { await moduleRef.close(); }); + it('threads a core `normalize` option through to the manager', async () => { + // The adapter passes options straight to LockoutManager, so a per-dimension + // normalizer configured here must collapse case end-to-end. + const moduleRef = await Test.createTestingModule({ + imports: [ + LockoutModule.forRoot({ + ...baseOptions(), + normalize: { username: (v) => v.toLowerCase() }, + }), + ], + }).compile(); + + const service = moduleRef.get(LockoutService); + assert.equal((await service.reportFailure({ username: 'Bob' })).locked, false); + assert.equal((await service.reportFailure({ username: 'bob' })).locked, true); + await moduleRef.close(); + }); + it('reset() administratively unlocks even with resetOnSuccess disabled', async () => { const moduleRef = await Test.createTestingModule({ imports: [ diff --git a/website/docs/api-reference.md b/website/docs/api-reference.md index 7bbafe2..1c67e32 100644 --- a/website/docs/api-reference.md +++ b/website/docs/api-reference.md @@ -40,6 +40,7 @@ title: API Reference | `windowMs?` | failure-counting window; defaults to the effective cooloff | | `tiers?` | escalating cooloff by failure count, e.g. `[{atFailures: 10, cooloffMs: 3_600_000}]` | | `parameters` | dimension combinations to evaluate; a lock trips if **any** trips | +| `normalize?` | `Record string>` — per-dimension normalizers applied before hashing (case/whitespace-bypass defence) | | `whitelist?` | `(id) => boolean \| Promise` — identities never counted or locked | | `resetOnSuccess?` | clear failures on success (default `true`) | | `failMode?` | `'open'` (default) or `'closed'` | @@ -98,9 +99,11 @@ proxy headers for you. trust. Because a lock trips if **any** parameter trips, a `['username']` parameter still catches a single-target brute force even when the IP is spoofable. And never `whitelist` on a dimension an attacker can forge. -- **Normalize identity dimensions.** `Alice` and `alice` (and unicode variants) - hash to different counters. If your auth is case-insensitive, lowercase / - canonicalize the username before passing it, or the limit is per-spelling. +- **Normalize identity dimensions.** `Alice`, `alice`, and `alice ` hash to + three different counters — on a case-insensitive login an attacker bypasses + the limit by varying case or whitespace. Set a per-dimension `normalize` map + (e.g. `{ username: (v) => v.trim().toLowerCase() }`); it is applied before + hashing on every path (check / record / reset). Or normalize in your extractor. - **Bound store growth.** Every distinct identity creates a record. Schedule `pruneExpired()` (it drops records past their window, capping the store to identities active within `windowMs`), put a rate limiter