Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions json/systemSchema.json
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,9 @@
{
"attribute": "user"
},
{
"attribute": "operations"
},
{
"attribute": "enabled"
},
Expand Down
7 changes: 6 additions & 1 deletion security/authn/oidc/tokenExchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 18 additions & 0 deletions security/authn/oidc/trustPolicyOperations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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`);
}
}
Comment thread
dawsontoth marked this conversation as resolved.

function trustTable() {
const table = (databases as any).system?.[OIDC_TRUST_TABLE];
if (!table) {
Expand All @@ -66,6 +80,7 @@ function toRecord(row: any): OidcTrustPolicy & Record<string, unknown> {
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,
Expand Down Expand Up @@ -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)
Expand All @@ -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);

Expand All @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions security/authn/oidc/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ export interface OidcTrustPolicy {
claims: Record<string, ClaimConstraint>;
/** 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;
Expand Down
6 changes: 6 additions & 0 deletions security/impersonation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
41 changes: 38 additions & 3 deletions security/tokenAuthentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,18 @@ export async function createTokens(authObj: AuthObject): Promise<JWTTokens> {
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;
Comment thread
dawsontoth marked this conversation as resolved.

const keys: JWTRSAKeys = await getJWTRSAKeys();

if (authObj.purpose === 'login') {
Expand Down Expand Up @@ -233,8 +242,15 @@ export async function refreshOperationToken(tokenObj: TokenObject): Promise<JWTT

const keys: JWTRSAKeys = await getJWTRSAKeys();
const decodedJWT = jwt.decode(refresh_token, { json: true });
const refreshedPayload: { username: string; super_user: boolean; operations?: string[] } = {
username: decodedJWT.username,
super_user: decodedJWT.super_user,
};
// Carry the scope forward, same invariant as createTokens: once a scoped credential can produce a
// scoped refresh token, refreshing it must not widen back to the full role.
if (Array.isArray(decodedJWT.operations)) refreshedPayload.operations = decodedJWT.operations;
const operationToken = jwt.sign(
{ username: decodedJWT.username, super_user: decodedJWT.super_user },
refreshedPayload,
{ key: keys.privateKey, passphrase: keys.passphrase } satisfies Secret,
{
expiresIn: OPERATION_TOKEN_TIMEOUT as StringValue,
Expand All @@ -257,12 +273,25 @@ export async function refreshOperationToken(tokenObj: TokenObject): Promise<JWTT
* to this token — nothing here re-checks that.
*/
export async function createOperationToken(
user: { username: string; super_user: boolean },
user: { username: string; super_user: boolean; operations?: string[] },
expiresIn: StringValue
): Promise<string> {
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,
Expand Down Expand Up @@ -308,6 +337,12 @@ async function validateToken(token: string, tokenType: string): Promise<any> {
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);
Expand Down
18 changes: 18 additions & 0 deletions unitTests/security/authn/oidc/tokenExchange.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']) {
Expand Down
39 changes: 39 additions & 0 deletions unitTests/security/authn/oidc/trustPolicyOperations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 }))));
Expand Down
19 changes: 19 additions & 0 deletions unitTests/security/impersonation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
29 changes: 29 additions & 0 deletions unitTests/security/tokenAuthentication.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Loading
Loading