WPB-23434: Support SCIM PATCH of multi-valued emails attribute - #5419
Open
blackheaven wants to merge 12 commits into
Open
WPB-23434: Support SCIM PATCH of multi-valued emails attribute#5419blackheaven wants to merge 12 commits into
blackheaven wants to merge 12 commits into
Conversation
zebot
added
the
ok-to-test
Implement value-path PATCH (e.g. `emails[type eq "work"].value`) for the `emails` multi-valued attribute in hscim's applyUserOperation, which Entra and other IdPs use to provision user emails. Any value-path target was previously rejected with "can not lens into multi-valued attributes yet". Supports the value/type/primary sub-attributes and whole-entry paths; other multi-valued attributes remain unsupported. Also fix email propagation in Spar: an email-only PATCH does not change externalId, so updateVsuUref (the only update-path route to validateEmail -> Brig.updateEmail, gated on externalId change) never ran, and GET reconstructs the email from Brig -- so the change was a silent no-op. Add an explicit email-diff validateEmail in updateValidScimUser, gated on externalId being unchanged to avoid double-validation with updateVsuUref.
blackheaven
force-pushed
the
gdifolco/WPB-23434-scim-patch
branch
from
August 5, 2026 07:37
a0fb7f7 to
e4acc08
Compare
akshaymankar
requested changes
Aug 5, 2026
akshaymankar
left a comment
akshaymankar
left a comment
Member
There was a problem hiding this comment.
Looks good overall. Two things I would request:
- Document somewhere that Entra is acting wrong or fix
applyUserOperationso it doesn't rewriteAddtoReplacein case of multi-values and complex attributes. - The integration test should be in the new suite.
Other comments are nits.
Handle Add on emails[...] value-path explicitly instead of rewriting to Replace: whole-entry Add appends (concat semantics) while sub-attribute Add delegates to the Replace path. Give ValuePath named record fields and drop the redundant valuePathFilter helper. Correct the create-on-absent NOTE to cite Entra's documented Add behaviour. Move the end-to-end email PATCH test to the new integration suite (patchScimUser helper + testSparPatchEmailValuePath) and remove the deprecated copy. Add hscim unit tests for the Add behaviour.
blackheaven
force-pushed
the
gdifolco/WPB-23434-scim-patch
branch
from
August 5, 2026 10:52
1715cd6 to
709b260
Compare
spar never persists the SCIM email 'type': synthesizeScimUser returns every email with typ=Nothing, so a value-path filter emails[type eq "work"] can never match an existing email. The original test created a user WITH an email, so the Add hit create-on-absent and appended a second entry; scimEmailsToEmailAddress (pickFirst) then returned the old email, so no email diff fired and Brig was never updated. Mirror the proven deprecated test instead: create an email-less SAML user, disable validateSAMLemails so the provisioned email auto-activates, and assert the Add on emails[type eq "work"].value provisions a work email that propagates to Brig. (Add delegates to the Replace path for the .value sub-attribute, so this is behaviour-equivalent to the prior Replace test.)
scimEmailsToEmailAddress now returns Either, rejecting (Left) when more than one email is marked primary (RFC 7643 §2.4 violation). spar's validateScimUser' turns the Left into a SCIM 400, covering POST/PUT/PATCH (PATCH re-validates via putUser). Previously spar silently first-wins, masking client-side misconfiguration. Other inputs (zero or one primary) are unchanged.
synthesizeScimUser now echoes type = Just "work" instead of Nothing. spar/brig store a single address with no type, but Entra PATCHes with a value-path filter emails[type eq "work"]; without an echoed type the filter never matched and such PATCHes appended a duplicate email rather than updating the address in place. Echoing the canonical type (RFC 7643 sec 4.1.2) at the single synthesis point makes the filter match across create/GET/PATCH. Adds testSparPatchEmailValuePathInPlace (in-place update), corrects the create-on-absent test comment, and adds a changelog fragment.
Follow-up to 1406a78 (synthesizeScimUser now echoes type = Just "work" instead of Nothing): update all SCIM email test-expectations to expect type = "work", since every GET/PUT/PATCH response now materializes it. integration framework (Test/Spar.hs): add a scimWorkEmail helper and route the 13 full-email-object assertions/inputs that compare to a response through it. PUT inputs never round-tripped to a response are left typeless, preserving coverage that the server synthesizes the type when omitted. spar test-integration: flip the two expected-email constructions (Util/Scim.hs setDefaultRoleAndEmailsIfEmpty, APISpec.hs scim-user-search) from Nothing to Just "work". Test-only; no production code.
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
3 participants
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.
https://wearezeta.atlassian.net/browse/WPB-23434
Checklist
changelog.d