Some Azure AD remove operations are not supported

[yubing24]

Describe the bug Here is a SCIM PATCH request that I received from Azure AD that looks like this:

{ op: 'remove', 'path': 'manager[value eq "manager-id"]'};

The error that I got was:

Error: Invalid SCIM Patch: Impossible to search on a mono valued attribute.
    at extractArray (/app/node_modules/scim-patch/lib/src/scimPatch.js:227:15)
    at applyRemoveOperation (/app/node_modules/scim-patch/lib/src/scimPatch.js:145:44)
    at /app/node_modules/scim-patch/lib/src/scimPatch.js:67:24
    at Array.reduce (<anonymous>)
    at scimPatch (/app/node_modules/scim-patch/lib/src/scimPatch.js:63:28)

To Reproduce

1. In the test file scimPatch.test.ts, add this test:

   it('REMOVE: array filter on complex field (Azure AD)', done => {
   const schemaExtension = 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User';
   const patch: ScimPatchRemoveOperation = { op: 'remove', 'path': 'manager[value eq "manager-id"]'};
   scimUser[schemaExtension] = {
   manager: {
       value: 'manager-id',
       $ref: ''
   }
   };
   const afterPatch: any = scimPatch(scimUser, [patch]);
   
   expect(afterPatch[schemaExtension].manager).not.to.exist;
   return done();
   });

2. Run npm run build and npm run test, receive the same error:

   SCIM PATCH
      remove
        REMOVE: array filter on complex field (Azure AD):
    Error: Invalid SCIM Patch: Impossible to search on a mono valued attribute.
     at extractArray (lib/src/scimPatch.js:60:563)
     at applyRemoveOperation (lib/src/scimPatch.js:41:59)
     at /Users/mac-yubingh/Projects/github.com/yubing24/scim-patch/lib/src/scimPatch.js:25:349
     at Array.reduce (<anonymous>)
     at scimPatch (lib/src/scimPatch.js:25:137)
     at Context.<anonymous> (lib/test/scimPatch.test.js:941:58)
     at process.processImmediate (node:internal/timers:471:21)

Expected behavior I am expecting that the manager field should be removed from the Enterprise User schema in the patched document.

Screenshots N/A

Desktop (please complete the following information):

• OS: Darwin <computer-name> 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:22 PDT 2023; root:xnu-8796.121.3~7/RELEASE_X86_64 x86_64
• Browser: N/A
• Version: 0.7.1

Smartphone (please complete the following information):

• Device: N/A
• OS: N/A
• Browser: N/A
• Version: N/A

Additional context Add any other context about the problem here.

[thomaspoignant]

Hello @yubing24, thanks for opening this issue. I've looked at your problem and I think the issue is not on the lib but on your patch query.

You are using a filter in your query while it is an object and not an array. To achieve what you expect your patch operation should look like this:

const patch: ScimPatchRemoveOperation = { op: 'remove', 'path': 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:manager'};

If what you wanted to do is having an array of manager, you could do something like this:

it('REMOVE: array filter on complex field (Azure AD)', done => {
    const schemaExtension = 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User';
    const patch: ScimPatchRemoveOperation = { op: 'remove', 'path': 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:manager[value eq "manager-id"]'};
    scimUser[schemaExtension] = {
        manager: [{
            value: 'manager-id',
            $ref: ''
        }]
    };
    const afterPatch: any = scimPatch(scimUser, [patch]);

    expect(afterPatch[schemaExtension].manager).not.to.exist;
    return done();
});

Both solutions are working well, with the actual version of scim-patch.

[yubing24]

@thomaspoignant Thank you for looking into it and the explanation. If I understand you correctly, are you saying that the manager field must be explicitly specified under the enterprise user schema extension, otherwise it will not work (as in automatically treat a manager field as the enterprise user schema's manager)?

[thomaspoignant]

What i mean is that you can’t use a filter [value eq "manager-id"] on something else than an array.

here you trying to use a filter on { value: 'manager-id', $ref: '' }

and this is not working. The error message is explicit, you can not use a filter on a complex field.

[yubing24]

Thank you. I'll look into it a bit more on the Azure side.

[thomaspoignant]

Don’t hesitate to re-open this issue if you see something in the RFC that we have implemented the wrong way.

[yubing24]

@thomaspoignant I looked into it a bit more, and here is what I've found:

In the schema definition (RFC 7643), it states that manager is a complex type:

   manager
      The user's manager.  A complex type that optionally allows service
      providers to represent organizational hierarchy by referencing the
      "id" attribute of another User.

      value  The "id" of the SCIM resource representing the user's
         manager.  RECOMMENDED.

      $ref  The URI of the SCIM resource representing the User's
         manager.  RECOMMENDED.

      displayName  The displayName of the user's manager.  This
         attribute is OPTIONAL, and mutability is "readOnly".

See https://www.rfc-editor.org/rfc/rfc7643.html#section-4.3.

Complex type in RFC 7643 uses the definition of "Object" from RFC 7159. See https://www.rfc-editor.org/rfc/rfc7643.html#section-2.3

In RFC 7159, an object is defined as a as a pair of curly brackets surrounding zero or more name/value pairs (or members). See https://www.rfc-editor.org/rfc/rfc7159#section-4.

Therefore, the manager field should be a single-valued complex object, rather than an array.

It looks like the schema extension is required so that parser can correctly identify field. For example, this test passes:

    it("REMOVE: should remove manager correctly", (done) => {
      const schemaExtension =
        "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User";
      const patch: ScimPatchRemoveOperation = {
        op: "remove",
        path: schemaExtension + ":manager",
      };
      scimUser[schemaExtension] = {
        manager: {
          value: "manager-id",
          $ref: "",
        },
      };
      const afterPatch: any = scimPatch(scimUser, [patch]);

      expect(afterPatch[schemaExtension].manager).not.to.exist;
      return done();
    });

and this test fails:

    it("REMOVE: should remove manager correctly", (done) => {
      const schemaExtension =
        "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User";
      const patch: ScimPatchRemoveOperation = {
        op: "remove",
        path: "manager",
      };
      scimUser[schemaExtension] = {
        manager: {
          value: "manager-id",
          $ref: "",
        },
      };
      const afterPatch: any = scimPatch(scimUser, [patch]);

      expect(afterPatch[schemaExtension].manager).not.to.exist;
      return done();
    });

I have not looked deep into the implementation yet. Do you know if the manager field may somehow be treated as an array instead of an object? Or is there some default assumption that a field is an array unless something else indicate that the field is a complex object?

[thomaspoignant]

Sorry for the late reply @yubing24. I think it woerk well for the manager field, this is more the format of you query that should not be between [] because this means that you are searching in an array, you should use a pointed notation for this.