limosa-io / laravel-scim-server

SCIM 2.0 Server implementation for Laravel
MIT License
51 stars 29 forks source link

Problem when creating a new user via SCIM that already exists #24

Closed uberbrady closed 1 year ago

uberbrady commented 2 years ago

I'm running into a problem where my users try to enable SCIM provisioning against an already-existing set of users in my application. When the username in our application already exists and already matches what the directory provider is trying to provide, we return an error for the SCIM request. What seems to happen is the SCIM provisioner tries to create the user, fails our model-level validation, and the SCIM provisioner thinks that the user has not been created. They've been able to sometimes work around the problem by 'resetting' the SCIM provisioning (starting, again, from scratch). This may be an Azure AD problem, as that's the only place where I've seen it so far (though it's also where most of our users try to do SCIM from, so that may not mean much).

I feel like another behavior that might work better for my users on the 'create' method would be to look up the already-existing user, and return that user - with the appropriate attributes modified as requested by the 'create' - as the "newly" created user. From SCIM's perspective, it doesn't matter that the userName was an already existing user ID; it's just able to now refer to that user uniquely, and update the attributes appropriately.

I'm happy to take direction if this means I should tweak my implementation however you recommend - or, if this is something that we can provide a PR for, we'll happily provide one, if it's something you'd be comfortable merging.

Thanks again for providing this library - I absolutely would not have been able to add SCIM support to our product without this, and it's been a big draw for our users. I deeply appreciate it.

uberbrady commented 2 years ago

So if I change this line: https://github.com/arietimmerman/laravel-scim-server/blob/master/src/Http/Controllers/ResourceController.php#L116 to instead be:

        $resourceObject = $class::firstOrNew(['username' => $input['userName']]);

The user syncs as expected. Obviously, for the purposes of this library I wouldn't want to hardcode that.

I was thinking of something in the $config like:

{
  //....
  'key' => ['username' => 'userName']
  // ...
}

Though, if it were to come to an email address needing to be unique, that would be much trickier and this approach might not make sense.

Please let me know if you'd be interested in such a change, or if you have a better idea on how to solve the problem. Thanks again!

dmyers commented 2 years ago

I could also use this as well as my application needs to have unique email addresses. @arietimmerman are you open to a PR for this? The config option seems like it would be an ideal solution @uberbrady.

arietimmerman commented 2 years ago

I don't think the supposed SCIM behavior for a create request is that it would update an existing object, if it exists. I think it should fail, isn't it? Perhaps it should be possible to configure a set of validation rules specifically for create, this would allow using the unique validation rule.

dmyers commented 2 years ago

@arietimmerman I think you are right. I just did some more testing with Okta and they do a GET request to the resource route with a filter on the username attribute to determine if the user already exists and not will then proceed to create with a second request to POST to the users route and if it does they do a PUT request to the users route.

Looks like /scim/v2/Users?filter=username%20eq%20email@example.com&startIndex=1&count=100. Documented here: https://support.okta.com/help/s/article/Existing-SCIM-User-Being-Created-at-Assignment?language=en_US