diff --git a/json/systemSchema.json b/json/systemSchema.json index 812c6ea12d..a3161f100a 100644 --- a/json/systemSchema.json +++ b/json/systemSchema.json @@ -494,6 +494,9 @@ { "attribute": "user" }, + { + "attribute": "operations" + }, { "attribute": "enabled" }, diff --git a/security/authn/oidc/tokenExchange.ts b/security/authn/oidc/tokenExchange.ts index 3aa7fcd9f7..99974608a7 100644 --- a/security/authn/oidc/tokenExchange.ts +++ b/security/authn/oidc/tokenExchange.ts @@ -203,9 +203,14 @@ export async function exchangeOidcToken(req: any) { await recordTokenUse(fingerprint, claims, policy.id); const operationToken = await createOperationToken( - { username: user.username, super_user: user.role?.permission?.super_user === true }, + { + 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}`); auditExchange(req, username, AUTH_AUDIT_STATUS.SUCCESS, audit); diff --git a/security/authn/oidc/trustPolicyOperations.ts b/security/authn/oidc/trustPolicyOperations.ts index ff090a5fea..83e6b593a4 100644 --- a/security/authn/oidc/trustPolicyOperations.ts +++ b/security/authn/oidc/trustPolicyOperations.ts @@ -19,6 +19,7 @@ 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; @@ -44,6 +45,19 @@ 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, which accepts group names and operations registered at runtime via + * server.registerOperation; a local OPERATIONS_ENUM check would reject those. + */ +function assertOperationsAreKnown(operations: string[]): void { + const invalidOperation = validateOperations(operations); + if (invalidOperation != null) { + throw new ClientError(`operations contains '${invalidOperation}', which is not a Harper operation`); + } +} + function trustTable() { const table = (databases as any).system?.[OIDC_TRUST_TABLE]; if (!table) { @@ -66,6 +80,7 @@ function toRecord(row: any): OidcTrustPolicy & Record { audience: row.audience, claims: row.claims ?? {}, user: row.user, + operations: row.operations ?? null, enabled: row.enabled !== false, description: row.description ?? null, updated_by: row.updated_by ?? null, @@ -112,6 +127,7 @@ export async function addOidcTrust(req: any) { 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(), enabled: Joi.boolean(), description: Joi.string().allow('').max(1024), }).unknown(true) @@ -123,6 +139,7 @@ export async function addOidcTrust(req: any) { // generic profile rather than a permissive default. Each throws ClientError naming the problem. const profile = profileForIssuer(issuer); profile.assertAudienceIsSpecific(req.audience); + if (req.operations) assertOperationsAreKnown(req.operations); validateClaimConstraintShape(req.claims); profile.assertPolicyIsSpecific(req.claims); @@ -146,6 +163,7 @@ export async function addOidcTrust(req: any) { 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, diff --git a/security/authn/oidc/types.ts b/security/authn/oidc/types.ts index 450d08b4f1..26891667f4 100644 --- a/security/authn/oidc/types.ts +++ b/security/authn/oidc/types.ts @@ -19,6 +19,11 @@ export interface OidcTrustPolicy { claims: Record; /** The exchanged token authenticates as this user, whose role is the least-privilege boundary. */ user: string; + /** + * Optional narrowing of that boundary: the minted token may perform only these operations, even + * where the role allows more. Never widens — an operation the role forbids stays forbidden. + */ + operations?: string[]; /** Defaults to true; false keeps the policy for reference without honoring it. */ enabled?: boolean; description?: string; diff --git a/security/impersonation.ts b/security/impersonation.ts index 4d2ea3e14e..de55d31785 100644 --- a/security/impersonation.ts +++ b/security/impersonation.ts @@ -38,6 +38,12 @@ 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. + const inheritedScope = (authenticatedUser as any).tokenOperations; + if (Array.isArray(inheritedScope)) (impersonatedUser as any).tokenOperations = inheritedScope; + // Tag for audit trail impersonatedUser._impersonated = true; impersonatedUser._impersonatedBy = authenticatedUser.username; diff --git a/security/tokenAuthentication.ts b/security/tokenAuthentication.ts index c724d97d9e..7556ca82d3 100644 --- a/security/tokenAuthentication.ts +++ b/security/tokenAuthentication.ts @@ -162,9 +162,18 @@ export async function createTokens(authObj: AuthObject): Promise { username: string; super_user: boolean; role?: any; + operations?: string[]; } = { username: authObj.username, super_user: superUser }; if (authObj.role) payload.role = authObj.role; + // A scoped credential can only mint an equally-scoped one (#2174). create_authentication_tokens is + // NO_AUTH, so verifyPerms — and the token-scope gate inside it — never runs here; if the caller + // authenticated with a scoped operation token, the operation AND refresh tokens it mints inherit + // that scope. Without this, a token scoped to e.g. deploy_component escalates to unscoped, full-role + // credentials simply by calling create_authentication_tokens with no username/password. + const inheritedScope = (authObj.hdb_user as any)?.tokenOperations; + if (Array.isArray(inheritedScope)) payload.operations = inheritedScope; + const keys: JWTRSAKeys = await getJWTRSAKeys(); if (authObj.purpose === 'login') { @@ -233,8 +242,15 @@ 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. Absent means + // the role governs alone, which is every token minted before this existed. + // + // `!= null` rather than a truthiness check on purpose: an EMPTY scope means "no operations", and + // a length check would drop it from the payload, leaving the token unscoped — a security control + // failing open. add_oidc_trust rejects an empty array, but a row can reach the table by + // replication from a peer, so this must not depend on that. + if (user.operations != null) payload.operations = user.operations; + return jwt.sign( - { username: user.username, super_user: user.super_user }, + payload, { key: keys.privateKey, passphrase: keys.passphrase } satisfies Secret, { expiresIn, @@ -308,6 +337,12 @@ async function validateToken(token: string, tokenType: string): Promise { throw new Error('Invalid token'); } + // Surfaced on the user 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 rather than narrow. verifyPerms intersects this + // separately, ahead of every bypass. + if (Array.isArray(tokenVerified.operations)) user.tokenOperations = tokenVerified.operations; + return user; } catch (err) { logger.warn(err); diff --git a/unitTests/security/authn/oidc/tokenExchange.test.js b/unitTests/security/authn/oidc/tokenExchange.test.js index 84d21e5007..a32ba8196b 100644 --- a/unitTests/security/authn/oidc/tokenExchange.test.js +++ b/unitTests/security/authn/oidc/tokenExchange.test.js @@ -438,6 +438,24 @@ describe('exchangeOidcToken', () => { }); }); + // 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']) { diff --git a/unitTests/security/authn/oidc/trustPolicyOperations.test.js b/unitTests/security/authn/oidc/trustPolicyOperations.test.js index b2e5be36ad..5615a2f8cf 100644 --- a/unitTests/security/authn/oidc/trustPolicyOperations.test.js +++ b/unitTests/security/authn/oidc/trustPolicyOperations.test.js @@ -17,6 +17,7 @@ const { 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'; @@ -188,6 +189,44 @@ describe('oidc trustPolicyOperations', () => { 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'); + }); + + // Operations registered at runtime via server.registerOperation are grantable in a role's + // allowlist, so a policy must be able to scope to them too — this is why validation delegates + // to validateOperations rather than checking OPERATIONS_ENUM locally. + it('accepts a dynamically registered operation', 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 })))); diff --git a/unitTests/security/impersonation.test.js b/unitTests/security/impersonation.test.js index de7395af06..c39372e80b 100644 --- a/unitTests/security/impersonation.test.js +++ b/unitTests/security/impersonation.test.js @@ -603,4 +603,23 @@ 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']); + }); + + 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/tokenAuthentication.test.js b/unitTests/security/tokenAuthentication.test.js index 9cafb18a9a..5bce6755e7 100644 --- a/unitTests/security/tokenAuthentication.test.js +++ b/unitTests/security/tokenAuthentication.test.js @@ -441,6 +441,35 @@ describe('test createTokens', () => { rw_get_tokens(); }); + // #2174: create_authentication_tokens is NO_AUTH, so verifyPerms — and the token-scope gate + // inside it — never runs here. A caller authenticated with a scoped operation token must not be + // able to mint UNSCOPED credentials; the minted operation and refresh tokens inherit the scope. + it('carries an inherited token scope into the minted operation and refresh tokens', async () => { + let rw = token_auth.__set__( + 'getJWTRSAKeys', + async () => new JWTRSAKeys(PUBLIC_KEY_VALUE, PRIVATE_KEY_VALUE, PASSPHRASE_VALUE) + ); + // No username/password: the caller is the already-authenticated bearer, whose hdb_user carries + // the scope — exactly the shape a scoped OIDC token presents to create_authentication_tokens. + let result = await token_auth.createTokens({ + hdb_user: { username: 'HDB_USER', tokenOperations: ['deploy_component'] }, + }); + assert.deepStrictEqual(jwt.decode(result.operation_token).operations, ['deploy_component']); + assert.deepStrictEqual(jwt.decode(result.refresh_token).operations, ['deploy_component']); + rw(); + }); + + it('mints unscoped credentials when the caller is unscoped', async () => { + let rw = token_auth.__set__( + 'getJWTRSAKeys', + async () => new JWTRSAKeys(PUBLIC_KEY_VALUE, PRIVATE_KEY_VALUE, PASSPHRASE_VALUE) + ); + let result = await token_auth.createTokens({ username: 'HDB_USER', password: 'pass' }); + assert.strictEqual(jwt.decode(result.operation_token).operations, undefined); + assert.strictEqual(jwt.decode(result.refresh_token).operations, undefined); + rw(); + }); + it('test update failed', async () => { update_stub.callsFake(async (_update_object) => { throw Error('update failed'); diff --git a/unitTests/security/tokenOperationScope.test.js b/unitTests/security/tokenOperationScope.test.js new file mode 100644 index 0000000000..7830e5d7eb --- /dev/null +++ b/unitTests/security/tokenOperationScope.test.js @@ -0,0 +1,132 @@ +'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. +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) { + const parsed = sql.convertSQLToAST(statement); + return sql.checkASTPermissions({ operation: 'sql', 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'); + }); + + // 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); + }); + + it('is inert for a token with no scope', () => { + const denial = checkSql('SELECT * FROM data.dog', userWithScope({ super_user: true })); + assert.strictEqual(denial, null); + }); +}); 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/utility/operation_authorization.ts b/utility/operation_authorization.ts index 8c3f09a8b8..cd217309ec 100644 --- a/utility/operation_authorization.ts +++ b/utility/operation_authorization.ts @@ -442,6 +442,49 @@ module.exports = { * @param operation - The operation specified in the call. * @returns {null | PermissionResponseObject} - null if permissions match, errors returned in the PermissionResponseObject */ +/** + * 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. + * + * `op` is the raw operation (a handler function name, or the literal `sql`); its snake_case api_name + * is resolved here rather than by the caller, so the unscoped default path — every request that is + * not a scoped token — does no registry lookup and no allocation before the `== null` return. + */ +function tokenScopeDenial(userObject: any, op: 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; + + const scopedOps = + userObject._expandedTokenOperations ?? + (userObject._expandedTokenOperations = expandOperationsPerms(tokenOperations)); + const opApiName = requiredPermissions.get(op)?.api_name ?? op; + if (scopedOps.has(opApiName)) return undefined; + + harperLogger.info(`Operation '${opApiName}' is outside the scope of the presented token`); + return new PermissionResponseObject().handleUnauthorizedItem(HDB_ERROR_MSGS.OP_NOT_IN_OPERATIONS(opApiName)); +} + export function verifyPermsAST(ast, userObject, operation) { //TODO - update these validation checks to use validate.js if (commonUtils.isEmptyOrZeroLength(ast)) { @@ -456,6 +499,13 @@ 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, so the + // scope is checked against `sql` — the operation the caller actually invoked. Ahead of the AST + // parsing below as well as the super_user bypass: a denied scope should not parse attacker SQL. + const scopeDenial = tokenScopeDenial(userObject, terms.OPERATIONS_ENUM.SQL); + if (scopeDenial) return scopeDenial; + try { const bucketModule = require('../sqlTranslator/sql_statement_bucket'); const bucket = bucketModule.default || bucketModule; @@ -582,6 +632,9 @@ export function verifyPerms(requestJson: any, operation: any, _options?: any) { const permsResponse = new PermissionResponseObject(); + const scopeDenial = tokenScopeDenial(requestJson.hdb_user, op); + if (scopeDenial) return scopeDenial; + if ( commonUtils.isEmptyOrZeroLength(requestJson.hdb_user?.role) || commonUtils.isEmptyOrZeroLength(requestJson.hdb_user?.role?.permission)