Skip to content

Commit cfcbceb

Browse files
author
Sylvester Damgaard
committed
fix(manifest): canonicalize the RBAC content-hash to match the reference byte-for-byte
The manifest content-hash diverged from the PHP reference and the other SDKs (key ordering, empty-description handling, unicode escaping), breaking the 'unchanged = no-op' idempotency of manifest publish. Canonicalize to the reference and lock it with a shared cross-SDK fixture. Also adds negative id_token tests (wrong-key, tampered payload, expired) so a regression that skipped signature verification fails CI. Release v0.6.0.
1 parent 411da92 commit cfcbceb

6 files changed

Lines changed: 280 additions & 9 deletions

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@cboxdk/id-js",
3-
"version": "0.5.1",
3+
"version": "0.6.0",
44
"description": "Turnkey Cbox ID client for JavaScript/TypeScript — OpenID Connect login (PKCE + id_token verification via JWKS), hosted profile-management redirect, machine tokens, UserInfo, RFC 7662 introspection, and webhook signature verification. Runs on Node, edge, and the browser, with a first-class Next.js adapter.",
55
"type": "module",
66
"license": "MIT",

src/authz.ts

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ export async function buildManifest(declaration: AuthzDeclaration): Promise<Auth
9191
const roles = (declaration.roles ?? []).map(canonicalRole);
9292
assertDeclaration(permissions, roles);
9393

94-
permissions.sort((a, b) => a.key.localeCompare(b.key));
95-
roles.sort((a, b) => a.key.localeCompare(b.key));
94+
permissions.sort((a, b) => byteCompare(a.key, b.key));
95+
roles.sort((a, b) => byteCompare(a.key, b.key));
9696

97-
const version = (await sha256Hex(JSON.stringify({ permissions, roles }))).slice(0, 16);
97+
const version = (await sha256Hex(canonicalManifestJson(permissions, roles))).slice(0, 16);
9898
return { version, permissions, roles };
9999
}
100100

@@ -219,12 +219,66 @@ function canonicalPermission(permission: PermissionDefinition): PermissionDefini
219219

220220
/** A role with keys in a fixed order, sorted permission refs, and no `undefined` fields. */
221221
function canonicalRole(role: RoleDefinition): RoleDefinition {
222-
const permissions = [...role.permissions].sort((a, b) => a.localeCompare(b));
222+
const permissions = [...role.permissions].sort(byteCompare);
223223
return role.description === undefined
224224
? { key: role.key, name: role.name, permissions }
225225
: { key: role.key, name: role.name, description: role.description, permissions };
226226
}
227227

228+
/**
229+
* Serialize {permissions, roles} to the exact canonical JSON the PHP reference hashes,
230+
* so the `version` is byte-for-byte identical across every Cbox ID SDK. Matches PHP
231+
* `json_encode` defaults: object keys in insertion order, permissions and roles sorted
232+
* by key, each role's permission refs sorted, an absent-or-empty description emitted as
233+
* `null`, forward slashes escaped as `\/`, and every non-ASCII code unit as `\uXXXX`.
234+
*/
235+
export function canonicalManifestJson(
236+
permissions: PermissionDefinition[],
237+
roles: RoleDefinition[],
238+
): string {
239+
const canonical = {
240+
permissions: [...permissions]
241+
.sort((a, b) => byteCompare(a.key, b.key))
242+
.map((p) => ({ key: p.key, description: emptyToNull(p.description) })),
243+
roles: [...roles]
244+
.sort((a, b) => byteCompare(a.key, b.key))
245+
.map((r) => ({
246+
key: r.key,
247+
name: r.name,
248+
description: emptyToNull(r.description),
249+
permissions: [...r.permissions].sort(byteCompare),
250+
})),
251+
};
252+
return escapeLikePhp(JSON.stringify(canonical));
253+
}
254+
255+
/** PHP treats an absent or empty description as `null` in the hashed catalog. */
256+
function emptyToNull(value: string | undefined): string | null {
257+
return value === undefined || value === '' ? null : value;
258+
}
259+
260+
/** Byte-wise (code-unit) comparison — matches PHP `strcmp` on the ASCII-only keys. */
261+
function byteCompare(a: string, b: string): number {
262+
return a < b ? -1 : a > b ? 1 : 0;
263+
}
264+
265+
/** Apply PHP `json_encode`'s default `\/` slash and `\uXXXX` non-ASCII escaping. */
266+
function escapeLikePhp(json: string): string {
267+
let out = '';
268+
for (let i = 0; i < json.length; i++) {
269+
const char = json.charAt(i);
270+
const code = json.charCodeAt(i);
271+
if (char === '/') {
272+
out += '\\/';
273+
} else if (code > 0x7f) {
274+
out += '\\u' + code.toString(16).padStart(4, '0');
275+
} else {
276+
out += char;
277+
}
278+
}
279+
return out;
280+
}
281+
228282
async function sha256Hex(input: string): Promise<string> {
229283
const digest = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(input));
230284
return Array.from(new Uint8Array(digest))

