diff --git a/.changeset/spotty-otters-guard.md b/.changeset/spotty-otters-guard.md new file mode 100644 index 0000000..0e16f12 --- /dev/null +++ b/.changeset/spotty-otters-guard.md @@ -0,0 +1,30 @@ +--- +'seamless-auth-api': minor +--- + +Bound how many sessions one user may hold at once. + +`max_concurrent_sessions` defaults to no limit, so an instance that predates the +setting is unaffected. Unlimited is `null` rather than `0`, and `0` is refused, +because zero would otherwise read as "no sessions allowed" and lock every user +out of a deployment that meant to remove the cap. `MAX_CONCURRENT_SESSIONS` +accepts a number, or an empty value, `null`, `none` or `unlimited` for no cap, +since a deployment template cannot easily unset a variable. + +At the limit a sign-in **succeeds** and the user's oldest session is revoked with +`revokedReason: 'concurrent_session_limit'`, recorded as a new `session_evicted` +auth event naming the session that ended. Refusing the new session instead would +lock a user out of the device in front of them until something they may not have +access to expires, which for the shared workstations this exists to protect is +the common case rather than the edge one. + +Enforcement runs before the new session row is created, so the limit counts the +session about to exist: at a limit of 3 a user holding 3 ends up with 3, not 4. +Lowering the limit leaves users above it, and each converges on their next +sign-in, which evicts everything above the cap in one pass. It never throws: a +session that cannot be revoked is logged and the sign-in continues, because +failing an authentication over a housekeeping step is worse than briefly +exceeding the cap. + +NIST 800-53 AC-10. Requires `@seamless-auth/types` 0.17.0, which publishes the +config key. diff --git a/.env.example b/.env.example index e2d3804..c5b7245 100644 --- a/.env.example +++ b/.env.example @@ -75,6 +75,12 @@ REFRESH_TOKEN_TTL=1d # REFRESH_TOKEN_TTL above; this only binds while it is the shorter of the two. # Government deployments typically want 15m to 30m here. SESSION_IDLE_TTL=8h +# How many sessions one user may hold at once. Empty, "null", "none" or "unlimited" +# all mean no cap, which is the default. At the limit, a new sign-in revokes the +# user's oldest session rather than being refused, and records a session_evicted +# auth event. Government deployments and shared workstations usually want a small +# number here. +MAX_CONCURRENT_SESSIONS=unlimited RATE_LIMIT=100 DELAY_AFTER=50 # Testing escape hatch. When true, all auth rate limiters (global, OTP, magic link, diff --git a/docs/configuration.md b/docs/configuration.md index ee0a072..2258178 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -75,6 +75,7 @@ first boot. | `PASSKEY_LOGIN_FALLBACK_ENABLED` | No | `true` | `passkey_login_fallback_enabled` | When `false`, an account that holds a passkey continues with passkey only. The client's `passkeyAvailable` hint on `POST /login` cannot widen that: a caller reporting no passkey support is still offered passkey only, because otherwise any caller could ask for a weaker method by claiming not to support passkeys. A browser that genuinely cannot run the ceremony cannot sign in, which is what this setting means. Accounts with no passkey are unaffected and keep the configured methods. | | `LOCKOUT_POLICY` | No | `{"enabled":true,"maxFailures":10,"windowSeconds":900,"lockoutSeconds":900}` | `lockout_policy` | JSON. Set `enabled:false` only when an upstream policy handles lockout. | | `SESSION_IDLE_TTL` | No | `8h` | `session_idle_ttl` | Format `\d+[smhd]`. How long a session may go unrefreshed. The absolute session lifetime is `REFRESH_TOKEN_TTL`; this only binds while it is the shorter of the two. | +| `MAX_CONCURRENT_SESSIONS` | No | unlimited | `max_concurrent_sessions` | How many sessions one user may hold at once. Unset, empty, `null`, `none` or `unlimited` all mean no cap, which is the default. When a signed-in user is at the limit, the oldest session is revoked to make room and a `session_evicted` auth event is recorded; the sign-in itself always succeeds. Lowering the limit converges on each user's next sign-in rather than one session per login. NIST 800-53 AC-10. | | `AUTHENTICATOR_POLICY` | No | `{"attachment":"any","userVerification":"required","attestation":"none","requireKnownAuthenticator":false,"syncedPasskeys":"block","aaguidAllowList":[],"aaguidDenyList":[]}` | `authenticator_policy` | JSON. `attachment` is `any`, `platform` or `cross-platform`; `any` offers both built-in authenticators and roaming security keys, and naming one narrows the browser picker and rejects a request asking for the other. `userVerification` is `required`, `preferred` or `discouraged` and drives both what the browser is asked for and what the server enforces. `attestation` is `none` or `direct`; `direct` asks the authenticator to identify itself, which is what enables validation against the FIDO Metadata Service, and carries a privacy cost so it is off unless needed. `requireKnownAuthenticator` refuses an authenticator the metadata service does not list, and only applies under `direct`. It also refuses a credential that self attests or presents no attestation, since neither can be looked up, so it rules out authenticators that ship no attestation certificate. Changing `attestation` needs a restart, because the metadata service is prepared at startup. `syncedPasskeys` is `block` or `allow`; `block`, the default, refuses a credential that can leave the device it was created on, which includes every iCloud Keychain and Google Password Manager passkey. `aaguidAllowList` and `aaguidDenyList` restrict which authenticator models may register, and need `attestation` set to `direct` to mean anything. | ### Service tokens and secrets @@ -271,6 +272,7 @@ Validation is enforced by [`systemConfig.schema.ts`](../src/schemas/systemConfig | `lockout_policy` | object | `LOCKOUT_POLICY` | `{enabled,maxFailures:10,windowSeconds:900,lockoutSeconds:900}` | | `authenticator_policy` | object | `AUTHENTICATOR_POLICY` | `{attachment:"any",userVerification:"required",attestation:"none",requireKnownAuthenticator:false}` | | `session_idle_ttl` | string (`\d+[smhd]`) | `SESSION_IDLE_TTL` | `8h` | +| `max_concurrent_sessions` | number or null | `MAX_CONCURRENT_SESSIONS` | `null` (no limit) | | `access_token_ttl` | string (`\d+[smhd]`) | `ACCESS_TOKEN_TTL` | - | | `refresh_token_ttl` | string (`\d+[smhd]`) | `REFRESH_TOKEN_TTL` | - | | `rate_limit` | integer > 0 | `RATE_LIMIT` | - | diff --git a/docs/security-posture.md b/docs/security-posture.md index 93e06be..f59ff6b 100644 --- a/docs/security-posture.md +++ b/docs/security-posture.md @@ -223,3 +223,32 @@ that is not one discloses part of the allowlist and helps the browser not at all The refusal is recorded as one `request_suspicious` auth event with the real client address and user agent, and the rejected origin in an `origin` metadata field. It used to be recorded with the origin string in the `ipAddress` field, which made the trail hard to read. + +## Concurrent sessions per user + +**Posture: uncapped by default, and a cap evicts rather than refuses.** + +`max_concurrent_sessions` bounds how many sessions one user may hold at once. It +defaults to no limit, so an instance that predates the setting is unaffected. +Unlimited is `null` rather than `0`, and the schema refuses `0`, because zero +would otherwise read as "no sessions allowed" and lock every user out of a +deployment that meant to remove the cap. + +When a signed-in user is at the limit, the new sign-in **succeeds** and their +oldest session is revoked with `revokedReason: 'concurrent_session_limit'`, and a +`session_evicted` auth event names the session that ended. Refusing the new +session instead would lock a user out of the device in front of them until +something they may not have access to expires, which for the shared workstations +this setting exists to protect is the common case rather than the edge one. + +Lowering the limit leaves users above it. Each converges on their next sign-in, +which evicts everything above the cap in one pass rather than shedding one +session per login indefinitely. + +Enforcement runs before the new session row is created, so the limit counts the +session about to exist: at a limit of 3, a user holding 3 ends up with 3, not 4. +It never throws. A session that cannot be revoked is logged and the sign-in +continues, because failing an authentication over a housekeeping step is worse +than briefly exceeding the cap. + +This is NIST 800-53 AC-10. diff --git a/openapi.json b/openapi.json index b9d178a..d32f160 100644 --- a/openapi.json +++ b/openapi.json @@ -1539,6 +1539,7 @@ "service_token_failed", "service_token_rotated", "service_token_success", + "session_evicted", "step_up_challenge", "step_up_failed", "step_up_success", @@ -1608,6 +1609,7 @@ "service_token_failed", "service_token_rotated", "service_token_success", + "session_evicted", "step_up_challenge", "step_up_failed", "step_up_success", @@ -7658,6 +7660,7 @@ "access_token_ttl": "string", "session_idle_ttl": null, "refresh_token_ttl": "string", + "max_concurrent_sessions": null, "rate_limit": 0, "delay_after": 0, "rpid": "string", @@ -7828,6 +7831,13 @@ "default": "8h" }, "refresh_token_ttl": { "type": "string", "pattern": "^\\d+[smhd]$" }, + "max_concurrent_sessions": { + "type": "integer", + "nullable": true, + "minimum": 0, + "exclusiveMinimum": true, + "default": null + }, "rate_limit": { "type": "integer", "minimum": 0, "exclusiveMinimum": true }, "delay_after": { "type": "integer", "minimum": 0 }, "rpid": { "type": "string", "minLength": 1 }, @@ -8034,6 +8044,12 @@ "access_token_ttl": { "type": "string", "pattern": "^\\d+[smhd]$" }, "session_idle_ttl": { "type": "string", "pattern": "^\\d+[smhd]$" }, "refresh_token_ttl": { "type": "string", "pattern": "^\\d+[smhd]$" }, + "max_concurrent_sessions": { + "type": "integer", + "nullable": true, + "minimum": 0, + "exclusiveMinimum": true + }, "rate_limit": { "type": "integer", "minimum": 0, "exclusiveMinimum": true }, "delay_after": { "type": "integer", "minimum": 0 }, "rpid": { "type": "string", "minLength": 1 }, diff --git a/package-lock.json b/package-lock.json index ccd168a..73cb8a2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,7 @@ "@seamless-auth/messaging": "^0.1.0", "@seamless-auth/messaging-aws": "^0.1.0", "@seamless-auth/messaging-twilio": "^0.1.0", - "@seamless-auth/types": "^0.15.0", + "@seamless-auth/types": "^0.17.0", "@simplewebauthn/server": "^13.3.3", "base64url": "^3.0.1", "bcrypt-ts": "^7.1.0", @@ -3338,9 +3338,9 @@ } }, "node_modules/@seamless-auth/types": { - "version": "0.15.0", - "resolved": "https://registry.npmjs.org/@seamless-auth/types/-/types-0.15.0.tgz", - "integrity": "sha512-Y0xpK67aYHMc8Lwys7zZc2N94AnHKPD+QmJFgOf969guiazKRGhV6NK/A+SxraC7R3nm2Rf6tkTJoYJXe3WYyQ==", + "version": "0.17.0", + "resolved": "https://registry.npmjs.org/@seamless-auth/types/-/types-0.17.0.tgz", + "integrity": "sha512-FnDuCBIlmR3ey28YOl/gCLQyPiFfLYCaKFg3lbp9N0Osb8BnmVWkHzP7k0gnp5+AtDRjCj5kn4Ueqd7DmJPPMA==", "license": "AGPL-3.0-only", "dependencies": { "zod": "^4.3.6" diff --git a/package.json b/package.json index 7c644d2..8f1c351 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "@seamless-auth/messaging": "^0.1.0", "@seamless-auth/messaging-aws": "^0.1.0", "@seamless-auth/messaging-twilio": "^0.1.0", - "@seamless-auth/types": "^0.15.0", + "@seamless-auth/types": "^0.17.0", "@simplewebauthn/server": "^13.3.3", "base64url": "^3.0.1", "bcrypt-ts": "^7.1.0", diff --git a/src/config/bootstrapSystemConfig.ts b/src/config/bootstrapSystemConfig.ts index 349076c..d429cd8 100644 --- a/src/config/bootstrapSystemConfig.ts +++ b/src/config/bootstrapSystemConfig.ts @@ -13,6 +13,19 @@ import { SYSTEM_CONFIG_ENV_MAP } from './systemConfig.envMap.js'; const logger = getLogger('bootstrapSystemConfig'); +/** + * `system_config.value` is `JSONB NOT NULL`, so a config value of `null` has no + * row to live in. Absence is how the store spells it instead: every nullable key + * carries a schema default of `null`, so "no row" and "null" resolve to the same + * configuration. + * + * A key that was set and is now null therefore has its row removed, rather than + * an update that the column would refuse. + */ +function isStorable(value: unknown): boolean { + return value !== null && value !== undefined; +} + export async function bootstrapSystemConfig() { const resolvedConfig: Record = {}; @@ -37,7 +50,12 @@ export async function bootstrapSystemConfig() { `the row is not marked admin-managed (updatedBy IS NULL). If this value was changed ` + `through the admin console, that change is being reverted here.`, ); - await existing.update({ value: parsed }); + + if (isStorable(parsed)) { + await existing.update({ value: parsed }); + } else { + await existing.destroy(); + } } resolvedConfig[key] = parsed; @@ -58,11 +76,13 @@ export async function bootstrapSystemConfig() { ); } - await SystemConfig.create({ - key, - value: defaultValue, - updatedBy: null, - }); + if (isStorable(defaultValue)) { + await SystemConfig.create({ + key, + value: defaultValue, + updatedBy: null, + }); + } resolvedConfig[key] = defaultValue; continue; @@ -70,11 +90,13 @@ export async function bootstrapSystemConfig() { const parsed = parseSystemConfigEnvValue(key as keyof typeof SYSTEM_CONFIG_ENV_MAP, envValue); - await SystemConfig.create({ - key, - value: parsed, - updatedBy: null, - }); + if (isStorable(parsed)) { + await SystemConfig.create({ + key, + value: parsed, + updatedBy: null, + }); + } resolvedConfig[key] = parsed; } diff --git a/src/config/systemConfig.defaults.ts b/src/config/systemConfig.defaults.ts index 61d2578..978fe70 100644 --- a/src/config/systemConfig.defaults.ts +++ b/src/config/systemConfig.defaults.ts @@ -21,5 +21,8 @@ export const SYSTEM_CONFIG_DEFAULTS: Partial = { // the default the schema gives it instead of being absent here until someone notices. authenticator_policy: AuthenticatorPolicySchema.parse({}), session_idle_ttl: '8h', + // No cap unless a deployment asks for one, so nothing changes for an instance + // that predates the key. + max_concurrent_sessions: null, passkey_login_fallback_enabled: true, }; diff --git a/src/config/systemConfig.envMap.ts b/src/config/systemConfig.envMap.ts index da7c5a1..00caa32 100644 --- a/src/config/systemConfig.envMap.ts +++ b/src/config/systemConfig.envMap.ts @@ -14,6 +14,7 @@ export const SYSTEM_CONFIG_ENV_MAP = { passkey_login_fallback_enabled: 'PASSKEY_LOGIN_FALLBACK_ENABLED', access_token_ttl: 'ACCESS_TOKEN_TTL', session_idle_ttl: 'SESSION_IDLE_TTL', + max_concurrent_sessions: 'MAX_CONCURRENT_SESSIONS', refresh_token_ttl: 'REFRESH_TOKEN_TTL', rate_limit: 'RATE_LIMIT', delay_after: 'DELAY_AFTER', diff --git a/src/generated/api.ts b/src/generated/api.ts index 09f22fb..afaa17e 100644 --- a/src/generated/api.ts +++ b/src/generated/api.ts @@ -1322,6 +1322,7 @@ export interface paths { | 'service_token_failed' | 'service_token_rotated' | 'service_token_success' + | 'session_evicted' | 'step_up_challenge' | 'step_up_failed' | 'step_up_success' @@ -1385,6 +1386,7 @@ export interface paths { | 'service_token_failed' | 'service_token_rotated' | 'service_token_success' + | 'session_evicted' | 'step_up_challenge' | 'step_up_failed' | 'step_up_success' @@ -7683,6 +7685,7 @@ export interface paths { * "access_token_ttl": "string", * "session_idle_ttl": null, * "refresh_token_ttl": "string", + * "max_concurrent_sessions": null, * "rate_limit": 0, * "delay_after": 0, * "rpid": "string", @@ -7797,6 +7800,8 @@ export interface paths { /** @default 8h */ session_idle_ttl: string; refresh_token_ttl: string; + /** @default null */ + max_concurrent_sessions: number | null; rate_limit: number; delay_after: number; rpid: string; @@ -7943,6 +7948,7 @@ export interface paths { access_token_ttl?: string; session_idle_ttl?: string; refresh_token_ttl?: string; + max_concurrent_sessions?: number | null; rate_limit?: number; delay_after?: number; rpid?: string; diff --git a/src/lib/authEventCategories.ts b/src/lib/authEventCategories.ts index c3383a5..fab90a6 100644 --- a/src/lib/authEventCategories.ts +++ b/src/lib/authEventCategories.ts @@ -53,6 +53,9 @@ const EXACT_CATEGORIES: Readonly> = { informational: 'system', internal_user_updated_by_owner: 'system', notification_sent: 'system', + // Session lifecycle sits with login, which is where logout_ already lands, and + // an eviction is a direct consequence of a sign-in rather than an admin action. + session_evicted: 'login', user_created: 'registration', user_deleted: 'system', }; diff --git a/src/schemas/authEvent.types.ts b/src/schemas/authEvent.types.ts index 1ef7979..3503da5 100644 --- a/src/schemas/authEvent.types.ts +++ b/src/schemas/authEvent.types.ts @@ -43,6 +43,7 @@ export const AUTH_EVENT_TYPES = [ 'service_token_failed', 'service_token_rotated', 'service_token_success', + 'session_evicted', 'step_up_challenge', 'step_up_failed', 'step_up_success', diff --git a/src/services/concurrentSessionPolicy.ts b/src/services/concurrentSessionPolicy.ts new file mode 100644 index 0000000..1e354e4 --- /dev/null +++ b/src/services/concurrentSessionPolicy.ts @@ -0,0 +1,97 @@ +/* + * Copyright © 2026 Fells Code, LLC + * Licensed under the GNU Affero General Public License v3.0 + * See LICENSE file in the project root for full license information + */ + +import { Request } from 'express'; +import { Op } from 'sequelize'; + +import { Session } from '../models/sessions.js'; +import getLogger from '../utils/logger.js'; +import { AuthEventService } from './authEventService.js'; + +const logger = getLogger('concurrentSessionPolicy'); + +/** The reason written to `sessions.revokedReason`, so an audit can tell why it ended. */ +export const EVICTION_REASON = 'concurrent_session_limit'; + +/** + * Makes room for one more session by revoking the oldest, when a user is at the + * configured limit. + * + * Called before the new session is created, so the limit counts what will exist + * once it is: at a limit of 3, a user holding 3 loses their oldest and ends up + * with 3 again rather than 4. + * + * Revoking rather than refusing is deliberate. A refusal locks a user out of the + * device in front of them until something they may not have access to expires, + * which for a shared workstation is the common case rather than the edge one. + * + * More than one may be revoked in a pass: a deployment that lowers the limit + * leaves users above it, and they should converge on the next sign-in rather + * than one session per login forever. + * + * Never throws. A session that could not be revoked is logged and the sign-in + * continues, because failing an authentication over a housekeeping step is worse + * than briefly exceeding the cap. + */ +export async function enforceConcurrentSessionLimit(params: { + userId: string; + limit: number | null | undefined; + req: Request; + now?: Date; +}): Promise { + const { userId, limit, req } = params; + + if (!limit || limit < 1) { + return 0; + } + + const now = params.now ?? new Date(); + + try { + const active = await Session.findAll({ + where: { + userId, + revokedAt: null, + replacedBySessionId: null, + expiresAt: { [Op.gt]: now }, + }, + order: [['createdAt', 'ASC']], + }); + + // The incoming session is not stored yet, so the limit is compared against + // what is already there plus the one about to be added. + const surplus = active.length + 1 - limit; + + if (surplus < 1) { + return 0; + } + + const evicted = active.slice(0, surplus); + + for (const session of evicted) { + await session.update({ revokedAt: now, revokedReason: EVICTION_REASON }); + + await AuthEventService.log({ + userId, + type: 'session_evicted', + req, + metadata: { + reason: 'Concurrent session limit reached', + sessionId: session.id, + limit, + activeBefore: active.length, + }, + }); + } + + logger.info(`Evicted ${evicted.length} session(s) to stay within the limit of ${limit}.`); + + return evicted.length; + } catch (error) { + logger.error(`Could not enforce the concurrent session limit: ${error}`); + return 0; + } +} diff --git a/src/services/sessionIssuance.ts b/src/services/sessionIssuance.ts index 23beaa9..abd0929 100644 --- a/src/services/sessionIssuance.ts +++ b/src/services/sessionIssuance.ts @@ -15,6 +15,7 @@ import { } from '../lib/token.js'; import { Session } from '../models/sessions.js'; import { computeSessionTimes, parseDurationToSeconds } from '../utils/utils.js'; +import { enforceConcurrentSessionLimit } from './concurrentSessionPolicy.js'; import { getDefaultOrganizationIdForUser } from './organizationService.js'; type IssueSessionParams = { @@ -34,13 +35,21 @@ export async function issueSessionAndRespond(params: IssueSessionParams): Promis const refreshToken = generateRefreshToken(); const refreshTokenHash = await hashRefreshToken(refreshToken); const refreshTokenLookup = createRefreshTokenLookup(refreshToken); - const { access_token_ttl, refresh_token_ttl, session_idle_ttl } = await getSystemConfig(); + const { access_token_ttl, refresh_token_ttl, session_idle_ttl, max_concurrent_sessions } = + await getSystemConfig(); const { expiresAt, idleExpiresAt } = computeSessionTimes({ absoluteTtl: refresh_token_ttl || '1d', idleTtl: session_idle_ttl || '8h', }); const organizationId = await getDefaultOrganizationIdForUser(user.id); + // Before the row exists, so the limit counts the session about to be created. + await enforceConcurrentSessionLimit({ + userId: user.id, + limit: max_concurrent_sessions, + req, + }); + const session = await Session.create({ userId: user.id, infraId: process.env.APP_ID!, diff --git a/src/utils/parseEnvConfigs.ts b/src/utils/parseEnvConfigs.ts index d834d2b..26895e2 100644 --- a/src/utils/parseEnvConfigs.ts +++ b/src/utils/parseEnvConfigs.ts @@ -31,6 +31,20 @@ export function parseSystemConfigEnvValue(key: keyof typeof SYSTEM_CONFIG_ENV_MA case 'delay_after': return Number(raw); + // An empty value, or one of the words an operator is likely to reach for, + // means no limit. Without this the only way to express "uncapped" through the + // environment would be to unset the variable, which a deployment template + // cannot easily do. + case 'max_concurrent_sessions': { + const value = raw.trim().toLowerCase(); + + if (value === '' || value === 'null' || value === 'none' || value === 'unlimited') { + return null; + } + + return Number(raw); + } + case 'passkey_login_fallback_enabled': return raw.trim().toLowerCase() === 'true'; diff --git a/tests/unit/config/bootstrapSystemConfigDefaults.spec.ts b/tests/unit/config/bootstrapSystemConfigDefaults.spec.ts index d9370be..e507b40 100644 --- a/tests/unit/config/bootstrapSystemConfigDefaults.spec.ts +++ b/tests/unit/config/bootstrapSystemConfigDefaults.spec.ts @@ -56,3 +56,49 @@ describe('bootstrapSystemConfig defaults', () => { expect(result).toBeDefined(); }); }); + +// system_config.value is JSONB NOT NULL, so seeding a null default would fail the +// insert and take the whole boot down. Caught in CI by a real container start +// rather than here, which is why the mocked path now asserts it too. +describe('bootstrapSystemConfig with a null default', () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + delete process.env.LOGIN_METHODS; + }); + + it('resolves the value without writing a row the column would reject', async () => { + const { SystemConfig } = await import('../../../src/models/systemConfig'); + const { SystemConfigSchema } = await import('../../../src/schemas/systemConfig.schema'); + + vi.doMock('../../../src/config/systemConfig.defaults', () => ({ + SYSTEM_CONFIG_DEFAULTS: { login_methods: null }, + })); + + (SystemConfig.findByPk as any).mockResolvedValue(null); + (SystemConfigSchema.safeParse as any).mockReturnValue({ success: true, data: {} }); + + const { bootstrapSystemConfig } = await import('../../../src/config/bootstrapSystemConfig'); + await bootstrapSystemConfig(); + + expect(SystemConfig.create).not.toHaveBeenCalled(); + }); + + it('removes an existing row when the environment turns the value off', async () => { + const { SystemConfig } = await import('../../../src/models/systemConfig'); + const { parseSystemConfigEnvValue } = await import('../../../src/utils/parseEnvConfigs'); + const { SystemConfigSchema } = await import('../../../src/schemas/systemConfig.schema'); + + const row = { value: 5, updatedBy: null, update: vi.fn(), destroy: vi.fn() }; + (SystemConfig.findByPk as any).mockResolvedValue(row); + (parseSystemConfigEnvValue as any).mockReturnValue(null); + (SystemConfigSchema.safeParse as any).mockReturnValue({ success: true, data: {} }); + process.env.LOGIN_METHODS = 'unlimited'; + + const { bootstrapSystemConfig } = await import('../../../src/config/bootstrapSystemConfig'); + await bootstrapSystemConfig(); + + expect(row.destroy).toHaveBeenCalled(); + expect(row.update).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/config/systemConfigEnvMap.spec.ts b/tests/unit/config/systemConfigEnvMap.spec.ts index 15c6e86..adf63a2 100644 --- a/tests/unit/config/systemConfigEnvMap.spec.ts +++ b/tests/unit/config/systemConfigEnvMap.spec.ts @@ -21,6 +21,7 @@ describe('SYSTEM_CONFIG_ENV_MAP', () => { access_token_ttl: 'ACCESS_TOKEN_TTL', refresh_token_ttl: 'REFRESH_TOKEN_TTL', session_idle_ttl: 'SESSION_IDLE_TTL', + max_concurrent_sessions: 'MAX_CONCURRENT_SESSIONS', rate_limit: 'RATE_LIMIT', delay_after: 'DELAY_AFTER', rpid: 'RPID', diff --git a/tests/unit/services/concurrentSessionPolicy.spec.ts b/tests/unit/services/concurrentSessionPolicy.spec.ts new file mode 100644 index 0000000..a0c272c --- /dev/null +++ b/tests/unit/services/concurrentSessionPolicy.spec.ts @@ -0,0 +1,151 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { Session } from '../../../src/models/sessions.js'; +import { AuthEventService } from '../../../src/services/authEventService.js'; +import { + EVICTION_REASON, + enforceConcurrentSessionLimit, +} from '../../../src/services/concurrentSessionPolicy.js'; + +const NOW = new Date('2026-08-30T12:00:00.000Z'); + +function buildActiveSession(id: string) { + return { id, update: vi.fn().mockResolvedValue(undefined) }; +} + +function req() { + return { ip: '127.0.0.1', headers: {}, get: () => undefined } as never; +} + +beforeEach(() => { + (Session.findAll as any).mockResolvedValue([]); +}); + +describe('when no limit is configured', () => { + it.each([[null], [undefined], [0]])('does nothing for %s', async (limit) => { + const evicted = await enforceConcurrentSessionLimit({ + userId: 'user-1', + limit: limit as number | null | undefined, + req: req(), + now: NOW, + }); + + expect(evicted).toBe(0); + // Not even a lookup: an uncapped deployment should not pay for this on every + // sign-in. + expect(Session.findAll).not.toHaveBeenCalled(); + }); + + // A negative value cannot arrive through the schema, but the enforcement runs on + // whatever the config row holds, so it fails open rather than evicting everything. + it('does nothing for a negative limit', async () => { + expect( + await enforceConcurrentSessionLimit({ userId: 'user-1', limit: -3, req: req(), now: NOW }), + ).toBe(0); + expect(Session.findAll).not.toHaveBeenCalled(); + }); +}); + +describe('when the user is below the limit', () => { + it('leaves existing sessions alone', async () => { + (Session.findAll as any).mockResolvedValue([buildActiveSession('s-1')]); + + const evicted = await enforceConcurrentSessionLimit({ + userId: 'user-1', + limit: 3, + req: req(), + now: NOW, + }); + + expect(evicted).toBe(0); + expect(AuthEventService.log).not.toHaveBeenCalled(); + }); +}); + +describe('when the user is at the limit', () => { + it('revokes the oldest to make room for the new session', async () => { + const oldest = buildActiveSession('s-oldest'); + const newer = buildActiveSession('s-newer'); + (Session.findAll as any).mockResolvedValue([oldest, newer]); + + const evicted = await enforceConcurrentSessionLimit({ + userId: 'user-1', + limit: 2, + req: req(), + now: NOW, + }); + + expect(evicted).toBe(1); + expect(oldest.update).toHaveBeenCalledWith({ + revokedAt: NOW, + revokedReason: EVICTION_REASON, + }); + expect(newer.update).not.toHaveBeenCalled(); + }); + + it('asks for active sessions oldest first', async () => { + (Session.findAll as any).mockResolvedValue([buildActiveSession('s-1')]); + + await enforceConcurrentSessionLimit({ userId: 'user-1', limit: 1, req: req(), now: NOW }); + + expect(Session.findAll).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ + userId: 'user-1', + revokedAt: null, + replacedBySessionId: null, + }), + order: [['createdAt', 'ASC']], + }), + ); + }); + + it('records an auth event naming the session it ended', async () => { + (Session.findAll as any).mockResolvedValue([buildActiveSession('s-oldest')]); + + await enforceConcurrentSessionLimit({ userId: 'user-1', limit: 1, req: req(), now: NOW }); + + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + userId: 'user-1', + type: 'session_evicted', + metadata: expect.objectContaining({ sessionId: 's-oldest', limit: 1, activeBefore: 1 }), + }), + ); + }); +}); + +describe('when the user is over the limit', () => { + // Lowering the limit leaves users above it. They should converge on the next + // sign-in rather than shedding one session per login forever. + it('evicts everything above the limit in one pass', async () => { + const sessions = ['s-1', 's-2', 's-3', 's-4'].map(buildActiveSession); + (Session.findAll as any).mockResolvedValue(sessions); + + const evicted = await enforceConcurrentSessionLimit({ + userId: 'user-1', + limit: 2, + req: req(), + now: NOW, + }); + + expect(evicted).toBe(3); + for (const session of sessions.slice(0, 3)) { + expect(session.update).toHaveBeenCalled(); + } + expect(sessions[3].update).not.toHaveBeenCalled(); + expect(AuthEventService.log).toHaveBeenCalledTimes(3); + }); +}); + +describe('when eviction fails', () => { + // Failing an authentication over housekeeping is worse than briefly exceeding + // the cap, so the sign-in continues. + it('reports nothing evicted rather than throwing', async () => { + (Session.findAll as any).mockRejectedValue(new Error('database is down')); + + await expect( + enforceConcurrentSessionLimit({ userId: 'user-1', limit: 1, req: req(), now: NOW }), + ).resolves.toBe(0); + }); +}); diff --git a/tests/unit/services/sessionIssueService.spec.ts b/tests/unit/services/sessionIssueService.spec.ts index 3783749..939a0a3 100644 --- a/tests/unit/services/sessionIssueService.spec.ts +++ b/tests/unit/services/sessionIssueService.spec.ts @@ -23,6 +23,10 @@ vi.mock('../../../src/services/organizationService.js', () => ({ getDefaultOrganizationIdForUser: vi.fn(), })); +vi.mock('../../../src/services/concurrentSessionPolicy.js', () => ({ + enforceConcurrentSessionLimit: vi.fn(), +})); + vi.mock('../../../src/utils/utils.js', () => ({ computeSessionTimes: vi.fn(), parseDurationToSeconds: vi.fn(), @@ -36,6 +40,7 @@ import { signAccessToken, } from '../../../src/lib/token.js'; import { Session } from '../../../src/models/sessions.js'; +import { enforceConcurrentSessionLimit } from '../../../src/services/concurrentSessionPolicy.js'; import { getDefaultOrganizationIdForUser } from '../../../src/services/organizationService.js'; import { computeSessionTimes, parseDurationToSeconds } from '../../../src/utils/utils.js'; @@ -78,6 +83,7 @@ beforeEach(() => { access_token_ttl: '15m', refresh_token_ttl: '1h', session_idle_ttl: '8h', + max_concurrent_sessions: null, }); (parseDurationToSeconds as any).mockImplementation((v: string) => (v === '15m' ? 900 : 3600)); @@ -208,3 +214,34 @@ describe('issueSessionAndRespond', () => { expect(parseDurationToSeconds).toHaveBeenCalledWith('1d'); }); }); + +describe('concurrent session limit', () => { + it('applies the configured limit before the new session is stored', async () => { + (getSystemConfig as any).mockResolvedValue({ + access_token_ttl: '15m', + refresh_token_ttl: '1h', + session_idle_ttl: '8h', + max_concurrent_sessions: 3, + }); + + await issueSessionAndRespond({ user: mockUser, req: mockReq(), res: mockRes() }); + + expect(enforceConcurrentSessionLimit).toHaveBeenCalledWith( + expect.objectContaining({ userId: mockUser.id, limit: 3 }), + ); + + // Ordering is the point: the limit counts the session about to exist, so it + // has to run before the row is created. + const evictionOrder = (enforceConcurrentSessionLimit as any).mock.invocationCallOrder[0]; + const createOrder = (Session.create as any).mock.invocationCallOrder[0]; + expect(evictionOrder).toBeLessThan(createOrder); + }); + + it('passes no limit through when the deployment has not set one', async () => { + await issueSessionAndRespond({ user: mockUser, req: mockReq(), res: mockRes() }); + + expect(enforceConcurrentSessionLimit).toHaveBeenCalledWith( + expect.objectContaining({ limit: null }), + ); + }); +}); diff --git a/tests/unit/utils/parseSystemConfigEnvValue.spec.ts b/tests/unit/utils/parseSystemConfigEnvValue.spec.ts index e1297c0..2aa99a9 100644 --- a/tests/unit/utils/parseSystemConfigEnvValue.spec.ts +++ b/tests/unit/utils/parseSystemConfigEnvValue.spec.ts @@ -145,6 +145,21 @@ describe('parseSystemConfigEnvValue', () => { }); }); + describe('max_concurrent_sessions parsing', () => { + it('parses a numeric limit', () => { + expect(parseSystemConfigEnvValue('max_concurrent_sessions', '5')).toBe(5); + }); + + // A deployment template cannot easily unset a variable, so the words an + // operator would reach for all mean "no limit". + it.each(['', ' ', 'null', 'none', 'unlimited', 'UNLIMITED'])( + 'treats %o as no limit', + (raw) => { + expect(parseSystemConfigEnvValue('max_concurrent_sessions', raw)).toBeNull(); + }, + ); + }); + describe('invalid key', () => { it('throws for unknown key', () => { expect(() => parseSystemConfigEnvValue('invalid_key' as any, 'value')).toThrow(