WPB-18929 fix: SCIM cannot re invite user if initial invitation was revoked or expired via teams UI

changelog.d/3-bug-fixes/WPB-18929

@@ -0,0 +1,3 @@
+Revoking a pending SCIM invitation now removes the associated Brig account and
+Spar SCIM metadata synchronously, allowing the same SCIM user to be invited
+again.

integration/test/API/Brig.hs

@@ -990,6 +990,11 @@ getInvitationByCode user code = do
   req <- baseRequest user Brig Versioned $ joinHttpPath ["teams", "invitations", "info"]
   submit "GET" (req & addQueryParams [("code", code)])
 
+deleteTeamInvitation :: (HasCallStack, MakesValue user) => user -> String -> String -> App Response
+deleteTeamInvitation user tid iid = do
+  req <- baseRequest user Brig Versioned (joinHttpPath ["teams", tid, "invitations", iid])
+  submit "DELETE" req
+
 passwordReset :: (HasCallStack, MakesValue domain) => domain -> String -> App Response
 passwordReset domain email = do
   req <- baseRequest domain Brig Versioned "password-reset"

integration/test/Test/Spar.hs

@@ -131,6 +131,36 @@ testTeamInvitationWhenScimInvitationPending = do
   user %. "managed_by" `shouldMatch` "scim"
   user %. "status" `shouldMatch` "pending-invitation"
 
+testScimReinviteAfterRevoke :: (HasCallStack) => App ()
+testScimReinviteAfterRevoke = do
+  let settings =
+        def
+          { brigCfg =
+              -- Controls when asynchronous cleanup removes expired SCIM pending accounts.
+              setField "optSettings.setExpiredUserCleanupTimeout" (3600 :: Int)
+          }
+  withModifiedBackend settings $ \domain -> do
+    (owner, tid, _) <- createTeam domain 1
+    token <- createScimToken owner def >>= getJSON 200 >>= (%. "token") >>= asString
+
+    email <- randomEmail
+    externalId <- randomExternalId
+    scimUser <- randomScimUserWithEmail externalId email
+    scid <- createScimUser domain token scimUser >>= getJSON 201 >>= (%. "id") >>= asString
+    handle <- scimUser %. "userName" >>= asString
+
+    -- assert that the SCIM handle is claimed
+    putHandle owner handle >>= assertStatus 409
+
+    -- cancel the invitation
+    void $ Brig.listInvitations owner tid >>= getJSON 200 >>= (%. "invitations") >>= asList >>= assertOne
+    Brig.deleteTeamInvitation owner tid scid >>= assertSuccess
+    void $ Brig.listInvitations owner tid >>= getJSON 200 >>= (%. "invitations") >>= shouldBeEmpty
+
+    -- retry the invite should work
+    createScimUser domain token scimUser `bindResponse` \resp -> do
+      resp.status `shouldMatchInt` 201
+
 testTeamInvitationWhenScimAccountExists :: (HasCallStack) => App ()
 testTeamInvitationWhenScimAccountExists = do
   (owner, tid, _) <- createTeam OwnDomain 1

libs/wire-api/src/Wire/API/Routes/Internal/Spar.hs

@@ -32,6 +32,7 @@ type InternalAPI =
   "i"
     :> ( Named "i_status" ("status" :> Get '[JSON] NoContent)
            :<|> Named "i_delete_team" ("teams" :> Capture "team" TeamId :> DeleteNoContent)
+           :<|> Named "i_delete_scim_user" ("scim" :> "users" :> Capture "team" TeamId :> Capture "user" UserId :> DeleteNoContent)
            :<|> Named "i_put_sso_settings" ("sso" :> "settings" :> ReqBody '[JSON] SsoSettings :> Put '[JSON] NoContent)
            :<|> Named "i_post_scim_user_info" ("scim" :> "userinfo" :> Capture "user" UserId :> Post '[JSON] ScimUserInfo)
            :<|> Named "i_get_identity_providers" ("identity-providers" :> Capture "team" TeamId :> Get '[JSON] IdPList)

libs/wire-subsystems/src/Wire/SparAPIAccess.hs

@@ -27,6 +27,7 @@ import Wire.API.User.IdentityProvider
 data SparAPIAccess m a where
   GetIdentityProviders :: TeamId -> SparAPIAccess m IdPList
   DeleteTeam :: TeamId -> SparAPIAccess m ()
+  DeleteScimUser :: TeamId -> UserId -> SparAPIAccess m ()
   LookupScimUserInfo :: UserId -> SparAPIAccess m ScimUserInfo
 
 makeSem ''SparAPIAccess

libs/wire-subsystems/src/Wire/SparAPIAccess/Rpc.hs

@@ -49,6 +49,7 @@ interpretSparAPIAccessToRpc sparEndpoint =
     runInputConst sparEndpoint . \case
       GetIdentityProviders tid -> getIdentityProvidersImpl tid
       DeleteTeam tid -> deleteTeamImpl tid
+      DeleteScimUser tid uid -> deleteScimUserImpl tid uid
       LookupScimUserInfo uid -> lookupScimUserInfoImpl uid
 
 sparRequest ::
@@ -93,6 +94,15 @@ deleteTeamImpl tid = do
         . paths ["i", "teams", toByteString' tid]
         . expect2xx
 
+deleteScimUserImpl :: (Member (Input Endpoint) r, Member Rpc r) => TeamId -> UserId -> Sem r ()
+deleteScimUserImpl tid uid = do
+  void $ sparRequest delReq
+  where
+    delReq =
+      method DELETE
+        . paths ["i", "scim", "users", toByteString' tid, toByteString' uid]
+        . expect2xx
+
 -- | Get the SCIM user info for a user.
 lookupScimUserInfoImpl ::
   ( Member (Error ParseException) r,

libs/wire-subsystems/test/unit/Wire/MockInterpreters/SparAPIAccess.hs

@@ -31,6 +31,7 @@ miniSparAPIAccess = interpret $ \case
   GetIdentityProviders tid ->
     Map.findWithDefault (IdPList []) tid <$> input
   DeleteTeam {} -> error "DeleteTeam not implemented in miniSparAPIAccess"
+  DeleteScimUser {} -> error "DeleteScimUser not implemented in miniSparAPIAccess"
   LookupScimUserInfo {} -> error "LookupScimUserInfo not implemented in miniSparAPIAccess"
 
 emptySparAPIAccess :: InterpreterFor SparAPIAccess r

services/brig/src/Brig/Team/API.hs

@@ -69,6 +69,7 @@ import Wire.API.Team.Size
 import Wire.API.User hiding (fromEmail)
 import Wire.AuthenticationSubsystem
 import Wire.BlockListStore
+import Wire.ClientStore (ClientStore)
 import Wire.EmailSubsystem.Interpreter (renderInvitationUrl)
 import Wire.Error
 import Wire.Events (Events)
@@ -77,18 +78,25 @@ import Wire.GalleyAPIAccess qualified as GalleyAPIAccess
 import Wire.IndexedUserStore (IndexedUserStore, getTeamSize)
 import Wire.InvitationStore (InvitationStore (..), PaginatedResult (..), StoredInvitation (..))
 import Wire.InvitationStore qualified as Store
+import Wire.NotificationSubsystem (NotificationSubsystem)
+import Wire.PropertySubsystem (PropertySubsystem)
 import Wire.Sem.Concurrency
+import Wire.SparAPIAccess (SparAPIAccess)
+import Wire.SparAPIAccess qualified as SparAPIAccess
 import Wire.TeamInvitationSubsystem
 import Wire.TeamInvitationSubsystem.Interpreter (toInvitation)
 import Wire.TeamSubsystem (TeamSubsystem)
 import Wire.TeamSubsystem qualified as TeamSubsystem
+import Wire.UserGroupSubsystem (UserGroupSubsystem)
 import Wire.UserKeyStore
 import Wire.UserPendingActivationStore (UserPendingActivationStore)
+import Wire.UserPendingActivationStore qualified as UserPendingActivationStore
 import Wire.UserStore
 import Wire.UserSubsystem
 import Wire.UserSubsystem.Error
 
 servantAPI ::
+  forall p r.
   ( Member GalleyAPIAccess r,
     Member TeamInvitationSubsystem r,
     Member UserSubsystem r,
@@ -98,7 +106,18 @@ servantAPI ::
     Member (Input (Local ())) r,
     Member (Error UserSubsystemError) r,
     Member IndexedUserStore r,
-    Member TeamSubsystem r
+    Member TeamSubsystem r,
+    Member SparAPIAccess r,
+    Member (Embed App.HttpClientIO) r,
+    Member NotificationSubsystem r,
+    Member ClientStore r,
+    Member PropertySubsystem r,
+    Member UserGroupSubsystem r,
+    Member Events r,
+    Member AuthenticationSubsystem r,
+    Member UserStore r,
+    Member UserKeyStore r,
+    Member (UserPendingActivationStore p) r
   ) =>
   ServerT TeamsAPI (Handler r)
 servantAPI =
@@ -202,16 +221,55 @@ logInvitationRequest context action =
         pure (Right result)
 
 deleteInvitation ::
+  forall p r.
   ( Member InvitationStore r,
     Member (Error UserSubsystemError) r,
-    Member TeamSubsystem r
+    Member TeamSubsystem r,
+    Member SparAPIAccess r,
+    Member TinyLog r,
+    Member (Embed App.HttpClientIO) r,
+    Member NotificationSubsystem r,
+    Member ClientStore r,
+    Member PropertySubsystem r,
+    Member UserGroupSubsystem r,
+    Member Events r,
+    Member AuthenticationSubsystem r,
+    Member UserSubsystem r,
+    Member UserStore r,
+    Member UserKeyStore r,
+    Member (UserPendingActivationStore p) r,
+    Member (Input (Local ())) r
   ) =>
   UserId ->
   TeamId ->
   InvitationId ->
   Sem r ()
 deleteInvitation uid tid iid = do
   ensurePermissions uid tid [AddTeamMember]
+  mInvitation <- Store.lookupInvitation tid iid
+  let scimUid = invitationIdToUserId iid
+  mUser <- getAccountNoFilter =<< qualifyLocal' scimUid
+  for_ mUser $ \user ->
+    for_ (userEmail user) $ \email -> do
+      pendingScimUsers <- Store.lookupPendingScimUsers tid email
+      let invitationMatches = maybe True (\inv -> inv.email == email) mInvitation
+      when
+        ( userId user == scimUid
+            && user.userTeam == Just tid
+            && user.userManagedBy == ManagedByScim
+            && user.userStatus == PendingInvitation
+            && invitationMatches
+            && scimUid `elem` pendingScimUsers
+        )
+        $ do
+          -- Remove Spar's external-id mapping before deleting the Brig account.
+          -- Otherwise a SCIM retry still sees the old external ID as owned.
+          SparAPIAccess.deleteScimUser tid scimUid
+          UserPendingActivationStore.remove scimUid
+          -- Use the same complete deletion logic as the asynchronous user
+          -- deletion worker, but run it synchronously before the invitation is
+          -- removed so a replacement SCIM invitation can be created safely.
+          API.deleteAccount user
   Store.deleteInvitation tid iid
 
 listInvitations ::

services/spar/src/Spar/API.hs

@@ -263,6 +263,7 @@ apiINTERNAL ::
     Member IdPConfigStore r,
     Member (Error SparError) r,
     Member SAMLUserStore r,
+    Member ScimExternalIdStore r,
     Member ScimUserMetaStore r,
     Member (Logger (Msg -> Msg)) r,
     Member Random r,
@@ -273,6 +274,7 @@ apiINTERNAL ::
 apiINTERNAL =
   Named @"i_status" internalStatus
     :<|> Named @"i_delete_team" internalDeleteTeam
+    :<|> Named @"i_delete_scim_user" internalDeleteScimUser
     :<|> Named @"i_put_sso_settings" internalPutSsoSettings
     :<|> Named @"i_post_scim_user_info" internalGetScimUserInfo
     :<|> Named @"i_get_identity_providers" idpGetAllByTeamId
@@ -1132,6 +1134,30 @@ internalDeleteTeam teamId = do
   deleteTeam teamId
   pure NoContent
 
+internalDeleteScimUser ::
+  ( Member BrigAPIAccess r,
+    Member ScimExternalIdStore r,
+    Member ScimUserMetaStore r,
+    Member SAMLUserStore r,
+    Member (Logger (Msg -> Msg)) r
+  ) =>
+  TeamId ->
+  UserId ->
+  Sem r NoContent
+internalDeleteScimUser teamId uid = do
+  Logger.info $
+    Log.msg ("Attempting to delete SCIM user data" :: String)
+      . Log.field "team" (idToText teamId)
+      . Log.field "user" (idToText uid)
+  BrigAPIAccess.getAccount WithPendingInvitations uid >>= \case
+    Just user
+      | userTeam user == Just teamId
+          && userManagedBy user == ManagedByScim
+          && userStatus user == PendingInvitation ->
+          deleteScimUserData teamId user
+    _ -> pure ()
+  pure NoContent
+
 internalPutSsoSettings ::
   ( Member DefaultSsoCode r,
     Member (Error SparError) r,

services/spar/src/Spar/Scim/User.hs

@@ -41,6 +41,7 @@ module Spar.Scim.User
     mkValidScimId,
     scimFindUserByExternalId,
     deleteScimUser,
+    deleteScimUserData,
   )
 where
 
@@ -899,6 +900,20 @@ deleteScimUser tokeninfo@ScimTokenInfo {stiTeam, stiIdP} uid =
           ScimExternalIdStore.delete stiTeam veid.validScimIdExternal
       lift $ ScimUserMetaStore.delete uid
 
+deleteScimUserData ::
+  ( Member ScimExternalIdStore r,
+    Member ScimUserMetaStore r,
+    Member SAMLUserStore r
+  ) =>
+  TeamId ->
+  User ->
+  Sem r ()
+deleteScimUserData teamId account = do
+  for_ (Intra.oldVeidFromBrigUser account) $ \veid -> do
+    for_ (justThere veid.validScimIdAuthInfo) (SAMLUserStore.delete (userId account))
+    ScimExternalIdStore.delete teamId veid.validScimIdExternal
+  ScimUserMetaStore.delete (userId account)
+
 ----------------------------------------------------------------------------
 -- Utilities