fix(shared): decode JWT v2 permission masks exactly (#9381)

Author: jeremy-clerk

.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.

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({
@@ -75,7 +102,7 @@ describe('JWTPayloadToAuthObjectProperties', () => {
     );
   });
 
-  test('if a feature is not mapped to any permissions it is added as is to the orgPermissions array', () => {
+  test('features without permissions use zero masks to preserve alignment', () => {
     const { sessionClaims: v2Claims, ...signedInAuthObject } = JWTPayloadToAuthObjectProperties({
       ...baseClaims,
       v: 2,
@@ -85,7 +112,7 @@ describe('JWTPayloadToAuthObjectProperties', () => {
         rol: 'admin',
         slg: 'org_slug',
         per: 'read,manage',
-        fpm: '1,3',
+        fpm: '1,3,0',
       },
     });
 
@@ -153,7 +180,7 @@ describe('JWTPayloadToAuthObjectProperties', () => {
         rol: 'admin',
         slg: 'org_slug',
         per: 'read,manage',
-        fpm: '3',
+        fpm: '3,0',
       },
     });
 
@@ -227,7 +254,7 @@ describe('JWTPayloadToAuthObjectProperties', () => {
         rol: 'admin',
         slg: 'org_slug',
         per: 'read,manage',
-        fpm: '1,2,3',
+        fpm: '1,2,3,0',
       },
     });
 
@@ -246,7 +273,7 @@ describe('JWTPayloadToAuthObjectProperties', () => {
         rol: 'admin',
         slg: 'org_slug',
         per: 'read,create,update,delete,revoke',
-        fpm: '7,21',
+        fpm: '7,21,0',
       },
     });
 
@@ -261,6 +288,89 @@ 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('keeps permission masks aligned when a targeted feature follows a feature without permissions', () => {
+    const authObject = JWTPayloadToAuthObjectProperties({
+      ...baseClaims,
+      v: 2,
+      fea: 'o:repositories,o:impersonation,o:billing',
+      o: {
+        id: 'org_id',
+        rol: 'admin',
+        per: 'manage,read,update',
+        fpm: '6,0,3',
+      },
+    });
+    const has = createCheckAuthorization({
+      userId: authObject.userId,
+      orgId: authObject.orgId,
+      orgRole: authObject.orgRole,
+      orgPermissions: authObject.orgPermissions,
+      factorVerificationAge: authObject.factorVerificationAge,
+      features: null,
+      plans: null,
+    });
+
+    expect(authObject.orgPermissions).toEqual([
+      'org:repositories:read',
+      'org:repositories:update',
+      'org:billing:manage',
+      'org:billing:read',
+    ]);
+    expect(has({ permission: 'org:impersonation:manage' })).toBe(false);
+    expect(has({ permission: 'org:billing:manage' })).toBe(true);
+  });
+
+  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 ', () => {

packages/shared/src/jwtPayloadParser.ts

@@ -6,26 +6,52 @@ 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: [] };
   }
 
   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]}`);
       }