diff --git a/docs/architecture-decisions/034-api-key-login-auth-mode.md b/docs/architecture-decisions/034-api-key-login-auth-mode.md new file mode 100644 index 000000000..787700dcf --- /dev/null +++ b/docs/architecture-decisions/034-api-key-login-auth-mode.md @@ -0,0 +1,83 @@ +# ADR-034: API-key login auth mode (module-validated) + +**Status**: Proposed +**Date**: 2026-08-20 +**Deciders**: Sean Matthews + +## Context + +Frigg apps that ship a browser SPA today have no first-class way to let an end user **log in with their own product API key** and land in an authenticated, tenant-scoped session. The existing `user.authModes` are: + +- `friggToken` — native username/password → bearer. Requires Frigg to own credentials; no notion of "your product's key is your login." +- `sharedSecret` — `x-frigg-api-key` + `x-frigg-appUserId`/`appOrgId` headers. Correct for backend-to-backend, but the master `x-frigg-api-key` can never live in a browser, so it forces every adopter to stand up a **BFF/token-broker** in front of Frigg (this is exactly what `lefthookhq/aes--frigg`'s `auth-proxy/` does: validate the product key against the product's API, derive an org id, then proxy to Frigg with the shared secret + `x-frigg-apporgid`). +- `adopterJwt` — designed for adopter-verified JWTs, but currently a `501` stub. + +The BFF pattern works and is secure, but it makes every "log in with your product key" app carry a second deployable service, duplicate an allowlist, and hand-roll session/refresh logic. For products where **the API key is already the unit of API authority** (the common case), Frigg can offer this natively. + +Critically, the pieces already exist in core: + +- `modules/use-cases/process-authorization-callback.js` already instantiates a `Module` and its Requester and creates/refreshes the **Credential + Entity** from `{ api_key }` — it is what `POST /api/authorize` runs. +- `user/use-cases/get-user-from-x-frigg-headers.js` already **find-or-creates** an individual/org user from identifiers. +- `login-user` / token minting already issues a Frigg session token. + +What's missing is the glue: validate a pasted key **through the api-module itself**, derive the tenant identity **from the provider** (not the client), find-or-create the user, create the credential/entity, and issue a session — behind a declared auth mode. + +## Decision + +Add a first-class, opt-in `apiKey` auth mode. An app declares which module is its **identity provider**: + +```js +user: { + authModes: { apiKey: { module: 'reevo' } }, + organizationUserRequired: true, // so the org user is created from the provider org id +} +``` + +### Route (reuse `POST /user/login`, polymorphic on credential shape) + +There is **no mode-specific path**. The existing `POST /user/login` becomes polymorphic: it dispatches on the credential shape in the body against the app's enabled `authModes`. + +- `{ username, password }` → `friggToken` (existing behavior, unchanged). +- `{ apiKey }` → the `apiKey` mode (this ADR). The identity module is fixed by config (`authModes.apiKey.module`); a multi-identity app MAY send `{ module, apiKey }` restricted to a configured allowlist. + +Both modes may be enabled simultaneously and coexist on the one route (the bodies are disjoint, so dispatch is unambiguous). The endpoint stays unauthenticated and rate-limited. Rationale: "log in" is the resource; which credential counts is a server-config detail, not something the URL should encode — and adding future modes never adds routes. + +### The use case (`LoginWithApiKey`) + +1. **Validate + identify via the module's Requester.** Instantiate the configured identity module with the supplied key and call its `requiredAuthMethods`: `testAuthRequest` (validity) and `getEntityDetails`/`getCredentialDetails` (identity + the properties to persist). The api-module — not a bespoke validator — is the source of truth for "is this key valid, and whose is it." A `401/403` from the provider → invalid key (generic error, no enumeration); a `5xx`/timeout → provider-unavailable (`503`), distinct from a bad key. + + **`testAuthRequest` login contract (normative).** On this path `testAuthRequest` MUST either **throw** (a provider error the classifier splits into 401 vs 503) or **return the strict boolean `true`**. The validity gate requires `=== true`; any other value — including a truthy error object, a non-empty string, or a response body — is treated as a failed validation (generic 401), NOT a pass. A module that signals a bad key by returning a truthy object instead of a falsy value therefore cannot clear the gate. `getEntityDetails` MUST return a stable **scalar** `identifiers.externalId` (string or number); a non-scalar (object/array/boolean) is rejected as "no stable identifier" rather than coerced. +2. **Derive a provider-authoritative identity.** `appOrgId` (and/or `appUserId`) MUST come from the provider response (e.g. the account/org id `getEntityDetails` returns), NEVER from client input, and MUST be a stable, tenant-unique identifier. Hashing the key (`sha256(apiKey)`) is explicitly disallowed as the identity — it changes on key rotation and orphans connections (the AES R1 flaw). The find-or-create identity is **namespaced by the resolved module name** (`${moduleName}:${externalId}`): in a multi-module allowlist two different providers can legitimately return the same `externalId`, and the namespace keeps those distinct tenants from collapsing onto one Frigg user. +3. **Find-or-create the Frigg user** from that identity, reusing the existing find-or-create path. The issued principal is an ordinary **app user**, never an admin. +4. **Create the Credential + Entity** by running `ProcessAuthorizationCallback(userId, module, { api_key })` — the same path `/api/authorize` uses — so the module's source is connected as part of login. The callback's return is asserted to carry a persisted `credential_id` **before** a session is minted; a callback that returns without one fails the login `500`-class rather than handing out a session over a half-provisioned tenant. (The user is found-or-created before the credential is provisioned; an orphaned user on partial callback failure is accepted cleanup debt, tolerated over reordering.) +5. **Issue a Frigg session token** and return it. Default: an httpOnly, `secure`, `sameSite` cookie plus a short-lived access token; the raw key is **not** returned to or re-sent by the browser after login (it lives only as the encrypted Credential). + +### Trust model (documented, accepted) + +Possession of a valid provider API key confers authority over that tenant's integrations in the Frigg app. This **mirrors** the authority the key already grants at the provider — it is not an escalation. There is no second factor; this is bearer-key trust, appropriate for products whose API key is already the unit of API authority. Adopters whose keys are broad, long-lived, and unrotatable should prefer `friggToken` or an external IdP via a BFF instead. + +## Security requirements (normative — the implementation MUST honor these) + +1. **Provider-authoritative identity.** `appOrgId`/`appUserId` derive only from the module's provider response; a client-supplied org/user id is ignored. Reject a login whose module returns no stable identifier. +2. **Session ≠ admin.** The minted session is a normal app-user token scoped to that tenant; it must not authorize `/user/*` management routes or cross-tenant access. +3. **Rate limiting.** `POST /user/login` (the polymorphic route) is rate-limited (per-IP and global) to prevent using it as a key-validation oracle against the provider. Cap key length before any work. Errors are generic (no user/key enumeration). The per-IP bucket key is derived from a **trusted** X-Forwarded-For position (rightmost by default, or `authModes.apiKey.rateLimit.trustedProxyDepth` hops from the right), NOT the client-controlled leftmost hop, so an attacker cannot rotate a spoofed leftmost XFF to mint a fresh bucket per request. The in-process limiter is a floor: `maxGlobal` is the only hard in-process ceiling (and only per container in a multi-instance deployment). The real per-IP control belongs at the edge (WAF / API Gateway throttling). +4. **Key at rest _and in logs_.** The key is persisted only as the Credential, through Frigg's field-level encryption (KMS/AES). It is never returned after login and never placed in a JWT claim. It is **never logged**: the framework logger (`initDebugLog`) redacts a denylist of credential-bearing request-body/header fields (`apiKey`, `api_key`, `password`, `token`, `authorization`, `refresh_token`, `access_token`) to `[REDACTED]` before the Lambda event is buffered — so the raw key cannot leak via the buffered debug dump on a 5xx nor via `DEBUG_VERBOSE=1`. This also protects the `friggToken` `password` on the shared route. +5. **Revocation latency is bounded by TTL.** Access tokens are short-lived; refresh (if implemented) MUST re-validate the stored key via the module's `testAuthRequest` before rotating, so a revoked provider key stops working within one TTL rather than for the life of a long session. +6. **Outage ≠ invalid.** Provider `5xx`/timeout returns `503` and does not revoke the session or clear cookies; only a definitive `401/403` invalidates. +7. **Cookie hygiene.** `httpOnly`, `secure` (in non-local stages), `sameSite: 'strict'`, and a cookie `Max-Age`/`Expires` aligned to the session-token TTL (cookie and token expire together). The Origin/Referer allowlist (CSRF) is **opt-in** via `authModes.apiKey.allowedOrigins` so it does not hard-break unconfigured local dev; when `apiKey` mode is enabled without it, the framework emits a one-time wiring-time `console.warn` noting that Origin enforcement is off and the `sameSite: 'strict'` cookie is the residual protection. `allowedOrigins`, when present, MUST be an array (validated at wiring time). Adopters serving a browser SPA should configure it. + +## Consequences + +- **Removes the mandatory BFF** for "log in with your product key" apps: the browser talks to Frigg directly (login → cookie → normal calls). The BFF remains the right tool when identity must come from a third-party IdP, or when a proxy is wanted for other reasons. +- **Reusable across every api-key module.** Any module exposing the standard `requiredAuthMethods` gets product-key login for free by naming it in `authModes.apiKey.module`. +- **Default-off, additive.** Apps that don't declare `authModes.apiKey` are unchanged. It composes with the existing modes (an app may keep `friggToken`/`sharedSecret` on). +- **Supersedes** the earlier sketches (`adopterJwt` completion, a bespoke `apiKeyResolver` hook): validating through the api-module is strictly better than a hand-supplied resolver because the module already encodes how to auth-test and identify a key. + +## Scope of the implementing change + +- `packages/core/user/use-cases/login-with-api-key.js` — the new use case (validate via module → derive identity → find-or-create user → ProcessAuthorizationCallback → mint token). Plus wiring in `authenticate-user.js`/the user router for the new mode and route. +- `packages/core/handlers/routers/*` — make the existing `POST /user/login` polymorphic (dispatch `{ apiKey }` → apiKey mode, `{ username, password }` → friggToken unchanged); rate-limited; cookie issuance. +- App-definition `user.authModes.apiKey` config validation + docs. +- Tests: valid key → user+credential+entity created and a session returned; invalid key → 401 generic; provider outage → 503 with no session; **client-supplied org id is ignored** (impersonation guard); rate-limit trips; refresh re-validates; the minted token cannot reach `/user/*`. Mutation-test the impersonation guard and the 401-vs-503 split. + +**Note:** end-to-end validation against a live provider is out of scope for the PR's automated tests (uses a mocked module Requester); the security requirements above are enforced by unit tests on the use case and route. diff --git a/packages/core/handlers/rate-limiter.js b/packages/core/handlers/rate-limiter.js new file mode 100644 index 000000000..9e4ffcef3 --- /dev/null +++ b/packages/core/handlers/rate-limiter.js @@ -0,0 +1,90 @@ +/** + * Minimal, dependency-free, in-process fixed-window rate limiter for the + * unauthenticated login route (ADR-034 §Security requirement 3). + * + * Two independent windows are enforced per call: + * - per key (typically the client IP), and + * - a global counter across all keys. + * + * Purpose is to blunt use of the endpoint as a key-validation oracle against the + * upstream provider. In a multi-instance serverless deployment each container + * holds its own counters, so this is a floor, not a global guarantee — pair it + * with an infra-level limit (API Gateway / WAF) for a hard ceiling. It is + * deliberately self-contained and unit-testable. + * + * @class FixedWindowRateLimiter + */ +class FixedWindowRateLimiter { + /** + * @param {Object} [options] + * @param {number} [options.windowMs=60000] - Window length in milliseconds. + * @param {number} [options.maxPerKey=10] - Max attempts per key per window. + * @param {number} [options.maxGlobal=1000] - Max attempts across all keys per window. + * @param {() => number} [options.now] - Clock (injectable for tests). + */ + constructor({ + windowMs = 60000, + maxPerKey = 10, + maxGlobal = 1000, + now = () => Date.now(), + } = {}) { + this.windowMs = windowMs; + this.maxPerKey = maxPerKey; + this.maxGlobal = maxGlobal; + this.now = now; + this.buckets = new Map(); // key -> { count, windowStart } + this.global = { count: 0, windowStart: 0 }; + } + + _rollGlobal(ts) { + if (ts - this.global.windowStart >= this.windowMs) { + this.global = { count: 0, windowStart: ts }; + } + } + + _rollKey(bucket, ts) { + if (!bucket || ts - bucket.windowStart >= this.windowMs) { + return { count: 0, windowStart: ts }; + } + return bucket; + } + + /** + * Record an attempt for `key` and report whether it is allowed. + * @param {string} key - Identity for the per-key window (e.g. client IP). + * @returns {{ allowed: boolean, scope?: 'key'|'global' }} + */ + check(key) { + const ts = this.now(); + const bucketKey = key || 'unknown'; + + this._rollGlobal(ts); + if (this.global.count >= this.maxGlobal) { + return { allowed: false, scope: 'global' }; + } + + let bucket = this._rollKey(this.buckets.get(bucketKey), ts); + if (bucket.count >= this.maxPerKey) { + this.buckets.set(bucketKey, bucket); + return { allowed: false, scope: 'key' }; + } + + bucket = { count: bucket.count + 1, windowStart: bucket.windowStart }; + this.buckets.set(bucketKey, bucket); + this.global = { + count: this.global.count + 1, + windowStart: this.global.windowStart, + }; + + // Opportunistic cleanup so the Map cannot grow unbounded across windows. + if (this.buckets.size > 10000) { + for (const [k, b] of this.buckets) { + if (ts - b.windowStart >= this.windowMs) this.buckets.delete(k); + } + } + + return { allowed: true }; + } +} + +module.exports = { FixedWindowRateLimiter }; diff --git a/packages/core/handlers/rate-limiter.test.js b/packages/core/handlers/rate-limiter.test.js new file mode 100644 index 000000000..8ca0b1a06 --- /dev/null +++ b/packages/core/handlers/rate-limiter.test.js @@ -0,0 +1,62 @@ +const { FixedWindowRateLimiter } = require('./rate-limiter'); + +describe('FixedWindowRateLimiter', () => { + it('allows up to maxPerKey attempts, then trips per-key', () => { + const rl = new FixedWindowRateLimiter({ + windowMs: 1000, + maxPerKey: 3, + maxGlobal: 100, + now: () => 1000, + }); + + expect(rl.check('1.1.1.1').allowed).toBe(true); + expect(rl.check('1.1.1.1').allowed).toBe(true); + expect(rl.check('1.1.1.1').allowed).toBe(true); + + const fourth = rl.check('1.1.1.1'); + expect(fourth.allowed).toBe(false); + expect(fourth.scope).toBe('key'); + }); + + it('keeps per-key windows independent', () => { + const rl = new FixedWindowRateLimiter({ + windowMs: 1000, + maxPerKey: 1, + maxGlobal: 100, + now: () => 1000, + }); + expect(rl.check('a').allowed).toBe(true); + expect(rl.check('a').allowed).toBe(false); + // A different key is unaffected. + expect(rl.check('b').allowed).toBe(true); + }); + + it('trips the global window across keys even when per-key is fine', () => { + const rl = new FixedWindowRateLimiter({ + windowMs: 1000, + maxPerKey: 100, + maxGlobal: 2, + now: () => 1000, + }); + expect(rl.check('a').allowed).toBe(true); + expect(rl.check('b').allowed).toBe(true); + const third = rl.check('c'); + expect(third.allowed).toBe(false); + expect(third.scope).toBe('global'); + }); + + it('resets counters after the window elapses', () => { + let clock = 1000; + const rl = new FixedWindowRateLimiter({ + windowMs: 1000, + maxPerKey: 1, + maxGlobal: 100, + now: () => clock, + }); + expect(rl.check('a').allowed).toBe(true); + expect(rl.check('a').allowed).toBe(false); + + clock += 1001; // advance past the window + expect(rl.check('a').allowed).toBe(true); + }); +}); diff --git a/packages/core/handlers/routers/user-router.handler-redaction.test.js b/packages/core/handlers/routers/user-router.handler-redaction.test.js new file mode 100644 index 000000000..377d35206 --- /dev/null +++ b/packages/core/handlers/routers/user-router.handler-redaction.test.js @@ -0,0 +1,125 @@ +/** + * Handler-level guard for ADR-034 §4: the raw apiKey (and password) in the + * request body MUST NOT reach any log sink, even on the 5xx / provider-outage + * path where the whole Lambda event is dumped by flushDebugLog. + * + * This drives the apiKey login through the REAL handler stack + * (createAppHandler → createHandler → serverless-http → express router → + * app-handler-helpers error middleware) so it exercises the actual buffering + * and flush, not a stand-in. The use case is stubbed to fail 503 (provider + * unavailable), which is precisely the path that triggers flushDebugLog. + */ + +// DB-free: never load Prisma. (createAppHandler is called with +// shouldUseDatabase=false below, but mock defensively in case that changes.) +jest.mock('../../database/prisma', () => ({ + connectPrisma: jest.fn().mockResolvedValue(undefined), +})); + +const Boom = require('@hapi/boom'); +const { createAppHandler } = require('../app-handler-helpers'); +const { buildUserRouter } = require('./user-router'); +const { FixedWindowRateLimiter } = require('../rate-limiter'); + +const RAW_KEY = 'top-secret-key-DO-NOT-LOG-abc123'; +const RAW_PASSWORD = 'p@ssw0rd-DO-NOT-LOG'; + +function makeApiGatewayEvent(body) { + return { + httpMethod: 'POST', + path: '/user/login', + headers: { + 'Content-Type': 'application/json', + host: 'example.com', + }, + multiValueHeaders: {}, + queryStringParameters: null, + pathParameters: null, + body: JSON.stringify(body), + isBase64Encoded: false, + requestContext: { identity: {}, http: {} }, + }; +} + +describe('apiKey login handler — request-body redaction on 503', () => { + const OLD_STAGE = process.env.STAGE; + let sinks; + let spies; + + beforeEach(() => { + process.env.STAGE = 'test'; + sinks = []; + const capture = + (name) => + (...args) => { + sinks.push(`${name}: ${args.map((a) => String(a)).join(' ')}`); + }; + spies = ['debug', 'error', 'info', 'warn', 'log'].map((m) => + // eslint-disable-next-line no-console + jest.spyOn(console, m).mockImplementation(capture(m)) + ); + }); + + afterEach(() => { + spies.forEach((s) => s.mockRestore()); + process.env.STAGE = OLD_STAGE; + }); + + function buildHandler() { + const loginWithApiKey = { + tokenExpiryMinutes: 120, + execute: jest + .fn() + .mockRejectedValue( + Boom.serverUnavailable('Identity provider unavailable') + ), + }; + const router = buildUserRouter({ + userConfig: { authModes: { apiKey: { module: 'reevo' } } }, + loginUser: { execute: jest.fn() }, + createIndividualUser: { execute: jest.fn() }, + createTokenForUserId: { execute: jest.fn() }, + loginWithApiKey, + apiKeyLoginLimiter: new FixedWindowRateLimiter({ + maxPerKey: 100, + maxGlobal: 1000, + }), + }); + // shouldUseDatabase=false → no Prisma connection needed in the test. + return createAppHandler('HTTP Event: User', router, false); + } + + it('never emits the raw apiKey to any console sink on the 503 path', async () => { + const handler = buildHandler(); + + const res = await handler(makeApiGatewayEvent({ apiKey: RAW_KEY }), { + awsRequestId: 'req-1', + }); + + // The provider-outage path returns 503 (the flushDebugLog trigger). + expect(res.statusCode).toBe(503); + + const allOutput = sinks.join('\n'); + // The whole point: the raw key is nowhere in the logs... + expect(allOutput).not.toContain(RAW_KEY); + // ...but the event WAS buffered and dumped (so this is a real test of + // redaction, not of the event simply being absent). + expect(allOutput).toContain('[REDACTED]'); + }); + + it('never emits the raw password on the same path', async () => { + const handler = buildHandler(); + + // A password body still reaches the apiKey stub only if apiKey present; + // send both so the login dispatches to the (stubbed 503) apiKey branch + // while a password field also rides along in the buffered event. + await handler( + makeApiGatewayEvent({ apiKey: RAW_KEY, password: RAW_PASSWORD }), + { awsRequestId: 'req-2' } + ); + + const allOutput = sinks.join('\n'); + expect(allOutput).not.toContain(RAW_PASSWORD); + expect(allOutput).not.toContain(RAW_KEY); + }); +}); diff --git a/packages/core/handlers/routers/user-router.js b/packages/core/handlers/routers/user-router.js new file mode 100644 index 000000000..e2742279f --- /dev/null +++ b/packages/core/handlers/routers/user-router.js @@ -0,0 +1,226 @@ +const express = require('express'); +const Boom = require('@hapi/boom'); +const { checkRequiredParams } = require('@friggframework/core'); +const catchAsyncError = require('express-async-handler'); + +const LOCAL_STAGES = ['dev', 'test', 'local']; + +/** + * Number of trusted proxies (API Gateway, ALB, CloudFront, …) in front of the + * app. The client IP is taken this many hops from the RIGHT of X-Forwarded-For. + * Defaults to 1 (the single trusted hop AWS API Gateway adds). Only a positive + * finite integer is honored; anything else falls back to 1. + */ +function trustedProxyDepth(userConfig) { + const configured = + userConfig?.authModes?.apiKey?.rateLimit?.trustedProxyDepth; + if ( + typeof configured === 'number' && + Number.isInteger(configured) && + configured > 0 + ) { + return configured; + } + return 1; +} + +/** + * Client IP for the per-IP rate-limit bucket, derived from a TRUSTED position in + * X-Forwarded-For. + * + * X-Forwarded-For is `client, proxy1, …, proxyN`, where each trusted proxy + * APPENDS the address it received the request from. The LEFTMOST entry is + * therefore attacker-controlled (a client can pre-seed it), so keying the limiter + * off `split(',')[0]` let an attacker mint a fresh bucket per request and defeat + * the per-IP cap entirely. We instead read the entry `trustedProxyDepth` hops + * from the right — the value stamped by the first trusted proxy — which the + * client cannot forge. Falls back to the socket address when no XFF is present. + * + * NOTE: `maxGlobal` on the limiter is the only hard in-process ceiling this + * endpoint has, and even that is per-container in a multi-instance serverless + * deployment. The real per-IP control belongs at the edge (WAF / API Gateway + * throttling); this limiter is a floor, not a guarantee. + */ +function getClientIp(req, userConfig) { + const xff = req.headers['x-forwarded-for']; + if (typeof xff === 'string' && xff.length > 0) { + const parts = xff + .split(',') + .map((p) => p.trim()) + .filter(Boolean); + if (parts.length > 0) { + const depth = trustedProxyDepth(userConfig); + const idx = Math.max(0, parts.length - depth); + return parts[idx]; + } + } + return req.ip || req.connection?.remoteAddress || 'unknown'; +} + +/** + * CSRF Origin/Referer allowlist for the cookie-bearing apiKey login (ADR-034 §7). + * Enforced only when the app configures `authModes.apiKey.allowedOrigins`. + * Default (unconfigured): no origin restriction beyond the SameSite cookie — + * documented, and appropriate for token-only (non-cookie) SPA usage. + */ +function assertOriginAllowed(req, userConfig) { + const allowed = userConfig?.authModes?.apiKey?.allowedOrigins; + if (!Array.isArray(allowed) || allowed.length === 0) { + return; // not configured → rely on SameSite; see ADR-034 §7. + } + const origin = req.headers.origin; + const referer = req.headers.referer || req.headers.referrer; + const candidate = + origin || + (referer + ? (() => { + try { + const u = new URL(referer); + return `${u.protocol}//${u.host}`; + } catch { + return null; + } + })() + : null); + + if (!candidate || !allowed.includes(candidate)) { + throw Boom.forbidden('Origin not allowed'); + } +} + +/** + * Set the session cookie with the hygiene ADR-034 §7 requires: httpOnly, secure + * in non-local stages, SameSite. The cookie lifetime is aligned to the session + * token TTL so the browser drops the cookie exactly when the token stops being + * valid (no stale cookie outliving its token, and no token outliving its cookie). + * The access token is ALSO returned in the body so token-only (header-bearer) + * clients work without reading the cookie. + * + * @param {import('express').Response} res + * @param {string} token + * @param {number} [ttlMinutes=120] - Session token TTL; drives Max-Age/Expires. + */ +function setSessionCookie(res, token, ttlMinutes = 120) { + const isLocal = LOCAL_STAGES.includes(process.env.STAGE); + const options = { + httpOnly: true, + secure: !isLocal, + sameSite: 'strict', + path: '/', + }; + // Align cookie lifetime to the token TTL (express sets both Max-Age and + // Expires from maxAge). Guard against a non-positive/NaN TTL. + if (Number.isFinite(ttlMinutes) && ttlMinutes > 0) { + options.maxAge = ttlMinutes * 60 * 1000; + } + res.cookie('frigg_session', token, options); +} + +/** + * Build the user router. Dependencies are injected so the routes can be tested + * in isolation. `POST /user/login` is polymorphic (ADR-034): + * { username, password } → friggToken (unchanged) + * { apiKey } → apiKey mode (module-validated), when enabled. + * + * This module has NO import-time side effects — the production wiring lives in + * `user.js`, which loads the app definition and calls this factory. + * + * @param {Object} deps + * @param {Object} deps.userConfig + * @param {import('../../user/use-cases/login-user').LoginUser} deps.loginUser + * @param {import('../../user/use-cases/create-individual-user').CreateIndividualUser} deps.createIndividualUser + * @param {import('../../user/use-cases/create-token-for-user-id').CreateTokenForUserId} deps.createTokenForUserId + * @param {import('../../user/use-cases/login-with-api-key').LoginWithApiKey|null} deps.loginWithApiKey - null when apiKey mode is off. + * @param {import('../rate-limiter').FixedWindowRateLimiter} deps.apiKeyLoginLimiter + * @returns {express.Router} + */ +function buildUserRouter({ + userConfig, + loginUser, + createIndividualUser, + createTokenForUserId, + loginWithApiKey, + apiKeyLoginLimiter, +}) { + const router = express(); + const apiKeyModeEnabled = Boolean(loginWithApiKey); + + router.route('/user/login').post( + catchAsyncError(async (req, res) => { + const body = req.body || {}; + + // Dispatch: an { apiKey } body selects the apiKey mode. The bodies + // are disjoint, so a password login is never affected by this branch. + if (typeof body.apiKey === 'string') { + // Generic rejection when the mode is not enabled — no enumeration. + if (!apiKeyModeEnabled) { + throw Boom.unauthorized('Invalid credentials'); + } + + // Rate limit BEFORE any provider work (oracle protection). The + // bucket key is derived from a trusted XFF position so a client + // cannot rotate it to escape the per-IP cap. + const { allowed } = apiKeyLoginLimiter.check( + getClientIp(req, userConfig) + ); + if (!allowed) { + throw Boom.tooManyRequests('Too many requests'); + } + + // CSRF: cookie-bearing route. + assertOriginAllowed(req, userConfig); + + const { token } = await loginWithApiKey.execute({ + apiKey: body.apiKey, + module: body.module, + }); + + // Align the cookie lifetime to the minted token's TTL. + setSessionCookie( + res, + token, + loginWithApiKey.tokenExpiryMinutes ?? 120 + ); + res.status(201); + res.json({ token }); + return; + } + + // friggToken path — UNCHANGED. + const { username, password } = checkRequiredParams(req.body, [ + 'username', + 'password', + ]); + const user = await loginUser.execute({ username, password }); + const token = await createTokenForUserId.execute(user.getId(), 120); + res.status(201); + res.json({ token }); + }) + ); + + router.route('/user/create').post( + catchAsyncError(async (req, res) => { + const { username, password } = checkRequiredParams(req.body, [ + 'username', + 'password', + ]); + + const user = await createIndividualUser.execute({ + username, + password, + }); + const token = await createTokenForUserId.execute(user.getId(), 120); + res.status(201); + res.json({ token }); + }) + ); + + return router; +} + +module.exports = { + buildUserRouter, + getClientIp, + assertOriginAllowed, + setSessionCookie, +}; diff --git a/packages/core/handlers/routers/user-router.test.js b/packages/core/handlers/routers/user-router.test.js new file mode 100644 index 000000000..8c306aafb --- /dev/null +++ b/packages/core/handlers/routers/user-router.test.js @@ -0,0 +1,304 @@ +const express = require('express'); +const request = require('supertest'); +const Boom = require('@hapi/boom'); +const { buildUserRouter } = require('./user-router'); +const { FixedWindowRateLimiter } = require('../rate-limiter'); + +// Mount the router under test on a minimal app with JSON parsing and a Boom +// error mapper mirroring app-handler-helpers, so status codes are asserted +// end-to-end. +function mountApp(router) { + const app = express(); + app.use(express.json()); + app.use(router); + app.use((err, req, res, _next) => { + const boom = err.isBoom ? err : Boom.boomify(err); + res.status(boom.output.statusCode).json({ + error: boom.message, + }); + }); + return app; +} + +function makeDeps(overrides = {}) { + const loginUser = { + execute: jest.fn().mockResolvedValue({ getId: () => 'user-1' }), + }; + const createIndividualUser = { + execute: jest.fn().mockResolvedValue({ getId: () => 'user-1' }), + }; + const createTokenForUserId = { + execute: jest.fn().mockResolvedValue('friggtoken-abc'), + }; + const loginWithApiKey = { + execute: jest.fn().mockResolvedValue({ + token: 'apikey-session-xyz', + userId: 'org-1', + module: 'reevo', + }), + }; + const apiKeyLoginLimiter = new FixedWindowRateLimiter({ + windowMs: 60000, + maxPerKey: 3, + maxGlobal: 1000, + }); + + return { + userConfig: { + authModes: { friggToken: true, apiKey: { module: 'reevo' } }, + }, + loginUser, + createIndividualUser, + createTokenForUserId, + loginWithApiKey, + apiKeyLoginLimiter, + ...overrides, + }; +} + +describe('POST /user/login (polymorphic)', () => { + const OLD_STAGE = process.env.STAGE; + beforeAll(() => { + process.env.STAGE = 'test'; // secure cookie off in local stages + }); + afterAll(() => { + process.env.STAGE = OLD_STAGE; + }); + + describe('apiKey body', () => { + it('dispatches { apiKey } to the apiKey mode and returns a token + hardened cookie', async () => { + const deps = makeDeps(); + const app = mountApp(buildUserRouter(deps)); + + const res = await request(app) + .post('/user/login') + .send({ apiKey: 'sk_live_abc' }); + + expect(res.status).toBe(201); + expect(res.body).toEqual({ token: 'apikey-session-xyz' }); + expect(deps.loginWithApiKey.execute).toHaveBeenCalledWith({ + apiKey: 'sk_live_abc', + module: undefined, + }); + // Password path was NOT taken. + expect(deps.loginUser.execute).not.toHaveBeenCalled(); + + // Cookie hygiene (ADR-034 §7). + const cookie = res.headers['set-cookie'][0]; + expect(cookie).toMatch(/^frigg_session=apikey-session-xyz/); + expect(cookie).toMatch(/HttpOnly/i); + expect(cookie).toMatch(/SameSite=Strict/i); + expect(cookie).not.toMatch(/Secure/i); // local stage + }); + + it('passes an allowlisted { module } through to the use case', async () => { + const deps = makeDeps(); + const app = mountApp(buildUserRouter(deps)); + + await request(app) + .post('/user/login') + .send({ apiKey: 'sk_live_abc', module: 'reevo' }) + .expect(201); + + expect(deps.loginWithApiKey.execute).toHaveBeenCalledWith({ + apiKey: 'sk_live_abc', + module: 'reevo', + }); + }); + + it('returns a generic 401 for an { apiKey } body when apiKey mode is disabled', async () => { + const deps = makeDeps({ loginWithApiKey: null }); + const app = mountApp(buildUserRouter(deps)); + + const res = await request(app) + .post('/user/login') + .send({ apiKey: 'sk_live_abc' }); + + expect(res.status).toBe(401); + expect(res.body.error).toBe('Invalid credentials'); + }); + + it('surfaces a 503 from the use case (provider outage) without minting a session', async () => { + const deps = makeDeps(); + deps.loginWithApiKey.execute = jest + .fn() + .mockRejectedValue( + Boom.serverUnavailable('Identity provider unavailable') + ); + const app = mountApp(buildUserRouter(deps)); + + const res = await request(app) + .post('/user/login') + .send({ apiKey: 'sk_live_abc' }); + + expect(res.status).toBe(503); + expect(res.headers['set-cookie']).toBeUndefined(); + }); + + it('trips the rate limit after maxPerKey attempts (429)', async () => { + const deps = makeDeps(); + const app = mountApp(buildUserRouter(deps)); + + // maxPerKey = 3 → 3 allowed, 4th blocked. + for (let i = 0; i < 3; i++) { + await request(app) + .post('/user/login') + .send({ apiKey: 'sk_live_abc' }) + .expect(201); + } + const res = await request(app) + .post('/user/login') + .send({ apiKey: 'sk_live_abc' }); + expect(res.status).toBe(429); + }); + + it('rotating the LEFTMOST X-Forwarded-For hop does NOT mint a fresh bucket (spoof-resistant)', async () => { + const deps = makeDeps(); // maxPerKey = 3 + const app = mountApp(buildUserRouter(deps)); + + // Attacker rotates the client-controlled leftmost hop on every + // request but the trusted rightmost hop (stamped by the proxy) is + // constant. With a trusted-position IP the bucket is shared, so the + // 4th request still trips. (Under the old split(',')[0] behavior each + // request would land in a new bucket and all four would be 201.) + for (let i = 0; i < 3; i++) { + await request(app) + .post('/user/login') + .set('X-Forwarded-For', `10.0.0.${i}, 203.0.113.7`) + .send({ apiKey: 'sk_live_abc' }) + .expect(201); + } + const res = await request(app) + .post('/user/login') + .set('X-Forwarded-For', '10.0.0.99, 203.0.113.7') + .send({ apiKey: 'sk_live_abc' }); + expect(res.status).toBe(429); + }); + + it('honors trustedProxyDepth to pick the client IP N hops from the right', async () => { + const deps = makeDeps({ + userConfig: { + authModes: { + apiKey: { + module: 'reevo', + rateLimit: { trustedProxyDepth: 2 }, + }, + }, + }, + }); + const app = mountApp(buildUserRouter(deps)); + + // XFF = spoof, client, proxy. With 2 trusted hops the client IP is + // the entry 2 from the right (index length-2). Keeping THAT constant + // while the spoofable leftmost hop and the rightmost proxy vary must + // still share a bucket and trip at #4. + for (let i = 0; i < 3; i++) { + await request(app) + .post('/user/login') + .set( + 'X-Forwarded-For', + `10.0.0.${i}, 198.51.100.5, 172.16.0.${i}` + ) + .send({ apiKey: 'sk_live_abc' }) + .expect(201); + } + const res = await request(app) + .post('/user/login') + .set('X-Forwarded-For', '10.0.0.9, 198.51.100.5, 172.16.0.9') + .send({ apiKey: 'sk_live_abc' }); + expect(res.status).toBe(429); + }); + + it('sets a cookie Max-Age aligned to the token TTL', async () => { + const deps = makeDeps(); + deps.loginWithApiKey.tokenExpiryMinutes = 30; + const app = mountApp(buildUserRouter(deps)); + + const res = await request(app) + .post('/user/login') + .send({ apiKey: 'sk_live_abc' }) + .expect(201); + + const cookie = res.headers['set-cookie'][0]; + // 30 minutes = 1800 seconds. + expect(cookie).toMatch(/Max-Age=1800\b/i); + expect(cookie).toMatch(/Expires=/i); + }); + }); + + describe('CSRF origin allowlist', () => { + it('rejects a disallowed Origin with 403 when allowedOrigins is configured', async () => { + const deps = makeDeps({ + userConfig: { + authModes: { + apiKey: { + module: 'reevo', + allowedOrigins: ['https://app.example.com'], + }, + }, + }, + }); + const app = mountApp(buildUserRouter(deps)); + + const res = await request(app) + .post('/user/login') + .set('Origin', 'https://evil.example.com') + .send({ apiKey: 'sk_live_abc' }); + + expect(res.status).toBe(403); + expect(deps.loginWithApiKey.execute).not.toHaveBeenCalled(); + }); + + it('allows a listed Origin', async () => { + const deps = makeDeps({ + userConfig: { + authModes: { + apiKey: { + module: 'reevo', + allowedOrigins: ['https://app.example.com'], + }, + }, + }, + }); + const app = mountApp(buildUserRouter(deps)); + + await request(app) + .post('/user/login') + .set('Origin', 'https://app.example.com') + .send({ apiKey: 'sk_live_abc' }) + .expect(201); + }); + }); + + describe('password body (friggToken) — UNCHANGED when both modes enabled', () => { + it('logs in with { username, password } and never touches the apiKey path', async () => { + const deps = makeDeps(); // both friggToken + apiKey enabled + const app = mountApp(buildUserRouter(deps)); + + const res = await request(app) + .post('/user/login') + .send({ username: 'alice', password: 'pw' }); + + expect(res.status).toBe(201); + expect(res.body).toEqual({ token: 'friggtoken-abc' }); + expect(deps.loginUser.execute).toHaveBeenCalledWith({ + username: 'alice', + password: 'pw', + }); + expect(deps.loginWithApiKey.execute).not.toHaveBeenCalled(); + // No cookie on the (unchanged) password path. + expect(res.headers['set-cookie']).toBeUndefined(); + }); + + it('still 400s a password login missing a field', async () => { + const deps = makeDeps(); + const app = mountApp(buildUserRouter(deps)); + + const res = await request(app) + .post('/user/login') + .send({ username: 'alice' }); + + expect(res.status).toBe(400); + }); + }); +}); diff --git a/packages/core/handlers/routers/user.js b/packages/core/handlers/routers/user.js index 652a5f667..5c684207f 100644 --- a/packages/core/handlers/routers/user.js +++ b/packages/core/handlers/routers/user.js @@ -1,9 +1,19 @@ -const express = require('express'); const { createAppHandler } = require('../app-handler-helpers'); -const { checkRequiredParams } = require('@friggframework/core'); const { createUserRepository, } = require('../../user/repositories/user-repository-factory'); +const { + createModuleRepository, +} = require('../../modules/repositories/module-repository-factory'); +const { + createCredentialRepository, +} = require('../../credential/repositories/credential-repository-factory'); +const { + createIntegrationRepository, +} = require('../../integrations/repositories/integration-repository-factory'); +const { + getModulesDefinitionFromIntegrationClasses, +} = require('../../integrations/utils/map-integration-dto'); const { CreateIndividualUser, } = require('../../user/use-cases/create-individual-user'); @@ -11,52 +21,101 @@ const { LoginUser } = require('../../user/use-cases/login-user'); const { CreateTokenForUserId, } = require('../../user/use-cases/create-token-for-user-id'); -const catchAsyncError = require('express-async-handler'); +const { + GetUserFromXFriggHeaders, +} = require('../../user/use-cases/get-user-from-x-frigg-headers'); +const { LoginWithApiKey } = require('../../user/use-cases/login-with-api-key'); +const { + validateApiKeyAuthMode, +} = require('../../user/use-cases/validate-api-key-auth-mode'); +const { + ProcessAuthorizationCallback, +} = require('../../modules/use-cases/process-authorization-callback'); +const { FixedWindowRateLimiter } = require('../rate-limiter'); const { loadAppDefinition } = require('../app-definition-loader'); +const { buildUserRouter } = require('./user-router'); + +// --------------------------------------------------------------------------- +// Module-scope wiring (production). Kept thin; all logic lives in use cases. +// The route factory (buildUserRouter) is side-effect-free and lives in +// ./user-router.js so it can be unit-tested without loading an app definition. +// --------------------------------------------------------------------------- +const { integrations: integrationClasses, userConfig } = loadAppDefinition(); +const moduleDefinitions = + getModulesDefinitionFromIntegrationClasses(integrationClasses); + +// Fail fast if apiKey mode names a module the app does not have (no-op when off). +validateApiKeyAuthMode(userConfig, moduleDefinitions); + +const apiKeyModeEnabled = Boolean(userConfig?.authModes?.apiKey); -const router = express(); -const { userConfig } = loadAppDefinition(); +// One-time wiring-time warning: apiKey mode is enabled but no Origin/Referer +// allowlist is configured, so the CSRF check (assertOriginAllowed) is a no-op +// and the SameSite=strict session cookie is the only residual protection. This +// is intentionally NOT a hard failure — it must not break unconfigured local +// dev — but adopters serving a browser SPA should set allowedOrigins (ADR-034 §7). +if ( + apiKeyModeEnabled && + !Array.isArray(userConfig?.authModes?.apiKey?.allowedOrigins) +) { + // eslint-disable-next-line no-console + console.warn( + '[Frigg] apiKey auth mode is enabled without user.authModes.apiKey.allowedOrigins. ' + + 'CSRF Origin/Referer enforcement is OFF; the SameSite=strict session cookie is the only ' + + 'residual protection. Set allowedOrigins to a list of trusted browser origins to lock this down (ADR-034 §7).' + ); +} const userRepository = createUserRepository(); const createIndividualUser = new CreateIndividualUser({ userRepository, userConfig, }); -const loginUser = new LoginUser({ - userRepository, - userConfig, -}); +const loginUser = new LoginUser({ userRepository, userConfig }); const createTokenForUserId = new CreateTokenForUserId({ userRepository }); -// define the login endpoint -router.route('/user/login').post( - catchAsyncError(async (req, res) => { - const { username, password } = checkRequiredParams(req.body, [ - 'username', - 'password', - ]); - const user = await loginUser.execute({ username, password }); - const token = await createTokenForUserId.execute(user.getId(), 120); - res.status(201); - res.json({ token }); - }) -); - -router.route('/user/create').post( - catchAsyncError(async (req, res) => { - const { username, password } = checkRequiredParams(req.body, [ - 'username', - 'password', - ]); - - const user = await createIndividualUser.execute({ - username, - password, - }); - const token = await createTokenForUserId.execute(user.getId(), 120); - res.status(201); - res.json({ token }); - }) -); +// apiKey-mode collaborators are only wired when the mode is enabled, so an app +// that never opts in pays nothing and behaves exactly as before. +let loginWithApiKey = null; +if (apiKeyModeEnabled) { + const moduleRepository = createModuleRepository(); + const credentialRepository = createCredentialRepository(); + const integrationRepository = createIntegrationRepository(); + + const getUserFromXFriggHeaders = new GetUserFromXFriggHeaders({ + userRepository, + userConfig, + }); + const processAuthorizationCallback = new ProcessAuthorizationCallback({ + moduleRepository, + credentialRepository, + integrationRepository, + moduleDefinitions, + }); + + loginWithApiKey = new LoginWithApiKey({ + userConfig, + moduleDefinitions, + getUserFromXFriggHeaders, + processAuthorizationCallback, + createTokenForUserId, + }); +} + +const rlConfig = userConfig?.authModes?.apiKey?.rateLimit || {}; +const apiKeyLoginLimiter = new FixedWindowRateLimiter({ + windowMs: rlConfig.windowMs ?? 60000, + maxPerKey: rlConfig.maxPerKey ?? 10, + maxGlobal: rlConfig.maxGlobal ?? 1000, +}); + +const router = buildUserRouter({ + userConfig, + loginUser, + createIndividualUser, + createTokenForUserId, + loginWithApiKey, + apiKeyLoginLimiter, +}); const handler = createAppHandler('HTTP Event: User', router); diff --git a/packages/core/index.js b/packages/core/index.js index f5d2b2764..df0416250 100644 --- a/packages/core/index.js +++ b/packages/core/index.js @@ -31,6 +31,10 @@ const { GetUserFromAdopterJwt, } = require('./user/use-cases/get-user-from-adopter-jwt'); const { AuthenticateUser } = require('./user/use-cases/authenticate-user'); +const { LoginWithApiKey } = require('./user/use-cases/login-with-api-key'); +const { + validateApiKeyAuthMode, +} = require('./user/use-cases/validate-api-key-auth-mode'); const { CredentialRepository, @@ -121,6 +125,8 @@ module.exports = { GetUserFromXFriggHeaders, GetUserFromAdopterJwt, AuthenticateUser, + LoginWithApiKey, + validateApiKeyAuthMode, CredentialRepository, ModuleRepository, IntegrationMappingRepository, diff --git a/packages/core/logs/index.js b/packages/core/logs/index.js index 2a566c855..79abe5ab1 100644 --- a/packages/core/logs/index.js +++ b/packages/core/logs/index.js @@ -1,7 +1,15 @@ -const {debug, initDebugLog, flushDebugLog} = require('./logger'); +const { + debug, + initDebugLog, + flushDebugLog, + redactSensitive, + SENSITIVE_KEYS, +} = require('./logger'); module.exports = { debug, initDebugLog, - flushDebugLog -} \ No newline at end of file + flushDebugLog, + redactSensitive, + SENSITIVE_KEYS, +}; diff --git a/packages/core/logs/logger.js b/packages/core/logs/logger.js index 340263d1d..36448bc05 100644 --- a/packages/core/logs/logger.js +++ b/packages/core/logs/logger.js @@ -6,6 +6,127 @@ const util = require('util'); const logs = []; let flushCalled = false; +/** + * Keys whose values must never reach the logs. Matched case-insensitively. + * These are the credential-bearing fields a request body / headers can carry — + * the apiKey-login body (`{ apiKey }`), the friggToken body (`{ password }`), + * OAuth material, and Authorization headers. Buffered debug output (and the + * verbose `DEBUG_VERBOSE=1` path) is dumped verbatim on any 5xx, so a raw secret + * in `event.body` would otherwise land in CloudWatch (ADR-034 §4). + * @constant {Set} + */ +const SENSITIVE_KEYS = new Set([ + 'apikey', + 'api_key', + 'password', + 'token', + 'authorization', + 'refresh_token', + 'access_token', +]); + +const REDACTED = '[REDACTED]'; +// Bound recursion and body-parse cost so a pathological event can never hang or +// blow the stack inside the logger. The logger must never throw. +const MAX_REDACT_DEPTH = 8; +const MAX_BODY_PARSE_LENGTH = 100000; + +function isSensitiveKey(key) { + return typeof key === 'string' && SENSITIVE_KEYS.has(key.toLowerCase()); +} + +/** + * Substring/regex fallback for a request body we could not (or should not) + * JSON-parse: a non-JSON body, a form-urlencoded body, or one too large to parse + * cheaply. Masks `"key":"value"` (JSON-ish) and `key=value` (form) shapes for the + * denylisted keys. Best-effort — never throws. + */ +function redactBodyStringFallback(body) { + let out = body; + for (const key of SENSITIVE_KEYS) { + // JSON-ish: "apiKey": "secret" -> "apiKey":"[REDACTED]" + out = out.replace( + new RegExp(`("${key}"\\s*:\\s*)"(?:[^"\\\\]|\\\\.)*"`, 'gi'), + `$1"${REDACTED}"` + ); + // Form-urlencoded: apiKey=secret -> apiKey=[REDACTED] + out = out.replace( + new RegExp(`(${key}=)[^&\\s]*`, 'gi'), + `$1${REDACTED}` + ); + } + return out; +} + +/** + * Redact a serialized request `body` string. JSON bodies are parsed, deep-redacted + * and re-serialized; non-JSON / oversized bodies fall back to pattern masking. + * Never throws. + */ +function redactBodyString(body) { + if (body.length <= MAX_BODY_PARSE_LENGTH) { + try { + const parsed = JSON.parse(body); + if (parsed && typeof parsed === 'object') { + return JSON.stringify(redactValue(parsed, 0, new Set())); + } + } catch (_) { + // Not JSON — fall through to pattern masking below. + } + } + return redactBodyStringFallback(body); +} + +/** + * Deep-clone `value`, masking any denylisted key anywhere in the structure and + * redacting an embedded `body` string (the Lambda/API-Gateway convention). + * Returns a NEW object so the caller's data is never mutated; circular refs and + * excessive depth are handled defensively. Never throws (guarded by the public + * `redactSensitive`). + */ +function redactValue(value, depth, seen) { + if (value === null || typeof value !== 'object') { + return value; + } + if (depth > MAX_REDACT_DEPTH || seen.has(value)) { + return value; + } + seen.add(value); + + if (Array.isArray(value)) { + return value.map((v) => redactValue(v, depth + 1, seen)); + } + + const out = {}; + for (const [k, v] of Object.entries(value)) { + if (isSensitiveKey(k)) { + out[k] = REDACTED; + } else if (k.toLowerCase() === 'body' && typeof v === 'string') { + out[k] = redactBodyString(v); + } else { + out[k] = redactValue(v, depth + 1, seen); + } + } + return out; +} + +/** + * Framework-wide redaction applied to anything buffered for logging. Strips + * credential-bearing fields from objects (e.g. a buffered Lambda event) before + * they are serialized. Non-object arguments (the event name, plain strings) pass + * through untouched. Guaranteed not to throw. + * @param {*} value + * @returns {*} + */ +function redactSensitive(value) { + try { + return redactValue(value, 0, new Set()); + } catch (_) { + // A logger must never break the request it is trying to describe. + return value; + } +} + function debug(...messages) { if (messages.length) { const date = new Date(); @@ -25,8 +146,11 @@ function initDebugLog(...initMessages) { // Hacky but fast way to empty an array. logs.length = 0; - // Log initial event - debug(...initMessages); + // Redact credential-bearing fields (e.g. the login request body buffered in + // the Lambda event) BEFORE they are serialized and buffered. This is the one + // choke point every handler passes its raw event through, so masking here + // protects both the buffered dump and the DEBUG_VERBOSE=1 immediate path. + debug(...initMessages.map(redactSensitive)); } function flushDebugLog(error) { @@ -62,4 +186,10 @@ function flushDebugLog(error) { } } -module.exports = { debug, initDebugLog, flushDebugLog }; +module.exports = { + debug, + initDebugLog, + flushDebugLog, + redactSensitive, + SENSITIVE_KEYS, +}; diff --git a/packages/core/logs/logger.test.js b/packages/core/logs/logger.test.js index 2d51d01dc..a65b8c1ec 100644 --- a/packages/core/logs/logger.test.js +++ b/packages/core/logs/logger.test.js @@ -1,4 +1,9 @@ -const { debug, initDebugLog, flushDebugLog } = require('./logger'); +const { + debug, + initDebugLog, + flushDebugLog, + redactSensitive, +} = require('./logger'); const sinon = require('sinon'); const { overrideEnvironment, @@ -73,4 +78,106 @@ describe('Logger', () => { expect(console.debug).toHaveProperty('callCount', 1); expect(console.error).toHaveProperty('callCount', 2); }); + + const printedDebug = () => + console.debug + .getCalls() + .map((c) => c.args.join(' ')) + .join('\n'); + + describe('request-body redaction (ADR-034 §4)', () => { + it('masks a JSON body apiKey buffered in the Lambda event, then dumped on error', () => { + const event = { + httpMethod: 'POST', + path: '/user/login', + headers: { authorization: 'Bearer sk_live_leak_me' }, + body: JSON.stringify({ + apiKey: 'super-secret-key-value', + module: 'reevo', + }), + }; + + initDebugLog('User', event); + flushDebugLog(new Error('boom')); + + const printed = printedDebug(); + expect(printed).not.toContain('super-secret-key-value'); + expect(printed).not.toContain('sk_live_leak_me'); + expect(printed).toContain('[REDACTED]'); + // Non-sensitive fields survive. + expect(printed).toContain('reevo'); + }); + + it('masks a password body too (protects the friggToken path)', () => { + initDebugLog('User', { + body: JSON.stringify({ + username: 'alice', + password: 'hunter2', + }), + }); + flushDebugLog(new Error()); + + const printed = printedDebug(); + expect(printed).not.toContain('hunter2'); + expect(printed).toContain('alice'); + expect(printed).toContain('[REDACTED]'); + }); + + it('redacts under DEBUG_VERBOSE=1 (immediate console path)', () => { + overrideEnvironment({ DEBUG_VERBOSE: '1' }); + + initDebugLog('User', { + body: JSON.stringify({ apiKey: 'verbose-secret' }), + }); + + const printed = printedDebug(); + expect(printed).not.toContain('verbose-secret'); + expect(printed).toContain('[REDACTED]'); + }); + + it('never throws on a non-JSON / form-urlencoded body, and still masks it', () => { + expect(() => { + initDebugLog('User', { + body: 'not-json&apiKey=urlencoded-leak&x=1', + }); + flushDebugLog(new Error()); + }).not.toThrow(); + + const printed = printedDebug(); + expect(printed).not.toContain('urlencoded-leak'); + }); + + it('never throws on a circular event object', () => { + const event = { body: JSON.stringify({ apiKey: 'x' }) }; + event.self = event; // circular + expect(() => { + initDebugLog('User', event); + flushDebugLog(new Error()); + }).not.toThrow(); + }); + }); + + describe('redactSensitive (unit)', () => { + it('masks denylisted keys anywhere in the structure and does not mutate the input', () => { + const input = { + access_token: 'a', + nested: { refresh_token: 'r', keep: 'ok' }, + list: [{ token: 't' }], + }; + const out = redactSensitive(input); + + expect(out.access_token).toBe('[REDACTED]'); + expect(out.nested.refresh_token).toBe('[REDACTED]'); + expect(out.nested.keep).toBe('ok'); + expect(out.list[0].token).toBe('[REDACTED]'); + // Original is untouched (deep clone). + expect(input.access_token).toBe('a'); + }); + + it('passes non-object values through unchanged', () => { + expect(redactSensitive('User')).toBe('User'); + expect(redactSensitive(42)).toBe(42); + expect(redactSensitive(null)).toBe(null); + }); + }); }); diff --git a/packages/core/user/tests/use-cases/login-with-api-key.test.js b/packages/core/user/tests/use-cases/login-with-api-key.test.js new file mode 100644 index 000000000..965f559a0 --- /dev/null +++ b/packages/core/user/tests/use-cases/login-with-api-key.test.js @@ -0,0 +1,748 @@ +// Mock the real Module class used INSIDE ProcessAuthorizationCallback (and as +// LoginWithApiKey's default ModuleClass, which we always override with FakeModule +// anyway). This lets one test exercise the real ProcessAuthorizationCallback +// without a DB or provider. All other tests inject FakeModule and a spy PAC, so +// they never touch this mock. +jest.mock('../../../modules/module', () => ({ + Module: jest.fn().mockImplementation(({ definition }) => ({ + definition, + credential: undefined, + apiClass: { requesterType: 'apiKey' }, + api: {}, + testAuth: jest.fn().mockResolvedValue(true), + apiParamsFromCredential: jest.fn().mockReturnValue({}), + apiParamsFromEntity: jest.fn().mockReturnValue({}), + getName: jest.fn().mockReturnValue('reevo'), + })), +})); + +const { + LoginWithApiKey, + classifyProviderError, +} = require('../../use-cases/login-with-api-key'); +const { + GetUserFromXFriggHeaders, +} = require('../../use-cases/get-user-from-x-frigg-headers'); +const { + CreateTokenForUserId, +} = require('../../use-cases/create-token-for-user-id'); +const { + GetUserFromBearerToken, +} = require('../../use-cases/get-user-from-bearer-token'); +const { TestUserRepository } = require('../doubles/test-user-repository'); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +// A trivial Module stand-in: the use case only needs `.api` from it; all +// validity/identity behaviour is driven by the module definition's +// requiredAuthMethods (jest fns), which is exactly how the real Module exposes +// them (Object.assign(this, definition.requiredAuthMethods)). +class FakeModule { + constructor({ definition }) { + this.definition = definition; + this.api = { + _apiKey: null, + setApiKey(k) { + this._apiKey = k; + }, + }; + } +} + +// An error shaped like a Requester/FetchError with an HTTP status. +function providerError(status) { + return Object.assign(new Error('provider error'), { statusCode: status }); +} + +const PROVIDER_ORG_ID = 'provider-org-123'; +// The Frigg user identity is namespaced by the resolved module name so two +// modules returning the same externalId map to distinct tenants. +const PROVIDER_ORG_IDENTITY = `reevo:${PROVIDER_ORG_ID}`; + +// Org-mode config: identity becomes appOrgId (per ADR-034 example). +const ORG_USER_CONFIG = { + primary: 'organization', + organizationUserRequired: true, + individualUserRequired: false, + authModes: { apiKey: { module: 'reevo' } }, +}; + +function makeModuleDefinition(overrides = {}) { + return { + moduleName: 'reevo', + modelName: 'Reevo', + API: class {}, + requiredAuthMethods: { + setAuthParams: jest.fn().mockResolvedValue({}), + testAuthRequest: jest.fn().mockResolvedValue(true), + // Mirrors a real module: the externalId is provider-authoritative, + // and identifiers.user echoes the userId the caller passes (only set + // by ProcessAuthorizationCallback; undefined during the validate + // step, which reads externalId only). + getEntityDetails: jest.fn( + async (api, params, tokenResponse, userId) => ({ + identifiers: { + externalId: PROVIDER_ORG_ID, + user: userId, + }, + details: { name: 'Provider Org' }, + }) + ), + getCredentialDetails: jest.fn().mockResolvedValue({ + identifiers: { externalId: PROVIDER_ORG_ID }, + details: { api_key: 'x' }, + }), + apiPropertiesToPersist: { credential: [], entity: [] }, + ...overrides, + }, + }; +} + +function buildUseCase({ + userConfig = ORG_USER_CONFIG, + moduleDefinition = makeModuleDefinition(), + processAuthorizationCallback = { + execute: jest.fn().mockResolvedValue({ + credential_id: 'cred-1', + entity_id: 'entity-1', + type: 'reevo', + }), + }, + userRepository = new TestUserRepository({ userConfig }), +} = {}) { + const getUserFromXFriggHeaders = new GetUserFromXFriggHeaders({ + userRepository, + userConfig, + }); + const createTokenForUserId = new CreateTokenForUserId({ userRepository }); + + const useCase = new LoginWithApiKey({ + userConfig, + moduleDefinitions: [moduleDefinition], + getUserFromXFriggHeaders, + processAuthorizationCallback, + createTokenForUserId, + ModuleClass: FakeModule, + }); + + return { + useCase, + userRepository, + moduleDefinition, + processAuthorizationCallback, + userConfig, + }; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('LoginWithApiKey', () => { + describe('valid key', () => { + it('creates the user + credential/entity and returns a session token', async () => { + const { useCase, userRepository, processAuthorizationCallback } = + buildUseCase(); + + const result = await useCase.execute({ apiKey: 'valid-key' }); + + // Session token returned. + expect(typeof result.token).toBe('string'); + expect(result.token.length).toBeGreaterThan(0); + expect(result.module).toBe('reevo'); + + // Credential + Entity created via the SAME path /api/authorize uses, + // and — critically — with { api_key }, never the raw client body. + expect(processAuthorizationCallback.execute).toHaveBeenCalledTimes( + 1 + ); + const [userIdArg, moduleArg, paramsArg] = + processAuthorizationCallback.execute.mock.calls[0]; + expect(moduleArg).toBe('reevo'); + expect(paramsArg).toEqual({ api_key: 'valid-key' }); + expect(userIdArg).toBe(result.userId); + + // The Frigg user was found-or-created from the PROVIDER identity, + // namespaced by the resolved module name. + const org = await userRepository.findOrganizationUserByAppOrgId( + PROVIDER_ORG_IDENTITY + ); + expect(org).toBeTruthy(); + expect(org.id).toBe(result.userId); + }); + + it('drives credential + entity creation through the authorize path (real ProcessAuthorizationCallback over in-memory doubles)', async () => { + // The real ProcessAuthorizationCallback, with a stubbed Module so no + // DB/provider is touched, proving the login actually persists a + // Credential and an Entity keyed on the provider identity. + const { + ProcessAuthorizationCallback, + } = require('../../../modules/use-cases/process-authorization-callback'); + + const created = { credentials: [], entities: [] }; + const credentialRepository = { + upsertCredential: jest.fn(async (details) => { + const cred = { + id: 'cred-1', + ...details, + authIsValid: true, + }; + created.credentials.push(cred); + return cred; + }), + }; + const moduleRepository = { + findEntitiesByUserIdAndModuleName: jest + .fn() + .mockResolvedValue([]), + findEntity: jest.fn().mockResolvedValue(null), + createEntity: jest.fn(async (entity) => { + const e = { id: 'entity-1', ...entity }; + created.entities.push(e); + return e; + }), + updateEntity: jest.fn(), + }; + + const moduleDefinition = makeModuleDefinition(); + const processAuthorizationCallback = + new ProcessAuthorizationCallback({ + moduleRepository, + credentialRepository, + moduleDefinitions: [moduleDefinition], + }); + + const { useCase } = buildUseCase({ + moduleDefinition, + processAuthorizationCallback, + }); + + const result = await useCase.execute({ apiKey: 'valid-key' }); + + expect(created.credentials).toHaveLength(1); + expect(created.entities).toHaveLength(1); + expect(created.entities[0].externalId).toBe(PROVIDER_ORG_ID); + expect(result.token).toBeTruthy(); + }); + }); + + describe('impersonation guard (identity is provider-authoritative)', () => { + it('IGNORES a client-supplied appOrgId/appUserId — identity comes only from getEntityDetails', async () => { + const { useCase, userRepository } = buildUseCase(); + + // Hostile client tries to steer the identity. execute() must ignore + // everything but { apiKey, module }. + const result = await useCase.execute({ + apiKey: 'valid-key', + appOrgId: 'attacker-org', + appUserId: 'attacker-user', + organizationUser: { appOrgId: 'attacker-org' }, + }); + + // The user is bound to the PROVIDER org id, never the attacker's. + const provider = + await userRepository.findOrganizationUserByAppOrgId( + PROVIDER_ORG_IDENTITY + ); + expect(provider).toBeTruthy(); + expect(provider.id).toBe(result.userId); + + // The attacker's org id was never created. + const attacker = + await userRepository.findOrganizationUserByAppOrgId( + 'attacker-org' + ); + expect(attacker).toBeFalsy(); + }); + + it('MUTATION CHECK: if identity were taken from client input, this assertion would fail', async () => { + // Provider returns a DIFFERENT id than the client sends. The created + // user must track the provider value. (If the implementation ever + // read client input, result.userId would map to the client id.) + const moduleDefinition = makeModuleDefinition({ + getEntityDetails: jest.fn().mockResolvedValue({ + identifiers: { externalId: 'provider-authoritative-999' }, + details: {}, + }), + }); + const { useCase, userRepository } = buildUseCase({ + moduleDefinition, + }); + + const result = await useCase.execute({ + apiKey: 'valid-key', + appOrgId: 'client-supplied-000', + }); + + const provider = + await userRepository.findOrganizationUserByAppOrgId( + 'reevo:provider-authoritative-999' + ); + expect(provider.id).toBe(result.userId); + expect( + await userRepository.findOrganizationUserByAppOrgId( + 'client-supplied-000' + ) + ).toBeFalsy(); + }); + }); + + describe('invalid key', () => { + it('returns a generic 401 and creates no user / credential / session', async () => { + const moduleDefinition = makeModuleDefinition({ + testAuthRequest: jest + .fn() + .mockRejectedValue(providerError(401)), + }); + const { useCase, userRepository, processAuthorizationCallback } = + buildUseCase({ moduleDefinition }); + + await expect( + useCase.execute({ apiKey: 'bad-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + + expect(processAuthorizationCallback.execute).not.toHaveBeenCalled(); + expect( + await userRepository.findOrganizationUserByAppOrgId( + PROVIDER_ORG_ID + ) + ).toBeFalsy(); + }); + + it('treats a 403 the same as a 401 (both definitive rejections)', async () => { + const moduleDefinition = makeModuleDefinition({ + testAuthRequest: jest + .fn() + .mockRejectedValue(providerError(403)), + }); + const { useCase } = buildUseCase({ moduleDefinition }); + + await expect( + useCase.execute({ apiKey: 'bad-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + }); + + it('a falsy (non-throwing) testAuthRequest is also a generic 401', async () => { + const moduleDefinition = makeModuleDefinition({ + testAuthRequest: jest.fn().mockResolvedValue(false), + }); + const { useCase } = buildUseCase({ moduleDefinition }); + + await expect( + useCase.execute({ apiKey: 'bad-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + }); + }); + + describe('provider outage (401-vs-503 split)', () => { + it('returns 503 (not 401) on a provider 5xx, and creates no session', async () => { + const moduleDefinition = makeModuleDefinition({ + testAuthRequest: jest + .fn() + .mockRejectedValue(providerError(503)), + }); + const { useCase, processAuthorizationCallback } = buildUseCase({ + moduleDefinition, + }); + + await expect( + useCase.execute({ apiKey: 'any-key' }) + ).rejects.toMatchObject({ output: { statusCode: 503 } }); + + expect(processAuthorizationCallback.execute).not.toHaveBeenCalled(); + }); + + it('returns 503 on a timeout / network error with no status', async () => { + const moduleDefinition = makeModuleDefinition({ + testAuthRequest: jest + .fn() + .mockRejectedValue(new Error('ETIMEDOUT')), + }); + const { useCase } = buildUseCase({ moduleDefinition }); + + await expect( + useCase.execute({ apiKey: 'any-key' }) + ).rejects.toMatchObject({ output: { statusCode: 503 } }); + }); + + it('MUTATION CHECK: classifyProviderError splits 4xx→invalid, 5xx/none→unavailable', () => { + expect(classifyProviderError(providerError(401))).toBe('invalid'); + expect(classifyProviderError(providerError(403))).toBe('invalid'); + expect(classifyProviderError(providerError(429))).toBe('invalid'); + expect(classifyProviderError(providerError(500))).toBe( + 'unavailable' + ); + expect(classifyProviderError(providerError(503))).toBe( + 'unavailable' + ); + expect(classifyProviderError(new Error('socket hang up'))).toBe( + 'unavailable' + ); + }); + }); + + describe('no stable identifier', () => { + it('rejects when getEntityDetails returns no externalId', async () => { + const moduleDefinition = makeModuleDefinition({ + getEntityDetails: jest.fn().mockResolvedValue({ + identifiers: {}, + details: {}, + }), + }); + const { useCase, processAuthorizationCallback } = buildUseCase({ + moduleDefinition, + }); + + await expect( + useCase.execute({ apiKey: 'valid-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + expect(processAuthorizationCallback.execute).not.toHaveBeenCalled(); + }); + + it('rejects on an empty-string identifier', async () => { + const moduleDefinition = makeModuleDefinition({ + getEntityDetails: jest.fn().mockResolvedValue({ + identifiers: { externalId: ' ' }, + details: {}, + }), + }); + const { useCase } = buildUseCase({ moduleDefinition }); + await expect( + useCase.execute({ apiKey: 'valid-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + }); + }); + + describe('input hardening', () => { + it('caps key length before any provider work (no testAuthRequest call)', async () => { + const moduleDefinition = makeModuleDefinition(); + const { useCase } = buildUseCase({ moduleDefinition }); + + const huge = 'a'.repeat(8193); + await expect( + useCase.execute({ apiKey: huge }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + + expect( + moduleDefinition.requiredAuthMethods.testAuthRequest + ).not.toHaveBeenCalled(); + }); + + it('rejects a missing/empty apiKey generically', async () => { + const { useCase } = buildUseCase(); + await expect(useCase.execute({})).rejects.toMatchObject({ + output: { statusCode: 401 }, + }); + await expect(useCase.execute({ apiKey: '' })).rejects.toMatchObject( + { output: { statusCode: 401 } } + ); + }); + + it('rejects generically when apiKey mode is not configured', async () => { + const { useCase } = buildUseCase({ + userConfig: { + primary: 'organization', + organizationUserRequired: true, + individualUserRequired: false, + authModes: { friggToken: true }, // no apiKey + }, + }); + await expect( + useCase.execute({ apiKey: 'valid-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + }); + }); + + describe('multi-identity module allowlist', () => { + const MULTI_CONFIG = { + primary: 'organization', + organizationUserRequired: true, + individualUserRequired: false, + authModes: { apiKey: { modules: ['reevo', 'acme'] } }, + }; + + it('accepts an allowlisted module named in the body', async () => { + const moduleDefinition = makeModuleDefinition(); + const { useCase } = buildUseCase({ + userConfig: MULTI_CONFIG, + moduleDefinition, + }); + const result = await useCase.execute({ + apiKey: 'valid-key', + module: 'reevo', + }); + expect(result.module).toBe('reevo'); + }); + + it('rejects a non-allowlisted module generically', async () => { + const moduleDefinition = makeModuleDefinition(); + const { useCase } = buildUseCase({ + userConfig: MULTI_CONFIG, + moduleDefinition, + }); + await expect( + useCase.execute({ apiKey: 'valid-key', module: 'evil-module' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + }); + + it('rejects when multiple modules are configured but none is named', async () => { + const moduleDefinition = makeModuleDefinition(); + const { useCase } = buildUseCase({ + userConfig: MULTI_CONFIG, + moduleDefinition, + }); + await expect( + useCase.execute({ apiKey: 'valid-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + }); + + it('resolves via `module` fallback when `modules` is an empty array (union semantics agree with validation)', async () => { + const moduleDefinition = makeModuleDefinition(); + const { useCase } = buildUseCase({ + userConfig: { + primary: 'organization', + organizationUserRequired: true, + individualUserRequired: false, + authModes: { apiKey: { modules: [], module: 'reevo' } }, + }, + moduleDefinition, + }); + const result = await useCase.execute({ apiKey: 'valid-key' }); + expect(result.module).toBe('reevo'); + expect(result.token).toBeTruthy(); + }); + }); + + describe('session scope (ordinary app user, not admin)', () => { + it('mints a normal, tenant-scoped, short-lived session token — resolvable as the same app user', async () => { + const userRepository = new TestUserRepository({ + userConfig: ORG_USER_CONFIG, + }); + const { useCase } = buildUseCase({ userRepository }); + + const { token, userId } = await useCase.execute({ + apiKey: 'valid-key', + }); + + // Short-lived by construction (default 120-min TTL). + expect(token).toContain('for-120-mins'); + + // The token resolves back to the SAME tenant app user — no elevation, + // no cross-tenant reach. This is the identical bearer path a normal + // friggToken user takes; there is no admin capability attached. + const getUserFromBearerToken = new GetUserFromBearerToken({ + userRepository, + userConfig: ORG_USER_CONFIG, + }); + const resolved = await getUserFromBearerToken.execute( + `Bearer ${token}` + ); + + expect(resolved.getId()).toBe(userId); + expect(resolved.getAppOrgId()).toBe(PROVIDER_ORG_IDENTITY); + // No admin/role field is set anywhere on the principal. + expect(resolved.organizationUser.isAdmin).toBeUndefined(); + expect(resolved.organizationUser.role).toBeUndefined(); + }); + }); + + describe('module-namespaced identity (multi-module allowlist collision)', () => { + // A subclass of the double whose ids increment deterministically, so two + // org users created in the same millisecond cannot collide on `Date.now()` + // — the collision we are proving is namespace-driven, not clock-driven. + class CountingUserRepository extends TestUserRepository { + constructor(args) { + super(args); + this._seq = 0; + } + async createOrganizationUser(params) { + const orgUserData = { + ...params, + id: `org-${(this._seq += 1)}`, + }; + this.organizationUsers.set(orgUserData.id, orgUserData); + return orgUserData; + } + } + + const MULTI_CONFIG = { + primary: 'organization', + organizationUserRequired: true, + individualUserRequired: false, + authModes: { apiKey: { modules: ['reevo', 'acme'] } }, + }; + + function moduleDefReturning(moduleName, externalId) { + const def = makeModuleDefinition({ + getEntityDetails: jest.fn().mockResolvedValue({ + identifiers: { externalId }, + details: {}, + }), + }); + def.moduleName = moduleName; + def.modelName = moduleName; + return def; + } + + it('two modules returning the SAME externalId produce DISTINCT Frigg userIds', async () => { + const sharedId = 'shared-account-42'; + const reevoDef = moduleDefReturning('reevo', sharedId); + const acmeDef = moduleDefReturning('acme', sharedId); + + const userRepository = new CountingUserRepository({ + userConfig: MULTI_CONFIG, + }); + const getUserFromXFriggHeaders = new GetUserFromXFriggHeaders({ + userRepository, + userConfig: MULTI_CONFIG, + }); + const createTokenForUserId = new CreateTokenForUserId({ + userRepository, + }); + const processAuthorizationCallback = { + execute: jest.fn().mockResolvedValue({ + credential_id: 'cred-1', + entity_id: 'entity-1', + }), + }; + + const useCase = new LoginWithApiKey({ + userConfig: MULTI_CONFIG, + moduleDefinitions: [reevoDef, acmeDef], + getUserFromXFriggHeaders, + processAuthorizationCallback, + createTokenForUserId, + ModuleClass: FakeModule, + }); + + const r1 = await useCase.execute({ + apiKey: 'reevo-key', + module: 'reevo', + }); + const r2 = await useCase.execute({ + apiKey: 'acme-key', + module: 'acme', + }); + + // The whole point: identical provider externalId, DIFFERENT tenants. + // (Drop the `${moduleName}:` prefix in the use case and these become + // the same user — the mutation this test guards.) + expect(r1.userId).toBeTruthy(); + expect(r2.userId).toBeTruthy(); + expect(r1.userId).not.toBe(r2.userId); + + expect( + await userRepository.findOrganizationUserByAppOrgId( + `reevo:${sharedId}` + ) + ).toBeTruthy(); + expect( + await userRepository.findOrganizationUserByAppOrgId( + `acme:${sharedId}` + ) + ).toBeTruthy(); + // The bare (un-namespaced) identity was never used as a key. + expect( + await userRepository.findOrganizationUserByAppOrgId(sharedId) + ).toBeFalsy(); + }); + }); + + describe('strict testAuthRequest gate (truthy != pass)', () => { + it('a truthy error OBJECT from testAuthRequest does not clear the gate (401)', async () => { + const moduleDefinition = makeModuleDefinition({ + // A module that returns a truthy value (an error object) instead + // of throwing on a bad key must still be rejected. + testAuthRequest: jest + .fn() + .mockResolvedValue({ error: 'nope', ok: false }), + }); + const { useCase, processAuthorizationCallback } = buildUseCase({ + moduleDefinition, + }); + + await expect( + useCase.execute({ apiKey: 'bad-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + // getEntityDetails / credential creation must never be reached. + expect( + moduleDefinition.requiredAuthMethods.getEntityDetails + ).not.toHaveBeenCalled(); + expect(processAuthorizationCallback.execute).not.toHaveBeenCalled(); + }); + + it('a truthy non-boolean (non-empty string) also fails the gate (401)', async () => { + const moduleDefinition = makeModuleDefinition({ + testAuthRequest: jest.fn().mockResolvedValue('valid'), + }); + const { useCase } = buildUseCase({ moduleDefinition }); + await expect( + useCase.execute({ apiKey: 'bad-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + }); + }); + + describe('non-scalar externalId is rejected, not coerced', () => { + it('an object externalId → 401 and creates no user', async () => { + const moduleDefinition = makeModuleDefinition({ + getEntityDetails: jest.fn().mockResolvedValue({ + identifiers: { externalId: {} }, + details: {}, + }), + }); + const { useCase, userRepository, processAuthorizationCallback } = + buildUseCase({ moduleDefinition }); + + await expect( + useCase.execute({ apiKey: 'valid-key' }) + ).rejects.toMatchObject({ output: { statusCode: 401 } }); + + expect(processAuthorizationCallback.execute).not.toHaveBeenCalled(); + // No user keyed on the coerced "[object Object]" string. + expect( + await userRepository.findOrganizationUserByAppOrgId( + 'reevo:[object Object]' + ) + ).toBeFalsy(); + }); + + it('a numeric externalId is accepted (scalar identity)', async () => { + const moduleDefinition = makeModuleDefinition({ + getEntityDetails: jest.fn().mockResolvedValue({ + identifiers: { externalId: 42 }, + details: {}, + }), + }); + const { useCase, userRepository } = buildUseCase({ + moduleDefinition, + }); + const result = await useCase.execute({ apiKey: 'valid-key' }); + expect( + await userRepository.findOrganizationUserByAppOrgId('reevo:42') + ).toBeTruthy(); + expect(result.userId).toBeTruthy(); + }); + }); + + describe('credential must exist before a session is minted', () => { + it('throws a 500-class error (no token) when the callback returns no credential_id', async () => { + const processAuthorizationCallback = { + // Simulates a callback that ran but did not persist a credential. + execute: jest.fn().mockResolvedValue({ entity_id: 'e-1' }), + }; + const { useCase } = buildUseCase({ processAuthorizationCallback }); + + await expect( + useCase.execute({ apiKey: 'valid-key' }) + ).rejects.toMatchObject({ output: { statusCode: 500 } }); + }); + + it('mints a token when the callback returns a credential_id', async () => { + const { useCase } = buildUseCase(); + const result = await useCase.execute({ apiKey: 'valid-key' }); + expect(result.token).toBeTruthy(); + }); + }); +}); diff --git a/packages/core/user/tests/use-cases/validate-api-key-auth-mode.test.js b/packages/core/user/tests/use-cases/validate-api-key-auth-mode.test.js new file mode 100644 index 000000000..aed38048a --- /dev/null +++ b/packages/core/user/tests/use-cases/validate-api-key-auth-mode.test.js @@ -0,0 +1,168 @@ +const { + validateApiKeyAuthMode, +} = require('../../use-cases/validate-api-key-auth-mode'); + +const modules = [{ moduleName: 'reevo' }, { moduleName: 'acme' }]; + +describe('validateApiKeyAuthMode', () => { + it('is a no-op when apiKey mode is not configured (default-off)', () => { + expect(() => validateApiKeyAuthMode({}, modules)).not.toThrow(); + expect(() => validateApiKeyAuthMode(null, modules)).not.toThrow(); + expect(() => + validateApiKeyAuthMode({ authModes: { friggToken: true } }, modules) + ).not.toThrow(); + }); + + it('passes when the single configured module exists', () => { + expect(() => + validateApiKeyAuthMode( + { authModes: { apiKey: { module: 'reevo' } } }, + modules + ) + ).not.toThrow(); + }); + + it('passes when every module in a modules allowlist exists', () => { + expect(() => + validateApiKeyAuthMode( + { authModes: { apiKey: { modules: ['reevo', 'acme'] } } }, + modules + ) + ).not.toThrow(); + }); + + it('throws when apiKey mode is enabled but names no module', () => { + expect(() => + validateApiKeyAuthMode({ authModes: { apiKey: {} } }, modules) + ).toThrow(/names no identity module/); + }); + + it('throws when the configured module is not registered', () => { + expect(() => + validateApiKeyAuthMode( + { authModes: { apiKey: { module: 'ghost' } } }, + modules + ) + ).toThrow(/'ghost' is not a registered module/); + }); + + it('throws when any allowlisted module is missing', () => { + expect(() => + validateApiKeyAuthMode( + { authModes: { apiKey: { modules: ['reevo', 'ghost'] } } }, + modules + ) + ).toThrow(/'ghost' is not a registered module/); + }); + + // ---- (4a) empty allowlist --------------------------------------------- + it('throws on modules: [] with no module (empty allowlist)', () => { + expect(() => + validateApiKeyAuthMode( + { authModes: { apiKey: { modules: [] } } }, + modules + ) + ).toThrow(/names no identity module/); + }); + + it('accepts modules: [] WITH a module fallback (union semantics)', () => { + expect(() => + validateApiKeyAuthMode( + { authModes: { apiKey: { modules: [], module: 'reevo' } } }, + modules + ) + ).not.toThrow(); + }); + + // ---- (4b) allowedOrigins must be an array ----------------------------- + it('throws when allowedOrigins is a bare string, not an array', () => { + expect(() => + validateApiKeyAuthMode( + { + authModes: { + apiKey: { + module: 'reevo', + allowedOrigins: 'https://app.example.com', + }, + }, + }, + modules + ) + ).toThrow(/allowedOrigins must be an array/); + }); + + it('accepts an array allowedOrigins', () => { + expect(() => + validateApiKeyAuthMode( + { + authModes: { + apiKey: { + module: 'reevo', + allowedOrigins: ['https://app.example.com'], + }, + }, + }, + modules + ) + ).not.toThrow(); + }); + + // ---- (4c) rateLimit numeric fields must be positive finite ------------ + it.each([ + ['maxPerKey', 0], + ['maxPerKey', -5], + ['maxGlobal', 0], + ['windowMs', -1], + ['windowMs', NaN], + ['maxGlobal', Infinity], + ])('throws when rateLimit.%s is %p', (field, value) => { + expect(() => + validateApiKeyAuthMode( + { + authModes: { + apiKey: { + module: 'reevo', + rateLimit: { [field]: value }, + }, + }, + }, + modules + ) + ).toThrow( + new RegExp(`rateLimit\\.${field} must be a positive finite number`) + ); + }); + + it('accepts positive finite rateLimit values', () => { + expect(() => + validateApiKeyAuthMode( + { + authModes: { + apiKey: { + module: 'reevo', + rateLimit: { + maxPerKey: 10, + maxGlobal: 1000, + windowMs: 60000, + }, + }, + }, + }, + modules + ) + ).not.toThrow(); + }); + + it('throws when rateLimit is not an object', () => { + expect(() => + validateApiKeyAuthMode( + { + authModes: { + apiKey: { module: 'reevo', rateLimit: 'fast' }, + }, + }, + modules + ) + ).toThrow(/rateLimit must be an object/); + }); +}); diff --git a/packages/core/user/use-cases/login-with-api-key.js b/packages/core/user/use-cases/login-with-api-key.js new file mode 100644 index 000000000..a86b10ca1 --- /dev/null +++ b/packages/core/user/use-cases/login-with-api-key.js @@ -0,0 +1,325 @@ +const Boom = require('@hapi/boom'); +const { Module } = require('../../modules/module'); + +/** + * Maximum accepted length of a submitted API key. Capped before any provider + * work so the endpoint cannot be used to smuggle arbitrarily large payloads or + * amplify an oracle attack. Generous enough for JWT-shaped or concatenated keys. + * @constant {number} + */ +const DEFAULT_MAX_API_KEY_LENGTH = 8192; + +/** + * Classify an error thrown by the module's Requester while validating or + * identifying a key, per ADR-034 §Security requirement 6 ("Outage ≠ invalid"). + * + * A definitive provider rejection (401/403, or any other non-5xx client error) + * means the key is bad. A 5xx, a network failure, or a timeout means the + * provider is unavailable and MUST NOT be reported as an invalid key nor allowed + * to mint a session. + * + * @param {*} err - The thrown error. + * @returns {'invalid'|'unavailable'} classification + */ +function classifyProviderError(err) { + const status = + err?.statusCode ?? err?.response?.status ?? err?.status ?? undefined; + + if (typeof status === 'number' && status >= 400 && status < 500) { + // 401/403 and any other definitive 4xx from the provider → bad key. + return 'invalid'; + } + // 5xx, or no status at all (timeout / DNS / socket error) → outage. + return 'unavailable'; +} + +/** + * Generic invalid-credentials error. Deliberately identical for every failure + * reason on the bad-key path so the endpoint never enumerates users or keys + * (ADR-034 §Security requirement 3). + * @returns {Boom} 401 + */ +function invalidCredentials() { + return Boom.unauthorized('Invalid credentials'); +} + +/** + * Provider-unavailable error, distinct from invalid credentials. No session is + * created and no cookie is cleared when this is thrown (ADR-034 §6). + * @returns {Boom} 503 + */ +function providerUnavailable() { + return Boom.serverUnavailable('Identity provider unavailable'); +} + +/** + * Use case implementing the ADR-034 `apiKey` auth mode: log a browser end user + * in with their own product API key, validated *through the api-module itself*. + * + * Flow (see ADR-034): + * 1. Validate + identify the key via the configured identity module's + * Requester (`testAuthRequest`, then `getEntityDetails`). + * 2. Derive a provider-authoritative tenant identity — NEVER from client input. + * 3. Find-or-create the Frigg user from that identity (ordinary app user). + * 4. Create the Credential + Entity via `ProcessAuthorizationCallback`. + * 5. Mint a short-lived Frigg session token and return it. + * + * @class LoginWithApiKey + */ +class LoginWithApiKey { + /** + * @param {Object} params + * @param {Object} params.userConfig - App-definition `user` config (reads `authModes.apiKey`). + * @param {Array} params.moduleDefinitions - Module definitions available to the app. + * @param {import('./get-user-from-x-frigg-headers').GetUserFromXFriggHeaders} params.getUserFromXFriggHeaders - Reused find-or-create path. + * @param {import('../../modules/use-cases/process-authorization-callback').ProcessAuthorizationCallback} params.processAuthorizationCallback - Reused credential/entity creation. + * @param {import('./create-token-for-user-id').CreateTokenForUserId} params.createTokenForUserId - Reused session-token minting. + * @param {number} [params.tokenExpiryMinutes=120] - Access token TTL. Short by design (ADR-034 §5): revocation latency is bounded by this. + * @param {number} [params.maxApiKeyLength=8192] - Length cap enforced before any provider work. + * @param {typeof Module} [params.ModuleClass=Module] - Injectable Module class (for testing without a real Requester). + */ + constructor({ + userConfig, + moduleDefinitions, + getUserFromXFriggHeaders, + processAuthorizationCallback, + createTokenForUserId, + tokenExpiryMinutes = 120, + maxApiKeyLength = DEFAULT_MAX_API_KEY_LENGTH, + ModuleClass = Module, + }) { + this.userConfig = userConfig || {}; + this.moduleDefinitions = moduleDefinitions || []; + this.getUserFromXFriggHeaders = getUserFromXFriggHeaders; + this.processAuthorizationCallback = processAuthorizationCallback; + this.createTokenForUserId = createTokenForUserId; + this.tokenExpiryMinutes = tokenExpiryMinutes; + this.maxApiKeyLength = maxApiKeyLength; + this.ModuleClass = ModuleClass; + } + + /** + * Resolve which identity module a request may use. + * + * The module is fixed by config (`authModes.apiKey.module`). A multi-identity + * app MAY configure an allowlist (`authModes.apiKey.modules`) and let the + * client pick one via the request body — but only from that allowlist. A + * client-supplied module that is not allowlisted is rejected generically, so + * the endpoint cannot be steered at an arbitrary module. + * + * @param {string} [requestedModule] - Optional module name from the request body. + * @returns {string} The resolved module name. + * @throws {Boom} 401 generic if apiKey mode is unconfigured or the request names a non-allowlisted module. + */ + resolveModuleName(requestedModule) { + const config = this.userConfig.authModes?.apiKey; + if (!config) { + // apiKey mode not enabled for this app. + throw invalidCredentials(); + } + + // Allowlist = the UNION of the explicit `modules` array and the single + // `module` (matching validateApiKeyAuthMode's own union). This keeps the + // resolver and the wiring-time validator in agreement: a config like + // `{ modules: [], module: 'reevo' }` passes validation AND resolves to + // ['reevo'] here, rather than validation passing while the resolver saw + // an empty list and rejected every login. Never an open set. + const allowlist = [ + ...(Array.isArray(config.modules) ? config.modules : []), + ...(config.module ? [config.module] : []), + ]; + + if (allowlist.length === 0) { + throw invalidCredentials(); + } + + if (requestedModule) { + if (!allowlist.includes(requestedModule)) { + throw invalidCredentials(); + } + return requestedModule; + } + + // No client-specified module: only unambiguous when exactly one is configured. + if (allowlist.length === 1) { + return allowlist[0]; + } + + // Multi-identity app but the client did not name a module. + throw invalidCredentials(); + } + + /** + * Validate the key against the provider and derive a provider-authoritative + * tenant identity. Reused by login (and available for refresh re-validation, + * ADR-034 §5): a revoked key stops validating here. + * + * @param {string} apiKey + * @param {string} moduleName + * @returns {Promise<{ externalId: string, moduleDefinition: Object }>} + * @throws {Boom} 401 invalid credentials on a bad/rejected key or missing identifier; 503 on provider outage. + */ + async validateAndIdentify(apiKey, moduleName) { + const moduleDefinition = this.moduleDefinitions.find( + (def) => def.moduleName === moduleName + ); + if (!moduleDefinition) { + // Should be caught at config-validation time; treat a runtime miss + // as a server misconfiguration rather than leaking specifics. + throw Boom.badImplementation( + `apiKey identity module '${moduleName}' is not registered` + ); + } + + const module = new this.ModuleClass({ definition: moduleDefinition }); + + // Seed the api client with the key without persisting anything. + const setAuthParams = + moduleDefinition.requiredAuthMethods?.setAuthParams; + if (typeof setAuthParams === 'function') { + await setAuthParams(module.api, { api_key: apiKey }); + } else if (typeof module.api?.setApiKey === 'function') { + module.api.setApiKey(apiKey); + } + + // 1) Validity. Call testAuthRequest directly (NOT module.testAuth, which + // swallows the error and would collapse 401 and 503 into one `false`). + let isValid; + try { + isValid = + await moduleDefinition.requiredAuthMethods.testAuthRequest( + module.api + ); + } catch (err) { + throw classifyProviderError(err) === 'unavailable' + ? providerUnavailable() + : invalidCredentials(); + } + // Require a STRICT boolean pass. A module that returns a truthy value on + // a bad key (e.g. an error object, a non-empty string, a response body) + // must NOT clear the validity gate — only an explicit `true` does. The + // login-path contract for testAuthRequest is: throw, or return `true`. + if (isValid !== true) { + throw invalidCredentials(); + } + + // 2) Identity. Derive the tenant id from the provider response only. + let entityDetails; + try { + entityDetails = + await moduleDefinition.requiredAuthMethods.getEntityDetails( + module.api, + { api_key: apiKey }, + undefined, + undefined + ); + } catch (err) { + throw classifyProviderError(err) === 'unavailable' + ? providerUnavailable() + : invalidCredentials(); + } + + const externalId = entityDetails?.identifiers?.externalId; + // Require a scalar, string-or-number identifier. Anything else (an + // object, array, boolean, null/undefined) is NOT a stable identifier and + // must be rejected rather than String()-coerced into a bogus one like + // "[object Object]" (ADR-034 §Security requirement 1). + if ( + (typeof externalId !== 'string' && + typeof externalId !== 'number') || + String(externalId).trim() === '' + ) { + throw invalidCredentials(); + } + + return { externalId: String(externalId), moduleDefinition }; + } + + /** + * Execute the api-key login. + * + * @param {Object} input + * @param {string} input.apiKey - The submitted product API key. + * @param {string} [input.module] - Optional module name (multi-identity apps only; allowlisted). + * @returns {Promise<{ token: string, userId: string, module: string }>} The minted session token and principal. + * @throws {Boom} 401 generic on invalid key / missing identifier / unconfigured mode; 503 on provider outage. + */ + async execute({ apiKey, module: requestedModule } = {}) { + // Cap length and shape BEFORE any provider work (ADR-034 §3). + if (typeof apiKey !== 'string' || apiKey.length === 0) { + throw invalidCredentials(); + } + if (apiKey.length > this.maxApiKeyLength) { + throw invalidCredentials(); + } + + const moduleName = this.resolveModuleName(requestedModule); + + const { externalId } = await this.validateAndIdentify( + apiKey, + moduleName + ); + + // Namespace the provider identity by the RESOLVED module. In a + // multi-module allowlist two different providers can legitimately return + // the SAME externalId (e.g. both use the numeric account id "42"); a bare + // externalId would then collapse those two distinct tenants onto one + // Frigg user. Prefixing with the module keeps them separate for BOTH the + // org-user and individual-user identity. + const identity = `${moduleName}:${externalId}`; + + // Find-or-create the Frigg user from the PROVIDER-DERIVED identity only. + // A client-supplied appOrgId/appUserId is never read here — the caller + // passes nothing but the key and (optionally) the allowlisted module. + const useOrg = this.userConfig.organizationUserRequired === true; + const appOrgId = useOrg ? identity : undefined; + const appUserId = useOrg ? undefined : identity; + + // NOTE (accepted cleanup debt): the user is found-or-created before the + // credential is provisioned below. If ProcessAuthorizationCallback fails, + // a user with no credential/entity is left behind. Reordering to create + // the credential first is intentionally out of scope here; orphaned users + // on partial failure are tolerated and cleaned up out of band. + const user = await this.getUserFromXFriggHeaders.execute( + appUserId, + appOrgId + ); + const userId = user.getId(); + if (!userId) { + throw invalidCredentials(); + } + + // Create/refresh the Credential + Entity through the same path + // /api/authorize uses. The key is persisted only as the encrypted + // Credential — never returned, never logged, never a JWT claim. + const callbackResult = await this.processAuthorizationCallback.execute( + userId, + moduleName, + { api_key: apiKey } + ); + + // Defense-in-depth: never mint a session unless the credential was + // actually persisted. A callback that returns without a credential id + // means the key was not connected; minting anyway would hand out a + // session over a half-provisioned tenant. Fail 500-class, not 401. + if (!callbackResult || !callbackResult.credential_id) { + throw Boom.badImplementation( + 'Login failed to create a credential for the api-key identity' + ); + } + + // Mint an ordinary, short-lived app-user session token (never admin). + const token = await this.createTokenForUserId.execute( + userId, + this.tokenExpiryMinutes + ); + + return { token, userId, module: moduleName }; + } +} + +module.exports = { + LoginWithApiKey, + classifyProviderError, + DEFAULT_MAX_API_KEY_LENGTH, +}; diff --git a/packages/core/user/use-cases/validate-api-key-auth-mode.js b/packages/core/user/use-cases/validate-api-key-auth-mode.js new file mode 100644 index 000000000..6a9329191 --- /dev/null +++ b/packages/core/user/use-cases/validate-api-key-auth-mode.js @@ -0,0 +1,103 @@ +/** + * Validate the `user.authModes.apiKey` block of an app definition against the + * app's registered module definitions (ADR-034 §Config). + * + * Default-off: when `authModes.apiKey` is absent this is a no-op, so apps that + * do not opt in are completely unaffected. When present, every named identity + * module MUST exist in the app's modules, or the app is misconfigured and we + * fail fast at wiring time rather than at first login. + * + * Accepts either: + * - `authModes.apiKey.module` — a single identity module (the common case), or + * - `authModes.apiKey.modules` — an allowlist of identity modules (multi-identity apps). + * + * @param {Object} userConfig - The app definition's `user` config (may be null). + * @param {Array} moduleDefinitions - Registered module definitions (each with `moduleName`). + * @throws {Error} If apiKey mode is declared but names no module, or names a module the app does not register. + * @returns {void} + */ +function validateApiKeyAuthMode(userConfig, moduleDefinitions = []) { + const config = userConfig?.authModes?.apiKey; + if (!config) { + return; // apiKey mode not enabled — nothing to validate. + } + + const named = []; + if (Array.isArray(config.modules)) { + named.push(...config.modules); + } + if (config.module) { + named.push(config.module); + } + + if (named.length === 0) { + throw new Error( + 'Invalid app definition: user.authModes.apiKey is enabled but names no identity module. ' + + "Set authModes.apiKey.module = '' (or authModes.apiKey.modules = ['', ...])." + ); + } + + const known = new Set( + (moduleDefinitions || []) + .map((def) => def && def.moduleName) + .filter(Boolean) + ); + + for (const moduleName of named) { + if (typeof moduleName !== 'string' || moduleName.trim() === '') { + throw new Error( + 'Invalid app definition: user.authModes.apiKey names a non-string module.' + ); + } + if (!known.has(moduleName)) { + throw new Error( + `Invalid app definition: user.authModes.apiKey.module '${moduleName}' is not a registered module. ` + + `Registered modules: ${[...known].join(', ') || '(none)'}.` + ); + } + } + + // allowedOrigins, when present, MUST be an array. A bare string would be + // iterated character-by-character by the Origin/Referer allowlist check + // (`allowed.includes(candidate)` on a string tests substrings), silently + // widening or breaking CSRF enforcement. + if ( + config.allowedOrigins !== undefined && + !Array.isArray(config.allowedOrigins) + ) { + throw new Error( + 'Invalid app definition: user.authModes.apiKey.allowedOrigins must be an array of origin strings.' + ); + } + + // rateLimit, when present, must be an object whose numeric knobs are positive + // finite numbers. A `0`, negative, or NaN would disable or corrupt the + // limiter (e.g. maxPerKey:0 rejects every request; windowMs:NaN never rolls), + // so fail fast at wiring time rather than shipping a broken oracle guard. + if (config.rateLimit !== undefined) { + if ( + typeof config.rateLimit !== 'object' || + config.rateLimit === null || + Array.isArray(config.rateLimit) + ) { + throw new Error( + 'Invalid app definition: user.authModes.apiKey.rateLimit must be an object.' + ); + } + for (const field of ['maxPerKey', 'maxGlobal', 'windowMs']) { + const value = config.rateLimit[field]; + if ( + value !== undefined && + (typeof value !== 'number' || + !Number.isFinite(value) || + value <= 0) + ) { + throw new Error( + `Invalid app definition: user.authModes.apiKey.rateLimit.${field} must be a positive finite number.` + ); + } + } + } +} + +module.exports = { validateApiKeyAuthMode };