Skip to content

Commit 24f6ac0

Browse files
authored
backport[2025-q2]: WPB-20203 backend updating user attributes via SCIM should be possible when E2EID is enabled (#4775)
This is a backport of #4772.
1 parent b9a60e6 commit 24f6ac0

File tree

5 files changed

+108
-7
lines changed

5 files changed

+108
-7
lines changed

changelog.d/2-features/WPB-20203

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Allow updates of SCIM users by SCIM even if E2EID is enabled

integration/test/SetupHelpers.hs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,9 @@ lhDeviceIdOf bob = do
363363
>>= (%. "id")
364364
>>= asString
365365

366+
randomUUIDString :: App String
367+
randomUUIDString = UUID.toString <$> liftIO nextRandom
368+
366369
randomScimUser :: App Value
367370
randomScimUser = do
368371
email <- randomEmail
@@ -455,6 +458,10 @@ updateTestIdpWithMetaWithPrivateCreds owner idpId = do
455458
loginWithSaml :: (HasCallStack) => Bool -> String -> String -> (String, (SAML.IdPMetadata, SAML.SignPrivCreds)) -> App (Maybe String, SAML.SignedAuthnResponse)
456459
loginWithSaml = loginWithSamlWithZHost Nothing OwnDomain
457460

461+
loginWithSamlEmail :: (HasCallStack) => Bool -> String -> String -> (String, (SAML.IdPMetadata, SAML.SignPrivCreds)) -> App (Maybe String, SAML.SignedAuthnResponse)
462+
loginWithSamlEmail expectSuccess tid email =
463+
loginWithSamlWithZHost Nothing OwnDomain expectSuccess tid email
464+
458465
-- | Given a team configured with saml sso, attempt a login with valid credentials. This
459466
-- function simulates client *and* IdP (instead of talking to an IdP). It can be used to test
460467
-- scim-provisioned users as well as saml auto-provisioning without scim.

integration/test/Test/Spar.hs

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,3 +483,94 @@ testIdpUpdate = do
483483
-- the SCIM users can still login
484484
for_ uids $ \(_, email) -> do
485485
void $ loginWithSaml True tid email idp3
486+
487+
testAllowUpdatesBySCIMWhenE2EIdEnabled :: (HasCallStack) => App ()
488+
testAllowUpdatesBySCIMWhenE2EIdEnabled = do
489+
(tok, uid, su) <- setup
490+
491+
su1 <- checkUpdateHandle tok uid su
492+
su2 <- checkUpdateDisplayName tok uid su1
493+
494+
-- the following should not be part of the e2eid certification, but are checked here anyway
495+
su3 <- checkUpdateLocale tok uid su2
496+
su4 <- checkUpdateEmail tok uid su3
497+
void $ checkUpdateExternalId tok uid su4
498+
where
499+
setup :: App (String, String, Value)
500+
setup = do
501+
(owner, tid, _) <- createTeam OwnDomain 1
502+
setTeamFeatureStatus owner tid "sso" "enabled" >>= assertSuccess
503+
setTeamFeatureStatus owner tid "mlsE2EId" "enabled" >>= assertSuccess
504+
void $ registerTestIdPWithMeta owner >>= getJSON 201
505+
tok <- createScimTokenV6 owner def >>= getJSON 200 >>= (%. "token") >>= asString
506+
scimUser <- randomScimUser
507+
email <- scimUser %. "emails" >>= asList >>= assertOne >>= (%. "value") >>= asString
508+
uid <- createScimUser OwnDomain tok scimUser >>= getJSON 201 >>= (%. "id") >>= asString
509+
activateEmail OwnDomain email
510+
pure (tok, uid, scimUser)
511+
512+
checkUpdateHandle :: (HasCallStack) => String -> String -> Value -> App Value
513+
checkUpdateHandle tok uid scimUser = do
514+
newHandle <- randomHandle
515+
su <- setField "userName" newHandle scimUser
516+
bindResponse (updateScimUser OwnDomain tok uid su) $ \res -> do
517+
res.status `shouldMatchInt` 200
518+
res.json %. "userName" `shouldMatch` newHandle
519+
bindResponse (getUsersId OwnDomain [uid]) $ \res -> do
520+
res.status `shouldMatchInt` 200
521+
u <- res.json >>= asList >>= assertOne
522+
u %. "handle" `shouldMatch` newHandle
523+
pure su
524+
525+
checkUpdateDisplayName :: (HasCallStack) => String -> String -> Value -> App Value
526+
checkUpdateDisplayName tok uid scimUser = do
527+
let displayName = "Alice in Wonderland"
528+
su <- setField "displayName" displayName scimUser
529+
bindResponse (updateScimUser OwnDomain tok uid su) $ \res -> do
530+
res.status `shouldMatchInt` 200
531+
res.json %. "displayName" `shouldMatch` displayName
532+
bindResponse (getUsersId OwnDomain [uid]) $ \res -> do
533+
res.status `shouldMatchInt` 200
534+
u <- res.json >>= asList >>= assertOne
535+
u %. "name" `shouldMatch` displayName
536+
pure su
537+
538+
checkUpdateLocale :: (HasCallStack) => String -> String -> Value -> App Value
539+
checkUpdateLocale tok uid scimUser = do
540+
su <- setField "preferredLanguage" "fr" scimUser
541+
bindResponse (updateScimUser OwnDomain tok uid su) $ \res -> do
542+
res.status `shouldMatchInt` 200
543+
res.json %. "preferredLanguage" `shouldMatch` "fr"
544+
bindResponse (getUsersId OwnDomain [uid]) $ \res -> do
545+
res.status `shouldMatchInt` 200
546+
u <- res.json >>= asList >>= assertOne
547+
u %. "locale" `shouldMatch` "fr"
548+
pure su
549+
550+
checkUpdateEmail :: (HasCallStack) => String -> String -> Value -> App Value
551+
checkUpdateEmail tok uid scimUser = do
552+
newEmail <- randomEmail
553+
su <- setField "emails" [object ["value" .= newEmail]] scimUser
554+
bindResponse (updateScimUser OwnDomain tok uid su) $ \res -> do
555+
res.status `shouldMatchInt` 200
556+
res.json %. "emails" `shouldMatch` [object ["value" .= newEmail]]
557+
activateEmail OwnDomain newEmail
558+
bindResponse (getUsersId OwnDomain [uid]) $ \res -> do
559+
res.status `shouldMatchInt` 200
560+
u <- res.json >>= asList >>= assertOne
561+
u %. "email" `shouldMatch` newEmail
562+
pure su
563+
564+
checkUpdateExternalId :: (HasCallStack) => String -> String -> Value -> App Value
565+
checkUpdateExternalId tok uid scimUser = do
566+
newExtId <- randomUUIDString
567+
su <- setField "externalId" newExtId scimUser
568+
bindResponse (updateScimUser OwnDomain tok uid su) $ \res -> do
569+
res.status `shouldMatchInt` 200
570+
res.json %. "externalId" `shouldMatch` newExtId
571+
bindResponse (getUsersId OwnDomain [uid]) $ \res -> do
572+
res.status `shouldMatchInt` 200
573+
u <- res.json >>= asList >>= assertOne
574+
subject <- u %. "sso_id.subject" >>= asString
575+
subject `shouldContainString` newExtId
576+
pure su

libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -513,11 +513,12 @@ guardLockedFields ::
513513
guardLockedFields user updateOrigin (MkUserProfileUpdate {..}) = do
514514
let idempName = isNothing name || name == Just user.name
515515
idempLocale = isNothing locale || locale == user.locale
516-
scim = updateOrigin == UpdateOriginWireClient && user.managedBy == Just ManagedByScim
516+
scimConflict = updateOrigin == UpdateOriginWireClient && user.managedBy == Just ManagedByScim
517+
updateByScim = updateOrigin == UpdateOriginScim && user.managedBy == Just ManagedByScim
517518
e2eid <- hasE2EId user
518-
when ((scim || e2eid) && not idempName) do
519+
when ((scimConflict || (e2eid && not updateByScim)) && not idempName) do
519520
throw UserSubsystemDisplayNameManagedByScim
520-
when (scim {- e2eid does not matter, it's not part of the e2eid cert! -} && not idempLocale) do
521+
when (scimConflict {- e2eid does not matter, it's not part of the e2eid cert! -} && not idempLocale) do
521522
throw UserSubsystemLocaleManagedByScim
522523

523524
guardLockedHandleField ::
@@ -530,10 +531,11 @@ guardLockedHandleField ::
530531
Sem r ()
531532
guardLockedHandleField user updateOrigin handle = do
532533
let idemp = Just handle == user.handle
533-
scim = updateOrigin == UpdateOriginWireClient && user.managedBy == Just ManagedByScim
534+
scimConflict = updateOrigin == UpdateOriginWireClient && user.managedBy == Just ManagedByScim
535+
updateByScim = updateOrigin == UpdateOriginScim && user.managedBy == Just ManagedByScim
534536
hasHandle = isJust user.handle
535537
e2eid <- hasE2EId user
536-
when ((scim || (e2eid && hasHandle)) && not idemp) do
538+
when ((scimConflict || (e2eid && hasHandle && not updateByScim)) && not idemp) do
537539
throw UserSubsystemHandleManagedByScim
538540

539541
updateUserProfileImpl ::

libs/wire-subsystems/test/unit/Wire/UserSubsystem/InterpreterSpec.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ spec = describe "UserSubsystem.Interpreter" do
646646
in profileErr === Left UserSubsystemLocaleManagedByScim
647647

648648
prop
649-
"if e2e identity is activated, the user name cannot be updated"
649+
"if e2e identity is activated, the user name cannot be updated by a wire client"
650650
\(NotPendingStoredUser alice) localDomain (newName :: Name) config ->
651651
(alice.name /= newName) ==>
652652
let lusr = toLocalUnsafe localDomain alice.id
@@ -670,7 +670,7 @@ spec = describe "UserSubsystem.Interpreter" do
670670
)
671671
config
672672
do
673-
updateUserProfile lusr Nothing UpdateOriginScim (def {name = Just newName, supportedProtocols = Nothing})
673+
updateUserProfile lusr Nothing UpdateOriginWireClient (def {name = Just newName, supportedProtocols = Nothing})
674674
getUserProfile lusr (tUntagged lusr)
675675
in profileErr === Left UserSubsystemDisplayNameManagedByScim
676676

0 commit comments

Comments
 (0)