test/authz.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,32 @@
1+
import { readFileSync } from 'node:fs';
2+
import { fileURLToPath } from 'node:url';
13
import { afterEach, describe, expect, it, vi } from 'vitest';
24
import {
35
buildManifest,
46
ConfigurationError,
57
defineAuthz,
68
publishManifest,
79
} from '../src/index.js';
10+
import { canonicalManifestJson } from '../src/authz.js';
811
import { discovery, ISSUER } from './helpers.js';
912

13+
interface FixtureCase {
14+
name: string;
15+
permissions: { key: string; description: string | null }[];
16+
roles: { key: string; name: string; description: string | null; permissions: string[] }[];
17+
canonical_json: string;
18+
sha256: string;
19+
version: string;
20+
}
21+
22+
// The shared cross-SDK fixture: manifests + their canonical JSON and hash, generated
23+
// from the PHP reference (Cbox\Id\AccessControl\Manifest\Manifest::checksum). id-js,
24+
// id-python, id-go and laravel-id all assert against this same file so the four stay
25+
// byte-for-byte locked together.
26+
const fixture = JSON.parse(
27+
readFileSync(fileURLToPath(new URL('./fixtures/manifest-hash.json', import.meta.url)), 'utf8'),
28+
) as { cases: FixtureCase[] };
29+
1030
afterEach(() => {
1131
vi.unstubAllGlobals();
1232
});
@@ -63,6 +83,29 @@ describe('buildManifest', () => {
6383
});
6484
});
6585

