diff --git a/.changeset/exact-jwt-permission-masks.md b/.changeset/exact-jwt-permission-masks.md new file mode 100644 index 00000000000..e78a893ba32 --- /dev/null +++ b/.changeset/exact-jwt-permission-masks.md @@ -0,0 +1,5 @@ +--- +'@clerk/shared': patch +--- + +Ensure organization permission checks remain accurate when JWT v2 permission masks exceed JavaScript's safe integer range. diff --git a/packages/shared/src/__tests__/jwtPayloadParser.spec.ts b/packages/shared/src/__tests__/jwtPayloadParser.spec.ts index 014a7292f48..e4bc0290ff4 100644 --- a/packages/shared/src/__tests__/jwtPayloadParser.spec.ts +++ b/packages/shared/src/__tests__/jwtPayloadParser.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from 'vitest'; -import { splitByScope } from '../authorization'; +import { createCheckAuthorization, splitByScope } from '../authorization'; import { __experimental_JWTPayloadToAuthObjectProperties as JWTPayloadToAuthObjectProperties } from '../jwtPayloadParser'; const baseClaims = { @@ -14,6 +14,33 @@ const baseClaims = { __raw: '', }; +const permissionNames = Array.from({ length: 54 }, (_, index) => `permission_${index}`); + +const authFromFeaturePermissionMask = (fpm: string) => { + const authObject = JWTPayloadToAuthObjectProperties({ + ...baseClaims, + v: 2, + fea: 'o:feature', + o: { + id: 'org_id', + rol: 'admin', + per: permissionNames.join(','), + fpm, + }, + }); + const has = createCheckAuthorization({ + userId: authObject.userId, + orgId: authObject.orgId, + orgRole: authObject.orgRole, + orgPermissions: authObject.orgPermissions, + factorVerificationAge: authObject.factorVerificationAge, + features: null, + plans: null, + }); + + return { authObject, has }; +}; + describe('JWTPayloadToAuthObjectProperties', () => { test('auth object with JWT v2 does not produces anything org related if there is no org active', () => { const { sessionClaims: v2Claims, ...signedInAuthObjectV2 } = JWTPayloadToAuthObjectProperties({ @@ -261,6 +288,57 @@ describe('JWTPayloadToAuthObjectProperties', () => { ].sort(), ); }); + + test('preserves permissions above the safe integer boundary', () => { + const { authObject, has } = authFromFeaturePermissionMask('9007199254740993'); + + expect(authObject.orgPermissions).toEqual(['org:feature:permission_0', 'org:feature:permission_53']); + expect(has({ permission: 'org:feature:permission_0' })).toBe(true); + expect(has({ permission: 'org:feature:permission_53' })).toBe(true); + }); + + test('does not introduce permissions when decoding a mask above the safe integer boundary', () => { + const { authObject, has } = authFromFeaturePermissionMask('9007199254740995'); + + expect(authObject.orgPermissions).toEqual([ + 'org:feature:permission_0', + 'org:feature:permission_1', + 'org:feature:permission_53', + ]); + expect(has({ permission: 'org:feature:permission_0' })).toBe(true); + expect(has({ permission: 'org:feature:permission_1' })).toBe(true); + expect(has({ permission: 'org:feature:permission_2' })).toBe(false); + expect(has({ permission: 'org:feature:permission_53' })).toBe(true); + }); + + test('discards mask bits outside the declared permission list', () => { + const { authObject, has } = authFromFeaturePermissionMask('18014398509481984'); + + expect(authObject.orgPermissions).toEqual([]); + expect(has({ permission: 'org:feature:undefined' })).toBe(false); + }); + + test.each([ + ['1', [0]], + ['3', [0, 1]], + ['7', [0, 1, 2]], + ['21', [0, 2, 4]], + ])('preserves permissions for the safe mask %s', (fpm, expectedPermissionIndexes) => { + const { authObject, has } = authFromFeaturePermissionMask(fpm); + const expectedPermissions = expectedPermissionIndexes.map(index => `org:feature:permission_${index}`); + + expect(authObject.orgPermissions).toEqual(expectedPermissions); + for (const permission of expectedPermissions) { + expect(has({ permission })).toBe(true); + } + }); + + test.each(['1invalid', '-1', '1.5'])('fails closed for the malformed mask %s', fpm => { + const { authObject, has } = authFromFeaturePermissionMask(fpm); + + expect(authObject.orgPermissions).toEqual([]); + expect(has({ permission: 'org:feature:permission_0' })).toBe(false); + }); }); describe('splitByScope ', () => { diff --git a/packages/shared/src/jwtPayloadParser.ts b/packages/shared/src/jwtPayloadParser.ts index 0fa7caf35db..233bf2dddcf 100644 --- a/packages/shared/src/jwtPayloadParser.ts +++ b/packages/shared/src/jwtPayloadParser.ts @@ -6,6 +6,42 @@ import type { SharedSignedInAuthObjectProperties, } from './types'; +const decimalToBinaryBits = (decimal: string, minimumLength: number): number[] | undefined => { + if (!/^\d+$/.test(decimal)) { + return undefined; + } + + let remaining = decimal.replace(/^0+/, '') || '0'; + const bits: number[] = []; + + while (remaining !== '0') { + let quotient = ''; + let remainder = 0; + + for (let i = 0; i < remaining.length; i++) { + const value = remainder * 10 + remaining.charCodeAt(i) - 48; + const quotientDigit = Math.floor(value / 2); + + if (quotient || quotientDigit !== 0) { + quotient += quotientDigit; + } + remainder = value % 2; + } + + bits.push(remainder); + remaining = quotient || '0'; + } + + if (bits.length === 0) { + bits.push(0); + } + while (bits.length < minimumLength) { + bits.push(0); + } + + return bits; +}; + export const parsePermissions = ({ per, fpm }: { per?: string; fpm?: string }) => { if (!per || !fpm) { return { permissions: [], featurePermissionMap: [] }; @@ -13,19 +49,9 @@ export const parsePermissions = ({ per, fpm }: { per?: string; fpm?: string }) = const permissions = per.split(',').map(p => p.trim()); - // TODO: make this more efficient const featurePermissionMap = fpm .split(',') - .map(permission => Number.parseInt(permission.trim(), 10)) - .map((permission: number) => - permission - .toString(2) - .padStart(permissions.length, '0') - .split('') - .map(bit => Number.parseInt(bit, 10)) - .reverse(), - ) - .filter(Boolean); + .map(permission => decimalToBinaryBits(permission.trim(), permissions.length) ?? []); return { permissions, featurePermissionMap }; }; @@ -62,7 +88,7 @@ function buildOrgPermissions({ continue; } - for (let permIndex = 0; permIndex < permissionBits.length; permIndex++) { + for (let permIndex = 0; permIndex < permissionBits.length && permIndex < permissions.length; permIndex++) { if (permissionBits[permIndex] === 1) { orgPermissions.push(`org:${feature}:${permissions[permIndex]}`); }