From aba0fc02ef0fe31810b3b510ab0bdf728ddeb44c Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sat, 29 May 2021 15:38:16 +0200 Subject: [PATCH 01/15] Update type. --- libs/wire-api/src/Wire/API/Connection.hs | 25 ++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/libs/wire-api/src/Wire/API/Connection.hs b/libs/wire-api/src/Wire/API/Connection.hs index 9eefcd75dd3..347e2f490f2 100644 --- a/libs/wire-api/src/Wire/API/Connection.hs +++ b/libs/wire-api/src/Wire/API/Connection.hs @@ -28,6 +28,8 @@ module Wire.API.Connection UserConnectionList (..), Message (..), Relation (..), + RelationWithHistory (..), + relationDropHistory, -- * Requests ConnectionRequest (..), @@ -165,6 +167,29 @@ data Relation deriving (Arbitrary) via (GenericUniform Relation) deriving (ToSchema) via (CustomSwagger '[ConstructorTagModifier CamelToKebab] Relation) +-- | 'updateConnectionInternal', requires knowledge of the previous state (before +-- 'MissingLegalholdConsent'), but the clients don't need that information. To avoid having +-- to change the API, we introduce an internal variant of 'Relation' with surjective mapping +-- 'relationDropHistory'. +data RelationWithHistory + = AcceptedWithHistory + | BlockedWithHistory + | PendingWithHistory + | IgnoredWithHistory + | SentWithHistory + | CancelledWithHistory + | MissingLegalholdConsentFromAccepted + | MissingLegalholdConsentFromBlocked + | MissingLegalholdConsentFromPending + | MissingLegalholdConsentFromIgnored + | MissingLegalholdConsentFromSent + | MissingLegalholdConsentFromCancelled + deriving stock (Eq, Ord, Show, Generic) + deriving (Arbitrary) via (GenericUniform RelationWithHistory) + +relationDropHistory :: RelationWithHistory -> Relation +relationDropHistory = undefined + typeRelation :: Doc.DataType typeRelation = Doc.string $ From c90fe3ed0501d8bde7c79e9a2d183abbdad54134 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sat, 29 May 2021 16:13:38 +0200 Subject: [PATCH 02/15] ... --- services/brig/src/Brig/Data/Connection.hs | 35 ++++++++++++----------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/services/brig/src/Brig/Data/Connection.hs b/services/brig/src/Brig/Data/Connection.hs index c020f779fa6..25807c81f21 100644 --- a/services/brig/src/Brig/Data/Connection.hs +++ b/services/brig/src/Brig/Data/Connection.hs @@ -46,6 +46,7 @@ import Data.Range import Data.Time (getCurrentTime) import Imports import UnliftIO.Async (pooledMapConcurrentlyN_) +import Wire.API.Connection connectUsers :: UserId -> [(UserId, ConvId)] -> AppIO [UserConnection] connectUsers from to = do @@ -66,7 +67,7 @@ insertConnection :: UserId -> -- | To UserId -> - Relation -> + RelationWithHistory -> Maybe Message -> ConvId -> AppIO UserConnection @@ -75,7 +76,7 @@ insertConnection from to status msg cid = do retry x5 . write connectionInsert $ params Quorum (from, to, status, now, msg, cid) return $ toUserConnection (from, to, status, now, msg, Just cid) -updateConnection :: UserConnection -> Relation -> AppIO UserConnection +updateConnection :: UserConnection -> RelationWithHistory -> AppIO UserConnection updateConnection c@UserConnection {..} status = do now <- toUTCTimeMillis <$> liftIO getCurrentTime retry x5 . write connectionUpdate $ params Quorum (status, now, ucFrom, ucTo) @@ -119,20 +120,22 @@ lookupContactList u = -- | For a given user 'A', lookup the list of users that form his contact list, -- i.e. the users to whom 'A' has an outgoing 'Accepted' relation (A -> B). -lookupContactListWithRelation :: UserId -> AppIO [(UserId, Relation)] +lookupContactListWithRelation :: UserId -> AppIO [(UserId, RelationWithHistory)] lookupContactListWithRelation u = retry x1 (query contactsSelect (params Quorum (Identity u))) -- | Count the number of connections a user has in a specific relation status. +-- (If you want to distinguish 'RelationWithHistory', write a new function.) -- Note: The count is eventually consistent. countConnections :: UserId -> [Relation] -> AppIO Int64 countConnections u r = do rels <- retry x1 . query selectStatus $ params One (Identity u) return $ foldl' count 0 rels where - selectStatus :: QueryString R (Identity UserId) (Identity Relation) + selectStatus :: QueryString R (Identity UserId) (Identity RelationWithHistory) selectStatus = "SELECT status FROM connection WHERE left = ?" - count n (Identity s) | s `elem` r = n + 1 + + count n (Identity s) | (relationDropHistory s) `elem` r = n + 1 count n _ = n deleteConnections :: UserId -> AppIO () @@ -146,25 +149,25 @@ deleteConnections u = do -- Queries -connectionInsert :: PrepQuery W (UserId, UserId, Relation, UTCTimeMillis, Maybe Message, ConvId) () +connectionInsert :: PrepQuery W (UserId, UserId, RelationWithHistory, UTCTimeMillis, Maybe Message, ConvId) () connectionInsert = "INSERT INTO connection (left, right, status, last_update, message, conv) VALUES (?, ?, ?, ?, ?, ?)" -connectionUpdate :: PrepQuery W (Relation, UTCTimeMillis, UserId, UserId) () +connectionUpdate :: PrepQuery W (RelationWithHistory, UTCTimeMillis, UserId, UserId) () connectionUpdate = "UPDATE connection SET status = ?, last_update = ? WHERE left = ? AND right = ?" -connectionSelect :: PrepQuery R (UserId, UserId) (UserId, UserId, Relation, UTCTimeMillis, Maybe Message, Maybe ConvId) +connectionSelect :: PrepQuery R (UserId, UserId) (UserId, UserId, RelationWithHistory, UTCTimeMillis, Maybe Message, Maybe ConvId) connectionSelect = "SELECT left, right, status, last_update, message, conv FROM connection WHERE left = ? AND right = ?" -connectionStatusSelect :: PrepQuery R ([UserId], [UserId]) (UserId, UserId, Relation) +connectionStatusSelect :: PrepQuery R ([UserId], [UserId]) (UserId, UserId, RelationWithHistory) connectionStatusSelect = "SELECT left, right, status FROM connection WHERE left IN ? AND right IN ?" -contactsSelect :: PrepQuery R (Identity UserId) (UserId, Relation) +contactsSelect :: PrepQuery R (Identity UserId) (UserId, RelationWithHistory) contactsSelect = "SELECT right, status FROM connection WHERE left = ?" -connectionsSelect :: PrepQuery R (Identity UserId) (UserId, UserId, Relation, UTCTimeMillis, Maybe Message, Maybe ConvId) +connectionsSelect :: PrepQuery R (Identity UserId) (UserId, UserId, RelationWithHistory, UTCTimeMillis, Maybe Message, Maybe ConvId) connectionsSelect = "SELECT left, right, status, last_update, message, conv FROM connection WHERE left = ? ORDER BY right ASC" -connectionsSelectFrom :: PrepQuery R (UserId, UserId) (UserId, UserId, Relation, UTCTimeMillis, Maybe Message, Maybe ConvId) +connectionsSelectFrom :: PrepQuery R (UserId, UserId) (UserId, UserId, RelationWithHistory, UTCTimeMillis, Maybe Message, Maybe ConvId) connectionsSelectFrom = "SELECT left, right, status, last_update, message, conv FROM connection WHERE left = ? AND right > ? ORDER BY right ASC" connectionDelete :: PrepQuery W (UserId, UserId) () @@ -175,8 +178,8 @@ connectionClear = "DELETE FROM connection WHERE left = ?" -- Conversions -toUserConnection :: (UserId, UserId, Relation, UTCTimeMillis, Maybe Message, Maybe ConvId) -> UserConnection -toUserConnection (l, r, rel, time, msg, cid) = UserConnection l r rel time msg cid +toUserConnection :: (UserId, UserId, RelationWithHistory, UTCTimeMillis, Maybe Message, Maybe ConvId) -> UserConnection +toUserConnection (l, r, relationDropHistory -> rel, time, msg, cid) = UserConnection l r rel time msg cid -toConnectionStatus :: (UserId, UserId, Relation) -> ConnectionStatus -toConnectionStatus (l, r, rel) = ConnectionStatus l r rel +toConnectionStatus :: (UserId, UserId, RelationWithHistory) -> ConnectionStatus +toConnectionStatus (l, r, relationDropHistory -> rel) = ConnectionStatus l r rel From 319a863a1d81649cd9815f8c9e7e064d144f4494 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Sat, 29 May 2021 15:40:58 +0200 Subject: [PATCH 03/15] implement relationDropHistory --- libs/wire-api/src/Wire/API/Connection.hs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/libs/wire-api/src/Wire/API/Connection.hs b/libs/wire-api/src/Wire/API/Connection.hs index 347e2f490f2..46e51e67d05 100644 --- a/libs/wire-api/src/Wire/API/Connection.hs +++ b/libs/wire-api/src/Wire/API/Connection.hs @@ -188,7 +188,19 @@ data RelationWithHistory deriving (Arbitrary) via (GenericUniform RelationWithHistory) relationDropHistory :: RelationWithHistory -> Relation -relationDropHistory = undefined +relationDropHistory = \case + AcceptedWithHistory -> Accepted + BlockedWithHistory -> Blocked + PendingWithHistory -> Pending + IgnoredWithHistory -> Ignored + SentWithHistory -> Sent + CancelledWithHistory -> Cancelled + MissingLegalholdConsentFromAccepted -> MissingLegalholdConsent + MissingLegalholdConsentFromBlocked -> MissingLegalholdConsent + MissingLegalholdConsentFromPending -> MissingLegalholdConsent + MissingLegalholdConsentFromIgnored -> MissingLegalholdConsent + MissingLegalholdConsentFromSent -> MissingLegalholdConsent + MissingLegalholdConsentFromCancelled -> MissingLegalholdConsent typeRelation :: Doc.DataType typeRelation = From 0dcaa4233bafc6e3fffe0e4254690832b2611e74 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Sat, 29 May 2021 16:44:19 +0200 Subject: [PATCH 04/15] Relation -> RelationHistory + TODOs --- services/brig/src/Brig/API/Connection.hs | 37 +++++++++--------- services/brig/src/Brig/Data/Connection.hs | 8 ++-- services/brig/src/Brig/Data/Instances.hs | 47 ++++++++++++++--------- services/brig/src/Brig/User/EJPD.hs | 6 +-- 4 files changed, 55 insertions(+), 43 deletions(-) diff --git a/services/brig/src/Brig/API/Connection.hs b/services/brig/src/Brig/API/Connection.hs index 9ebb35ff5d2..37933d24061 100644 --- a/services/brig/src/Brig/API/Connection.hs +++ b/services/brig/src/Brig/API/Connection.hs @@ -59,6 +59,7 @@ import qualified Galley.Types.Teams as Team import Imports import qualified System.Logger.Class as Log import System.Logger.Message +import Wire.API.Connection (RelationWithHistory (..)) import qualified Wire.API.Conversation as Conv createConnection :: @@ -106,8 +107,8 @@ createConnectionToLocalUser self crUser ConnectionRequest {crName, crMessage} co logConnection self crUser . msg (val "Creating connection") cnv <- Intra.createConnectConv self crUser (Just crName) (Just crMessage) (Just conn) - s2o' <- Data.insertConnection self crUser Sent (Just crMessage) cnv - o2s' <- Data.insertConnection crUser self Pending (Just crMessage) cnv + s2o' <- Data.insertConnection self crUser SentWithHistory (Just crMessage) cnv + o2s' <- Data.insertConnection crUser self PendingWithHistory (Just crMessage) cnv e2o <- ConnectionUpdated o2s' (ucStatus <$> o2s) <$> Data.lookupName self let e2s = ConnectionUpdated s2o' (ucStatus <$> s2o) Nothing mapM_ (Intra.onConnectionEvent self (Just conn)) [e2o, e2s] @@ -121,7 +122,7 @@ createConnectionToLocalUser self crUser ConnectionRequest {crName, crMessage} co (Accepted, Blocked) -> return $ ConnectionExists s2o (Sent, Blocked) -> return $ ConnectionExists s2o (Blocked, _) -> throwE $ InvalidTransition self Sent - (_, Blocked) -> change s2o Sent + (_, Blocked) -> change s2o SentWithHistory (_, Sent) -> accept s2o o2s (_, Accepted) -> accept s2o o2s (_, Ignored) -> resend s2o o2s @@ -136,12 +137,12 @@ createConnectionToLocalUser self crUser ConnectionRequest {crName, crMessage} co logConnection self (ucTo s2o) . msg (val "Accepting connection") cnv <- lift $ for (ucConvId s2o) $ Intra.acceptConnectConv self (Just conn) - s2o' <- lift $ Data.updateConnection s2o Accepted + s2o' <- lift $ Data.updateConnection s2o AcceptedWithHistory o2s' <- lift $ if (cnvType <$> cnv) == Just ConnectConv - then Data.updateConnection o2s Blocked - else Data.updateConnection o2s Accepted + then Data.updateConnection o2s BlockedWithHistory + else Data.updateConnection o2s AcceptedWithHistory e2o <- lift $ ConnectionUpdated o2s' (Just $ ucStatus o2s) <$> Data.lookupName self let e2s = ConnectionUpdated s2o' (Just $ ucStatus s2o) Nothing lift $ mapM_ (Intra.onConnectionEvent self (Just conn)) [e2o, e2s] @@ -157,7 +158,7 @@ createConnectionToLocalUser self crUser ConnectionRequest {crName, crMessage} co s2o' <- insert (Just s2o) (Just o2s) return $ ConnectionExists s2o' - change :: UserConnection -> Relation -> ExceptT ConnectionError AppIO ConnectionResult + change :: UserConnection -> RelationWithHistory -> ExceptT ConnectionError AppIO ConnectionResult change c s = ConnectionExists <$> lift (Data.updateConnection c s) belongSameTeam :: AppIO Bool @@ -270,11 +271,11 @@ updateConnection self other newStatus conn = do when (ucStatus o2s `elem` [Sent, Pending]) . lift $ do o2s' <- if (cnvType <$> cnv) /= Just ConnectConv - then Data.updateConnection o2s Accepted - else Data.updateConnection o2s Blocked + then Data.updateConnection o2s AcceptedWithHistory + else Data.updateConnection o2s BlockedWithHistory e2o <- ConnectionUpdated o2s' (Just $ ucStatus o2s) <$> Data.lookupName self Intra.onConnectionEvent self conn e2o - lift $ Just <$> Data.updateConnection s2o Accepted + lift $ Just <$> Data.updateConnection s2o AcceptedWithHistory block :: UserConnection -> ExceptT ConnectionError AppIO (Maybe UserConnection) block s2o = lift $ do @@ -282,7 +283,7 @@ updateConnection self other newStatus conn = do logConnection self (ucTo s2o) . msg (val "Blocking connection") for_ (ucConvId s2o) $ Intra.blockConv (ucFrom s2o) conn - Just <$> Data.updateConnection s2o Blocked + Just <$> Data.updateConnection s2o BlockedWithHistory unblock :: UserConnection -> UserConnection -> Relation -> ExceptT ConnectionError AppIO (Maybe UserConnection) unblock s2o o2s new = do @@ -295,12 +296,12 @@ updateConnection self other newStatus conn = do when (ucStatus o2s == Sent && new == Accepted) . lift $ do o2s' :: UserConnection <- if (cnvType <$> cnv) /= Just ConnectConv - then Data.updateConnection o2s Accepted - else Data.updateConnection o2s Blocked + then Data.updateConnection o2s AcceptedWithHistory + else Data.updateConnection o2s BlockedWithHistory e2o :: ConnectionEvent <- ConnectionUpdated o2s' (Just $ ucStatus o2s) <$> Data.lookupName self -- TODO: is this correct? shouldnt o2s be sent to other? Intra.onConnectionEvent self conn e2o - lift $ Just <$> Data.updateConnection s2o new + lift $ Just <$> Data.updateConnection s2o ((error "TODO") new) cancel :: UserConnection -> UserConnection -> ExceptT ConnectionError AppIO (Maybe UserConnection) cancel s2o o2s = do @@ -308,13 +309,13 @@ updateConnection self other newStatus conn = do logConnection self (ucTo s2o) . msg (val "Cancelling connection") lift . for_ (ucConvId s2o) $ Intra.blockConv (ucFrom s2o) conn - o2s' <- lift $ Data.updateConnection o2s Cancelled + o2s' <- lift $ Data.updateConnection o2s CancelledWithHistory let e2o = ConnectionUpdated o2s' (Just $ ucStatus o2s) Nothing lift $ Intra.onConnectionEvent self conn e2o change s2o Cancelled change :: UserConnection -> Relation -> ExceptT ConnectionError AppIO (Maybe UserConnection) - change c s = lift $ Just <$> Data.updateConnection c s + change c s = lift $ Just <$> Data.updateConnection c ((error "TODO") s) connection :: UserId -> UserId -> ExceptT ConnectionError AppIO UserConnection connection a b = lift (Data.lookupConnection a b) >>= tryJust (NotConnected a b) @@ -339,7 +340,7 @@ updateConnectionInternal = \case for_ [s2o, o2s] $ \(uconn :: UserConnection) -> lift $ do -- TODO: check if Ignored is a possibility Intra.blockConv (ucFrom uconn) Nothing `mapM_` ucConvId uconn - uconn' <- Data.updateConnection uconn MissingLegalholdConsent + uconn' <- Data.updateConnection uconn ((error "TODO") MissingLegalholdConsent) let ev = ConnectionUpdated uconn' (Just $ ucStatus uconn) Nothing Intra.onConnectionEvent self Nothing ev @@ -379,7 +380,7 @@ updateConnectionInternal = \case Just One2OneConv -> pure Accepted Just ConnectConv -> pure Sent Nothing -> throwE (InvalidTransition (ucFrom uconn) Accepted) - lift $ Data.updateConnection uconnRev newRelation + lift $ Data.updateConnection uconnRev ((error "TODO") newRelation) connEvent :: ConnectionEvent <- lift $ ConnectionUpdated uconnRev' (Just $ ucStatus uconnRev) <$> Data.lookupName (ucFrom uconn) lift $ Intra.onConnectionEvent (ucFrom uconn) Nothing connEvent diff --git a/services/brig/src/Brig/Data/Connection.hs b/services/brig/src/Brig/Data/Connection.hs index 25807c81f21..4fba3117fcb 100644 --- a/services/brig/src/Brig/Data/Connection.hs +++ b/services/brig/src/Brig/Data/Connection.hs @@ -55,8 +55,8 @@ connectUsers from to = do setType BatchLogged setConsistency Quorum forM_ to $ \(u, c) -> do - addPrepQuery connectionInsert (from, u, Accepted, now, Nothing, c) - addPrepQuery connectionInsert (u, from, Accepted, now, Nothing, c) + addPrepQuery connectionInsert (from, u, AcceptedWithHistory, now, Nothing, c) + addPrepQuery connectionInsert (u, from, AcceptedWithHistory, now, Nothing, c) return . concat . (`map` to) $ \(u, c) -> [ UserConnection from u Accepted now Nothing (Just c), UserConnection u from Accepted now Nothing (Just c) @@ -82,7 +82,7 @@ updateConnection c@UserConnection {..} status = do retry x5 . write connectionUpdate $ params Quorum (status, now, ucFrom, ucTo) return $ c - { ucStatus = status, + { ucStatus = relationDropHistory status, ucLastUpdate = now } @@ -116,7 +116,7 @@ lookupConnectionStatus from to = -- | See 'lookupContactListWithRelation'. lookupContactList :: UserId -> AppIO [UserId] lookupContactList u = - fst <$$> (filter ((== Accepted) . snd) <$> lookupContactListWithRelation u) + fst <$$> (filter ((== AcceptedWithHistory) . snd) <$> lookupContactListWithRelation u) -- | For a given user 'A', lookup the list of users that form his contact list, -- i.e. the users to whom 'A' has an outgoing 'Accepted' relation (A -> B). diff --git a/services/brig/src/Brig/Data/Instances.hs b/services/brig/src/Brig/Data/Instances.hs index aee068550ec..b2822916233 100644 --- a/services/brig/src/Brig/Data/Instances.hs +++ b/services/brig/src/Brig/Data/Instances.hs @@ -37,6 +37,7 @@ import Data.Range () import Data.String.Conversions (LBS, ST, cs) import Data.Text.Ascii () import Imports +import Wire.API.Connection (RelationWithHistory (..)) import Wire.API.User.RichInfo deriving instance Cql Name @@ -83,27 +84,37 @@ instance Cql UserSSOId where toCql = toCql . cs @LBS @ST . encode -instance Cql Relation where +instance Cql RelationWithHistory where ctype = Tagged IntColumn fromCql (CqlInt i) = case i of - 0 -> return Accepted - 1 -> return Blocked - 2 -> return Pending - 3 -> return Ignored - 4 -> return Sent - 5 -> return Cancelled - 6 -> return MissingLegalholdConsent - n -> Left $ "unexpected relation: " ++ show n - fromCql _ = Left "relation: int expected" - - toCql Accepted = CqlInt 0 - toCql Blocked = CqlInt 1 - toCql Pending = CqlInt 2 - toCql Ignored = CqlInt 3 - toCql Sent = CqlInt 4 - toCql Cancelled = CqlInt 5 - toCql MissingLegalholdConsent = CqlInt 6 + 0 -> pure AcceptedWithHistory + 1 -> pure BlockedWithHistory + 2 -> pure PendingWithHistory + 3 -> pure IgnoredWithHistory + 4 -> pure SentWithHistory + 5 -> pure CancelledWithHistory + 6 -> pure MissingLegalholdConsentFromAccepted + 7 -> pure MissingLegalholdConsentFromBlocked + 8 -> pure MissingLegalholdConsentFromPending + 9 -> pure MissingLegalholdConsentFromIgnored + 10 -> pure MissingLegalholdConsentFromSent + 11 -> pure MissingLegalholdConsentFromCancelled + n -> Left $ "unexpected RelationWithHistory: " ++ show n + fromCql _ = Left "RelationWithHistory: int expected" + + toCql AcceptedWithHistory = CqlInt 0 + toCql BlockedWithHistory = CqlInt 1 + toCql PendingWithHistory = CqlInt 2 + toCql IgnoredWithHistory = CqlInt 3 + toCql SentWithHistory = CqlInt 4 + toCql CancelledWithHistory = CqlInt 5 + toCql MissingLegalholdConsentFromAccepted = CqlInt 6 + toCql MissingLegalholdConsentFromBlocked = CqlInt 7 + toCql MissingLegalholdConsentFromPending = CqlInt 8 + toCql MissingLegalholdConsentFromIgnored = CqlInt 9 + toCql MissingLegalholdConsentFromSent = CqlInt 10 + toCql MissingLegalholdConsentFromCancelled = CqlInt 11 -- DEPRECATED instance Cql Pict where diff --git a/services/brig/src/Brig/User/EJPD.hs b/services/brig/src/Brig/User/EJPD.hs index 706f90c3f16..f41d885adeb 100644 --- a/services/brig/src/Brig/User/EJPD.hs +++ b/services/brig/src/Brig/User/EJPD.hs @@ -35,7 +35,7 @@ import Data.Id (UserId) import qualified Data.Set as Set import Imports hiding (head) import Servant.Swagger.Internal.Orphans () -import Wire.API.Connection (Relation) +import Wire.API.Connection (Relation, RelationWithHistory (..), relationDropHistory) import qualified Wire.API.Push.Token as PushTok import qualified Wire.API.Team.Member as Team import Wire.API.User (User, userDisplayName, userEmail, userHandle, userId, userPhone, userTeam) @@ -62,11 +62,11 @@ ejpdRequest includeContacts (EJPDRequestBody handles) = do mbContacts <- if includeContacts' then do - contacts :: [(UserId, Relation)] <- + contacts :: [(UserId, RelationWithHistory)] <- Conn.lookupContactListWithRelation uid contactsFull :: [Maybe (Relation, EJPDResponseItem)] <- - forM contacts $ \(uid', rel) -> do + forM contacts $ \(uid', relationDropHistory -> rel) -> do mbUsr <- lookupUser NoPendingInvitations uid' maybe (pure Nothing) (\usr -> Just . (rel,) <$> go2 False usr) mbUsr From 22b22da156141b29d1b0ffee723a7f16ba3e2e5d Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Sat, 29 May 2021 17:23:16 +0200 Subject: [PATCH 05/15] work on TODOs --- services/brig/src/Brig/API/Connection.hs | 29 ++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/services/brig/src/Brig/API/Connection.hs b/services/brig/src/Brig/API/Connection.hs index 37933d24061..8b594805efd 100644 --- a/services/brig/src/Brig/API/Connection.hs +++ b/services/brig/src/Brig/API/Connection.hs @@ -287,6 +287,7 @@ updateConnection self other newStatus conn = do unblock :: UserConnection -> UserConnection -> Relation -> ExceptT ConnectionError AppIO (Maybe UserConnection) unblock s2o o2s new = do + -- FUTUREWORK: new is always in [Sent, Accepted]. Refactor to total function. when (new `elem` [Sent, Accepted]) $ checkLimit self Log.info $ @@ -301,7 +302,7 @@ updateConnection self other newStatus conn = do e2o :: ConnectionEvent <- ConnectionUpdated o2s' (Just $ ucStatus o2s) <$> Data.lookupName self -- TODO: is this correct? shouldnt o2s be sent to other? Intra.onConnectionEvent self conn e2o - lift $ Just <$> Data.updateConnection s2o ((error "TODO") new) + lift $ Just <$> Data.updateConnection s2o (mkRelationWithHistory (error "impossible") new) cancel :: UserConnection -> UserConnection -> ExceptT ConnectionError AppIO (Maybe UserConnection) cancel s2o o2s = do @@ -315,11 +316,31 @@ updateConnection self other newStatus conn = do change s2o Cancelled change :: UserConnection -> Relation -> ExceptT ConnectionError AppIO (Maybe UserConnection) - change c s = lift $ Just <$> Data.updateConnection c ((error "TODO") s) + change c s = do + -- FUTUREWORK: refactor to total function. Gets only called with either Ignored, Accepted, Cancelled + lift $ Just <$> Data.updateConnection c (mkRelationWithHistory (error "impossible") s) connection :: UserId -> UserId -> ExceptT ConnectionError AppIO UserConnection connection a b = lift (Data.lookupConnection a b) >>= tryJust (NotConnected a b) +mkRelationWithHistory :: HasCallStack => Relation -> Relation -> RelationWithHistory +mkRelationWithHistory oldRel = \case + Accepted -> AcceptedWithHistory + Blocked -> BlockedWithHistory + Pending -> PendingWithHistory + Ignored -> IgnoredWithHistory + Sent -> SentWithHistory + Cancelled -> CancelledWithHistory + MissingLegalholdConsent -> + case oldRel of + Accepted -> MissingLegalholdConsentFromAccepted + Blocked -> MissingLegalholdConsentFromBlocked + Pending -> MissingLegalholdConsentFromPending + Ignored -> MissingLegalholdConsentFromIgnored + Sent -> MissingLegalholdConsentFromSent + Cancelled -> MissingLegalholdConsentFromCancelled + MissingLegalholdConsent -> error "impossible old relation" + updateConnectionInternal :: UpdateConnectionsInternal -> ExceptT ConnectionError AppIO () @@ -338,9 +359,8 @@ updateConnectionInternal = \case s2o <- connection self other o2s <- connection other self for_ [s2o, o2s] $ \(uconn :: UserConnection) -> lift $ do - -- TODO: check if Ignored is a possibility Intra.blockConv (ucFrom uconn) Nothing `mapM_` ucConvId uconn - uconn' <- Data.updateConnection uconn ((error "TODO") MissingLegalholdConsent) + uconn' <- Data.updateConnection uconn (mkRelationWithHistory (ucStatus uconn) MissingLegalholdConsent) let ev = ConnectionUpdated uconn' (Just $ ucStatus uconn) Nothing Intra.onConnectionEvent self Nothing ev @@ -373,6 +393,7 @@ updateConnectionInternal = \case unblockDirected :: UserConnection -> UserConnection -> ExceptT ConnectionError AppIO () unblockDirected uconn uconnRev = do cnv :: Maybe Conv.Conversation <- lift . for (ucConvId uconn) $ Intra.unblockConv (ucFrom uconn) Nothing + uconnRev' :: UserConnection <- do newRelation <- case cnvType <$> cnv of Just RegularConv -> throwE (InvalidTransition (ucFrom uconn) Accepted) -- (impossible, connection conv is always 1:1) From 57e90fbc6c773831d3e7f46722ca33e4ee4634e0 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sat, 29 May 2021 17:58:25 +0200 Subject: [PATCH 06/15] work on TODO --- services/brig/src/Brig/API/Connection.hs | 27 ++++++++++++++++++++--- services/brig/src/Brig/Data/Connection.hs | 15 +++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/services/brig/src/Brig/API/Connection.hs b/services/brig/src/Brig/API/Connection.hs index 8b594805efd..0c59f3f0a51 100644 --- a/services/brig/src/Brig/API/Connection.hs +++ b/services/brig/src/Brig/API/Connection.hs @@ -394,18 +394,39 @@ updateConnectionInternal = \case unblockDirected uconn uconnRev = do cnv :: Maybe Conv.Conversation <- lift . for (ucConvId uconn) $ Intra.unblockConv (ucFrom uconn) Nothing + uconnRevRel :: RelationWithHistory <- relationWithHistory (ucFrom uconnRev) (ucTo uconnRev) uconnRev' :: UserConnection <- do newRelation <- case cnvType <$> cnv of Just RegularConv -> throwE (InvalidTransition (ucFrom uconn) Accepted) -- (impossible, connection conv is always 1:1) Just SelfConv -> throwE (InvalidTransition (ucFrom uconn) Accepted) - Just One2OneConv -> pure Accepted - Just ConnectConv -> pure Sent + Just One2OneConv -> pure AcceptedWithHistory + Just ConnectConv -> pure $ undoRelationHistory uconnRevRel Nothing -> throwE (InvalidTransition (ucFrom uconn) Accepted) - lift $ Data.updateConnection uconnRev ((error "TODO") newRelation) + lift $ Data.updateConnection uconnRev newRelation connEvent :: ConnectionEvent <- lift $ ConnectionUpdated uconnRev' (Just $ ucStatus uconnRev) <$> Data.lookupName (ucFrom uconn) lift $ Intra.onConnectionEvent (ucFrom uconn) Nothing connEvent + relationWithHistory :: UserId -> UserId -> ExceptT ConnectionError AppIO RelationWithHistory + relationWithHistory a b = lift (Data.lookupRelationWithHistory a b) >>= tryJust (NotConnected a b) + + undoRelationHistory :: RelationWithHistory -> RelationWithHistory + undoRelationHistory = \case + -- these cases are relevant. + MissingLegalholdConsentFromAccepted -> AcceptedWithHistory + MissingLegalholdConsentFromBlocked -> BlockedWithHistory + MissingLegalholdConsentFromPending -> PendingWithHistory + MissingLegalholdConsentFromIgnored -> IgnoredWithHistory + MissingLegalholdConsentFromSent -> SentWithHistory + MissingLegalholdConsentFromCancelled -> CancelledWithHistory + -- these cases should not be reachable, but if they are, this is probably what is expected from this function. + AcceptedWithHistory -> AcceptedWithHistory + BlockedWithHistory -> BlockedWithHistory + PendingWithHistory -> PendingWithHistory + IgnoredWithHistory -> IgnoredWithHistory + SentWithHistory -> SentWithHistory + CancelledWithHistory -> CancelledWithHistory + autoConnect :: UserId -> Set UserId -> diff --git a/services/brig/src/Brig/Data/Connection.hs b/services/brig/src/Brig/Data/Connection.hs index 4fba3117fcb..17b89549123 100644 --- a/services/brig/src/Brig/Data/Connection.hs +++ b/services/brig/src/Brig/Data/Connection.hs @@ -23,6 +23,7 @@ module Brig.Data.Connection insertConnection, updateConnection, lookupConnection, + lookupRelationWithHistory, lookupConnections, lookupConnectionStatus, lookupContactList, @@ -97,6 +98,17 @@ lookupConnection from to = liftM toUserConnection <$> retry x1 (query1 connectionSelect (params Quorum (from, to))) +-- | 'lookupConnection' with more 'Relation' info. +lookupRelationWithHistory :: + -- | User 'A' + UserId -> + -- | User 'B' + UserId -> + AppIO (Maybe RelationWithHistory) +lookupRelationWithHistory from to = + liftM runIdentity + <$> retry x1 (query1 relationSelect (params Quorum (from, to))) + -- | For a given user 'A', lookup his outgoing connections (A -> X) to other users. lookupConnections :: UserId -> Maybe UserId -> Range 1 500 Int32 -> AppIO (ResultPage UserConnection) lookupConnections from start (fromRange -> size) = @@ -158,6 +170,9 @@ connectionUpdate = "UPDATE connection SET status = ?, last_update = ? WHERE left connectionSelect :: PrepQuery R (UserId, UserId) (UserId, UserId, RelationWithHistory, UTCTimeMillis, Maybe Message, Maybe ConvId) connectionSelect = "SELECT left, right, status, last_update, message, conv FROM connection WHERE left = ? AND right = ?" +relationSelect :: PrepQuery R (UserId, UserId) (Identity RelationWithHistory) +relationSelect = "SELECT status FROM connection WHERE left = ? AND right = ?" + connectionStatusSelect :: PrepQuery R ([UserId], [UserId]) (UserId, UserId, RelationWithHistory) connectionStatusSelect = "SELECT left, right, status FROM connection WHERE left IN ? AND right IN ?" From 5f70c158aa108979360f602a51afbc172da564d4 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Sat, 29 May 2021 17:59:20 +0200 Subject: [PATCH 07/15] add integration test --- .../test/integration/API/Teams/LegalHold.hs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/services/galley/test/integration/API/Teams/LegalHold.hs b/services/galley/test/integration/API/Teams/LegalHold.hs index 5bdd008ca52..5a9adde5b43 100644 --- a/services/galley/test/integration/API/Teams/LegalHold.hs +++ b/services/galley/test/integration/API/Teams/LegalHold.hs @@ -203,6 +203,10 @@ tests s = s "If LH is activated for other user in 1:1 conv, 1:1 conv is blocked (connect before, team peer, approve LH device)" (testNoConsentBlockOne2OneConv True True True False), + test + s + "XXXXXX Pending/Sent connections revert to original state after consent" + testNoConsentBlockAndRestorePending, test s "If LH is activated for other user in group conv, this user gets removed with helpful message" @@ -1097,6 +1101,44 @@ testNoConsentBlockOne2OneConv connectFirst teamPeer approveLH testPendingConnect postConnection legalholder peer !!! do testResponse 412 (Just "missing-legalhold-consent") postConnection peer legalholder !!! do testResponse 412 (Just "missing-legalhold-consent") +testNoConsentBlockAndRestorePending :: HasCallStack => TestM () +testNoConsentBlockAndRestorePending = do + (legalholder :: UserId, tid) <- createBindingTeam + (peer :: UserId, teamPeer) <- createBindingTeam + galley <- view tsGalley + + let doEnableLH :: HasCallStack => TestM ClientId + doEnableLH = do + withLHWhitelist tid (requestLegalHoldDevice' galley legalholder legalholder tid) !!! testResponse 201 Nothing + withLHWhitelist tid (approveLegalHoldDevice' galley (Just defPassword) legalholder legalholder tid) !!! testResponse 200 Nothing + UserLegalHoldStatusResponse userStatus _ _ <- withLHWhitelist tid (getUserStatusTyped' galley legalholder tid) + liftIO $ assertEqual "approving should change status" UserLegalHoldEnabled userStatus + + getInternalClientsFull (UserSet $ Set.fromList [legalholder]) + <&> userClientsFull + <&> Map.elems + <&> Set.unions + <&> Set.toList + <&> (\[x] -> x) + <&> clientId + + let assertConnsPendingSent :: HasCallStack => TestM () + assertConnsPendingSent = do + assertConnections legalholder [ConnectionStatus legalholder peer Conn.Sent] + assertConnections peer [ConnectionStatus peer legalholder Conn.Pending] + + withDummyTestServiceForTeam legalholder tid $ \_chan -> do + postConnection legalholder peer !!! const 201 === statusCode + assertConnsPendingSent + + ensureQueueEmpty + void doEnableLH + assertConnections legalholder [ConnectionStatus legalholder peer Conn.MissingLegalholdConsent] + assertConnections peer [ConnectionStatus peer legalholder Conn.MissingLegalholdConsent] + + grantConsent teamPeer peer + assertConnsPendingSent + testNoConsentBlockGroupConv :: TestM () testNoConsentBlockGroupConv = do -- "If LH is activated for other user in group conv, this user gets removed with helpful message" From a9bdd83f07e58f327d38ed78aa0fcdffc0f22477 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Sat, 29 May 2021 18:17:08 +0200 Subject: [PATCH 08/15] s/grantConsent/doDisableLH --- .../test/integration/API/Teams/LegalHold.hs | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/services/galley/test/integration/API/Teams/LegalHold.hs b/services/galley/test/integration/API/Teams/LegalHold.hs index 5a9adde5b43..a8ab79657b8 100644 --- a/services/galley/test/integration/API/Teams/LegalHold.hs +++ b/services/galley/test/integration/API/Teams/LegalHold.hs @@ -1104,7 +1104,7 @@ testNoConsentBlockOne2OneConv connectFirst teamPeer approveLH testPendingConnect testNoConsentBlockAndRestorePending :: HasCallStack => TestM () testNoConsentBlockAndRestorePending = do (legalholder :: UserId, tid) <- createBindingTeam - (peer :: UserId, teamPeer) <- createBindingTeam + (peer :: UserId, _teamPeer) <- createBindingTeam galley <- view tsGalley let doEnableLH :: HasCallStack => TestM ClientId @@ -1122,22 +1122,25 @@ testNoConsentBlockAndRestorePending = do <&> (\[x] -> x) <&> clientId - let assertConnsPendingSent :: HasCallStack => TestM () - assertConnsPendingSent = do - assertConnections legalholder [ConnectionStatus legalholder peer Conn.Sent] - assertConnections peer [ConnectionStatus peer legalholder Conn.Pending] + doDisableLH :: HasCallStack => TestM () + doDisableLH = do + -- remove (only) LH device again + withLHWhitelist tid (disableLegalHoldForUser' galley (Just defPassword) tid legalholder legalholder) + !!! testResponse 200 Nothing withDummyTestServiceForTeam legalholder tid $ \_chan -> do postConnection legalholder peer !!! const 201 === statusCode - assertConnsPendingSent + assertConnections legalholder [ConnectionStatus legalholder peer Conn.Sent] + assertConnections peer [ConnectionStatus peer legalholder Conn.Pending] - ensureQueueEmpty void doEnableLH assertConnections legalholder [ConnectionStatus legalholder peer Conn.MissingLegalholdConsent] assertConnections peer [ConnectionStatus peer legalholder Conn.MissingLegalholdConsent] - grantConsent teamPeer peer - assertConnsPendingSent + -- FUTUREWORK: @grantConsent teamPeer peer@ <- also test this + doDisableLH + assertConnections legalholder [ConnectionStatus legalholder peer Conn.Sent] + assertConnections peer [ConnectionStatus peer legalholder Conn.Pending] testNoConsentBlockGroupConv :: TestM () testNoConsentBlockGroupConv = do From 08e8239a13155f53f9909815af60aed6bb504ff9 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sat, 29 May 2021 21:43:21 +0200 Subject: [PATCH 09/15] Remove redundant test case; make existing test case exhaustive. The redundant test case was hitting an execution path that was not hit before because I was reluctant to call the test with all combinations of options. I'll know better next time, I hope... --- .../test/integration/API/Teams/LegalHold.hs | 81 +------------------ 1 file changed, 4 insertions(+), 77 deletions(-) diff --git a/services/galley/test/integration/API/Teams/LegalHold.hs b/services/galley/test/integration/API/Teams/LegalHold.hs index a8ab79657b8..ebe9c70eae5 100644 --- a/services/galley/test/integration/API/Teams/LegalHold.hs +++ b/services/galley/test/integration/API/Teams/LegalHold.hs @@ -171,42 +171,10 @@ tests s = "teams listed" [ test s "happy flow" testInWhitelist, test s "handshake between LH device and user with old clients is blocked" testOldClientsBlockDeviceHandshake, - test - s - "If LH is activated for other user in 1:1 conv, 1:1 conv is blocked (connect after, personal peer)" - (testNoConsentBlockOne2OneConv False False False False), - test - s - "If LH is activated for other user in 1:1 conv, 1:1 conv is blocked (connect after, team peer)" - (testNoConsentBlockOne2OneConv False True False False), - test - s - "If LH is activated for other user in 1:1 conv, 1:1 conv is blocked (connect after, team peer, approve LH device)" - (testNoConsentBlockOne2OneConv False True True False), - test - s - "If LH is activated for other user in 1:1 conv, 1:1 conv is blocked (connect after, team peer, leave conn pending)" - (testNoConsentBlockOne2OneConv False True False True), - test - s - "If LH is activated for other user in 1:1 conv, 1:1 conv is blocked (connect after, team peer, approve LH device, leave conn pending)" - (testNoConsentBlockOne2OneConv False True True True), - test - s - "If LH is activated for other user in 1:1 conv, 1:1 conv is blocked (connect before, personal peer)" - (testNoConsentBlockOne2OneConv True False False False), - test - s - "If LH is activated for other user in 1:1 conv, 1:1 conv is blocked (connect before, team peer)" - (testNoConsentBlockOne2OneConv True True False False), - test - s - "If LH is activated for other user in 1:1 conv, 1:1 conv is blocked (connect before, team peer, approve LH device)" - (testNoConsentBlockOne2OneConv True True True False), - test - s - "XXXXXX Pending/Sent connections revert to original state after consent" - testNoConsentBlockAndRestorePending, + testGroup "XXX no-consent" $ + flip fmap [(a, b, c, d) | a <- [minBound ..], b <- [minBound ..], c <- [minBound ..], d <- [minBound ..]] $ + \args@(a, b, c, d) -> + test s (show args) $ testNoConsentBlockOne2OneConv a b c d, test s "If LH is activated for other user in group conv, this user gets removed with helpful message" @@ -1101,47 +1069,6 @@ testNoConsentBlockOne2OneConv connectFirst teamPeer approveLH testPendingConnect postConnection legalholder peer !!! do testResponse 412 (Just "missing-legalhold-consent") postConnection peer legalholder !!! do testResponse 412 (Just "missing-legalhold-consent") -testNoConsentBlockAndRestorePending :: HasCallStack => TestM () -testNoConsentBlockAndRestorePending = do - (legalholder :: UserId, tid) <- createBindingTeam - (peer :: UserId, _teamPeer) <- createBindingTeam - galley <- view tsGalley - - let doEnableLH :: HasCallStack => TestM ClientId - doEnableLH = do - withLHWhitelist tid (requestLegalHoldDevice' galley legalholder legalholder tid) !!! testResponse 201 Nothing - withLHWhitelist tid (approveLegalHoldDevice' galley (Just defPassword) legalholder legalholder tid) !!! testResponse 200 Nothing - UserLegalHoldStatusResponse userStatus _ _ <- withLHWhitelist tid (getUserStatusTyped' galley legalholder tid) - liftIO $ assertEqual "approving should change status" UserLegalHoldEnabled userStatus - - getInternalClientsFull (UserSet $ Set.fromList [legalholder]) - <&> userClientsFull - <&> Map.elems - <&> Set.unions - <&> Set.toList - <&> (\[x] -> x) - <&> clientId - - doDisableLH :: HasCallStack => TestM () - doDisableLH = do - -- remove (only) LH device again - withLHWhitelist tid (disableLegalHoldForUser' galley (Just defPassword) tid legalholder legalholder) - !!! testResponse 200 Nothing - - withDummyTestServiceForTeam legalholder tid $ \_chan -> do - postConnection legalholder peer !!! const 201 === statusCode - assertConnections legalholder [ConnectionStatus legalholder peer Conn.Sent] - assertConnections peer [ConnectionStatus peer legalholder Conn.Pending] - - void doEnableLH - assertConnections legalholder [ConnectionStatus legalholder peer Conn.MissingLegalholdConsent] - assertConnections peer [ConnectionStatus peer legalholder Conn.MissingLegalholdConsent] - - -- FUTUREWORK: @grantConsent teamPeer peer@ <- also test this - doDisableLH - assertConnections legalholder [ConnectionStatus legalholder peer Conn.Sent] - assertConnections peer [ConnectionStatus peer legalholder Conn.Pending] - testNoConsentBlockGroupConv :: TestM () testNoConsentBlockGroupConv = do -- "If LH is activated for other user in group conv, this user gets removed with helpful message" From 31fd018b0907530543f4d69ce181308b058217ca Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sat, 29 May 2021 22:29:03 +0200 Subject: [PATCH 10/15] Extend internal brig end-point for connection lookup. --- CHANGELOG.md | 2 +- libs/brig-types/src/Brig/Types/Connection.hs | 2 +- services/brig/src/Brig/API/Connection.hs | 1 + services/brig/src/Brig/API/Internal.hs | 2 +- services/brig/src/Brig/Data/Connection.hs | 10 ++++++++++ services/galley/src/Galley/API/Util.hs | 4 ++-- services/galley/src/Galley/Intra/User.hs | 2 +- 7 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c5743247d7..08776ceab9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ ## Release Notes -Deploy brig before galley (#1526) +Deploy brig before galley (#1526, #1549) ## Features diff --git a/libs/brig-types/src/Brig/Types/Connection.hs b/libs/brig-types/src/Brig/Types/Connection.hs index 1403815ad00..3d6b37ff99a 100644 --- a/libs/brig-types/src/Brig/Types/Connection.hs +++ b/libs/brig-types/src/Brig/Types/Connection.hs @@ -55,7 +55,7 @@ data UserIds = UserIds -- | Data that is passed to the @\/i\/users\/connections-status@ endpoint. data ConnectionsStatusRequest = ConnectionsStatusRequest { csrFrom :: ![UserId], - csrTo :: ![UserId] + csrTo :: !(Maybe [UserId]) } deriving (Eq, Show, Generic) diff --git a/services/brig/src/Brig/API/Connection.hs b/services/brig/src/Brig/API/Connection.hs index 0c59f3f0a51..aeb5ccad2ec 100644 --- a/services/brig/src/Brig/API/Connection.hs +++ b/services/brig/src/Brig/API/Connection.hs @@ -30,6 +30,7 @@ module Brig.API.Connection lookupConnections, Data.lookupConnection, Data.lookupConnectionStatus, + Data.lookupConnectionStatus', Data.lookupContactList, ) where diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index f5dfb368242..6476d9c63ca 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -499,7 +499,7 @@ getConnectionsStatusH (_ ::: req ::: flt) = do getConnectionsStatus :: ConnectionsStatusRequest -> Maybe Relation -> AppIO [ConnectionStatus] getConnectionsStatus ConnectionsStatusRequest {csrFrom, csrTo} flt = do - r <- API.lookupConnectionStatus csrFrom csrTo + r <- maybe (API.lookupConnectionStatus' csrFrom) (API.lookupConnectionStatus csrFrom) csrTo return $ maybe r (filterByRelation r) flt where filterByRelation l rel = filter ((== rel) . csStatus) l diff --git a/services/brig/src/Brig/Data/Connection.hs b/services/brig/src/Brig/Data/Connection.hs index 17b89549123..1988b94a5a3 100644 --- a/services/brig/src/Brig/Data/Connection.hs +++ b/services/brig/src/Brig/Data/Connection.hs @@ -26,6 +26,7 @@ module Brig.Data.Connection lookupRelationWithHistory, lookupConnections, lookupConnectionStatus, + lookupConnectionStatus', lookupContactList, lookupContactListWithRelation, countConnections, @@ -125,6 +126,12 @@ lookupConnectionStatus from to = map toConnectionStatus <$> retry x1 (query connectionStatusSelect (params Quorum (from, to))) +-- | Lookup all relations between two sets of users (cartesian product). +lookupConnectionStatus' :: [UserId] -> AppIO [ConnectionStatus] +lookupConnectionStatus' from = + map toConnectionStatus + <$> retry x1 (query connectionStatusSelect' (params Quorum (Identity from))) + -- | See 'lookupContactListWithRelation'. lookupContactList :: UserId -> AppIO [UserId] lookupContactList u = @@ -176,6 +183,9 @@ relationSelect = "SELECT status FROM connection WHERE left = ? AND right = ?" connectionStatusSelect :: PrepQuery R ([UserId], [UserId]) (UserId, UserId, RelationWithHistory) connectionStatusSelect = "SELECT left, right, status FROM connection WHERE left IN ? AND right IN ?" +connectionStatusSelect' :: PrepQuery R (Identity [UserId]) (UserId, UserId, RelationWithHistory) +connectionStatusSelect' = "SELECT left, right, status FROM connection WHERE left IN ?" + contactsSelect :: PrepQuery R (Identity UserId) (UserId, RelationWithHistory) contactsSelect = "SELECT right, status FROM connection WHERE left = ?" diff --git a/services/galley/src/Galley/API/Util.hs b/services/galley/src/Galley/API/Util.hs index 88d8c778116..5f658f83d01 100644 --- a/services/galley/src/Galley/API/Util.hs +++ b/services/galley/src/Galley/API/Util.hs @@ -94,8 +94,8 @@ ensureConnectedToLocals :: UserId -> [UserId] -> Galley () ensureConnectedToLocals _ [] = pure () ensureConnectedToLocals u uids = do (connsFrom, connsTo) <- - getConnections [u] uids (Just Accepted) - `concurrently` getConnections uids [u] (Just Accepted) + getConnections [u] (Just uids) (Just Accepted) + `concurrently` getConnections uids (Just [u]) (Just Accepted) unless (length connsFrom == length uids && length connsTo == length uids) $ throwM notConnected diff --git a/services/galley/src/Galley/Intra/User.hs b/services/galley/src/Galley/Intra/User.hs index 0c0cb6103f2..94637c0d1f6 100644 --- a/services/galley/src/Galley/Intra/User.hs +++ b/services/galley/src/Galley/Intra/User.hs @@ -56,7 +56,7 @@ import Wire.API.User.RichInfo (RichInfo) -- -- When a connection does not exist, it is skipped. -- Calls 'Brig.API.getConnectionsStatusH'. -getConnections :: [UserId] -> [UserId] -> Maybe Relation -> Galley [ConnectionStatus] +getConnections :: [UserId] -> Maybe [UserId] -> Maybe Relation -> Galley [ConnectionStatus] getConnections uFrom uTo rlt = do (h, p) <- brigReq r <- From ae9a9b1961b8f594d850f1d84d6c26f7948cd984 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sat, 29 May 2021 22:48:26 +0200 Subject: [PATCH 11/15] Fix: lookup connections for blocking 1:1 convs, not convs directly. Connect convs do not contain the other user, so in order to find the other user it's not good to go through that. --- services/galley/src/Galley/API/LegalHold.hs | 27 ++++++++------------- services/galley/src/Galley/API/Query.hs | 16 ------------ 2 files changed, 10 insertions(+), 33 deletions(-) diff --git a/services/galley/src/Galley/API/LegalHold.hs b/services/galley/src/Galley/API/LegalHold.hs index 1cf6137c900..9887d21e8fd 100644 --- a/services/galley/src/Galley/API/LegalHold.hs +++ b/services/galley/src/Galley/API/LegalHold.hs @@ -31,6 +31,7 @@ where import Brig.Types.Client.Prekey import Brig.Types.Connection (UpdateConnectionsInternal (..)) +import Brig.Types.Intra (ConnectionStatus (..)) import Brig.Types.Provider import Brig.Types.Team.LegalHold hiding (userId) import Control.Lens (view, (^.)) @@ -41,11 +42,7 @@ import Data.LegalHold (UserLegalHoldStatus (..), defUserLegalHoldStatus) import Data.List.Split (chunksOf) import qualified Data.Map.Strict as Map import Data.Misc -import Data.Proxy -import Data.Qualified (Qualified, partitionRemoteOrLocalIds) -import Data.Range (toRange) import Galley.API.Error -import Galley.API.Query (iterateConversations) import Galley.API.Util import Galley.App import qualified Galley.Data as Data @@ -53,7 +50,7 @@ import qualified Galley.Data.LegalHold as LegalHoldData import qualified Galley.Data.TeamFeatures as TeamFeatures import qualified Galley.External.LegalHoldService as LHService import qualified Galley.Intra.Client as Client -import Galley.Intra.User (putConnectionInternal) +import Galley.Intra.User (getConnections, putConnectionInternal) import qualified Galley.Options as Opts import Galley.Types.Teams as Team import Imports @@ -64,7 +61,6 @@ import Network.Wai.Predicate hiding (or, result, setStatus, _3) import Network.Wai.Utilities as Wai import qualified System.Logger.Class as Log import UnliftIO.Async (pooledMapConcurrentlyN_) -import Wire.API.Conversation (ConvMembers (..), ConvType (..), Conversation (..), OtherMember (..), cnvType) import qualified Wire.API.Team.Feature as Public import qualified Wire.API.Team.LegalHold as Public @@ -415,22 +411,19 @@ changeLegalholdStatus tid uid old new = do -- FUTUREWORK: make this async? blockConnectionsFrom1on1s :: UserId -> Galley () blockConnectionsFrom1on1s uid = do - errmsgs <- - iterateConversations uid (toRange (Proxy @500)) $ \convs -> do - conflicts <- mconcat <$> findConflicts (filter ((== One2OneConv) . cnvType) convs) - blockConflicts uid conflicts + conns <- getConnections [uid] Nothing Nothing + errmsgs <- do + conflicts <- mconcat <$> findConflicts conns + blockConflicts uid conflicts case mconcat errmsgs of [] -> pure () msgs@(_ : _) -> do - Log.warn $ Log.msg @String (intercalate ", " msgs) + Log.warn $ Log.msg @String msgs throwM legalHoldCouldNotBlockConnections where - findConflicts :: [Conversation] -> Galley [[UserId]] - findConflicts convs = do - let otherUids :: [Qualified UserId] = - concatMap (fmap omQualifiedId . cmOthers . cnvMembers) convs - ownDomain <- viewFederationDomain - let (_remoteUsers, localUids) = partitionRemoteOrLocalIds ownDomain otherUids + findConflicts :: [ConnectionStatus] -> Galley [[UserId]] + findConflicts conns = do + let localUids = csTo <$> conns -- FUTUREWORK: Handle remoteUsers here when federation is implemented for (chunksOf 32 localUids) $ \others -> do teamsOfUsers <- Data.usersTeams others diff --git a/services/galley/src/Galley/API/Query.hs b/services/galley/src/Galley/API/Query.hs index 52eac89a470..4531f6861c6 100644 --- a/services/galley/src/Galley/API/Query.hs +++ b/services/galley/src/Galley/API/Query.hs @@ -21,7 +21,6 @@ module Galley.API.Query getConversationRoles, getConversationIds, getConversations, - iterateConversations, getSelfH, internalGetMemberH, getConversationMetaH, @@ -118,21 +117,6 @@ getConversations user mids mstart msize = do | Data.isConvDeleted c = Data.deleteConversation (Data.convId c) >> pure False | otherwise = pure True -iterateConversations :: forall a. UserId -> Range 1 500 Int32 -> ([Public.Conversation] -> Galley a) -> Galley [a] -iterateConversations uid pageSize handleConvs = catMaybes <$> go Nothing - where - go :: Maybe ConvId -> Galley [Maybe a] - go mbConv = do - convResult <- getConversations uid Nothing mbConv (Just pageSize) - resultHead <- Just <$> handleConvs (convList convResult) - resultTail <- case convList convResult of - (conv : rest) -> - if convHasMore convResult - then go (Just (maximum (cnvId <$> (conv : rest)))) - else pure [] - _ -> pure [] - pure $ resultHead : resultTail - getSelfH :: UserId ::: ConvId -> Galley Response getSelfH (zusr ::: cnv) = do json <$> getSelf zusr cnv From dac94f3433d015fe0b00b1f37a0091ed64285738 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Sat, 29 May 2021 23:19:01 +0200 Subject: [PATCH 12/15] fix: unblock filters unneccessarily on convtype --- services/brig/src/Brig/API/Connection.hs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/services/brig/src/Brig/API/Connection.hs b/services/brig/src/Brig/API/Connection.hs index aeb5ccad2ec..0cb7796966a 100644 --- a/services/brig/src/Brig/API/Connection.hs +++ b/services/brig/src/Brig/API/Connection.hs @@ -393,18 +393,8 @@ updateConnectionInternal = \case unblockDirected :: UserConnection -> UserConnection -> ExceptT ConnectionError AppIO () unblockDirected uconn uconnRev = do - cnv :: Maybe Conv.Conversation <- lift . for (ucConvId uconn) $ Intra.unblockConv (ucFrom uconn) Nothing - uconnRevRel :: RelationWithHistory <- relationWithHistory (ucFrom uconnRev) (ucTo uconnRev) - uconnRev' :: UserConnection <- do - newRelation <- case cnvType <$> cnv of - Just RegularConv -> throwE (InvalidTransition (ucFrom uconn) Accepted) -- (impossible, connection conv is always 1:1) - Just SelfConv -> throwE (InvalidTransition (ucFrom uconn) Accepted) - Just One2OneConv -> pure AcceptedWithHistory - Just ConnectConv -> pure $ undoRelationHistory uconnRevRel - Nothing -> throwE (InvalidTransition (ucFrom uconn) Accepted) - lift $ Data.updateConnection uconnRev newRelation - + uconnRev' <- lift $ Data.updateConnection uconnRev (undoRelationHistory uconnRevRel) connEvent :: ConnectionEvent <- lift $ ConnectionUpdated uconnRev' (Just $ ucStatus uconnRev) <$> Data.lookupName (ucFrom uconn) lift $ Intra.onConnectionEvent (ucFrom uconn) Nothing connEvent From 1072f3f0cf402f6ac2ddef9604a451f40c610389 Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Sat, 29 May 2021 23:20:00 +0200 Subject: [PATCH 13/15] remove test focus --- services/galley/test/integration/API/Teams/LegalHold.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/galley/test/integration/API/Teams/LegalHold.hs b/services/galley/test/integration/API/Teams/LegalHold.hs index ebe9c70eae5..4674fce6ff2 100644 --- a/services/galley/test/integration/API/Teams/LegalHold.hs +++ b/services/galley/test/integration/API/Teams/LegalHold.hs @@ -171,7 +171,7 @@ tests s = "teams listed" [ test s "happy flow" testInWhitelist, test s "handshake between LH device and user with old clients is blocked" testOldClientsBlockDeviceHandshake, - testGroup "XXX no-consent" $ + testGroup "no-consent" $ flip fmap [(a, b, c, d) | a <- [minBound ..], b <- [minBound ..], c <- [minBound ..], d <- [minBound ..]] $ \args@(a, b, c, d) -> test s (show args) $ testNoConsentBlockOne2OneConv a b c d, From 7afdaff86bb1d47be4cae3e13d72fcf3cb15e8cc Mon Sep 17 00:00:00 2001 From: Stefan Matting Date: Sun, 30 May 2021 09:40:29 +0200 Subject: [PATCH 14/15] fix mistake --- services/brig/src/Brig/API/Connection.hs | 1 + 1 file changed, 1 insertion(+) diff --git a/services/brig/src/Brig/API/Connection.hs b/services/brig/src/Brig/API/Connection.hs index 0cb7796966a..6b170f9f26b 100644 --- a/services/brig/src/Brig/API/Connection.hs +++ b/services/brig/src/Brig/API/Connection.hs @@ -393,6 +393,7 @@ updateConnectionInternal = \case unblockDirected :: UserConnection -> UserConnection -> ExceptT ConnectionError AppIO () unblockDirected uconn uconnRev = do + void . lift . for (ucConvId uconn) $ Intra.unblockConv (ucFrom uconn) Nothing uconnRevRel :: RelationWithHistory <- relationWithHistory (ucFrom uconnRev) (ucTo uconnRev) uconnRev' <- lift $ Data.updateConnection uconnRev (undoRelationHistory uconnRevRel) connEvent :: ConnectionEvent <- lift $ ConnectionUpdated uconnRev' (Just $ ucStatus uconnRev) <$> Data.lookupName (ucFrom uconn) From 765a3e1501a2108256e53a7ba37c7c2c44c9ced1 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Sun, 30 May 2021 09:43:45 +0200 Subject: [PATCH 15/15] Make test slightly more exhaustive. --- services/galley/test/integration/API/Teams/LegalHold.hs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/galley/test/integration/API/Teams/LegalHold.hs b/services/galley/test/integration/API/Teams/LegalHold.hs index 4674fce6ff2..1b77321828c 100644 --- a/services/galley/test/integration/API/Teams/LegalHold.hs +++ b/services/galley/test/integration/API/Teams/LegalHold.hs @@ -1011,8 +1011,6 @@ testNoConsentBlockOne2OneConv connectFirst teamPeer approveLH testPendingConnect assertConnections peer [ConnectionStatus peer legalholder Conn.MissingLegalholdConsent] forM_ [legalholderWs, peerWs] $ \ws -> do - -- (if this fails, it may be because there are other messages in the queue, but i - -- think we implemented this in a way that doens't trip over wrong orderings.) assertNotification ws $ \case (Ev.ConnectionEvent (Ev.ConnectionUpdated (Conn.ucStatus -> rel) _prev _name)) -> do @@ -1053,6 +1051,13 @@ testNoConsentBlockOne2OneConv connectFirst teamPeer approveLH testPendingConnect if testPendingConnection then Conn.Pending else Conn.Accepted ] + forM_ [legalholderWs, peerWs] $ \ws -> do + assertNotification ws $ + \case + (Ev.ConnectionEvent (Ev.ConnectionUpdated (Conn.ucStatus -> rel) _prev _name)) -> do + assertBool "" (rel `elem` [Conn.Sent, Conn.Pending, Conn.Accepted]) + _ -> assertBool "wrong event type" False + -- conversation reappears. peer can send message to legalholder again for_ ((,) <$> (mbConn >>= Conn.ucConvId) <*> mbLegalholderLHDevice) $ \(convId, legalholderLHDevice) -> do postOtrMessage