diff --git a/npm/src/directory-sync/scim/utils.ts b/npm/src/directory-sync/scim/utils.ts index 064765270..862bed3b5 100644 --- a/npm/src/directory-sync/scim/utils.ts +++ b/npm/src/directory-sync/scim/utils.ts @@ -22,7 +22,10 @@ const parseUserRoles = (roles: string | string[]) => { }; export const parseGroupOperation = (operation: GroupPatchOperation) => { - const { op, path, value } = operation; + const { path, value } = operation; + // Microsoft Entra sends PascalCase ops ("Add"/"Remove"/"Replace") unless the + // tenant enables the aadOptscim062020 flag, so compare case-insensitively. + const op = operation.op?.toLowerCase(); if (path === 'members' && typeof value == 'object') { if (op === 'add') { @@ -80,7 +83,10 @@ export const getDirectorySyncProviders = (): { [K: string]: string } => { // Parse the PATCH request body and return the user attributes (both standard and custom) export const parseUserPatchRequest = (operation: UserPatchOperation) => { - const { op, value, path } = operation; + const { value, path } = operation; + // Entra sends PascalCase ops (e.g. "Remove") unless aadOptscim062020 is set; + // normalize so removals aren't misread as sets. See parseGroupOperation above. + const op = operation.op?.toLowerCase(); const attributes: Partial = {}; const rawAttributes = {}; diff --git a/npm/test/dsync/scim-utils.test.ts b/npm/test/dsync/scim-utils.test.ts new file mode 100644 index 000000000..3c9592d8f --- /dev/null +++ b/npm/test/dsync/scim-utils.test.ts @@ -0,0 +1,50 @@ +import tap from 'tap'; +import { parseGroupOperation, parseUserPatchRequest } from '../../src/directory-sync/scim/utils'; +import type { GroupPatchOperation, UserPatchOperation } from '../../src/directory-sync/types'; + +// Microsoft Entra sends PascalCase SCIM PATCH ops ("Add"/"Remove"/"Replace") +// unless the tenant enables the aadOptscim062020 flag. These bodies arrive off +// the wire, so op can hold values outside the declared lowercase union. +const groupOp = (op: string, path: string, value: unknown) => + parseGroupOperation({ op, path, value } as unknown as GroupPatchOperation); + +tap.test('parseGroupOperation parses SCIM PATCH op case-insensitively', async (t) => { + const members = [{ value: 'u1091' }]; + + t.equal(groupOp('Add', 'members', members).action, 'addGroupMember'); + t.equal(groupOp('Remove', 'members', members).action, 'removeGroupMember'); + t.equal(groupOp('Replace', 'displayName', 'Eng').action, 'updateGroupName'); + + // Lowercase ops (Entra with aadOptscim062020, and other IdPs) still parse. + t.equal(groupOp('add', 'members', members).action, 'addGroupMember'); + t.equal(groupOp('remove', 'members', members).action, 'removeGroupMember'); + t.equal(groupOp('replace', 'displayName', 'Eng').action, 'updateGroupName'); + + // A malformed op-less operation degrades to 'unknown' rather than throwing. + t.equal( + parseGroupOperation({ path: 'members', value: members } as unknown as GroupPatchOperation).action, + 'unknown' + ); + + t.end(); +}); + +const userOp = (op: string, path: string, value?: unknown) => + parseUserPatchRequest({ op, path, value } as unknown as UserPatchOperation); + +tap.test('parseUserPatchRequest recognizes PascalCase "Remove" as a removal', async (t) => { + // Entra remove: the attribute is marked for deletion (REMOVE_SENTINEL) and the + // standard-model field is cleared, regardless of op casing. + for (const op of ['Remove', 'remove']) { + const { attributes, rawAttributes } = userOp(op, 'name.givenName'); + t.equal(attributes.first_name, '', `${op}: standard field cleared`); + t.equal(typeof rawAttributes['name.givenName'], 'symbol', `${op}: raw attr marked for removal`); + } + + // A non-remove op sets the value rather than removing it. + const { attributes, rawAttributes } = userOp('Replace', 'name.givenName', 'Jane'); + t.equal(attributes.first_name, 'Jane'); + t.equal(rawAttributes['name.givenName'], 'Jane'); + + t.end(); +});