WPB-23434: Support SCIM PATCH of multi-valued emails attribute by blackheaven · Pull Request #5419 · wireapp/wire-server

blackheaven · 2026-08-05T06:56:38Z

https://wearezeta.atlassian.net/browse/WPB-23434

Checklist

Add a new entry in an appropriate subdirectory of changelog.d
Read and follow the PR guidelines

akshaymankar

Looks good overall. Two things I would request:

Document somewhere that Entra is acting wrong or fix applyUserOperation so it doesn't rewrite Add to Replace in case of multi-values and complex attributes.
The integration test should be in the new suite.

Other comments are nits.

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.

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.

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.

…test rename)

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.

akshaymankar

Looks good, but needs some explanation for the eventually use and I just want to be sure if we're changing APIs it is well tested and is very visible in release notes.

akshaymankar · 2026-08-18T14:00:37Z

-    resp.json %. "emails" `shouldMatch` (toJSON [object ["value" .= email]])
+    resp.json %. "emails" `shouldMatch` (toJSON [scimWorkEmail email])


Hmm, this looks like an API change, we must add an entry to release notes I think to warn people.
Btw, have we tested this with different setups? I'm scared this will break some customers.

Done: added changelog.d/1-api-changes/WPB-23434-scim-emails-now-echo-type (6cce05f), flagging that SCIM user resources now include type: "work" on emails and that clients doing strict equality on the emails array will see the extra member. On testing coverage: the PR covers SCIM PUT/PATCH round-trips (create-on-absent, in-place update, unchanged-email PUT), Brig email activation/propagation, and the zero/one/multiple primary cases. Microsoft Entra ID is the only known consumer of emails[type eq ...] value-path filters; if you know of others we should test against, I'm happy to add cases.

Co-authored-by: Akshay Mankar <axeman@mailbox.org>

