Handle SCIM group syncing with very large user+group syncs - #19496
Merged
Merged
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 7 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This should hopefully fix an OOM crash on SCIM group syncs with large member lists (over 100k). A user syncs their user list from Okta/Azure/etc into Snipe-IT over SCIM, and in the case, one of their groups had 100,000 members in it. When their identity provider tried to push that whole group across in one request, the server ran out of memory and returned a 500 error before it did any real work.
The reason it ran out of memory is that Laravel's form-validation system was set up to check that every single member entry in the request had an ID attached. That all sounds fine, except the way Laravel implements that check is to pre-build a little rule-checker object for each entry before it actually validates anything.
101,000 members means 101,000 rule-checker objects sitting in memory all at once, and that alone was enough to blow past the server's memory ceiling.
Two changes to fix it:
I also added a defensive upper bound of 200,000 members per request as a guardrail so a genuinely runaway payload (accidental duplication, buggy bespoke client script) still gets rejected cleanly instead of finding some other way to exhaust memory.
So 101k-member groups should sync fine now, 500k+ still gets rejected fast, and clients that send malformed member entries get a helpful 400 error naming the bad rows instead of the previous confusing 500.