limosa-io / laravel-scim-server

SCIM 2.0 Server implementation for Laravel
MIT License
47 stars 28 forks source link

Possible Azure AD SCIM issues when changing an email address? #34

Closed uberbrady closed 1 week ago

uberbrady commented 1 year ago

This could absolutely be my own screw-up, but I'd love to get your collective eyes on this issue I've been having with Azure.

I'm getting the following exception when I try to update a user's email address and then force a SCIM provisioning.:

Exception caught! Replace is not implemented for ":urn:ietf:params:scim:schemas:core:2.0:User.emails.value" of type: ArieTimmerman\Laravel\SCIMServer\Exceptions\SCIMException when executing:
PATCH https://msscim.snipe-it.io/scim/v2/Users/21

{"schemas":["urn:ietf:params:scim:api:messages:2.0:PatchOp"],"Operations":[{"op":"Replace","path":"emails[type eq \"work\"].value","value":"AdeleV2@REDACTED.onmicrosoft.com"},{"op":"Replace","path":"addresses[type eq \"work\"].formatted","value":"18/2111"},{"op":"Add","path":"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:department","value":"Retail"}]}  

The relevant portion of my SCIM config is this:

<?php
/* ....... */
        $config['validations'][$core.'emails'] = 'nullable|array';         // emails are not required in Snipe-IT...
        $config['validations'][$core.'emails.*.value'] = 'email'; // ...(had to remove the recommended 'required' here)

        $mappings['emails'] = [[
            "value" => AttributeMapping::eloquent("email"),
            "display" => null,
            "type" => AttributeMapping::constant("work")->ignoreWrite(),
            "primary" => AttributeMapping::constant(true)->ignoreWrite()
        ]];
/* ..... */

(The entire SCIM configuration can be viewed, if you need it, here: https://github.com/snipe/snipe-it/blob/master/app/Models/SnipeSCIMConfig.php )

I experimented with adding an additional mapping for ['emails']['value'] that I set ignoreRead() on, but that didn't seem to help either.

Am I doing something wrong, or have I maybe run into a bug? It does seem like Microsoft's PatchOp request seems to be correctly formatted (I think?).

Thanks everybody for this spectacular software!!!

uberbrady commented 1 year ago

I've seen a few more, similar to this - it's all about embedded 'stuff' on a PATCH request -

Exception caught! Replace is not implemented for ":urn:ietf:params:scim:schemas:core:2.0:User.addresses.country" of type: ArieTimmerman\Laravel\SCIMServer\Exceptions\SCIMException when executing:
PATCH https://REDACTED.COM/scim/v2/Users/123456789 //<- also redacted, btw.

{"schemas":["urn:ietf:params:scim:api:messages:2.0:PatchOp"],"Operations":[{"op":"Add","path":"addresses[type eq \"work\"].locality","value":"REDACTED"},{"op":"Replace","path":"addresses[type eq \"work\"].country","value":"REDACTED"},{"op":"Replace","path":"phoneNumbers[type eq \"work\"].value","value":"+REDACTED_NUMBER"}]}  

It's completely fine if it turns out that I'm doing something wrong here; I'm happy to be educated, and, if possible, improve the documentation to better educate others that follow me. Or if there's some kind of bug, I've gotten SO much value out of this library that it I would love to try and help chase it down. Please do let me know.

Thank you again for providing this amazing software - it's really helped us, and if there's any way I can help support it, I'd love to. Please let me know. Thanks again!

uberbrady commented 10 months ago

So I have something that I tried that seemed to at least help with this - basically, it looks like if I go here: https://github.com/arietimmerman/laravel-scim-server/blob/351937587b0120e0083b38a98d81f050bd18aa67/src/Attribute/Collection.php#L87 and add a ->setReplace() call (which looks exactly identical to the ->setAdd() callback), it seems to accept the 'replace' request from Azure. It's a little coarse - in that it would seem to do it for 'everything in the collection' - but I've gotten it to work now for Phone Number (which is also a collection), and Email address (which was my original problem).

I'd be happy to offer back a patch with this change if there's any interest. Or happy to workshop it further if that might help. Please do just let me know. Thank you again for your wonderful software!

atymic commented 10 months ago

@uberbrady thanks for your work here, having the exact same issues!

Would you consider bringing your fork up to date with this repo? Unfortunately we need the model factories and new laravel version support.

arietimmerman commented 1 week ago

I believe the rewritten code in the latest release should fix these issues. Please let me know if you conclude otherwise.