86+
describe('cross-SDK manifest hash fixture', () => {
87+
for (const testCase of fixture.cases) {
88+
it(`matches the PHP reference canonical hash: ${testCase.name}`, async () => {
89+
// A null description in the fixture means "not declared" — omit the field.
90+
const permissions = testCase.permissions.map((p) =>
91+
p.description === null ? { key: p.key } : { key: p.key, description: p.description },
92+
);
93+
const roles = testCase.roles.map((r) =>
94+
r.description === null
95+
? { key: r.key, name: r.name, permissions: r.permissions }
96+
: { key: r.key, name: r.name, description: r.description, permissions: r.permissions },
97+
);
98+
99+
// Byte-for-byte identical canonical serialization to PHP's json_encode.
100+
expect(canonicalManifestJson(permissions, roles)).toBe(testCase.canonical_json);
101+
// The SDK's own sha256 of those bytes, truncated to 16 hex, matches the fixture.
102+
const manifest = await buildManifest({ permissions, roles });
103+
expect(manifest.version).toBe(testCase.version);
104+
expect(testCase.version).toBe(testCase.sha256.slice(0, 16));
105+
});
106+
}
107+
});
108+
66109
describe('defineAuthz', () => {
67110
it('rejects a role that grants an undeclared permission', () => {
68111
expect(() =>

test/client.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,67 @@ describe('authenticate', () => {
185185
client.authenticate({ params: { code: 'auth-code', state: 'state-1' }, stored }),
186186
).rejects.toBeInstanceOf(AuthenticationError);
187187
});
188+
189+
it('rejects an id_token signed by a key the JWKS does not advertise', async () => {
190+
const inst = await fakeInstance();
191+
// Signed with a foreign keypair; the JWKS only advertises the real key, so the
192+
// signature must fail to verify. A regression that skipped the signature check
193+
// would let this token through.
194+
const forged = await inst.foreignIdToken({
195+
iss: ISSUER,
196+
aud: 'client-abc',
197+
sub: 'user-1',
198+
nonce: NONCE,
199+
});
200+
inst.setTokenResponse({ access_token: 'access-abc', id_token: forged });
201+
vi.stubGlobal('fetch', inst.fetchMock);
202+
const client = new CboxIdClient(baseConfig);
203+
204+
await expect(
205+
client.authenticate({ params: { code: 'auth-code', state: 'state-1' }, stored }),
206+
).rejects.toBeInstanceOf(AuthenticationError);
207+
});
208+
209+
it('rejects an id_token whose payload was tampered after signing', async () => {
210+
const inst = await fakeInstance();
211+
const valid = await inst.signIdToken({
212+
iss: ISSUER,
213+
aud: 'client-abc',
214+
sub: 'user-1',
215+
nonce: NONCE,
216+
});
217+
// Escalate the subject but re-attach the ORIGINAL signature — verification must reject.
218+
const [header, payload, signature] = valid.split('.');
219+
const decoded = JSON.parse(Buffer.from(payload!, 'base64url').toString('utf8')) as Record<
220+
string,
221+
unknown
222+
>;
223+
decoded['sub'] = 'attacker';
224+
const tampered = `${header}.${Buffer.from(JSON.stringify(decoded)).toString('base64url')}.${signature}`;
225+
inst.setTokenResponse({ access_token: 'access-abc', id_token: tampered });
226+
vi.stubGlobal('fetch', inst.fetchMock);
227+
const client = new CboxIdClient(baseConfig);
228+
229+
await expect(
230+
client.authenticate({ params: { code: 'auth-code', state: 'state-1' }, stored }),
231+
).rejects.toBeInstanceOf(AuthenticationError);
232+
});
233+
234+
it('rejects an expired id_token', async () => {
235+
const inst = await fakeInstance();
236+
const past = Math.floor(Date.now() / 1000) - 60;
237+
const expired = await inst.signIdToken(
238+
{ iss: ISSUER, aud: 'client-abc', sub: 'user-1', nonce: NONCE },
239+
{ expiresAt: past, issuedAt: past - 300 },
240+
);
241+
inst.setTokenResponse({ access_token: 'access-abc', id_token: expired });
242+
vi.stubGlobal('fetch', inst.fetchMock);
243+
const client = new CboxIdClient(baseConfig);
244+
245+
await expect(
246+
client.authenticate({ params: { code: 'auth-code', state: 'state-1' }, stored }),
247+
).rejects.toBeInstanceOf(AuthenticationError);
248+
});
188249
});
189250

190251
describe('refresh', () => {

test/fixtures/manifest-hash.json

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
{
2+
"cases": [
3+
{
4+
"name": "empty",
5+
"permissions": [],
6+
"roles": [],
7+
"canonical_json": "{\"permissions\":[],\"roles\":[]}",
8+
"sha256": "0d1152d7f8bb231b10af8f84c60d2630f94c3c8a0ad8f4fa58eb3b50165b4a1d",
9+
"version": "0d1152d7f8bb231b"
10+
},
11+
{
12+
"name": "basic",
13+
"permissions": [
14+
{
15+
"key": "invoices:read",
16+
"description": "View invoices"
17+
},
18+
{
19+
"key": "invoices:create",
20+
"description": "Create invoices"
21+
},
22+
{
23+
"key": "invoices:refund",
24+
"description": "Refund invoices"
25+
}
26+
],
27+
"roles": [
28+
{
29+
"key": "viewer",
30+
"name": "Viewer",
31+
"description": "Read-only",
32+
"permissions": [
33+
"invoices:read"
34+
]
35+
},
36+
{
37+
"key": "billing-admin",
38+
"name": "Billing Admin",
39+
"description": "Full billing access",
40+
"permissions": [
41+
"invoices:refund",
42+
"invoices:create",
43+
"invoices:read"
44+
]
45+
}
46+
],
47+
"canonical_json": "{\"permissions\":[{\"key\":\"invoices:create\",\"description\":\"Create invoices\"},{\"key\":\"invoices:read\",\"description\":\"View invoices\"},{\"key\":\"invoices:refund\",\"description\":\"Refund invoices\"}],\"roles\":[{\"key\":\"billing-admin\",\"name\":\"Billing Admin\",\"description\":\"Full billing access\",\"permissions\":[\"invoices:create\",\"invoices:read\",\"invoices:refund\"]},{\"key\":\"viewer\",\"name\":\"Viewer\",\"description\":\"Read-only\",\"permissions\":[\"invoices:read\"]}]}",
48+
"sha256": "d3f48d571dd697b60eb72e656dee724f8550639f80e7a6c1a659346d06f00179",
49+
"version": "d3f48d571dd697b6"
50+
},
51+
{
52+
"name": "edge_cases",
53+
"permissions": [
54+
{
55+
"key": "reports:export",
56+
"description": "Export reports as CSV/PDF"
57+
},
58+
{
59+
"key": "reports:read",
60+
"description": null
61+
},
62+
{
63+
"key": "accounts:read",
64+
"description": "Se konti — 日本語 café 😀"
65+
}
66+
],
67+
"roles": [
68+
{
69+
"key": "analyst",
70+
"name": "Analyst",
71+
"description": null,
72+
"permissions": [
73+
"reports:read",
74+
"reports:export",
75+
"accounts:read"
76+
]
77+
},
78+
{
79+
"key": "admin",
80+
"name": "Administrator",
81+
"description": "Everything, incl. I/O",
82+
"permissions": [
83+
"reports:export"
84+
]
85+
}
86+
],
87+
"canonical_json": "{\"permissions\":[{\"key\":\"accounts:read\",\"description\":\"Se konti \\u2014 \\u65e5\\u672c\\u8a9e caf\\u00e9 \\ud83d\\ude00\"},{\"key\":\"reports:export\",\"description\":\"Export reports as CSV\\/PDF\"},{\"key\":\"reports:read\",\"description\":null}],\"roles\":[{\"key\":\"admin\",\"name\":\"Administrator\",\"description\":\"Everything, incl. I\\/O\",\"permissions\":[\"reports:export\"]},{\"key\":\"analyst\",\"name\":\"Analyst\",\"description\":null,\"permissions\":[\"accounts:read\",\"reports:export\",\"reports:read\"]}]}",
88+
"sha256": "b4dd2edc5c58332a5444378e4d0663b7eac219aa31bc126e52811ea887aa9d07",
89+
"version": "b4dd2edc5c58332a"
90+
}
91+
]
92+
}

test/helpers.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,19 @@ function json(body: unknown, status = 200): Response {
2222

2323
export const NONCE = 'test-nonce';
2424

25+
/** Optional overrides for the timestamps a signed id_token carries. */
26+
export interface SignOptions {
27+
/** `exp` — an epoch-seconds number or a jose duration string (default `'5m'`). */
28+
expiresAt?: number | string;
29+
/** `iat` — epoch seconds (default: now). */
30+
issuedAt?: number;
31+
}
32+
2533
export interface FakeInstance {
2634
jwk: JWK;
27-
signIdToken(claims: Record<string, unknown>): Promise<string>;
35+
signIdToken(claims: Record<string, unknown>, opts?: SignOptions): Promise<string>;
36+
/** Sign an id_token with a DIFFERENT key than the JWKS advertises (kid still `test-key`). */
37+
foreignIdToken(claims: Record<string, unknown>): Promise<string>;
2838
/** Replace what the token endpoint returns for an authorization_code exchange. */
2939
setTokenResponse(response: Record<string, unknown>): void;
3040
fetchMock: ReturnType<typeof vi.fn>;
@@ -48,12 +58,23 @@ export async function fakeInstance(
4858
jwk.alg = 'RS256';
4959
jwk.use = 'sig';
5060

51-
const signIdToken = (claims: Record<string, unknown>): Promise<string> =>
61+
const signIdToken = (claims: Record<string, unknown>, opts: SignOptions = {}): Promise<string> =>
5262
new SignJWT(claims)
63+
.setProtectedHeader({ alg: 'RS256', kid: 'test-key' })
64+
.setIssuedAt(opts.issuedAt)
65+
.setExpirationTime(opts.expiresAt ?? '5m')
66+
.sign(privateKey);
67+
68+
// A token signed by a foreign keypair but presenting the advertised kid, so the
69+
// verifier picks the real JWKS key and the signature check must fail.
70+
const foreignIdToken = async (claims: Record<string, unknown>): Promise<string> => {
71+
const foreign = await generateKeyPair('RS256', { extractable: true });
72+
return new SignJWT(claims)
5373
.setProtectedHeader({ alg: 'RS256', kid: 'test-key' })
5474
.setIssuedAt()
5575
.setExpirationTime('5m')
56-
.sign(privateKey);
76+
.sign(foreign.privateKey);
77+
};
5778

5879
const defaultIdToken = await signIdToken({
5980
iss: ISSUER,
@@ -104,5 +125,5 @@ export async function fakeInstance(
104125
tokenResponse = response;
105126
};
106127

107-
return { jwk, signIdToken, setTokenResponse, fetchMock };
128+
return { jwk, signIdToken, foreignIdToken, setTokenResponse, fetchMock };
108129
}

0 commit comments

Comments
 (0)