Possible SCIMv2 protocol violation?

[uberbrady]

Hello again! Thanks again for providing this amazing library; there's no way we would've been able to implement SCIM in our product without it.

Unfortunately, however, we're seeing some SCIM responses that look like the following:

{
    "id": 49,
    "meta": {
        "created": "2022-01-27T15:57:41-08:00",
        "lastModified": "2022-10-06T10:35:15-07:00",
        "location": "https:\/\/domain.example.com\/scim\/v2\/Users\/77",
        "resourceType": "User"
    },
    "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:core:2.0:User": {
        "userName": "username@domain.com",
        "name": {
            "formatted": "User\u00e9 Name",
            "familyName": "Name",
            "givenName": "User\u00e9"
        },
        "title": "Software developer",
        "preferredLanguage": "en",
        "active": true,
        "emails": [
            {
                "value": "username@domain.com",
                "type": "work",
                "primary": true
            }
        ],
        "phoneNumbers": [
            {
                "type": "work",
                "primary": true
            }
        ],
        "addresses": [
            {
                "type": "work",
                "formatted": "n\/a",
                "primary": true
            }
        ]
    }
}

But it looks like according to the spec, embedding the User attributes under the urn:ietf:params:scim:schemas:core:2.0:User namespace might not be correct. In section 3 - SCIM Resources, it says (under "Core Resources"):

A resource's core attributes are those attributes that sit at the top level of the JSON object together with the common attributes (such as the resource "id"). The list of valid attributes is specified by the resource's resource type "schema" attribute (see Section 6). This same value is also present in the resource's "schemas" attribute.

Which makes it sound like the various things in that User namespace belong at the top level of the JSON. Additional namespaces (such as Enterprise) are being handled correctly, however.

So it sounds to me like there are two different ways of fixing this. One is we can put together some changes to the ScimConfig.php file so that the User resources for the User part of the config are at the top level of the JSON, and not under the User namespace. This should probably be enough to fix it - I was able to add externalId support at that top-level to our own implementation easy enough; we'd just have to repeat that for the other examples.

Another way to do it might be to build-in the knowledge of stuff like this: https://datatracker.ietf.org/doc/html/rfc7643#section-4.1 into the software itself - specifically:

SCIM provides a resource type for "User" resources. The core schema for "User" is identified using the following schema URI: "urn:ietf:params:scim:schemas:core:2.0:User". ...

And then laravel-scim-server can see that you're returning a User, and that you're returning attributes from the User core schema, and (somehow?) auto-promote those various attributes to the top level.

If you'd be interested in PR's for either of those solutions, I'd be happy to take a stab at either one. Or, if you don't want to do either of them, that's fine too! We can just probably fix it on our end and that would probably be enough.

Thanks again for providing this software and having it be open source! It's helping us immensely.

[arietimmerman]

Interesting point. That piece of specification you have quotes seem to indicate that the current implementation is wrong. It is not entirely clear to me if these attributes MUST be at the top level or that this is optional. Nevertheless, it looks very much like it is a common thing to do.

[uberbrady]

What we're finding - so far - is that some providers can handle it as-is - Azure, specifically, and once my externalId patch is merged into my software, I'm hoping that Okta will be able to muddle through (it kind-of can right now). But OneLogin definitely can't.

I'm looking into writing a pretty gnarly patch for our customized SCIMConfig, which might be informative if I can get it going. I'll definitely report back.

[uberbrady]

I've been doing some work on this and still running into a lot of exceptions; I think having the validations in one namespace and the mappings in another is causing some problems. It seems to be punching through the Validations OK, but the mapping is having some issues. I'll keep digging.

[arietimmerman]

A possible solution might be to introduce a peace of middleware that transforms messages. If an incoming message contains top-level elements belonging to the main schema, it should move these to that specific schema. I.e. a form of canonicalization. For returning elements another solution needs to be implemented, as it would be really inefficient to walk over a large response containing many resources.

[uberbrady]

I think that's a great idea, and I took a stab at it here: https://github.com/grokability/laravel-scim-server/pull/2/files - it's still ugly and needs a lot of love, but it's at least a start. It seems like it kinda does what you're talking about. Let me know if I can offer you a patch here as well once I clean it up a little.

[dmyers]

@uberbrady I tried out your fork to see if it would help with my Okta update/patch issues and I got a few undefined property errors, but once I fixed them by editing the file in my vendor folder it did fix my issue, but it might be better to make a separate middleware maybe which is only attached to the routes that needs it instead of the SCIMHeaders which is used by many. https://github.com/grokability/laravel-scim-server/pull/2/files#diff-440ed5d6003d807a851ea3feaa5d8cee016a0af978340a542c54917d650f368bR36

[arietimmerman]

Please check #31 Is this what you guys are looking for? I should fix some phpunit errors though. I think it's somewhat like @uberbrady his solution, but now I think this is a better place to fix it.

[dmyers]

@arietimmerman Yep that fixes it for me too and is a very clean solution. I think this is the last issue I had for deploying this fully to get into internal testing. Thank you for the work on this, really appreciate it.

[arietimmerman]

Cool. I'll consider this as done. If you @uberbrady stumble upon issues or requests something different, please let me know.

[uberbrady]

This looks much better than my fix; I've taken it into my fork and my initial testing looks pretty positive. I'll report back if I run into any problems. Thanks again!