search

scimmyjs / scimmy

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

EnterpriseUser manager value should not be required #28

Closed brianpeiris closed 4 months ago

brianpeiris commented 4 months ago

For EnterpriseUser, Scimmy defines the "value" attribute under "manager" as "required"^1. The current RFC is confusing on this point, since it uses both "RECOMMENDED", "REQUIRED", and "required: false" in different sections of the RFC^2. However, it seems the intent was to mark it as "recommended", not "required", according to a message about the errata in the IETF mailing list^4. Unless there are objections from the Scimmy maintainers or users, I'd suggest following the intent of the spec authors and loosen the schema here.

I ran into this in an edge-case scenario when integrating with Okta, where Okta could send a "manager" structure with a "displayName" attribute, but without a "value" attribute.

sleelin commented 4 months ago

Hi @brianpeiris, Thanks again for another well-researched contribution! Digging up the IETF errata was very much appreciated, and heavily implies that the use of "REQUIRED" in the JSON schema description of the manager.value attribute was meant to be "RECOMMENDED" - reinforced by the definition including "required": false immediately afterward.

I believe your suggestion to loosen the schema and change it to be optional by default is sensible and in keeping with the intent of the spec authors, and will release v1.2.2 with this change included.

For anyone wishing to revert to the previous behaviour of having the value attribute required, the following line of code should achieve this:

SCIMMY.Schemas.EnterpriseUser.definition.attribute("manager.value").config.required = true;
brianpeiris commented 4 months ago

Thanks for the fix!