Document why the two eventually blocks in Test/Spar.hs are needed: the PATCH 200 precedes the asynchronous email re-provisioning in Brig, and there is no synchronizing event for SCIM-provisioned users (same pattern as the deprecated suite's specEmailValidation). Clarify in the bug-fix changelog that the old behavior appended a duplicate (RFC 7644 create-on-absent) rather than erroring, and add an api-changes entry for the echoed emails type field.

scimEmailsToEmailAddress returned Nothing when no emails entry had primary = true, contradicting its documented selection rule (primary, else first entry). Fall back to the first entry instead.

Instead of hardcoding type="work" when synthesizing SCIM users, spar now persists the type and primary sub-attributes of the stored email entry in spar.scim_user_times (migration V23) and echoes them verbatim. Users provisioned without metadata echo neither field (strict round-trip), so value-path filters only match when the IdP actually supplied them. - hscim: new scimEmailsToEmail exposes the selected Email entry (address + metadata); scimEmailsToEmailAddress refactored on top of it. - wire-api: ValidScimUser gains emailType/emailPrimary (included in Eq, so type-only/primary-only PATCHes become effective). - wire-subsystems: ScimUserTimesStore.Write gains the two metadata args; Read/ReadMulti return a ScimUserTimes record. - spar: validateScimUser' captures client-sent metadata; synthesizeStoredUser reads (or, at creation, forces) it; createValidScimUserSpar no longer upserts redundantly; updateValidScimUser persists new metadata.

- Regenerate cassandra-schema.cql with the two new scim_user_times columns. - Integration tests: randomScimUserWithEmailAndMeta, scimStoredEmail (strict echo), reworked value-path tests (stored type 'home'), new testSparScimEmailMetaRoundTrip (work+primary round-trip, Okta-style primary-filter in-place update, strict no-metadata echo); shared scimAddPatchOp helper. - spar test-integration: metadata round-trip spec incl. legacy-row simulation; expectations updated for strict echo. - Review fixes: bound email type length (64 chars) at validateScimUser'; drop dead storedUser param of createValidScimUserSpar; simplify synthesizeStoredUser metadata/writeState; document mEmailMeta precondition; ScimUserTimes derives (Eq, Show); changelog wording.

Add testSparScimEmailTypeNonWorkEcho to the integration suite (covers "home" and "other": POST 201 + exact GET echo) and a matching spec in spar's UserSpec GET /Users/:id block, proving no normalization to "work".

…d (RFC 7643 2.5, RFC 7644 3.5.2.2) - PatchOp: keep explicit "value": null distinct from an absent value member - User: remove emails[...].type/.primary unassigns the field and keeps the entry; remove/null on .value is a 400; filterless remove emails clears all entries; replace with null unassigns Maybe-typed sub-attributes - unit + spar integration tests; changelog

blackheaven requested review from a team as code owners August 5, 2026 06:56

zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Aug 5, 2026

blackheaven force-pushed the gdifolco/WPB-23434-scim-patch branch from a0fb7f7 to e4acc08 Compare August 5, 2026 07:37

akshaymankar requested changes Aug 5, 2026

View reviewed changes

Comment thread libs/hscim/src/Web/Scim/Schema/User.hs Outdated

Comment thread libs/hscim/src/Web/Scim/Schema/User.hs Outdated

Comment thread libs/hscim/test/Test/Schema/UserSpec.hs Outdated

Comment thread services/spar/test-integration/Test/Spar/Scim/UserSpec.hs Outdated

blackheaven requested a review from akshaymankar August 5, 2026 10:50

blackheaven force-pushed the gdifolco/WPB-23434-scim-patch branch from 1715cd6 to 709b260 Compare August 5, 2026 10:52

blackheaven mentioned this pull request Aug 11, 2026

SCIM Error Responses Not Compliant with RFC 7644 Section 3.12 #5434

Closed

blackheaven added 7 commits August 18, 2026 15:42

WPB-23434: Resolve SCIM PATCH review feedback (Add->Replace comment, …

c820f08

…test rename)

blackheaven force-pushed the gdifolco/WPB-23434-scim-patch branch from 7a95f96 to f88dea5 Compare August 18, 2026 14:04

akshaymankar requested changes Aug 18, 2026

View reviewed changes

blackheaven and others added 2 commits August 18, 2026 17:23

Update libs/hscim/src/Web/Scim/Schema/User/Email.hs

cbb7ce9

Co-authored-by: Akshay Mankar <axeman@mailbox.org>

blackheaven force-pushed the gdifolco/WPB-23434-scim-patch branch from 6cce05f to 931f486 Compare August 18, 2026 16:11

blackheaven requested a review from akshaymankar August 18, 2026 16:12

blackheaven added 2 commits August 18, 2026 18:13

Hello CI

34de966

hscim: pick first email when no entry is marked primary

cffcf87

scimEmailsToEmailAddress returned Nothing when no emails entry had primary = true, contradicting its documented selection rule (primary, else first entry). Fall back to the first entry instead.

blackheaven force-pushed the gdifolco/WPB-23434-scim-patch branch from 03149a2 to cffcf87 Compare August 18, 2026 16:53

blackheaven added 5 commits August 18, 2026 20:20

Hello CI

3fc1bfd

Hello CI

9e8904e

fix: formatting

706f213

blackheaven added 2 commits August 19, 2026 15:56

Hello CI

882fc8b

test: non-work SCIM email types accepted and echoed verbatim

9d84951

Add testSparScimEmailTypeNonWorkEcho to the integration suite (covers "home" and "other": POST 201 + exact GET echo) and a matching spec in spar's UserSpec GET /Users/:id block, proving no normalization to "work".

blackheaven force-pushed the gdifolco/WPB-23434-scim-patch branch from 43d090d to 9d84951 Compare August 19, 2026 15:08

blackheaven added 3 commits August 19, 2026 17:45

Hello CI

53dc1b0

Hello CI

490c736

Navigation Menu

Uh oh!

WPB-23434: Support SCIM PATCH of multi-valued emails attribute - #5419

WPB-23434: Support SCIM PATCH of multi-valued emails attribute#5419
blackheaven wants to merge 21 commits into
developfrom
gdifolco/WPB-23434-scim-patch

blackheaven commented Aug 5, 2026

Uh oh!

akshaymankar left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akshaymankar left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akshaymankar Aug 18, 2026

Uh oh!

blackheaven Aug 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		resp.json %. "emails" `shouldMatch` (toJSON [object ["value" .= email]])
		resp.json %. "emails" `shouldMatch` (toJSON [scimWorkEmail email])

Uh oh!

Conversation

blackheaven commented Aug 5, 2026

Checklist

Uh oh!

akshaymankar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akshaymankar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akshaymankar Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

blackheaven Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants