fix: scim remove op with filters

GitOrigin-RevId: e1c1afb9b2a98f1e5347e3b97868f9f326b37f09

Author: pi1814

npm/src/directory-sync/scim/utils.ts

@@ -96,6 +96,11 @@ export const parseUserPatchRequest = (operation: UserPatchOperation) => {
   if (op === 'remove' && path) {
     rawAttributes[path] = REMOVE_SENTINEL;
 
+    // Propagate removal to standard user model attributes
+    if (path in attributesMap) {
+      attributes[attributesMap[path]] = '';
+    }
+
     return {
       attributes,
       rawAttributes,
@@ -156,11 +161,17 @@ export const extractStandardUserAttributes = (body: any) => {
   return userAttributes;
 };
 
+// Match SCIM filter paths with a sub-attribute: `phoneNumbers[type eq "work"].value`
+const SCIM_FILTER_SUB_ATTR_RE = /^(\w+)\[(\w+)\s+eq\s+"([^"]+)"\]\.(\w+)$/;
+
+// Match SCIM filter paths without a sub-attribute: `phoneNumbers[type eq "work"]`
+const SCIM_FILTER_RE = /^(\w+)\[(\w+)\s+eq\s+"([^"]+)"\]$/;
+
 // Resolve a SCIM filter path like `phoneNumbers[type eq "work"].value` to a
 // lodash-compatible path like `["phoneNumbers", 0, "value"]`. Returns false for
 // non-filter paths so the caller can fall back to plain _.set.
 const applySCIMFilterUpdate = (raw: any, path: string, value: any): boolean => {
-  const match = path.match(/^(\w+)\[(\w+)\s+eq\s+"([^"]+)"\]\.(\w+)$/);
+  const match = path.match(SCIM_FILTER_SUB_ATTR_RE);
   if (!match) {
     return false;
   }
@@ -188,6 +199,49 @@ const applySCIMFilterUpdate = (raw: any, path: string, value: any): boolean => {
   return true;
 };
 
+// Remove matching entries from a multi-valued attribute using a SCIM filter path.
+// Handles two forms:
+//   - `attr[filter]`         → remove entire matching array entries
+//   - `attr[filter].subAttr` → unset only the sub-attribute from matching entries
+// Returns false for non-filter paths so the caller can fall back to _.unset.
+const removeSCIMFilterPath = (raw: any, path: string): boolean => {
+  // Try sub-attribute form first: phoneNumbers[type eq "work"].value
+  const subMatch = path.match(SCIM_FILTER_SUB_ATTR_RE);
+  if (subMatch) {
+    const [, attribute, filterAttr, filterValue, subAttribute] = subMatch;
+    const arr = _.get(raw, attribute);
+    if (!Array.isArray(arr)) {
+      return true; // Nothing to remove — already absent
+    }
+
+    for (const el of arr) {
+      if (el[filterAttr] === filterValue) {
+        delete el[subAttribute];
+      }
+    }
+    return true;
+  }
+
+  // Try whole-entry form: phoneNumbers[type eq "work"]
+  const match = path.match(SCIM_FILTER_RE);
+  if (match) {
+    const [, attribute, filterAttr, filterValue] = match;
+    const arr = _.get(raw, attribute);
+    if (!Array.isArray(arr)) {
+      return true; // Nothing to remove — already absent
+    }
+
+    _.set(
+      raw,
+      attribute,
+      arr.filter((el: any) => el[filterAttr] !== filterValue)
+    );
+    return true;
+  }
+
+  return false;
+};
+
 // Update raw user attributes
 export const updateRawUserAttributes = (raw, attributes) => {
   const keys = Object.keys(attributes);
@@ -210,11 +264,16 @@ export const updateRawUserAttributes = (raw, attributes) => {
           // Path format: "urn:...:User:attribute" — remove the attribute from the schema object.
           const i = key.lastIndexOf(':');
           const urn = key.substring(0, i);
+          const attrName = key.substring(i + 1);
           if (raw[urn] && typeof raw[urn] === 'object') {
-            delete raw[urn][key.substring(i + 1)];
+            // If the attribute contains a SCIM filter (e.g. "roles[value eq ...]"),
+            // delegate to removeSCIMFilterPath scoped to the schema sub-object.
+            if (!removeSCIMFilterPath(raw[urn], attrName)) {
+              delete raw[urn][attrName];
+            }
           }
         }
-      } else {
+      } else if (!removeSCIMFilterPath(raw, key)) {
         _.unset(raw, key);
       }
       continue;

npm/test/dsync/data/user-requests.ts

@@ -259,6 +259,30 @@ const requests = {
     };
   },
 
+  // Remove standard mapped attributes using op: "remove" (name.givenName, name.familyName)
+  removeStandardMappedAttributes: (directory: Directory, userId: string): DirectorySyncRequest => {
+    return {
+      method: 'PATCH',
+      body: {
+        Operations: [
+          {
+            op: 'remove',
+            path: 'name.givenName',
+          },
+          {
+            op: 'remove',
+            path: 'name.familyName',
+          },
+        ],
+      },
+      directoryId: directory.id,
+      resourceType: 'users',
+      resourceId: userId,
+      apiSecret: directory.scim.secret,
+      query: {},
+    };
+  },
+
   // Remove standard attribute using op: "remove" with no value
   removeTitle: (directory: Directory, userId: string): DirectorySyncRequest => {
     return {
@@ -433,6 +457,53 @@ const requests = {
     };
   },
 
+  // Entra: set extension multi-valued attribute via URN path
+  entraSetExtensionMultiValued: (directory: Directory, userId: string): DirectorySyncRequest => {
+    return {
+      method: 'PATCH',
+      body: {
+        Operations: [
+          {
+            op: 'replace',
+            value: {
+              'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User': {
+                roles: [
+                  { value: 'admin-role-id', display: 'Admin' },
+                  { value: 'user-role-id', display: 'User' },
+                ],
+              },
+            },
+          },
+        ],
+      },
+      directoryId: directory.id,
+      resourceType: 'users',
+      resourceId: userId,
+      apiSecret: directory.scim.secret,
+      query: {},
+    };
+  },
+
+  // Entra: remove a specific value from extension multi-valued attribute via URN + filter path
+  entraRemoveExtensionFilterPath: (directory: Directory, userId: string): DirectorySyncRequest => {
+    return {
+      method: 'PATCH',
+      body: {
+        Operations: [
+          {
+            op: 'remove',
+            path: 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:roles[value eq "admin-role-id"]',
+          },
+        ],
+      },
+      directoryId: directory.id,
+      resourceType: 'users',
+      resourceId: userId,
+      apiSecret: directory.scim.secret,
+      query: {},
+    };
+  },
+
   // Arbitrary attribute names with SCIM filter paths (e.g. ims, photos)
   arbitraryFilterPaths: (directory: Directory, userId: string): DirectorySyncRequest => {
     return {

npm/test/dsync/users.test.ts

@@ -186,6 +186,29 @@ tap.test('Directory users /', async (t) => {
       t.notOk(data.title, 'title should be removed');
     });
 
+    t.test('Should propagate remove op for standard mapped attributes to user model', async (t) => {
+      // Verify user has name fields from initial creation
+      const { data: initialUser } = await directorySync.requests.handle(
+        requests.getById(directory, createdUser.id)
+      );
+      t.equal(initialUser.name.givenName, 'Jackson', 'user should have givenName initially');
+      t.equal(initialUser.name.familyName, 'M', 'user should have familyName initially');
+
+      // Remove name.givenName and name.familyName using op: "remove"
+      const { status, data } = await directorySync.requests.handle(
+        requests.removeStandardMappedAttributes(directory, createdUser.id)
+      );
+
+      t.equal(status, 200);
+      t.notOk(data.name?.givenName, 'givenName should be removed from raw');
+      t.notOk(data.name?.familyName, 'familyName should be removed from raw');
+
+      // Verify the user model fields are cleared via the internal user record
+      const user = await directorySync.users.get(createdUser.id);
+      t.equal(user.data?.first_name, '', 'first_name should be cleared on user model');
+      t.equal(user.data?.last_name, '', 'last_name should be cleared on user model');
+    });
+
     t.test('Entra: should set and clear extension attributes via no-path object format', async (t) => {
       // Set extension attributes (Entra no-path format with nested schema object)
       const { status: setStatus, data: setData } = await directorySync.requests.handle(
@@ -376,6 +399,39 @@ tap.test('Directory users /', async (t) => {
       t.equal(workAddress.country, 'US');
     });
 
+    t.test(
+      'Entra: should remove specific value from extension multi-valued attribute via URN filter path',
+      async (t) => {
+        const extKey = 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User';
+
+        // First, set a multi-valued extension attribute with two roles
+        const { status: setStatus, data: setData } = await directorySync.requests.handle(
+          requests.entraSetExtensionMultiValued(directory, createdUser.id)
+        );
+
+        t.equal(setStatus, 200);
+        t.ok(setData[extKey], 'extension schema key should exist');
+        t.equal(setData[extKey].roles.length, 2, 'should have two roles');
+
+        // Remove only the admin role using URN + filter path
+        const { status, data } = await directorySync.requests.handle(
+          requests.entraRemoveExtensionFilterPath(directory, createdUser.id)
+        );
+
+        t.equal(status, 200);
+        t.ok(data[extKey], 'extension schema key should still exist');
+        t.equal(data[extKey].roles.length, 1, 'should have one role remaining');
+        t.equal(data[extKey].roles[0].value, 'user-role-id', 'only the user role should remain');
+
+        // Verify removal persists
+        const { data: fetched } = await directorySync.requests.handle(
+          requests.getById(directory, createdUser.id)
+        );
+        t.equal(fetched[extKey].roles.length, 1, 'removal should persist after re-fetch');
+        t.equal(fetched[extKey].roles[0].value, 'user-role-id', 'correct role should persist');
+      }
+    );
+
     t.test('Should handle SCIM filter paths for arbitrary attribute names', async (t) => {
       const { status, data } = await directorySync.requests.handle(
         requests.arbitraryFilterPaths(directory, createdUser.id)