diff --git a/DESIGN.md b/DESIGN.md index 8b8a19dcbd..08294b211a 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -190,6 +190,33 @@ System tables replicate by default. To opt out, add the name to `NON_REPLICATING If the table needs `audit: true`, set it both in the schema (for fresh installs) **and** on the `CreateTableObject` instance in the directive (for upgrades) — otherwise the two paths diverge. +## OIDC trusted publishing (`security/authn/oidc/`) + +`exchange_oidc_token` lets a workload authenticate with no stored Harper credential (#2171): it presents an identity token minted by its runtime, and gets back a one-hour operation token for the user a stored trust policy names. It is in `NO_AUTH_OPERATIONS` because it _is_ the authentication, the same way `create_authentication_tokens` is against a password — the same three wiring points apply (`serverHandlers.js` `NO_AUTH_OPERATIONS`, the `verifyPerms` bypass in `serverUtilities.ts`, and a `permission(false, [])` registration). + +**The core is issuer-agnostic; everything issuer-specific lives in `providers/`.** That split is the point of the layout, not an accident of it — a new workload-identity issuer should be a profile, not a change to verification, matching, or storage. + +- `claims.ts` — matching and constraint _shape_ validation. Knows nothing about any issuer. +- `jwks.ts` — issuer keys. The rate-limit clock for unknown-`kid` refetches lives _outside_ the cache entry: a successful fetch replaces the entry, and a rate limit that resets whenever it fires is not a rate limit. Keeping it separate also means a genuine key rotation is picked up on first use rather than after the window. +- `identityToken.ts` — signature, issuer, audience, `exp`, and a bounded lifetime. Owns `rejectToken`, shared with the exchange so both halves refuse identically. +- `tokenExchange.ts` — policy selection, replay, minting, audit. Verification is memoized per audience, so N policies sharing one cost one signature check. +- `providers/` — `assertPolicyIsSpecific` / `assertAudienceIsSpecific` / `normalizeClaims` / `describePrincipal` / optional `vetoClaims`, resolved by normalized issuer. + +**An unregistered issuer gets `providers/generic.ts`, which is strict rather than permissive:** the policy must pin `sub`. That is what makes Kubernetes service accounts, GCP service accounts, and SPIFFE SVIDs work with zero provider code — each has a stable canonical subject. GitHub needs its own profile precisely because its `sub` is the one claim you should _not_ pin: it varies by trigger, and its format changed for repositories created after 2026-07-15. + +Four constraints that look like choices but are not: + +1. **Every rejection returns the same message.** The endpoint is unauthenticated; a caller told which check failed can enumerate a policy one claim at a time. Reasons go to the `oidc-trust` logger. +2. **A GitHub policy must gate the ref.** `githubActionsProfile.assertPolicyIsSpecific` rejects a policy pinning only repository + workflow, because anyone who can push a branch could then add that workflow to it and mint a token. Stricter than npm's trusted-publishing model, which mitigates the same hole with environment protection instead — and profile-scoped, so it never constrains another issuer. +3. **`createOperationToken`, not `createTokens`.** `createTokens` overwrites `hdb_user.refresh_token` as a side effect, so minting for CI would silently revoke whatever credential that user already held (#2018) — the exact problem this feature removes. +4. **The role is the boundary; the per-policy `operations` allowlist only narrows it.** Least privilege is primarily the role of the user the policy names. A policy may _optionally_ carry an `operations` scope, which can only subtract from that role — never add to it. It is deliberately not merged into `permission.operations`: gate 2 in `operation_authorization.ts` treats an explicit listing of an SU-only operation as a deliberate grant, so reusing that field would _widen_ where this must only narrow. The scope is carried as a separate `tokenOperations` claim and intersected ahead of every early return, including the super_user bypass. + + Its enforcement surface is the operations API and SQL (`verifyPerms` / `verifyPermsAST`) — **not** the application REST/GraphQL resource path, which authorizes through table-level `checkPermission` and does not consult the scope. A scoped token therefore still carries its role's full CRUD there, which is why the role has to be least-privilege on its own; the scope is defense in depth, not a substitute. Closing that gap is a follow-up on the same surface as CORE-3061. Because a second authorization mechanism beside roles is one more place for the two to disagree, whether to keep this at all is an open design question on #2173 rather than a settled constraint. + + Naming `sql` in a scope grants the SQL interface, not unrestricted DML through it: a write statement additionally requires its matching data operation (`insert`/`update`/`delete`) in scope. That is what keeps `read_only` — which expands to include `sql` — from admitting a DELETE, given that `verifyPermsAST` returns early for a super_user before any table check runs. + +`hdb_oidc_token_use` (created lazily via `table()`, not the system schema) records spent tokens keyed on a SHA-256 of the token itself, with `expiresAt` past the token's own expiry. Hashed rather than stored, so the table never holds a credential; keyed on the token's **signed input** (`header.payload`) rather than `jti` because not every issuer emits one (Azure uses `uti`). Not on the whole token string: the signature segment is covered by nothing, and base64url decoding ignores the surplus low bits of its final character, so 16 distinct spellings of an RS256 signature decode to the same bytes, all verify, and all hash differently — one leaked token would buy 16 exchanges. ES\* malleability (`s → n−s`) is a second such vector. The signed input is exactly what the issuer asserted, so every variant collapses to one fingerprint. The get-then-put is not atomic and does not claim to be: a concurrent replay is not a privilege escalation, since whoever holds the token could obtain one operation token anyway. + ## Table drops, the `dropping` tombstone, and ghost tables A table is a set of RocksDB column families (`T/` plus `T/`) and a set of catalog rows diff --git a/bin/cliOperations.ts b/bin/cliOperations.ts index 74b33ceebb..438d00694d 100644 --- a/bin/cliOperations.ts +++ b/bin/cliOperations.ts @@ -6,6 +6,7 @@ import * as envMgr from '../utility/environment/environmentManager.ts'; envMgr.initSync(); import * as terms from '../utility/hdbTerms.ts'; import { httpRequest } from '../utility/common_utils.ts'; +import { workloadIdentityAvailable, exchangeWorkloadIdentityForToken } from './workloadIdentity.ts'; import * as fs from 'fs-extra'; import * as YAML from 'yaml'; import { Readable } from 'node:stream'; @@ -55,11 +56,11 @@ const TRANSPORT_ONLY_FIELDS = new Set([ 'by_ref', 'ref', 'credential', - // `deploy setup=true`'s token, read off the parsed request by deploySetup and sealed locally. No - // operation takes a *top-level* `token`, so keeping it out of every body costs nothing and means a - // mistyped `setup` — which parses as a bare word and falls through to a real deploy — can't carry a - // PAT to the server. Distinct from `credentials[].token`, which is nested inside a field that IS - // sent (ingestCredentials seals it server-side) and is only kept out of the operations log. + // `deploy setup=true`'s token, read off the parsed request by deploySetup and sealed locally. + // Stripping it means a mistyped `setup` — which parses as a bare word and falls through to a real + // deploy — can't carry a PAT to the server. Distinct from `credentials[].token`, which is nested + // inside a field that IS sent (ingestCredentials seals it server-side) and is only kept out of the + // operations log. See OPERATIONS_TAKING_A_TOKEN for the one operation this must not apply to. 'token', ]); @@ -179,10 +180,24 @@ async function* wrapPackagingStream(stream: Readable, projectPath: string): Asyn // Build the JSON operation-field set from `req`, dropping the CLI's internal (`_`-prefixed) // and transport-only fields so neither the CLI internals nor credentials leak into the // request body. Shared by the multipart and legacy-JSON deploy body builders. +/** + * Operations whose own request body has a top-level `token`, which must therefore survive the + * transport-only stripping above. + * + * `exchange_oidc_token` (#2171) is one: the identity token IS the request. The blanket strip was + * written when no operation took a top-level `token`, and left this one reaching the server without + * the field it requires — so the issuer-agnostic path was unusable through the generic CLI even + * though direct HTTP worked. Keyed on the operation rather than dropping the strip, because the + * mistyped-`setup` case it guards against is real. + */ +const OPERATIONS_TAKING_A_TOKEN = new Set(['exchange_oidc_token']); + function operationFields(req: any): any { + const keepsToken = OPERATIONS_TAKING_A_TOKEN.has(req?.operation); const fields: any = {}; for (const [key, value] of Object.entries(req)) { - if (key.startsWith('_') || TRANSPORT_ONLY_FIELDS.has(key)) continue; + if (key.startsWith('_')) continue; + if (TRANSPORT_ONLY_FIELDS.has(key) && !(key === 'token' && keepsToken)) continue; fields[key] = value; } return fields; @@ -764,7 +779,8 @@ export async function resolveRequestOptions(req: any): Promise<{ options: any; t options.timeout = SSE_OPERATIONS.has(req.operation) ? SSE_OPERATION_TIMEOUT_MS : CLI_OPERATION_TIMEOUT_MS; // Authentication precedence: explicitly configured credentials (dedicated args, URL // userinfo, env vars) beat everything, then env-var tokens, then the saved `harper login` - // token, and only then the legacy `username=`/`password=` payload fallback below. The + // token, then a CI identity token exchanged via OIDC (#2171 — ambient, so it ranks below + // everything configured), and only then the legacy `username=`/`password=` fallback. The // tokens must outrank that fallback: for add_user/alter_user those args are the credentials // of the user being created/altered, so treating them as auth would authenticate as a user // who doesn't exist yet (or as the wrong identity) instead of using the admin's session. @@ -799,9 +815,12 @@ export async function resolveRequestOptions(req: any): Promise<{ options: any; t const envRefreshToken = tokenPrefix ? process.env[`${tokenPrefix}_REFRESH_TOKEN`]?.trim() : undefined; // A namespace that is set but blank is a broken CI secret, not a request to fall back to // whatever the developer last logged in as — say so rather than switching identity silently. - if (tokenPrefix && !envOperationToken && !envRefreshToken) { + const tokenNamespaceBlank = !!tokenPrefix && !envOperationToken && !envRefreshToken; + if (tokenNamespaceBlank) { console.error( - `Ignoring empty ${tokenPrefix}_OPERATION_TOKEN/${tokenPrefix}_REFRESH_TOKEN; falling back to saved login credentials.` + `Ignoring empty ${tokenPrefix}_OPERATION_TOKEN/${tokenPrefix}_REFRESH_TOKEN; falling back to saved ` + + `login credentials. Workload identity is deliberately NOT used here: a blank token namespace is a ` + + `failed secret, and deploying as a different identity would hide that.` ); } @@ -819,6 +838,24 @@ export async function resolveRequestOptions(req: any): Promise<{ options: any; t if (tokens.operation_token) { options.headers.Authorization = `Bearer ${tokens.operation_token}`; } + } else if (workloadIdentityAvailable() && !tokenNamespaceBlank) { + // Last credential source: no configured token, but this runner can prove its identity to + // the cluster directly (#2171). Deliberately below the env-var and saved tokens — an + // explicitly configured credential should keep working exactly as it did when someone adds + // `id-token: write` to a workflow, rather than silently switching which identity deploys. + // + // `!tokenNamespaceBlank` extends that invariant to the half-configured case. A CI secret + // that failed to populate leaves the namespace set but empty; without this, such a run + // would quietly deploy as the OIDC policy's user instead of failing, which is the same + // silent identity switch in a shape that is harder to notice. + // Standard operation timeout, not the caller's — by this point `options.timeout` may carry + // the 10-minute SSE timeout for a streaming deploy_component, and the exchange is a small + // fast request. Without the override a stalled exchange hangs the deploy for ten minutes, + // on the very operation this feature exists to serve. Same reasoning, same fix as + // refreshExpiredOperationToken above. + const exchangeOptions = { ...options, timeout: CLI_OPERATION_TIMEOUT_MS }; + const operationToken = await exchangeWorkloadIdentityForToken(exchangeOptions, target.resolvedTarget); + if (operationToken) options.headers.Authorization = `Bearer ${operationToken}`; } } // Legacy fallback for operations where `username=`/`password=` genuinely ARE the caller's diff --git a/bin/workloadIdentity.ts b/bin/workloadIdentity.ts new file mode 100644 index 0000000000..53675889b0 --- /dev/null +++ b/bin/workloadIdentity.ts @@ -0,0 +1,119 @@ +/** + * Client half of OIDC trusted publishing (#2171): ask the runtime for a workload identity token + * addressed to this instance, trade it for a short-lived operation token via `exchange_oidc_token`. + * + * Structured as a provider list because the runtimes differ only in how the token is obtained. + * GitHub Actions is entry one; a Kubernetes entry is `available()` testing for a projected + * service-account token path and `requestToken()` reading that file. A runtime none of them + * recognizes falls through to the CLI's other credential sources. + */ + +import { httpRequest } from '../utility/common_utils.ts'; + +interface WorkloadIdentityProvider { + name: string; + /** True when this process can obtain a token from this runtime. */ + available(): boolean; + /** + * Obtains an identity token bound to `audience`. Binding it is what makes the token unusable + * anywhere else — see SHARED_DEFAULT_AUDIENCE in security/authn/oidc/providers/githubActions.ts + * for what an unbound one costs. + */ + requestToken(audience: string): Promise; +} + +/** GitHub sets both of these on a job that declares `permissions: id-token: write`. */ +const GITHUB_TOKEN_REQUEST_URL = 'ACTIONS_ID_TOKEN_REQUEST_URL'; +const GITHUB_TOKEN_REQUEST_TOKEN = 'ACTIONS_ID_TOKEN_REQUEST_TOKEN'; + +const IDENTITY_REQUEST_TIMEOUT_MS = 10_000; + +const githubActions: WorkloadIdentityProvider = { + name: 'GitHub Actions', + + /** + * Both variables are required: GitHub sets them together, so their absence means the workflow did + * not grant `id-token: write` — a configuration answer, not a failure to report here. + */ + available(): boolean { + return Boolean(process.env[GITHUB_TOKEN_REQUEST_URL] && process.env[GITHUB_TOKEN_REQUEST_TOKEN]); + }, + + async requestToken(audience: string): Promise { + const requestUrl = new URL(process.env[GITHUB_TOKEN_REQUEST_URL] as string); + requestUrl.searchParams.set('audience', audience); + + const response = await fetch(requestUrl, { + headers: { + authorization: `Bearer ${process.env[GITHUB_TOKEN_REQUEST_TOKEN]}`, + accept: 'application/json', + }, + signal: AbortSignal.timeout(IDENTITY_REQUEST_TIMEOUT_MS), + }); + if (!response.ok) throw new Error(`GitHub returned ${response.status} for an identity token`); + + const body: any = await response.json(); + if (typeof body?.value !== 'string' || body.value === '') { + throw new Error('GitHub returned no identity token value'); + } + return body.value; + }, +}; + +const PROVIDERS: WorkloadIdentityProvider[] = [githubActions]; + +function activeProvider(): WorkloadIdentityProvider | undefined { + return PROVIDERS.find((provider) => provider.available()); +} + +/** True when this runtime can prove its own identity to the cluster. */ +export function workloadIdentityAvailable(): boolean { + return activeProvider() !== undefined; +} + +/** + * Trades a workload identity token for a Harper operation token, or undefined when this runtime has + * no identity to offer. Failures are reported and swallowed: this is the last credential source + * before the request goes out unauthenticated, and the resulting 401 says nothing useful, so the + * reason is worth printing even though it is not by itself fatal. + */ +export async function exchangeWorkloadIdentityForToken(options: any, audience: string): Promise { + const provider = activeProvider(); + if (!provider) return undefined; + + console.error(`Requesting a ${provider.name} identity token for ${audience}...`); + let identityToken: string; + try { + identityToken = await provider.requestToken(audience); + } catch (error) { + console.error(`Could not obtain a ${provider.name} identity token: ${(error as Error).message}`); + return undefined; + } + + try { + const response = await httpRequest(options, { operation: 'exchange_oidc_token', token: identityToken }); + if (response.statusCode === 200) { + const data = JSON.parse(response.body); + if (data.operation_token) { + console.error(`Authenticated as '${data.username}' via OIDC trust policy '${data.policy}'.`); + return data.operation_token; + } + console.error('The OIDC exchange returned no operation token.'); + return undefined; + } + if (response.statusCode === 401) { + // The server deliberately does not say which check failed, so point at the two things the + // operator can actually inspect rather than inventing a cause. + console.error( + 'Harper rejected the identity token. Check that a trust policy matches this workload ' + + '(list_oidc_trust) and that its audience is this instance; the server log records the reason.' + ); + return undefined; + } + console.error(`OIDC exchange failed: ${response.statusCode}`); + return undefined; + } catch (error) { + console.error(`Error exchanging the identity token: ${(error as Error).message}`); + return undefined; + } +} diff --git a/components/mcp/tools/operations.ts b/components/mcp/tools/operations.ts index 5572b79508..e6d8ea04ed 100644 --- a/components/mcp/tools/operations.ts +++ b/components/mcp/tools/operations.ts @@ -144,6 +144,11 @@ export const DEFAULT_ALLOW: readonly string[] = [ * `mcp.operations.allow` (an explicit allow list replaces the default and is * not filtered by this set), where the audit-log redaction of * `value`/`values`/`envelope` applies. + * + * OIDC trust policies (#2171) are excluded on the same grounds and then some: a + * policy grants an external CI workflow the right to authenticate as a Harper + * user, so reading the set tells an attacker exactly which repository and + * workflow to compromise, and writing one is a way to grant itself access. */ export const DEFAULT_EXCLUDED: ReadonlySet = new Set([ 'set_secret', @@ -152,6 +157,10 @@ export const DEFAULT_EXCLUDED: ReadonlySet = new Set([ 'list_secrets', 'delete_secret', 'get_secrets_public_key', + 'add_oidc_trust', + 'list_oidc_trust', + 'drop_oidc_trust', + 'exchange_oidc_token', ]); /** diff --git a/json/systemSchema.json b/json/systemSchema.json index 6f44989b5b..bd29bb501c 100644 --- a/json/systemSchema.json +++ b/json/systemSchema.json @@ -472,5 +472,57 @@ "attribute": "__updatedtime__" } ] + }, + "hdb_oidc_trust": { + "hash_attribute": "id", + "name": "hdb_oidc_trust", + "schema": "system", + "audit": true, + "attributes": [ + { + "attribute": "id" + }, + { + "attribute": "issuer" + }, + { + "attribute": "audience" + }, + { + "attribute": "claims" + }, + { + "attribute": "user" + }, + { + "attribute": "operations" + }, + { + "attribute": "enabled" + }, + { + "attribute": "description" + }, + { + "attribute": "updated_by" + }, + { + "attribute": "__createdtime__" + }, + { + "attribute": "__updatedtime__" + } + ] + }, + "hdb_oidc_token_use": { + "hash_attribute": "id", + "name": "hdb_oidc_token_use", + "schema": "system", + "audit": true, + "attributes": [ + { + "attribute": "id" + } + ] } } diff --git a/security/authn/oidc/claims.ts b/security/authn/oidc/claims.ts new file mode 100644 index 0000000000..dc7e8a53b8 --- /dev/null +++ b/security/authn/oidc/claims.ts @@ -0,0 +1,72 @@ +/** + * Issuer-agnostic claim matching and constraint validation (#2171). + * + * Pure — no network, no storage, and nothing that knows which issuer a token came from. Rules that + * depend on the issuer live in providers/. + */ + +import { ClientError } from '../../../utility/errors/hdbError.ts'; +import type { ClaimConstraint, TokenClaims } from './types.ts'; + +/** Issuers may encode a numeric id as a JSON number; anything non-scalar is not comparable. */ +function claimToString(value: unknown): string | undefined { + if (typeof value === 'string') return value; + if (typeof value === 'number' && Number.isFinite(value)) return String(value); + return undefined; +} + +/** + * Returns undefined on a match, or the first failure's reason — for the log, never the caller. + * + * A constrained claim absent from the token fails rather than passes, so a policy cannot be weakened + * by an issuer that stops emitting a claim. + */ +export function matchTrustPolicyClaims( + claims: TokenClaims, + policyClaims: Record +): string | undefined { + const constraints = Object.entries(policyClaims); + // validateClaimConstraintShape rejects this at write time; this backstops a row that reached the + // table another way, such as replication from a peer. + if (constraints.length === 0) return 'policy constrains no claims'; + + for (const [claimName, constraint] of constraints) { + const actual = claimToString(claims[claimName]); + if (actual === undefined || actual === '') return `token has no usable ${claimName} claim`; + const accepted = Array.isArray(constraint) ? constraint : [constraint]; + // Exact membership, deliberately — never a prefix, wildcard, or regex. Relaxing this to + // something like `startsWith` is the classic trusted-publishing escalation: a policy pinning + // `HarperFast/my-app` would then also admit `HarperFast/my-app-evil`, a repository anyone can + // create. unitTests/security/authn/oidc/claims.test.js pins both argument orders. + if (!accepted.includes(actual)) return `${claimName} does not match the policy`; + } + return undefined; +} + +/** + * Validates the *shape* of a policy's constraints — that each is a usable set of comparable values. + * Whether the set is specific enough to be safe depends on the issuer; that is the provider profile's + * assertPolicyIsSpecific. + * + * Throws ClientError naming the first problem; the reader is an administrator writing a policy. + */ +export function validateClaimConstraintShape( + policyClaims: unknown +): asserts policyClaims is Record { + if (!policyClaims || typeof policyClaims !== 'object' || Array.isArray(policyClaims)) { + throw new ClientError('claims must be an object of claim constraints'); + } + + const entries = Object.entries(policyClaims as Record); + if (entries.length === 0) throw new ClientError('claims must constrain at least one claim'); + + for (const [claimName, constraint] of entries) { + const values = Array.isArray(constraint) ? constraint : [constraint]; + if (values.length === 0) throw new ClientError(`claims.${claimName} must accept at least one value`); + for (const value of values) { + if (typeof value !== 'string' || value === '') { + throw new ClientError(`claims.${claimName} must be a non-empty string or an array of non-empty strings`); + } + } + } +} diff --git a/security/authn/oidc/identityToken.ts b/security/authn/oidc/identityToken.ts new file mode 100644 index 0000000000..52e8030bf3 --- /dev/null +++ b/security/authn/oidc/identityToken.ts @@ -0,0 +1,129 @@ +/** + * Identity-token verification for OIDC trusted publishing (#2171). + * + * Verifies that a token was signed by the configured issuer and is addressed to this instance. + * Matching it against a policy's claim constraints is separate (claims.ts), because one verification + * serves every policy sharing an issuer and audience. + */ + +import jwt, { type Algorithm, type JwtPayload } from 'jsonwebtoken'; +import type { KeyObject } from 'node:crypto'; +import { ClientError } from '../../../utility/errors/hdbError.ts'; +import { loggerWithTag } from '../../../utility/logging/logger.ts'; +import { getSigningKey as defaultGetSigningKey, normalizeIssuer } from './jwks.ts'; +import { profileForIssuer } from './providers/index.ts'; +import type { TokenClaims } from './types.ts'; + +const logger = loggerWithTag('oidc-trust'); + +/** + * Asymmetric signatures only. Passing this to jwt.verify is what prevents algorithm confusion: an + * `alg: none` token, or one HMAC-signed with a public key as the secret, is rejected before the + * signature is considered. + */ +const ALLOWED_ALGORITHMS: Algorithm[] = [ + 'RS256', + 'RS384', + 'RS512', + 'ES256', + 'ES384', + 'ES512', + 'PS256', + 'PS384', + 'PS512', +]; + +const CLOCK_TOLERANCE_SECONDS = 60; + +/** CI identity tokens are minted per job and live minutes; a far-future expiry is not one to honor. */ +const MAX_TOKEN_LIFETIME_SECONDS = 3_600; + +export interface VerifyTokenTarget { + issuer: string; + audience: string; +} + +export interface VerifyTokenOptions { + /** Overridable for tests; defaults to the network-backed JWKS lookup. */ + getSigningKey?: (issuer: string, kid: unknown) => Promise; + /** Seconds since the epoch to evaluate expiry against; defaults to the real clock. */ + clockTimestamp?: number; +} + +/** + * Refuses a token. Shared with the exchange so both halves fail identically: the endpoint is + * unauthenticated, and a caller told exactly which check failed can probe a policy one claim at a + * time. The reason goes to the log instead. + */ +export function rejectToken(detail: string): never { + logger.warn?.(`Rejecting identity token: ${detail}`); + throw new ClientError('Identity token was rejected', 401); +} + +/** Verifies a CI identity token against one issuer/audience pair and returns its normalized claims. */ +export async function verifyIdentityToken( + token: unknown, + target: VerifyTokenTarget, + options: VerifyTokenOptions = {} +): Promise { + if (typeof token !== 'string' || token === '') throw new ClientError('token is required'); + const issuer = normalizeIssuer(target.issuer); + if (typeof target.audience !== 'string' || target.audience === '') { + throw new ClientError('audience is required'); + } + + const decoded = jwt.decode(token, { complete: true }); + if (!decoded) rejectToken('token is not a well-formed JWT'); + + // Checked before resolving a key so a garbage header costs no outbound request. + if (!ALLOWED_ALGORITHMS.includes(decoded.header.alg as Algorithm)) { + rejectToken(`unsupported algorithm ${decoded.header.alg}`); + } + + const getSigningKey = options.getSigningKey ?? defaultGetSigningKey; + const key = await getSigningKey(issuer, decoded.header.kid); + + let payload: JwtPayload; + try { + payload = jwt.verify(token, key, { + algorithms: ALLOWED_ALGORITHMS, + // Both spellings, because `issuer` here is normalized (trailing slash stripped) while + // jsonwebtoken compares it byte-for-byte against the raw `iss`. Azure AD v1 emits + // `https://sts.windows.net//`, which would otherwise never authenticate — and the + // generic profile exists precisely to serve issuers like it with no provider code. This + // cannot widen trust: normalizeIssuer already maps the two forms to one value, which is + // what keys the JWKS cache, the discovery check, and the policy lookup. + issuer: [issuer, `${issuer}/`], + audience: target.audience, + clockTolerance: CLOCK_TOLERANCE_SECONDS, + ...(options.clockTimestamp === undefined ? {} : { clockTimestamp: options.clockTimestamp }), + }) as JwtPayload; + } catch (error) { + rejectToken((error as Error).message); + } + + // jsonwebtoken only enforces `exp` when present, so a token without one never expires. + if (typeof payload.exp !== 'number') rejectToken('token has no exp claim'); + // `iat` is REQUIRED by OIDC, and required here rather than treated as optional: skipping the + // ceiling when it is absent means a token carrying a distant `exp` and no `iat` is unbounded — + // the ceiling would silently not apply to exactly the token that most needs it. + if (typeof payload.iat !== 'number') rejectToken('token has no iat claim'); + + // Bounded against the verification clock, not only as `exp - iat`. jsonwebtoken validates `exp` + // and `nbf` but never rejects a future `iat`, so an issuer that shifts the whole pair forward + // keeps a small delta and still hands out a token valid far longer than the ceiling. Both the + // delta and the absolute distance to `exp` are checked; the tolerance mirrors the one given to + // jwt.verify so genuine clock skew is not treated as an attack. + const now = options.clockTimestamp ?? Math.floor(Date.now() / 1000); + if (payload.iat > now + CLOCK_TOLERANCE_SECONDS) rejectToken('token iat is in the future'); + if (payload.exp - payload.iat > MAX_TOKEN_LIFETIME_SECONDS) { + rejectToken(`token lifetime exceeds ${MAX_TOKEN_LIFETIME_SECONDS}s`); + } + if (payload.exp - now > MAX_TOKEN_LIFETIME_SECONDS + CLOCK_TOLERANCE_SECONDS) { + rejectToken(`token expires more than ${MAX_TOKEN_LIFETIME_SECONDS}s from now`); + } + // No `jti` requirement: not every issuer emits one (Azure uses `uti`, others omit it), and the + // exchange keys replay on a hash of the token itself, which is universal. + + return profileForIssuer(issuer).normalizeClaims(payload as TokenClaims); +} diff --git a/security/authn/oidc/jwks.ts b/security/authn/oidc/jwks.ts new file mode 100644 index 0000000000..3669c55a17 --- /dev/null +++ b/security/authn/oidc/jwks.ts @@ -0,0 +1,260 @@ +/** + * OIDC discovery and JWKS retrieval for trusted publishing (#2171). + * + * The issuer's signing keys are the root of trust for an exchanged token, and the endpoint that + * reaches them is unauthenticated — hence HTTPS only, bounded body, bounded time, and a rate limit + * on the refetch an unrecognized `kid` triggers, without which forged `kid`s become one outbound + * fetch per request. + */ + +import { createPublicKey, type KeyObject } from 'node:crypto'; +import { loggerWithTag } from '../../../utility/logging/logger.ts'; +import { ClientError, ServerError } from '../../../utility/errors/hdbError.ts'; + +const logger = loggerWithTag('oidc-trust'); + +const DISCOVERY_PATH = '/.well-known/openid-configuration'; +/** How long a fetched key set is served without revalidation. */ +const JWKS_CACHE_TTL_MS = 3_600_000; +/** Floor between refetches triggered by an unrecognized `kid`. */ +const MIN_REFETCH_INTERVAL_MS = 60_000; +/** + * How long a cached key set survives a failed refetch. Bounded, so a network blip does not break + * deploys but a pulled key does not stay honored forever. + */ +const STALE_KEY_GRACE_MS = 86_400_000; +const FETCH_TIMEOUT_MS = 5_000; +const MAX_RESPONSE_BYTES = 1_048_576; +/** Asymmetric only: an `oct` key in a JWKS is the setup for algorithm confusion. */ +const SUPPORTED_KEY_TYPES = ['RSA', 'EC']; + +interface IssuerKeys { + keys: Map; + fetchedAt: number; +} + +const issuerKeyCache = new Map(); +const inFlightLoads = new Map>(); +/** + * When an unrecognized `kid` last drove a refetch, per issuer. Outside the cache entry on purpose: + * every successful fetch replaces that entry, and a rate limit that resets whenever it fires is not + * a rate limit. Separating them also lets a genuine key rotation be picked up on first use. + */ +const unknownKidRefetchAt = new Map(); + +/** + * When a refresh for an issuer last FAILED, per issuer. Tracked separately from `fetchedAt`, which a + * failed fetch does not advance — so without this, every request wave arriving after the cache went + * stale starts discovery again and rides the same timeout before falling back to the same stale key. + * The exchange is unauthenticated and picks its issuer from an unverified JWT, and issuer key ids are + * public, so an anonymous caller could keep that cycle running for the length of an issuer outage: + * exactly when the stale-key grace is supposed to be absorbing load, not generating it. + */ +const failedRefreshAt = new Map(); + +/** How long a failed refresh suppresses the next one, while a usable stale key is still on hand. */ +const FAILED_REFRESH_BACKOFF_MS = 30_000; + +/** + * Bumped by every clear. A fetch that was already in flight when the cache was cleared must not + * write its now-stale result back: clearing drops `inFlightLoads`, but the orphaned fetch still + * holds a reference and would repopulate the entry an operator just discarded — which for a clear + * issued precisely because the cached key is wrong means the re-read silently does nothing. + */ +let cacheGeneration = 0; + +/** Drops all cached key sets. Exported for tests and for an operator forcing a re-read. */ +export function clearJwksCache(): void { + cacheGeneration++; + issuerKeyCache.clear(); + inFlightLoads.clear(); + unknownKidRefetchAt.clear(); + failedRefreshAt.clear(); +} + +/** + * Canonicalizes an issuer URL. The result is both the cache key and the discovery base, so a policy + * stored with a trailing slash and one without must not become two entries for the same issuer. + */ +export function normalizeIssuer(issuer: unknown): string { + if (typeof issuer !== 'string' || issuer === '') throw new ClientError('issuer is required'); + let url: URL; + try { + url = new URL(issuer); + } catch { + throw new ClientError(`issuer is not a valid URL: ${issuer}`); + } + if (url.protocol !== 'https:') throw new ClientError('issuer must be an https URL'); + if (url.search !== '' || url.hash !== '') throw new ClientError('issuer must not carry a query or fragment'); + return url.origin + (url.pathname === '/' ? '' : url.pathname.replace(/\/$/, '')); +} + +/** + * `content-length` is checked first as a cheap rejection, then the body is counted as it streams — + * the header is advisory, and a hostile endpoint can simply omit it. + */ +async function readBoundedJson(response: Response, url: string): Promise { + const declaredLength = Number(response.headers.get('content-length')); + if (Number.isFinite(declaredLength) && declaredLength > MAX_RESPONSE_BYTES) { + throw new ServerError(`Response from ${url} exceeds ${MAX_RESPONSE_BYTES} bytes`); + } + const chunks: Buffer[] = []; + let total = 0; + if (response.body) { + for await (const chunk of response.body as unknown as AsyncIterable) { + total += chunk.length; + if (total > MAX_RESPONSE_BYTES) { + throw new ServerError(`Response from ${url} exceeds ${MAX_RESPONSE_BYTES} bytes`); + } + chunks.push(Buffer.from(chunk)); + } + } + try { + return JSON.parse(Buffer.concat(chunks).toString('utf8')); + } catch (error) { + throw new ServerError(`Response from ${url} is not valid JSON: ${(error as Error).message}`); + } +} + +async function fetchJson(url: string): Promise { + let response: Response; + try { + response = await fetch(url, { + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), + headers: { accept: 'application/json' }, + redirect: 'error', + }); + } catch (error) { + throw new ServerError(`Could not reach ${url}: ${(error as Error).message}`); + } + if (!response.ok) throw new ServerError(`${url} responded ${response.status}`); + return readBoundedJson(response, url); +} + +/** + * The discovery document's own `issuer` must equal the one we asked about — the spec requires it, + * and it stops a misdirected document from quietly re-pointing an issuer we trust. + */ +async function discoverJwksUri(issuer: string): Promise { + const document = await fetchJson(issuer + DISCOVERY_PATH); + // Swallowed rather than propagated: normalizeIssuer raises ClientError, the wrong shape for a + // malformed *server* response. + let declaredIssuer: string | undefined; + try { + declaredIssuer = normalizeIssuer(document?.issuer); + } catch { + declaredIssuer = undefined; + } + if (declaredIssuer !== issuer) { + throw new ServerError(`Discovery document at ${issuer} declares a different issuer`); + } + const jwksUri = document?.jwks_uri; + if (typeof jwksUri !== 'string' || !jwksUri.startsWith('https://')) { + throw new ServerError(`Discovery document at ${issuer} has no https jwks_uri`); + } + return jwksUri; +} + +/** + * Undefined for a key we will not honor. One unusable entry must not poison the set: issuers publish + * keys for other purposes, and an unknown future type should degrade rather than fail the fetch. + */ +function toSigningKey(jwk: any): KeyObject | undefined { + if (!jwk || typeof jwk !== 'object') return undefined; + if (typeof jwk.kid !== 'string' || jwk.kid === '') return undefined; + if (jwk.use !== undefined && jwk.use !== 'sig') return undefined; + if (!SUPPORTED_KEY_TYPES.includes(jwk.kty)) return undefined; + try { + return createPublicKey({ key: jwk, format: 'jwk' }); + } catch (error) { + logger.warn?.(`Skipping unusable JWK ${jwk.kid}: ${(error as Error).message}`); + return undefined; + } +} + +async function fetchIssuerKeys(issuer: string): Promise { + // Sampled before the first await, so any clear during the fetch is detectable at the end. + const generation = cacheGeneration; + const jwksUri = await discoverJwksUri(issuer); + const jwks = await fetchJson(jwksUri); + if (!Array.isArray(jwks?.keys)) throw new ServerError(`JWKS at ${jwksUri} has no keys array`); + + const keys = new Map(); + for (const jwk of jwks.keys) { + const key = toSigningKey(jwk); + if (key) keys.set(jwk.kid, key); + } + if (keys.size === 0) throw new ServerError(`JWKS at ${jwksUri} contains no usable signing keys`); + + const entry: IssuerKeys = { keys, fetchedAt: Date.now() }; + // Still returned to the caller that asked for it — it is freshly fetched and valid — but only + // cached if no clear happened while this was in flight. + if (generation === cacheGeneration) issuerKeyCache.set(issuer, entry); + logger.debug?.(`Loaded ${keys.size} signing key(s) for ${issuer}`); + return entry; +} + +/** Loads an issuer's keys, collapsing concurrent callers onto one fetch. */ +function loadIssuerKeys(issuer: string): Promise { + const existing = inFlightLoads.get(issuer); + if (existing) return existing; + + const load = fetchIssuerKeys(issuer).finally(() => inFlightLoads.delete(issuer)); + inFlightLoads.set(issuer, load); + return load; +} + +/** Resolves the public key an issuer used to sign a token, by `kid`. */ +/** + * `now` exists so the two time-based bounds below can be tested — the cache TTL and, more + * importantly, the STALE_KEY_GRACE_MS ceiling that stops a pulled key being honored forever through + * a prolonged outage. Reaching either otherwise needs a cache aged past an hour, and this repo bars + * new fake timers. Mirrors the `clockTimestamp` seam verifyIdentityToken already takes, rather than + * inventing a second convention. Callers in production pass nothing. + */ +export async function getSigningKey(issuer: string, kid: unknown, now: number = Date.now()): Promise { + if (typeof kid !== 'string' || kid === '') throw new ClientError('Token has no key id', 401); + const normalizedIssuer = normalizeIssuer(issuer); + const cached = issuerKeyCache.get(normalizedIssuer); + + if (cached && now - cached.fetchedAt < JWKS_CACHE_TTL_MS) { + const key = cached.keys.get(kid); + if (key) return key; + // Unknown kid against a still-fresh set: refetch once, then hold the line for the window. The + // timestamp is recorded before the fetch so a failing issuer is rate-limited like a succeeding one. + if (now - (unknownKidRefetchAt.get(normalizedIssuer) ?? 0) < MIN_REFETCH_INTERVAL_MS) { + throw new ClientError('Token signing key is not recognized', 401); + } + unknownKidRefetchAt.set(normalizedIssuer, now); + } + + // A usable stale key, if the cache is expired but still inside the grace window. + const staleKey = cached && now - cached.fetchedAt < STALE_KEY_GRACE_MS ? cached.keys.get(kid) : undefined; + + // Skip the fetch entirely while a recent one is known to have failed. Not gated on having a stale + // key: the fetch is the expensive part, and the issuer we have NEVER cached is the worse case — + // there is nothing to fall back to, so without this every request rides the full discovery and + // timeout. Refusing for the backoff interval bounds what an anonymous caller can drive, at the + // cost of a legitimate first exchange waiting out that interval after a blip. Fails closed. + if (now - (failedRefreshAt.get(normalizedIssuer) ?? 0) < FAILED_REFRESH_BACKOFF_MS) { + if (staleKey) return staleKey; + throw new ServerError(`Signing keys for ${normalizedIssuer} are temporarily unavailable; a recent refresh failed`); + } + + let refreshed: IssuerKeys; + try { + refreshed = await loadIssuerKeys(normalizedIssuer); + } catch (error) { + // Recorded whether or not a stale key rescues this request, so the backoff also covers the + // issuer whose keys we have never held. + failedRefreshAt.set(normalizedIssuer, now); + if (!staleKey) throw error; + logger.warn?.(`Using cached signing key for ${normalizedIssuer}; refresh failed: ${(error as Error).message}`); + return staleKey; + } + failedRefreshAt.delete(normalizedIssuer); + + const key = refreshed.keys.get(kid); + if (!key) throw new ClientError('Token signing key is not recognized', 401); + return key; +} diff --git a/security/authn/oidc/providers/generic.ts b/security/authn/oidc/providers/generic.ts new file mode 100644 index 0000000000..3c5fa303bd --- /dev/null +++ b/security/authn/oidc/providers/generic.ts @@ -0,0 +1,40 @@ +/** + * Fallback profile for issuers with no registered provider profile (#2171). + * + * Strict rather than permissive: the policy must pin `sub`. That is what makes workload identity work + * with zero provider code — a Kubernetes service-account token + * (`system:serviceaccount::`), a GCP service account, and a SPIFFE SVID all carry a + * stable canonical subject, so pinning it identifies exactly one principal. + * + * GitHub Actions needs its own profile precisely because its `sub` is the one claim you should not + * pin: it varies by trigger, and its format changed for repositories created after 2026-07-15. + */ + +import { ClientError } from '../../../../utility/errors/hdbError.ts'; +import type { ClaimConstraint, TokenClaims } from '../types.ts'; +import type { IdentityProviderProfile } from './index.ts'; + +export const genericProfile: IdentityProviderProfile = { + name: 'generic OIDC', + + assertPolicyIsSpecific(policyClaims: Record): void { + if (policyClaims.sub === undefined) { + throw new ClientError( + 'claims must pin `sub` for an issuer with no registered provider profile — it is the only ' + + 'claim every OIDC issuer defines as identifying a single principal. Register a provider ' + + 'profile if this issuer needs richer rules.' + ); + } + }, + + // No known shared default audience; the required `sub` pin already binds the policy to one principal. + assertAudienceIsSpecific(): void {}, + + normalizeClaims(payload: TokenClaims): TokenClaims { + return { ...payload }; + }, + + describePrincipal(claims: TokenClaims): string { + return typeof claims.sub === 'string' ? claims.sub : 'unknown principal'; + }, +}; diff --git a/security/authn/oidc/providers/githubActions.ts b/security/authn/oidc/providers/githubActions.ts new file mode 100644 index 0000000000..1f90115f83 --- /dev/null +++ b/security/authn/oidc/providers/githubActions.ts @@ -0,0 +1,137 @@ +/** + * GitHub Actions provider profile (#2171). + * + * The only module that should know anything about GitHub. Its rules are deliberately stricter than + * npm's trusted-publishing model, and scoping them here means that strictness never constrains + * another issuer. + */ + +import { ClientError } from '../../../../utility/errors/hdbError.ts'; +import type { ClaimConstraint, TokenClaims } from '../types.ts'; +import type { IdentityProviderProfile } from './index.ts'; + +/** + * Workflow references are `//@` and the ref is always a full `refs/...` name. + * Splitting on `@refs/` rather than the first or last `@` is exact, because a path segment cannot + * contain `/` — so this cannot occur inside the path, and a branch named `release@2` cannot fool it. + */ +const REF_QUALIFIER = '@refs/'; + +/** + * GitHub's default audience is the repository owner's URL, shared by every repository under that + * owner, so accepting it would make a token minted by any repo in the org valid here — the one + * mistake the audience field exists to prevent. + */ +const SHARED_DEFAULT_AUDIENCE = /^https:\/\/github\.com\/[^/]+\/?$/i; + +/** + * Structural requirements on a policy's claim set, each guarding a distinct way a policy can be + * accidentally broad. Satisfying none of a row's claims admits, in order: any repository, any + * workflow in that repository, any branch that can be pushed to it. + * + * `repository_owner` is absent from the first row on purpose — it identifies an org, not a + * repository, so it would admit every repo in the org. `ref_type` is absent from the third for the + * same shape of reason: `ref_type: tag` still admits any tag, and anyone with push access can create + * one. A tag-triggered release pins `environment` and leans on GitHub's environment protection. + * + * `job_workflow_ref` pins the workflow (second row) but deliberately does NOT gate the ref (third). + * For a reusable workflow it names the workflow that RAN, not the caller that invoked it — the + * caller's ref lives in `workflow_ref`/`ref`. Its `@ref` suffix therefore describes the reusable + * workflow's own branch, which is constant however it is called, so accepting it as a ref gate would + * admit any branch of any caller repository that references that reusable workflow: precisely the + * hole the third row exists to close. + * + * `sub` is deliberately not accepted as a pin: it varies by trigger, and its format changed for + * repositories created after 2026-07-15 (immutable subjects embed owner and repo ids). + */ +const STRUCTURAL_REQUIREMENTS = [ + { + requirement: 'pin the repository', + claims: ['repository_id', 'repository'], + because: ' (repository_id is immutable and survives renames)', + }, + { + requirement: 'pin the workflow', + claims: ['workflow_ref', 'workflow_path', 'job_workflow_ref', 'job_workflow_path'], + because: '', + }, + { + requirement: 'gate the ref', + claims: ['workflow_ref', 'ref', 'environment'], + because: ' — otherwise any branch that can be pushed to the repository can run the workflow and mint a token', + }, +]; + +/** Returns undefined rather than guessing when the value is not a ref-qualified reference. */ +export function splitWorkflowPath(workflowRef: unknown): string | undefined { + if (typeof workflowRef !== 'string') return undefined; + const qualifierIndex = workflowRef.indexOf(REF_QUALIFIER); + return qualifierIndex === -1 ? undefined : workflowRef.slice(0, qualifierIndex); +} + +export const githubActionsProfile: IdentityProviderProfile = { + name: 'GitHub Actions', + + assertPolicyIsSpecific(policyClaims: Record): void { + const constrained = new Set(Object.keys(policyClaims)); + for (const { requirement, claims, because } of STRUCTURAL_REQUIREMENTS) { + if (!claims.some((claimName) => constrained.has(claimName))) { + throw new ClientError(`claims must ${requirement} with one of: ${claims.join(', ')}${because}`); + } + } + }, + + assertAudienceIsSpecific(audience: string): void { + if (SHARED_DEFAULT_AUDIENCE.test(audience)) { + throw new ClientError( + `'audience' must identify this instance, not '${audience}' — GitHub's default audience is shared ` + + `by every repository under an owner, so a token minted for any of them would be accepted here. ` + + `Use the instance URL the CI client targets.` + ); + } + }, + + /** + * Adds `workflow_path` / `job_workflow_path` — the workflow reference with the ref removed — so a + * policy can pin the workflow *file* while gating the ref some other way. A tag-triggered release + * cannot pin `workflow_ref`, because the tag is unknown when the policy is written. + * + * A claim the token actually carries is never displaced by one we synthesized. + */ + normalizeClaims(payload: TokenClaims): TokenClaims { + const claims: TokenClaims = { ...payload }; + for (const [source, derived] of [ + ['workflow_ref', 'workflow_path'], + ['job_workflow_ref', 'job_workflow_path'], + ]) { + const path = splitWorkflowPath(payload[source]); + if (path !== undefined && claims[derived] === undefined) claims[derived] = path; + } + return claims; + }, + + /** + * A `pull_request_target` run executes the base repository's workflow with its secrets while a + * fork controls the checked-out code. A plain `pull_request` run from a fork gets no + * `id-token: write` and so cannot mint at all, but `pull_request_target` can — so it is denied + * unless a policy constrains `event_name`, which is the explicit opt-in. + */ + vetoClaims(claims: TokenClaims, policyClaims: Record): string | undefined { + if (claims.event_name === 'pull_request_target' && policyClaims.event_name === undefined) { + return 'pull_request_target is denied unless the policy constrains event_name'; + } + return undefined; + }, + + describePrincipal(claims: TokenClaims): string { + return [ + claims.repository, + claims.workflow_ref ?? claims.workflow_path, + claims.environment && `environment=${claims.environment}`, + claims.run_id && `run=${claims.run_id}`, + claims.actor && `actor=${claims.actor}`, + ] + .filter(Boolean) + .join(' '); + }, +}; diff --git a/security/authn/oidc/providers/index.ts b/security/authn/oidc/providers/index.ts new file mode 100644 index 0000000000..552a28d2fe --- /dev/null +++ b/security/authn/oidc/providers/index.ts @@ -0,0 +1,52 @@ +/** + * Provider profiles for OIDC trusted publishing (#2171). + * + * Everything issuer-specific lives behind this interface, resolved by normalized issuer, so the core + * stays generic: a new workload-identity issuer is a profile, not a change to verification, matching, + * or storage. Issuers with no registered profile fall back to `generic`, which is strict rather than + * permissive — see its `assertPolicyIsSpecific`. + */ + +import type { ClaimConstraint, TokenClaims } from '../types.ts'; +import { genericProfile } from './generic.ts'; +import { githubActionsProfile } from './githubActions.ts'; + +export interface IdentityProviderProfile { + /** Shown in policy-validation errors and the audit trail. */ + name: string; + + /** + * Rejects a claim set too broad to be a safe policy for this issuer. Called at write time, when + * the reader is an administrator who can act on the message. + */ + assertPolicyIsSpecific(policyClaims: Record): void; + + /** + * Rejects an audience the issuer shares across principals — the mistake that makes an audience + * check meaningless. A no-op for issuers with no such default. + */ + assertAudienceIsSpecific(audience: string): void; + + /** Adds issuer-specific derived claims. Must never displace a claim the token actually carries. */ + normalizeClaims(payload: TokenClaims): TokenClaims; + + /** One-line principal description for the audit trail. */ + describePrincipal(claims: TokenClaims): string; + + /** + * A match-time veto applied after the policy's own constraints pass, for runs this issuer should + * refuse unless a policy opts in explicitly. Returns a reason to deny, or undefined to allow. + */ + vetoClaims?(claims: TokenClaims, policyClaims: Record): string | undefined; +} + +const PROFILES_BY_ISSUER = new Map([ + ['https://token.actions.githubusercontent.com', githubActionsProfile], +]); + +/** Never returns undefined: an unregistered issuer gets the strict generic profile. */ +export function profileForIssuer(issuer: string): IdentityProviderProfile { + return PROFILES_BY_ISSUER.get(issuer) ?? genericProfile; +} + +export { genericProfile, githubActionsProfile }; diff --git a/security/authn/oidc/tokenExchange.ts b/security/authn/oidc/tokenExchange.ts new file mode 100644 index 0000000000..fcd18266a6 --- /dev/null +++ b/security/authn/oidc/tokenExchange.ts @@ -0,0 +1,300 @@ +'use strict'; + +// exchange_oidc_token — the unauthenticated half of OIDC trusted publishing (#2171). +// +// A CI runner presents an identity token minted by its provider. If it verifies against a stored +// trust policy, Harper mints a short-lived operation token for the user that policy names. This is +// the only unauthenticated operation that yields a credential, so it fails closed throughout and +// refuses through rejectToken, which tells the caller nothing beyond "no". + +import jwt from 'jsonwebtoken'; +import Joi from 'joi'; +import { createHash } from 'node:crypto'; +import { table, type Table } from '../../../resources/databases.ts'; +import { ClientError } from '../../../utility/errors/hdbError.ts'; +import { validateBySchema } from '../../../validation/validationWrapper.ts'; +import { loggerWithTag } from '../../../utility/logging/logger.ts'; +import harperLogger from '../../../utility/logging/harper_logger.ts'; +import * as env from '../../../utility/environment/environmentManager.ts'; +import { AUTH_AUDIT_STATUS, AUTH_AUDIT_TYPES, CONFIG_PARAMS } from '../../../utility/hdbTerms.ts'; +import { getUsersWithRolesCache } from '../../user.ts'; +import { createOperationToken } from '../../tokenAuthentication.ts'; +import { rejectToken, verifyIdentityToken } from './identityToken.ts'; +import { matchTrustPolicyClaims } from './claims.ts'; +import { normalizeIssuer } from './jwks.ts'; +import { profileForIssuer, type IdentityProviderProfile } from './providers/index.ts'; +import { loadEnabledPolicies } from './trustPolicyOperations.ts'; +import type { OidcTrustPolicy, TokenClaims } from './types.ts'; + +const logger = loggerWithTag('oidc-trust'); +const { AuthAuditLog } = harperLogger; +// Same stream and same switches as every other authentication event (security/auth.ts), so an +// operator who turns on auth auditing sees OIDC exchanges alongside Basic, Bearer, and mTLS. +const authEventLog = harperLogger.forComponent('authentication').withTag('auth-event'); + +/** Long enough to cover a slow deploy, short enough to be worthless by the time it reaches a log. */ +const EXCHANGED_TOKEN_LIFETIME_SECONDS = 3600; + +/** Keeps the replay record alive past the token's expiry by more than the verifier's clock leeway. */ +const REPLAY_RECORD_PADDING_MS = 120_000; + +/** Real identity tokens are ~1-2 KB; this bounds what we are willing to even parse. */ +const MAX_TOKEN_LENGTH = 8192; + +const TOKEN_USE_TABLE = 'hdb_oidc_token_use'; + +/** + * Spent identity tokens, keyed by the fingerprint below and expiring with the token, so the table + * stays proportional to in-flight tokens rather than to deploy history and never holds a credential. + * + * Replicated like other system tables, which extends the check across the cluster — but replication + * is asynchronous, so two simultaneous replays against different nodes can both land. That race is + * not a privilege escalation: whoever holds the token could obtain one operation token regardless. + * What it stops is the realistic case, a token that leaks after a legitimate run and is reused + * inside its window. + * + * table() also registers into `databases.system`, so the lookup finds it after the first call. + */ +let tokenUseTable: any; + +/** Drops the memo below so a test can install a different table. Not used in production. */ +export function clearTokenUseTableCache(): void { + tokenUseTable = undefined; +} + +function getTokenUseTable(): any { + // Memoized per process, but `table()` is NOT skipped merely because the table already exists. + // The systemSchema stub and the 5.3.0 directive declare only the primary key — the `expiresAt` + // TTL is not expressible through CreateTableObject — so this call is what actually installs the + // TTL, layered on top of the bootstrap. Short-circuiting on existence would mean a table that + // arrived any other way (bootstrap, replication, restore, a manual create_table) never gets it, + // and replay records would then accumulate in a system table forever. hdb_certificate_cache + // does the same two-step for the same reason. + tokenUseTable ??= table({ + table: TOKEN_USE_TABLE, + database: 'system', + // `audit: true` explicitly, NOT the default (which follows logging.auditLog). Auditing is the + // replication change feed (databases.ts: "auditing must be enabled for replication"), and + // replay records MUST replicate so a token spent on one node cannot be re-spent on another + // inside its window. Without this, an operator with logging.auditLog:false would silently lose + // cross-node replay protection. Its sibling hdb_oidc_trust is audited for the same reason. + audit: true, + attributes: [ + { name: 'id', isPrimaryKey: true }, + { name: 'policy_id' }, + { name: 'used_at' }, + { name: 'expiresAt', expiresAt: true, indexed: true }, + ], + }); + return tokenUseTable; +} + +/** + * Records the exchange in the authentication audit stream. Emitted for failures as well as successes: + * a run repeatedly failing to authenticate is exactly what an audit trail is for. + */ +function auditExchange(req: any, username: string | undefined, status: string, detail: Record) { + const logSuccessful = env.get(CONFIG_PARAMS.LOGGING_AUDITAUTHEVENTS_LOGSUCCESSFUL) ?? false; + const logFailed = env.get(CONFIG_PARAMS.LOGGING_AUDITAUTHEVENTS_LOGFAILED) ?? false; + if (status === AUTH_AUDIT_STATUS.SUCCESS ? !logSuccessful : !logFailed) return; + + // serverHandlers injects baseRequest for NO_AUTH_OPERATIONS, so the transport details are here; + // they stay optional because an in-process caller (server.operation) has none. + const baseRequest = req?.baseRequest; + const log: any = new (AuthAuditLog as any)( + username, + status, + AUTH_AUDIT_TYPES.AUTHENTICATION, + // Same precedence as every other auth event (security/auth.ts): behind a load balancer — + // which is every Fabric and cloud deployment — `ip` is the proxy, so an audit trail for the + // one unauthenticated credential-minting operation would record the proxy, not the runner. + baseRequest?.headers?.['x-forwarded-for'] ?? baseRequest?.ip, + baseRequest?.method, + baseRequest?.pathname + ); + log.auth_strategy = 'oidc'; + Object.assign(log, detail); + if (status === AUTH_AUDIT_STATUS.SUCCESS) authEventLog.info?.(log); + else authEventLog.error?.(log); +} + +/** + * Auditing must never change the outcome it is recording. The success emit sits inside the + * exchange's try, so a throw there would be caught and re-reported as a FAILURE for a request that + * actually succeeded; on the failure path a throw would replace the original error with the audit's. + * Swallowing here fixes both, and keeps the two call sites free of defensive wrapping. + */ +function auditExchangeSafely(req: any, username: string | undefined, status: string, detail: Record) { + try { + auditExchange(req, username, status, detail); + } catch (error) { + logger.warn?.(`Failed to emit OIDC exchange audit record: ${(error as Error).message}`); + } +} + +/** Verifies each distinct audience at most once, so N policies sharing one cost one verification. */ +async function findMatchingPolicy( + token: string, + issuer: string, + policies: OidcTrustPolicy[], + profile: IdentityProviderProfile +): Promise<{ policy: OidcTrustPolicy; claims: TokenClaims } | undefined> { + const claimsByAudience = new Map(); + + for (const policy of policies) { + // Re-validate specificity at exchange time. add_oidc_trust enforces these, but a stored row can + // arrive another way — replication from an older node that predates a check, or a restored/older + // system-DB backup — and the exchange must not trust that every row was validated when written. + // Skip (don't honor) any row that would be rejected for writing, so an under-specified policy + // (e.g. a repository pinned but no workflow/ref gate) can't mint a token. Fail closed. + try { + profile.assertAudienceIsSpecific(policy.audience); + profile.assertPolicyIsSpecific(policy.claims); + } catch (error) { + logger.warn?.(`Ignoring trust policy '${policy.id}': ${(error as Error).message}`); + continue; + } + + if (!claimsByAudience.has(policy.audience)) { + // verifyIdentityToken logs its own reason via rejectToken, but everything jwks.ts throws + // (unknown kid, unreachable host, non-2xx, oversized body, bad JSON, no usable keys) is + // thrown directly and would otherwise vanish here — the request then falls through to + // "no trust policy matched", which is actively misleading when a policy DID match and the + // failure was operational. Log every swallowed reason so an issuer outage is + // distinguishable from a misconfigured policy in the log. + const verified = await verifyIdentityToken(token, { issuer, audience: policy.audience }).catch((error) => { + logger.warn?.( + `Verification failed for policy '${policy.id}' (audience '${policy.audience}'): ${(error as Error).message}` + ); + return undefined; + }); + claimsByAudience.set(policy.audience, verified); + } + const claims = claimsByAudience.get(policy.audience); + if (!claims) continue; + + const mismatch = matchTrustPolicyClaims(claims, policy.claims) ?? profile.vetoClaims?.(claims, policy.claims); + if (!mismatch) return { policy, claims }; + logger.debug?.(`Trust policy '${policy.id}' did not match: ${mismatch}`); + } + return undefined; +} + +/** + * Identifies a token for replay purposes: a hash of its SIGNED INPUT (`header.payload`), not of the + * whole token string. Keyed this way rather than on `issuer|jti` because not every issuer emits + * `jti` (Azure uses `uti`, others omit it), so this is strictly more general. + * + * Hashing the raw token instead would be bypassable, because the signature segment is covered by + * nothing. Base64url decoding ignores the surplus low bits of the final character, so for an RS256 + * signature there are 16 distinct spellings of that segment that decode to identical bytes — every + * one of them passes `jwt.verify`, and every one hashes differently. One leaked identity token would + * buy 16 operation tokens. ES* issuers add a second, independent vector, since `s → n−s` is a + * different valid signature over the same input. + * + * The signed input has neither problem: it is exactly what the issuer asserted and what the + * signature covers, so every variant spelling and every malleable re-signing of one assertion + * collapses to one fingerprint. Hashed rather than stored so the table never holds a credential. + */ +function tokenFingerprint(token: string): string { + // Not `slice(0, 2).join('.')` on a split of the whole token: lastIndexOf keeps this O(1) in the + // signature length and cannot silently succeed on a malformed token with too few segments — + // verifyIdentityToken has already established this is a well-formed three-segment JWT. + const signedInput = token.slice(0, token.lastIndexOf('.')); + return createHash('sha256').update(signedInput).digest('base64url'); +} + +/** + * Marks an identity token as spent, refusing one already recorded. + * + * Recorded before the token is minted: if minting then fails the credential is burned, costing a CI + * re-run, where the reverse ordering would leave a spendable token behind. The get-then-put is not + * atomic and deliberately does not depend on Harper's optimistic concurrency to make it so — see + * getTokenUseTable for why the race is tolerable. + */ +async function recordTokenUse(fingerprint: string, claims: TokenClaims, policyId: string): Promise { + const useTable = getTokenUseTable(); + if (await useTable.get(fingerprint)) rejectToken(`token ${fingerprint.slice(0, 12)} has already been exchanged`); + + await useTable.put({ + id: fingerprint, + policy_id: policyId, + used_at: Date.now(), + expiresAt: (claims.exp as number) * 1000 + REPLAY_RECORD_PADDING_MS, + }); +} + +/** + * Exchanges a CI identity token for a short-lived Harper operation token. Unauthenticated by design — + * this operation *is* the authentication, the way create_authentication_tokens is against a password. + */ +export async function exchangeOidcToken(req: any) { + const validation = validateBySchema( + req, + Joi.object({ token: Joi.string().min(1).max(MAX_TOKEN_LENGTH).required() }).unknown(true) + ); + if (validation) throw new ClientError(validation.message); + + // Populated as identification progresses so a failure audits with whatever was established. + const audit: Record = {}; + let username: string | undefined; + try { + // Nothing is trusted from this decode; it only selects candidate policies, and the signature + // is then checked against the issuer those policies declare. + const unverified = jwt.decode(req.token, { complete: true }); + let issuer: string; + try { + issuer = normalizeIssuer((unverified?.payload as any)?.iss); + } catch { + rejectToken('token has no usable iss claim'); + } + audit.issuer = issuer; + + const profile = profileForIssuer(issuer); + audit.provider = profile.name; + + const policies = await loadEnabledPolicies(issuer); + if (policies.length === 0) rejectToken(`no enabled trust policy for issuer ${issuer}`); + + const matched = await findMatchingPolicy(req.token, issuer, policies, profile); + if (!matched) rejectToken(`no trust policy matched a token from ${issuer}`); + const { policy, claims } = matched; + audit.oidc_policy = policy.id; + audit.principal = profile.describePrincipal(claims); + + // Resolved before the token is spent, so a policy naming a deleted or deactivated user fails + // without burning a token the runner cannot re-mint. + const users = await getUsersWithRolesCache(); + const user = users?.get(policy.user); + if (!user) rejectToken(`trust policy '${policy.id}' names user '${policy.user}', which does not exist`); + if (user.active === false) rejectToken(`trust policy '${policy.id}' names inactive user '${policy.user}'`); + username = user.username; + + const fingerprint = tokenFingerprint(req.token); + audit.token_fingerprint = fingerprint.slice(0, 12); + await recordTokenUse(fingerprint, claims, policy.id); + + const operationToken = await createOperationToken( + { + username: user.username, + super_user: user.role?.permission?.super_user === true, + operations: policy.operations, + }, + EXCHANGED_TOKEN_LIFETIME_SECONDS + ); + if (policy.operations?.length) audit.scoped_operations = policy.operations; + + logger.info?.(`OIDC exchange: policy '${policy.id}' authenticated '${user.username}' for ${audit.principal}`); + auditExchangeSafely(req, username, AUTH_AUDIT_STATUS.SUCCESS, audit); + + return { + operation_token: operationToken, + expires_in: EXCHANGED_TOKEN_LIFETIME_SECONDS, + username: user.username, + policy: policy.id, + }; + } catch (error) { + auditExchangeSafely(req, username, AUTH_AUDIT_STATUS.FAILURE, audit); + throw error; + } +} diff --git a/security/authn/oidc/trustPolicyOperations.ts b/security/authn/oidc/trustPolicyOperations.ts new file mode 100644 index 0000000000..b4d1a1f4ee --- /dev/null +++ b/security/authn/oidc/trustPolicyOperations.ts @@ -0,0 +1,352 @@ +'use strict'; + +// Operations against system.hdb_oidc_trust — the trust policies for OIDC trusted publishing (#2171). +// +// A policy lets an external CI run authenticate as a Harper user without holding any Harper +// credential, so these are super_user only. Following components/secretOperations.ts, super_user is +// enforced in-handler as well as via requiredPermissions: a role's `operations` allowlist can +// otherwise delegate an SU-only operation (see the gate-2 bypass in utility/operation_authorization.ts). +// +// Rows reach peers through normal system-table replication; these operations deliberately do not +// call replicateOperation, which would double-apply on top of it. + +import Joi from 'joi'; +import { databases } from '../../../resources/databases.ts'; +import * as terms from '../../../utility/hdbTerms.ts'; +import { ClientError, hdbErrors } from '../../../utility/errors/hdbError.ts'; +import { validateBySchema } from '../../../validation/validationWrapper.ts'; +import { loggerWithTag } from '../../../utility/logging/logger.ts'; +import { getUsersWithRolesCache } from '../../user.ts'; +import { validateClaimConstraintShape } from './claims.ts'; +import { normalizeIssuer } from './jwks.ts'; +import { profileForIssuer } from './providers/index.ts'; +import { validateOperations } from '../../../utility/operationPermissions.ts'; +import type { OidcTrustPolicy } from './types.ts'; + +const { HTTP_STATUS_CODES } = hdbErrors; +const OIDC_TRUST_TABLE = terms.SYSTEM_TABLE_NAMES.OIDC_TRUST_TABLE_NAME; +// Same tag as tokenExchange: an ignored row and the exchange that ignored it belong in one stream. +const logger = loggerWithTag('oidc-trust'); + +const POLICY_ID = Joi.string() + .min(1) + .max(128) + .pattern(/^[\w.-]+$/) + .required() + .messages({ 'string.pattern.base': "'id' may contain only letters, numbers, '_', '-', and '.'" }); + +function requireSuperUser(req: any): void { + if (!req?.hdb_user?.role?.permission?.super_user) { + throw new ClientError( + `Operation '${req?.operation}' is restricted to super_user roles`, + HTTP_STATUS_CODES.FORBIDDEN + ); + } +} + +function validate(validation: any): void { + if (validation) throw new ClientError(validation.message); +} + +/** + * A typo would otherwise fail closed at request time, in CI, with nothing to point at — so it is + * caught here, where the reader is the administrator who wrote it. Delegates to the same helper + * add_role/alter_role use, so group names resolve identically rather than through a second + * definition that could drift. + * + * Known limitation, inherited rather than introduced: that helper's registry of runtime-registered + * operations is process-local, and the OPERATION_REGISTERED bridge propagates only name→thread + * routing, never grantability (server/serverHelpers/registeredOperations.ts). A component's + * `server.registerOperation` runs in a worker while this operation runs on the main thread, so an + * operation registered that way is NOT recognized here and a policy naming one is rejected. It + * fails closed — a rejected policy, never a widened one — and `add_role`, `alter_role`, and + * impersonation validation all share the gap, which is why the fix belongs to that bridge rather + * than to a local workaround here. + */ +function assertOperationsAreKnown(operations: string[]): void { + const invalidOperation = validateOperations(operations); + if (invalidOperation != null) { + throw new ClientError(`operations contains '${invalidOperation}', which is not a Harper operation`); + } +} + +/** + * The audience is matched by exact string equality at verification time (`jwt.verify`), and the CLI + * asks its provider for a token whose audience is `normalizeTarget(target)` — which supplies `:9925` + * when no port was written, and a trailing slash. So `audience=https://my-instance.example.com`, the + * natural reading of "the instance URL the CI client targets", stores a policy that can never match + * anything; the exchange then refuses with the same opaque message it gives every other failure, + * leaving the operator nothing to look at, in CI. Caught here instead, where the reader is the + * administrator who wrote it — the same bargain assertOperationsAreKnown makes. + * + * Rejected rather than rewritten: silently canonicalizing a value whose whole job is to be compared + * byte-for-byte is worse than refusing it, and it would also have to run after the profile's + * shared-audience guard to avoid disarming it. Stating the requirement keeps this independent of + * normalizeTarget's exact spelling rules, which live in the CLI; a unit test pins the two together. + * + * Only http(s) URLs are shaped this way. An issuer-specific audience (an `api://` identifier, a bare + * GUID) is not ours to constrain, so anything that is not an http(s) URL passes untouched. + */ +function assertAudienceIsCanonical(audience: string): void { + let url: URL; + try { + url = new URL(audience); + } catch { + return; + } + if (url.protocol !== 'http:' && url.protocol !== 'https:') return; + + // `url.port` is empty for a default port, so consult the raw authority — with any userinfo + // removed first, since that colon is not a port. + const authority = audience.slice(audience.indexOf('://') + 3).split(/[/?#]/)[0]; + const hasExplicitPort = !!url.port || authority.slice(authority.lastIndexOf('@') + 1).includes(':'); + if (hasExplicitPort && audience.endsWith('/')) return; + + throw new ClientError( + `'audience' must be the exact string the CI client requests its token for, which includes an ` + + `explicit port and a trailing slash (for example 'https://my-instance.example.com:9925/'). ` + + `'${audience}' is missing ${!hasExplicitPort ? 'a port' : 'a trailing slash'}, and the match is ` + + `byte-for-byte, so the policy could never authenticate.` + ); +} + +function trustTable() { + const table = (databases as any).system?.[OIDC_TRUST_TABLE]; + if (!table) { + throw new ClientError( + `OIDC trust policies are not initialized on this node (system.${OIDC_TRUST_TABLE} missing). ` + + `Run upgrade or restart the server to provision the table.` + ); + } + return table; +} + +/** + * Rebuild a plain record from a stored row's known attributes. Never spread rows — RecordObject + * prototype fields don't survive a spread reliably (see DESIGN.md). + */ +function toRecord(row: any, problem?: string): OidcTrustPolicy & Record { + return { + id: row.id, + issuer: row.issuer, + audience: row.audience, + claims: row.claims ?? {}, + user: row.user, + operations: row.operations ?? null, + // Reported as stored rather than coerced. `row.enabled !== false` would render the string + // 'false' — one of the shapes the exchange refuses — as `enabled: true`, telling the operator + // a dead trust is live. Absent still means enabled, which is the documented default. + enabled: row.enabled === undefined ? true : row.enabled, + // Present only on a row the exchange will refuse, so a listing says why the trust is dead + // instead of leaving a per-attempt log line as the only signal. Shape problems are set here; + // listOidcTrust adds the ones that depend on state outside the row (a missing or inactive user). + ...(problem ? { invalid_reason: problem } : {}), + description: row.description ?? null, + updated_by: row.updated_by ?? null, + __createdtime__: row.__createdtime__, + __updatedtime__: row.__updatedtime__, + } as any; +} + +/** + * The policy set is small and administrator-managed, so a scan beats maintaining an index — and + * sorting by id keeps both the listing and the exchange's match order deterministic rather than + * dependent on an index's iteration order. + */ +async function readPolicies( + includeDisabled: boolean, + issuer?: string, + forExchange = false +): Promise { + const table = trustTable(); + const policies: OidcTrustPolicy[] = []; + for await (const row of table.search([])) { + if (!includeDisabled && row.enabled === false) continue; + if (issuer !== undefined && row.issuer !== issuer) continue; + // Validated on BOTH paths, always against the RAW row — normalizing first is precisely what + // hides the shapes that fail open (see storedPolicyProblem). The two paths then differ in what + // they do with the answer: the exchange refuses the row, while a listing reports it. A listing + // that silently skipped a refused policy, or that showed it as healthy, would leave the + // operator's belief that the trust works corroborated by the very command they ran to check. + const problem = storedPolicyProblem(row); + if (forExchange) { + if (problem) { + logger.warn?.(`Ignoring trust policy '${row.id}': ${problem}`); + continue; + } + } + policies.push(toRecord(row, problem)); + } + return policies.sort((a, b) => String(a.id).localeCompare(String(b.id))); +} + +/** + * Why a stored row must not be honored, or undefined if it is usable. + * + * add_oidc_trust enforces all of this at write time, but a row can reach the table another way — + * replication from a node predating a check, a restored backup, a direct write to the system table — + * so the exchange re-runs the same validators rather than trusting that every row was validated when + * written. Two shapes in particular fail OPEN if merely normalized instead of rejected: + * + * operations: 'deploy_component' — a scalar rather than an array. hasOperationScope tests + * Array.isArray, so the scope is silently dropped and the token is minted UNSCOPED, carrying the + * policy user's entire role. A malformed narrowing must never widen. + * enabled: 'false' — a string. `row.enabled !== false` is true for it, so a policy + * an operator meant to disable keeps minting tokens. + * + * Shares validateOperations and validateClaimConstraintShape with the add path deliberately: two + * implementations of "is this row valid" is how the write path and the read path drift apart. + */ +export function storedPolicyProblem(row: any): string | undefined { + if (row.enabled !== undefined && typeof row.enabled !== 'boolean') { + return `'enabled' is a ${typeof row.enabled}, not a boolean`; + } + if (row.operations != null) { + if (!Array.isArray(row.operations)) return `'operations' is a ${typeof row.operations}, not an array`; + const invalidOperation = validateOperations(row.operations); + if (invalidOperation != null) return `operations contains '${invalidOperation}', which is not a Harper operation`; + } + try { + validateClaimConstraintShape(row.claims); + } catch (error) { + return (error as Error).message; + } + return undefined; +} + +/** + * The policies the exchange will consider, narrowed to one issuer. + * + * Filtered inside the scan rather than by the caller: the exchange is unauthenticated and reaches + * here having done nothing but `jwt.decode`, so an anonymous caller presenting any syntactically + * valid JWT would otherwise drive a `toRecord()` allocation for every stored policy before anything + * has been verified. The scan itself stays — the table is administrator-sized, and an index here + * would buy little (see readPolicies) — but non-matching rows now cost nothing beyond the compare. + * + * A missing table yields no policies rather than throwing. On a node where the upgrade directive has + * not run, trustTable()'s descriptive ClientError would surface to an anonymous caller as a 400 with + * a body unlike the uniform 401 every other rejection returns, which both leaks node state and + * breaks the single-message property. The SU-only add/list/drop handlers still get that error, where + * it is the useful thing to say. + */ +export function loadEnabledPolicies(issuer: string): Promise { + if (!(databases as any).system?.[OIDC_TRUST_TABLE]) return Promise.resolve([]); + return readPolicies(false, issuer, true); +} + +/** + * Creates or replaces a trust policy. + * + * Replace rather than merge: a partial update to a claim set is how an over-broad policy gets + * created by accident, and the whole point of `claims` is that every constraint in it was written + * deliberately. + */ +export async function addOidcTrust(req: any) { + requireSuperUser(req); + validate( + validateBySchema( + req, + Joi.object({ + id: POLICY_ID, + issuer: Joi.string().min(1).max(512).required(), + audience: Joi.string().min(1).max(512).required(), + claims: Joi.object().min(1).required(), + user: Joi.string().min(1).max(512).required(), + operations: Joi.array().items(Joi.string().min(1)).min(1).max(100).unique(), + // `.strict()` — Joi coerces by default, but validateBySchema keeps only `result.error` + // and discards the converted value, so `"false"` would validate cleanly and then be + // stored as the string it arrived as. `req.enabled !== false` is true for that string, + // which silently leaves a policy an operator meant to disable still minting tokens. + // A revocation control has to fail closed, so reject the string outright. + enabled: Joi.boolean().strict(), + description: Joi.string().allow('').max(1024), + }).unknown(true) + ) + ); + + const issuer = normalizeIssuer(req.issuer); + // Issuer-specific rules live in the provider profile; an unregistered issuer gets the strict + // generic profile rather than a permissive default. Each throws ClientError naming the problem. + const profile = profileForIssuer(issuer); + // On the RAW audience, before the canonical-form check below. GitHub's shared-audience guard + // matches `https://github.com/`, and a canonicalized form of that would no longer match + // the regex — checking the other way round would silently disarm it. + profile.assertAudienceIsSpecific(req.audience); + assertAudienceIsCanonical(req.audience); + if (req.operations) assertOperationsAreKnown(req.operations); + validateClaimConstraintShape(req.claims); + profile.assertPolicyIsSpecific(req.claims); + + // Resolve the target user now: a policy pointing at a user that does not exist would fail only at + // exchange time, in CI, with nothing to point at. Read the users cache directly rather than + // findAndValidateUser — with validatePassword false, that returns a bare `{ username }` for an + // unknown user instead of failing, so it cannot answer "does this user exist". + const users = await getUsersWithRolesCache(); + const targetUser = users?.get(req.user); + if (!targetUser) { + throw new ClientError(`No such user '${req.user}'; create the user before granting it to a workflow`); + } + if (targetUser.active === false) { + throw new ClientError(`User '${req.user}' is inactive; a policy naming it could never authenticate`); + } + + const table = trustTable(); + await table.put({ + id: req.id, + issuer, + audience: req.audience, + claims: req.claims, + user: req.user, + operations: req.operations ?? null, + enabled: req.enabled !== false, + description: req.description ?? null, + updated_by: req.hdb_user?.username ?? null, + }); + + const result: Record = { message: `Successfully set OIDC trust policy '${req.id}'` }; + // Not an error — an admin may genuinely want this — but a policy that hands super_user to a + // workflow deserves to be said out loud rather than discovered later. + if (targetUser.role?.permission?.super_user) { + result.warning = + `Policy '${req.id}' authenticates as '${req.user}', which is a super_user. Any run matching this ` + + `policy gains full administrative access; consider a user whose role grants only the operations CI needs.`; + } + return result; +} + +export async function listOidcTrust(req: any) { + requireSuperUser(req); + const policies = await readPolicies(true); + + // The exchange also refuses a well-formed row whose user has since been deleted or deactivated + // (tokenExchange resolves it before spending the token), with the same opaque 401 — so without + // this the listing would still say the trust is fine for the most mundane arrival of all: someone + // deletes or deactivates the CI user and every deploy starts failing. + // + // Annotated here rather than in readPolicies, and deliberately: this is one cache read for the + // whole listing on an SU-only path, where doing it per row in readPolicies would put a user lookup + // on the unauthenticated exchange path, which already resolves the user itself at the right moment. + // A shape problem already reported wins, since it is the more fundamental complaint. + const users = await getUsersWithRolesCache(); + for (const policy of policies as any[]) { + if (policy.invalid_reason) continue; + const user = users?.get(policy.user); + if (!user) policy.invalid_reason = `names user '${policy.user}', which does not exist`; + else if (user.active === false) policy.invalid_reason = `names inactive user '${policy.user}'`; + } + + return { policies }; +} + +export async function dropOidcTrust(req: any) { + requireSuperUser(req); + validate(validateBySchema(req, Joi.object({ id: POLICY_ID }).unknown(true))); + + const table = trustTable(); + const row = await table.get(req.id); + if (!row) { + throw new ClientError(`No OIDC trust policy found with id '${req.id}'`, HTTP_STATUS_CODES.NOT_FOUND); + } + await table.delete(req.id); + return { message: `Successfully dropped OIDC trust policy '${req.id}'` }; +} diff --git a/security/authn/oidc/types.ts b/security/authn/oidc/types.ts new file mode 100644 index 0000000000..9fbaed89e1 --- /dev/null +++ b/security/authn/oidc/types.ts @@ -0,0 +1,41 @@ +/** + * Types for OIDC trusted publishing (#2171). + */ + +/** One accepted value, or a set of them. */ +export type ClaimConstraint = string | string[]; + +/** + * A stored trust policy. Matching one lets an external CI run act as `user` without holding any + * Harper credential, so `claims` is validated at write time rather than trusted as written — see + * validateClaimConstraintShape (claims.ts) and the profile's assertPolicyIsSpecific for the + * structural requirements, and addOidcTrust for the rest. + */ +export interface OidcTrustPolicy { + id: string; + /** Expected `iss`, and the base for OIDC discovery. */ + issuer: string; + /** Expected `aud`. Must identify this instance — see SHARED_DEFAULT_AUDIENCE. */ + audience: string; + claims: Record; + /** The exchanged token authenticates as this user, whose role is the least-privilege boundary. */ + user: string; + /** + * Optional narrowing of that boundary, enforced on the **operations API and SQL** paths (the + * verifyPerms / verifyPermsAST gate): the minted token may invoke only these operations, even + * where the role allows more. Never widens — an operation the role forbids stays forbidden. + * + * NOT enforced on the application REST/GraphQL resource path, which authorizes through the + * table-level checkPermission and does not consult this scope — so a scoped operation token still + * carries the role's full CRUD there. Extending the scope to the resource path is a follow-up on + * the same surface as the GraphQL-bypasses-ops-allowlist gap (CORE-3061); until then, scope a + * policy's `user` to a role that is itself least-privilege for the data the token can reach. + */ + operations?: string[]; + /** Defaults to true; false keeps the policy for reference without honoring it. */ + enabled?: boolean; + description?: string; +} + +/** A verified token's payload, plus the entries the profile's normalizeClaims derives. */ +export type TokenClaims = Record; diff --git a/security/credentialProvenance.ts b/security/credentialProvenance.ts new file mode 100644 index 0000000000..ae1225f902 --- /dev/null +++ b/security/credentialProvenance.ts @@ -0,0 +1,47 @@ +/** + * Provenance for a credential minted from a workload identity exchange (#2171) rather than from a + * password. + * + * The exchange's whole guarantee is that CI holds nothing durable: a one-hour operation token, no + * refresh token, nothing on disk to leak. That guarantee is only as strong as the paths that turn + * one credential into another. `create_authentication_tokens` is such a path and does not look like + * one: it is in `NO_AUTH_OPERATIONS`, but `serverHandlers.js` special-cases it so a call with no + * username/password authenticates by Bearer token instead — so an exchanged token is accepted there + * as if it were a password, honors a caller-supplied `expires_in` verbatim, and returns a 30-day + * refresh token. That converts a minutes-long leak into a month-long one, which is the exact + * exposure this feature exists to remove. + * + * A scope (operationScope.ts) cannot carry this weight. A trust policy names `operations` only when + * the operator opts in, so the ordinary exchanged token is UNSCOPED and a scope check finds nothing + * to deny — the common case would sail through a scope-only guard. Provenance is therefore recorded + * independently of scope, on every exchanged token. + * + * Like the scope, it lives under two names depending on the carrier: the claim `workload_identity` + * on a JWT payload, and `fromWorkloadIdentity` on an in-memory principal (validateToken lifts the + * claim across). And like the scope, every path that produces a credential or a principal has to + * carry it forward — impersonation included, or a workload token launders its provenance by + * impersonating and then mints freely. + */ + +/** The JWT claim name. Signed with the rest of the payload, so it cannot be stripped in transit. */ +export const WORKLOAD_IDENTITY_CLAIM = 'workload_identity'; + +/** Stamps a token payload as workload-identity provenance. Returns the payload. */ +export function markTokenAsWorkloadIdentity(payload: T): T { + (payload as any)[WORKLOAD_IDENTITY_CLAIM] = true; + return payload; +} + +/** Lifts the claim from a verified token onto a user principal. Returns the user. */ +export function attachWorkloadIdentityToUser(user: T, claim: unknown): T { + if (claim === true) (user as any).fromWorkloadIdentity = true; + return user; +} + +/** + * True when this principal authenticated with a workload-identity token. Checked strictly against + * `true` so a forged string or object on a user record cannot widen it. + */ +export function isWorkloadIdentityPrincipal(user: unknown): boolean { + return (user as any)?.fromWorkloadIdentity === true; +} diff --git a/security/impersonation.ts b/security/impersonation.ts index 4d2ea3e14e..9405016209 100644 --- a/security/impersonation.ts +++ b/security/impersonation.ts @@ -5,6 +5,8 @@ import { validateOperations } from '../utility/operationPermissions.ts'; import { ClientError } from '../utility/errors/hdbError.ts'; import harperLogger from '../utility/logging/harper_logger.ts'; import { getRoleByName } from './role.ts'; +import { attachScopeToUser } from './operationScope.ts'; +import { attachWorkloadIdentityToUser } from './credentialProvenance.ts'; /** * Applies impersonation to a request. The authenticated user must be a super_user. @@ -38,6 +40,15 @@ export async function applyImpersonation(authenticatedUser: User, payload: Imper // Enforce downgrade: never allow escalation enforceDowngrade(impersonatedUser); + // A token's operation scope (#2174) constrains the credential regardless of which principal it + // acts as, so it survives impersonation. enforceDowngrade only bounds the impersonated role's + // permissions; without carrying the scope, a scoped super_user token would shed it by impersonating. + attachScopeToUser(impersonatedUser, (authenticatedUser as any).tokenOperations); + // Same reasoning for provenance (#2171), and the omission would be worse: impersonation returns a + // NEW principal, so dropping the marker here would let a workload token impersonate — even down to + // a lesser role — and then mint a 30-day credential that createTokens would no longer refuse. + attachWorkloadIdentityToUser(impersonatedUser, (authenticatedUser as any).fromWorkloadIdentity); + // Tag for audit trail impersonatedUser._impersonated = true; impersonatedUser._impersonatedBy = authenticatedUser.username; diff --git a/security/operationScope.ts b/security/operationScope.ts new file mode 100644 index 0000000000..863786237f --- /dev/null +++ b/security/operationScope.ts @@ -0,0 +1,33 @@ +/** + * A token operation scope (#2174) narrows a credential to a subset of its user's operations. + * + * It lives under two property names depending on the carrier: the claim `operations` on a JWT + * payload, and `tokenOperations` on an in-memory user principal (validateToken lifts the claim onto + * the user; verifyPerms reads it there). Every path that PRODUCES a credential or principal must + * carry the scope forward or the result is unscoped — the five bypasses this feature had to close + * were each a produce-a-credential path that forgot to. So the guard lives here, called at each such + * site, rather than re-inlined as `Array.isArray(...)` in six places that could drift apart. + * + * A present scope is an array — INCLUDING an empty (deny-all) array, which must be preserved rather + * than treated as "no scope". Anything else (absent/null) is skipped, so unscoped credentials behave + * exactly as they did before this existed. + */ + +export type OperationScope = string[]; + +/** True for a present scope: an array, including the empty deny-all array. */ +export function hasOperationScope(scope: unknown): scope is OperationScope { + return Array.isArray(scope); +} + +/** Copies a present scope onto a JWT payload (claim name `operations`). Returns the payload. */ +export function attachScopeToToken(payload: T, scope: unknown): T { + if (hasOperationScope(scope)) (payload as any).operations = scope; + return payload; +} + +/** Copies a present scope onto a user principal (property `tokenOperations`). Returns the user. */ +export function attachScopeToUser(user: T, scope: unknown): T { + if (hasOperationScope(scope)) (user as any).tokenOperations = scope; + return user; +} diff --git a/security/tokenAuthentication.ts b/security/tokenAuthentication.ts index 0191f906a4..09e7be8583 100644 --- a/security/tokenAuthentication.ts +++ b/security/tokenAuthentication.ts @@ -15,6 +15,13 @@ const { HTTP_STATUS_CODES, AUTHENTICATION_ERROR_MSGS } = hdbErrors; import logger from '../utility/logging/harper_logger.ts'; import * as password from '../utility/password.ts'; import { findAndValidateUser, type User } from './user.ts'; +import { attachScopeToToken, attachScopeToUser, hasOperationScope } from './operationScope.ts'; +import { + WORKLOAD_IDENTITY_CLAIM, + attachWorkloadIdentityToUser, + isWorkloadIdentityPrincipal, + markTokenAsWorkloadIdentity, +} from './credentialProvenance.ts'; import { update } from '../dataLayer/insert.ts'; import UpdateObject from '../dataLayer/UpdateObject.ts'; import * as signalling from '../utility/signalling.ts'; @@ -133,6 +140,31 @@ export async function createTokens(authObj: AuthObject): Promise { ); if (validation) throw new ClientError(validation.message); + // create_authentication_tokens is NO_AUTH, so verifyPerms — and the token-scope gate inside it — + // never runs here. A caller holding a workload-identity token (#2171) must not mint any standing + // credential through it: honoring expires_in verbatim turns a minutes-long leak into an + // arbitrarily long-lived one, and the refresh_token write below hands out a 30-day credential — + // both defeating the exchange's ephemerality guarantee. A CI token needs none of this; it already + // holds the operation token the exchange gave it, and gets a fresh one next run. + // + // Gated on provenance, NOT on the scope: a trust policy carries `operations` only when the + // operator opts in, so the ordinary exchanged token is unscoped and a scope-only check would let + // exactly the common case through. The scope check stays as well, covering a scoped credential + // from any other source. + // + // First, ahead of the user lookup and the `purpose` branch: this reads only the caller's own + // principal, so a refused request should cost no database read and write nothing. That also + // covers the login path, where a session is minted from a username alone and so cannot be + // narrowed after the fact. + // `authObj?.` — a bare createTokens() reaches here, and must still fail as invalid credentials + // below rather than as a TypeError out of this guard. + if (isWorkloadIdentityPrincipal(authObj?.hdb_user)) { + throw new ClientError('a workload identity token cannot mint authentication tokens', HTTP_STATUS_CODES.FORBIDDEN); + } + if (hasOperationScope((authObj?.hdb_user as any)?.tokenOperations)) { + throw new ClientError('a scoped token cannot mint authentication tokens', HTTP_STATUS_CODES.FORBIDDEN); + } + let user: any; try { // Trusted bypass is dispatch/async-context state (set by a component calling @@ -233,8 +265,14 @@ export async function refreshOperationToken(tokenObj: TokenObject): Promise { + const keys: JWTRSAKeys = await getJWTRSAKeys(); + const payload: { username: string; super_user: boolean; operations?: string[] } = { + username: user.username, + super_user: user.super_user, + }; + // A narrowing scope, never a grant: verifyPerms intersects it with the user's role. An empty scope + // (deny-all) is preserved rather than dropped — attachScopeToToken carries any array, which is + // what keeps this from failing open. + attachScopeToToken(payload, user.operations); + // Unconditional, because every token this function mints is minted without a password — the + // caller vouched for the user instead. That is precisely the credential that must not be able to + // trade itself for a longer-lived one, whether or not a scope narrows it. + markTokenAsWorkloadIdentity(payload); + + return jwt.sign( + payload, + { key: keys.privateKey, passphrase: keys.passphrase } satisfies Secret, + { + expiresIn, + algorithm: RSA_ALGORITHM, + subject: TOKEN_TYPE.OPERATION, + } satisfies SignOptions + ); +} + export async function validateOperationToken(token: string): Promise { return validateToken(token, TOKEN_TYPE.OPERATION); } @@ -282,6 +359,15 @@ async function validateToken(token: string, tokenType: string): Promise { throw new Error('Invalid token'); } + // Surfaced as `tokenOperations` rather than merged into role.permission.operations: that field + // is not purely narrowing (verifyPerms gate 2 treats an explicit SU-only listing as a grant), + // so merging a token scope into it could widen. verifyPerms intersects this separately, ahead + // of every bypass. + attachScopeToUser(user, tokenVerified.operations); + // Provenance rides the same lift: the claim is signed, so a caller cannot strip it to look + // like a password-minted principal at createTokens. + attachWorkloadIdentityToUser(user, tokenVerified[WORKLOAD_IDENTITY_CLAIM]); + return user; } catch (err) { logger.warn(err); diff --git a/server/serverHelpers/serverHandlers.js b/server/serverHelpers/serverHandlers.js index f38dd65f5f..4d621f1141 100644 --- a/server/serverHelpers/serverHandlers.js +++ b/server/serverHelpers/serverHandlers.js @@ -41,6 +41,9 @@ const NO_AUTH_OPERATIONS = [ terms.OPERATIONS_ENUM.CREATE_AUTHENTICATION_TOKENS, terms.OPERATIONS_ENUM.LOGIN, terms.OPERATIONS_ENUM.LOGOUT, + // The OIDC exchange *is* the authentication (#2171): the caller proves identity with a token + // from a trusted issuer, not with Harper credentials it does not have. + terms.OPERATIONS_ENUM.EXCHANGE_OIDC_TOKEN, ]; const UNSAFE_REQUEST_BODY_PROPERTIES = ['__proto__', 'constructor', 'prototype']; diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index 6ee01ef591..c98d82b740 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/server/serverHelpers/serverUtilities.ts @@ -41,6 +41,8 @@ import * as status from '../status/index.ts'; import * as regDeprecated from '../../resources/registrationDeprecated.ts'; import * as deploymentOperations from '../../components/deploymentOperations.ts'; import * as secretOperations from '../../components/secretOperations.ts'; +import * as trustPolicyOperations from '../../security/authn/oidc/trustPolicyOperations.ts'; +import * as tokenExchange from '../../security/authn/oidc/tokenExchange.ts'; import { contextStorage } from '../../resources/transaction.ts'; import { isMainThread } from 'node:worker_threads'; import { @@ -79,6 +81,42 @@ export type OperationFunctionName = ValueOf; * handles the response to the sender. */ // TODO: Replace Function type with an actual function type (e.g. (): Thingy) +/** + * Fields stripped from an operation body before it reaches the operations log. + * + * `credentials` carries a transient token on deploy_component (`registryAuth` is its pre-rename + * name — still stripped, since this runs ahead of the validation that now rejects it). `value` / + * `values` carry .env secrets from set_env_value; `value` / `envelope` carry secrets from + * set_secret. `token` is the login-purpose token (login) and the CI identity token + * (exchange_oidc_token), and is also stripped defensively — no operation declares a top-level `token`, + * but validation allows unknown keys, so a mistyped `harper deploy setup token=…` must not log a live + * credential. `refresh_token` is the 30-day credential (refresh_operation_token). + * + * Redaction runs *before* the handler, so a rejected request logs a still-spendable credential — + * which is why a new secret-bearing field belongs here rather than left to the default (harper#1527 + * was this same miss for set_env_value). + */ +export const UNLOGGABLE_OPERATION_FIELDS = [ + 'hdb_user', + 'hdbAuthHeader', + 'password', + 'payload', + 'credentials', + 'registryAuth', + 'value', + 'values', + 'envelope', + 'token', + 'refresh_token', +]; + +/** Callers gate this on log level: it allocates, and the operations log is often off. */ +export function redactForOperationLog(body: Record): Record { + const clean = { ...body }; + for (const field of UNLOGGABLE_OPERATION_FIELDS) delete clean[field]; + return clean; +} + export async function processLocalTransaction(req: OperationRequest, operationFunction: Function) { try { if ( @@ -87,31 +125,7 @@ export async function processLocalTransaction(req: OperationRequest, operationFu harperLogger.logLevel === terms.LOG_LEVELS.DEBUG || harperLogger.logLevel === terms.LOG_LEVELS.TRACE) ) { - // Need to remove auth variables and secret-bearing fields, but we don't want to create - // an object unless the logging is actually going to happen. credentials carries a - // transient token on deploy_component (registryAuth is its pre-rename name — still - // stripped, since this runs ahead of the validation that now rejects it, and a stale - // caller's token must not reach the log); value/values carry .env secrets from - // set_env_value; value/envelope carry secrets from set_secret — none may reach the - // operations log. `token` is stripped defensively: no operation declares one at the top - // level today, but validation allows unknown keys, so a caller that sends one anyway (a - // mistyped `harper deploy setup token=…`) would otherwise log a live credential. - /* eslint-disable no-unused-vars, @typescript-eslint/no-unused-vars */ - const { - hdb_user, - hdbAuthHeader, - password, - payload, - credentials, - registryAuth, - value, - values, - envelope, - token, - ...cleanBody - } = req.body; - /* eslint-enable no-unused-vars, @typescript-eslint/no-unused-vars */ - operationLog.info(cleanBody); + operationLog.info(redactForOperationLog(req.body)); } } catch (e) { operationLog.error(e); @@ -235,8 +249,16 @@ export function chooseOperation(json: OperationRequestBody, bypassAuth = false) const sqlStatement = json.operation === 'sql' ? json.sql : json.search_operation.sql; const parsedSqlObject = sql.convertSQLToAST(sqlStatement); json.parsed_sql_object = parsedSqlObject; + // NOTE: a job's SQL is re-parsed from its nested search_operation when the job runs, so the + // check there sees `sql` rather than the job's own operation. Carrying the real one on the + // request was tried and reverted: on the direct-SQL path the request is the client's body, + // so any property consulted by that check is forgeable, and it is the only gate on that + // path. This changes no outcome today — the branch that would act on the denial is dead + // (#2202) — but #2202 needs an unforgeable carrier before making it live. if (!bypassAuth) { - const astPermCheck = sql.checkASTPermissions(json, parsedSqlObject); + // `json.operation` explicitly — the operation this dispatch already resolved, not a + // field read back off the request body. + const astPermCheck = sql.checkASTPermissions(json, parsedSqlObject, json.operation); if (astPermCheck) { operationLog.error(`${HTTP_STATUS_CODES.FORBIDDEN} from operation ${json.operation}`); operationLog.warn(`User '${json.hdb_user?.username}' is not permitted to ${json.operation}`); @@ -255,7 +277,10 @@ export function chooseOperation(json: OperationRequestBody, bypassAuth = false) !bypassAuth && json.operation !== terms.OPERATIONS_ENUM.CREATE_AUTHENTICATION_TOKENS && json.operation !== terms.OPERATIONS_ENUM.LOGIN && - json.operation !== terms.OPERATIONS_ENUM.LOGOUT + json.operation !== terms.OPERATIONS_ENUM.LOGOUT && + // Same rationale: the OIDC exchange authenticates its own caller (#2171), so there is no + // hdb_user for verifyPerms to check against. + json.operation !== terms.OPERATIONS_ENUM.EXCHANGE_OIDC_TOKEN ) { const functionToCheck = job_operation_function === undefined ? operation_function : job_operation_function; const operation_json = json.search_operation ? json.search_operation : json; @@ -263,7 +288,12 @@ export function chooseOperation(json: OperationRequestBody, bypassAuth = false) operation_json.hdb_user = json.hdb_user; } - const verifyPermsResult = opAuth.verifyPerms(operation_json, functionToCheck); + // Pass the top-level operation for the token-scope check: for an export job, operation_json + // is the nested search_operation, so json.operation (export_local/export_to_s3) is the op the + // scope must gate — not the inner search. + const verifyPermsResult = opAuth.verifyPerms(operation_json, functionToCheck, { + apiOperation: json.operation, + }); if (verifyPermsResult) { operationLog.error(`${HTTP_STATUS_CODES.FORBIDDEN} from operation ${json.operation}`); @@ -581,6 +611,19 @@ function initializeOperationFunctionMap(): Map 0` + // discards a denial that was computed correctly. Pre-existing and not specific to this + // feature, so it is fixed separately in #2202 rather than bundled here. This PR does not + // depend on it: the outer gate in serverUtilities refuses an out-of-scope job operation, + // and sqlWriteScopeDenial refuses write SQL, both through correct truthiness tests. if (permissionsCheck && permissionsCheck.length > 0) { return callback(UNAUTHORIZED_RESPONSE, permissionsCheck); } diff --git a/unitTests/bin/cliOperations.test.js b/unitTests/bin/cliOperations.test.js index 22d126a3fd..599fef2880 100644 --- a/unitTests/bin/cliOperations.test.js +++ b/unitTests/bin/cliOperations.test.js @@ -52,6 +52,42 @@ describe('cliOperations', () => { fs.ensureDirSync(testDir); }); + // `token` is stripped from every request body as transport-only, so that a mistyped `deploy + // setup=...` cannot carry a PAT to the server. exchange_oidc_token is the one operation whose body + // legitimately has a top-level `token` — the identity token IS the request — so the strip must not + // apply to it, or the issuer-agnostic path is unusable through the generic CLI (#2171). + it('sends the token for exchange_oidc_token instead of stripping it', async () => { + let sentBody; + commonUtilsModule.httpRequest = async (_options, body) => { + sentBody = body; + return { statusCode: 200, body: JSON.stringify({ operation_token: 'minted' }) }; + }; + + await cliOperationsModule.cliOperations( + { operation: 'exchange_oidc_token', token: 'the-identity-token', target: 'example.com' }, + true + ); + + assert.strictEqual(sentBody.operation, 'exchange_oidc_token'); + assert.strictEqual(sentBody.token, 'the-identity-token', 'the identity token must reach the server'); + }); + + // The other direction: the strip still protects every other operation. + it('still strips a top-level token from other operations', async () => { + let sentBody; + commonUtilsModule.httpRequest = async (_options, body) => { + sentBody = body; + return { statusCode: 200, body: JSON.stringify({}) }; + }; + + await cliOperationsModule.cliOperations( + { operation: 'test', token: 'a-pat-that-must-not-leave', target: 'example.com' }, + true + ); + + assert.strictEqual(sentBody.token, undefined, 'a PAT must not reach the server on other operations'); + }); + it('Leg 1: should use non-expired token directly', async () => { const target = 'https://example.com:9925/'; saveCredentials(target, { @@ -299,6 +335,159 @@ describe('cliOperations', () => { }); }); + // OIDC trusted publishing (#2171): the runner proves its identity to the cluster instead of + // carrying a Harper credential. Ambient, so it ranks below everything explicitly configured. + describe('CI identity auth (OIDC)', () => { + const target = 'https://example.com:9925/'; + const envVars = [ + 'HARPER_CLI_OPERATION_TOKEN', + 'HARPER_CLI_REFRESH_TOKEN', + 'CLI_TARGET_OPERATION_TOKEN', + 'CLI_TARGET_REFRESH_TOKEN', + 'ACTIONS_ID_TOKEN_REQUEST_URL', + 'ACTIONS_ID_TOKEN_REQUEST_TOKEN', + ]; + const saved = {}; + let originalFetch; + let identityRequests; + + beforeEach(() => { + for (const v of envVars) { + saved[v] = process.env[v]; + delete process.env[v]; + } + process.env.ACTIONS_ID_TOKEN_REQUEST_URL = 'https://pipelines.example/idtoken?api-version=2.0'; + process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = 'runner-request-token'; + + identityRequests = []; + originalFetch = globalThis.fetch; + globalThis.fetch = async (url) => { + identityRequests.push(new URL(String(url))); + return new Response(JSON.stringify({ value: 'identity.token.value' }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + for (const v of envVars) { + if (saved[v] === undefined) delete process.env[v]; + else process.env[v] = saved[v]; + } + }); + + it('exchanges a CI identity for an operation token when nothing else is configured', async () => { + const requested = []; + commonUtilsModule.httpRequest = async (options, req) => { + requested.push({ auth: options.headers.Authorization, operation: req.operation }); + if (req.operation === 'exchange_oidc_token') { + return { + statusCode: 200, + body: JSON.stringify({ operation_token: 'oidc-op-token', username: 'ci-deploy', policy: 'p' }), + }; + } + return { statusCode: 200, body: JSON.stringify({ success: true }) }; + }; + + const result = await cliOperationsModule.cliOperations({ operation: 'test', target: 'example.com' }, true); + + assert.strictEqual(result.success, true); + assert.deepStrictEqual( + requested.map((r) => r.operation), + ['exchange_oidc_token', 'test'] + ); + assert.strictEqual(requested[1].auth, 'Bearer oidc-op-token'); + // The audience must be this instance, not the provider's shared default. + assert.strictEqual(identityRequests[0].searchParams.get('audience'), target); + }); + + // Adding `id-token: write` to a workflow that still sets HARPER_CLI_REFRESH_TOKEN must not + // silently change which identity deploys. + it('leaves a configured env token in charge', async () => { + process.env.HARPER_CLI_OPERATION_TOKEN = 'env-op-token'; + tokenAuthModule.isJWTExpired = () => false; + + const requested = []; + commonUtilsModule.httpRequest = async (options, req) => { + requested.push({ auth: options.headers.Authorization, operation: req.operation }); + return { statusCode: 200, body: JSON.stringify({ success: true }) }; + }; + + await cliOperationsModule.cliOperations({ operation: 'test', target: 'example.com' }, true); + + assert.deepStrictEqual( + requested.map((r) => r.operation), + ['test'] + ); + assert.strictEqual(requested[0].auth, 'Bearer env-op-token'); + assert.strictEqual(identityRequests.length, 0, 'must not ask the provider for an identity token'); + }); + + it('leaves saved login credentials in charge', async () => { + saveCredentials(target, { operation_token: 'file-token', refresh_token: 'file-refresh' }); + tokenAuthModule.isJWTExpired = () => false; + + let seenAuth; + commonUtilsModule.httpRequest = async (options) => { + seenAuth = options.headers.Authorization; + return { statusCode: 200, body: JSON.stringify({ success: true }) }; + }; + + await cliOperationsModule.cliOperations({ operation: 'test', target: 'example.com' }, true); + assert.strictEqual(seenAuth, 'Bearer file-token'); + assert.strictEqual(identityRequests.length, 0); + }); + + // Same rationale as the env-var tokens: a local operation is trusted via bypassLocalAuth, + // which only applies when no Authorization header is present. + it('does not exchange for a local (no-target) operation', async () => { + const originalGetHdbPid = processManagementModule.getHdbPid; + const originalInitConfig = configUtilsModule.initConfig; + const originalGetConfigPath = configUtilsModule.getConfigPath; + const socketPath = path.join(testDir, 'oidc-local-check.sock'); + fs.ensureFileSync(socketPath); + configUtilsModule.initConfig = () => {}; + processManagementModule.getHdbPid = () => 12345; + configUtilsModule.getConfigPath = () => socketPath; + + const requested = []; + commonUtilsModule.httpRequest = async (options, req) => { + requested.push({ auth: options.headers.Authorization, operation: req.operation }); + return { statusCode: 200, body: JSON.stringify({ success: true }) }; + }; + + try { + await cliOperationsModule.cliOperations({ operation: 'test' }, true); + } finally { + processManagementModule.getHdbPid = originalGetHdbPid; + configUtilsModule.initConfig = originalInitConfig; + configUtilsModule.getConfigPath = originalGetConfigPath; + } + + assert.strictEqual(identityRequests.length, 0); + assert.strictEqual(requested.length, 1); + assert.strictEqual(requested[0].auth, undefined); + }); + + // A rejected exchange must not leave a half-authenticated request; it goes out with no + // Authorization header and fails the way an unauthenticated request normally does. + it('proceeds unauthenticated when the exchange is rejected', async () => { + const requested = []; + commonUtilsModule.httpRequest = async (options, req) => { + requested.push({ auth: options.headers.Authorization, operation: req.operation }); + if (req.operation === 'exchange_oidc_token') { + return { statusCode: 401, body: JSON.stringify({ error: 'Identity token was rejected' }) }; + } + return { statusCode: 200, body: JSON.stringify({ success: true }) }; + }; + + await cliOperationsModule.cliOperations({ operation: 'test', target: 'example.com' }, true); + assert.strictEqual(requested[1].auth, undefined); + }); + }); + // The resolved target is an identity, not a credential: it keys ~/.harperdb/credentials.json, is // echoed by "Connecting to ...", written to .env, and emitted by `harper login --for-ci`. // Userinfo is stripped once, in normalizeTarget, so none of those sites can leak a password. diff --git a/unitTests/bin/workloadIdentity.test.js b/unitTests/bin/workloadIdentity.test.js new file mode 100644 index 0000000000..604630e806 --- /dev/null +++ b/unitTests/bin/workloadIdentity.test.js @@ -0,0 +1,171 @@ +'use strict'; + +const assert = require('node:assert'); +const { workloadIdentityAvailable, exchangeWorkloadIdentityForToken } = require('#src/bin/workloadIdentity'); +const commonUtilsModule = require('#src/utility/common_utils'); + +const REQUEST_URL = 'https://pipelines.actions.githubusercontent.com/abc/idtoken?api-version=2.0'; +const REQUEST_TOKEN = 'runner-request-token'; +const AUDIENCE = 'https://my-instance.harperdb.io:9925/'; + +describe('workloadIdentity', () => { + let originalFetch; + let originalHttpRequest; + let originalEnv; + let originalConsoleError; + let fetchCalls; + let operationCalls; + let stderr; + + before(() => { + originalFetch = globalThis.fetch; + originalHttpRequest = commonUtilsModule.httpRequest; + originalConsoleError = console.error; + }); + + after(() => { + globalThis.fetch = originalFetch; + commonUtilsModule.httpRequest = originalHttpRequest; + console.error = originalConsoleError; + }); + + beforeEach(() => { + originalEnv = { + url: process.env.ACTIONS_ID_TOKEN_REQUEST_URL, + token: process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN, + }; + process.env.ACTIONS_ID_TOKEN_REQUEST_URL = REQUEST_URL; + process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = REQUEST_TOKEN; + + fetchCalls = []; + operationCalls = []; + stderr = []; + console.error = (message) => stderr.push(String(message)); + + globalThis.fetch = async (url, init) => { + fetchCalls.push({ url: new URL(String(url)), init }); + return new Response(JSON.stringify({ value: 'identity.token.value' }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }; + commonUtilsModule.httpRequest = async (options, body) => { + operationCalls.push({ options, body }); + return { + statusCode: 200, + body: JSON.stringify({ + operation_token: 'minted-operation-token', + username: 'ci-deploy', + policy: 'my-app-prod', + expires_in: 3600, + }), + }; + }; + }); + + afterEach(() => { + if (originalEnv.url === undefined) delete process.env.ACTIONS_ID_TOKEN_REQUEST_URL; + else process.env.ACTIONS_ID_TOKEN_REQUEST_URL = originalEnv.url; + if (originalEnv.token === undefined) delete process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN; + else process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = originalEnv.token; + }); + + describe('workloadIdentityAvailable', () => { + it('is true when the runner offers an identity token', () => { + assert.strictEqual(workloadIdentityAvailable(), true); + }); + + // GitHub sets both together; either one missing means the workflow did not grant + // `id-token: write`, which is a configuration answer rather than something to report. + it('is false unless both variables are present', () => { + delete process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN; + assert.strictEqual(workloadIdentityAvailable(), false); + process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = REQUEST_TOKEN; + delete process.env.ACTIONS_ID_TOKEN_REQUEST_URL; + assert.strictEqual(workloadIdentityAvailable(), false); + }); + }); + + describe('exchangeWorkloadIdentityForToken', () => { + it('returns an operation token', async () => { + const token = await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE); + assert.strictEqual(token, 'minted-operation-token'); + }); + + // The audience is what binds the token to this instance; GitHub's default is shared by every + // repository under an owner, so it must be set explicitly on the request. + it('requests the token for this instance as the audience', async () => { + await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE); + assert.strictEqual(fetchCalls.length, 1); + assert.strictEqual(fetchCalls[0].url.searchParams.get('audience'), AUDIENCE); + // The api-version already on the URL must survive. + assert.strictEqual(fetchCalls[0].url.searchParams.get('api-version'), '2.0'); + assert.strictEqual(fetchCalls[0].init.headers.authorization, `Bearer ${REQUEST_TOKEN}`); + }); + + it('sends the identity token to exchange_oidc_token', async () => { + await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE); + assert.strictEqual(operationCalls.length, 1); + assert.deepStrictEqual(operationCalls[0].body, { + operation: 'exchange_oidc_token', + token: 'identity.token.value', + }); + }); + + it('names the policy and user it authenticated as', async () => { + await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE); + assert.ok( + stderr.some((line) => line.includes('ci-deploy') && line.includes('my-app-prod')), + `expected the identity to be reported; got ${JSON.stringify(stderr)}` + ); + }); + + it('does nothing on a runner with no identity to offer', async () => { + delete process.env.ACTIONS_ID_TOKEN_REQUEST_URL; + assert.strictEqual(await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE), undefined); + assert.strictEqual(fetchCalls.length, 0); + assert.strictEqual(operationCalls.length, 0); + }); + + it('gives up without exchanging when the provider refuses a token', async () => { + globalThis.fetch = async () => new Response('forbidden', { status: 403 }); + assert.strictEqual(await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE), undefined); + assert.strictEqual(operationCalls.length, 0, 'nothing to exchange'); + assert.ok(stderr.some((line) => line.includes('403'))); + }); + + it('gives up when the provider returns no token value', async () => { + globalThis.fetch = async () => + new Response(JSON.stringify({}), { status: 200, headers: { 'content-type': 'application/json' } }); + assert.strictEqual(await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE), undefined); + assert.strictEqual(operationCalls.length, 0); + }); + + // The server deliberately does not say which check failed, so the CLI has to point at what the + // operator can inspect — otherwise this surfaces as a bare 401 later. + it('explains a rejected exchange', async () => { + commonUtilsModule.httpRequest = async () => ({ + statusCode: 401, + body: '{"error":"Identity token was rejected"}', + }); + assert.strictEqual(await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE), undefined); + assert.ok( + stderr.some((line) => line.includes('list_oidc_trust')), + `expected actionable guidance; got ${JSON.stringify(stderr)}` + ); + }); + + it('returns undefined when the exchange yields no token', async () => { + commonUtilsModule.httpRequest = async () => ({ statusCode: 200, body: '{}' }); + assert.strictEqual(await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE), undefined); + }); + + it('survives a transport failure', async () => { + commonUtilsModule.httpRequest = async () => { + throw new Error('socket hang up'); + }; + assert.strictEqual(await exchangeWorkloadIdentityForToken({ headers: {} }, AUDIENCE), undefined); + assert.ok(stderr.some((line) => line.includes('socket hang up'))); + }); + }); +}); diff --git a/unitTests/components/mcp/tools/operations.test.js b/unitTests/components/mcp/tools/operations.test.js index 617e9ea659..7bde76b3ca 100644 --- a/unitTests/components/mcp/tools/operations.test.js +++ b/unitTests/components/mcp/tools/operations.test.js @@ -126,10 +126,13 @@ describe('mcp/tools/operations — registration', () => { } }); - it('excludes ALL secret-store operations from the default-allow surface', () => { - // list_secrets matches the `list_*` glob and get_secrets_public_key looks like a safe - // getter, but the secrets store is key custody management — never default-exposed to an - // LLM surface. DEFAULT_EXCLUDED pins every secret op, present and future-glob-matching. + it('excludes ALL credential-management operations from the default-allow surface', () => { + // list_secrets / list_oidc_trust match the `list_*` glob and get_secrets_public_key looks + // like a safe getter, but both stores are credential management — never default-exposed to + // an LLM surface. The secrets store is key custody; an OIDC trust policy grants an external + // workflow the right to authenticate as a Harper user, so reading the set names the + // repository worth compromising and writing one is a way to grant access. + // DEFAULT_EXCLUDED pins each op, present and future-glob-matching. const secretOps = [ 'set_secret', 'grant_secret', @@ -138,15 +141,19 @@ describe('mcp/tools/operations — registration', () => { 'delete_secret', 'get_secrets_public_key', ]; - assert.deepEqual([...DEFAULT_EXCLUDED].sort(), [...secretOps].sort()); + // exchange_oidc_token matches no DEFAULT_ALLOW glob today, but it is the operation that hands + // out a credential — pinned here so a future glob cannot quietly pull it onto the surface. + const oidcTrustOps = ['add_oidc_trust', 'list_oidc_trust', 'drop_oidc_trust', 'exchange_oidc_token']; + const credentialOps = [...secretOps, ...oidcTrustOps]; + assert.deepEqual([...DEFAULT_EXCLUDED].sort(), [...credentialOps].sort()); _setOperationFunctionMapForTest( - makeOpMap([...secretOps.map((name) => [name, async () => ({})]), ['list_users', async () => ({})]]) + makeOpMap([...credentialOps.map((name) => [name, async () => ({})]), ['list_users', async () => ({})]]) ); registerOperationsTools(); const { tools } = listTools({ user: SUPER, profile: 'operations', sessionId: 's', limit: 200 }); const names = new Set(tools.map((t) => t.name)); - for (const op of secretOps) { + for (const op of credentialOps) { assert.ok(!names.has(op), `${op} must not be on the default MCP surface`); } assert.ok(names.has('list_users'), 'list_* glob still works for non-excluded ops'); diff --git a/unitTests/security/authn/oidc/claims.test.js b/unitTests/security/authn/oidc/claims.test.js new file mode 100644 index 0000000000..5cb96647b8 --- /dev/null +++ b/unitTests/security/authn/oidc/claims.test.js @@ -0,0 +1,124 @@ +'use strict'; + +const assert = require('node:assert'); +const { matchTrustPolicyClaims, validateClaimConstraintShape } = require('#src/security/authn/oidc/claims'); + +// Deliberately not a GitHub token: this layer must not know which issuer minted anything. +const CLAIMS = Object.freeze({ + iss: 'https://oidc.example.com', + aud: 'https://my-instance.harperdb.io:9925/', + sub: 'system:serviceaccount:prod:deployer', + namespace: 'prod', + service_account: 'deployer', + numeric_id: '67890', + exp: 1_800_000_300, +}); + +describe('oidc claims', () => { + describe('matchTrustPolicyClaims', () => { + it('matches when every constraint is satisfied', () => { + const reason = matchTrustPolicyClaims(CLAIMS, { + sub: 'system:serviceaccount:prod:deployer', + namespace: 'prod', + }); + assert.strictEqual(reason, undefined); + }); + + it('accepts any value from a set', () => { + assert.strictEqual(matchTrustPolicyClaims(CLAIMS, { namespace: ['prod', 'staging'] }), undefined); + }); + + it('rejects a value outside the set', () => { + const reason = matchTrustPolicyClaims(CLAIMS, { namespace: ['staging', 'dev'] }); + assert.ok(reason); + assert.match(reason, /namespace/); + }); + + // Matching is exact membership, never a prefix. This is the classic trusted-publishing + // escalation: `HarperFast/my-app-evil` is a repository anyone can create, so a policy pinning + // `HarperFast/my-app` must not admit it. Both argument orders are pinned, because a + // `startsWith` introduced on either side would be a hole and only one order catches each. + it('does not match a repository that merely extends the pinned one', () => { + const reason = matchTrustPolicyClaims( + { repository: 'HarperFast/my-app-evil' }, + { repository: 'HarperFast/my-app' } + ); + assert.ok(reason, 'a longer repository name must not satisfy a shorter pin'); + assert.match(reason, /repository/); + }); + + it('does not match a pinned repository that merely extends the token claim', () => { + const reason = matchTrustPolicyClaims( + { repository: 'HarperFast/my-app' }, + { repository: 'HarperFast/my-app-evil' } + ); + assert.ok(reason, 'a shorter repository name must not satisfy a longer pin'); + }); + + // Same property on `sub`, the claim the generic profile requires a policy to pin. + it('does not match a sub that merely extends the pinned one', () => { + assert.ok( + matchTrustPolicyClaims( + { sub: 'system:serviceaccount:prod:deployer-admin' }, + { sub: 'system:serviceaccount:prod:deployer' } + ) + ); + }); + + // The central fail-closed property: a constrained claim the token does not carry must deny, + // so a policy cannot be silently weakened by an issuer that stops emitting a claim. + it('rejects when a constrained claim is absent from the token', () => { + const reason = matchTrustPolicyClaims(CLAIMS, { environment: 'production' }); + assert.ok(reason); + assert.match(reason, /environment/); + }); + + it('rejects an empty-string claim value', () => { + assert.ok(matchTrustPolicyClaims({ ...CLAIMS, namespace: '' }, { namespace: 'prod' })); + }); + + it('rejects a policy that constrains nothing', () => { + assert.ok(matchTrustPolicyClaims(CLAIMS, {})); + }); + + it('compares a numerically-encoded claim as a string', () => { + assert.strictEqual(matchTrustPolicyClaims({ ...CLAIMS, numeric_id: 67890 }, { numeric_id: '67890' }), undefined); + }); + + it('refuses to compare a non-scalar claim', () => { + for (const value of [true, { nested: 'object' }, ['array'], null]) { + assert.ok( + matchTrustPolicyClaims({ ...CLAIMS, namespace: value }, { namespace: 'prod' }), + `expected rejection for ${JSON.stringify(value)}` + ); + } + }); + }); + + // Shape only — whether a claim set is *specific enough* is the provider profile's call. + describe('validateClaimConstraintShape', () => { + it('accepts a well-formed constraint set', () => { + assert.doesNotThrow(() => validateClaimConstraintShape({ sub: 'x', namespace: ['a', 'b'] })); + }); + + it('rejects a non-object', () => { + for (const value of [undefined, null, 'claims', 42, ['sub']]) { + assert.throws(() => validateClaimConstraintShape(value), /claims must be an object/); + } + }); + + it('rejects an empty object', () => { + assert.throws(() => validateClaimConstraintShape({}), /at least one claim/); + }); + + it('rejects an empty accepted-value set', () => { + assert.throws(() => validateClaimConstraintShape({ sub: 'x', event_name: [] }), /at least one value/); + }); + + it('rejects non-string and empty-string values', () => { + assert.throws(() => validateClaimConstraintShape({ sub: '' }), /non-empty/); + assert.throws(() => validateClaimConstraintShape({ sub: 42 }), /non-empty/); + assert.throws(() => validateClaimConstraintShape({ sub: ['x', ''] }), /non-empty/); + }); + }); +}); diff --git a/unitTests/security/authn/oidc/jwks.test.js b/unitTests/security/authn/oidc/jwks.test.js new file mode 100644 index 0000000000..68085455f3 --- /dev/null +++ b/unitTests/security/authn/oidc/jwks.test.js @@ -0,0 +1,289 @@ +'use strict'; + +const assert = require('node:assert'); +const { generateKeyPairSync, createPublicKey } = require('node:crypto'); +const { getSigningKey, normalizeIssuer, clearJwksCache } = require('#src/security/authn/oidc/jwks'); + +const ISSUER = 'https://token.actions.githubusercontent.com'; +const JWKS_URI = 'https://token.actions.githubusercontent.com/.well-known/jwks'; +const DISCOVERY_URI = ISSUER + '/.well-known/openid-configuration'; + +describe('oidc jwks', () => { + describe('normalizeIssuer', () => { + it('drops a trailing slash so one issuer is one cache entry', () => { + assert.strictEqual(normalizeIssuer(ISSUER + '/'), ISSUER); + assert.strictEqual(normalizeIssuer(ISSUER), ISSUER); + }); + + it('keeps a path-qualified issuer', () => { + assert.strictEqual(normalizeIssuer('https://gitlab.example.com/oidc/'), 'https://gitlab.example.com/oidc'); + }); + + it('requires https', () => { + assert.throws(() => normalizeIssuer('http://token.actions.githubusercontent.com'), /https/); + }); + + it('rejects a query or fragment', () => { + assert.throws(() => normalizeIssuer(ISSUER + '?a=b'), /query or fragment/); + assert.throws(() => normalizeIssuer(ISSUER + '#frag'), /query or fragment/); + }); + + it('rejects values that are not URLs', () => { + for (const value of ['', undefined, null, 42, 'token.actions.githubusercontent.com']) { + assert.throws(() => normalizeIssuer(value), `expected rejection for ${JSON.stringify(value)}`); + } + }); + }); + + describe('getSigningKey', () => { + let realFetch; + let requestLog; + let respond; + let signingJwk; + + before(() => { + const { publicKey } = generateKeyPairSync('rsa', { + modulusLength: 2048, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }); + signingJwk = { ...createPublicKey(publicKey).export({ format: 'jwk' }), kid: 'key-1', use: 'sig', alg: 'RS256' }; + }); + + beforeEach(() => { + clearJwksCache(); + requestLog = []; + realFetch = globalThis.fetch; + // Route fetch through a per-test responder so the real caching, discovery, and bounding + // logic runs — only the network is displaced. + globalThis.fetch = async (url) => { + requestLog.push(String(url)); + return respond(String(url)); + }; + respond = (url) => json(url === DISCOVERY_URI ? discoveryDocument() : jwksDocument([signingJwk])); + }); + + afterEach(() => { + globalThis.fetch = realFetch; + clearJwksCache(); + }); + + function json(body, init) { + return new Response(typeof body === 'string' ? body : JSON.stringify(body), { + status: 200, + headers: { 'content-type': 'application/json' }, + ...init, + }); + } + + function discoveryDocument(overrides = {}) { + return { issuer: ISSUER, jwks_uri: JWKS_URI, ...overrides }; + } + + function jwksDocument(keys) { + return { keys }; + } + + it('discovers the JWKS and resolves a key by kid', async () => { + const key = await getSigningKey(ISSUER, 'key-1'); + assert.strictEqual(key.type, 'public'); + assert.deepStrictEqual(requestLog, [DISCOVERY_URI, JWKS_URI]); + }); + + it('serves later lookups from cache', async () => { + await getSigningKey(ISSUER, 'key-1'); + await getSigningKey(ISSUER, 'key-1'); + assert.strictEqual(requestLog.length, 2, 'expected no refetch for a cached kid'); + }); + + it('treats a trailing slash as the same issuer', async () => { + await getSigningKey(ISSUER, 'key-1'); + await getSigningKey(ISSUER + '/', 'key-1'); + assert.strictEqual(requestLog.length, 2, 'expected the normalized issuer to hit the same cache entry'); + }); + + it('collapses concurrent lookups onto one fetch', async () => { + const keys = await Promise.all([ + getSigningKey(ISSUER, 'key-1'), + getSigningKey(ISSUER, 'key-1'), + getSigningKey(ISSUER, 'key-1'), + ]); + assert.strictEqual(keys.length, 3); + assert.deepStrictEqual(requestLog, [DISCOVERY_URI, JWKS_URI]); + }); + + // An unknown kid is either a rotation or a forgery. The first triggers one refetch; repeats + // inside the rate-limit window must not, or the unauthenticated exchange endpoint becomes a + // way to drive outbound requests. + it('refetches once for an unknown kid, then rate-limits', async () => { + await getSigningKey(ISSUER, 'key-1'); + assert.strictEqual(requestLog.length, 2); + + await assert.rejects(() => getSigningKey(ISSUER, 'forged-kid'), /not recognized/); + assert.strictEqual(requestLog.length, 4, 'expected one refetch for the unknown kid'); + + await assert.rejects(() => getSigningKey(ISSUER, 'forged-kid'), /not recognized/); + await assert.rejects(() => getSigningKey(ISSUER, 'another-forged-kid'), /not recognized/); + assert.strictEqual(requestLog.length, 4, 'expected no further refetches inside the window'); + }); + + it('picks up a rotated key when the issuer publishes it', async () => { + await getSigningKey(ISSUER, 'key-1'); + const rotated = { ...signingJwk, kid: 'key-2' }; + respond = (url) => json(url === DISCOVERY_URI ? discoveryDocument() : jwksDocument([signingJwk, rotated])); + const key = await getSigningKey(ISSUER, 'key-2'); + assert.strictEqual(key.type, 'public'); + }); + + it('falls back to a cached key when a refresh fails', async () => { + await getSigningKey(ISSUER, 'key-1'); + respond = () => { + throw new Error('network down'); + }; + // The unknown kid forces a refresh, which fails; the still-cached kid must survive it. + await assert.rejects(() => getSigningKey(ISSUER, 'forged-kid')); + const key = await getSigningKey(ISSUER, 'key-1'); + assert.strictEqual(key.type, 'public'); + }); + + it('rejects a discovery document that declares a different issuer', async () => { + respond = (url) => + json( + url === DISCOVERY_URI ? discoveryDocument({ issuer: 'https://evil.example.com' }) : jwksDocument([signingJwk]) + ); + await assert.rejects(() => getSigningKey(ISSUER, 'key-1'), /different issuer/); + }); + + it('rejects a non-https jwks_uri', async () => { + respond = (url) => + json(url === DISCOVERY_URI ? discoveryDocument({ jwks_uri: 'http://insecure.example/jwks' }) : {}); + await assert.rejects(() => getSigningKey(ISSUER, 'key-1'), /https jwks_uri/); + }); + + // A symmetric key in a JWKS is the setup for algorithm confusion; it must never become a + // candidate signing key. + it('ignores symmetric and non-signing keys', async () => { + respond = (url) => + json( + url === DISCOVERY_URI + ? discoveryDocument() + : jwksDocument([ + { kty: 'oct', kid: 'symmetric', k: 'c2VjcmV0' }, + { ...signingJwk, kid: 'encryption-only', use: 'enc' }, + signingJwk, + ]) + ); + await assert.rejects(() => getSigningKey(ISSUER, 'symmetric'), /not recognized/); + await assert.rejects(() => getSigningKey(ISSUER, 'encryption-only'), /not recognized/); + assert.strictEqual((await getSigningKey(ISSUER, 'key-1')).type, 'public'); + }); + + it('rejects a JWKS with no usable keys', async () => { + respond = (url) => + json(url === DISCOVERY_URI ? discoveryDocument() : jwksDocument([{ kty: 'oct', kid: 'x', k: 'c2VjcmV0' }])); + await assert.rejects(() => getSigningKey(ISSUER, 'key-1'), /no usable signing keys/); + }); + + it('rejects a non-JSON response', async () => { + respond = () => json('not json'); + await assert.rejects(() => getSigningKey(ISSUER, 'key-1'), /not valid JSON/); + }); + + it('rejects an error status', async () => { + respond = () => new Response('nope', { status: 503 }); + await assert.rejects(() => getSigningKey(ISSUER, 'key-1'), /responded 503/); + }); + + it('rejects an oversized response body', async () => { + respond = () => json('x'.repeat(1_100_000)); + await assert.rejects(() => getSigningKey(ISSUER, 'key-1'), /exceeds/); + }); + + it('rejects an oversized declared content-length without reading the body', async () => { + respond = () => + new Response('{}', { + status: 200, + headers: { 'content-type': 'application/json', 'content-length': '99999999' }, + }); + await assert.rejects(() => getSigningKey(ISSUER, 'key-1'), /exceeds/); + }); + + // The expired-cache half, reachable now that getSigningKey takes a clock. A cached set survives + // a failed refresh so a network blip does not break deploys — but only for STALE_KEY_GRACE_MS, + // because `fetchedAt` is advanced ONLY by a successful fetch. Without the ceiling, a key the + // issuer has pulled stays honored for the whole length of an outage rather than 24h. Both + // sides are asserted: dropping the bound leaves the grace+1 case passing on the served key, + // and dropping the grace entirely would take the grace−1 case with it. + describe('the stale-key grace ceiling', () => { + const GRACE_MS = 86_400_000; + + async function cacheThenFail() { + await getSigningKey(ISSUER, 'key-1'); // populate + respond = () => { + throw new Error('issuer is down'); + }; + } + + it('still serves a cached key just inside the grace window', async () => { + await cacheThenFail(); + const key = await getSigningKey(ISSUER, 'key-1', Date.now() + GRACE_MS - 1000); + assert.strictEqual(key.type, 'public', 'a blip must not break deploys inside the window'); + }); + + it('refuses a cached key once the grace window has passed', async () => { + await cacheThenFail(); + await assert.rejects( + () => getSigningKey(ISSUER, 'key-1', Date.now() + GRACE_MS + 1000), + 'a pulled key must not be honored indefinitely through an outage' + ); + }); + }); + + // An issuer whose keys we have NEVER cached is the worse half of the outage case: there is no + // stale key to fall back on, so without a backoff every request rides the full discovery and + // fetch timeout — and the exchange is unauthenticated, with the issuer chosen from an + // unverified JWT. Reachable without time travel; the expired-cache half above needs the clock. + it('stops re-fetching for an issuer whose keys have never been cached', async () => { + respond = () => { + throw new Error('issuer is down'); + }; + + await assert.rejects(() => getSigningKey(ISSUER, 'key-1')); + const afterFirst = requestLog.length; + assert.ok(afterFirst > 0, 'expected the first attempt to actually try'); + + await assert.rejects(() => getSigningKey(ISSUER, 'key-1')); + await assert.rejects(() => getSigningKey(ISSUER, 'key-1')); + + assert.strictEqual( + requestLog.length, + afterFirst, + 'a failed refresh must suppress further fetches for the backoff interval' + ); + }); + + // The backoff must not outlive the outage: a recovered issuer is picked up once the interval + // passes, and clearing the cache is the operator's way to force that immediately. + it('fetches again for a recovered issuer once the failure is cleared', async () => { + respond = () => { + throw new Error('issuer is down'); + }; + await assert.rejects(() => getSigningKey(ISSUER, 'key-1')); + + clearJwksCache(); + requestLog.length = 0; + respond = (url) => json(url === DISCOVERY_URI ? discoveryDocument() : jwksDocument([signingJwk])); + + const key = await getSigningKey(ISSUER, 'key-1'); + assert.strictEqual(key.type, 'public'); + assert.ok(requestLog.length > 0, 'expected the recovered issuer to be fetched again'); + }); + + it('requires a key id', async () => { + for (const kid of ['', undefined, null, 42]) { + await assert.rejects(() => getSigningKey(ISSUER, kid), /no key id/); + } + assert.deepStrictEqual(requestLog, [], 'expected no fetch for a token with no kid'); + }); + }); +}); diff --git a/unitTests/security/authn/oidc/providers/generic.test.js b/unitTests/security/authn/oidc/providers/generic.test.js new file mode 100644 index 0000000000..58bc1c8a49 --- /dev/null +++ b/unitTests/security/authn/oidc/providers/generic.test.js @@ -0,0 +1,73 @@ +'use strict'; + +const assert = require('node:assert'); +const { genericProfile } = require('#src/security/authn/oidc/providers/generic'); +const { profileForIssuer } = require('#src/security/authn/oidc/providers/index'); + +// Representative subjects from the workload-identity issuers this profile is meant to serve with no +// provider code at all. +const KUBERNETES_SUB = 'system:serviceaccount:prod:deployer'; +const SPIFFE_SUB = 'spiffe://example.org/ns/prod/sa/deployer'; + +describe('generic provider profile', () => { + it('is the fallback for an unregistered issuer', () => { + for (const issuer of ['https://kubernetes.default.svc', 'https://accounts.google.com', 'https://gitlab.com']) { + assert.strictEqual(profileForIssuer(issuer), genericProfile); + } + }); + + // sub is the one claim every OIDC issuer defines as identifying a single principal, so requiring + // it is what makes an unregistered issuer safe by default rather than permissive by default. + describe('assertPolicyIsSpecific', () => { + it('accepts a policy that pins sub', () => { + for (const sub of [KUBERNETES_SUB, SPIFFE_SUB, 'deployer@project.iam.gserviceaccount.com']) { + assert.doesNotThrow(() => genericProfile.assertPolicyIsSpecific({ sub })); + } + }); + + it('accepts sub alongside further constraints', () => { + assert.doesNotThrow(() => genericProfile.assertPolicyIsSpecific({ sub: KUBERNETES_SUB, namespace: 'prod' })); + }); + + it('rejects a policy that does not pin sub', () => { + assert.throws(() => genericProfile.assertPolicyIsSpecific({ namespace: 'prod' }), /must pin `sub`/); + }); + + it('rejects a policy pinning only claims that do not identify a principal', () => { + assert.throws( + () => genericProfile.assertPolicyIsSpecific({ aud: 'https://my-instance.harperdb.io:9925/' }), + /must pin `sub`/ + ); + }); + }); + + describe('normalizeClaims', () => { + it('derives nothing and does not mutate', () => { + const payload = { sub: KUBERNETES_SUB, namespace: 'prod' }; + const claims = genericProfile.normalizeClaims(payload); + assert.deepStrictEqual(claims, payload); + assert.notStrictEqual(claims, payload, 'should be a copy'); + }); + }); + + describe('assertAudienceIsSpecific', () => { + // No known shared default; the required sub pin already binds the policy to one principal. + it('accepts any audience', () => { + assert.doesNotThrow(() => genericProfile.assertAudienceIsSpecific('https://github.com/HarperFast')); + }); + }); + + describe('describePrincipal', () => { + it('is the subject', () => { + assert.strictEqual(genericProfile.describePrincipal({ sub: SPIFFE_SUB }), SPIFFE_SUB); + }); + + it('tolerates a missing subject', () => { + assert.strictEqual(typeof genericProfile.describePrincipal({}), 'string'); + }); + }); + + it('declares no match-time veto', () => { + assert.strictEqual(genericProfile.vetoClaims, undefined); + }); +}); diff --git a/unitTests/security/authn/oidc/providers/githubActions.test.js b/unitTests/security/authn/oidc/providers/githubActions.test.js new file mode 100644 index 0000000000..edeab2c9cc --- /dev/null +++ b/unitTests/security/authn/oidc/providers/githubActions.test.js @@ -0,0 +1,233 @@ +'use strict'; + +const assert = require('node:assert'); +const { githubActionsProfile, splitWorkflowPath } = require('#src/security/authn/oidc/providers/githubActions'); +const { profileForIssuer, genericProfile } = require('#src/security/authn/oidc/providers/index'); + +const WORKFLOW_REF = 'HarperFast/my-app/.github/workflows/deploy.yml@refs/heads/main'; + +const GITHUB_CLAIMS = Object.freeze({ + iss: 'https://token.actions.githubusercontent.com', + sub: 'repo:HarperFast/my-app:environment:production', + repository: 'HarperFast/my-app', + repository_id: '67890', + repository_owner: 'HarperFast', + repository_owner_id: '12345', + workflow_ref: WORKFLOW_REF, + job_workflow_ref: WORKFLOW_REF, + environment: 'production', + ref: 'refs/heads/main', + event_name: 'push', + run_id: '99', + actor: 'octocat', +}); + +const VALID_POLICY_CLAIMS = Object.freeze({ repository_id: '67890', workflow_ref: WORKFLOW_REF }); + +describe('githubActions provider profile', () => { + it('is the profile registered for the GitHub Actions issuer', () => { + assert.strictEqual(profileForIssuer('https://token.actions.githubusercontent.com'), githubActionsProfile); + }); + + it('does not claim other issuers', () => { + assert.strictEqual(profileForIssuer('https://gitlab.example.com'), genericProfile); + }); + + describe('splitWorkflowPath', () => { + it('strips the ref', () => { + assert.strictEqual(splitWorkflowPath(WORKFLOW_REF), 'HarperFast/my-app/.github/workflows/deploy.yml'); + }); + + it('splits on the ref qualifier, not on an @ inside the branch name', () => { + assert.strictEqual( + splitWorkflowPath('HarperFast/my-app/.github/workflows/deploy.yml@refs/heads/release@2'), + 'HarperFast/my-app/.github/workflows/deploy.yml' + ); + }); + + it('handles a tag ref', () => { + assert.strictEqual( + splitWorkflowPath('HarperFast/my-app/.github/workflows/release.yml@refs/tags/v1.2.3'), + 'HarperFast/my-app/.github/workflows/release.yml' + ); + }); + + it('returns undefined rather than guessing', () => { + for (const value of ['HarperFast/my-app/.github/workflows/deploy.yml', 'no-at-sign', undefined, null, 42]) { + assert.strictEqual(splitWorkflowPath(value), undefined); + } + }); + }); + + describe('normalizeClaims', () => { + it('derives workflow_path and job_workflow_path', () => { + const claims = githubActionsProfile.normalizeClaims(GITHUB_CLAIMS); + assert.strictEqual(claims.workflow_path, 'HarperFast/my-app/.github/workflows/deploy.yml'); + assert.strictEqual(claims.job_workflow_path, 'HarperFast/my-app/.github/workflows/deploy.yml'); + }); + + it('preserves the original claims and does not mutate the input', () => { + const payload = { ...GITHUB_CLAIMS }; + const claims = githubActionsProfile.normalizeClaims(payload); + assert.strictEqual(claims.repository_id, '67890'); + assert.strictEqual(claims.workflow_ref, WORKFLOW_REF); + assert.strictEqual(payload.workflow_path, undefined); + }); + + it('omits a derived claim when the reference is not ref-qualified', () => { + const claims = githubActionsProfile.normalizeClaims({ workflow_ref: 'owner/repo/.github/workflows/x.yml' }); + assert.strictEqual(claims.workflow_path, undefined); + }); + + // An issuer that one day emits workflow_path itself must win over our derivation. + it('does not displace a claim the token already carries', () => { + const claims = githubActionsProfile.normalizeClaims({ + workflow_ref: WORKFLOW_REF, + workflow_path: 'issuer-supplied', + }); + assert.strictEqual(claims.workflow_path, 'issuer-supplied'); + }); + }); + + describe('assertPolicyIsSpecific', () => { + it('accepts a repository pin plus a ref-qualified workflow pin', () => { + assert.doesNotThrow(() => githubActionsProfile.assertPolicyIsSpecific({ ...VALID_POLICY_CLAIMS })); + }); + + it('accepts a workflow path gated by an environment', () => { + assert.doesNotThrow(() => + githubActionsProfile.assertPolicyIsSpecific({ + repository_id: '67890', + workflow_path: 'HarperFast/my-app/.github/workflows/release.yml', + environment: 'production', + }) + ); + }); + + // repository_owner identifies an org, not a repository — it would admit every repo in the org. + it('rejects a policy with no repository pin', () => { + assert.throws( + () => + githubActionsProfile.assertPolicyIsSpecific({ repository_owner: 'HarperFast', workflow_ref: WORKFLOW_REF }), + /pin the repository/ + ); + }); + + it('rejects a policy with no workflow pin', () => { + assert.throws( + () => githubActionsProfile.assertPolicyIsSpecific({ repository_id: '67890', environment: 'production' }), + /pin the workflow/ + ); + }); + + // The npm-style "repository + workflow filename" policy: any branch that can be pushed can run + // the workflow and mint a token. + it('rejects a workflow pin with no ref gate', () => { + assert.throws( + () => + githubActionsProfile.assertPolicyIsSpecific({ + repository_id: '67890', + workflow_path: 'HarperFast/my-app/.github/workflows/deploy.yml', + }), + /gate the ref/ + ); + }); + + // job_workflow_ref names the reusable workflow that RAN, not the caller that invoked it. Its + // @ref suffix is the reusable workflow's own branch and is constant however it is called, so + // accepting it as a ref gate would admit any branch of any caller repository that references + // that reusable workflow — exactly the hole the ref gate exists to close. + it('does not accept job_workflow_ref as a ref gate', () => { + assert.throws( + () => + githubActionsProfile.assertPolicyIsSpecific({ + repository_id: '67890', + job_workflow_ref: 'HarperFast/shared/.github/workflows/deploy.yml@refs/heads/main', + }), + /gate the ref/ + ); + }); + + // It remains a valid workflow pin — only the ref row rejects it. + it('accepts job_workflow_ref as a workflow pin alongside a real ref gate', () => { + assert.doesNotThrow(() => + githubActionsProfile.assertPolicyIsSpecific({ + repository_id: '67890', + job_workflow_ref: 'HarperFast/shared/.github/workflows/deploy.yml@refs/heads/main', + environment: 'production', + }) + ); + }); + + it('does not accept ref_type alone as a ref gate', () => { + assert.throws( + () => + githubActionsProfile.assertPolicyIsSpecific({ + repository_id: '67890', + workflow_path: 'HarperFast/my-app/.github/workflows/release.yml', + ref_type: 'tag', + }), + /gate the ref/ + ); + }); + + // GitHub's sub varies by trigger and changed format for repos created after 2026-07-15, so it + // is not one of the accepted pins — unlike the generic profile, which requires it. + it('does not accept sub as a repository pin', () => { + assert.throws( + () => githubActionsProfile.assertPolicyIsSpecific({ sub: GITHUB_CLAIMS.sub, workflow_ref: WORKFLOW_REF }), + /pin the repository/ + ); + }); + }); + + describe('assertAudienceIsSpecific', () => { + it('rejects the shared org-wide default', () => { + for (const audience of ['https://github.com/HarperFast', 'https://github.com/HarperFast/']) { + assert.throws(() => githubActionsProfile.assertAudienceIsSpecific(audience), /must identify this instance/); + } + }); + + it('accepts an instance URL', () => { + assert.doesNotThrow(() => githubActionsProfile.assertAudienceIsSpecific('https://my-instance.harperdb.io:9925/')); + }); + }); + + // A pull_request_target run executes the base repo's workflow with its secrets while a fork + // controls the code, so it is denied unless a policy opts in by constraining event_name. + describe('vetoClaims', () => { + it('denies pull_request_target when the policy does not constrain event_name', () => { + const reason = githubActionsProfile.vetoClaims( + { ...GITHUB_CLAIMS, event_name: 'pull_request_target' }, + VALID_POLICY_CLAIMS + ); + assert.ok(reason); + assert.match(reason, /pull_request_target/); + }); + + it('allows pull_request_target when the policy opts in', () => { + const reason = githubActionsProfile.vetoClaims( + { ...GITHUB_CLAIMS, event_name: 'pull_request_target' }, + { ...VALID_POLICY_CLAIMS, event_name: 'pull_request_target' } + ); + assert.strictEqual(reason, undefined); + }); + + it('leaves ordinary events alone', () => { + assert.strictEqual(githubActionsProfile.vetoClaims(GITHUB_CLAIMS, VALID_POLICY_CLAIMS), undefined); + }); + }); + + describe('describePrincipal', () => { + it('names the repository, workflow, environment, run, and actor', () => { + const described = githubActionsProfile.describePrincipal(GITHUB_CLAIMS); + for (const fragment of ['HarperFast/my-app', WORKFLOW_REF, 'environment=production', 'run=99', 'actor=octocat']) { + assert.ok(described.includes(fragment), `expected "${fragment}" in "${described}"`); + } + }); + + it('tolerates a sparse token', () => { + assert.strictEqual(typeof githubActionsProfile.describePrincipal({}), 'string'); + }); + }); +}); diff --git a/unitTests/security/authn/oidc/tokenExchange.test.js b/unitTests/security/authn/oidc/tokenExchange.test.js new file mode 100644 index 0000000000..2f8b7d1b46 --- /dev/null +++ b/unitTests/security/authn/oidc/tokenExchange.test.js @@ -0,0 +1,598 @@ +'use strict'; + +// End-to-end tests for exchange_oidc_token: a real signed identity token, served through a real +// JWKS fetch, verified and matched against real trust policies, exchanged for a real operation +// token. Only two things are displaced — the network (a fetch responder) and the two system tables +// (Map-backed mocks on databases.system). + +const assert = require('node:assert'); +const testUtils = require('../../../testUtils.js'); +testUtils.preTestPrep(); + +const jwt = require('jsonwebtoken'); +const { generateKeyPairSync, createPublicKey } = require('node:crypto'); +const { exchangeOidcToken, clearTokenUseTableCache } = require('#src/security/authn/oidc/tokenExchange'); +const { addOidcTrust } = require('#src/security/authn/oidc/trustPolicyOperations'); +const { clearJwksCache } = require('#src/security/authn/oidc/jwks'); +const { validateOperationToken, clearJWTRSAKeysCache, decodeJWT } = require('#src/security/tokenAuthentication'); +const { databases } = require('#src/resources/databases'); +const { setUsersWithRolesCache } = require('#src/security/user'); +const terms = require('#src/utility/hdbTerms'); + +const TRUST_TABLE = terms.SYSTEM_TABLE_NAMES.OIDC_TRUST_TABLE_NAME; +const TOKEN_USE_TABLE = 'hdb_oidc_token_use'; +const ISSUER = 'https://token.actions.githubusercontent.com'; +// An issuer with no registered provider profile — served by the same fake JWKS, so the generic +// profile is exercised end-to-end with zero provider code. +const GENERIC_ISSUER = 'https://kubernetes.default.svc'; +const AUDIENCE = 'https://my-instance.harperdb.io:9925/'; +const WORKFLOW_REF = 'HarperFast/my-app/.github/workflows/deploy.yml@refs/heads/main'; +const OIDC_KID = 'gh-signing-key'; + +let issuerPrivateKey; +let signingJwk; + +function installMockTable(name, primaryKey) { + const rows = new Map(); + const mock = { + rows, + async get(id) { + return rows.get(id); + }, + async put(row) { + rows.set(row[primaryKey], row); + }, + async delete(id) { + return rows.delete(id); + }, + search() { + return (async function* () { + yield* rows.values(); + })(); + }, + }; + if (!databases.system) databases.system = {}; + const prior = databases.system[name]; + databases.system[name] = mock; + return { mock, restore: () => (databases.system[name] = prior) }; +} + +/** + * The replay table is reached through an unconditional `table()` call, not a lookup — it has to be, + * because the systemSchema bootstrap declares only the primary key and `table()` is what layers the + * expiresAt TTL on top. So intercept the factory rather than seeding `databases.system`, which that + * call would otherwise sail straight past. + */ +function installMockTableFactory(name, mock) { + const databasesModule = require('#src/resources/databases'); + const realTable = databasesModule.table; + databasesModule.table = (definition) => (definition?.table === name ? mock : realTable(definition)); + return () => (databasesModule.table = realTable); +} + +function seedUsers() { + const users = new Map(); + users.set('ci-deploy', { + username: 'ci-deploy', + active: true, + role: { role: 'deployer', permission: { super_user: false } }, + }); + users.set('admin', { username: 'admin', active: true, role: { role: 'su', permission: { super_user: true } } }); + users.set('retired', { username: 'retired', active: false, role: { role: 'deployer', permission: {} } }); + return setUsersWithRolesCache(users); +} + +const asAdmin = (body) => ({ + operation: 'add_oidc_trust', + hdb_user: { username: 'admin', role: { permission: { super_user: true } } }, + ...body, +}); + +describe('exchangeOidcToken', () => { + let removeJwtKeys; + let trustTable; + let useTable; + let restoreTableFactory; + let realFetch; + let tokenCounter = 0; + + before(() => { + // Shared helper so the keys are removed afterwards — they live in a directory another suite + // asserts is empty (see testUtils.installTestJwtKeys). + removeJwtKeys = testUtils.installTestJwtKeys(); + clearJWTRSAKeysCache(); + const pair = generateKeyPairSync('rsa', { + modulusLength: 2048, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }); + issuerPrivateKey = pair.privateKey; + signingJwk = { + ...createPublicKey(pair.publicKey).export({ format: 'jwk' }), + kid: OIDC_KID, + use: 'sig', + alg: 'RS256', + }; + }); + + beforeEach(async () => { + clearJwksCache(); + trustTable = installMockTable(TRUST_TABLE, 'id'); + useTable = installMockTable(TOKEN_USE_TABLE, 'id'); + restoreTableFactory = installMockTableFactory(TOKEN_USE_TABLE, useTable.mock); + clearTokenUseTableCache(); // or every test after the first reuses the first one's mock + await seedUsers(); + realFetch = globalThis.fetch; + // Serves discovery for any issuer asked about, and one shared key set, so a second issuer needs + // no extra plumbing. + globalThis.fetch = async (url) => { + const asString = String(url); + const discoveryFor = [ISSUER, GENERIC_ISSUER].find( + (issuer) => asString === issuer + '/.well-known/openid-configuration' + ); + const body = discoveryFor + ? { issuer: discoveryFor, jwks_uri: discoveryFor + '/.well-known/jwks' } + : { keys: [signingJwk] }; + return new Response(JSON.stringify(body), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }; + }); + + afterEach(() => { + globalThis.fetch = realFetch; + trustTable.restore(); + useTable.restore(); + restoreTableFactory(); + }); + + after(() => { + removeJwtKeys(); + clearJWTRSAKeysCache(); + }); + + function identityToken(overrides = {}) { + const now = Math.floor(Date.now() / 1000); + return jwt.sign( + { + iss: ISSUER, + aud: AUDIENCE, + sub: 'repo:HarperFast/my-app:environment:production', + jti: `token-${++tokenCounter}`, + iat: now, + exp: now + 300, + repository: 'HarperFast/my-app', + repository_id: '67890', + repository_owner_id: '12345', + workflow_ref: WORKFLOW_REF, + environment: 'production', + ref: 'refs/heads/main', + event_name: 'push', + runner_environment: 'github-hosted', + run_id: '99', + actor: 'octocat', + ...overrides, + }, + issuerPrivateKey, + { algorithm: 'RS256', header: { alg: 'RS256', kid: OIDC_KID } } + ); + } + + function addPolicy(overrides = {}) { + return addOidcTrust( + asAdmin({ + id: 'my-app-prod', + issuer: ISSUER, + audience: AUDIENCE, + claims: { repository_id: '67890', workflow_ref: WORKFLOW_REF }, + user: 'ci-deploy', + ...overrides, + }) + ); + } + + async function assertRejected(promise) { + await assert.rejects(promise, (error) => { + assert.strictEqual(error.statusCode, 401); + // One message for every failure: a caller told which check failed can enumerate a policy. + assert.strictEqual(error.message, 'Identity token was rejected'); + return true; + }); + } + + it('exchanges a matching token for a usable operation token', async () => { + await addPolicy(); + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() }); + + assert.strictEqual(result.username, 'ci-deploy'); + assert.strictEqual(result.policy, 'my-app-prod'); + assert.strictEqual(result.expires_in, 3600); + + const user = await validateOperationToken(result.operation_token); + assert.strictEqual(user.username, 'ci-deploy'); + }); + + // A stored row that add_oidc_trust would reject (repository pinned, but no workflow/ref gate) — as + // could arrive via replication from an older node or a restored backup — must be ignored at + // exchange, not honored. Seeded directly into the store to bypass add_oidc_trust's validation. + // A row can reach the table without passing add_oidc_trust — replication from a node predating a + // check, a restored backup, a direct system-table write. These two shapes fail OPEN if the exchange + // merely normalizes them instead of refusing the row, which is why validation is shared rather than + // duplicated. Written straight to the store, deliberately bypassing the add handler. + function storePolicyDirectly(overrides) { + trustTable.mock.rows.set('my-app-prod', { + id: 'my-app-prod', + issuer: ISSUER, + audience: AUDIENCE, + claims: { repository_id: '67890', workflow_ref: WORKFLOW_REF }, + user: 'ci-deploy', + enabled: true, + ...overrides, + }); + } + + // operations as a scalar: hasOperationScope tests Array.isArray, so the scope would be dropped and + // the token minted UNSCOPED with the policy user's whole role. A malformed narrowing must never + // widen — so the row is refused rather than honored without its scope. + // + // A NUMBER, not a string. A string scalar is refused even without the Array.isArray guard, because + // validateOperations iterates it character by character and reports 'd' as an unknown operation — + // the right outcome via the wrong check, which would let the guard be deleted with tests green. A + // number isolates it: `for (const op of 42)` throws TypeError, which would escape readPolicies and + // turn one malformed row into a 500 on every exchange for that issuer instead of a uniform 401. + // assertRejected requires exactly that 401, so it fails if the guard goes. + it('refuses a stored policy whose operations is a non-iterable scalar', async () => { + storePolicyDirectly({ operations: 42 }); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() })); + }); + + it('refuses a stored policy whose operations is a string rather than an array', async () => { + storePolicyDirectly({ operations: 'deploy_component' }); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() })); + }); + + it('refuses a stored policy naming an operation that does not exist', async () => { + storePolicyDirectly({ operations: ['not_a_harper_operation'] }); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() })); + }); + + // enabled as the string 'false': `row.enabled !== false` is true for it, so a policy an operator + // meant to disable would keep minting tokens. + it('refuses a stored policy whose enabled is a string rather than a boolean', async () => { + storePolicyDirectly({ enabled: 'false' }); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() })); + }); + + // The constraint list MATCHES on its string entry, so matchTrustPolicyClaims would admit this row + // — only validateClaimConstraintShape refuses it. A wholly-bad shape (`{nested: 'object'}`) is + // refused downstream by the matcher too, so it cannot tell the new validator from the pre-existing + // one; this can. A non-string entry in an otherwise-matching list is how a constraint gets + // silently wider than it reads. + it('refuses a stored policy whose claim list has a non-string entry, even though it matches', async () => { + storePolicyDirectly({ claims: { repository_id: ['67890', 42], workflow_ref: WORKFLOW_REF } }); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() })); + }); + + it('refuses a stored policy whose claims are not a usable constraint shape', async () => { + storePolicyDirectly({ claims: { repository_id: { nested: 'object' } } }); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() })); + }); + + // The control: the same direct-store path with a well-formed row still authenticates, so the four + // refusals above are the validator working rather than the helper failing to store anything. + it('still exchanges a well-formed directly-stored policy', async () => { + storePolicyDirectly({ operations: ['deploy_component'] }); + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() }); + assert.strictEqual(result.username, 'ci-deploy'); + }); + + it('ignores an under-specified stored policy that bypassed write-time validation', async () => { + trustTable.mock.rows.set('smuggled', { + id: 'smuggled', + issuer: ISSUER, + audience: AUDIENCE, + claims: { repository_id: '67890' }, // no workflow pin, no ref gate — add_oidc_trust rejects this + user: 'ci-deploy', + enabled: true, + }); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() })); + }); + + // The whole point of trusted publishing: CI ends up holding nothing durable, and the user's + // existing refresh credential is not rotated out from under whoever holds it (#2018). + it('mints no refresh token', async () => { + await addPolicy(); + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() }); + assert.strictEqual(result.refresh_token, undefined); + assert.ok(decodeJWT(result.operation_token).exp, 'expected a bounded lifetime'); + }); + + it('refuses to spend the same token twice', async () => { + await addPolicy(); + const token = identityToken(); + await exchangeOidcToken({ operation: 'exchange_oidc_token', token }); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token })); + }); + + // The replay key must be the SIGNED INPUT, not the token string. Base64url decoding ignores the + // surplus low bits of the final character, so an RS256 signature has 16 distinct spellings that + // decode to identical bytes — every one passes jwt.verify. Keying on the whole token gave each a + // different fingerprint, so one leaked identity token bought 16 operation tokens. + it('refuses a re-spelled signature that decodes to the same bytes', async () => { + await addPolicy(); + const token = identityToken(); + await exchangeOidcToken({ operation: 'exchange_oidc_token', token }); + + const [header, payload, signature] = token.split('.'); + const signatureBytes = Buffer.from(signature, 'base64url'); + const alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_'; + const twins = []; + for (const character of alphabet) { + const candidate = signature.slice(0, -1) + character; + if (candidate === signature) continue; + if (Buffer.from(candidate, 'base64url').equals(signatureBytes)) twins.push(candidate); + } + assert.ok(twins.length > 0, 'expected at least one surplus-bit spelling of the signature'); + + for (const twin of twins) { + await assertRejected( + exchangeOidcToken({ operation: 'exchange_oidc_token', token: `${header}.${payload}.${twin}` }) + ); + } + }); + + it('records the spent token against the policy, expiring with the token', async () => { + await addPolicy(); + const token = identityToken(); + await exchangeOidcToken({ operation: 'exchange_oidc_token', token }); + + const [record] = [...useTable.mock.rows.values()]; + assert.strictEqual(record.policy_id, 'my-app-prod'); + const tokenExpiryMs = decodeJWT(token).exp * 1000; + assert.ok(record.expiresAt > tokenExpiryMs, 'record must outlive the token it guards'); + }); + + it('rejects a token minted for a different audience', async () => { + await addPolicy(); + await assertRejected( + exchangeOidcToken({ + operation: 'exchange_oidc_token', + token: identityToken({ aud: 'https://github.com/HarperFast' }), + }) + ); + }); + + it('rejects a run from a branch the policy does not name', async () => { + await addPolicy(); + await assertRejected( + exchangeOidcToken({ + operation: 'exchange_oidc_token', + token: identityToken({ + workflow_ref: 'HarperFast/my-app/.github/workflows/deploy.yml@refs/heads/attacker', + ref: 'refs/heads/attacker', + }), + }) + ); + }); + + it('rejects a run from a different repository', async () => { + await addPolicy(); + await assertRejected( + exchangeOidcToken({ + operation: 'exchange_oidc_token', + token: identityToken({ repository_id: '11111', repository: 'attacker/evil' }), + }) + ); + }); + + it('rejects when no policy exists for the issuer', async () => { + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() })); + }); + + it('ignores a disabled policy', async () => { + await addPolicy({ enabled: false }); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() })); + }); + + it('selects the first matching policy by id when several match', async () => { + await addPolicy({ id: 'zulu', user: 'admin' }); + await addPolicy({ id: 'alpha', user: 'ci-deploy' }); + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() }); + assert.strictEqual(result.policy, 'alpha'); + assert.strictEqual(result.username, 'ci-deploy'); + }); + + it('rejects a policy naming a user that no longer exists, without spending the token', async () => { + await addPolicy(); + await setUsersWithRolesCache(new Map()); + const token = identityToken(); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token })); + assert.strictEqual(useTable.mock.rows.size, 0, 'a token the runner cannot re-mint must not be burned'); + }); + + // add_oidc_trust refuses an inactive user outright, so the case that reaches here is a user + // deactivated after the policy was written — deactivating a user must stop its workflows. + it('rejects a policy whose user has since been deactivated', async () => { + await addPolicy(); + const users = new Map(); + users.set('ci-deploy', { username: 'ci-deploy', active: false, role: { role: 'deployer', permission: {} } }); + await setUsersWithRolesCache(users); + + const token = identityToken(); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token })); + assert.strictEqual(useTable.mock.rows.size, 0, 'a token the runner cannot re-mint must not be burned'); + }); + + it('rejects an expired token', async () => { + await addPolicy(); + const past = Math.floor(Date.now() / 1000) - 7200; + await assertRejected( + exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken({ iat: past, exp: past + 300 }) }) + ); + }); + + it('rejects a token signed by a key the issuer does not publish', async () => { + await addPolicy(); + const { privateKey } = generateKeyPairSync('rsa', { + modulusLength: 2048, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }); + const forged = jwt.sign( + { iss: ISSUER, aud: AUDIENCE, jti: 'forged', exp: Math.floor(Date.now() / 1000) + 300 }, + privateKey, + { + algorithm: 'RS256', + header: { alg: 'RS256', kid: OIDC_KID }, + } + ); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token: forged })); + }); + + // Replay is keyed on a hash of the token, not on `jti`, so an issuer that omits it is still + // protected — which is the point of the change: Azure uses `uti`, others omit it entirely. + it('refuses to spend a token twice even when it carries no jti', async () => { + await addPolicy(); + const { jti: _jti, ...withoutJti } = JSON.parse( + Buffer.from(identityToken().split('.')[1], 'base64url').toString('utf8') + ); + const token = jwt.sign(withoutJti, issuerPrivateKey, { + algorithm: 'RS256', + header: { alg: 'RS256', kid: OIDC_KID }, + }); + + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token }); + assert.strictEqual(result.username, 'ci-deploy'); + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token })); + }); + + it('stores only a hash, never the token itself', async () => { + await addPolicy(); + const token = identityToken(); + await exchangeOidcToken({ operation: 'exchange_oidc_token', token }); + + const [record] = [...useTable.mock.rows.values()]; + assert.ok(!JSON.stringify(record).includes(token), 'the raw token must not be stored'); + assert.ok(!/eyJ/.test(JSON.stringify(record)), 'nothing JWT-shaped should be stored'); + }); + + // A pull_request_target run executes the base repo's workflow with its secrets while a fork + // controls the code. Denied unless the policy opts in by constraining event_name. + it('denies a pull_request_target run when the policy does not constrain event_name', async () => { + await addPolicy(); + await assertRejected( + exchangeOidcToken({ + operation: 'exchange_oidc_token', + token: identityToken({ event_name: 'pull_request_target' }), + }) + ); + }); + + it('allows a pull_request_target run when the policy opts in', async () => { + await addPolicy({ + claims: { repository_id: '67890', workflow_ref: WORKFLOW_REF, event_name: 'pull_request_target' }, + }); + const result = await exchangeOidcToken({ + operation: 'exchange_oidc_token', + token: identityToken({ event_name: 'pull_request_target' }), + }); + assert.strictEqual(result.username, 'ci-deploy'); + }); + + // The generic profile with no provider code: a Kubernetes-shaped token, a policy pinning `sub`. + describe('an issuer with no registered profile', () => { + function serviceAccountToken(overrides = {}) { + const now = Math.floor(Date.now() / 1000); + return jwt.sign( + { + iss: GENERIC_ISSUER, + aud: AUDIENCE, + sub: 'system:serviceaccount:prod:deployer', + iat: now, + exp: now + 300, + ...overrides, + }, + issuerPrivateKey, + { algorithm: 'RS256', header: { alg: 'RS256', kid: OIDC_KID } } + ); + } + + it('exchanges a token whose sub the policy pins', async () => { + await addOidcTrust( + asAdmin({ + id: 'k8s-prod', + issuer: GENERIC_ISSUER, + audience: AUDIENCE, + claims: { sub: 'system:serviceaccount:prod:deployer' }, + user: 'ci-deploy', + }) + ); + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token: serviceAccountToken() }); + assert.strictEqual(result.username, 'ci-deploy'); + assert.strictEqual(result.policy, 'k8s-prod'); + }); + + it('rejects a different service account', async () => { + await addOidcTrust( + asAdmin({ + id: 'k8s-prod', + issuer: GENERIC_ISSUER, + audience: AUDIENCE, + claims: { sub: 'system:serviceaccount:prod:deployer' }, + user: 'ci-deploy', + }) + ); + await assertRejected( + exchangeOidcToken({ + operation: 'exchange_oidc_token', + token: serviceAccountToken({ sub: 'system:serviceaccount:prod:attacker' }), + }) + ); + }); + + it('refuses to store a policy that does not pin sub', async () => { + await assert.rejects( + () => + addOidcTrust( + asAdmin({ + id: 'k8s-loose', + issuer: GENERIC_ISSUER, + audience: AUDIENCE, + claims: { namespace: 'prod' }, + user: 'ci-deploy', + }) + ), + /must pin `sub`/ + ); + }); + }); + + // End to end: the policy's scope reaches the minted token, so verifyPerms can narrow on it. + it('carries the policy operation scope into the minted token', async () => { + await addPolicy({ operations: ['deploy_component'] }); + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() }); + + const payload = JSON.parse(Buffer.from(result.operation_token.split('.')[1], 'base64url').toString('utf8')); + assert.deepStrictEqual(payload.operations, ['deploy_component']); + assert.strictEqual(payload.username, 'ci-deploy'); + }); + + it('omits the claim entirely when the policy does not scope', async () => { + await addPolicy(); + const result = await exchangeOidcToken({ operation: 'exchange_oidc_token', token: identityToken() }); + + const payload = JSON.parse(Buffer.from(result.operation_token.split('.')[1], 'base64url').toString('utf8')); + assert.strictEqual(payload.operations, undefined, 'an unscoped token must look exactly as it did before'); + }); + + it('rejects malformed input', async () => { + await addPolicy(); + for (const token of ['not-a-jwt', 'a.b.c']) { + await assertRejected(exchangeOidcToken({ operation: 'exchange_oidc_token', token })); + } + for (const token of ['', undefined, 42, { nested: true }]) { + await assert.rejects(() => exchangeOidcToken({ operation: 'exchange_oidc_token', token })); + } + }); +}); diff --git a/unitTests/security/authn/oidc/trustPolicyOperations.test.js b/unitTests/security/authn/oidc/trustPolicyOperations.test.js new file mode 100644 index 0000000000..a1542365b5 --- /dev/null +++ b/unitTests/security/authn/oidc/trustPolicyOperations.test.js @@ -0,0 +1,419 @@ +'use strict'; + +// Op-flow tests for the hdb_oidc_trust operations, following the secretOperations.test.js pattern: +// a Map-backed mock table on databases.system plus a seeded users cache, so the real handlers run +// without stubs. + +const assert = require('node:assert'); +const testUtils = require('../../../testUtils.js'); +testUtils.preTestPrep(); + +const { + addOidcTrust, + listOidcTrust, + dropOidcTrust, + loadEnabledPolicies, +} = require('#src/security/authn/oidc/trustPolicyOperations'); +const { databases } = require('#src/resources/databases'); +const { setUsersWithRolesCache } = require('#src/security/user'); +const terms = require('#src/utility/hdbTerms'); +const opAuth = require('#src/utility/operation_authorization'); + +const OIDC_TRUST_TABLE = terms.SYSTEM_TABLE_NAMES.OIDC_TRUST_TABLE_NAME; +const ISSUER = 'https://token.actions.githubusercontent.com'; +const AUDIENCE = 'https://my-instance.harperdb.io:9925/'; + +const VALID_CLAIMS = { + repository_id: '67890', + workflow_ref: 'HarperFast/my-app/.github/workflows/deploy.yml@refs/heads/main', +}; + +function installMockTable() { + const rows = new Map(); + const mock = { + rows, + async get(id) { + return rows.get(id); + }, + async put(row) { + rows.set(row.id, row); + }, + async delete(id) { + return rows.delete(id); + }, + search() { + return (async function* () { + yield* rows.values(); + })(); + }, + }; + if (!databases.system) databases.system = {}; + const prior = databases.system[OIDC_TRUST_TABLE]; + databases.system[OIDC_TRUST_TABLE] = mock; + return { + mock, + restore() { + if (databases.system) databases.system[OIDC_TRUST_TABLE] = prior; + }, + }; +} + +function seedUsers() { + const users = new Map(); + users.set('ci-deploy', { + username: 'ci-deploy', + active: true, + role: { role: 'deployer', permission: { super_user: false } }, + }); + users.set('admin', { username: 'admin', active: true, role: { role: 'su', permission: { super_user: true } } }); + users.set('retired', { username: 'retired', active: false, role: { role: 'deployer', permission: {} } }); + return setUsersWithRolesCache(users); +} + +const su = (op, body = {}) => ({ + operation: op, + hdb_user: { username: 'admin', role: { permission: { super_user: true } } }, + ...body, +}); +// A role whose `operations` allowlist names the op: the in-handler check must still refuse. +const delegated = (op, body = {}) => ({ + operation: op, + hdb_user: { username: 'joe', role: { permission: { super_user: false, operations: [op] } } }, + ...body, +}); + +function validPolicy(overrides = {}) { + return { + id: 'my-app-prod', + issuer: ISSUER, + audience: AUDIENCE, + claims: { ...VALID_CLAIMS }, + user: 'ci-deploy', + ...overrides, + }; +} + +describe('oidc trustPolicyOperations', () => { + let installed; + + beforeEach(async () => { + installed = installMockTable(); + await seedUsers(); + }); + + afterEach(() => { + installed.restore(); + }); + + describe('super_user enforcement (in-handler, allowlist-proof)', () => { + const cases = [ + ['add_oidc_trust', addOidcTrust, validPolicy()], + ['list_oidc_trust', listOidcTrust, {}], + ['drop_oidc_trust', dropOidcTrust, { id: 'my-app-prod' }], + ]; + for (const [name, handler, body] of cases) { + it(`${name} refuses a non-super_user even when the role allowlists it`, async () => { + await assert.rejects(() => handler(delegated(name, body)), /restricted to super_user/); + }); + } + }); + + describe('addOidcTrust', () => { + it('stores a policy', async () => { + const result = await addOidcTrust(su('add_oidc_trust', validPolicy())); + assert.match(result.message, /my-app-prod/); + assert.strictEqual(result.warning, undefined); + + const stored = installed.mock.rows.get('my-app-prod'); + assert.strictEqual(stored.issuer, ISSUER); + assert.strictEqual(stored.user, 'ci-deploy'); + assert.strictEqual(stored.enabled, true); + assert.strictEqual(stored.updated_by, 'admin'); + assert.deepStrictEqual(stored.claims, VALID_CLAIMS); + }); + + it('normalizes the issuer so one issuer is one spelling', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ issuer: ISSUER + '/' }))); + assert.strictEqual(installed.mock.rows.get('my-app-prod').issuer, ISSUER); + }); + + it('replaces rather than merges, so a narrowed claim set actually narrows', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ claims: { ...VALID_CLAIMS, environment: 'staging' } }))); + await addOidcTrust(su('add_oidc_trust', validPolicy())); + assert.deepStrictEqual(installed.mock.rows.get('my-app-prod').claims, VALID_CLAIMS); + }); + + // The default GitHub audience is shared by every repo under an owner; accepting it would make a + // token minted by any of them valid here. + it('rejects an issuer default audience', async () => { + for (const audience of ['https://github.com/HarperFast', 'https://github.com/HarperFast/']) { + await assert.rejects( + () => addOidcTrust(su('add_oidc_trust', validPolicy({ audience }))), + /must identify this instance/ + ); + } + }); + + // The audience is compared byte-for-byte at verification time against what the CLI requested, + // and the CLI requests normalizeTarget(target) — port and trailing slash included. A policy + // written the natural way would therefore never match, and the exchange says only "rejected", + // so the operator learns nothing. It has to fail here, at write time, instead. + // `enabled` is a revocation control, so it has to fail closed. Joi coerces by default and + // validateBySchema keeps only `result.error`, discarding the converted value — so without + // `.strict()` the string "false" validated cleanly, survived as a string, and `!== false` read + // it as enabled. An operator disabling a policy this way would get no error and a policy that + // kept minting tokens. + it('rejects a non-boolean enabled rather than coercing it', async () => { + for (const enabled of ['false', 'true', 0, 1]) { + await assert.rejects( + () => addOidcTrust(su('add_oidc_trust', validPolicy({ id: 'coerce-me', enabled }))), + `expected ${JSON.stringify(enabled)} to be refused rather than coerced` + ); + } + }); + + it('stores a genuinely disabled policy as disabled', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ id: 'off', enabled: false }))); + assert.strictEqual(installed.mock.rows.get('off').enabled, false); + }); + + it('rejects an audience missing the port or the trailing slash', async () => { + for (const audience of [ + 'https://my-instance.harperdb.io', + 'https://my-instance.harperdb.io/', + 'https://my-instance.harperdb.io:9925', + ]) { + await assert.rejects( + () => addOidcTrust(su('add_oidc_trust', validPolicy({ audience }))), + /byte-for-byte/, + `expected '${audience}' to be refused as non-canonical` + ); + } + }); + + // Pins this check to the CLI's actual normalization rather than to my reading of it: whatever + // normalizeTarget produces must be accepted here, or the two drift and the feature breaks in + // the one place nobody is watching. + it('accepts every shape normalizeTarget produces', async () => { + const { normalizeTarget } = require('#src/bin/cliCredentials'); + for (const raw of [ + 'my-instance.harperdb.io', + 'https://my-instance.harperdb.io', + 'https://my-instance.harperdb.io:443', + 'http://localhost:9925', + ]) { + const audience = normalizeTarget(raw); + await assert.doesNotReject( + () => addOidcTrust(su('add_oidc_trust', validPolicy({ audience }))), + `normalizeTarget('${raw}') => '${audience}' must be a writable audience` + ); + } + }); + + it('rejects a non-https issuer', async () => { + await assert.rejects( + () => addOidcTrust(su('add_oidc_trust', validPolicy({ issuer: 'http://token.actions.githubusercontent.com' }))), + /https/ + ); + }); + + // Delegated to validateTrustPolicyClaims, which has its own coverage; this asserts the handler + // actually calls it rather than storing whatever it is given. + it('rejects an over-broad claim set', async () => { + await assert.rejects( + () => addOidcTrust(su('add_oidc_trust', validPolicy({ claims: { repository_id: '67890' } }))), + /pin the workflow/ + ); + assert.strictEqual(installed.mock.rows.size, 0, 'expected nothing stored'); + }); + + it('rejects a policy naming a user that does not exist', async () => { + await assert.rejects( + () => addOidcTrust(su('add_oidc_trust', validPolicy({ user: 'ghost' }))), + /No such user 'ghost'/ + ); + }); + + it('rejects a policy naming an inactive user', async () => { + await assert.rejects(() => addOidcTrust(su('add_oidc_trust', validPolicy({ user: 'retired' }))), /inactive/); + }); + + it('warns when the policy hands a workflow super_user', async () => { + const result = await addOidcTrust(su('add_oidc_trust', validPolicy({ user: 'admin' }))); + assert.match(result.warning, /super_user/); + // Still stored — an admin may mean it; it just should not be silent. + assert.strictEqual(installed.mock.rows.get('my-app-prod').user, 'admin'); + }); + + it('stores an operation scope', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ operations: ['deploy_component'] }))); + assert.deepStrictEqual(installed.mock.rows.get('my-app-prod').operations, ['deploy_component']); + }); + + it('accepts an operation group', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ operations: ['read_only'] }))); + assert.deepStrictEqual(installed.mock.rows.get('my-app-prod').operations, ['read_only']); + }); + + // A typo would otherwise fail closed at request time, in CI, with nothing to point at. + it('rejects an operation name that is not a Harper operation', async () => { + await assert.rejects( + () => addOidcTrust(su('add_oidc_trust', validPolicy({ operations: ['deploy_compnent'] }))), + /not a Harper operation/ + ); + assert.strictEqual(installed.mock.rows.size, 0, 'expected nothing stored'); + }); + + // Delegating to validateOperations rather than checking OPERATIONS_ENUM locally means an + // operation registered in THIS process is accepted. That is the seam, not a promise about + // component-registered operations: the registry is process-local, `add_oidc_trust` runs on the + // main thread, and `server.registerOperation` runs in a worker whose OPERATION_REGISTERED + // announcement carries only name→thread routing, never grantability. So a component's + // operation is NOT recognized here in production, and this same-thread test cannot show that + // — it is asserting the delegation, not the topology. A policy naming such an operation is + // rejected, which fails closed; add_role, alter_role, and impersonation validation share the + // gap, so the fix belongs to that bridge rather than to a workaround here. + it('accepts an operation registered in this process', async () => { + const dynamicOp = 'test_dynamic_scope_op'; + opAuth.registerOperationPermission(dynamicOp, { requiresSu: true }); + try { + await addOidcTrust(su('add_oidc_trust', validPolicy({ operations: [dynamicOp] }))); + assert.deepStrictEqual(installed.mock.rows.get('my-app-prod').operations, [dynamicOp]); + } finally { + opAuth.unregisterOperationPermission(dynamicOp); + } + }); + + it('leaves operations null when the policy does not scope', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy())); + assert.strictEqual(installed.mock.rows.get('my-app-prod').operations, null); + }); + + it('rejects a malformed id', async () => { + for (const id of ['', 'has spaces', 'has/slash', 'x'.repeat(129)]) { + await assert.rejects(() => addOidcTrust(su('add_oidc_trust', validPolicy({ id })))); + } + }); + }); + + // A row the exchange refuses must not list as healthy. The failure mode this guards: the row + // arrives by replication or a restore, every exchange returns the deliberately opaque 401, and + // list_oidc_trust — the one command an operator runs to check — tells them the trust is enabled. + describe('listing an unusable stored policy', () => { + it('reports why a refused policy is dead instead of showing it as healthy', async () => { + installed.mock.rows.set('broken', { + id: 'broken', + issuer: ISSUER, + audience: AUDIENCE, + claims: { ...VALID_CLAIMS }, + user: 'ci-deploy', + enabled: 'false', + }); + + const listed = (await listOidcTrust(su('list_oidc_trust'))).policies.find((policy) => policy.id === 'broken'); + assert.ok(listed, 'the row must still be listed — a listing tells the truth about what is stored'); + assert.notStrictEqual(listed.enabled, true, "'false' must not be reported as enabled: true"); + assert.match(listed.invalid_reason, /enabled/, 'expected the listing to say why it is refused'); + }); + + // The exchange also refuses a well-formed row whose user has been deleted or deactivated, with + // the same opaque 401 — and that is the most mundane arrival of all: someone removes the CI + // user and every deploy starts failing while the listing still says the trust is fine. + it('reports a policy naming a user that no longer exists', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ id: 'gone' }))); + await setUsersWithRolesCache(new Map()); + + const listed = (await listOidcTrust(su('list_oidc_trust'))).policies.find((p) => p.id === 'gone'); + assert.match(listed.invalid_reason, /does not exist/); + }); + + it('reports a policy naming a deactivated user', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ id: 'inactive' }))); + const users = new Map(); + users.set('ci-deploy', { username: 'ci-deploy', active: false, role: { role: 'r', permission: {} } }); + await setUsersWithRolesCache(users); + + const listed = (await listOidcTrust(su('list_oidc_trust'))).policies.find((p) => p.id === 'inactive'); + assert.match(listed.invalid_reason, /inactive/); + }); + + // Precedence: when a row is both malformed AND names a missing user, the shape problem is the + // one reported. It is the more fundamental complaint — the row would be refused even if the + // user were restored — and without this case the `continue` implementing it can be mutated to + // a no-op with every test still green. + it('reports the shape problem, not the user, when a row has both', async () => { + installed.mock.rows.set('both', { + id: 'both', + issuer: ISSUER, + audience: AUDIENCE, + claims: { ...VALID_CLAIMS }, + user: 'ghost-user', + operations: 'deploy_component', + }); + + const listed = (await listOidcTrust(su('list_oidc_trust'))).policies.find((p) => p.id === 'both'); + assert.match(listed.invalid_reason, /operations/, 'the shape problem is the more fundamental one'); + assert.doesNotMatch(listed.invalid_reason, /ghost-user/); + }); + + it('leaves a well-formed policy unannotated', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy())); + const listed = (await listOidcTrust(su('list_oidc_trust'))).policies.find( + (policy) => policy.id === 'my-app-prod' + ); + assert.strictEqual(listed.enabled, true); + assert.strictEqual(listed.invalid_reason, undefined); + }); + }); + + describe('listOidcTrust', () => { + it('returns policies sorted by id', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ id: 'zulu' }))); + await addOidcTrust(su('add_oidc_trust', validPolicy({ id: 'alpha' }))); + const { policies } = await listOidcTrust(su('list_oidc_trust')); + assert.deepStrictEqual( + policies.map((policy) => policy.id), + ['alpha', 'zulu'] + ); + }); + + it('includes disabled policies', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ enabled: false }))); + const { policies } = await listOidcTrust(su('list_oidc_trust')); + assert.strictEqual(policies.length, 1); + assert.strictEqual(policies[0].enabled, false); + }); + }); + + describe('dropOidcTrust', () => { + it('removes a policy', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy())); + const result = await dropOidcTrust(su('drop_oidc_trust', { id: 'my-app-prod' })); + assert.match(result.message, /my-app-prod/); + assert.strictEqual(installed.mock.rows.size, 0); + }); + + it('reports a missing policy as not found', async () => { + await assert.rejects( + () => dropOidcTrust(su('drop_oidc_trust', { id: 'never-existed' })), + (error) => { + assert.strictEqual(error.statusCode, 404); + return true; + } + ); + }); + }); + + describe('loadEnabledPolicies', () => { + it('skips disabled policies and sorts by id', async () => { + await addOidcTrust(su('add_oidc_trust', validPolicy({ id: 'zulu' }))); + await addOidcTrust(su('add_oidc_trust', validPolicy({ id: 'alpha' }))); + await addOidcTrust(su('add_oidc_trust', validPolicy({ id: 'disabled', enabled: false }))); + const policies = await loadEnabledPolicies(); + assert.deepStrictEqual( + policies.map((policy) => policy.id), + ['alpha', 'zulu'] + ); + }); + }); +}); diff --git a/unitTests/security/authn/oidc/verifyIdentityToken.test.js b/unitTests/security/authn/oidc/verifyIdentityToken.test.js new file mode 100644 index 0000000000..c6be8a973f --- /dev/null +++ b/unitTests/security/authn/oidc/verifyIdentityToken.test.js @@ -0,0 +1,220 @@ +'use strict'; + +const assert = require('node:assert'); +const { generateKeyPairSync, createPublicKey } = require('node:crypto'); +const jwt = require('jsonwebtoken'); +const { verifyIdentityToken } = require('#src/security/authn/oidc/identityToken'); + +const ISSUER = 'https://token.actions.githubusercontent.com'; +const AUDIENCE = 'https://my-instance.harperdb.io:9925/'; +const SIGNING_KID = 'test-signing-key'; + +// Fixed clock so expiry assertions do not depend on how long the suite takes to run. +const NOW_SECONDS = 1_800_000_000; + +describe('verifyIdentityToken', () => { + let privateKey; + let publicKey; + let otherPrivateKey; + + before(() => { + const pair = generateKeyPairSync('rsa', { + modulusLength: 2048, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }); + privateKey = pair.privateKey; + publicKey = pair.publicKey; + otherPrivateKey = generateKeyPairSync('rsa', { + modulusLength: 2048, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }).privateKey; + }); + + // Stands in for the JWKS lookup so these tests exercise the real verification path without network. + function lookupKey(issuer, kid) { + assert.strictEqual(issuer, ISSUER, 'expected the normalized issuer'); + if (kid !== SIGNING_KID) return Promise.reject(new Error(`unknown kid ${kid}`)); + return Promise.resolve(createPublicKey(publicKey)); + } + + function claimsFor(overrides = {}) { + return { + iss: ISSUER, + aud: AUDIENCE, + sub: 'repo:HarperFast/my-app:environment:production', + jti: 'token-id-1', + iat: NOW_SECONDS, + exp: NOW_SECONDS + 300, + repository: 'HarperFast/my-app', + repository_id: '67890', + workflow_ref: 'HarperFast/my-app/.github/workflows/deploy.yml@refs/heads/main', + environment: 'production', + ...overrides, + }; + } + + function sign(claims, { key = privateKey, algorithm = 'RS256', kid = SIGNING_KID } = {}) { + return jwt.sign(claims, key, { algorithm, header: { alg: algorithm, kid } }); + } + + function verify(token, options = {}) { + return verifyIdentityToken( + token, + { issuer: ISSUER, audience: AUDIENCE }, + { getSigningKey: lookupKey, clockTimestamp: NOW_SECONDS, ...options } + ); + } + + async function assertRejected(token, options) { + await assert.rejects( + () => verify(token, options), + (error) => { + assert.strictEqual(error.statusCode, 401); + // The reason belongs in the log, not in a response to an unauthenticated caller. + assert.strictEqual(error.message, 'Identity token was rejected'); + return true; + } + ); + } + + it('accepts a well-formed token and returns normalized claims', async () => { + const claims = await verify(sign(claimsFor())); + assert.strictEqual(claims.repository_id, '67890'); + assert.strictEqual(claims.environment, 'production'); + assert.strictEqual(claims.workflow_path, 'HarperFast/my-app/.github/workflows/deploy.yml'); + }); + + it('tolerates modest clock skew', async () => { + const token = sign(claimsFor()); + // 30s past expiry is inside the 60s tolerance. + const claims = await verify(token, { clockTimestamp: NOW_SECONDS + 330 }); + assert.strictEqual(claims.repository_id, '67890'); + }); + + it('rejects an expired token', async () => { + await assertRejected(sign(claimsFor()), { clockTimestamp: NOW_SECONDS + 3600 }); + }); + + it('rejects a token issued for another audience', async () => { + await assertRejected(sign(claimsFor({ aud: 'https://someone-elses-service.example/' }))); + }); + + it('rejects a token from another issuer', async () => { + await assertRejected(sign(claimsFor({ iss: 'https://gitlab.example.com' }))); + }); + + it('rejects a token signed by a key we do not trust', async () => { + await assertRejected(sign(claimsFor(), { key: otherPrivateKey })); + }); + + // Algorithm confusion: the public key is public, so a token HMAC-signed with it must never verify. + it('rejects an HMAC-signed token', async () => { + const token = jwt.sign(claimsFor(), publicKey, { + algorithm: 'HS256', + header: { alg: 'HS256', kid: SIGNING_KID }, + }); + await assertRejected(token); + }); + + it('rejects an unsigned token', async () => { + const token = jwt.sign(claimsFor(), '', { algorithm: 'none', header: { alg: 'none', kid: SIGNING_KID } }); + await assertRejected(token); + }); + + it('rejects a tampered payload', async () => { + const [header, , signature] = sign(claimsFor()).split('.'); + const forged = Buffer.from(JSON.stringify(claimsFor({ repository_id: '99999' }))).toString('base64url'); + await assertRejected(`${header}.${forged}.${signature}`); + }); + + // jsonwebtoken only enforces exp when it is present, so a token without one would never expire. + it('rejects a token with no exp claim', async () => { + const { exp: _exp, ...withoutExp } = claimsFor(); + await assertRejected(sign(withoutExp)); + }); + + it('rejects a token whose declared lifetime exceeds the ceiling', async () => { + await assertRejected(sign(claimsFor({ exp: NOW_SECONDS + 86_400 }))); + }); + + // The ceiling is measured from `iat`, so treating `iat` as optional means a token that omits it + // is not bounded at all — the check would skip exactly the token that most needs it. OIDC + // requires the claim, so demanding it costs nothing. + // The expectation handed to jwt.verify is the NORMALIZED issuer, but the comparison against `iss` + // is byte-for-byte — so an issuer that publishes a trailing slash (Azure AD v1: + // `https://sts.windows.net//`) could never authenticate. Both spellings already collapse + // to one value everywhere else (cache key, discovery check, policy lookup), so accepting both + // here is consistency, not widened trust. + it('accepts a token whose iss carries a trailing slash', async () => { + const claims = await verify(sign(claimsFor({ iss: `${ISSUER}/` }))); + assert.strictEqual(claims.repository, 'HarperFast/my-app'); + }); + + it('still rejects a token from a different issuer', async () => { + await assertRejected(sign(claimsFor({ iss: 'https://evil.example.com' }))); + }); + + it('rejects a token with no iat claim', async () => { + const { iat: _iat, ...withoutIat } = claimsFor({ exp: NOW_SECONDS + 86_400 }); + await assertRejected(sign(withoutIat)); + }); + + // jsonwebtoken validates exp and nbf but never rejects a future iat. Shifting the whole pair + // forward keeps `exp - iat` under the ceiling while leaving the token usable for a day, so the + // distance from the verification clock to `exp` has to be bounded independently. + it('rejects a token whose iat and exp are both shifted into the future', async () => { + const shifted = claimsFor({ iat: NOW_SECONDS + 86_400, exp: NOW_SECONDS + 86_700 }); + await assertRejected(sign(shifted)); + }); + + // Genuine clock skew is not an attack: an iat inside the tolerance still verifies. + it('accepts an iat slightly ahead of the verification clock', async () => { + const claims = await verify(sign(claimsFor({ iat: NOW_SECONDS + 30, exp: NOW_SECONDS + 300 }))); + assert.strictEqual(claims.repository, 'HarperFast/my-app'); + }); + + // Replay is keyed on a hash of the token itself, so `jti` is not required — which is what lets + // issuers that use `uti` (Azure) or omit it entirely work with no provider code. + it('accepts a token with no jti claim', async () => { + const { jti: _jti, ...withoutJti } = claimsFor(); + const claims = await verify(sign(withoutJti)); + assert.strictEqual(claims.repository_id, '67890'); + assert.strictEqual(claims.jti, undefined); + }); + + it('rejects a token whose kid is unknown to the key lookup', async () => { + await assert.rejects(() => verify(sign(claimsFor(), { kid: 'rotated-away' })), /unknown kid/); + }); + + it('rejects malformed input', async () => { + for (const value of ['', 'not-a-jwt', null, undefined, 42]) { + await assert.rejects(() => verify(value), 'expected rejection for ' + JSON.stringify(value)); + } + }); + + it('requires an https issuer', async () => { + await assert.rejects( + () => + verifyIdentityToken( + sign(claimsFor()), + { issuer: 'http://token.actions.githubusercontent.com', audience: AUDIENCE }, + { getSigningKey: lookupKey, clockTimestamp: NOW_SECONDS } + ), + /https/ + ); + }); + + it('requires an audience', async () => { + await assert.rejects( + () => + verifyIdentityToken( + sign(claimsFor()), + { issuer: ISSUER, audience: '' }, + { getSigningKey: lookupKey, clockTimestamp: NOW_SECONDS } + ), + /audience is required/ + ); + }); +}); diff --git a/unitTests/security/credentialProvenance.test.js b/unitTests/security/credentialProvenance.test.js new file mode 100644 index 0000000000..6d8a3f7623 --- /dev/null +++ b/unitTests/security/credentialProvenance.test.js @@ -0,0 +1,110 @@ +'use strict'; + +// A credential minted from a workload identity exchange (#2171) must not be tradeable for a +// longer-lived one. `create_authentication_tokens` is the path that makes that possible and does not +// look like it: it is NO_AUTH, but serverHandlers.js special-cases a call with no username/password +// to authenticate by Bearer token instead — so an exchanged token is accepted there in place of a +// password, honors a caller-supplied `expires_in`, and returns a 30-day refresh token. +// +// These cases need no stubbing because createTokens refuses ahead of the user lookup: the guard +// reads only the caller's principal, so a refused request costs no database read and writes nothing. + +const assert = require('node:assert'); +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const { createTokens, createOperationToken, clearJWTRSAKeysCache } = require('#src/security/tokenAuthentication'); +const { WORKLOAD_IDENTITY_CLAIM, attachWorkloadIdentityToUser } = require('#src/security/credentialProvenance'); + +function payloadOf(token) { + return JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString('utf8')); +} + +/** createTokens must reject with 403 — and reach neither the key material nor the user record. */ +async function assertDenied(authObj, message) { + await assert.rejects( + () => createTokens(authObj), + (e) => { + assert.strictEqual(e.statusCode, 403, message); + return true; + }, + message + ); +} + +describe('workload identity provenance blocks credential minting', () => { + // The case a scope-only guard misses: a trust policy names `operations` only when the operator + // opts in, so the ORDINARY exchanged token is unscoped. This is the PR's own headline example. + it('denies an unscoped workload identity caller', async () => { + await assertDenied( + { + expires_in: '3650d', // the lifetime extension the denial exists to prevent + hdb_user: { username: 'HDB_USER', fromWorkloadIdentity: true }, + }, + 'an unscoped workload token must not mint standing credentials' + ); + }); + + it('denies a scoped workload identity caller', async () => { + await assertDenied({ + hdb_user: { username: 'HDB_USER', fromWorkloadIdentity: true, tokenOperations: ['deploy_component'] }, + }); + }); + + // A session is minted from a username alone, so it cannot be narrowed after the fact — the guard + // has to sit ahead of the `purpose` branch, not inside the standing-credential path. + it('denies the login path too', async () => { + await assertDenied({ purpose: 'login', hdb_user: { username: 'HDB_USER', fromWorkloadIdentity: true } }); + }); + + // Provenance is independent of scope, but a scoped credential from any other source is still + // refused — that guard predates this one and both are load-bearing. + it('denies a scoped caller regardless of provenance', async () => { + await assertDenied({ hdb_user: { username: 'HDB_USER', tokenOperations: ['deploy_component'] } }); + }); + + it('denies a deny-all ([]) scope, which is a scope and not the absence of one', async () => { + await assertDenied({ hdb_user: { username: 'HDB_USER', tokenOperations: [] } }); + }); + + // Strict `=== true`, so a truthy value on a user record cannot be coerced into provenance and a + // falsy one cannot silently drop it. + it('treats only a literal true as provenance', () => { + for (const claim of [false, undefined, null, 'true', 1, {}]) { + const user = attachWorkloadIdentityToUser({}, claim); + assert.strictEqual(user.fromWorkloadIdentity, undefined, `${JSON.stringify(claim)} must not mark a principal`); + } + assert.strictEqual(attachWorkloadIdentityToUser({}, true).fromWorkloadIdentity, true); + }); +}); + +describe('provenance on a minted token', () => { + const user = { username: 'ci-deploy', super_user: false }; + let removeJwtKeys; + + before(() => { + // The keys land in a directory another suite asserts is empty, so they must come back out — + // see testUtils.installTestJwtKeys. + removeJwtKeys = testUtils.installTestJwtKeys(); + clearJWTRSAKeysCache(); + }); + + after(() => { + removeJwtKeys(); + clearJWTRSAKeysCache(); + }); + + // Signed with the rest of the payload: a caller cannot strip the claim to look password-minted. + it('stamps every token createOperationToken mints', async () => { + const token = await createOperationToken({ ...user, operations: ['deploy_component'] }, 3600); + assert.strictEqual(payloadOf(token)[WORKLOAD_IDENTITY_CLAIM], true); + }); + + // Unconditional, because the function mints without a password in every case — an unscoped + // exchanged token needs the marker just as much as a scoped one. + it('stamps an unscoped token too', async () => { + const token = await createOperationToken(user, 3600); + assert.strictEqual(payloadOf(token)[WORKLOAD_IDENTITY_CLAIM], true); + assert.strictEqual(payloadOf(token).operations, undefined, 'this token carries no scope to rely on'); + }); +}); diff --git a/unitTests/security/impersonation.test.js b/unitTests/security/impersonation.test.js index de7395af06..444d7797a4 100644 --- a/unitTests/security/impersonation.test.js +++ b/unitTests/security/impersonation.test.js @@ -603,4 +603,38 @@ describe('security/impersonation.ts', () => { assert.strictEqual(modeC.role.id, '_impersonated_ctx_user'); }); }); + + describe('token operation scope survives impersonation (#2174)', () => { + // A scoped operation token constrains the credential regardless of which principal it acts as, + // so impersonating must not shed the scope — otherwise a scoped super_user token escalates by + // impersonating a broader (still-downgraded) role. + const INLINE_ROLE = { role: { permission: { super_user: false, dev: { tables: {} } } } }; + + it('carries tokenOperations onto the impersonated user', async () => { + const su = makeSuperUser(); + su.tokenOperations = ['deploy_component']; + const impersonated = await applyImpersonation(su, INLINE_ROLE); + assert.deepStrictEqual(impersonated.tokenOperations, ['deploy_component']); + }); + + // Impersonation returns a NEW principal, so dropping the provenance marker would let a + // workload token launder it — impersonate, then mint the 30-day credential that + // createTokens would otherwise refuse. Carried for the same reason the scope is (#2171). + it('carries workload identity provenance onto the impersonated user', async () => { + const su = makeSuperUser(); + su.fromWorkloadIdentity = true; + const impersonated = await applyImpersonation(su, INLINE_ROLE); + assert.strictEqual(impersonated.fromWorkloadIdentity, true); + }); + + it('marks no provenance when the authenticating token was password-minted', async () => { + const impersonated = await applyImpersonation(makeSuperUser(), INLINE_ROLE); + assert.strictEqual(impersonated.fromWorkloadIdentity, undefined); + }); + + it('adds no scope when the authenticating token was unscoped', async () => { + const impersonated = await applyImpersonation(makeSuperUser(), INLINE_ROLE); + assert.strictEqual(impersonated.tokenOperations, undefined); + }); + }); }); diff --git a/unitTests/security/operationScope.test.js b/unitTests/security/operationScope.test.js new file mode 100644 index 0000000000..7a53f6daf9 --- /dev/null +++ b/unitTests/security/operationScope.test.js @@ -0,0 +1,63 @@ +'use strict'; + +const assert = require('node:assert'); +const { hasOperationScope, attachScopeToToken, attachScopeToUser } = require('#src/security/operationScope'); + +describe('operationScope', () => { + describe('hasOperationScope', () => { + it('is true for any array, including the empty deny-all scope', () => { + assert.strictEqual(hasOperationScope(['deploy_component']), true); + assert.strictEqual(hasOperationScope([]), true); + }); + + it('is false for an absent scope', () => { + for (const value of [undefined, null, 'deploy_component', 42, {}]) { + assert.strictEqual(hasOperationScope(value), false, `expected false for ${JSON.stringify(value)}`); + } + }); + }); + + describe('attachScopeToToken', () => { + it('copies a present scope onto the `operations` claim', () => { + const payload = { username: 'u', super_user: false }; + assert.strictEqual(attachScopeToToken(payload, ['deploy_component']), payload); + assert.deepStrictEqual(payload.operations, ['deploy_component']); + }); + + // The fail-closed case this whole feature hinges on: an empty scope means deny-all and must + // survive, not be treated as "no scope". + it('preserves an empty deny-all scope', () => { + const payload = { username: 'u', super_user: false }; + attachScopeToToken(payload, []); + assert.deepStrictEqual(payload.operations, []); + }); + + it('leaves the payload untouched for an absent scope', () => { + for (const scope of [undefined, null]) { + const payload = { username: 'u', super_user: false }; + attachScopeToToken(payload, scope); + assert.ok(!('operations' in payload), `operations must be absent for ${JSON.stringify(scope)}`); + } + }); + }); + + describe('attachScopeToUser', () => { + it('copies a present scope onto `tokenOperations`', () => { + const user = { username: 'u' }; + assert.strictEqual(attachScopeToUser(user, ['get_status']), user); + assert.deepStrictEqual(user.tokenOperations, ['get_status']); + }); + + it('preserves an empty deny-all scope', () => { + const user = { username: 'u' }; + attachScopeToUser(user, []); + assert.deepStrictEqual(user.tokenOperations, []); + }); + + it('leaves the user untouched for an absent scope', () => { + const user = { username: 'u' }; + attachScopeToUser(user, undefined); + assert.ok(!('tokenOperations' in user)); + }); + }); +}); diff --git a/unitTests/security/tokenOperationScope.test.js b/unitTests/security/tokenOperationScope.test.js new file mode 100644 index 0000000000..723ec55125 --- /dev/null +++ b/unitTests/security/tokenOperationScope.test.js @@ -0,0 +1,300 @@ +'use strict'; + +// The narrowing contract for a token-scoped operation allowlist: it may only ever subtract from what +// the user's role allows, and it must not be bypassable by any of verifyPerms' early-return paths. + +const assert = require('node:assert'); +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const opAuth = require('#src/utility/operation_authorization'); +const sql = require('#src/sqlTranslator/index'); + +// `insertData` is the internal function name for the `insert` operation; verifyPerms resolves the +// api_name via the permission registry, which is what a scope is written against. +const INSERT_FN = 'insertData'; + +function requestAs(permission, tokenOperations) { + const hdb_user = { username: 'ci-deploy', role: { role: 'r', permission } }; + if (tokenOperations !== undefined) hdb_user.tokenOperations = tokenOperations; + return { operation: 'insert', schema: 'data', table: 'dog', hdb_user, records: [] }; +} + +/** verifyPerms returns null when allowed, or a response object describing the denial. */ +function isAllowed(result) { + return result === null || result === undefined; +} + +describe('token-scoped operation narrowing', () => { + it('allows an operation inside the scope', () => { + const result = opAuth.verifyPerms(requestAs({ super_user: true }, ['insert']), INSERT_FN); + assert.ok(isAllowed(result), 'expected the in-scope operation to be permitted'); + }); + + it('denies an operation outside the scope', () => { + const result = opAuth.verifyPerms(requestAs({ super_user: true }, ['get_status']), INSERT_FN); + assert.ok(!isAllowed(result), 'expected the out-of-scope operation to be denied'); + }); + + // The whole point: a super_user returns null early in verifyPerms, so a narrowing check placed + // after that bypass would do nothing for exactly the identity that most needs constraining. + it('constrains a super_user', () => { + assert.ok(!isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }, ['get_status']), INSERT_FN))); + }); + + // Gate 2 treats an explicit SU-only listing in role.permission.operations as a deliberate grant + // and returns null. The scope must still win. + it('constrains an operation the role granted through its own operations allowlist', () => { + const permission = { super_user: false, operations: ['insert'] }; + assert.ok(!isAllowed(opAuth.verifyPerms(requestAs(permission, ['get_status']), INSERT_FN))); + }); + + // Narrowing only: naming an operation the role forbids must not grant it. + it('does not grant an operation the role forbids', () => { + const permission = { super_user: false, operations: ['get_status'] }; + const result = opAuth.verifyPerms(requestAs(permission, ['insert']), INSERT_FN); + assert.ok(!isAllowed(result), 'a scope must never widen the role'); + }); + + it('is inert when the token carries no scope', () => { + assert.ok(isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }), INSERT_FN))); + }); + + // A group name must be expanded rather than compared literally, or naming one would deny + // everything. `read_only` deliberately excludes insert, so it also proves the expansion narrows. + it('expands operation groups', () => { + const withGroup = requestAs({ super_user: true }, ['read_only']); + const result = opAuth.verifyPerms(withGroup, INSERT_FN); + + assert.ok(withGroup.hdb_user._expandedTokenOperations.size > 1, 'expected the group to expand'); + assert.ok(withGroup.hdb_user._expandedTokenOperations.has('search_by_value'), 'expected group members'); + assert.ok(!isAllowed(result), 'read_only must not admit insert'); + }); + + it('memoizes the expansion on the user', () => { + const request = requestAs({ super_user: true }, ['insert']); + opAuth.verifyPerms(request, INSERT_FN); + const first = request.hdb_user._expandedTokenOperations; + opAuth.verifyPerms(request, INSERT_FN); + assert.strictEqual(request.hdb_user._expandedTokenOperations, first, 'expected one expansion'); + }); + + // A null scope is what an unscoped policy stores; it must fall through to the role, not throw. + it('falls through to the role when the scope is null', () => { + assert.ok(isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }, null), INSERT_FN))); + }); + + it('denies everything when the scope is empty', () => { + assert.ok(!isAllowed(opAuth.verifyPerms(requestAs({ super_user: true }, []), INSERT_FN))); + }); +}); + +// `sql` dispatches to verifyPermsAST, in a branch mutually exclusive with the verifyPerms call in +// chooseOperation. A gate in only one of them lets a token scoped to e.g. get_status run arbitrary +// SQL against whatever its role can reach — which would falsify the whole "can only subtract" claim. +// The scope is written in API-operation names (`deploy_component`), which is what the caller sends as +// `operation`. verifyPerms must gate on that, not on the handler function name — deployComponent has +// no api_name mapping, so gating on the handler denied the feature's own headline operation. +describe('token scope gates on the API operation, not the handler name', () => { + function requestFor(operation, tokenOperations) { + const hdb_user = { username: 'ci-deploy', role: { role: 'r', permission: { super_user: true } } }; + if (tokenOperations !== undefined) hdb_user.tokenOperations = tokenOperations; + return { operation, hdb_user }; + } + + it('allows deploy_component when the scope names it', () => { + // deployComponent (the handler) has no api_name; the scope names the API op `deploy_component`. + const result = opAuth.verifyPerms(requestFor('deploy_component', ['deploy_component']), 'deployComponent'); + assert.ok(isAllowed(result), 'a token scoped to deploy_component must be able to deploy_component'); + }); + + it('denies deploy_component when the scope does not name it', () => { + const result = opAuth.verifyPerms(requestFor('deploy_component', ['get_status']), 'deployComponent'); + assert.ok(!isAllowed(result)); + }); + + // Shared-handler aliases must be distinguished by the API op, not conflated by handler name. + it('distinguishes shared-handler aliases (search_by_id vs search_by_value)', () => { + const allowed = opAuth.verifyPerms(requestFor('search_by_id', ['search_by_id']), 'searchByHash'); + assert.ok(isAllowed(allowed), 'search_by_id is in scope'); + const denied = opAuth.verifyPerms(requestFor('search_by_value', ['search_by_id']), 'searchByHash'); + assert.ok(!isAllowed(denied), 'search_by_value is not in scope even though it shares a handler'); + }); + + // A job (export_local/export_to_s3) is dispatched with its nested search_operation as requestJson, + // so the scope must gate the top-level op passed via options.apiOperation, not the inner search — + // otherwise a read-scoped token could exfiltrate data through an export. + it('gates a non-SQL export job on the export operation, not the nested search', () => { + const jobRequest = (tokenOperations) => ({ + operation: 'search_by_conditions', // the nested op the dispatcher hands verifyPerms + hdb_user: { username: 'ci-deploy', role: { role: 'r', permission: { super_user: true } }, tokenOperations }, + }); + const denied = opAuth.verifyPerms(jobRequest(['search_by_conditions']), 'searchByConditions', { + apiOperation: 'export_local', + }); + assert.ok(!isAllowed(denied), 'a search-scoped token must not be able to run export_local'); + + const allowed = opAuth.verifyPerms(jobRequest(['export_local']), 'searchByConditions', { + apiOperation: 'export_local', + }); + assert.ok(isAllowed(allowed), 'an export_local-scoped token may run the export'); + }); +}); + +describe('token-scoped narrowing on the SQL path', () => { + function userWithScope(permission, tokenOperations) { + const user = { username: 'ci-deploy', role: { role: 'r', permission } }; + if (tokenOperations !== undefined) user.tokenOperations = tokenOperations; + return user; + } + + function checkSql(statement, user, operation = 'sql') { + const parsed = sql.convertSQLToAST(statement); + return sql.checkASTPermissions({ operation, sql: statement, hdb_user: user }, parsed); + } + + it('denies SQL when the scope does not include it', () => { + const denial = checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true }, ['get_status'])); + assert.ok(denial, 'a token scoped away from sql must not be able to run SQL'); + }); + + // `api_operation` is how a job tells this check which top-level operation it was invoked as. For a + // direct `sql` call the request IS the client's body, so if that field were read from the body a + // caller could name any operation their scope happens to allow and run arbitrary SQL under it — + // and on this path checkASTPermissions is the ONLY gate, since the sql branch of chooseOperation + // is mutually exclusive with verifyPerms. It must never be honored from an inbound request. + it('ignores a caller-supplied api_operation on a direct SQL call', () => { + const user = userWithScope({ super_user: true }, ['deploy_component']); + const parsed = sql.convertSQLToAST('SELECT * FROM data.dog'); + const denial = sql.checkASTPermissions( + { operation: 'sql', sql: 'SELECT * FROM data.dog', api_operation: 'deploy_component', hdb_user: user }, + parsed + ); + assert.ok(denial, 'a body-supplied api_operation must not satisfy the scope gate'); + }); + + // The dangerous case: verifyPermsAST returns null unconditionally for a super_user. + it('denies a super_user whose scope excludes SQL', () => { + const denial = checkSql('DELETE FROM data.dog', userWithScope({ super_user: true }, ['deploy_component'])); + assert.ok(denial, 'the super_user bypass must not outrank the token scope'); + }); + + it('allows SQL when the scope includes it', () => { + const denial = checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true }, ['sql'])); + assert.strictEqual(denial, null, 'an in-scope sql statement should reach the normal perms checks'); + }); + + it('allows SQL through a group that contains it', () => { + const denial = checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true }, ['read_only'])); + assert.strictEqual(denial, null); + }); + + // `read_only` expands to include `sql` — the group defers DML enforcement to table CRUD perms — + // but verifyPermsAST returns null outright for a super_user before any table check runs. Without + // a variant check, a token scoped to `read_only` could DELETE: the one thing that name promises + // it cannot do. A write statement must additionally name its matching data operation. + for (const statement of [ + 'DELETE FROM data.dog', + "UPDATE data.dog SET name = 'x'", + 'INSERT INTO data.dog (id) VALUES (1)', + ]) { + const variant = statement.split(' ')[0].toLowerCase(); + + it(`denies ${variant} SQL for a read_only scope, even for a super_user`, () => { + const denial = checkSql(statement, userWithScope({ super_user: true }, ['read_only'])); + assert.ok(denial, `read_only must not admit ${variant} through SQL`); + }); + + // The same statement is admitted once the scope names the data operation — this is what + // separates read_only from standard_user without tracking which group admitted `sql`. + it(`allows ${variant} SQL when the scope names the data operation`, () => { + const denial = checkSql(statement, userWithScope({ super_user: true }, ['sql', variant])); + assert.strictEqual(denial, null, `a scope naming ${variant} may run it`); + }); + + // A write-capable non-SU role is bound by the scope too: the role permitting the write is + // not the question, the credential's scope is. + it(`denies ${variant} SQL for a write-capable non-super-user outside the scope`, () => { + const permission = { super_user: false, operations: ['sql', variant] }; + assert.ok(checkSql(statement, userWithScope(permission, ['read_only']))); + }); + } + + it('allows write SQL through standard_user, which names the data operations', () => { + const denial = checkSql('DELETE FROM data.dog', userWithScope({ super_user: true }, ['standard_user'])); + assert.strictEqual(denial, null); + }); + + // A bare ['sql'] grants the SQL interface, not unrestricted DML through it. + it('admits SELECT but not DELETE for a bare sql scope', () => { + assert.strictEqual(checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true }, ['sql'])), null); + assert.ok(checkSql('DELETE FROM data.dog', userWithScope({ super_user: true }, ['sql']))); + }); + + // An unscoped token is unaffected: the variant gate only ever narrows a scope that exists. + it('is inert for write SQL when the token carries no scope', () => { + assert.strictEqual(checkSql('DELETE FROM data.dog', userWithScope({ super_user: true })), null); + }); + + it('is inert for a token with no scope', () => { + const denial = checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true })); + assert.strictEqual(denial, null); + }); + + // TRIPWIRE for the #2202 split — this test exists to go red, not to describe desired behavior. + // + // An export job runs by re-parsing its nested search_operation, so the check inside job execution + // sees `operation: 'sql'` rather than `export_local`. Carrying the real operation on the request + // was reverted here because that object is client-supplied on the direct-SQL path, making any + // property it consults forgeable. That costs nothing TODAY only because the branch in processAST + // that would act on the denial is dead (its guard tests `.length` on an object that has none). + // + // #2202 makes that branch live. The moment it does, this admits-an-in-scope-export assertion + // fails, because the job's own SQL would be judged as `sql` against a scope naming only + // `export_local`. The two PRs can merge in either order, so whichever lands second turns CI red + // here instead of silently shipping an export-scoped token that 403s on its own export. + // + // If you are reading this because it just went red: the fix is not to relax the scope check. It is + // to give the job's real operation a carrier a client cannot set, then assert it here. + it('admits an in-scope export job through the path export.ts actually dispatches', async () => { + const outcome = await new Promise((resolve) => { + sql.evaluateSQL( + { + // Exactly what export.ts hands the SQL handler: the nested search_operation, with + // hdb_user attached and no parsed_sql_object, so it re-parses and re-checks. + operation: 'sql', + sql: 'SELECT * FROM data.dog', + hdb_user: userWithScope({ super_user: true }, ['export_local']), + }, + (error) => resolve({ error }) + ); + }); + + // Not "no error" — the table does not exist in this unit context, so it fails downstream. The + // assertion is specifically that it was not refused by the PERMISSION gate, identified by + // shape rather than by value: that path is the only one that calls back with a bare numeric + // status (`UNAUTHORIZED_RESPONSE`), while every other failure forwards an Error. evaluateSQL + // drops the second callback argument on error, so the denial object itself never arrives here + // — the number is the whole signal. + // + // Deliberately not `!== 403`: that literal lives in the file this test watches, so changing it + // would leave the tripwire green while the refusal it exists to catch still happened. A + // tripwire must not depend on a constant its own target owns. + assert.ok( + typeof outcome.error !== 'number', + `an export_local-scoped token must not be denied by its own export job (see #2202); got status ${outcome.error}` + ); + }); + + it('gates a nested-SQL export job on the export operation, not on `sql`', () => { + // export_local carries its query as SQL, but the scope names the job, not `sql`. A token scoped + // only to `sql` must not be able to start an export it was never granted. + const deniedUser = userWithScope({ super_user: true }, ['sql']); + const denied = checkSql('SELECT * FROM data.dog', deniedUser, 'export_local'); + assert.ok(denied, 'export_local is outside a `sql`-only scope'); + + const allowedUser = userWithScope({ super_user: true }, ['export_local']); + const allowed = checkSql('SELECT * FROM data.dog', allowedUser, 'export_local'); + assert.strictEqual(allowed, null, 'an export_local-scoped token may run the export'); + }); +}); diff --git a/unitTests/security/tokenOperationScopeMinting.test.js b/unitTests/security/tokenOperationScopeMinting.test.js new file mode 100644 index 0000000000..f61ab70870 --- /dev/null +++ b/unitTests/security/tokenOperationScopeMinting.test.js @@ -0,0 +1,51 @@ +'use strict'; + +// The mint side of the narrowing contract. Kept separate from tokenOperationScope.test.js because +// this needs real JWT signing keys, and the point of these cases is the *round trip*: a scope that +// survives verifyPerms in isolation is worthless if createOperationToken drops it on the way out. + +const assert = require('node:assert'); +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const { createOperationToken, clearJWTRSAKeysCache } = require('#src/security/tokenAuthentication'); + +function payloadOf(token) { + return JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString('utf8')); +} + +describe('operation scope on a minted token', () => { + const user = { username: 'ci-deploy', super_user: false }; + let removeJwtKeys; + + before(() => { + // The keys land in a directory another suite asserts is empty, so they must come back out — + // see testUtils.installTestJwtKeys. + removeJwtKeys = testUtils.installTestJwtKeys(); + clearJWTRSAKeysCache(); + }); + + after(() => { + removeJwtKeys(); + clearJWTRSAKeysCache(); + }); + + it('carries a scope', async () => { + const token = await createOperationToken({ ...user, operations: ['deploy_component'] }, 3600); + assert.deepStrictEqual(payloadOf(token).operations, ['deploy_component']); + }); + + // The bug this exists to prevent: an empty scope means "no operations". A truthiness check on + // length drops the claim, which leaves the token UNSCOPED — a security control failing open. + it('carries an empty scope rather than dropping it', async () => { + const token = await createOperationToken({ ...user, operations: [] }, 3600); + assert.deepStrictEqual(payloadOf(token).operations, [], 'an empty scope must not become an absent scope'); + }); + + it('omits the claim when there is no scope', async () => { + for (const operations of [undefined, null]) { + const token = await createOperationToken({ ...user, operations }, 3600); + assert.strictEqual(payloadOf(token).operations, undefined); + } + }); +}); diff --git a/unitTests/security/tokenScopeRefresh.test.js b/unitTests/security/tokenScopeRefresh.test.js new file mode 100644 index 0000000000..548e88b983 --- /dev/null +++ b/unitTests/security/tokenScopeRefresh.test.js @@ -0,0 +1,88 @@ +'use strict'; + +// A scoped credential must only ever produce an equally-scoped one (#2174). refresh_operation_token +// mints a fresh operation token from a refresh token; if it dropped the `operations` claim, a scoped +// refresh credential would refresh back to the full role. This drives the real validate→decode→sign +// path with the test signing keys rather than asserting the payload in isolation. + +const assert = require('node:assert'); +const testUtils = require('../testUtils.js'); +testUtils.preTestPrep(); + +const fs = require('node:fs'); +const path = require('node:path'); +const jwt = require('jsonwebtoken'); +const { refreshOperationToken, clearJWTRSAKeysCache } = require('#src/security/tokenAuthentication'); +const password = require('#src/utility/password'); +const { setUsersWithRolesCache } = require('#src/security/user'); +const env = require('#src/utility/environment/environmentManager'); +const terms = require('#src/utility/hdbTerms'); + +const USERNAME = 'ci-deploy'; + +function payloadOf(token) { + return JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString('utf8')); +} + +describe('refresh_operation_token operation scope', () => { + let removeJwtKeys; + let privateKey; + let passphrase; + + before(async () => { + removeJwtKeys = testUtils.installTestJwtKeys(); + clearJWTRSAKeysCache(); + // Sign the refresh token with the exact keys refreshOperationToken will verify against. + const keysDir = path.join(env.getHdbBasePath(), terms.LICENSE_KEY_DIR_NAME); + privateKey = fs.readFileSync(path.join(keysDir, terms.JWT_ENUM.JWT_PRIVATE_KEY_NAME)); + passphrase = fs.readFileSync(path.join(keysDir, terms.JWT_ENUM.JWT_PASSPHRASE_NAME), 'utf8'); + }); + + after(() => { + removeJwtKeys(); + clearJWTRSAKeysCache(); + }); + + // Mints a `refresh`-subject token and seeds the users cache so validateRefreshToken accepts it + // (it matches the token against the SHA-256 hash stored on the user). + async function seedRefreshToken(claims) { + const refreshToken = jwt.sign( + claims, + { key: privateKey, passphrase }, + { + algorithm: 'RS256', + subject: 'refresh', + expiresIn: '30d', + } + ); + const users = new Map([ + [ + USERNAME, + { + username: USERNAME, + active: true, + refresh_token: password.hash(refreshToken, password.HASH_FUNCTION.SHA256), + role: { role: 'deployer', permission: { super_user: false } }, + }, + ], + ]); + await setUsersWithRolesCache(users); + return refreshToken; + } + + it('carries the scope from the refresh token into the minted operation token', async () => { + const refreshToken = await seedRefreshToken({ + username: USERNAME, + super_user: false, + operations: ['deploy_component'], + }); + const { operation_token } = await refreshOperationToken({ refresh_token: refreshToken }); + assert.deepStrictEqual(payloadOf(operation_token).operations, ['deploy_component']); + }); + + it('mints an unscoped operation token from an unscoped refresh token', async () => { + const refreshToken = await seedRefreshToken({ username: USERNAME, super_user: false }); + const { operation_token } = await refreshOperationToken({ refresh_token: refreshToken }); + assert.strictEqual(payloadOf(operation_token).operations, undefined); + }); +}); diff --git a/unitTests/server/serverHelpers/serverUtilities.test.js b/unitTests/server/serverHelpers/serverUtilities.test.js index be79349c1b..5a6a4b4007 100644 --- a/unitTests/server/serverHelpers/serverUtilities.test.js +++ b/unitTests/server/serverHelpers/serverUtilities.test.js @@ -73,6 +73,44 @@ describe('Test serverUtilities.js module ', () => { request.operation = 'add_user'; assert.doesNotThrow(() => serverUtilities.chooseOperation(request, true)); }); + + // The token scope's "can only ever subtract" invariant, asserted where it is ENFORCED rather + // than where it is computed (#2171/#2174). This is the front door for an export job carrying + // nested SQL: the job is gated here, before it is ever queued, and the check inside the job's + // own SQL execution is a dead branch (#2202). So this gate is the whole safety argument, and + // the rest of the scope suite only asserts that a denial object comes back — not that anyone + // throws on it. + function exportJobRequest(tokenOperations, sql) { + const request = testUtils.deepClone(TEST_JSON_SUPER_USER); + request.operation = 'export_local'; + request.search_operation = { operation: 'sql', sql }; + request.hdb_user.tokenOperations = tokenOperations; + return request; + } + + it('throws 403 for an export job whose nested write SQL is outside the token scope', function () { + // Scoped to the export itself but not to `delete`: a write statement additionally requires + // its matching data operation, which is what keeps `read_only` from admitting a DELETE. + assert.throws( + () => serverUtilities.chooseOperation(exportJobRequest(['export_local'], 'DELETE FROM data.dog')), + (error) => { + assert.strictEqual(error.statusCode ?? error.http_code, 403, 'expected a forbidden status'); + return true; + }, + 'an export job must not smuggle write SQL past the scope gate' + ); + }); + + it('throws 403 for an export job when the export operation itself is outside the scope', function () { + assert.throws(() => serverUtilities.chooseOperation(exportJobRequest(['get_status'], 'SELECT * FROM data.dog'))); + }); + + // The other direction, so this cannot pass by refusing everything: an in-scope export runs. + it('admits an export job whose nested SQL is inside the token scope', function () { + assert.doesNotThrow(() => + serverUtilities.chooseOperation(exportJobRequest(['export_local'], 'SELECT * FROM data.dog')) + ); + }); }); describe('registered operation authorization envelope', function () { @@ -875,3 +913,58 @@ describe('Test serverUtilities.js module ', () => { }); }); }); + +// processLocalTransaction builds `operationLog` from mainLogger at module load, so the logged body +// cannot be intercepted after the fact — which is why the older redaction test above guards on +// `if (info_log_stub.called)` and passes vacuously. Testing the redaction directly avoids that. +describe('redactForOperationLog', () => { + const { UNLOGGABLE_OPERATION_FIELDS, redactForOperationLog } = serverUtilities; + + // Redaction runs before the handler, so a rejected request logs a still-spendable credential. + const CREDENTIAL_FIELDS = { + hdb_user: { username: 'admin' }, + hdbAuthHeader: 'Basic abc', + password: 'pw', + payload: 'blob', + credentials: [{ secret: 'deploy.app.github.com' }], + registryAuth: 'auth', + value: 'env-secret', + values: ['env-secret'], + envelope: 'enc:v1:sealed', + // login (#1876) and exchange_oidc_token (#2171) both carry a live token here. + token: 'eyJhbGciOiJSUzI1NiJ9.identity.signature', + // refresh_operation_token carries the 30-day credential. + refresh_token: 'eyJhbGciOiJSUzI1NiJ9.refresh.signature', + }; + + it('strips every credential-bearing field', () => { + const clean = redactForOperationLog({ operation: 'exchange_oidc_token', ...CREDENTIAL_FIELDS }); + for (const field of Object.keys(CREDENTIAL_FIELDS)) { + assert.ok(!(field in clean), `${field} must not reach the operations log`); + } + }); + + it('leaves nothing JWT-shaped behind', () => { + const clean = redactForOperationLog({ operation: 'exchange_oidc_token', ...CREDENTIAL_FIELDS }); + assert.ok(!/eyJ[A-Za-z0-9_-]/.test(JSON.stringify(clean)), 'no JWT-shaped value should survive'); + }); + + it('preserves everything else', () => { + const clean = redactForOperationLog({ operation: 'create_schema', schema: 'test', database: 'data' }); + assert.deepStrictEqual(clean, { operation: 'create_schema', schema: 'test', database: 'data' }); + }); + + it('does not mutate the request body', () => { + const body = { operation: 'exchange_oidc_token', token: 'live-credential' }; + redactForOperationLog(body); + assert.equal(body.token, 'live-credential', 'the handler still needs the field it was sent'); + }); + + // The list is the contract; pin the credential-bearing entries so a refactor cannot quietly drop + // one the way harper#1527 did for set_env_value. + it('pins the fields the list must contain', () => { + for (const field of Object.keys(CREDENTIAL_FIELDS)) { + assert.ok(UNLOGGABLE_OPERATION_FIELDS.includes(field), `${field} must stay in the redaction list`); + } + }); +}); diff --git a/unitTests/testUtils.js b/unitTests/testUtils.js index 64d9565158..126af469b8 100644 --- a/unitTests/testUtils.js +++ b/unitTests/testUtils.js @@ -506,11 +506,45 @@ function orderedArray(iterator) { return array; } +/** + * Writes a throwaway RSA keypair where getJWTRSAKeys() looks for it, and returns a cleanup function + * that removes exactly the files it created. + * + * The keys directory is shared with unitTests/utility/install/checkJWTTokensExist.test.js, whose + * happy path asserts those files are ABSENT (it expects accessSync to throw ENOENT). Mocha runs + * every file in one process against one base path, so a test that writes keys and does not clean up + * silently breaks that suite depending on file order — which is exactly how it broke, and why this + * lives here instead of being copy-pasted into each caller. + */ +function installTestJwtKeys() { + const { generateKeyPairSync } = require('node:crypto'); + const passphrase = 'test-passphrase'; + const { privateKey, publicKey } = generateKeyPairSync('rsa', { + modulusLength: 2048, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem', cipher: 'aes-256-cbc', passphrase }, + }); + const keysDir = path.join(env.getHdbBasePath(), terms.LICENSE_KEY_DIR_NAME); + fs.mkdirpSync(keysDir); + + const written = [ + [path.join(keysDir, terms.JWT_ENUM.JWT_PRIVATE_KEY_NAME), privateKey], + [path.join(keysDir, terms.JWT_ENUM.JWT_PUBLIC_KEY_NAME), publicKey], + [path.join(keysDir, terms.JWT_ENUM.JWT_PASSPHRASE_NAME), passphrase], + ]; + for (const [file, contents] of written) fs.writeFileSync(file, contents); + + return function removeTestJwtKeys() { + for (const [file] of written) fs.removeSync(file); + }; +} + module.exports = { changeProcessToBinDir, deepClone, mochaAsyncWrapper, preTestPrep, + installTestJwtKeys, cleanUpDirectories, createMockDB, tearDownMockDB, diff --git a/upgrade/directives/5-3-0.ts b/upgrade/directives/5-3-0.ts new file mode 100644 index 0000000000..86033401b7 --- /dev/null +++ b/upgrade/directives/5-3-0.ts @@ -0,0 +1,132 @@ +'use strict'; + +// 5.3.0 — introduces system.hdb_oidc_trust and system.hdb_oidc_token_use for OIDC trusted +// publishing (#2171). +// +// Fresh installs get the table from json/systemSchema.json; this covers existing installs. The +// version must match the release that ships the dependent operations — see 5-1-0.ts for what +// happens when it does not, and DESIGN.md "System table bootstrap" for the three touchpoints. + +import { databases } from '../../resources/databases.ts'; +import systemSchema from '../../json/systemSchema.json'; +import * as terms from '../../utility/hdbTerms.ts'; +import * as initPaths from '../../dataLayer/harperBridge/lmdbBridge/lmdbUtility/initializePaths.js'; +import bridge from '../../dataLayer/harperBridge/harperBridge.ts'; +import hdbLogger from '../../utility/logging/harper_logger.ts'; + +const OIDC_TRUST_TABLE = terms.SYSTEM_TABLE_NAMES.OIDC_TRUST_TABLE_NAME; +const OIDC_TOKEN_USE_TABLE = terms.SYSTEM_TABLE_NAMES.OIDC_TOKEN_USE_TABLE_NAME; + +/** + * The replay table gets the same bootstrap as the trust table, for a reason specific to it: replay + * records must replicate, and a node that has never completed an exchange would otherwise not have + * the table at all when a peer's record arrives. Provisioning it at install/upgrade time removes + * that question entirely rather than depending on how replication treats a row for an unknown table. + * + * Only the primary key is declared here. The `expiresAt` TTL attribute is not expressible through + * CreateTableObject, so tokenExchange.ts layers it on with an unconditional `table()` call — the + * same two-step hdb_certificate_cache uses. + * + * KNOWN LIMITATION, shared with hdb_certificate_cache rather than specific to this table: that + * second step runs on the exchange path, so a node that never performs an exchange — the passive + * members of a cluster, which is most of them when CI always targets one endpoint — has the table + * from this directive but never registers `expiresAt` locally. Replicated replay rows still land + * there (that is what `audit: true` buys) but get no eviction, so the table grows on exactly the + * nodes doing no work. Rows are small and bounded by exchange volume, so this is slow rather than + * dangerous. Fixing it properly means installing the TTL at system-table setup instead of on first + * use, which is a change to how every lazily-extended system table is provisioned — worth doing + * once, for both tables, rather than special-casing this one. + */ +async function createHdbOidcTokenUseIfMissing() { + if (databases.system?.[OIDC_TOKEN_USE_TABLE]) { + hdbLogger.info(`system.${OIDC_TOKEN_USE_TABLE} already exists; skipping create.`); + await patchIsHashAttribute(OIDC_TOKEN_USE_TABLE); + return; + } + + hdbLogger.info(`Creating system.${OIDC_TOKEN_USE_TABLE} table for OIDC replay protection.`); + + const CreateTableObject = + require('../../dataLayer/CreateTableObject').default || require('../../dataLayer/CreateTableObject'); + const schema = (systemSchema as any)[OIDC_TOKEN_USE_TABLE]; + if (!schema) { + throw new Error(`systemSchema.${OIDC_TOKEN_USE_TABLE} is missing; cannot run 5.3.0 directive.`); + } + + initPaths.initSystemSchemaPaths(terms.SYSTEM_SCHEMA_NAME, OIDC_TOKEN_USE_TABLE); + const createTable = new (CreateTableObject as any)( + terms.SYSTEM_SCHEMA_NAME, + OIDC_TOKEN_USE_TABLE, + schema.hash_attribute + ); + createTable.attributes = schema.attributes; + const primaryKeyAttribute = createTable.attributes.find(({ attribute }) => attribute === schema.hash_attribute); + if (primaryKeyAttribute) primaryKeyAttribute.isPrimaryKey = true; + // Must match `"audit": true` in systemSchema.json — and here auditing is not cosmetic: it is the + // replication change feed, without which replay protection would be node-local. + createTable.audit = true; + + await bridge.createTable(OIDC_TOKEN_USE_TABLE, createTable); + await patchIsHashAttribute(OIDC_TOKEN_USE_TABLE); +} + +async function createHdbOidcTrustIfMissing() { + if (databases.system?.[OIDC_TRUST_TABLE]) { + hdbLogger.info(`system.${OIDC_TRUST_TABLE} already exists; skipping create.`); + await patchIsHashAttribute(OIDC_TRUST_TABLE); + return; + } + + hdbLogger.info(`Creating system.${OIDC_TRUST_TABLE} table for OIDC trusted publishing.`); + + const CreateTableObject = + require('../../dataLayer/CreateTableObject').default || require('../../dataLayer/CreateTableObject'); + const schema = (systemSchema as any)[OIDC_TRUST_TABLE]; + if (!schema) { + throw new Error(`systemSchema.${OIDC_TRUST_TABLE} is missing; cannot run 5.3.0 directive.`); + } + + initPaths.initSystemSchemaPaths(terms.SYSTEM_SCHEMA_NAME, OIDC_TRUST_TABLE); + const createTable = new (CreateTableObject as any)(terms.SYSTEM_SCHEMA_NAME, OIDC_TRUST_TABLE, schema.hash_attribute); + createTable.attributes = schema.attributes; + const primaryKeyAttribute = createTable.attributes.find(({ attribute }) => attribute === schema.hash_attribute); + if (primaryKeyAttribute) primaryKeyAttribute.isPrimaryKey = true; + // Must match `"audit": true` in systemSchema.json, or the fresh-install and upgrade paths diverge. + createTable.audit = true; + + await bridge.createTable(OIDC_TRUST_TABLE, createTable); + await patchIsHashAttribute(OIDC_TRUST_TABLE); +} + +/** + * Ensure a table's __dbis__ primary-key entry carries is_hash_attribute: true. + * + * harperdb@4.x reads is_hash_attribute from __dbis__ to derive the LMDB DBI open flags; without it + * the DBI is opened with the opposite flags (DUPSORT set) and LMDB throws MDB_INCOMPATIBLE, breaking + * downgrade — the same guard 5-1-0.ts and 5-2-0.ts apply to their tables. Both tables created here + * go through the identical CreateTableObject + bridge.createTable path, so both need it; exempting + * one would be a silent asymmetry rather than a decision. Idempotent: no-op when already set. + */ +async function patchIsHashAttribute(tableName: string) { + const systemTable = (databases as any).system?.[tableName]; + if (!systemTable?.dbisDB) return; + + const dbiName = `${tableName}/`; + const primaryAttr = systemTable.dbisDB.getSync(dbiName); + if (!primaryAttr || primaryAttr.is_hash_attribute) return; // already correct + + primaryAttr.is_hash_attribute = true; + await systemTable.dbisDB.put(dbiName, primaryAttr); + hdbLogger.info( + `Patched system.${tableName} __dbis__ entry with is_hash_attribute=true for harperdb@4.x downgrade compatibility.` + ); +} + +const directive530 = { + version: '5.3.0', + description: 'create system.hdb_oidc_trust and system.hdb_oidc_token_use tables for OIDC trusted publishing', + sync_functions: [] as Array<() => unknown>, + async_functions: [createHdbOidcTrustIfMissing, createHdbOidcTokenUseIfMissing] as Array<() => Promise>, +}; + +export default [directive530]; diff --git a/upgrade/directives/directivesController.ts b/upgrade/directives/directivesController.ts index 4674e09232..ed1775fbfb 100644 --- a/upgrade/directives/directivesController.ts +++ b/upgrade/directives/directivesController.ts @@ -11,13 +11,14 @@ import * as hdbTerms from '../../utility/hdbTerms.ts'; import hdbLog from '../../utility/logging/harper_logger.ts'; import directive510 from './5-1-0.ts'; import directive520 from './5-2-0.ts'; +import directive530 from './5-3-0.ts'; const { DATA_VERSION, UPGRADE_VERSION } = hdbTerms.UPGRADE_JSON_FIELD_NAMES_ENUM as any; let versions: any = new Map(); // All directive modules export an array of { version, sync_functions, async_functions }. // New directives must be imported above and registered here. -for (const directive of [...directive510, ...directive520]) { +for (const directive of [...directive510, ...directive520, ...directive530]) { versions.set(directive.version, directive); } diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index 1c9ab2141d..dc911eb9f5 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -211,6 +211,8 @@ export const SYSTEM_TABLE_NAMES = { DEPLOYMENT_TABLE_NAME: 'hdb_deployment', AGENT_SESSION_TABLE_NAME: 'hdb_agent_session', SECRET_TABLE_NAME: 'hdb_secret', + OIDC_TRUST_TABLE_NAME: 'hdb_oidc_trust', + OIDC_TOKEN_USE_TABLE_NAME: 'hdb_oidc_token_use', } as const; /** Hash attribute for the system info table */ @@ -348,6 +350,10 @@ export const OPERATIONS_ENUM = { LIST_SECRETS: 'list_secrets', DELETE_SECRET: 'delete_secret', GET_SECRETS_PUBLIC_KEY: 'get_secrets_public_key', + ADD_OIDC_TRUST: 'add_oidc_trust', + LIST_OIDC_TRUST: 'list_oidc_trust', + DROP_OIDC_TRUST: 'drop_oidc_trust', + EXCHANGE_OIDC_TOKEN: 'exchange_oidc_token', GET_DEPLOYMENT_PAYLOAD: 'get_deployment_payload', DELETE_DEPLOYMENT_PAYLOAD: 'delete_deployment_payload', AGENT_PROMPT: 'agent_prompt', diff --git a/utility/operation_authorization.ts b/utility/operation_authorization.ts index 148fe26bf2..1c09895e88 100644 --- a/utility/operation_authorization.ts +++ b/utility/operation_authorization.ts @@ -47,6 +47,8 @@ import { handleHDBError, hdbErrors } from '../utility/errors/hdbError.ts'; import * as regDeprecated from '../resources/registrationDeprecated.ts'; import * as deploymentOperations from '../components/deploymentOperations.ts'; import * as secretOperations from '../components/secretOperations.ts'; +import * as trustPolicyOperations from '../security/authn/oidc/trustPolicyOperations.ts'; +import * as tokenExchange from '../security/authn/oidc/tokenExchange.ts'; const requiredPermissions = new Map(); const DELETE_PERM = 'delete'; @@ -359,6 +361,28 @@ requiredPermissions.set( new (permission as any)(true, [], terms.OPERATIONS_ENUM.GET_SECRETS_PUBLIC_KEY) ); +// OIDC trust policies (#2171). A policy lets an external CI run authenticate as a Harper user, so +// these are SU-only; the handlers ALSO enforce super_user directly so they cannot be delegated +// through a role's `operations` allowlist (gate-2 bypass below). +requiredPermissions.set( + trustPolicyOperations.addOidcTrust.name, + new (permission as any)(true, [], terms.OPERATIONS_ENUM.ADD_OIDC_TRUST) +); +requiredPermissions.set( + trustPolicyOperations.listOidcTrust.name, + new (permission as any)(true, [], terms.OPERATIONS_ENUM.LIST_OIDC_TRUST) +); +requiredPermissions.set( + trustPolicyOperations.dropOidcTrust.name, + new (permission as any)(true, [], terms.OPERATIONS_ENUM.DROP_OIDC_TRUST) +); +// The exchange is unauthenticated by design — it authenticates its own caller against a trust +// policy, the way create_authentication_tokens does against a password. +requiredPermissions.set( + tokenExchange.exchangeOidcToken.name, + new (permission as any)(false, [], terms.OPERATIONS_ENUM.EXCHANGE_OIDC_TOKEN) +); + //Below are functions that are currently open to all roles requiredPermissions.set(regDeprecated.getRegistrationInfo.name, new (permission as any)(false, [])); requiredPermissions.set(user.userInfo.name, new (permission as any)(false, [], terms.OPERATIONS_ENUM.USER_INFO)); @@ -418,7 +442,77 @@ module.exports = { * @param operation - The operation specified in the call. * @returns {null | PermissionResponseObject} - null if permissions match, errors returned in the PermissionResponseObject */ -export function verifyPermsAST(ast, userObject, operation) { +/** + * Token-scoped narrowing: a minted operation token may carry a subset of what its user's role allows, + * so one credential can be handed out with less authority than the user has (an OIDC trust policy + * uses this to scope a single workflow). Returns a denial, or undefined when the scope permits — or + * when there is no scope, which is every token minted before this existed. + * + * Shared by verifyPerms AND verifyPermsAST, and called first in both. Four ways this gets bypassed + * if it moves or is incomplete: + * + * 1. `sql` dispatches to verifyPermsAST, in a branch mutually exclusive with the verifyPerms call + * (server/serverHelpers/serverUtilities.ts). A gate in only one of them means a token scoped to + * `get_status` can still run arbitrary SQL against whatever its role can reach. + * 2. Both functions `return null` early for a super_user — the identity that most needs constraining. + * 3. verifyPerms' `operations` gate 2 also returns null, treating an explicit listing of an SU-only + * operation as a deliberate grant. + * 4. Operations that never reach verifyPerms at all: create_authentication_tokens (NO_AUTH), + * refresh_operation_token, and impersonation each PRODUCE a new credential or principal, so the + * scope has to be carried forward there too or a scoped token mints an unscoped one. That carry- + * forward lives in tokenAuthentication.ts and impersonation.ts, not here. + * + * It can only subtract. The scope is deliberately NOT merged into `permission.operations`, because + * that field is not purely narrowing (see gate 2) — merging into it could widen instead. + * + * `apiOperation` MUST be the snake_case API operation the caller actually invoked (`deploy_component`, + * `sql`, `export_local`, ...) — i.e. `requestJson.operation`, the same namespace the policy's scope is + * written in. It is emphatically NOT the handler function name: many handlers have no `api_name` + * mapping (deploy_component → `deployComponent`) and some are shared across operations + * (`search_by_id`/`search_by_hash`), so resolving the scope name from the handler both denies the + * feature's own headline op and conflates aliases. Callers pass the real operation. + */ +function tokenScopeDenial(userObject: any, apiOperation: string) { + // `!= null`, not `!== undefined`: an unscoped policy stores `operations: null`, and expanding a + // null would throw rather than fall through to the role. + const tokenOperations = userObject?.tokenOperations; + if (tokenOperations == null) return undefined; + + // `instanceof Set`, not just presence: this memo rides on hdb_user, and a job persists that user + // into hdb_job.request, where msgpackr round-trips a Set back as a plain Array. Trusting the + // memo's presence would then call .has() on an Array and throw a TypeError out of the auth gate + // — still fail-closed, but as a 500 rather than a denial, and only inside a job worker. + let scopedOps = userObject._expandedTokenOperations; + if (!(scopedOps instanceof Set)) { + scopedOps = userObject._expandedTokenOperations = expandOperationsPerms(tokenOperations); + } + if (scopedOps.has(apiOperation)) return undefined; + + harperLogger.info(`Operation '${apiOperation}' is outside the scope of the presented token`); + return new PermissionResponseObject().handleUnauthorizedItem(HDB_ERROR_MSGS.OP_NOT_IN_OPERATIONS(apiOperation)); +} + +/** + * A scope naming `sql` grants the SQL interface, not unrestricted DML through it. + * + * `read_only` expands to include `sql` — the group defers DML enforcement to table CRUD permissions + * (see its note in operationPermissions.ts) — but verifyPermsAST returns null outright for a + * super_user before any table check runs. Without this, a token scoped to `read_only` could DELETE, + * which is the one thing that name promises it cannot do. + * + * So a write statement additionally requires its matching data operation in scope. That is exactly + * what separates `read_only` (no insert/update/delete) from `standard_user` (all three), with no + * need to track which group admitted `sql`. A bare `['sql']` therefore admits SELECT only; name the + * write operations alongside it to allow more. Anything that is not a recognized SELECT falls + * through to the same check and is denied unless named, so an unfamiliar statement type fails closed. + */ +function sqlWriteScopeDenial(userObject: any, sqlVariant: string) { + if (userObject?.tokenOperations == null) return undefined; + if (sqlVariant === terms.VALID_SQL_OPS_ENUM.SELECT) return undefined; + return tokenScopeDenial(userObject, sqlVariant); +} + +export function verifyPermsAST(ast, userObject, operation, apiOperation = terms.OPERATIONS_ENUM.SQL) { //TODO - update these validation checks to use validate.js if (commonUtils.isEmptyOrZeroLength(ast)) { harperLogger.info('verify_perms_ast has an empty user parameter'); @@ -432,6 +526,15 @@ export function verifyPermsAST(ast, userObject, operation) { harperLogger.info('verify_perms_ast has a null operation parameter'); throw handleHDBError(new Error()); } + + // `operation` here is the SQL statement variant (select/insert/...), not the API operation. The + // scope is checked against `apiOperation` — the top-level operation the caller invoked, which is + // `sql` for a direct SQL call but `export_local`/`export_to_s3` for a job whose inner + // search_operation is SQL. Checking a hardcoded `sql` would let a token scoped only to `sql` start + // an export that its scope excludes. Ahead of the super_user bypass, as on the NoSQL path. + const scopeDenial = tokenScopeDenial(userObject, apiOperation) ?? sqlWriteScopeDenial(userObject, operation); + if (scopeDenial) return scopeDenial; + try { const bucketModule = require('../sqlTranslator/sql_statement_bucket'); const bucket = bucketModule.default || bucketModule; @@ -527,7 +630,7 @@ export function verifyPermsAST(ast, userObject, operation) { * @param operation - The name of the operation specified in the request. * @returns { null | PermissionResponseObject } - null if permissions match, errors are consolidated into PermissionResponseObj. */ -export function verifyPerms(requestJson: any, operation: any, _options?: any) { +export function verifyPerms(requestJson: any, operation: any, options?: { apiOperation?: string }) { if ( requestJson === null || operation === null || @@ -558,6 +661,14 @@ export function verifyPerms(requestJson: any, operation: any, _options?: any) { const permsResponse = new PermissionResponseObject(); + // The top-level API operation the caller invoked — the namespace the policy scope is written in. + // For a job (export_local/export_to_s3), the dispatcher passes the nested search_operation as + // requestJson, so requestJson.operation is the *inner* op (e.g. search_by_conditions); the caller + // threads the real top-level op through `options.apiOperation` so a read-scoped token cannot ride + // an export. Never `op` (the handler name), which would deny deploy_component and conflate aliases. + const scopeDenial = tokenScopeDenial(requestJson.hdb_user, options?.apiOperation ?? requestJson.operation); + if (scopeDenial) return scopeDenial; + if ( commonUtils.isEmptyOrZeroLength(requestJson.hdb_user?.role) || commonUtils.isEmptyOrZeroLength(requestJson.hdb_user?.role?.permission)