scimmyjs / scimmy

SCIM m(ade eas)y - SCIM 2.0 library for NodeJS
https://scimmyjs.github.io
MIT License
37 stars 9 forks source link

Extension attributes are filtered when there's excludedAttributes param #37

Closed LesterLian closed 1 month ago

LesterLian commented 1 month ago

Hi @sleelin,

We found another issue while trying to integrate with Entra ID. When calling GET /Users?excludedAttributes=userName the extension attributes are filtered out in response. If we remove the excludedAttributes param then the response is fine.

The problem seems to be caused by: https://github.com/scimmyjs/scimmy/blob/9e81f47ceab7d0b518bc516ad088278cd0137774/src/lib/types/definition.js#L342 https://github.com/scimmyjs/scimmy/blob/9e81f47ceab7d0b518bc516ad088278cd0137774/src/lib/types/definition.js#L372-L373 https://github.com/scimmyjs/scimmy/blob/9e81f47ceab7d0b518bc516ad088278cd0137774/src/lib/types/definition.js#L306-L310

First of all, the inclusions are names of attributes, which would be EnterpriseUser for the SCIMMY.Schemas.EnterpriseUser extension or name for new SCIMMY.Types.SchemaDefinition(name, ...).

Then, the logic seems to expect the key of data returned by handler to be individual extension attributes like 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:user:organization'. However, SchemaDefinition.coerce constructs an object and stores it in urn:ietf:params:scim:schemas:extension:enterprise:2.0:user.

I think we should check type of attribute and get the id for SchemaDefinition instances, and remove the ':' at end of the key checking expression.

LesterLian commented 1 month ago

Suggested changes:

// L342
- const inclusions = attributes.map(({name}) => name); 
+ const inclusions = attributes.map((attribute) => (attribute instanceof SchemaDefinition) ? attribute.id : attribute.name);
// L372
- if (Object.keys(data[key]).length && inclusions.some(k => k.toLowerCase().startsWith(`${key.toLowerCase()}:`)))
+ if (Object.keys(data[key]).length && inclusions.some(k => k.toLowerCase().startsWith(`${key.toLowerCase()}`)))