Captain-P-Goldfish / SCIM-SDK

a scim implementation as described in RFC7643 and RFC7644
https://github.com/Captain-P-Goldfish/SCIM/wiki
BSD 3-Clause "New" or "Revised" License
122 stars 38 forks source link

Enterprise User manager.value is required in the SDK even though it is not according to the SCIM RFC #689

Closed stefan-wauters closed 4 months ago

stefan-wauters commented 4 months ago

We are in the process of setting up a SCIM Server that integrates with Okta. When a User has a manager assigned in Okta and subsequently removes it, Okta sends the following PUT request:

{
    "schemas": [
        "urn:ietf:params:scim:schemas:core:2.0:User",
        "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"
    ],
    (...)
    "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User": {
        (...)
        "manager": {
            "value": null
        }
    },
}

This is a valid request according to the SCIM RFC because manager.value is considered 'recommended', while in the implementation of the SCIM SDK it is marked as required here.

This results in the following error being sent even though manager.value is not required according to the SCIM Spec:

{
    "detail": "Required 'READ_WRITE' attribute 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:manager.value' is missing",
    "schemas": [
        "urn:ietf:params:scim:api:messages:2.0:Error"
    ],
    "status": 400,
    "errors": {
        "fieldErrors": {
            "manager.value": [
                "Required 'READ_WRITE' attribute 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:manager.value' is missing"
            ]
        }
    }
}

We have worked around this for now by adding a custom RequestValidator<User> to the ResourceHandler<User> that removes this validation on update:

class UserRequestValidator : RequestValidator<User> {
   (...)

    override fun validateUpdate(
        oldResourceSupplier: Supplier<User>?,
        newResource: User?,
        validationContext: ValidationContext?,
        requestContext: Context?,
    ) {
        validationContext?.fieldErrors?.let { fieldErrors ->
            // Remove the error message for missing manager.value because it is not strictly required according to the SCIM RFC
            val managerErrors = fieldErrors["manager.value"]
            managerErrors?.removeAll {
                it.equals(
                    "Required 'READ_WRITE' attribute 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:manager.value' is missing",
                )
            }

            if (managerErrors?.isEmpty() == true) {
                fieldErrors.remove("manager.value")
            }
        }
    }
}

This feels more like a workaround than a proper solution for the issue. Could you advise on a more appropriate way to solve this? Can manager.value be marked as required = false or would a similar pre-processing step be required as the Microsoft Azure PATCH requests?

Thanks in advance.

Captain-P-Goldfish commented 4 months ago

Fixed. But there is also another solution you could have used. You could have set the value-attribute of the manager to required: false

Either by reregistering the UserEndpoint with a customized schema or on the ResourceHandler#postConstruct method

 @Override
  public void postConstruct(de.captaingoldfish.scim.sdk.server.schemas.ResourceType resourceType)
  {
    resourceType.getSchemaAttribute(String.format("%s.%s",
                                                  AttributeNames.RFC7643.MANAGER,
                                                  AttributeNames.RFC7643.VALUE))
                .ifPresent(managerValue -> {
                  managerValue.setRequired(false);
                });
  }