From 00923ccb9bfa4365f799eba0126f41da886c1156 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 16 Nov 2023 11:27:32 +0000 Subject: [PATCH 01/34] fed config store --- .../Wire/API/Routes/FederationDomainConfig.hs | 33 ++++++++++++------- services/brig/brig.cabal | 1 + services/brig/src/Brig/API/Federation.hs | 2 +- .../src/Brig/Effects/FederationConfigStore.hs | 11 +++++++ 4 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 services/brig/src/Brig/Effects/FederationConfigStore.hs diff --git a/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs b/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs index e6ed4ecc4c2..2d0b9a5e933 100644 --- a/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs +++ b/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs @@ -1,6 +1,10 @@ +{-# LANGUAGE GeneralizedNewtypeDeriving #-} +{-# LANGUAGE StrictData #-} +{-# LANGUAGE TemplateHaskell #-} + -- This file is part of the Wire Server implementation. -- --- Copyright (C) 2022 Wire Swiss GmbH +-- Copyright (C) 2023 Wire Swiss GmbH -- -- This program is free software: you can redistribute it and/or modify it under -- the terms of the GNU Affero General Public License as published by the Free @@ -25,7 +29,7 @@ module Wire.API.Routes.FederationDomainConfig ) where -import Control.Lens ((?~)) +import Control.Lens (makePrisms, (?~)) import Data.Aeson (FromJSON, ToJSON) import Data.Domain (Domain) import Data.Id @@ -36,25 +40,32 @@ import Imports import Wire.API.User.Search (FederatedUserSearchPolicy) import Wire.Arbitrary (Arbitrary, GenericUniform (..)) -data FederationRestriction = FederationRestrictionAllowAll | FederationRestrictionByTeam +data FederationRestriction + = FederationRestrictionAllowAll + | FederationRestrictionByTeam [TeamId] deriving (Eq, Show, Generic, Ord) - deriving (ToJSON, FromJSON, S.ToSchema) via Schema FederationRestriction deriving (Arbitrary) via (GenericUniform FederationRestriction) +makePrisms ''FederationRestriction + instance ToSchema FederationRestriction where schema = - enum @Text "FederationRestriction" $ - mconcat - [ element "allow_all" FederationRestrictionAllowAll, - element "restrict_by_team" FederationRestrictionByTeam - ] + named "FederationRestriction" $ + tag _FederationRestrictionAllowAll null_ + <> tag _FederationRestrictionByTeam (array (unnamed schema)) + +deriving via (Schema FederationRestriction) instance FromJSON FederationRestriction + +deriving via (Schema FederationRestriction) instance ToJSON FederationRestriction + +deriving via (Schema FederationRestriction) instance S.ToSchema FederationRestriction -- | Everything we need to know about a remote instance in order to federate with it. Comes -- in `AllowedDomains` if `AllowStrategy` is `AllowDynamic`. If `AllowAll`, we still use this -- information for search policy. data FederationDomainConfig = FederationDomainConfig { domain :: Domain, - cfgSearchPolicy :: FederatedUserSearchPolicy, + searchPolicy :: FederatedUserSearchPolicy, restriction :: FederationRestriction } deriving (Eq, Ord, Show, Generic) @@ -66,7 +77,7 @@ instance ToSchema FederationDomainConfig where object "FederationDomainConfig" $ FederationDomainConfig <$> domain .= field "domain" schema - <*> cfgSearchPolicy .= field "search_policy" schema + <*> searchPolicy .= field "search_policy" schema <*> restriction .= field "restriction" schema data FederationDomainConfigs = FederationDomainConfigs diff --git a/services/brig/brig.cabal b/services/brig/brig.cabal index d6daec5b14d..62e9c4776a5 100644 --- a/services/brig/brig.cabal +++ b/services/brig/brig.cabal @@ -126,6 +126,7 @@ library Brig.Effects.CodeStore Brig.Effects.CodeStore.Cassandra Brig.Effects.Delay + Brig.Effects.FederationConfigStore Brig.Effects.GalleyProvider Brig.Effects.GalleyProvider.RPC Brig.Effects.JwtTools diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 90ddd22a281..6c90d1f156a 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -244,4 +244,4 @@ lookupSearchPolicy :: Domain -> (Handler r) FederatedUserSearchPolicy lookupSearchPolicy domain = do domainConfigs <- getFederationRemotes let mConfig = find ((== domain) . FD.domain) (domainConfigs.remotes) - pure $ maybe NoSearch FD.cfgSearchPolicy mConfig + pure $ maybe NoSearch FD.searchPolicy mConfig diff --git a/services/brig/src/Brig/Effects/FederationConfigStore.hs b/services/brig/src/Brig/Effects/FederationConfigStore.hs new file mode 100644 index 00000000000..eae58d0cba1 --- /dev/null +++ b/services/brig/src/Brig/Effects/FederationConfigStore.hs @@ -0,0 +1,11 @@ +{-# LANGUAGE TemplateHaskell #-} + +module Brig.Effects.FederationConfigStore where + +import Data.Domain +import Polysemy + +data FederationConfigStore m a where + GetFederationConfig :: Domain -> FederationConfigStore m () + +makeSem ''FederationConfigStore From 95558d19147364469775eac15b33d3cc339d4c0e Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 16 Nov 2023 12:55:31 +0000 Subject: [PATCH 02/34] wip --- .../Wire/API/Routes/FederationDomainConfig.hs | 29 +++----- services/brig/brig.cabal | 2 +- services/brig/src/Brig/API/Internal.hs | 1 - .../src/Brig/Effects/FederationConfigStore.hs | 20 +++++- .../FederationConfigStore/Cassandra.hs} | 68 +++++++++++-------- 5 files changed, 69 insertions(+), 51 deletions(-) rename services/brig/src/Brig/{Data/Federation.hs => Effects/FederationConfigStore/Cassandra.hs} (55%) diff --git a/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs b/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs index 2d0b9a5e933..95ed33c5558 100644 --- a/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs +++ b/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs @@ -1,10 +1,6 @@ -{-# LANGUAGE GeneralizedNewtypeDeriving #-} -{-# LANGUAGE StrictData #-} -{-# LANGUAGE TemplateHaskell #-} - -- This file is part of the Wire Server implementation. -- --- Copyright (C) 2023 Wire Swiss GmbH +-- Copyright (C) 2022 Wire Swiss GmbH -- -- This program is free software: you can redistribute it and/or modify it under -- the terms of the GNU Affero General Public License as published by the Free @@ -29,7 +25,7 @@ module Wire.API.Routes.FederationDomainConfig ) where -import Control.Lens (makePrisms, (?~)) +import Control.Lens ((?~)) import Data.Aeson (FromJSON, ToJSON) import Data.Domain (Domain) import Data.Id @@ -40,25 +36,18 @@ import Imports import Wire.API.User.Search (FederatedUserSearchPolicy) import Wire.Arbitrary (Arbitrary, GenericUniform (..)) -data FederationRestriction - = FederationRestrictionAllowAll - | FederationRestrictionByTeam [TeamId] +data FederationRestriction = FederationRestrictionAllowAll | FederationRestrictionByTeam deriving (Eq, Show, Generic, Ord) + deriving (ToJSON, FromJSON, S.ToSchema) via Schema FederationRestriction deriving (Arbitrary) via (GenericUniform FederationRestriction) -makePrisms ''FederationRestriction - instance ToSchema FederationRestriction where schema = - named "FederationRestriction" $ - tag _FederationRestrictionAllowAll null_ - <> tag _FederationRestrictionByTeam (array (unnamed schema)) - -deriving via (Schema FederationRestriction) instance FromJSON FederationRestriction - -deriving via (Schema FederationRestriction) instance ToJSON FederationRestriction - -deriving via (Schema FederationRestriction) instance S.ToSchema FederationRestriction + enum @Text "FederationRestriction" $ + mconcat + [ element "allow_all" FederationRestrictionAllowAll, + element "restrict_by_team" FederationRestrictionByTeam + ] -- | Everything we need to know about a remote instance in order to federate with it. Comes -- in `AllowedDomains` if `AllowStrategy` is `AllowDynamic`. If `AllowAll`, we still use this diff --git a/services/brig/brig.cabal b/services/brig/brig.cabal index 62e9c4776a5..a38dc13e43b 100644 --- a/services/brig/brig.cabal +++ b/services/brig/brig.cabal @@ -110,7 +110,6 @@ library Brig.Data.Activation Brig.Data.Client Brig.Data.Connection - Brig.Data.Federation Brig.Data.Instances Brig.Data.LoginCode Brig.Data.MLS.KeyPackage @@ -127,6 +126,7 @@ library Brig.Effects.CodeStore.Cassandra Brig.Effects.Delay Brig.Effects.FederationConfigStore + Brig.Effects.FederationConfigStore.Cassandra Brig.Effects.GalleyProvider Brig.Effects.GalleyProvider.RPC Brig.Effects.JwtTools diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 61368a7012d..70d6834ebb1 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -38,7 +38,6 @@ import Brig.Code qualified as Code import Brig.Data.Activation import Brig.Data.Client qualified as Data import Brig.Data.Connection qualified as Data -import Brig.Data.Federation qualified as Data import Brig.Data.MLS.KeyPackage qualified as Data import Brig.Data.User qualified as Data import Brig.Effects.BlacklistPhonePrefixStore (BlacklistPhonePrefixStore) diff --git a/services/brig/src/Brig/Effects/FederationConfigStore.hs b/services/brig/src/Brig/Effects/FederationConfigStore.hs index eae58d0cba1..1d2a85dce34 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore.hs @@ -3,9 +3,27 @@ module Brig.Effects.FederationConfigStore where import Data.Domain +import Data.Id +import Imports import Polysemy +import Wire.API.Routes.FederationDomainConfig qualified as API +import Wire.API.User.Search (FederatedUserSearchPolicy) + +data FederationRestriction = FederationRestrictionAllowAll | FederationRestrictionByTeam [TeamId] + deriving (Eq, Show, Ord) + +data FederationDomainConfig = FederationDomainConfig + { domain :: Domain, + searchPolicy :: FederatedUserSearchPolicy, + restriction :: FederationRestriction + } data FederationConfigStore m a where - GetFederationConfig :: Domain -> FederationConfigStore m () + GetFederationConfig :: Domain -> FederationConfigStore m FederationDomainConfig + GetFederationConfigs :: FederationConfigStore m [FederationDomainConfig] + AddFederationConfig :: API.FederationDomainConfig -> FederationConfigStore m () + UpdateFederationConfig :: API.FederationDomainConfig -> FederationConfigStore m Bool + AddFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () + RemoveFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () makeSem ''FederationConfigStore diff --git a/services/brig/src/Brig/Data/Federation.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs similarity index 55% rename from services/brig/src/Brig/Data/Federation.hs rename to services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index 3ae38325d40..830722c4d78 100644 --- a/services/brig/src/Brig/Data/Federation.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -15,19 +15,14 @@ -- You should have received a copy of the GNU Affero General Public License along -- with this program. If not, see . -module Brig.Data.Federation - ( getFederationRemotes, - addFederationRemote, - updateFederationRemote, - deleteFederationRemote, - addFederationRemoteTeam, - getFederationRemoteTeams, - deleteFederationRemoteTeam, +module Brig.Effects.FederationConfigStore.Cassandra + ( interpretFederationDomainConfig, AddFederationRemoteResult (..), ) where import Brig.Data.Instances () +import Brig.Effects.FederationConfigStore import Cassandra import Control.Exception (ErrorCall (ErrorCall)) import Control.Monad.Catch (throwM) @@ -35,66 +30,83 @@ import Data.Domain import Data.Id import Database.CQL.Protocol (SerialConsistency (LocalSerialConsistency), serialConsistency) import Imports -import Wire.API.Routes.FederationDomainConfig +import Polysemy +import Wire.API.Routes.FederationDomainConfig qualified as API import Wire.API.User.Search +interpretFederationDomainConfig :: forall m r a. (MonadClient m, Member (Embed m) r) => Sem (FederationConfigStore ': r) a -> Sem r a +interpretFederationDomainConfig = + interpret $ + embed @m . \case + GetFederationConfig _ -> undefined + GetFederationConfigs -> getFederationConfigs' + AddFederationConfig _ -> pure () + UpdateFederationConfig _ -> pure False + AddFederationRemoteTeam _ _ -> pure () + RemoveFederationRemoteTeam _ _ -> pure () + +getFederationConfigs' :: forall m. MonadClient m => m [FederationDomainConfig] +getFederationConfigs' = do + _xs <- getFederationRemotes + pure undefined + maxKnownNodes :: Int maxKnownNodes = 10000 -getFederationRemotes :: forall m. MonadClient m => m [FederationDomainConfig] -getFederationRemotes = (\(d, p, r) -> FederationDomainConfig d p r) <$$> qry +getFederationRemotes :: forall m. MonadClient m => m [API.FederationDomainConfig] +getFederationRemotes = (\(d, p, r) -> API.FederationDomainConfig d p r) <$$> qry where - qry :: m [(Domain, FederatedUserSearchPolicy, FederationRestriction)] + qry :: m [(Domain, FederatedUserSearchPolicy, API.FederationRestriction)] qry = retry x1 . query get $ params LocalQuorum () - get :: PrepQuery R () (Domain, FederatedUserSearchPolicy, FederationRestriction) + get :: PrepQuery R () (Domain, FederatedUserSearchPolicy, API.FederationRestriction) get = fromString $ "SELECT domain, search_policy, restriction FROM federation_remotes LIMIT " <> show maxKnownNodes data AddFederationRemoteResult = AddFederationRemoteSuccess | AddFederationRemoteMaxRemotesReached -addFederationRemote :: MonadClient m => FederationDomainConfig -> m AddFederationRemoteResult -addFederationRemote (FederationDomainConfig rDomain searchPolicy restriction) = do +_addFederationRemote :: MonadClient m => API.FederationDomainConfig -> m AddFederationRemoteResult +_addFederationRemote (API.FederationDomainConfig rDomain searchPolicy restriction) = do l <- length <$> getFederationRemotes if l >= maxKnownNodes then pure AddFederationRemoteMaxRemotesReached else AddFederationRemoteSuccess <$ retry x5 (write add (params LocalQuorum (rDomain, searchPolicy, restriction))) where - add :: PrepQuery W (Domain, FederatedUserSearchPolicy, FederationRestriction) () + add :: PrepQuery W (Domain, FederatedUserSearchPolicy, API.FederationRestriction) () add = "INSERT INTO federation_remotes (domain, search_policy, restriction) VALUES (?, ?, ?)" -updateFederationRemote :: MonadClient m => FederationDomainConfig -> m Bool -updateFederationRemote (FederationDomainConfig rDomain searchPolicy restriction) = do +_updateFederationRemote :: MonadClient m => API.FederationDomainConfig -> m Bool +_updateFederationRemote (API.FederationDomainConfig rDomain searchPolicy restriction) = do retry x1 (trans upd (params LocalQuorum (searchPolicy, restriction, rDomain)) {serialConsistency = Just LocalSerialConsistency}) >>= \case [] -> pure False [_] -> pure True _ -> throwM $ ErrorCall "Primary key violation detected federation_remotes" where - upd :: PrepQuery W (FederatedUserSearchPolicy, FederationRestriction, Domain) x + upd :: PrepQuery W (FederatedUserSearchPolicy, API.FederationRestriction, Domain) x upd = "UPDATE federation_remotes SET search_policy = ?, restriction = ? WHERE domain = ? IF EXISTS" -deleteFederationRemote :: MonadClient m => Domain -> m () -deleteFederationRemote rDomain = +_deleteFederationRemote :: MonadClient m => Domain -> m () +_deleteFederationRemote rDomain = retry x1 $ write delete (params LocalQuorum (Identity rDomain)) where delete :: PrepQuery W (Identity Domain) () delete = "DELETE FROM federation_remotes WHERE domain = ?" -addFederationRemoteTeam :: MonadClient m => Domain -> FederationRemoteTeam -> m () -addFederationRemoteTeam rDomain rteam = +_addFederationRemoteTeam :: MonadClient m => Domain -> API.FederationRemoteTeam -> m () +_addFederationRemoteTeam rDomain rteam = retry x1 $ write add (params LocalQuorum (rDomain, rteam.teamId)) where add :: PrepQuery W (Domain, TeamId) () add = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" -getFederationRemoteTeams :: MonadClient m => Domain -> m [FederationRemoteTeam] -getFederationRemoteTeams rDomain = do - fmap (FederationRemoteTeam . runIdentity) <$> retry x1 (query get (params LocalQuorum (Identity rDomain))) +_getFederationRemoteTeams :: MonadClient m => Domain -> m [API.FederationRemoteTeam] +_getFederationRemoteTeams rDomain = do + fmap (API.FederationRemoteTeam . runIdentity) <$> retry x1 (query get (params LocalQuorum (Identity rDomain))) where get :: PrepQuery R (Identity Domain) (Identity TeamId) get = "SELECT team FROM federation_remote_teams WHERE domain = ?" -deleteFederationRemoteTeam :: MonadClient m => Domain -> TeamId -> m () -deleteFederationRemoteTeam rDomain rteam = +_deleteFederationRemoteTeam :: MonadClient m => Domain -> TeamId -> m () +_deleteFederationRemoteTeam rDomain rteam = retry x1 $ write delete (params LocalQuorum (rDomain, rteam)) where delete :: PrepQuery W (Domain, TeamId) () From 1cd6bad794ad8c28fcdfec62e62a27a12d6ab1e0 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 20 Nov 2023 11:06:02 +0000 Subject: [PATCH 03/34] federation config store interpreter --- services/brig/brig.cabal | 1 - services/brig/default.nix | 1 - services/brig/src/Brig/API/Federation.hs | 26 +++++--- services/brig/src/Brig/API/Internal.hs | 44 ++++++------- .../brig/src/Brig/CanonicalInterpreter.hs | 6 +- .../src/Brig/Effects/FederationConfigStore.hs | 11 +++- .../FederationConfigStore/Cassandra.hs | 61 +++++++++++-------- 7 files changed, 82 insertions(+), 68 deletions(-) diff --git a/services/brig/brig.cabal b/services/brig/brig.cabal index a38dc13e43b..93622af252c 100644 --- a/services/brig/brig.cabal +++ b/services/brig/brig.cabal @@ -319,7 +319,6 @@ library , polysemy-plugin , polysemy-wire-zoo , proto-lens >=0.1 - , random , random-shuffle >=0.0.3 , raw-strings-qq , resource-pool >=0.2 diff --git a/services/brig/default.nix b/services/brig/default.nix index 6887c802f38..14f1634b1d5 100644 --- a/services/brig/default.nix +++ b/services/brig/default.nix @@ -242,7 +242,6 @@ mkDerivation { polysemy-plugin polysemy-wire-zoo proto-lens - random random-shuffle raw-strings-qq resource-pool diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 6c90d1f156a..8e49ce5d5ba 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -23,7 +23,6 @@ import Brig.API.Client qualified as API import Brig.API.Connection.Remote (performRemoteAction) import Brig.API.Error import Brig.API.Handler (Handler) -import Brig.API.Internal hiding (getMLSClients) import Brig.API.Internal qualified as Internal import Brig.API.MLS.CipherSuite import Brig.API.MLS.KeyPackages @@ -32,6 +31,8 @@ import Brig.API.User qualified as API import Brig.App import Brig.Data.Connection qualified as Data import Brig.Data.User qualified as Data +import Brig.Effects.FederationConfigStore (FederationConfigStore) +import Brig.Effects.FederationConfigStore qualified as FederationConfigStore import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.IO.Intra (notify) import Brig.Options @@ -76,7 +77,8 @@ type FederationAPI = "federation" :> BrigApi federationSitemap :: ( Member GalleyProvider r, - Member (Concurrency 'Unsafe) r + Member (Concurrency 'Unsafe) r, + Member FederationConfigStore r ) => ServerT FederationAPI (Handler r) federationSitemap = @@ -96,13 +98,13 @@ federationSitemap = -- Allow remote domains to send their known remote federation instances, and respond -- with the subset of those we aren't connected to. -getFederationStatus :: Domain -> DomainSet -> Handler r NonConnectedBackends +getFederationStatus :: (Member FederationConfigStore r) => Domain -> DomainSet -> Handler r NonConnectedBackends getFederationStatus _ request = do cfg <- ask case setFederationStrategy (cfg ^. settings) of Just AllowAll -> pure $ NonConnectedBackends mempty _ -> do - fedDomains <- fromList . fmap (.domain) . (.remotes) <$> getFederationRemotes + fedDomains <- fromList . fmap (.domain) <$> lift (liftSem $ FederationConfigStore.getFederationConfigs) pure $ NonConnectedBackends (request.domains \\ fedDomains) sendConnectionAction :: Domain -> NewConnectionRequest -> Handler r NewConnectionResponse @@ -118,7 +120,9 @@ sendConnectionAction originDomain NewConnectionRequest {..} = do else pure NewConnectionResponseUserNotActivated getUserByHandle :: - Member GalleyProvider r => + ( Member GalleyProvider r, + Member FederationConfigStore r + ) => Domain -> Handle -> ExceptT Error (AppT r) (Maybe UserProfile) @@ -179,7 +183,9 @@ fedClaimKeyPackages domain ckpr = -- (This decision may change in the future) searchUsers :: forall r. - Member GalleyProvider r => + ( Member GalleyProvider r, + Member FederationConfigStore r + ) => Domain -> SearchRequest -> ExceptT Error (AppT r) SearchResponse @@ -240,8 +246,8 @@ onUserDeleted origDomain udcn = lift $ do pure EmptyResponse -- | If domain is not configured fall back to `NoSearch` -lookupSearchPolicy :: Domain -> (Handler r) FederatedUserSearchPolicy +lookupSearchPolicy :: (Member FederationConfigStore r) => Domain -> (Handler r) FederatedUserSearchPolicy lookupSearchPolicy domain = do - domainConfigs <- getFederationRemotes - let mConfig = find ((== domain) . FD.domain) (domainConfigs.remotes) - pure $ maybe NoSearch FD.searchPolicy mConfig + domainConfigs <- lift $ liftSem $ FederationConfigStore.getFederationConfigs + let mConfig = find ((== domain) . (.domain)) domainConfigs + pure $ maybe NoSearch (.searchPolicy) mConfig diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 70d6834ebb1..967702f5cab 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -43,6 +43,8 @@ import Brig.Data.User qualified as Data import Brig.Effects.BlacklistPhonePrefixStore (BlacklistPhonePrefixStore) import Brig.Effects.BlacklistStore (BlacklistStore) import Brig.Effects.CodeStore (CodeStore) +import Brig.Effects.FederationConfigStore (AddFederationRemoteResult (..), FederationConfigStore) +import Brig.Effects.FederationConfigStore qualified as FederationConfigStore import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.PasswordResetStore (PasswordResetStore) import Brig.Effects.UserPendingActivationStore (UserPendingActivationStore) @@ -77,7 +79,6 @@ import Polysemy import Servant hiding (Handler, JSON, addHeader, respond) import Servant.OpenApi.Internal.Orphans () import System.Logger.Class qualified as Log -import System.Random (randomRIO) import UnliftIO.Async import Wire.API.Connection import Wire.API.Error @@ -105,7 +106,8 @@ servantSitemap :: Member BlacklistPhonePrefixStore r, Member PasswordResetStore r, Member GalleyProvider r, - Member (UserPendingActivationStore p) r + Member (UserPendingActivationStore p) r, + Member FederationConfigStore r ) => ServerT BrigIRoutes.API (Handler r) servantSitemap = @@ -213,7 +215,7 @@ authAPI = :<|> Named @"login-code" getLoginCode :<|> Named @"reauthenticate" reauthenticate -federationRemotesAPI :: ServerT BrigIRoutes.FederationRemotesAPI (Handler r) +federationRemotesAPI :: (Member FederationConfigStore r) => ServerT BrigIRoutes.FederationRemotesAPI (Handler r) federationRemotesAPI = Named @"add-federation-remotes" addFederationRemote :<|> Named @"get-federation-remotes" getFederationRemotes @@ -222,25 +224,25 @@ federationRemotesAPI = :<|> Named @"get-federation-remote-teams" getFederationRemoteTeams :<|> Named @"delete-federation-remote-team" deleteFederationRemoteTeam -deleteFederationRemoteTeam :: Domain -> TeamId -> (Handler r) () +deleteFederationRemoteTeam :: (Member FederationConfigStore r) => Domain -> TeamId -> (Handler r) () deleteFederationRemoteTeam domain teamId = - lift . wrapClient $ Data.deleteFederationRemoteTeam domain teamId + lift $ liftSem $ FederationConfigStore.removeFederationRemoteTeam domain teamId getFederationRemoteTeams :: Domain -> (Handler r) [FederationRemoteTeam] -getFederationRemoteTeams domain = - lift . wrapClient $ Data.getFederationRemoteTeams domain +getFederationRemoteTeams _domain = + error "todo" addFederationRemoteTeam :: Domain -> FederationRemoteTeam -> (Handler r) () addFederationRemoteTeam domain rt = - lift . wrapClient $ Data.addFederationRemoteTeam domain rt + addFederationRemoteTeam domain rt -addFederationRemote :: FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () +addFederationRemote :: (Member FederationConfigStore r) => FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () addFederationRemote fedDomConf = do assertNoDivergingDomainInConfigFiles fedDomConf - result <- lift . wrapClient $ Data.addFederationRemote fedDomConf + result <- lift $ liftSem $ FederationConfigStore.addFederationConfig fedDomConf case result of - Data.AddFederationRemoteSuccess -> pure () - Data.AddFederationRemoteMaxRemotesReached -> + AddFederationRemoteSuccess -> pure () + AddFederationRemoteMaxRemotesReached -> throwError . fedError . FederationUnexpectedError $ "Maximum number of remote backends reached. If you need to create more connections, \ \please contact wire.com." @@ -282,14 +284,14 @@ assertNoDivergingDomainInConfigFiles fedComConf = do <> cs (show (Map.lookup (domain fedComConf) cfg)) ) -getFederationRemotes :: ExceptT Brig.API.Error.Error (AppT r) FederationDomainConfigs +getFederationRemotes :: (Member FederationConfigStore r) => ExceptT Brig.API.Error.Error (AppT r) FederationDomainConfigs getFederationRemotes = lift $ do -- FUTUREWORK: we should solely rely on `db` in the future for remote domains; merging -- remote domains from `cfg` is just for providing an easier, more robust migration path. -- See -- https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections, -- http://docs.wire.com/developer/developer/federation-design-aspects.html#configuring-remote-connections-dev-perspective - db <- wrapClient Data.getFederationRemotes + db <- liftSem $ FederationConfigStore.getFederationConfigs (ms :: Maybe FederationStrategy, mf :: [FederationDomainConfig], mu :: Maybe Int) <- do cfg <- ask domcfgs <- remotesListFromCfgFile -- (it's not very elegant to prove the env twice here, but this code is transitory.) @@ -299,25 +301,17 @@ getFederationRemotes = lift $ do setFederationDomainConfigsUpdateFreq (cfg ^. settings) ) - -- update frequency settings of `<1` are interpreted as `1 second`. only warn about this every now and - -- then, that'll be noise enough for the logs given the traffic on this end-point. - unless (maybe True (> 0) mu) $ - randomRIO (0 :: Int, 1000) - >>= \case - 0 -> Log.warn (Log.msg (Log.val "Invalid brig configuration: setFederationDomainConfigsUpdateFreq must be > 0. setting to 1 second.")) - _ -> pure () - defFederationDomainConfigs & maybe id (\v cfg -> cfg {strategy = v}) ms - & (\cfg -> cfg {remotes = nub $ db <> mf}) + & (\cfg -> cfg {remotes = nub $ (fmap FederationConfigStore.fromFederationDomainConfig db) <> mf}) & maybe id (\v cfg -> cfg {updateInterval = min 1 v}) mu & pure -updateFederationRemote :: Domain -> FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () +updateFederationRemote :: (Member FederationConfigStore r) => Domain -> FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () updateFederationRemote dom fedcfg = do assertDomainIsNotUpdated dom fedcfg assertNoDomainsFromConfigFiles dom - (lift . wrapClient . Data.updateFederationRemote $ fedcfg) >>= \case + (lift . liftSem . FederationConfigStore.updateFederationConfig $ fedcfg) >>= \case True -> pure () False -> throwError . fedError . FederationUnexpectedError . cs $ diff --git a/services/brig/src/Brig/CanonicalInterpreter.hs b/services/brig/src/Brig/CanonicalInterpreter.hs index 5322de63927..efb5164a20a 100644 --- a/services/brig/src/Brig/CanonicalInterpreter.hs +++ b/services/brig/src/Brig/CanonicalInterpreter.hs @@ -7,6 +7,8 @@ import Brig.Effects.BlacklistStore (BlacklistStore) import Brig.Effects.BlacklistStore.Cassandra (interpretBlacklistStoreToCassandra) import Brig.Effects.CodeStore (CodeStore) import Brig.Effects.CodeStore.Cassandra (codeStoreToCassandra, interpretClientToIO) +import Brig.Effects.FederationConfigStore (FederationConfigStore) +import Brig.Effects.FederationConfigStore.Cassandra (interpretFederationDomainConfig) import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.GalleyProvider.RPC (interpretGalleyProviderToRPC) import Brig.Effects.JwtTools @@ -36,7 +38,8 @@ import Wire.Sem.Now.IO (nowToIOAction) import Wire.Sem.Paging.Cassandra (InternalPaging) type BrigCanonicalEffects = - '[ Jwk, + '[ FederationConfigStore, + Jwk, PublicKeyBundle, JwtTools, BlacklistPhonePrefixStore, @@ -79,6 +82,7 @@ runBrigToIO e (AppT ma) = do . interpretJwtTools . interpretPublicKeyBundle . interpretJwk + . interpretFederationDomainConfig ) ) $ runReaderT ma e diff --git a/services/brig/src/Brig/Effects/FederationConfigStore.hs b/services/brig/src/Brig/Effects/FederationConfigStore.hs index 1d2a85dce34..75ac54d589b 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore.hs @@ -10,7 +10,8 @@ import Wire.API.Routes.FederationDomainConfig qualified as API import Wire.API.User.Search (FederatedUserSearchPolicy) data FederationRestriction = FederationRestrictionAllowAll | FederationRestrictionByTeam [TeamId] - deriving (Eq, Show, Ord) + +data AddFederationRemoteResult = AddFederationRemoteSuccess | AddFederationRemoteMaxRemotesReached data FederationDomainConfig = FederationDomainConfig { domain :: Domain, @@ -18,10 +19,14 @@ data FederationDomainConfig = FederationDomainConfig restriction :: FederationRestriction } +fromFederationDomainConfig :: FederationDomainConfig -> API.FederationDomainConfig +fromFederationDomainConfig (FederationDomainConfig d p FederationRestrictionAllowAll) = API.FederationDomainConfig d p API.FederationRestrictionAllowAll +fromFederationDomainConfig (FederationDomainConfig d p (FederationRestrictionByTeam _)) = API.FederationDomainConfig d p API.FederationRestrictionByTeam + data FederationConfigStore m a where - GetFederationConfig :: Domain -> FederationConfigStore m FederationDomainConfig + GetFederationConfig :: Domain -> FederationConfigStore m (Maybe FederationDomainConfig) GetFederationConfigs :: FederationConfigStore m [FederationDomainConfig] - AddFederationConfig :: API.FederationDomainConfig -> FederationConfigStore m () + AddFederationConfig :: API.FederationDomainConfig -> FederationConfigStore m AddFederationRemoteResult UpdateFederationConfig :: API.FederationDomainConfig -> FederationConfigStore m Bool AddFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () RemoveFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index 830722c4d78..c735e07a062 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -38,21 +38,37 @@ interpretFederationDomainConfig :: forall m r a. (MonadClient m, Member (Embed m interpretFederationDomainConfig = interpret $ embed @m . \case - GetFederationConfig _ -> undefined + GetFederationConfig d -> getFederationConfig' d GetFederationConfigs -> getFederationConfigs' - AddFederationConfig _ -> pure () - UpdateFederationConfig _ -> pure False - AddFederationRemoteTeam _ _ -> pure () - RemoveFederationRemoteTeam _ _ -> pure () + AddFederationConfig cnf -> addFederationConfig' cnf + UpdateFederationConfig cnf -> updateFederationConfig' cnf + AddFederationRemoteTeam d t -> addFederationRemoteTeam' d t + RemoveFederationRemoteTeam d t -> removeFederationRemoteTeam' d t getFederationConfigs' :: forall m. MonadClient m => m [FederationDomainConfig] getFederationConfigs' = do - _xs <- getFederationRemotes - pure undefined + xs <- getFederationRemotes + forM xs $ \case + API.FederationDomainConfig d p API.FederationRestrictionAllowAll -> + pure $ FederationDomainConfig d p FederationRestrictionAllowAll + API.FederationDomainConfig d p API.FederationRestrictionByTeam -> + FederationDomainConfig d p . FederationRestrictionByTeam . fmap API.teamId <$> getFederationRemoteTeams d maxKnownNodes :: Int maxKnownNodes = 10000 +getFederationConfig' :: MonadClient m => Domain -> m (Maybe FederationDomainConfig) +getFederationConfig' rDomain = do + mCnf <- retry x1 (query1 q (params LocalQuorum (Identity rDomain))) + teams <- fmap API.teamId <$> getFederationRemoteTeams rDomain + pure $ + mCnf <&> \case + (sp, API.FederationRestrictionAllowAll) -> FederationDomainConfig rDomain sp FederationRestrictionAllowAll + (sp, API.FederationRestrictionByTeam) -> FederationDomainConfig rDomain sp (FederationRestrictionByTeam teams) + where + q :: PrepQuery R (Identity Domain) (FederatedUserSearchPolicy, API.FederationRestriction) + q = "SELECT search_policy, restriction FROM federation_remotes WHERE domain = ?" + getFederationRemotes :: forall m. MonadClient m => m [API.FederationDomainConfig] getFederationRemotes = (\(d, p, r) -> API.FederationDomainConfig d p r) <$$> qry where @@ -62,10 +78,8 @@ getFederationRemotes = (\(d, p, r) -> API.FederationDomainConfig d p r) <$$> qry get :: PrepQuery R () (Domain, FederatedUserSearchPolicy, API.FederationRestriction) get = fromString $ "SELECT domain, search_policy, restriction FROM federation_remotes LIMIT " <> show maxKnownNodes -data AddFederationRemoteResult = AddFederationRemoteSuccess | AddFederationRemoteMaxRemotesReached - -_addFederationRemote :: MonadClient m => API.FederationDomainConfig -> m AddFederationRemoteResult -_addFederationRemote (API.FederationDomainConfig rDomain searchPolicy restriction) = do +addFederationConfig' :: MonadClient m => API.FederationDomainConfig -> m AddFederationRemoteResult +addFederationConfig' (API.FederationDomainConfig rDomain searchPolicy restriction) = do l <- length <$> getFederationRemotes if l >= maxKnownNodes then pure AddFederationRemoteMaxRemotesReached @@ -74,8 +88,8 @@ _addFederationRemote (API.FederationDomainConfig rDomain searchPolicy restrictio add :: PrepQuery W (Domain, FederatedUserSearchPolicy, API.FederationRestriction) () add = "INSERT INTO federation_remotes (domain, search_policy, restriction) VALUES (?, ?, ?)" -_updateFederationRemote :: MonadClient m => API.FederationDomainConfig -> m Bool -_updateFederationRemote (API.FederationDomainConfig rDomain searchPolicy restriction) = do +updateFederationConfig' :: MonadClient m => API.FederationDomainConfig -> m Bool +updateFederationConfig' (API.FederationDomainConfig rDomain searchPolicy restriction) = do retry x1 (trans upd (params LocalQuorum (searchPolicy, restriction, rDomain)) {serialConsistency = Just LocalSerialConsistency}) >>= \case [] -> pure False [_] -> pure True @@ -84,29 +98,22 @@ _updateFederationRemote (API.FederationDomainConfig rDomain searchPolicy restric upd :: PrepQuery W (FederatedUserSearchPolicy, API.FederationRestriction, Domain) x upd = "UPDATE federation_remotes SET search_policy = ?, restriction = ? WHERE domain = ? IF EXISTS" -_deleteFederationRemote :: MonadClient m => Domain -> m () -_deleteFederationRemote rDomain = - retry x1 $ write delete (params LocalQuorum (Identity rDomain)) - where - delete :: PrepQuery W (Identity Domain) () - delete = "DELETE FROM federation_remotes WHERE domain = ?" - -_addFederationRemoteTeam :: MonadClient m => Domain -> API.FederationRemoteTeam -> m () -_addFederationRemoteTeam rDomain rteam = - retry x1 $ write add (params LocalQuorum (rDomain, rteam.teamId)) +addFederationRemoteTeam' :: MonadClient m => Domain -> TeamId -> m () +addFederationRemoteTeam' rDomain tid = + retry x1 $ write add (params LocalQuorum (rDomain, tid)) where add :: PrepQuery W (Domain, TeamId) () add = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" -_getFederationRemoteTeams :: MonadClient m => Domain -> m [API.FederationRemoteTeam] -_getFederationRemoteTeams rDomain = do +getFederationRemoteTeams :: MonadClient m => Domain -> m [API.FederationRemoteTeam] +getFederationRemoteTeams rDomain = do fmap (API.FederationRemoteTeam . runIdentity) <$> retry x1 (query get (params LocalQuorum (Identity rDomain))) where get :: PrepQuery R (Identity Domain) (Identity TeamId) get = "SELECT team FROM federation_remote_teams WHERE domain = ?" -_deleteFederationRemoteTeam :: MonadClient m => Domain -> TeamId -> m () -_deleteFederationRemoteTeam rDomain rteam = +removeFederationRemoteTeam' :: MonadClient m => Domain -> TeamId -> m () +removeFederationRemoteTeam' rDomain rteam = retry x1 $ write delete (params LocalQuorum (rDomain, rteam)) where delete :: PrepQuery W (Domain, TeamId) () From d62529ce767754d4e6cca9bb3e42526589746db0 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 20 Nov 2023 12:13:29 +0000 Subject: [PATCH 04/34] fix --- services/brig/src/Brig/API/Federation.hs | 10 +++++----- services/brig/src/Brig/API/Internal.hs | 10 +++++----- .../brig/src/Brig/Effects/FederationConfigStore.hs | 3 +++ .../Brig/Effects/FederationConfigStore/Cassandra.hs | 9 +++++---- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 8e49ce5d5ba..781984d2bf5 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -23,6 +23,7 @@ import Brig.API.Client qualified as API import Brig.API.Connection.Remote (performRemoteAction) import Brig.API.Error import Brig.API.Handler (Handler) +import Brig.API.Internal hiding (getMLSClients) import Brig.API.Internal qualified as Internal import Brig.API.MLS.CipherSuite import Brig.API.MLS.KeyPackages @@ -32,7 +33,6 @@ import Brig.App import Brig.Data.Connection qualified as Data import Brig.Data.User qualified as Data import Brig.Effects.FederationConfigStore (FederationConfigStore) -import Brig.Effects.FederationConfigStore qualified as FederationConfigStore import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.IO.Intra (notify) import Brig.Options @@ -104,7 +104,7 @@ getFederationStatus _ request = do case setFederationStrategy (cfg ^. settings) of Just AllowAll -> pure $ NonConnectedBackends mempty _ -> do - fedDomains <- fromList . fmap (.domain) <$> lift (liftSem $ FederationConfigStore.getFederationConfigs) + fedDomains <- fromList . fmap (.domain) . (.remotes) <$> getFederationRemotes pure $ NonConnectedBackends (request.domains \\ fedDomains) sendConnectionAction :: Domain -> NewConnectionRequest -> Handler r NewConnectionResponse @@ -248,6 +248,6 @@ onUserDeleted origDomain udcn = lift $ do -- | If domain is not configured fall back to `NoSearch` lookupSearchPolicy :: (Member FederationConfigStore r) => Domain -> (Handler r) FederatedUserSearchPolicy lookupSearchPolicy domain = do - domainConfigs <- lift $ liftSem $ FederationConfigStore.getFederationConfigs - let mConfig = find ((== domain) . (.domain)) domainConfigs - pure $ maybe NoSearch (.searchPolicy) mConfig + domainConfigs <- getFederationRemotes + let mConfig = find ((== domain) . FD.domain) (domainConfigs.remotes) + pure $ maybe NoSearch FD.searchPolicy mConfig diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 967702f5cab..dc4a020b15c 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -228,13 +228,13 @@ deleteFederationRemoteTeam :: (Member FederationConfigStore r) => Domain -> Team deleteFederationRemoteTeam domain teamId = lift $ liftSem $ FederationConfigStore.removeFederationRemoteTeam domain teamId -getFederationRemoteTeams :: Domain -> (Handler r) [FederationRemoteTeam] -getFederationRemoteTeams _domain = - error "todo" +getFederationRemoteTeams :: (Member FederationConfigStore r) => Domain -> (Handler r) [FederationRemoteTeam] +getFederationRemoteTeams domain = + lift $ liftSem $ FederationConfigStore.getFederationRemoteTeams domain -addFederationRemoteTeam :: Domain -> FederationRemoteTeam -> (Handler r) () +addFederationRemoteTeam :: (Member FederationConfigStore r) => Domain -> FederationRemoteTeam -> (Handler r) () addFederationRemoteTeam domain rt = - addFederationRemoteTeam domain rt + lift $ liftSem $ FederationConfigStore.addFederationRemoteTeam domain rt.teamId addFederationRemote :: (Member FederationConfigStore r) => FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () addFederationRemote fedDomConf = do diff --git a/services/brig/src/Brig/Effects/FederationConfigStore.hs b/services/brig/src/Brig/Effects/FederationConfigStore.hs index 75ac54d589b..c5fa3a2c730 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore.hs @@ -10,6 +10,7 @@ import Wire.API.Routes.FederationDomainConfig qualified as API import Wire.API.User.Search (FederatedUserSearchPolicy) data FederationRestriction = FederationRestrictionAllowAll | FederationRestrictionByTeam [TeamId] + deriving stock (Eq, Show, Ord) data AddFederationRemoteResult = AddFederationRemoteSuccess | AddFederationRemoteMaxRemotesReached @@ -18,6 +19,7 @@ data FederationDomainConfig = FederationDomainConfig searchPolicy :: FederatedUserSearchPolicy, restriction :: FederationRestriction } + deriving stock (Show) fromFederationDomainConfig :: FederationDomainConfig -> API.FederationDomainConfig fromFederationDomainConfig (FederationDomainConfig d p FederationRestrictionAllowAll) = API.FederationDomainConfig d p API.FederationRestrictionAllowAll @@ -30,5 +32,6 @@ data FederationConfigStore m a where UpdateFederationConfig :: API.FederationDomainConfig -> FederationConfigStore m Bool AddFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () RemoveFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () + GetFederationRemoteTeams :: Domain -> FederationConfigStore m [API.FederationRemoteTeam] makeSem ''FederationConfigStore diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index c735e07a062..b2408172060 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -44,6 +44,7 @@ interpretFederationDomainConfig = UpdateFederationConfig cnf -> updateFederationConfig' cnf AddFederationRemoteTeam d t -> addFederationRemoteTeam' d t RemoveFederationRemoteTeam d t -> removeFederationRemoteTeam' d t + GetFederationRemoteTeams d -> getFederationRemoteTeams' d getFederationConfigs' :: forall m. MonadClient m => m [FederationDomainConfig] getFederationConfigs' = do @@ -52,7 +53,7 @@ getFederationConfigs' = do API.FederationDomainConfig d p API.FederationRestrictionAllowAll -> pure $ FederationDomainConfig d p FederationRestrictionAllowAll API.FederationDomainConfig d p API.FederationRestrictionByTeam -> - FederationDomainConfig d p . FederationRestrictionByTeam . fmap API.teamId <$> getFederationRemoteTeams d + FederationDomainConfig d p . FederationRestrictionByTeam . fmap API.teamId <$> getFederationRemoteTeams' d maxKnownNodes :: Int maxKnownNodes = 10000 @@ -60,7 +61,7 @@ maxKnownNodes = 10000 getFederationConfig' :: MonadClient m => Domain -> m (Maybe FederationDomainConfig) getFederationConfig' rDomain = do mCnf <- retry x1 (query1 q (params LocalQuorum (Identity rDomain))) - teams <- fmap API.teamId <$> getFederationRemoteTeams rDomain + teams <- fmap API.teamId <$> getFederationRemoteTeams' rDomain pure $ mCnf <&> \case (sp, API.FederationRestrictionAllowAll) -> FederationDomainConfig rDomain sp FederationRestrictionAllowAll @@ -105,8 +106,8 @@ addFederationRemoteTeam' rDomain tid = add :: PrepQuery W (Domain, TeamId) () add = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" -getFederationRemoteTeams :: MonadClient m => Domain -> m [API.FederationRemoteTeam] -getFederationRemoteTeams rDomain = do +getFederationRemoteTeams' :: MonadClient m => Domain -> m [API.FederationRemoteTeam] +getFederationRemoteTeams' rDomain = do fmap (API.FederationRemoteTeam . runIdentity) <$> retry x1 (query get (params LocalQuorum (Identity rDomain))) where get :: PrepQuery R (Identity Domain) (Identity TeamId) From 22706726bc48ac63f5a30b3fec5b230da0fe43e8 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 20 Nov 2023 16:23:39 +0000 Subject: [PATCH 05/34] move stuff into interpreter --- services/brig/src/Brig/API/Internal.hs | 29 +++------- .../brig/src/Brig/CanonicalInterpreter.hs | 3 +- .../src/Brig/Effects/FederationConfigStore.hs | 2 +- .../FederationConfigStore/Cassandra.hs | 58 ++++++++++++++----- 4 files changed, 56 insertions(+), 36 deletions(-) diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index dc4a020b15c..84fea996015 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -236,7 +236,7 @@ addFederationRemoteTeam :: (Member FederationConfigStore r) => Domain -> Federat addFederationRemoteTeam domain rt = lift $ liftSem $ FederationConfigStore.addFederationRemoteTeam domain rt.teamId -addFederationRemote :: (Member FederationConfigStore r) => FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () +addFederationRemote :: (Member FederationConfigStore r) => FederationDomainConfig -> (Handler r) () addFederationRemote fedDomConf = do assertNoDivergingDomainInConfigFiles fedDomConf result <- lift $ liftSem $ FederationConfigStore.addFederationConfig fedDomConf @@ -259,14 +259,9 @@ remotesMapFromCfgFile = do else error $ "error in config file: conflicting parameters on domain: " <> show (c, c') pure $ Map.fromListWith merge dict --- | Return the config file list. Use this to make sure the config file is consistent (ie., --- no two entries for the same domain). Based on `remotesMapFromCfgFile`. -remotesListFromCfgFile :: AppT r [FederationDomainConfig] -remotesListFromCfgFile = Map.elems <$> remotesMapFromCfgFile - -- | If remote domain is registered in config file, the version that can be added to the -- database must be the same. -assertNoDivergingDomainInConfigFiles :: FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () +assertNoDivergingDomainInConfigFiles :: FederationDomainConfig -> (Handler r) () assertNoDivergingDomainInConfigFiles fedComConf = do cfg <- lift remotesMapFromCfgFile let diverges = case Map.lookup (domain fedComConf) cfg of @@ -284,7 +279,7 @@ assertNoDivergingDomainInConfigFiles fedComConf = do <> cs (show (Map.lookup (domain fedComConf) cfg)) ) -getFederationRemotes :: (Member FederationConfigStore r) => ExceptT Brig.API.Error.Error (AppT r) FederationDomainConfigs +getFederationRemotes :: (Member FederationConfigStore r) => (Handler r) FederationDomainConfigs getFederationRemotes = lift $ do -- FUTUREWORK: we should solely rely on `db` in the future for remote domains; merging -- remote domains from `cfg` is just for providing an easier, more robust migration path. @@ -292,22 +287,16 @@ getFederationRemotes = lift $ do -- https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections, -- http://docs.wire.com/developer/developer/federation-design-aspects.html#configuring-remote-connections-dev-perspective db <- liftSem $ FederationConfigStore.getFederationConfigs - (ms :: Maybe FederationStrategy, mf :: [FederationDomainConfig], mu :: Maybe Int) <- do + ms :: Maybe FederationStrategy <- do cfg <- ask - domcfgs <- remotesListFromCfgFile -- (it's not very elegant to prove the env twice here, but this code is transitory.) - pure - ( setFederationStrategy (cfg ^. settings), - domcfgs, - setFederationDomainConfigsUpdateFreq (cfg ^. settings) - ) + pure (setFederationStrategy (cfg ^. settings)) defFederationDomainConfigs & maybe id (\v cfg -> cfg {strategy = v}) ms - & (\cfg -> cfg {remotes = nub $ (fmap FederationConfigStore.fromFederationDomainConfig db) <> mf}) - & maybe id (\v cfg -> cfg {updateInterval = min 1 v}) mu + & (\cfg -> cfg {remotes = fmap FederationConfigStore.fromFederationDomainConfig db}) & pure -updateFederationRemote :: (Member FederationConfigStore r) => Domain -> FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () +updateFederationRemote :: (Member FederationConfigStore r) => Domain -> FederationDomainConfig -> (Handler r) () updateFederationRemote dom fedcfg = do assertDomainIsNotUpdated dom fedcfg assertNoDomainsFromConfigFiles dom @@ -317,14 +306,14 @@ updateFederationRemote dom fedcfg = do throwError . fedError . FederationUnexpectedError . cs $ "federation domain does not exist and cannot be updated: " <> show (dom, fedcfg) -assertDomainIsNotUpdated :: Domain -> FederationDomainConfig -> ExceptT Brig.API.Error.Error (AppT r) () +assertDomainIsNotUpdated :: Domain -> FederationDomainConfig -> (Handler r) () assertDomainIsNotUpdated dom fedcfg = do when (dom /= domain fedcfg) $ throwError . fedError . FederationUnexpectedError . cs $ "federation domain of a given peer cannot be changed from " <> show (domain fedcfg) <> " to " <> show dom <> "." -- | FUTUREWORK: should go away in the future; see 'getFederationRemotes'. -assertNoDomainsFromConfigFiles :: Domain -> ExceptT Brig.API.Error.Error (AppT r) () +assertNoDomainsFromConfigFiles :: Domain -> (Handler r) () assertNoDomainsFromConfigFiles dom = do cfg <- fmap (.federationDomainConfig) <$> asks (fromMaybe [] . setFederationDomainConfigs . view settings) when (dom `elem` (domain <$> cfg)) $ do diff --git a/services/brig/src/Brig/CanonicalInterpreter.hs b/services/brig/src/Brig/CanonicalInterpreter.hs index efb5164a20a..cd259218a4e 100644 --- a/services/brig/src/Brig/CanonicalInterpreter.hs +++ b/services/brig/src/Brig/CanonicalInterpreter.hs @@ -21,6 +21,7 @@ import Brig.Effects.ServiceRPC (Service (Galley), ServiceRPC) import Brig.Effects.ServiceRPC.IO (interpretServiceRpcToRpc) import Brig.Effects.UserPendingActivationStore (UserPendingActivationStore) import Brig.Effects.UserPendingActivationStore.Cassandra (userPendingActivationStoreToCassandra) +import Brig.Options (ImplicitNoFederationRestriction (federationDomainConfig), federationDomainConfigs) import Brig.RPC (ParseException) import Cassandra qualified as Cas import Control.Lens ((^.)) @@ -82,7 +83,7 @@ runBrigToIO e (AppT ma) = do . interpretJwtTools . interpretPublicKeyBundle . interpretJwk - . interpretFederationDomainConfig + . interpretFederationDomainConfig (maybe [] (fmap (.federationDomainConfig)) (e ^. settings . federationDomainConfigs)) ) ) $ runReaderT ma e diff --git a/services/brig/src/Brig/Effects/FederationConfigStore.hs b/services/brig/src/Brig/Effects/FederationConfigStore.hs index c5fa3a2c730..b84df100889 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore.hs @@ -19,7 +19,7 @@ data FederationDomainConfig = FederationDomainConfig searchPolicy :: FederatedUserSearchPolicy, restriction :: FederationRestriction } - deriving stock (Show) + deriving stock (Show, Eq) fromFederationDomainConfig :: FederationDomainConfig -> API.FederationDomainConfig fromFederationDomainConfig (FederationDomainConfig d p FederationRestrictionAllowAll) = API.FederationDomainConfig d p API.FederationRestrictionAllowAll diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index b2408172060..5b62e0e2f03 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -25,45 +25,75 @@ import Brig.Data.Instances () import Brig.Effects.FederationConfigStore import Cassandra import Control.Exception (ErrorCall (ErrorCall)) +import Control.Lens import Control.Monad.Catch (throwM) import Data.Domain import Data.Id +import Data.Map qualified as Map import Database.CQL.Protocol (SerialConsistency (LocalSerialConsistency), serialConsistency) import Imports import Polysemy import Wire.API.Routes.FederationDomainConfig qualified as API import Wire.API.User.Search -interpretFederationDomainConfig :: forall m r a. (MonadClient m, Member (Embed m) r) => Sem (FederationConfigStore ': r) a -> Sem r a -interpretFederationDomainConfig = +interpretFederationDomainConfig :: + forall m r a. + ( MonadClient m, + Member (Embed m) r + ) => + [API.FederationDomainConfig] -> + Sem (FederationConfigStore ': r) a -> + Sem r a +interpretFederationDomainConfig cfgs = interpret $ embed @m . \case - GetFederationConfig d -> getFederationConfig' d - GetFederationConfigs -> getFederationConfigs' + GetFederationConfig d -> getFederationConfig' cfgs d + GetFederationConfigs -> getFederationConfigs' cfgs AddFederationConfig cnf -> addFederationConfig' cnf UpdateFederationConfig cnf -> updateFederationConfig' cnf AddFederationRemoteTeam d t -> addFederationRemoteTeam' d t RemoveFederationRemoteTeam d t -> removeFederationRemoteTeam' d t GetFederationRemoteTeams d -> getFederationRemoteTeams' d -getFederationConfigs' :: forall m. MonadClient m => m [FederationDomainConfig] -getFederationConfigs' = do +-- | Compile config file list into a map indexed by domains. Use this to make sure the config +-- file is consistent (ie., no two entries for the same domain). +remotesMapFromCfgFile :: (Monad m) => [API.FederationDomainConfig] -> m (Map Domain API.FederationDomainConfig) +remotesMapFromCfgFile cfg = do + let dict = [(cnf.domain, cnf) | cnf <- cfg] + merge c c' = + if c == c' + then c + else error $ "error in config file: conflicting parameters on domain: " <> show (c, c') + pure $ Map.fromListWith merge dict + +-- | Return the config file list. Use this to make sure the config file is consistent (ie., +-- no two entries for the same domain). Based on `remotesMapFromCfgFile`. +remotesListFromCfgFile :: Monad m => [API.FederationDomainConfig] -> m [API.FederationDomainConfig] +remotesListFromCfgFile cfgs = Map.elems <$> remotesMapFromCfgFile cfgs + +getFederationConfigs' :: forall m. (MonadClient m) => [API.FederationDomainConfig] -> m [FederationDomainConfig] +getFederationConfigs' cfgs = do xs <- getFederationRemotes - forM xs $ \case - API.FederationDomainConfig d p API.FederationRestrictionAllowAll -> - pure $ FederationDomainConfig d p FederationRestrictionAllowAll - API.FederationDomainConfig d p API.FederationRestrictionByTeam -> - FederationDomainConfig d p . FederationRestrictionByTeam . fmap API.teamId <$> getFederationRemoteTeams' d + ys <- remotesListFromCfgFile cfgs + configs <- + forM (xs <> ys) $ + \case + API.FederationDomainConfig d p API.FederationRestrictionAllowAll -> + pure $ FederationDomainConfig d p FederationRestrictionAllowAll + API.FederationDomainConfig d p API.FederationRestrictionByTeam -> + FederationDomainConfig d p . FederationRestrictionByTeam . fmap API.teamId <$> getFederationRemoteTeams' d + pure $ nub configs maxKnownNodes :: Int maxKnownNodes = 10000 -getFederationConfig' :: MonadClient m => Domain -> m (Maybe FederationDomainConfig) -getFederationConfig' rDomain = do +getFederationConfig' :: MonadClient m => [API.FederationDomainConfig] -> Domain -> m (Maybe FederationDomainConfig) +getFederationConfig' cfgs rDomain = do + let mFromCfgFile = (\c -> (c.searchPolicy, c.restriction)) <$> find ((== rDomain) . API.domain) cfgs mCnf <- retry x1 (query1 q (params LocalQuorum (Identity rDomain))) teams <- fmap API.teamId <$> getFederationRemoteTeams' rDomain pure $ - mCnf <&> \case + (mFromCfgFile <|> mCnf) <&> \case (sp, API.FederationRestrictionAllowAll) -> FederationDomainConfig rDomain sp FederationRestrictionAllowAll (sp, API.FederationRestrictionByTeam) -> FederationDomainConfig rDomain sp (FederationRestrictionByTeam teams) where From cce888dcb7ca9c9fe4aac99ac89f0ed7376f5698 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 20 Nov 2023 15:03:40 +0000 Subject: [PATCH 06/34] wip --- libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs | 2 +- .../test/Test/Wire/API/Federation/API/BrigSpec.hs | 2 +- services/brig/src/Brig/API/Federation.hs | 4 +--- services/brig/src/Brig/User/API/Search.hs | 4 +++- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs b/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs index ed3586aa373..92a886d722c 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs @@ -44,7 +44,7 @@ import Wire.API.UserMap (UserMap) import Wire.API.Util.Aeson (CustomEncoded (..)) import Wire.Arbitrary (GenericUniform (..)) -newtype SearchRequest = SearchRequest {term :: Text} +data SearchRequest = SearchRequest {term :: Text, onlyInTeams :: Maybe [TeamId]} deriving (Show, Eq, Generic, Typeable) deriving (Arbitrary) via (GenericUniform SearchRequest) diff --git a/libs/wire-api-federation/test/Test/Wire/API/Federation/API/BrigSpec.hs b/libs/wire-api-federation/test/Test/Wire/API/Federation/API/BrigSpec.hs index 90031646ea8..83279c73fee 100644 --- a/libs/wire-api-federation/test/Test/Wire/API/Federation/API/BrigSpec.hs +++ b/libs/wire-api-federation/test/Test/Wire/API/Federation/API/BrigSpec.hs @@ -33,7 +33,7 @@ spec = describe "Wire.API.Federation.API.Brig" $ do describe "RoundTripTests" $ do jsonRoundTrip @SearchRequest describe "JSON Golden Tests" $ do - jsonGoldenTest "SearchRequest" [aesonQQ|{"term": "searchedThing"}|] (SearchRequest "searchedThing") + jsonGoldenTest "SearchRequest" [aesonQQ|{"term": "searchedThing"}|] (SearchRequest "searchedThing" Nothing) -- | FUTUREWORK: Extract this into a library so it is not repeated everywhere. jsonRoundTrip :: forall a. (Arbitrary a, Typeable a, ToJSON a, FromJSON a, Eq a, Show a) => Spec diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 781984d2bf5..e7f4d6004fd 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -178,9 +178,7 @@ fedClaimKeyPackages domain ckpr = claimLocalKeyPackages (tUntagged rusr) Nothing suite ltarget False -> pure Nothing --- | Searching for federated users on a remote backend should --- only search by exact handle search, not in elasticsearch. --- (This decision may change in the future) +-- | Searching for federated users on a remote backend searchUsers :: forall r. ( Member GalleyProvider r, diff --git a/services/brig/src/Brig/User/API/Search.hs b/services/brig/src/Brig/User/API/Search.hs index 43d3a760707..8e9ef7b2269 100644 --- a/services/brig/src/Brig/User/API/Search.hs +++ b/services/brig/src/Brig/User/API/Search.hs @@ -28,6 +28,7 @@ import Brig.API.Error (fedError) import Brig.API.Handler import Brig.App import Brig.Data.User qualified as DB +import Brig.Effects.FederationConfigStore (FederationConfigStore, getFederationConfigs) import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.GalleyProvider qualified as GalleyProvider import Brig.Federation.Client qualified as Federation @@ -78,12 +79,13 @@ search searcherId searchTerm maybeDomain maybeMaxResults = do then searchLocally searcherId searchTerm maybeMaxResults else searchRemotely queryDomain searchTerm -searchRemotely :: Domain -> Text -> (Handler r) (Public.SearchResult Public.Contact) +searchRemotely :: (Member FederationConfigStore r) => Domain -> Text -> (Handler r) (Public.SearchResult Public.Contact) searchRemotely domain searchTerm = do lift . Log.info $ msg (val "searchRemotely") ~~ field "domain" (show domain) ~~ field "searchTerm" searchTerm + cfgs <- getFederationConfigs searchResponse <- Federation.searchUsers domain (FedBrig.SearchRequest searchTerm) !>> fedError let contacts = S.contacts searchResponse let count = length contacts From 02cf325e2f0dae35a8678f6f86ee0df1d2c890b2 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Tue, 21 Nov 2023 11:42:34 +0000 Subject: [PATCH 07/34] wip --- libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs | 2 +- .../test/Test/Wire/API/Federation/API/BrigSpec.hs | 2 +- services/brig/src/Brig/API/Federation.hs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs b/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs index 92a886d722c..01ae3dca416 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs @@ -44,7 +44,7 @@ import Wire.API.UserMap (UserMap) import Wire.API.Util.Aeson (CustomEncoded (..)) import Wire.Arbitrary (GenericUniform (..)) -data SearchRequest = SearchRequest {term :: Text, onlyInTeams :: Maybe [TeamId]} +data SearchRequest = SearchRequest {term :: Text, from :: Maybe TeamId, onlyInTeams :: Maybe [TeamId]} deriving (Show, Eq, Generic, Typeable) deriving (Arbitrary) via (GenericUniform SearchRequest) diff --git a/libs/wire-api-federation/test/Test/Wire/API/Federation/API/BrigSpec.hs b/libs/wire-api-federation/test/Test/Wire/API/Federation/API/BrigSpec.hs index 83279c73fee..f43bac1a895 100644 --- a/libs/wire-api-federation/test/Test/Wire/API/Federation/API/BrigSpec.hs +++ b/libs/wire-api-federation/test/Test/Wire/API/Federation/API/BrigSpec.hs @@ -33,7 +33,7 @@ spec = describe "Wire.API.Federation.API.Brig" $ do describe "RoundTripTests" $ do jsonRoundTrip @SearchRequest describe "JSON Golden Tests" $ do - jsonGoldenTest "SearchRequest" [aesonQQ|{"term": "searchedThing"}|] (SearchRequest "searchedThing" Nothing) + jsonGoldenTest "SearchRequest" [aesonQQ|{"term": "searchedThing"}|] (SearchRequest "searchedThing" Nothing Nothing) -- | FUTUREWORK: Extract this into a library so it is not repeated everywhere. jsonRoundTrip :: forall a. (Arbitrary a, Typeable a, ToJSON a, FromJSON a, Eq a, Show a) => Spec diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index e7f4d6004fd..29ce1db75d7 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -187,7 +187,7 @@ searchUsers :: Domain -> SearchRequest -> ExceptT Error (AppT r) SearchResponse -searchUsers domain (SearchRequest searchTerm) = do +searchUsers domain (SearchRequest searchTerm _mTeam _mOnlyInTeams) = do searchPolicy <- lookupSearchPolicy domain let searches = case searchPolicy of From 5c5f12a69bc942d9fd73c695b2e12c7522286560 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Tue, 21 Nov 2023 13:30:18 +0000 Subject: [PATCH 08/34] wip --- services/brig/src/Brig/API/Public.hs | 4 +++- services/brig/src/Brig/User/API/Search.hs | 24 +++++++++++++------ .../brig/test/integration/API/Federation.hs | 12 +++++----- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/services/brig/src/Brig/API/Public.hs b/services/brig/src/Brig/API/Public.hs index 2f20f960bb4..5d07821f192 100644 --- a/services/brig/src/Brig/API/Public.hs +++ b/services/brig/src/Brig/API/Public.hs @@ -48,6 +48,7 @@ import Brig.Data.UserKey qualified as UserKey import Brig.Effects.BlacklistPhonePrefixStore (BlacklistPhonePrefixStore) import Brig.Effects.BlacklistStore (BlacklistStore) import Brig.Effects.CodeStore (CodeStore) +import Brig.Effects.FederationConfigStore (FederationConfigStore) import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.GalleyProvider qualified as GalleyProvider import Brig.Effects.JwtTools (JwtTools) @@ -268,7 +269,8 @@ servantSitemap :: Member PasswordResetStore r, Member PublicKeyBundle r, Member (UserPendingActivationStore p) r, - Member Jwk r + Member Jwk r, + Member FederationConfigStore r ) => ServerT BrigAPI (Handler r) servantSitemap = diff --git a/services/brig/src/Brig/User/API/Search.hs b/services/brig/src/Brig/User/API/Search.hs index 8e9ef7b2269..f39269c25f2 100644 --- a/services/brig/src/Brig/User/API/Search.hs +++ b/services/brig/src/Brig/User/API/Search.hs @@ -28,7 +28,8 @@ import Brig.API.Error (fedError) import Brig.API.Handler import Brig.App import Brig.Data.User qualified as DB -import Brig.Effects.FederationConfigStore (FederationConfigStore, getFederationConfigs) +import Brig.Effects.FederationConfigStore +import Brig.Effects.FederationConfigStore qualified as FederationConfigStore import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.GalleyProvider qualified as GalleyProvider import Brig.Federation.Client qualified as Federation @@ -61,7 +62,9 @@ import Wire.API.User.Search qualified as Public -- FUTUREWORK: Consider augmenting 'SearchResult' with full user profiles -- for all results. This is tracked in https://wearezeta.atlassian.net/browse/SQCORE-599 search :: - (Member GalleyProvider r) => + ( Member GalleyProvider r, + Member FederationConfigStore r + ) => UserId -> Text -> Maybe Domain -> @@ -74,19 +77,26 @@ search searcherId searchTerm maybeDomain maybeMaxResults = do -- looked up again. ensurePermissionsOrPersonalUser searcherId [SearchContacts] federationDomain <- viewFederationDomain + mSearcherTeamId <- lift $ wrapClient $ DB.lookupUserTeam searcherId let queryDomain = fromMaybe federationDomain maybeDomain if queryDomain == federationDomain then searchLocally searcherId searchTerm maybeMaxResults - else searchRemotely queryDomain searchTerm + else searchRemotely queryDomain mSearcherTeamId searchTerm -searchRemotely :: (Member FederationConfigStore r) => Domain -> Text -> (Handler r) (Public.SearchResult Public.Contact) -searchRemotely domain searchTerm = do +searchRemotely :: (Member FederationConfigStore r) => Domain -> Maybe TeamId -> Text -> (Handler r) (Public.SearchResult Public.Contact) +searchRemotely domain mTid searchTerm = do lift . Log.info $ msg (val "searchRemotely") ~~ field "domain" (show domain) ~~ field "searchTerm" searchTerm - cfgs <- getFederationConfigs - searchResponse <- Federation.searchUsers domain (FedBrig.SearchRequest searchTerm) !>> fedError + mFedCnf <- lift $ liftSem $ FederationConfigStore.getFederationConfig domain + let onlyInTeams = case restriction <$> mFedCnf of + Just FederationRestrictionAllowAll -> Nothing + Just (FederationRestrictionByTeam teams) -> Just teams + -- if we are not federating at all, we also do not allow to search any remote teams + Nothing -> Just [] + + searchResponse <- Federation.searchUsers domain (FedBrig.SearchRequest searchTerm mTid onlyInTeams) !>> fedError let contacts = S.contacts searchResponse let count = length contacts pure diff --git a/services/brig/test/integration/API/Federation.hs b/services/brig/test/integration/API/Federation.hs index ddaf9dc9b78..7dd448eb307 100644 --- a/services/brig/test/integration/API/Federation.hs +++ b/services/brig/test/integration/API/Federation.hs @@ -98,7 +98,7 @@ testSearchSuccess opts brig = do searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ - SearchRequest (fromHandle handle) + SearchRequest (fromHandle handle) Nothing Nothing liftIO $ do let contacts = contactQualifiedId <$> S.contacts searchResponse @@ -115,7 +115,7 @@ testFulltextSearchSuccess opts brig = do searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ - SearchRequest (fromName $ userDisplayName user) + SearchRequest (fromName $ userDisplayName user) Nothing Nothing liftIO $ do let contacts = contactQualifiedId <$> S.contacts searchResponse @@ -142,7 +142,7 @@ testFulltextSearchMultipleUsers opts brig = do searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ - SearchRequest (fromHandle handle) + SearchRequest (fromHandle handle) Nothing Nothing liftIO $ do let contacts = contactQualifiedId <$> S.contacts searchResponse @@ -155,7 +155,7 @@ testSearchNotFound opts = do searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ - SearchRequest "this-handle-should-not-exist" + SearchRequest "this-handle-should-not-exist" Nothing Nothing liftIO $ assertEqual "should return empty array of users" [] (S.contacts searchResponse) @@ -166,7 +166,7 @@ testSearchNotFoundEmpty opts = do searchResponse <- withSettingsOverrides (allowFullSearch domain opts) $ do runWaiTestFedClient domain $ createWaiTestFedClient @"search-users" @'Brig $ - SearchRequest "this-handle-should-not-exist" + SearchRequest "this-handle-should-not-exist" Nothing Nothing liftIO $ assertEqual "should return empty array of users" [] (S.contacts searchResponse) @@ -194,7 +194,7 @@ testSearchRestrictions opts brig = do let squery = either fromHandle fromName handleOrName searchResponse <- runWaiTestFedClient domain $ - createWaiTestFedClient @"search-users" @'Brig (SearchRequest squery) + createWaiTestFedClient @"search-users" @'Brig (SearchRequest squery Nothing Nothing) liftIO $ do case (mExpectedUser, handleOrName) of (Just expectedUser, Right _) -> From b0a0f4a88b05a201f62133dffcd5496b71403a30 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Tue, 21 Nov 2023 15:43:09 +0000 Subject: [PATCH 09/34] implement team federation policy for remote search --- services/brig/src/Brig/API/Federation.hs | 37 +++++++++++++++---- .../brig/src/Brig/User/Search/SearchIndex.hs | 23 +++++++++--- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 29ce1db75d7..5fa63c52181 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -33,6 +33,7 @@ import Brig.App import Brig.Data.Connection qualified as Data import Brig.Data.User qualified as Data import Brig.Effects.FederationConfigStore (FederationConfigStore) +import Brig.Effects.FederationConfigStore qualified as FCS import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.IO.Intra (notify) import Brig.Options @@ -44,7 +45,7 @@ import Control.Lens ((^.)) import Control.Monad.Trans.Except import Data.Domain import Data.Handle (Handle (..), parseHandle) -import Data.Id (ClientId, UserId) +import Data.Id (ClientId, TeamId, UserId) import Data.List.NonEmpty (nonEmpty) import Data.List1 import Data.Qualified @@ -127,6 +128,7 @@ getUserByHandle :: Handle -> ExceptT Error (AppT r) (Maybe UserProfile) getUserByHandle domain handle = do + -- TODO: should we do the team federation check here, too? searchPolicy <- lookupSearchPolicy domain let performHandleLookup = @@ -187,8 +189,8 @@ searchUsers :: Domain -> SearchRequest -> ExceptT Error (AppT r) SearchResponse -searchUsers domain (SearchRequest searchTerm _mTeam _mOnlyInTeams) = do - searchPolicy <- lookupSearchPolicy domain +searchUsers domain (SearchRequest searchTerm mTeam mOnlyInTeams) = do + searchPolicy <- lookupSearchPolicyWithTeam domain mTeam let searches = case searchPolicy of NoSearch -> [] @@ -208,7 +210,7 @@ searchUsers domain (SearchRequest searchTerm _mTeam _mOnlyInTeams) = do fullSearch :: Int -> ExceptT Error (AppT r) [Contact] fullSearch n - | n > 0 = lift $ searchResults <$> Q.searchIndex Q.FederatedSearch searchTerm n + | n > 0 = lift $ searchResults <$> Q.searchIndex (Q.FederatedSearch mOnlyInTeams) searchTerm n | otherwise = pure [] exactHandleSearch :: Int -> ExceptT Error (AppT r) [Contact] @@ -218,9 +220,16 @@ searchUsers domain (SearchRequest searchTerm _mTeam _mOnlyInTeams) = do maybeOwnerId <- maybe (pure Nothing) (wrapHttpClientE . API.lookupHandle) maybeHandle case maybeOwnerId of Nothing -> pure [] - Just foundUser -> lift $ contactFromProfile <$$> API.lookupLocalProfiles Nothing [foundUser] + Just foundUser -> do + mFoundUserTeamId <- lift $ wrapClient $ Data.lookupUserTeam foundUser + if isTeamAllowed mOnlyInTeams mFoundUserTeamId + then lift $ contactFromProfile <$$> API.lookupLocalProfiles Nothing [foundUser] + else pure [] | otherwise = pure [] + isTeamAllowed :: Maybe [TeamId] -> Maybe TeamId -> Bool + isTeamAllowed = undefined + getUserClients :: Domain -> GetUserClients -> (Handler r) (UserMap (Set PubClient)) getUserClients _ (GetUserClients uids) = API.lookupLocalPubClientsBulk uids !>> clientError @@ -246,6 +255,18 @@ onUserDeleted origDomain udcn = lift $ do -- | If domain is not configured fall back to `NoSearch` lookupSearchPolicy :: (Member FederationConfigStore r) => Domain -> (Handler r) FederatedUserSearchPolicy lookupSearchPolicy domain = do - domainConfigs <- getFederationRemotes - let mConfig = find ((== domain) . FD.domain) (domainConfigs.remotes) - pure $ maybe NoSearch FD.searchPolicy mConfig + mConfig <- lift $ liftSem $ FCS.getFederationConfig domain + pure $ maybe NoSearch FCS.searchPolicy mConfig + +-- | If domain is not configured fall back to `NoSearch` +-- if a team is provided, check if the team is allowed to search +-- if no team is provided, and restriction is set by team, fall back to `NoSearch` +lookupSearchPolicyWithTeam :: (Member FederationConfigStore r) => Domain -> Maybe TeamId -> (Handler r) FederatedUserSearchPolicy +lookupSearchPolicyWithTeam domain mSearcherTeamId = + lift $ + liftSem $ + FCS.getFederationConfig domain <&> \case + Nothing -> NoSearch + Just (FCS.FederationDomainConfig _ sp FCS.FederationRestrictionAllowAll) -> sp + Just (FCS.FederationDomainConfig _ sp (FCS.FederationRestrictionByTeam teams)) -> + maybe NoSearch (\tid -> if tid `elem` teams then sp else NoSearch) $ mSearcherTeamId diff --git a/services/brig/src/Brig/User/Search/SearchIndex.hs b/services/brig/src/Brig/User/Search/SearchIndex.hs index 53c633ff4ca..7419802c851 100644 --- a/services/brig/src/Brig/User/Search/SearchIndex.hs +++ b/services/brig/src/Brig/User/Search/SearchIndex.hs @@ -44,14 +44,14 @@ import Wire.API.User.Search -- Team of user that is performing the search -- Outgoing search restrictions data SearchSetting - = FederatedSearch + = FederatedSearch (Maybe [TeamId]) | LocalSearch UserId (Maybe TeamId) TeamSearchInfo searchSettingTeam :: SearchSetting -> Maybe TeamId -searchSettingTeam FederatedSearch = Nothing +searchSettingTeam (FederatedSearch _) = Nothing searchSettingTeam (LocalSearch _ mbTeam _) = mbTeam searchIndex :: @@ -186,12 +186,12 @@ termQ f v = Nothing matchSelf :: SearchSetting -> Maybe ES.Query -matchSelf FederatedSearch = Nothing +matchSelf (FederatedSearch _) = Nothing matchSelf (LocalSearch searcher _tid _searchInfo) = Just (termQ "_id" (review _TextId searcher)) -- | See 'TeamSearchInfo' restrictSearchSpace :: SearchSetting -> ES.Query -restrictSearchSpace FederatedSearch = +restrictSearchSpace (FederatedSearch Nothing) = ES.QueryBoolQuery boolQuery { ES.boolQueryShouldMatch = @@ -199,6 +199,16 @@ restrictSearchSpace FederatedSearch = matchTeamMembersSearchableByAllTeams ] } +restrictSearchSpace (FederatedSearch (Just teams)) = + ES.QueryBoolQuery + boolQuery + { ES.boolQueryMustMatch = + [ matchTeamMembersSearchableByAllTeams, + onlyInTeams + ] + } + where + onlyInTeams = ES.QueryBoolQuery boolQuery {ES.boolQueryShouldMatch = map matchTeamMembersOf teams} restrictSearchSpace (LocalSearch _uid mteam searchInfo) = case (mteam, searchInfo) of (Nothing, _) -> matchNonTeamMemberUsers @@ -216,8 +226,9 @@ restrictSearchSpace (LocalSearch _uid mteam searchInfo) = matchTeamMembersOf searcherTeam ] } - where - matchTeamMembersOf team = ES.TermQuery (ES.Term "team" $ idToText team) Nothing + +matchTeamMembersOf :: TeamId -> ES.Query +matchTeamMembersOf team = ES.TermQuery (ES.Term "team" $ idToText team) Nothing matchTeamMembersSearchableByAllTeams :: ES.Query matchTeamMembersSearchableByAllTeams = From 0b1c0e1be27e5fa60092ca4e7b8030231c304e19 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Tue, 21 Nov 2023 15:59:40 +0000 Subject: [PATCH 10/34] impl isTeamAllowed --- services/brig/src/Brig/API/Federation.hs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 5fa63c52181..73c721e00a4 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -228,7 +228,9 @@ searchUsers domain (SearchRequest searchTerm mTeam mOnlyInTeams) = do | otherwise = pure [] isTeamAllowed :: Maybe [TeamId] -> Maybe TeamId -> Bool - isTeamAllowed = undefined + isTeamAllowed Nothing _ = True + isTeamAllowed (Just _) Nothing = False + isTeamAllowed (Just teams) (Just tid) = tid `elem` teams getUserClients :: Domain -> GetUserClients -> (Handler r) (UserMap (Set PubClient)) getUserClients _ (GetUserClients uids) = API.lookupLocalPubClientsBulk uids !>> clientError From 877ec64ec9b9ac78c6f2470750822e23d1d4bce6 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 22 Nov 2023 09:56:16 +0000 Subject: [PATCH 11/34] wip --- integration/integration.cabal | 1 + integration/test/Test/Brig.hs | 48 ------------------------- integration/test/Test/Search.hs | 64 +++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 48 deletions(-) create mode 100644 integration/test/Test/Search.hs diff --git a/integration/integration.cabal b/integration/integration.cabal index e31f550f900..df86cf50248 100644 --- a/integration/integration.cabal +++ b/integration/integration.cabal @@ -120,6 +120,7 @@ library Test.Notifications Test.Presence Test.Roles + Test.Search Test.User Testlib.App Testlib.Assertions diff --git a/integration/test/Test/Brig.hs b/integration/test/Test/Brig.hs index cd067699659..3e3dba63e4f 100644 --- a/integration/test/Test/Brig.hs +++ b/integration/test/Test/Brig.hs @@ -2,8 +2,6 @@ module Test.Brig where import API.Brig qualified as BrigP import API.BrigInternal qualified as BrigI -import API.Common qualified as API -import API.GalleyInternal qualified as GalleyI import Data.Aeson.Types hiding ((.=)) import Data.Set qualified as Set import Data.String.Conversions @@ -14,17 +12,6 @@ import SetupHelpers import Testlib.Assertions import Testlib.Prelude -testSearchContactForExternalUsers :: HasCallStack => App () -testSearchContactForExternalUsers = do - owner <- randomUser OwnDomain def {BrigI.team = True} - partner <- randomUser OwnDomain def {BrigI.team = True} - - bindResponse (GalleyI.putTeamMember partner (partner %. "team") (API.teamRole "partner")) $ \resp -> - resp.status `shouldMatchInt` 200 - - bindResponse (BrigP.searchContacts partner (owner %. "name") OwnDomain) $ \resp -> - resp.status `shouldMatchInt` 403 - testCrudFederationRemotes :: HasCallStack => App () testCrudFederationRemotes = do otherDomain <- asString OtherDomain @@ -139,41 +126,6 @@ testSwagger = do resp.status `shouldMatchInt` 200 void resp.json -testRemoteUserSearch :: HasCallStack => App () -testRemoteUserSearch = do - startDynamicBackends [def, def] $ \[d1, d2] -> do - void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" "allow_all") - - u1 <- randomUser d1 def - u2 <- randomUser d2 def - BrigI.refreshIndex d2 - uidD2 <- objId u2 - - bindResponse (BrigP.searchContacts u1 (u2 %. "name") d2) $ \resp -> do - resp.status `shouldMatchInt` 200 - docs <- resp.json %. "documents" >>= asList - case docs of - [] -> assertFailure "Expected a non empty result, but got an empty one" - doc : _ -> doc %. "id" `shouldMatch` uidD2 - -testRemoteUserSearchExactHandle :: HasCallStack => App () -testRemoteUserSearchExactHandle = do - startDynamicBackends [def, def] $ \[d1, d2] -> do - void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "exact_handle_search" "allow_all") - - u1 <- randomUser d1 def - u2 <- randomUser d2 def - u2Handle <- API.randomHandle - bindResponse (BrigP.putHandle u2 u2Handle) $ assertSuccess - BrigI.refreshIndex d2 - - bindResponse (BrigP.searchContacts u1 u2Handle d2) $ \resp -> do - resp.status `shouldMatchInt` 200 - docs <- resp.json %. "documents" >>= asList - case docs of - [] -> assertFailure "Expected a non empty result, but got an empty one" - doc : _ -> objQid doc `shouldMatch` objQid u2 - testCrudFederationRemoteTeams :: HasCallStack => App () testCrudFederationRemoteTeams = do (_, tid, _) <- createTeam OwnDomain 1 diff --git a/integration/test/Test/Search.hs b/integration/test/Test/Search.hs new file mode 100644 index 00000000000..9dfafc4330d --- /dev/null +++ b/integration/test/Test/Search.hs @@ -0,0 +1,64 @@ +module Test.Search where + +import API.Brig qualified as BrigP +import API.BrigInternal qualified as BrigI +import API.Common qualified as API +import API.GalleyInternal qualified as GalleyI +import GHC.Stack +import SetupHelpers +import Testlib.Assertions +import Testlib.Prelude + +-------------------------------------------------------------------------------- +-- LOCAL SEARCH + +testSearchContactForExternalUsers :: HasCallStack => App () +testSearchContactForExternalUsers = do + owner <- randomUser OwnDomain def {BrigI.team = True} + partner <- randomUser OwnDomain def {BrigI.team = True} + + bindResponse (GalleyI.putTeamMember partner (partner %. "team") (API.teamRole "partner")) $ \resp -> + resp.status `shouldMatchInt` 200 + + bindResponse (BrigP.searchContacts partner (owner %. "name") OwnDomain) $ \resp -> + resp.status `shouldMatchInt` 403 + +-------------------------------------------------------------------------------- +-- FEDERATION SEARCH + +testRemoteUserSearch :: HasCallStack => App () +testRemoteUserSearch = do + startDynamicBackends [def, def] $ \[d1, d2] -> do + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" "allow_all") + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" "allow_all") + + u1 <- randomUser d1 def + u2 <- randomUser d2 def + BrigI.refreshIndex d2 + uidD2 <- objId u2 + + bindResponse (BrigP.searchContacts u1 (u2 %. "name") d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> assertFailure "Expected a non empty result, but got an empty one" + doc : _ -> doc %. "id" `shouldMatch` uidD2 + +testRemoteUserSearchExactHandle :: HasCallStack => App () +testRemoteUserSearchExactHandle = do + startDynamicBackends [def, def] $ \[d1, d2] -> do + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "exact_handle_search" "allow_all") + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "exact_handle_search" "allow_all") + + u1 <- randomUser d1 def + u2 <- randomUser d2 def + u2Handle <- API.randomHandle + bindResponse (BrigP.putHandle u2 u2Handle) $ assertSuccess + BrigI.refreshIndex d2 + + bindResponse (BrigP.searchContacts u1 u2Handle d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> assertFailure "Expected a non empty result, but got an empty one" + doc : _ -> objQid doc `shouldMatch` objQid u2 From 26b79ea761f9198b030bafdd39217155c216af0e Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 22 Nov 2023 14:24:07 +0000 Subject: [PATCH 12/34] tests --- integration/test/API/GalleyInternal.hs | 7 + integration/test/Test/Search.hs | 175 +++++++++++++++++++++++++ 2 files changed, 182 insertions(+) diff --git a/integration/test/API/GalleyInternal.hs b/integration/test/API/GalleyInternal.hs index 428199004e5..f683e41a639 100644 --- a/integration/test/API/GalleyInternal.hs +++ b/integration/test/API/GalleyInternal.hs @@ -37,6 +37,13 @@ getTeamFeature domain_ featureName tid = do req <- baseRequest domain_ Galley Unversioned $ joinHttpPath ["i", "teams", tid, "features", featureName] submit "GET" $ req +setTeamFeatureStatus :: (HasCallStack, MakesValue domain, MakesValue team) => domain -> team -> String -> String -> App () +setTeamFeatureStatus domain team featureName status = do + tid <- asString team + req <- baseRequest domain Galley Unversioned $ joinHttpPath ["i", "teams", tid, "features", featureName] + res <- submit "PATCH" $ req & addJSONObject ["status" .= status] + res.status `shouldMatchInt` 200 + getFederationStatus :: ( HasCallStack, MakesValue user diff --git a/integration/test/Test/Search.hs b/integration/test/Test/Search.hs index 9dfafc4330d..e8f14ec60f0 100644 --- a/integration/test/Test/Search.hs +++ b/integration/test/Test/Search.hs @@ -62,3 +62,178 @@ testRemoteUserSearchExactHandle = do case docs of [] -> assertFailure "Expected a non empty result, but got an empty one" doc : _ -> objQid doc `shouldMatch` objQid u2 + +-------------------------------------------------------------------------------- +-- FEDERATION SEARCH WITH TEAM RESTRICTIONS + +data Restriction = AllowAll | TeamAllowed | TeamNotAllowed + deriving (Eq, Ord, Show) + +data FedUserSearchTestCase = FedUserSearchTestCase + { searchPolicy :: String, + restrictionD1D2 :: Restriction, + restrictionD2D1 :: Restriction, + exactHandleSearchExpectFound :: Bool, + fullSearchExpectFound :: Bool + } + deriving (Eq, Ord, Show) + +testFederatedUserSearchNoSearch :: HasCallStack => App () +testFederatedUserSearchNoSearch = do + let testCases = + [ FedUserSearchTestCase "no_search" AllowAll AllowAll False False, + FedUserSearchTestCase "no_search" TeamAllowed TeamAllowed False False + ] + forM_ testCases federatedUserSearch + +testFederatedUserSearchExactHandleSearchAllowAll :: HasCallStack => App () +testFederatedUserSearchExactHandleSearchAllowAll = do + let testCases = + [ FedUserSearchTestCase "exact_handle_search" AllowAll AllowAll True False, + FedUserSearchTestCase "exact_handle_search" TeamAllowed TeamAllowed True False, + FedUserSearchTestCase "exact_handle_search" AllowAll TeamAllowed True False, + FedUserSearchTestCase "exact_handle_search" TeamAllowed AllowAll True False + ] + forM_ testCases federatedUserSearch + +testFederatedUserSearchExactHandleSearchTeamNotAllowed :: HasCallStack => App () +testFederatedUserSearchExactHandleSearchTeamNotAllowed = do + let testCases = + [ FedUserSearchTestCase "exact_handle_search" TeamNotAllowed AllowAll False False, + FedUserSearchTestCase "exact_handle_search" AllowAll TeamNotAllowed False False + ] + forM_ testCases federatedUserSearch + +testFederatedUserSearchFullSearchAllowAll :: HasCallStack => App () +testFederatedUserSearchFullSearchAllowAll = do + let testCases = + [ FedUserSearchTestCase "full_search" AllowAll AllowAll True True, + FedUserSearchTestCase "full_search" TeamAllowed TeamAllowed True True, + FedUserSearchTestCase "full_search" TeamAllowed AllowAll True True, + FedUserSearchTestCase "full_search" AllowAll TeamAllowed True True + ] + forM_ testCases federatedUserSearch + +testFederatedUserSearchFullSearchTeamNotAllowed :: HasCallStack => App () +testFederatedUserSearchFullSearchTeamNotAllowed = do + let testCases = + [ FedUserSearchTestCase "full_search" TeamNotAllowed AllowAll False False, + FedUserSearchTestCase "full_search" AllowAll TeamNotAllowed False False + ] + forM_ testCases federatedUserSearch + +federatedUserSearch :: HasCallStack => FedUserSearchTestCase -> App () +federatedUserSearch test = do + startDynamicBackends [def, def] $ \[d1, d2] -> do + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 test.searchPolicy (restriction test.restrictionD2D1)) + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 test.searchPolicy (restriction test.restrictionD1D2)) + + u1 <- randomUser d1 def {BrigI.team = True} + teamU1 <- u1 %. "team" + u2 <- randomUser d2 def {BrigI.team = True} + uidD2 <- objId u2 + team2 <- u2 %. "team" + GalleyI.setTeamFeatureStatus d2 team2 "searchVisibilityInbound" "enabled" + + addTeamRestriction d1 d2 team2 test.restrictionD1D2 + addTeamRestriction d2 d1 teamU1 test.restrictionD2D1 + + u2Handle <- API.randomHandle + bindResponse (BrigP.putHandle u2 u2Handle) $ assertSuccess + BrigI.refreshIndex d2 + + bindResponse (BrigP.searchContacts u1 u2Handle d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> + when (test.exactHandleSearchExpectFound) $ assertFailure $ "Expected a non empty result, but got an empty one, for test case " <> show test + doc : _ -> + if test.exactHandleSearchExpectFound + then doc %. "id" `shouldMatch` uidD2 + else assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " <> show test + + bindResponse (BrigP.searchContacts u1 (u2 %. "name") d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> when (test.fullSearchExpectFound) $ assertFailure $ "Expected a non empty result, but got an empty one, for test case " <> show test + doc : _ -> + if test.fullSearchExpectFound + then doc %. "id" `shouldMatch` uidD2 + else assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " <> show test + where + restriction :: Restriction -> String + restriction = \case + AllowAll -> "allow_all" + TeamAllowed -> "restrict_by_team" + TeamNotAllowed -> "restrict_by_team" + + addTeamRestriction :: (MakesValue ownDomain, MakesValue remoteDomain, MakesValue remoteTeam) => ownDomain -> remoteDomain -> remoteTeam -> Restriction -> App () + addTeamRestriction ownDomain remoteDomain remoteTeam = \case + AllowAll -> + pure () + TeamNotAllowed -> + pure () + TeamAllowed -> do + BrigI.addFederationRemoteTeam ownDomain remoteDomain remoteTeam + +testFederatedUserSearchNonTeamSearcher :: HasCallStack => App () +testFederatedUserSearchNonTeamSearcher = do + startDynamicBackends [def, def] $ \[d1, d2] -> do + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" "restrict_by_team") + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" "allow_all") + + u1 <- randomUser d1 def + u2 <- randomUser d2 def {BrigI.team = True} + team2 <- u2 %. "team" + GalleyI.setTeamFeatureStatus d2 team2 "searchVisibilityInbound" "enabled" + + u2Handle <- API.randomHandle + bindResponse (BrigP.putHandle u2 u2Handle) $ assertSuccess + BrigI.refreshIndex d2 + + bindResponse (BrigP.searchContacts u1 u2Handle d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> pure () + doc : _ -> + assertFailure $ "Expected an empty result, but got " <> show doc + + bindResponse (BrigP.searchContacts u1 (u2 %. "name") d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> pure () + doc : _ -> + assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " + +testFederatedSearchForNonTeamUser :: HasCallStack => App () +testFederatedSearchForNonTeamUser = do + startDynamicBackends [def, def] $ \[d1, d2] -> do + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" "allow_all") + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" "restrict_by_team") + + u1 <- randomUser d1 def {BrigI.team = True} + u2 <- randomUser d2 def + + u2Handle <- API.randomHandle + bindResponse (BrigP.putHandle u2 u2Handle) $ assertSuccess + BrigI.refreshIndex d2 + + bindResponse (BrigP.searchContacts u1 u2Handle d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> pure () + doc : _ -> + assertFailure $ "Expected an empty result, but got " <> show doc + + bindResponse (BrigP.searchContacts u1 (u2 %. "name") d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> pure () + doc : _ -> + assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " From 7911d238614e803a792f7b08b973c0faa1cfe7b2 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 23 Nov 2023 09:29:36 +0000 Subject: [PATCH 13/34] changelog --- changelog.d/2-features/WPB-5105 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/changelog.d/2-features/WPB-5105 b/changelog.d/2-features/WPB-5105 index a57f38f2bee..933b56ee005 100644 --- a/changelog.d/2-features/WPB-5105 +++ b/changelog.d/2-features/WPB-5105 @@ -1,3 +1,4 @@ Allowlist for who on cloud can connect to on-prem: - Internal API to configure allowlist -(#3697) +- Restrict federated user search according to team federation policy +(#3697, #3732) From 171b6a75a59ed461f4dcddd9a7594bfad6ac7940 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 23 Nov 2023 15:27:38 +0000 Subject: [PATCH 14/34] unify API and domain models --- integration/test/API/BrigInternal.hs | 6 +- integration/test/Test/Brig.hs | 6 +- integration/test/Test/Conversation.hs | 28 ++-- integration/test/Test/Search.hs | 24 ++-- .../Wire/API/Routes/FederationDomainConfig.hs | 23 ++-- services/brig/src/Brig/API/Federation.hs | 16 +-- services/brig/src/Brig/API/Internal.hs | 4 +- services/brig/src/Brig/Data/Instances.hs | 11 -- .../src/Brig/Effects/FederationConfigStore.hs | 23 +--- .../FederationConfigStore/Cassandra.hs | 130 ++++++++++++------ services/brig/src/Brig/User/API/Search.hs | 1 + 11 files changed, 152 insertions(+), 120 deletions(-) diff --git a/integration/test/API/BrigInternal.hs b/integration/test/API/BrigInternal.hs index 6a4ae4d6e21..c78ee800ec2 100644 --- a/integration/test/API/BrigInternal.hs +++ b/integration/test/API/BrigInternal.hs @@ -4,6 +4,8 @@ import API.Common import Data.Aeson qualified as Aeson import Data.Function import Data.Maybe +import Data.String.Conversions (cs) +import Data.Vector qualified as Vector import Testlib.Prelude data CreateUser = CreateUser @@ -54,7 +56,7 @@ createUser domain cu = do data FedConn = FedConn { domain :: String, searchStrategy :: String, - restriction :: String + restriction :: Maybe [String] } deriving (Eq, Ord, Show) @@ -63,7 +65,7 @@ instance ToJSON FedConn where Aeson.object [ "domain" .= d, "search_policy" .= s, - "restriction" .= r + "restriction" .= maybe Aeson.Null (\teams -> Aeson.Array (Vector.fromList (Aeson.String . cs <$> teams))) r ] instance MakesValue FedConn where diff --git a/integration/test/Test/Brig.hs b/integration/test/Test/Brig.hs index 3e3dba63e4f..b1256750446 100644 --- a/integration/test/Test/Brig.hs +++ b/integration/test/Test/Brig.hs @@ -41,11 +41,11 @@ testCrudFederationRemotes = do dom1 :: String <- (<> ".example.com") . UUID.toString <$> liftIO UUID.nextRandom let remote1, remote1' :: BrigI.FedConn - remote1 = BrigI.FedConn dom1 "no_search" "allow_all" - remote1' = remote1 {BrigI.searchStrategy = "full_search", BrigI.restriction = "restrict_by_team"} + remote1 = BrigI.FedConn dom1 "no_search" Nothing + remote1' = remote1 {BrigI.searchStrategy = "full_search", BrigI.restriction = Just []} cfgRemotesExpect :: BrigI.FedConn - cfgRemotesExpect = BrigI.FedConn (cs otherDomain) "full_search" "allow_all" + cfgRemotesExpect = BrigI.FedConn (cs otherDomain) "full_search" Nothing cfgRemotes <- parseFedConns =<< BrigI.readFedConns ownDomain cfgRemotes `shouldMatch` ([] @Value) diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index 73464a7973d..87b588614ed 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -78,7 +78,7 @@ testDynamicBackendsFullyConnectedWhenAllowDynamic = do -- Allowing 'full_search' or any type of search is how we enable federation -- between backends when the federation strategy is 'allowDynamic'. sequence_ - [ createFedConn x (FedConn y "full_search" "allow_all") + [ createFedConn x (FedConn y "full_search" Nothing) | x <- [domainA, domainB, domainC], y <- [domainA, domainB, domainC], x /= y @@ -100,10 +100,10 @@ testDynamicBackendsNotFullyConnected :: HasCallStack => App () testDynamicBackendsNotFullyConnected = do withFederatingBackendsAllowDynamic $ \(domainA, domainB, domainC) -> do -- A is connected to B and C, but B and C are not connected to each other - void $ createFedConn domainA $ FedConn domainB "full_search" "allow_all" - void $ createFedConn domainB $ FedConn domainA "full_search" "allow_all" - void $ createFedConn domainA $ FedConn domainC "full_search" "allow_all" - void $ createFedConn domainC $ FedConn domainA "full_search" "allow_all" + void $ createFedConn domainA $ FedConn domainB "full_search" Nothing + void $ createFedConn domainB $ FedConn domainA "full_search" Nothing + void $ createFedConn domainA $ FedConn domainC "full_search" Nothing + void $ createFedConn domainC $ FedConn domainA "full_search" Nothing uidA <- randomUser domainA def {team = True} retryT $ bindResponse @@ -149,10 +149,10 @@ testCreateConversationNonFullyConnected :: HasCallStack => App () testCreateConversationNonFullyConnected = do withFederatingBackendsAllowDynamic $ \(domainA, domainB, domainC) -> do -- A is connected to B and C, but B and C are not connected to each other - void $ createFedConn domainA $ FedConn domainB "full_search" "allow_all" - void $ createFedConn domainB $ FedConn domainA "full_search" "allow_all" - void $ createFedConn domainA $ FedConn domainC "full_search" "allow_all" - void $ createFedConn domainC $ FedConn domainA "full_search" "allow_all" + void $ createFedConn domainA $ FedConn domainB "full_search" Nothing + void $ createFedConn domainB $ FedConn domainA "full_search" Nothing + void $ createFedConn domainA $ FedConn domainC "full_search" Nothing + void $ createFedConn domainC $ FedConn domainA "full_search" Nothing liftIO $ threadDelay (2 * 1000 * 1000) u1 <- randomUser domainA def @@ -184,10 +184,10 @@ testAddMembersFullyConnectedProteus = do testAddMembersNonFullyConnectedProteus :: HasCallStack => App () testAddMembersNonFullyConnectedProteus = do withFederatingBackendsAllowDynamic $ \(domainA, domainB, domainC) -> do - void $ createFedConn domainA (FedConn domainB "full_search" "allow_all") - void $ createFedConn domainB (FedConn domainA "full_search" "allow_all") - void $ createFedConn domainA (FedConn domainC "full_search" "allow_all") - void $ createFedConn domainC (FedConn domainA "full_search" "allow_all") + void $ createFedConn domainA (FedConn domainB "full_search" Nothing) + void $ createFedConn domainB (FedConn domainA "full_search" Nothing) + void $ createFedConn domainA (FedConn domainC "full_search" Nothing) + void $ createFedConn domainC (FedConn domainA "full_search" Nothing) liftIO $ threadDelay (2 * 1000 * 1000) -- wait for federation status to be updated -- add users @@ -386,7 +386,7 @@ testAddingUserNonFullyConnectedFederation = do -- Ensure that dynamic backend only federates with own domain, but not other -- domain. - void $ createFedConn dynBackend (FedConn own "full_search" "allow_all") + void $ createFedConn dynBackend (FedConn own "full_search" Nothing) alice <- randomUser own def bob <- randomUser other def diff --git a/integration/test/Test/Search.hs b/integration/test/Test/Search.hs index e8f14ec60f0..b8c293f2966 100644 --- a/integration/test/Test/Search.hs +++ b/integration/test/Test/Search.hs @@ -29,8 +29,8 @@ testSearchContactForExternalUsers = do testRemoteUserSearch :: HasCallStack => App () testRemoteUserSearch = do startDynamicBackends [def, def] $ \[d1, d2] -> do - void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" "allow_all") - void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" "allow_all") + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" Nothing) + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" Nothing) u1 <- randomUser d1 def u2 <- randomUser d2 def @@ -47,8 +47,8 @@ testRemoteUserSearch = do testRemoteUserSearchExactHandle :: HasCallStack => App () testRemoteUserSearchExactHandle = do startDynamicBackends [def, def] $ \[d1, d2] -> do - void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "exact_handle_search" "allow_all") - void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "exact_handle_search" "allow_all") + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "exact_handle_search" Nothing) + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "exact_handle_search" Nothing) u1 <- randomUser d1 def u2 <- randomUser d2 def @@ -163,11 +163,11 @@ federatedUserSearch test = do then doc %. "id" `shouldMatch` uidD2 else assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " <> show test where - restriction :: Restriction -> String + restriction :: Restriction -> Maybe [String] restriction = \case - AllowAll -> "allow_all" - TeamAllowed -> "restrict_by_team" - TeamNotAllowed -> "restrict_by_team" + AllowAll -> Nothing + TeamAllowed -> Just [] + TeamNotAllowed -> Just [] addTeamRestriction :: (MakesValue ownDomain, MakesValue remoteDomain, MakesValue remoteTeam) => ownDomain -> remoteDomain -> remoteTeam -> Restriction -> App () addTeamRestriction ownDomain remoteDomain remoteTeam = \case @@ -181,8 +181,8 @@ federatedUserSearch test = do testFederatedUserSearchNonTeamSearcher :: HasCallStack => App () testFederatedUserSearchNonTeamSearcher = do startDynamicBackends [def, def] $ \[d1, d2] -> do - void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" "restrict_by_team") - void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" "allow_all") + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" (Just [])) + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" Nothing) u1 <- randomUser d1 def u2 <- randomUser d2 def {BrigI.team = True} @@ -212,8 +212,8 @@ testFederatedUserSearchNonTeamSearcher = do testFederatedSearchForNonTeamUser :: HasCallStack => App () testFederatedSearchForNonTeamUser = do startDynamicBackends [def, def] $ \[d1, d2] -> do - void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" "allow_all") - void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" "restrict_by_team") + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" Nothing) + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" (Just [])) u1 <- randomUser d1 def {BrigI.team = True} u2 <- randomUser d2 def diff --git a/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs b/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs index 95ed33c5558..250e49988ee 100644 --- a/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs +++ b/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs @@ -1,3 +1,5 @@ +{-# LANGUAGE TemplateHaskell #-} + -- This file is part of the Wire Server implementation. -- -- Copyright (C) 2022 Wire Swiss GmbH @@ -25,7 +27,7 @@ module Wire.API.Routes.FederationDomainConfig ) where -import Control.Lens ((?~)) +import Control.Lens (makePrisms, (?~)) import Data.Aeson (FromJSON, ToJSON) import Data.Domain (Domain) import Data.Id @@ -36,18 +38,23 @@ import Imports import Wire.API.User.Search (FederatedUserSearchPolicy) import Wire.Arbitrary (Arbitrary, GenericUniform (..)) -data FederationRestriction = FederationRestrictionAllowAll | FederationRestrictionByTeam +data FederationRestriction = FederationRestrictionAllowAll | FederationRestrictionByTeam [TeamId] deriving (Eq, Show, Generic, Ord) - deriving (ToJSON, FromJSON, S.ToSchema) via Schema FederationRestriction deriving (Arbitrary) via (GenericUniform FederationRestriction) +makePrisms ''FederationRestriction + +deriving via Schema FederationRestriction instance (S.ToSchema FederationRestriction) + +deriving via Schema FederationRestriction instance (FromJSON FederationRestriction) + +deriving via Schema FederationRestriction instance (ToJSON FederationRestriction) + instance ToSchema FederationRestriction where schema = - enum @Text "FederationRestriction" $ - mconcat - [ element "allow_all" FederationRestrictionAllowAll, - element "restrict_by_team" FederationRestrictionByTeam - ] + named "FederationRestriction" $ + tag _FederationRestrictionAllowAll null_ + <> tag _FederationRestrictionByTeam (array schema) -- | Everything we need to know about a remote instance in order to federate with it. Comes -- in `AllowedDomains` if `AllowStrategy` is `AllowDynamic`. If `AllowAll`, we still use this diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 73c721e00a4..178db62b840 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -33,7 +33,7 @@ import Brig.App import Brig.Data.Connection qualified as Data import Brig.Data.User qualified as Data import Brig.Effects.FederationConfigStore (FederationConfigStore) -import Brig.Effects.FederationConfigStore qualified as FCS +import Brig.Effects.FederationConfigStore qualified as FederationConfigStore import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.IO.Intra (notify) import Brig.Options @@ -59,7 +59,7 @@ import Servant (ServerT) import Servant.API import UnliftIO.Async (pooledForConcurrentlyN_) import Wire.API.Connection -import Wire.API.Federation.API.Brig +import Wire.API.Federation.API.Brig hiding (searchPolicy) import Wire.API.Federation.API.Common import Wire.API.Federation.Version import Wire.API.MLS.KeyPackage @@ -70,7 +70,7 @@ import Wire.API.Team.LegalHold (LegalholdProtectee (LegalholdPlusFederationNotIm import Wire.API.User (UserProfile) import Wire.API.User.Client import Wire.API.User.Client.Prekey -import Wire.API.User.Search +import Wire.API.User.Search hiding (searchPolicy) import Wire.API.UserMap (UserMap) import Wire.Sem.Concurrency @@ -257,8 +257,8 @@ onUserDeleted origDomain udcn = lift $ do -- | If domain is not configured fall back to `NoSearch` lookupSearchPolicy :: (Member FederationConfigStore r) => Domain -> (Handler r) FederatedUserSearchPolicy lookupSearchPolicy domain = do - mConfig <- lift $ liftSem $ FCS.getFederationConfig domain - pure $ maybe NoSearch FCS.searchPolicy mConfig + mConfig <- lift $ liftSem $ FederationConfigStore.getFederationConfig domain + pure $ maybe NoSearch searchPolicy mConfig -- | If domain is not configured fall back to `NoSearch` -- if a team is provided, check if the team is allowed to search @@ -267,8 +267,8 @@ lookupSearchPolicyWithTeam :: (Member FederationConfigStore r) => Domain -> Mayb lookupSearchPolicyWithTeam domain mSearcherTeamId = lift $ liftSem $ - FCS.getFederationConfig domain <&> \case + FederationConfigStore.getFederationConfig domain <&> \case Nothing -> NoSearch - Just (FCS.FederationDomainConfig _ sp FCS.FederationRestrictionAllowAll) -> sp - Just (FCS.FederationDomainConfig _ sp (FCS.FederationRestrictionByTeam teams)) -> + Just (FederationDomainConfig _ sp FederationRestrictionAllowAll) -> sp + Just (FederationDomainConfig _ sp (FederationRestrictionByTeam teams)) -> maybe NoSearch (\tid -> if tid `elem` teams then sp else NoSearch) $ mSearcherTeamId diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 84fea996015..405581f1a1d 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -286,14 +286,14 @@ getFederationRemotes = lift $ do -- See -- https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections, -- http://docs.wire.com/developer/developer/federation-design-aspects.html#configuring-remote-connections-dev-perspective - db <- liftSem $ FederationConfigStore.getFederationConfigs + remotes <- liftSem $ FederationConfigStore.getFederationConfigs ms :: Maybe FederationStrategy <- do cfg <- ask pure (setFederationStrategy (cfg ^. settings)) defFederationDomainConfigs & maybe id (\v cfg -> cfg {strategy = v}) ms - & (\cfg -> cfg {remotes = fmap FederationConfigStore.fromFederationDomainConfig db}) + & (\cfg -> cfg {remotes = remotes}) & pure updateFederationRemote :: (Member FederationConfigStore r) => Domain -> FederationDomainConfig -> (Handler r) () diff --git a/services/brig/src/Brig/Data/Instances.hs b/services/brig/src/Brig/Data/Instances.hs index 47f0ccf3373..34309315771 100644 --- a/services/brig/src/Brig/Data/Instances.hs +++ b/services/brig/src/Brig/Data/Instances.hs @@ -41,7 +41,6 @@ import Wire.API.Asset (AssetKey, assetKeyToText, nilAssetKey) import Wire.API.Connection (RelationWithHistory (..)) import Wire.API.MLS.CipherSuite import Wire.API.Properties -import Wire.API.Routes.FederationDomainConfig (FederationRestriction (..)) import Wire.API.User import Wire.API.User.Activation import Wire.API.User.Client @@ -302,16 +301,6 @@ instance Cql FederatedUserSearchPolicy where fromCql (CqlInt 2) = pure FullSearch fromCql n = Left $ "Unexpected SearchVisibilityInbound: " ++ show n -instance Cql FederationRestriction where - ctype = Tagged IntColumn - - toCql FederationRestrictionAllowAll = CqlInt 0 - toCql FederationRestrictionByTeam = CqlInt 1 - - fromCql (CqlInt 0) = pure FederationRestrictionAllowAll - fromCql (CqlInt 1) = pure FederationRestrictionByTeam - fromCql n = Left $ "Unexpected FederationRestriction: " ++ show n - instance Cql (Imports.Set BaseProtocolTag) where ctype = Tagged IntColumn diff --git a/services/brig/src/Brig/Effects/FederationConfigStore.hs b/services/brig/src/Brig/Effects/FederationConfigStore.hs index b84df100889..a30da2ed4e8 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore.hs @@ -6,32 +6,17 @@ import Data.Domain import Data.Id import Imports import Polysemy -import Wire.API.Routes.FederationDomainConfig qualified as API -import Wire.API.User.Search (FederatedUserSearchPolicy) - -data FederationRestriction = FederationRestrictionAllowAll | FederationRestrictionByTeam [TeamId] - deriving stock (Eq, Show, Ord) +import Wire.API.Routes.FederationDomainConfig data AddFederationRemoteResult = AddFederationRemoteSuccess | AddFederationRemoteMaxRemotesReached -data FederationDomainConfig = FederationDomainConfig - { domain :: Domain, - searchPolicy :: FederatedUserSearchPolicy, - restriction :: FederationRestriction - } - deriving stock (Show, Eq) - -fromFederationDomainConfig :: FederationDomainConfig -> API.FederationDomainConfig -fromFederationDomainConfig (FederationDomainConfig d p FederationRestrictionAllowAll) = API.FederationDomainConfig d p API.FederationRestrictionAllowAll -fromFederationDomainConfig (FederationDomainConfig d p (FederationRestrictionByTeam _)) = API.FederationDomainConfig d p API.FederationRestrictionByTeam - data FederationConfigStore m a where GetFederationConfig :: Domain -> FederationConfigStore m (Maybe FederationDomainConfig) GetFederationConfigs :: FederationConfigStore m [FederationDomainConfig] - AddFederationConfig :: API.FederationDomainConfig -> FederationConfigStore m AddFederationRemoteResult - UpdateFederationConfig :: API.FederationDomainConfig -> FederationConfigStore m Bool + AddFederationConfig :: FederationDomainConfig -> FederationConfigStore m AddFederationRemoteResult + UpdateFederationConfig :: FederationDomainConfig -> FederationConfigStore m Bool AddFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () RemoveFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () - GetFederationRemoteTeams :: Domain -> FederationConfigStore m [API.FederationRemoteTeam] + GetFederationRemoteTeams :: Domain -> FederationConfigStore m [FederationRemoteTeam] makeSem ''FederationConfigStore diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index 5b62e0e2f03..5caec5a33ce 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -33,7 +33,7 @@ import Data.Map qualified as Map import Database.CQL.Protocol (SerialConsistency (LocalSerialConsistency), serialConsistency) import Imports import Polysemy -import Wire.API.Routes.FederationDomainConfig qualified as API +import Wire.API.Routes.FederationDomainConfig import Wire.API.User.Search interpretFederationDomainConfig :: @@ -41,7 +41,7 @@ interpretFederationDomainConfig :: ( MonadClient m, Member (Embed m) r ) => - [API.FederationDomainConfig] -> + [FederationDomainConfig] -> Sem (FederationConfigStore ': r) a -> Sem r a interpretFederationDomainConfig cfgs = @@ -57,7 +57,7 @@ interpretFederationDomainConfig cfgs = -- | Compile config file list into a map indexed by domains. Use this to make sure the config -- file is consistent (ie., no two entries for the same domain). -remotesMapFromCfgFile :: (Monad m) => [API.FederationDomainConfig] -> m (Map Domain API.FederationDomainConfig) +remotesMapFromCfgFile :: (Monad m) => [FederationDomainConfig] -> m (Map Domain FederationDomainConfig) remotesMapFromCfgFile cfg = do let dict = [(cnf.domain, cnf) | cnf <- cfg] merge c c' = @@ -68,66 +68,92 @@ remotesMapFromCfgFile cfg = do -- | Return the config file list. Use this to make sure the config file is consistent (ie., -- no two entries for the same domain). Based on `remotesMapFromCfgFile`. -remotesListFromCfgFile :: Monad m => [API.FederationDomainConfig] -> m [API.FederationDomainConfig] +remotesListFromCfgFile :: Monad m => [FederationDomainConfig] -> m [FederationDomainConfig] remotesListFromCfgFile cfgs = Map.elems <$> remotesMapFromCfgFile cfgs -getFederationConfigs' :: forall m. (MonadClient m) => [API.FederationDomainConfig] -> m [FederationDomainConfig] -getFederationConfigs' cfgs = do - xs <- getFederationRemotes - ys <- remotesListFromCfgFile cfgs - configs <- - forM (xs <> ys) $ - \case - API.FederationDomainConfig d p API.FederationRestrictionAllowAll -> - pure $ FederationDomainConfig d p FederationRestrictionAllowAll - API.FederationDomainConfig d p API.FederationRestrictionByTeam -> - FederationDomainConfig d p . FederationRestrictionByTeam . fmap API.teamId <$> getFederationRemoteTeams' d - pure $ nub configs +getFederationConfigs' :: forall m. (MonadClient m) => [FederationDomainConfig] -> m [FederationDomainConfig] +getFederationConfigs' cfgs = + (<>) + <$> getFederationRemotes + <*> remotesListFromCfgFile cfgs maxKnownNodes :: Int maxKnownNodes = 10000 -getFederationConfig' :: MonadClient m => [API.FederationDomainConfig] -> Domain -> m (Maybe FederationDomainConfig) +getFederationConfig' :: MonadClient m => [FederationDomainConfig] -> Domain -> m (Maybe FederationDomainConfig) getFederationConfig' cfgs rDomain = do - let mFromCfgFile = (\c -> (c.searchPolicy, c.restriction)) <$> find ((== rDomain) . API.domain) cfgs + let mFromCfgFile = find ((== rDomain) . domain) cfgs mCnf <- retry x1 (query1 q (params LocalQuorum (Identity rDomain))) - teams <- fmap API.teamId <$> getFederationRemoteTeams' rDomain - pure $ - (mFromCfgFile <|> mCnf) <&> \case - (sp, API.FederationRestrictionAllowAll) -> FederationDomainConfig rDomain sp FederationRestrictionAllowAll - (sp, API.FederationRestrictionByTeam) -> FederationDomainConfig rDomain sp (FederationRestrictionByTeam teams) + case mCnf of + Nothing -> pure mFromCfgFile + Just (p, rInt) -> do + r <- toRestriction rDomain rInt + pure $ Just $ FederationDomainConfig rDomain p r where - q :: PrepQuery R (Identity Domain) (FederatedUserSearchPolicy, API.FederationRestriction) + q :: PrepQuery R (Identity Domain) (FederatedUserSearchPolicy, Int32) q = "SELECT search_policy, restriction FROM federation_remotes WHERE domain = ?" -getFederationRemotes :: forall m. MonadClient m => m [API.FederationDomainConfig] -getFederationRemotes = (\(d, p, r) -> API.FederationDomainConfig d p r) <$$> qry +getFederationRemotes :: forall m. MonadClient m => m [FederationDomainConfig] +getFederationRemotes = (\(d, p, r) -> FederationDomainConfig d p r) <$$> qry where - qry :: m [(Domain, FederatedUserSearchPolicy, API.FederationRestriction)] - qry = retry x1 . query get $ params LocalQuorum () + qry :: m [(Domain, FederatedUserSearchPolicy, FederationRestriction)] + qry = do + res <- retry x1 . query get $ params LocalQuorum () + forM res $ \(d, p, rInt) -> do + (d,p,) <$> toRestriction d rInt - get :: PrepQuery R () (Domain, FederatedUserSearchPolicy, API.FederationRestriction) + get :: PrepQuery R () (Domain, FederatedUserSearchPolicy, Int32) get = fromString $ "SELECT domain, search_policy, restriction FROM federation_remotes LIMIT " <> show maxKnownNodes -addFederationConfig' :: MonadClient m => API.FederationDomainConfig -> m AddFederationRemoteResult -addFederationConfig' (API.FederationDomainConfig rDomain searchPolicy restriction) = do +addFederationConfig' :: MonadClient m => FederationDomainConfig -> m AddFederationRemoteResult +addFederationConfig' (FederationDomainConfig rDomain searchPolicy restriction) = do l <- length <$> getFederationRemotes if l >= maxKnownNodes then pure AddFederationRemoteMaxRemotesReached - else AddFederationRemoteSuccess <$ retry x5 (write add (params LocalQuorum (rDomain, searchPolicy, restriction))) + else + AddFederationRemoteSuccess <$ do + retry x5 (write addConfig (params LocalQuorum (rDomain, searchPolicy, fromRestriction restriction))) + case restriction of + FederationRestrictionByTeam tids -> + retry x5 . batch . forM_ tids $ addPrepQuery addTeams . (rDomain,) + FederationRestrictionAllowAll -> pure () where - add :: PrepQuery W (Domain, FederatedUserSearchPolicy, API.FederationRestriction) () - add = "INSERT INTO federation_remotes (domain, search_policy, restriction) VALUES (?, ?, ?)" + addConfig :: PrepQuery W (Domain, FederatedUserSearchPolicy, Int32) () + addConfig = "INSERT INTO federation_remotes (domain, search_policy, restriction) VALUES (?, ?, ?)" -updateFederationConfig' :: MonadClient m => API.FederationDomainConfig -> m Bool -updateFederationConfig' (API.FederationDomainConfig rDomain searchPolicy restriction) = do - retry x1 (trans upd (params LocalQuorum (searchPolicy, restriction, rDomain)) {serialConsistency = Just LocalSerialConsistency}) >>= \case + addTeams :: PrepQuery W (Domain, TeamId) () + addTeams = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" + +updateFederationConfig' :: MonadClient m => FederationDomainConfig -> m Bool +updateFederationConfig' (FederationDomainConfig rDomain searchPolicy restriction) = do + let configParams = + ( params + LocalQuorum + (searchPolicy, fromRestriction restriction, rDomain) + ) + { serialConsistency = Just LocalSerialConsistency + } + r <- retry x1 (trans updateConfig configParams) + updateTeams + case r of [] -> pure False [_] -> pure True _ -> throwM $ ErrorCall "Primary key violation detected federation_remotes" where - upd :: PrepQuery W (FederatedUserSearchPolicy, API.FederationRestriction, Domain) x - upd = "UPDATE federation_remotes SET search_policy = ?, restriction = ? WHERE domain = ? IF EXISTS" + updateConfig :: PrepQuery W (FederatedUserSearchPolicy, Int32, Domain) x + updateConfig = "UPDATE federation_remotes SET search_policy = ?, restriction = ? WHERE domain = ? IF EXISTS" + updateTeams = retry x5 $ do + write dropTeams (params LocalQuorum (Identity rDomain)) + case restriction of + FederationRestrictionByTeam tids -> + batch . forM_ tids $ addPrepQuery insertTeam . (rDomain,) + FederationRestrictionAllowAll -> pure () + + dropTeams :: PrepQuery W (Identity Domain) () + dropTeams = "DELETE FROM federation_remote_teams WHERE domain = ?" + + insertTeam :: PrepQuery W (Domain, TeamId) () + insertTeam = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" addFederationRemoteTeam' :: MonadClient m => Domain -> TeamId -> m () addFederationRemoteTeam' rDomain tid = @@ -136,9 +162,9 @@ addFederationRemoteTeam' rDomain tid = add :: PrepQuery W (Domain, TeamId) () add = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" -getFederationRemoteTeams' :: MonadClient m => Domain -> m [API.FederationRemoteTeam] +getFederationRemoteTeams' :: MonadClient m => Domain -> m [FederationRemoteTeam] getFederationRemoteTeams' rDomain = do - fmap (API.FederationRemoteTeam . runIdentity) <$> retry x1 (query get (params LocalQuorum (Identity rDomain))) + fmap (FederationRemoteTeam . runIdentity) <$> retry x1 (query get (params LocalQuorum (Identity rDomain))) where get :: PrepQuery R (Identity Domain) (Identity TeamId) get = "SELECT team FROM federation_remote_teams WHERE domain = ?" @@ -149,3 +175,25 @@ removeFederationRemoteTeam' rDomain rteam = where delete :: PrepQuery W (Domain, TeamId) () delete = "DELETE FROM federation_remote_teams WHERE domain = ? AND team = ?" + +data RestrictionException = RestrictionException Int32 + +instance Show RestrictionException where + show (RestrictionException v) = + "Expected a RestrictionPolicy encoding, but found a value " <> show v + +instance Exception RestrictionException + +toRestriction :: MonadClient m => Domain -> Int32 -> m FederationRestriction +toRestriction _ 0 = pure FederationRestrictionAllowAll +toRestriction dom 1 = + fmap FederationRestrictionByTeam $ + runIdentity <$$> retry x1 (query getTeams (params LocalQuorum (Identity dom))) + where + getTeams :: PrepQuery R (Identity Domain) (Identity TeamId) + getTeams = fromString $ "SELECT team FROM federation_remote_teams WHERE domain = ?" +toRestriction _ v = throwM . RestrictionException $ v + +fromRestriction :: FederationRestriction -> Int32 +fromRestriction FederationRestrictionAllowAll = 0 +fromRestriction (FederationRestrictionByTeam _) = 1 diff --git a/services/brig/src/Brig/User/API/Search.hs b/services/brig/src/Brig/User/API/Search.hs index f39269c25f2..4d1c3b83598 100644 --- a/services/brig/src/Brig/User/API/Search.hs +++ b/services/brig/src/Brig/User/API/Search.hs @@ -54,6 +54,7 @@ import System.Logger.Class (val, (~~)) import System.Logger.Class qualified as Log import Wire.API.Federation.API.Brig qualified as FedBrig import Wire.API.Federation.API.Brig qualified as S +import Wire.API.Routes.FederationDomainConfig import Wire.API.Team.Permission qualified as Public import Wire.API.Team.SearchVisibility (TeamSearchVisibility (..)) import Wire.API.User.Search From 638956f82274a017b1b4268b79e69b5ae5578e10 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Thu, 23 Nov 2023 16:11:05 +0000 Subject: [PATCH 15/34] tagged union --- integration/test/API/BrigInternal.hs | 8 +++- .../Wire/API/Routes/FederationDomainConfig.hs | 31 ++++++++++++-- .../golden/Test/Wire/API/Golden/Manual.hs | 7 ++++ .../Golden/Manual/FederationDomainConfig.hs | 40 +++++++++++++++++++ .../testObject_FederationDomainConfig_1.json | 8 ++++ .../testObject_FederationDomainConfig_2.json | 8 ++++ .../testObject_FederationDomainConfig_3.json | 11 +++++ libs/wire-api/wire-api.cabal | 1 + 8 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/FederationDomainConfig.hs create mode 100644 libs/wire-api/test/golden/testObject_FederationDomainConfig_1.json create mode 100644 libs/wire-api/test/golden/testObject_FederationDomainConfig_2.json create mode 100644 libs/wire-api/test/golden/testObject_FederationDomainConfig_3.json diff --git a/integration/test/API/BrigInternal.hs b/integration/test/API/BrigInternal.hs index c78ee800ec2..7a4c871ad58 100644 --- a/integration/test/API/BrigInternal.hs +++ b/integration/test/API/BrigInternal.hs @@ -65,7 +65,13 @@ instance ToJSON FedConn where Aeson.object [ "domain" .= d, "search_policy" .= s, - "restriction" .= maybe Aeson.Null (\teams -> Aeson.Array (Vector.fromList (Aeson.String . cs <$> teams))) r + "restriction" + .= maybe + (Aeson.object ["tag" .= "allow_all", "value" .= Aeson.Null]) + ( \teams -> + Aeson.object ["tag" .= "restrict_by_team", "value" .= Aeson.Array (Vector.fromList (Aeson.String . cs <$> teams))] + ) + r ] instance MakesValue FedConn where diff --git a/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs b/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs index 250e49988ee..74257a99e66 100644 --- a/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs +++ b/libs/wire-api/src/Wire/API/Routes/FederationDomainConfig.hs @@ -28,6 +28,7 @@ module Wire.API.Routes.FederationDomainConfig where import Control.Lens (makePrisms, (?~)) +import Control.Lens.Tuple (_1) import Data.Aeson (FromJSON, ToJSON) import Data.Domain (Domain) import Data.Id @@ -44,17 +45,41 @@ data FederationRestriction = FederationRestrictionAllowAll | FederationRestricti makePrisms ''FederationRestriction +data FederationRestrictionTag = FederationRestrictionAllowAllTag | FederationRestrictionByTeamTag + deriving (Eq, Enum, Bounded) + +makePrisms ''FederationRestrictionTag + deriving via Schema FederationRestriction instance (S.ToSchema FederationRestriction) deriving via Schema FederationRestriction instance (FromJSON FederationRestriction) deriving via Schema FederationRestriction instance (ToJSON FederationRestriction) +tagSchema :: ValueSchema NamedSwaggerDoc FederationRestrictionTag +tagSchema = + enum @Text "FederationRestrictionTag" $ + mconcat [element "allow_all" FederationRestrictionAllowAllTag, element "restrict_by_team" FederationRestrictionByTeamTag] + instance ToSchema FederationRestriction where schema = - named "FederationRestriction" $ - tag _FederationRestrictionAllowAll null_ - <> tag _FederationRestrictionByTeam (array schema) + object "FederationRestriction" $ + fromTagged + <$> toTagged + .= bind + (fst .= field "tag" tagSchema) + (snd .= fieldOver _1 "value" untaggedSchema) + where + toTagged :: FederationRestriction -> (FederationRestrictionTag, FederationRestriction) + toTagged d@(FederationRestrictionAllowAll) = (FederationRestrictionAllowAllTag, d) + toTagged d@(FederationRestrictionByTeam _) = (FederationRestrictionByTeamTag, d) + + fromTagged :: (FederationRestrictionTag, FederationRestriction) -> FederationRestriction + fromTagged = snd + + untaggedSchema = dispatch $ \case + FederationRestrictionAllowAllTag -> tag _FederationRestrictionAllowAll null_ + FederationRestrictionByTeamTag -> tag _FederationRestrictionByTeam (array schema) -- | Everything we need to know about a remote instance in order to federate with it. Comes -- in `AllowedDomains` if `AllowStrategy` is `AllowDynamic`. If `AllowAll`, we still use this diff --git a/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual.hs b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual.hs index 6a8e528ad20..9d4122ea94f 100644 --- a/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual.hs +++ b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual.hs @@ -30,6 +30,7 @@ import Test.Wire.API.Golden.Manual.ConversationsResponse import Test.Wire.API.Golden.Manual.CreateGroupConversation import Test.Wire.API.Golden.Manual.CreateScimToken import Test.Wire.API.Golden.Manual.FeatureConfigEvent +import Test.Wire.API.Golden.Manual.FederationDomainConfig import Test.Wire.API.Golden.Manual.FederationStatus import Test.Wire.API.Golden.Manual.GetPaginatedConversationIds import Test.Wire.API.Golden.Manual.GroupId @@ -172,5 +173,11 @@ tests = testObjects [ (testObject_RemoteDomains_1, "testObject_RemoteDomains_1.json"), (testObject_RemoteDomains_2, "testObject_RemoteDomains_2.json") + ], + testGroup "FederationDomainConfig" $ + testObjects + [ (testObject_FederationDomainConfig_1, "testObject_FederationDomainConfig_1.json"), + (testObject_FederationDomainConfig_2, "testObject_FederationDomainConfig_2.json"), + (testObject_FederationDomainConfig_3, "testObject_FederationDomainConfig_3.json") ] ] diff --git a/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/FederationDomainConfig.hs b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/FederationDomainConfig.hs new file mode 100644 index 00000000000..6f06862589b --- /dev/null +++ b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/FederationDomainConfig.hs @@ -0,0 +1,40 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2022 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +module Test.Wire.API.Golden.Manual.FederationDomainConfig where + +import Data.Domain (Domain (Domain)) +import Data.Id +import Data.UUID qualified as UUID +import Imports +import Wire.API.Routes.FederationDomainConfig +import Wire.API.User.Search + +testObject_FederationDomainConfig_1 :: FederationDomainConfig +testObject_FederationDomainConfig_1 = + FederationDomainConfig (Domain "foo.example.com") FullSearch FederationRestrictionAllowAll + +testObject_FederationDomainConfig_2 :: FederationDomainConfig +testObject_FederationDomainConfig_2 = FederationDomainConfig (Domain "foo.example.com") FullSearch (FederationRestrictionByTeam []) + +testObject_FederationDomainConfig_3 :: FederationDomainConfig +testObject_FederationDomainConfig_3 = + FederationDomainConfig (Domain "foo.example.com") FullSearch $ + FederationRestrictionByTeam + [ Id (fromJust (UUID.fromString "0000304a-0000-0d5e-0000-3fac00003993")), + Id (fromJust (UUID.fromString "00003c90-0000-2207-0000-5249000018b1")) + ] diff --git a/libs/wire-api/test/golden/testObject_FederationDomainConfig_1.json b/libs/wire-api/test/golden/testObject_FederationDomainConfig_1.json new file mode 100644 index 00000000000..d4d1b4a5b63 --- /dev/null +++ b/libs/wire-api/test/golden/testObject_FederationDomainConfig_1.json @@ -0,0 +1,8 @@ +{ + "domain": "foo.example.com", + "restriction": { + "tag": "allow_all", + "value": null + }, + "search_policy": "full_search" +} diff --git a/libs/wire-api/test/golden/testObject_FederationDomainConfig_2.json b/libs/wire-api/test/golden/testObject_FederationDomainConfig_2.json new file mode 100644 index 00000000000..1c32964a751 --- /dev/null +++ b/libs/wire-api/test/golden/testObject_FederationDomainConfig_2.json @@ -0,0 +1,8 @@ +{ + "domain": "foo.example.com", + "restriction": { + "tag": "restrict_by_team", + "value": [] + }, + "search_policy": "full_search" +} diff --git a/libs/wire-api/test/golden/testObject_FederationDomainConfig_3.json b/libs/wire-api/test/golden/testObject_FederationDomainConfig_3.json new file mode 100644 index 00000000000..ace4103d980 --- /dev/null +++ b/libs/wire-api/test/golden/testObject_FederationDomainConfig_3.json @@ -0,0 +1,11 @@ +{ + "domain": "foo.example.com", + "restriction": { + "tag": "restrict_by_team", + "value": [ + "0000304a-0000-0d5e-0000-3fac00003993", + "00003c90-0000-2207-0000-5249000018b1" + ] + }, + "search_policy": "full_search" +} diff --git a/libs/wire-api/wire-api.cabal b/libs/wire-api/wire-api.cabal index 16b9c9b54eb..9f5ca3a68f0 100644 --- a/libs/wire-api/wire-api.cabal +++ b/libs/wire-api/wire-api.cabal @@ -571,6 +571,7 @@ test-suite wire-api-golden-tests Test.Wire.API.Golden.Manual.CreateGroupConversation Test.Wire.API.Golden.Manual.CreateScimToken Test.Wire.API.Golden.Manual.FeatureConfigEvent + Test.Wire.API.Golden.Manual.FederationDomainConfig Test.Wire.API.Golden.Manual.FederationStatus Test.Wire.API.Golden.Manual.GetPaginatedConversationIds Test.Wire.API.Golden.Manual.GroupId From d152b1c7a7143091bc521da6f60a4199f5052541 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Fri, 24 Nov 2023 09:55:53 +0000 Subject: [PATCH 16/34] move everything into interpreter --- services/brig/src/Brig/API/Federation.hs | 3 +- services/brig/src/Brig/API/Internal.hs | 95 ++++------------ .../brig/src/Brig/CanonicalInterpreter.hs | 4 +- .../src/Brig/Effects/FederationConfigStore.hs | 15 ++- .../FederationConfigStore/Cassandra.hs | 105 ++++++++++++------ services/brig/src/Brig/Options.hs | 1 + 6 files changed, 109 insertions(+), 114 deletions(-) diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 178db62b840..8d465ea461e 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -23,7 +23,6 @@ import Brig.API.Client qualified as API import Brig.API.Connection.Remote (performRemoteAction) import Brig.API.Error import Brig.API.Handler (Handler) -import Brig.API.Internal hiding (getMLSClients) import Brig.API.Internal qualified as Internal import Brig.API.MLS.CipherSuite import Brig.API.MLS.KeyPackages @@ -105,7 +104,7 @@ getFederationStatus _ request = do case setFederationStrategy (cfg ^. settings) of Just AllowAll -> pure $ NonConnectedBackends mempty _ -> do - fedDomains <- fromList . fmap (.domain) . (.remotes) <$> getFederationRemotes + fedDomains <- fromList . fmap (.domain) . (.remotes) <$> lift (liftSem $ FederationConfigStore.getFederationConfigs) pure $ NonConnectedBackends (request.domains \\ fedDomains) sendConnectionAction :: Domain -> NewConnectionRequest -> Handler r NewConnectionResponse diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 405581f1a1d..4a4d7770868 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -19,7 +19,6 @@ module Brig.API.Internal servantSitemap, BrigIRoutes.API, getMLSClients, - getFederationRemotes, ) where @@ -43,7 +42,7 @@ import Brig.Data.User qualified as Data import Brig.Effects.BlacklistPhonePrefixStore (BlacklistPhonePrefixStore) import Brig.Effects.BlacklistStore (BlacklistStore) import Brig.Effects.CodeStore (CodeStore) -import Brig.Effects.FederationConfigStore (AddFederationRemoteResult (..), FederationConfigStore) +import Brig.Effects.FederationConfigStore (AddFederationRemoteResult (..), FederationConfigStore, UpdateFederationResult (..)) import Brig.Effects.FederationConfigStore qualified as FederationConfigStore import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.PasswordResetStore (PasswordResetStore) @@ -63,7 +62,7 @@ import Brig.User.API.Search qualified as Search import Brig.User.EJPD qualified import Brig.User.Search.Index qualified as Index import Control.Error hiding (bool) -import Control.Lens (view, (^.)) +import Control.Lens (view) import Data.CommaSeparatedList import Data.Domain (Domain) import Data.Handle @@ -238,7 +237,6 @@ addFederationRemoteTeam domain rt = addFederationRemote :: (Member FederationConfigStore r) => FederationDomainConfig -> (Handler r) () addFederationRemote fedDomConf = do - assertNoDivergingDomainInConfigFiles fedDomConf result <- lift $ liftSem $ FederationConfigStore.addFederationConfig fedDomConf case result of AddFederationRemoteSuccess -> pure () @@ -246,80 +244,35 @@ addFederationRemote fedDomConf = do throwError . fedError . FederationUnexpectedError $ "Maximum number of remote backends reached. If you need to create more connections, \ \please contact wire.com." - --- | Compile config file list into a map indexed by domains. Use this to make sure the config --- file is consistent (ie., no two entries for the same domain). -remotesMapFromCfgFile :: AppT r (Map Domain FederationDomainConfig) -remotesMapFromCfgFile = do - cfg <- fmap (.federationDomainConfig) <$> asks (fromMaybe [] . setFederationDomainConfigs . view settings) - let dict = [(cnf.domain, cnf) | cnf <- cfg] - merge c c' = - if c == c' - then c - else error $ "error in config file: conflicting parameters on domain: " <> show (c, c') - pure $ Map.fromListWith merge dict - --- | If remote domain is registered in config file, the version that can be added to the --- database must be the same. -assertNoDivergingDomainInConfigFiles :: FederationDomainConfig -> (Handler r) () -assertNoDivergingDomainInConfigFiles fedComConf = do - cfg <- lift remotesMapFromCfgFile - let diverges = case Map.lookup (domain fedComConf) cfg of - Nothing -> False - Just fedComConf' -> fedComConf' /= fedComConf - when diverges $ do - throwError . fedError . FederationUnexpectedError $ - "keeping track of remote domains in the brig config file is deprecated, but as long as we \ - \do that, adding a domain with different settings than in the config file is nto allowed. want " - <> ( "Just " - <> cs (show fedComConf) - <> "or Nothing, " - ) - <> ( "got " - <> cs (show (Map.lookup (domain fedComConf) cfg)) - ) + AddFederationRemoteDivergingConfig cfg -> + throwError . fedError . FederationUnexpectedError $ + "keeping track of remote domains in the brig config file is deprecated, but as long as we \ + \do that, adding a domain with different settings than in the config file is nto allowed. want " + <> ( "Just " + <> cs (show fedDomConf) + <> "or Nothing, " + ) + <> ( "got " + <> cs (show (Map.lookup (domain fedDomConf) cfg)) + ) getFederationRemotes :: (Member FederationConfigStore r) => (Handler r) FederationDomainConfigs -getFederationRemotes = lift $ do - -- FUTUREWORK: we should solely rely on `db` in the future for remote domains; merging - -- remote domains from `cfg` is just for providing an easier, more robust migration path. - -- See - -- https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections, - -- http://docs.wire.com/developer/developer/federation-design-aspects.html#configuring-remote-connections-dev-perspective - remotes <- liftSem $ FederationConfigStore.getFederationConfigs - ms :: Maybe FederationStrategy <- do - cfg <- ask - pure (setFederationStrategy (cfg ^. settings)) - - defFederationDomainConfigs - & maybe id (\v cfg -> cfg {strategy = v}) ms - & (\cfg -> cfg {remotes = remotes}) - & pure +getFederationRemotes = lift $ liftSem $ FederationConfigStore.getFederationConfigs updateFederationRemote :: (Member FederationConfigStore r) => Domain -> FederationDomainConfig -> (Handler r) () updateFederationRemote dom fedcfg = do - assertDomainIsNotUpdated dom fedcfg - assertNoDomainsFromConfigFiles dom - (lift . liftSem . FederationConfigStore.updateFederationConfig $ fedcfg) >>= \case - True -> pure () - False -> + lift (liftSem (FederationConfigStore.updateFederationConfig dom fedcfg)) >>= \case + UpdateFederationSuccess -> pure () + UpdateFederationRemoteNotFound -> throwError . fedError . FederationUnexpectedError . cs $ "federation domain does not exist and cannot be updated: " <> show (dom, fedcfg) - -assertDomainIsNotUpdated :: Domain -> FederationDomainConfig -> (Handler r) () -assertDomainIsNotUpdated dom fedcfg = do - when (dom /= domain fedcfg) $ - throwError . fedError . FederationUnexpectedError . cs $ - "federation domain of a given peer cannot be changed from " <> show (domain fedcfg) <> " to " <> show dom <> "." - --- | FUTUREWORK: should go away in the future; see 'getFederationRemotes'. -assertNoDomainsFromConfigFiles :: Domain -> (Handler r) () -assertNoDomainsFromConfigFiles dom = do - cfg <- fmap (.federationDomainConfig) <$> asks (fromMaybe [] . setFederationDomainConfigs . view settings) - when (dom `elem` (domain <$> cfg)) $ do - throwError . fedError . FederationUnexpectedError $ - "keeping track of remote domains in the brig config file is deprecated, but as long as we \ - \do that, removing or updating items listed in the config file is not allowed." + UpdateFederationRemoteDivergingConfig -> + throwError . fedError . FederationUnexpectedError $ + "keeping track of remote domains in the brig config file is deprecated, but as long as we \ + \do that, removing or updating items listed in the config file is not allowed." + UpdateFederationRemoteDomainMismatch -> + throwError . fedError . FederationUnexpectedError . cs $ + "federation domain of a given peer cannot be changed from " <> show (domain fedcfg) <> " to " <> show dom <> "." -- | Responds with 'Nothing' if field is NULL in existing user or user does not exist. getAccountConferenceCallingConfig :: UserId -> (Handler r) (ApiFt.WithStatusNoLock ApiFt.ConferenceCallingConfig) diff --git a/services/brig/src/Brig/CanonicalInterpreter.hs b/services/brig/src/Brig/CanonicalInterpreter.hs index cd259218a4e..4a2474dba6a 100644 --- a/services/brig/src/Brig/CanonicalInterpreter.hs +++ b/services/brig/src/Brig/CanonicalInterpreter.hs @@ -21,7 +21,7 @@ import Brig.Effects.ServiceRPC (Service (Galley), ServiceRPC) import Brig.Effects.ServiceRPC.IO (interpretServiceRpcToRpc) import Brig.Effects.UserPendingActivationStore (UserPendingActivationStore) import Brig.Effects.UserPendingActivationStore.Cassandra (userPendingActivationStoreToCassandra) -import Brig.Options (ImplicitNoFederationRestriction (federationDomainConfig), federationDomainConfigs) +import Brig.Options (ImplicitNoFederationRestriction (federationDomainConfig), federationDomainConfigs, federationStrategy) import Brig.RPC (ParseException) import Cassandra qualified as Cas import Control.Lens ((^.)) @@ -83,7 +83,7 @@ runBrigToIO e (AppT ma) = do . interpretJwtTools . interpretPublicKeyBundle . interpretJwk - . interpretFederationDomainConfig (maybe [] (fmap (.federationDomainConfig)) (e ^. settings . federationDomainConfigs)) + . interpretFederationDomainConfig (e ^. settings . federationStrategy) (maybe [] (fmap (.federationDomainConfig)) (e ^. settings . federationDomainConfigs)) ) ) $ runReaderT ma e diff --git a/services/brig/src/Brig/Effects/FederationConfigStore.hs b/services/brig/src/Brig/Effects/FederationConfigStore.hs index a30da2ed4e8..dc51e753b16 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore.hs @@ -8,13 +8,22 @@ import Imports import Polysemy import Wire.API.Routes.FederationDomainConfig -data AddFederationRemoteResult = AddFederationRemoteSuccess | AddFederationRemoteMaxRemotesReached +data AddFederationRemoteResult + = AddFederationRemoteSuccess + | AddFederationRemoteMaxRemotesReached + | AddFederationRemoteDivergingConfig (Map Domain FederationDomainConfig) + +data UpdateFederationResult + = UpdateFederationSuccess + | UpdateFederationRemoteNotFound + | UpdateFederationRemoteDivergingConfig + | UpdateFederationRemoteDomainMismatch data FederationConfigStore m a where GetFederationConfig :: Domain -> FederationConfigStore m (Maybe FederationDomainConfig) - GetFederationConfigs :: FederationConfigStore m [FederationDomainConfig] + GetFederationConfigs :: FederationConfigStore m FederationDomainConfigs AddFederationConfig :: FederationDomainConfig -> FederationConfigStore m AddFederationRemoteResult - UpdateFederationConfig :: FederationDomainConfig -> FederationConfigStore m Bool + UpdateFederationConfig :: Domain -> FederationDomainConfig -> FederationConfigStore m UpdateFederationResult AddFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () RemoveFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () GetFederationRemoteTeams :: Domain -> FederationConfigStore m [FederationRemoteTeam] diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index 5caec5a33ce..2fecef89b39 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -41,16 +41,17 @@ interpretFederationDomainConfig :: ( MonadClient m, Member (Embed m) r ) => + Maybe FederationStrategy -> [FederationDomainConfig] -> Sem (FederationConfigStore ': r) a -> Sem r a -interpretFederationDomainConfig cfgs = +interpretFederationDomainConfig mFedStrategy cfgs = interpret $ embed @m . \case GetFederationConfig d -> getFederationConfig' cfgs d - GetFederationConfigs -> getFederationConfigs' cfgs - AddFederationConfig cnf -> addFederationConfig' cnf - UpdateFederationConfig cnf -> updateFederationConfig' cnf + GetFederationConfigs -> getFederationConfigs' mFedStrategy cfgs + AddFederationConfig cnf -> addFederationConfig' cfgs cnf + UpdateFederationConfig d cnf -> updateFederationConfig' cfgs d cnf AddFederationRemoteTeam d t -> addFederationRemoteTeam' d t RemoveFederationRemoteTeam d t -> removeFederationRemoteTeam' d t GetFederationRemoteTeams d -> getFederationRemoteTeams' d @@ -71,11 +72,22 @@ remotesMapFromCfgFile cfg = do remotesListFromCfgFile :: Monad m => [FederationDomainConfig] -> m [FederationDomainConfig] remotesListFromCfgFile cfgs = Map.elems <$> remotesMapFromCfgFile cfgs -getFederationConfigs' :: forall m. (MonadClient m) => [FederationDomainConfig] -> m [FederationDomainConfig] -getFederationConfigs' cfgs = - (<>) - <$> getFederationRemotes - <*> remotesListFromCfgFile cfgs +getFederationConfigs' :: forall m. (MonadClient m) => Maybe FederationStrategy -> [FederationDomainConfig] -> m FederationDomainConfigs +getFederationConfigs' mFedStrategy cfgs = do + -- FUTUREWORK: we should solely rely on `db` in the future for remote domains; merging + -- remote domains from `cfg` is just for providing an easier, more robust migration path. + -- See + -- https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections, + -- http://docs.wire.com/developer/developer/federation-design-aspects.html#configuring-remote-connections-dev-perspective + remotes <- + (<>) + <$> getFederationRemotes + <*> remotesListFromCfgFile cfgs + + defFederationDomainConfigs + & maybe id (\v cfg -> cfg {strategy = v}) mFedStrategy + & (\cfg -> cfg {remotes = remotes}) + & pure maxKnownNodes :: Int maxKnownNodes = 10000 @@ -105,43 +117,64 @@ getFederationRemotes = (\(d, p, r) -> FederationDomainConfig d p r) <$$> qry get :: PrepQuery R () (Domain, FederatedUserSearchPolicy, Int32) get = fromString $ "SELECT domain, search_policy, restriction FROM federation_remotes LIMIT " <> show maxKnownNodes -addFederationConfig' :: MonadClient m => FederationDomainConfig -> m AddFederationRemoteResult -addFederationConfig' (FederationDomainConfig rDomain searchPolicy restriction) = do - l <- length <$> getFederationRemotes - if l >= maxKnownNodes - then pure AddFederationRemoteMaxRemotesReached - else - AddFederationRemoteSuccess <$ do - retry x5 (write addConfig (params LocalQuorum (rDomain, searchPolicy, fromRestriction restriction))) - case restriction of - FederationRestrictionByTeam tids -> - retry x5 . batch . forM_ tids $ addPrepQuery addTeams . (rDomain,) - FederationRestrictionAllowAll -> pure () +addFederationConfig' :: MonadClient m => [FederationDomainConfig] -> FederationDomainConfig -> m AddFederationRemoteResult +addFederationConfig' cfgs (FederationDomainConfig rDomain searchPolicy restriction) = do + cfg <- remotesMapFromCfgFile cfgs + conflict <- domainExistsInConfig cfg (FederationDomainConfig rDomain searchPolicy restriction) + if conflict + then pure $ AddFederationRemoteDivergingConfig cfg + else do + l <- length <$> getFederationRemotes + if l >= maxKnownNodes + then pure AddFederationRemoteMaxRemotesReached + else + AddFederationRemoteSuccess <$ do + retry x5 (write addConfig (params LocalQuorum (rDomain, searchPolicy, fromRestriction restriction))) + case restriction of + FederationRestrictionByTeam tids -> + retry x5 . batch . forM_ tids $ addPrepQuery addTeams . (rDomain,) + FederationRestrictionAllowAll -> pure () where + -- If remote domain is registered in config file, the version that can be added to the + -- database must be the same. + domainExistsInConfig :: (Monad m) => (Map Domain FederationDomainConfig) -> FederationDomainConfig -> m Bool + domainExistsInConfig cfg fedDomConf = do + pure $ case Map.lookup (domain fedDomConf) cfg of + Nothing -> False + Just fedDomConf' -> fedDomConf' /= fedDomConf + addConfig :: PrepQuery W (Domain, FederatedUserSearchPolicy, Int32) () addConfig = "INSERT INTO federation_remotes (domain, search_policy, restriction) VALUES (?, ?, ?)" addTeams :: PrepQuery W (Domain, TeamId) () addTeams = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" -updateFederationConfig' :: MonadClient m => FederationDomainConfig -> m Bool -updateFederationConfig' (FederationDomainConfig rDomain searchPolicy restriction) = do - let configParams = - ( params - LocalQuorum - (searchPolicy, fromRestriction restriction, rDomain) - ) - { serialConsistency = Just LocalSerialConsistency - } - r <- retry x1 (trans updateConfig configParams) - updateTeams - case r of - [] -> pure False - [_] -> pure True - _ -> throwM $ ErrorCall "Primary key violation detected federation_remotes" +updateFederationConfig' :: MonadClient m => [FederationDomainConfig] -> Domain -> FederationDomainConfig -> m UpdateFederationResult +updateFederationConfig' cfgs dom (FederationDomainConfig rDomain searchPolicy restriction) = do + if dom /= rDomain + then pure UpdateFederationRemoteDomainMismatch + else + if dom `elem` (domain <$> cfgs) + then pure UpdateFederationRemoteDivergingConfig + else do + let configParams = + ( params + LocalQuorum + (searchPolicy, fromRestriction restriction, rDomain) + ) + { serialConsistency = Just LocalSerialConsistency + } + r <- retry x1 (trans updateConfig configParams) + updateTeams + case r of + [] -> pure UpdateFederationRemoteNotFound + [_] -> pure UpdateFederationSuccess + _ -> throwM $ ErrorCall "Primary key violation detected federation_remotes" where updateConfig :: PrepQuery W (FederatedUserSearchPolicy, Int32, Domain) x updateConfig = "UPDATE federation_remotes SET search_policy = ?, restriction = ? WHERE domain = ? IF EXISTS" + + updateTeams :: MonadClient m => m () updateTeams = retry x5 $ do write dropTeams (params LocalQuorum (Identity rDomain)) case restriction of diff --git a/services/brig/src/Brig/Options.hs b/services/brig/src/Brig/Options.hs index 7f70cb1be31..dac8446d2ac 100644 --- a/services/brig/src/Brig/Options.hs +++ b/services/brig/src/Brig/Options.hs @@ -928,6 +928,7 @@ Lens.makeLensesFor ("setSftStaticUrl", "sftStaticUrl"), ("setSftListAllServers", "sftListAllServers"), ("setFederationDomainConfigs", "federationDomainConfigs"), + ("setFederationStrategy", "federationStrategy"), ("setEnableDevelopmentVersions", "enableDevelopmentVersions"), ("setRestrictUserCreation", "restrictUserCreation"), ("setEnableMLS", "enableMLS"), From 0829f1ef2f693f26a812a67177625282aa326dca Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Fri, 24 Nov 2023 10:53:10 +0000 Subject: [PATCH 17/34] improve add remote team --- integration/test/API/BrigInternal.hs | 8 +++++-- integration/test/Test/Brig.hs | 9 +++++++- services/brig/src/Brig/API/Internal.hs | 21 ++++++++++++------- .../src/Brig/Effects/FederationConfigStore.hs | 7 ++++++- .../FederationConfigStore/Cassandra.hs | 16 ++++++++++---- 5 files changed, 46 insertions(+), 15 deletions(-) diff --git a/integration/test/API/BrigInternal.hs b/integration/test/API/BrigInternal.hs index 7a4c871ad58..e8b5cd4c57c 100644 --- a/integration/test/API/BrigInternal.hs +++ b/integration/test/API/BrigInternal.hs @@ -167,11 +167,15 @@ connectWithRemoteUser userFrom userTo = do addFederationRemoteTeam :: (HasCallStack, MakesValue domain, MakesValue remoteDomain, MakesValue team) => domain -> remoteDomain -> team -> App () addFederationRemoteTeam domain remoteDomain team = do + res <- addFederationRemoteTeam' domain remoteDomain team + res.status `shouldMatchInt` 200 + +addFederationRemoteTeam' :: (HasCallStack, MakesValue domain, MakesValue remoteDomain, MakesValue team) => domain -> remoteDomain -> team -> App Response +addFederationRemoteTeam' domain remoteDomain team = do d <- asString remoteDomain t <- make team req <- baseRequest domain Brig Unversioned $ joinHttpPath ["i", "federation", "remotes", d, "teams"] - res <- submit "POST" (req & addJSONObject ["team_id" .= t]) - res.status `shouldMatchInt` 200 + submit "POST" (req & addJSONObject ["team_id" .= t]) getFederationRemoteTeams :: (HasCallStack, MakesValue domain, MakesValue remoteDomain) => domain -> remoteDomain -> App Response getFederationRemoteTeams domain remoteDomain = do diff --git a/integration/test/Test/Brig.hs b/integration/test/Test/Brig.hs index b1256750446..f35fbd18568 100644 --- a/integration/test/Test/Brig.hs +++ b/integration/test/Test/Brig.hs @@ -2,6 +2,7 @@ module Test.Brig where import API.Brig qualified as BrigP import API.BrigInternal qualified as BrigI +import API.Common (randomName) import Data.Aeson.Types hiding ((.=)) import Data.Set qualified as Set import Data.String.Conversions @@ -130,7 +131,13 @@ testCrudFederationRemoteTeams :: HasCallStack => App () testCrudFederationRemoteTeams = do (_, tid, _) <- createTeam OwnDomain 1 (_, tid2, _) <- createTeam OwnDomain 1 - let rd = "some-remote-domain.wire.com" + rd <- (\n -> n <> ".wire.com") <$> randomName + bindResponse (BrigI.addFederationRemoteTeam' OwnDomain rd tid) $ \resp -> do + resp.status `shouldMatchInt` 533 + void $ BrigI.createFedConn OwnDomain $ BrigI.FedConn rd "full_search" Nothing + bindResponse (BrigI.addFederationRemoteTeam' OwnDomain rd tid) $ \resp -> do + resp.status `shouldMatchInt` 533 + void $ BrigI.updateFedConn OwnDomain rd $ BrigI.FedConn rd "full_search" (Just []) bindResponse (BrigI.getFederationRemoteTeams OwnDomain rd) $ \resp -> do resp.status `shouldMatchInt` 200 checkAbsence resp [tid, tid2] diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 4a4d7770868..9e54e500822 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -42,7 +42,7 @@ import Brig.Data.User qualified as Data import Brig.Effects.BlacklistPhonePrefixStore (BlacklistPhonePrefixStore) import Brig.Effects.BlacklistStore (BlacklistStore) import Brig.Effects.CodeStore (CodeStore) -import Brig.Effects.FederationConfigStore (AddFederationRemoteResult (..), FederationConfigStore, UpdateFederationResult (..)) +import Brig.Effects.FederationConfigStore (AddFederationRemoteResult (..), AddFederationRemoteTeamResult (..), FederationConfigStore, UpdateFederationResult (..)) import Brig.Effects.FederationConfigStore qualified as FederationConfigStore import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.PasswordResetStore (PasswordResetStore) @@ -233,12 +233,22 @@ getFederationRemoteTeams domain = addFederationRemoteTeam :: (Member FederationConfigStore r) => Domain -> FederationRemoteTeam -> (Handler r) () addFederationRemoteTeam domain rt = - lift $ liftSem $ FederationConfigStore.addFederationRemoteTeam domain rt.teamId + lift (liftSem $ FederationConfigStore.addFederationRemoteTeam domain rt.teamId) >>= \case + AddFederationRemoteTeamSuccess -> pure () + AddFederationRemoteTeamDomainNotFound -> + throwError . fedError . FederationUnexpectedError $ + "Federation domain does not exist. Please add it first." + AddFederationRemoteTeamRestrictionAllowAll -> + throwError . fedError . FederationUnexpectedError $ + "Federation is not configured to be restricted by teams. Therefore adding a team to a \ + \remote domain is not allowed." + +getFederationRemotes :: (Member FederationConfigStore r) => (Handler r) FederationDomainConfigs +getFederationRemotes = lift $ liftSem $ FederationConfigStore.getFederationConfigs addFederationRemote :: (Member FederationConfigStore r) => FederationDomainConfig -> (Handler r) () addFederationRemote fedDomConf = do - result <- lift $ liftSem $ FederationConfigStore.addFederationConfig fedDomConf - case result of + lift (liftSem $ FederationConfigStore.addFederationConfig fedDomConf) >>= \case AddFederationRemoteSuccess -> pure () AddFederationRemoteMaxRemotesReached -> throwError . fedError . FederationUnexpectedError $ @@ -256,9 +266,6 @@ addFederationRemote fedDomConf = do <> cs (show (Map.lookup (domain fedDomConf) cfg)) ) -getFederationRemotes :: (Member FederationConfigStore r) => (Handler r) FederationDomainConfigs -getFederationRemotes = lift $ liftSem $ FederationConfigStore.getFederationConfigs - updateFederationRemote :: (Member FederationConfigStore r) => Domain -> FederationDomainConfig -> (Handler r) () updateFederationRemote dom fedcfg = do lift (liftSem (FederationConfigStore.updateFederationConfig dom fedcfg)) >>= \case diff --git a/services/brig/src/Brig/Effects/FederationConfigStore.hs b/services/brig/src/Brig/Effects/FederationConfigStore.hs index dc51e753b16..2742c7d43b6 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore.hs @@ -19,12 +19,17 @@ data UpdateFederationResult | UpdateFederationRemoteDivergingConfig | UpdateFederationRemoteDomainMismatch +data AddFederationRemoteTeamResult + = AddFederationRemoteTeamSuccess + | AddFederationRemoteTeamDomainNotFound + | AddFederationRemoteTeamRestrictionAllowAll + data FederationConfigStore m a where GetFederationConfig :: Domain -> FederationConfigStore m (Maybe FederationDomainConfig) GetFederationConfigs :: FederationConfigStore m FederationDomainConfigs AddFederationConfig :: FederationDomainConfig -> FederationConfigStore m AddFederationRemoteResult UpdateFederationConfig :: Domain -> FederationDomainConfig -> FederationConfigStore m UpdateFederationResult - AddFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () + AddFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m AddFederationRemoteTeamResult RemoveFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () GetFederationRemoteTeams :: Domain -> FederationConfigStore m [FederationRemoteTeam] diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index 2fecef89b39..2406a933c83 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -52,7 +52,7 @@ interpretFederationDomainConfig mFedStrategy cfgs = GetFederationConfigs -> getFederationConfigs' mFedStrategy cfgs AddFederationConfig cnf -> addFederationConfig' cfgs cnf UpdateFederationConfig d cnf -> updateFederationConfig' cfgs d cnf - AddFederationRemoteTeam d t -> addFederationRemoteTeam' d t + AddFederationRemoteTeam d t -> addFederationRemoteTeam' cfgs d t RemoveFederationRemoteTeam d t -> removeFederationRemoteTeam' d t GetFederationRemoteTeams d -> getFederationRemoteTeams' d @@ -188,9 +188,17 @@ updateFederationConfig' cfgs dom (FederationDomainConfig rDomain searchPolicy re insertTeam :: PrepQuery W (Domain, TeamId) () insertTeam = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" -addFederationRemoteTeam' :: MonadClient m => Domain -> TeamId -> m () -addFederationRemoteTeam' rDomain tid = - retry x1 $ write add (params LocalQuorum (rDomain, tid)) +addFederationRemoteTeam' :: MonadClient m => [FederationDomainConfig] -> Domain -> TeamId -> m AddFederationRemoteTeamResult +addFederationRemoteTeam' cfgs rDomain tid = do + mDom <- getFederationConfig' cfgs rDomain + case mDom of + Nothing -> + pure AddFederationRemoteTeamDomainNotFound + Just (FederationDomainConfig _ _ FederationRestrictionAllowAll) -> + pure AddFederationRemoteTeamRestrictionAllowAll + Just _ -> do + retry x1 $ write add (params LocalQuorum (rDomain, tid)) + pure AddFederationRemoteTeamSuccess where add :: PrepQuery W (Domain, TeamId) () add = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" From 95dd15759875e3235e649cf926dc4ab3e11ef7fb Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Fri, 24 Nov 2023 13:36:06 +0000 Subject: [PATCH 18/34] handle empty list, optimize integration tests --- integration/test/Test/Search.hs | 129 ++++++++---------- services/brig/src/Brig/API/Federation.hs | 3 + .../brig/src/Brig/User/Search/SearchIndex.hs | 9 ++ 3 files changed, 68 insertions(+), 73 deletions(-) diff --git a/integration/test/Test/Search.hs b/integration/test/Test/Search.hs index b8c293f2966..d23f0e996e1 100644 --- a/integration/test/Test/Search.hs +++ b/integration/test/Test/Search.hs @@ -78,90 +78,73 @@ data FedUserSearchTestCase = FedUserSearchTestCase } deriving (Eq, Ord, Show) -testFederatedUserSearchNoSearch :: HasCallStack => App () -testFederatedUserSearchNoSearch = do +testFederatedUserSearch :: HasCallStack => App () +testFederatedUserSearch = do let testCases = - [ FedUserSearchTestCase "no_search" AllowAll AllowAll False False, - FedUserSearchTestCase "no_search" TeamAllowed TeamAllowed False False - ] - forM_ testCases federatedUserSearch - -testFederatedUserSearchExactHandleSearchAllowAll :: HasCallStack => App () -testFederatedUserSearchExactHandleSearchAllowAll = do - let testCases = - [ FedUserSearchTestCase "exact_handle_search" AllowAll AllowAll True False, + [ -- no search + FedUserSearchTestCase "no_search" AllowAll AllowAll False False, + FedUserSearchTestCase "no_search" TeamAllowed TeamAllowed False False, + -- exact handle search allow all/team allowed + FedUserSearchTestCase "exact_handle_search" AllowAll AllowAll True False, FedUserSearchTestCase "exact_handle_search" TeamAllowed TeamAllowed True False, FedUserSearchTestCase "exact_handle_search" AllowAll TeamAllowed True False, - FedUserSearchTestCase "exact_handle_search" TeamAllowed AllowAll True False - ] - forM_ testCases federatedUserSearch - -testFederatedUserSearchExactHandleSearchTeamNotAllowed :: HasCallStack => App () -testFederatedUserSearchExactHandleSearchTeamNotAllowed = do - let testCases = - [ FedUserSearchTestCase "exact_handle_search" TeamNotAllowed AllowAll False False, - FedUserSearchTestCase "exact_handle_search" AllowAll TeamNotAllowed False False - ] - forM_ testCases federatedUserSearch - -testFederatedUserSearchFullSearchAllowAll :: HasCallStack => App () -testFederatedUserSearchFullSearchAllowAll = do - let testCases = - [ FedUserSearchTestCase "full_search" AllowAll AllowAll True True, + FedUserSearchTestCase "exact_handle_search" TeamAllowed AllowAll True False, + -- exact handle search team not allowed + FedUserSearchTestCase "exact_handle_search" TeamNotAllowed AllowAll False False, + FedUserSearchTestCase "exact_handle_search" AllowAll TeamNotAllowed False False, + -- full search allow all/team allowed + FedUserSearchTestCase "full_search" AllowAll AllowAll True True, FedUserSearchTestCase "full_search" TeamAllowed TeamAllowed True True, FedUserSearchTestCase "full_search" TeamAllowed AllowAll True True, - FedUserSearchTestCase "full_search" AllowAll TeamAllowed True True - ] - forM_ testCases federatedUserSearch - -testFederatedUserSearchFullSearchTeamNotAllowed :: HasCallStack => App () -testFederatedUserSearchFullSearchTeamNotAllowed = do - let testCases = - [ FedUserSearchTestCase "full_search" TeamNotAllowed AllowAll False False, + FedUserSearchTestCase "full_search" AllowAll TeamAllowed True True, + -- full search team not allowed + FedUserSearchTestCase "full_search" TeamNotAllowed AllowAll False False, FedUserSearchTestCase "full_search" AllowAll TeamNotAllowed False False ] - forM_ testCases federatedUserSearch - -federatedUserSearch :: HasCallStack => FedUserSearchTestCase -> App () -federatedUserSearch test = do startDynamicBackends [def, def] $ \[d1, d2] -> do - void $ BrigI.createFedConn d2 (BrigI.FedConn d1 test.searchPolicy (restriction test.restrictionD2D1)) - void $ BrigI.createFedConn d1 (BrigI.FedConn d2 test.searchPolicy (restriction test.restrictionD1D2)) + void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" Nothing) + void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" Nothing) + forM_ testCases (federatedUserSearch d1 d2) - u1 <- randomUser d1 def {BrigI.team = True} - teamU1 <- u1 %. "team" - u2 <- randomUser d2 def {BrigI.team = True} - uidD2 <- objId u2 - team2 <- u2 %. "team" - GalleyI.setTeamFeatureStatus d2 team2 "searchVisibilityInbound" "enabled" +federatedUserSearch :: HasCallStack => String -> String -> FedUserSearchTestCase -> App () +federatedUserSearch d1 d2 test = do + void $ BrigI.updateFedConn d2 d1 (BrigI.FedConn d1 test.searchPolicy (restriction test.restrictionD2D1)) + void $ BrigI.updateFedConn d1 d2 (BrigI.FedConn d2 test.searchPolicy (restriction test.restrictionD1D2)) - addTeamRestriction d1 d2 team2 test.restrictionD1D2 - addTeamRestriction d2 d1 teamU1 test.restrictionD2D1 + u1 <- randomUser d1 def {BrigI.team = True} + teamU1 <- u1 %. "team" + u2 <- randomUser d2 def {BrigI.team = True} + uidD2 <- objId u2 + team2 <- u2 %. "team" + GalleyI.setTeamFeatureStatus d2 team2 "searchVisibilityInbound" "enabled" - u2Handle <- API.randomHandle - bindResponse (BrigP.putHandle u2 u2Handle) $ assertSuccess - BrigI.refreshIndex d2 + addTeamRestriction d1 d2 team2 test.restrictionD1D2 + addTeamRestriction d2 d1 teamU1 test.restrictionD2D1 - bindResponse (BrigP.searchContacts u1 u2Handle d2) $ \resp -> do - resp.status `shouldMatchInt` 200 - docs <- resp.json %. "documents" >>= asList - case docs of - [] -> - when (test.exactHandleSearchExpectFound) $ assertFailure $ "Expected a non empty result, but got an empty one, for test case " <> show test - doc : _ -> - if test.exactHandleSearchExpectFound - then doc %. "id" `shouldMatch` uidD2 - else assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " <> show test + u2Handle <- API.randomHandle + bindResponse (BrigP.putHandle u2 u2Handle) $ assertSuccess + BrigI.refreshIndex d2 - bindResponse (BrigP.searchContacts u1 (u2 %. "name") d2) $ \resp -> do - resp.status `shouldMatchInt` 200 - docs <- resp.json %. "documents" >>= asList - case docs of - [] -> when (test.fullSearchExpectFound) $ assertFailure $ "Expected a non empty result, but got an empty one, for test case " <> show test - doc : _ -> - if test.fullSearchExpectFound - then doc %. "id" `shouldMatch` uidD2 - else assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " <> show test + bindResponse (BrigP.searchContacts u1 u2Handle d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> + when (test.exactHandleSearchExpectFound) $ assertFailure $ "Expected a non empty result, but got an empty one, for test case " <> show test + doc : _ -> + if test.exactHandleSearchExpectFound + then doc %. "id" `shouldMatch` uidD2 + else assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " <> show test + + bindResponse (BrigP.searchContacts u1 (u2 %. "name") d2) $ \resp -> do + resp.status `shouldMatchInt` 200 + docs <- resp.json %. "documents" >>= asList + case docs of + [] -> when (test.fullSearchExpectFound) $ assertFailure $ "Expected a non empty result, but got an empty one, for test case " <> show test + doc : _ -> + if test.fullSearchExpectFound + then doc %. "id" `shouldMatch` uidD2 + else assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " <> show test where restriction :: Restriction -> Maybe [String] restriction = \case @@ -209,8 +192,8 @@ testFederatedUserSearchNonTeamSearcher = do doc : _ -> assertFailure $ "Expected an empty result, but got " <> show doc <> " for test case " -testFederatedSearchForNonTeamUser :: HasCallStack => App () -testFederatedSearchForNonTeamUser = do +testFederatedUserSearchForNonTeamUser :: HasCallStack => App () +testFederatedUserSearchForNonTeamUser = do startDynamicBackends [def, def] $ \[d1, d2] -> do void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" Nothing) void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" (Just [])) diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 8d465ea461e..bfdd7a54707 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -188,6 +188,9 @@ searchUsers :: Domain -> SearchRequest -> ExceptT Error (AppT r) SearchResponse +searchUsers domain (SearchRequest _ mTeam (Just [])) = do + searchPolicy <- lookupSearchPolicyWithTeam domain mTeam + pure $ SearchResponse [] searchPolicy searchUsers domain (SearchRequest searchTerm mTeam mOnlyInTeams) = do searchPolicy <- lookupSearchPolicyWithTeam domain mTeam diff --git a/services/brig/src/Brig/User/Search/SearchIndex.hs b/services/brig/src/Brig/User/Search/SearchIndex.hs index 7419802c851..b70fcf3deb3 100644 --- a/services/brig/src/Brig/User/Search/SearchIndex.hs +++ b/services/brig/src/Brig/User/Search/SearchIndex.hs @@ -199,6 +199,15 @@ restrictSearchSpace (FederatedSearch Nothing) = matchTeamMembersSearchableByAllTeams ] } +restrictSearchSpace (FederatedSearch (Just [])) = + ES.QueryBoolQuery + boolQuery + { ES.boolQueryMustMatch = + [ -- if the list of allowed teams is empty, this is impossible to fulfill, and no results will be returned + -- this case should be handled earlier, so this is just a safety net + ES.TermQuery (ES.Term "team" "must not match any team") Nothing + ] + } restrictSearchSpace (FederatedSearch (Just teams)) = ES.QueryBoolQuery boolQuery From ff103e6b5df64573e839ffe09de673fc0066fdcf Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 27 Nov 2023 16:04:39 +0000 Subject: [PATCH 19/34] Haddocks --- .../src/Wire/API/Federation/API/Brig.hs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs b/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs index 01ae3dca416..94ecad6dba2 100644 --- a/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs +++ b/libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs @@ -44,7 +44,13 @@ import Wire.API.UserMap (UserMap) import Wire.API.Util.Aeson (CustomEncoded (..)) import Wire.Arbitrary (GenericUniform (..)) -data SearchRequest = SearchRequest {term :: Text, from :: Maybe TeamId, onlyInTeams :: Maybe [TeamId]} +data SearchRequest = SearchRequest + { term :: Text, + -- | The searcher's team ID, used to matched against the remote backend's team federation policy. + from :: Maybe TeamId, + -- | The remote teams that the calling backend is allowed to federate with. + onlyInTeams :: Maybe [TeamId] + } deriving (Show, Eq, Generic, Typeable) deriving (Arbitrary) via (GenericUniform SearchRequest) From dab866b70c4d2ac5929a9b3f4c9ca0f638c7adba Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 27 Nov 2023 17:16:45 +0100 Subject: [PATCH 20/34] Update integration/test/API/BrigInternal.hs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marko Dimjašević --- integration/test/API/BrigInternal.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration/test/API/BrigInternal.hs b/integration/test/API/BrigInternal.hs index e8b5cd4c57c..f628da8bbb3 100644 --- a/integration/test/API/BrigInternal.hs +++ b/integration/test/API/BrigInternal.hs @@ -167,8 +167,7 @@ connectWithRemoteUser userFrom userTo = do addFederationRemoteTeam :: (HasCallStack, MakesValue domain, MakesValue remoteDomain, MakesValue team) => domain -> remoteDomain -> team -> App () addFederationRemoteTeam domain remoteDomain team = do - res <- addFederationRemoteTeam' domain remoteDomain team - res.status `shouldMatchInt` 200 +void $ addFederationRemoteTeam' domain remoteDomain team >>= getBody 200 addFederationRemoteTeam' :: (HasCallStack, MakesValue domain, MakesValue remoteDomain, MakesValue team) => domain -> remoteDomain -> team -> App Response addFederationRemoteTeam' domain remoteDomain team = do From 8caee10be1741ee6cb9c8fef2245f2a0fd14804d Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 27 Nov 2023 16:10:00 +0000 Subject: [PATCH 21/34] golden test for federation restriction --- .../golden/Test/Wire/API/Golden/Manual.hs | 7 ++++ .../Golden/Manual/FederationRestriction.hs | 36 +++++++++++++++++++ .../testObject_FederationRestriction_1.json | 4 +++ .../testObject_FederationRestriction_2.json | 4 +++ .../testObject_FederationRestriction_3.json | 7 ++++ libs/wire-api/wire-api.cabal | 1 + 6 files changed, 59 insertions(+) create mode 100644 libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/FederationRestriction.hs create mode 100644 libs/wire-api/test/golden/testObject_FederationRestriction_1.json create mode 100644 libs/wire-api/test/golden/testObject_FederationRestriction_2.json create mode 100644 libs/wire-api/test/golden/testObject_FederationRestriction_3.json diff --git a/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual.hs b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual.hs index 9d4122ea94f..e67567078a0 100644 --- a/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual.hs +++ b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual.hs @@ -31,6 +31,7 @@ import Test.Wire.API.Golden.Manual.CreateGroupConversation import Test.Wire.API.Golden.Manual.CreateScimToken import Test.Wire.API.Golden.Manual.FeatureConfigEvent import Test.Wire.API.Golden.Manual.FederationDomainConfig +import Test.Wire.API.Golden.Manual.FederationRestriction import Test.Wire.API.Golden.Manual.FederationStatus import Test.Wire.API.Golden.Manual.GetPaginatedConversationIds import Test.Wire.API.Golden.Manual.GroupId @@ -179,5 +180,11 @@ tests = [ (testObject_FederationDomainConfig_1, "testObject_FederationDomainConfig_1.json"), (testObject_FederationDomainConfig_2, "testObject_FederationDomainConfig_2.json"), (testObject_FederationDomainConfig_3, "testObject_FederationDomainConfig_3.json") + ], + testGroup "FederationRestriction" $ + testObjects + [ (testObject_FederationRestriction_1, "testObject_FederationRestriction_1.json"), + (testObject_FederationRestriction_2, "testObject_FederationRestriction_2.json"), + (testObject_FederationRestriction_3, "testObject_FederationRestriction_3.json") ] ] diff --git a/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/FederationRestriction.hs b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/FederationRestriction.hs new file mode 100644 index 00000000000..315a7c44c21 --- /dev/null +++ b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/FederationRestriction.hs @@ -0,0 +1,36 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2022 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +module Test.Wire.API.Golden.Manual.FederationRestriction where + +import Data.Id +import Data.UUID qualified as UUID +import Imports +import Wire.API.Routes.FederationDomainConfig + +testObject_FederationRestriction_1 :: FederationRestriction +testObject_FederationRestriction_1 = FederationRestrictionAllowAll + +testObject_FederationRestriction_2 :: FederationRestriction +testObject_FederationRestriction_2 = FederationRestrictionByTeam [] + +testObject_FederationRestriction_3 :: FederationRestriction +testObject_FederationRestriction_3 = + FederationRestrictionByTeam + [ Id (fromJust (UUID.fromString "0000304a-0000-0d5e-0000-3fac00003993")), + Id (fromJust (UUID.fromString "00003c90-0000-2207-0000-5249000018b1")) + ] diff --git a/libs/wire-api/test/golden/testObject_FederationRestriction_1.json b/libs/wire-api/test/golden/testObject_FederationRestriction_1.json new file mode 100644 index 00000000000..c23bb04a454 --- /dev/null +++ b/libs/wire-api/test/golden/testObject_FederationRestriction_1.json @@ -0,0 +1,4 @@ +{ + "tag": "allow_all", + "value": null +} diff --git a/libs/wire-api/test/golden/testObject_FederationRestriction_2.json b/libs/wire-api/test/golden/testObject_FederationRestriction_2.json new file mode 100644 index 00000000000..2af921efe73 --- /dev/null +++ b/libs/wire-api/test/golden/testObject_FederationRestriction_2.json @@ -0,0 +1,4 @@ +{ + "tag": "restrict_by_team", + "value": [] +} diff --git a/libs/wire-api/test/golden/testObject_FederationRestriction_3.json b/libs/wire-api/test/golden/testObject_FederationRestriction_3.json new file mode 100644 index 00000000000..da09dfe7682 --- /dev/null +++ b/libs/wire-api/test/golden/testObject_FederationRestriction_3.json @@ -0,0 +1,7 @@ +{ + "tag": "restrict_by_team", + "value": [ + "0000304a-0000-0d5e-0000-3fac00003993", + "00003c90-0000-2207-0000-5249000018b1" + ] +} diff --git a/libs/wire-api/wire-api.cabal b/libs/wire-api/wire-api.cabal index 9f5ca3a68f0..be55353f40c 100644 --- a/libs/wire-api/wire-api.cabal +++ b/libs/wire-api/wire-api.cabal @@ -572,6 +572,7 @@ test-suite wire-api-golden-tests Test.Wire.API.Golden.Manual.CreateScimToken Test.Wire.API.Golden.Manual.FeatureConfigEvent Test.Wire.API.Golden.Manual.FederationDomainConfig + Test.Wire.API.Golden.Manual.FederationRestriction Test.Wire.API.Golden.Manual.FederationStatus Test.Wire.API.Golden.Manual.GetPaginatedConversationIds Test.Wire.API.Golden.Manual.GroupId From 5f29b773c34552ba5016570dc9773d2a4177baf5 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 27 Nov 2023 16:16:25 +0000 Subject: [PATCH 22/34] shorter code --- integration/test/API/BrigInternal.hs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/integration/test/API/BrigInternal.hs b/integration/test/API/BrigInternal.hs index f628da8bbb3..713b3e240e8 100644 --- a/integration/test/API/BrigInternal.hs +++ b/integration/test/API/BrigInternal.hs @@ -4,8 +4,6 @@ import API.Common import Data.Aeson qualified as Aeson import Data.Function import Data.Maybe -import Data.String.Conversions (cs) -import Data.Vector qualified as Vector import Testlib.Prelude data CreateUser = CreateUser @@ -69,7 +67,7 @@ instance ToJSON FedConn where .= maybe (Aeson.object ["tag" .= "allow_all", "value" .= Aeson.Null]) ( \teams -> - Aeson.object ["tag" .= "restrict_by_team", "value" .= Aeson.Array (Vector.fromList (Aeson.String . cs <$> teams))] + Aeson.object ["tag" .= "restrict_by_team", "value" .= Aeson.toJSON teams] ) r ] From b0b1e3a74bf1c21dc2695fe72bd36d1508fe3a9a Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 27 Nov 2023 16:18:49 +0000 Subject: [PATCH 23/34] fix syntax --- integration/test/API/BrigInternal.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/test/API/BrigInternal.hs b/integration/test/API/BrigInternal.hs index 713b3e240e8..28267ac3a5f 100644 --- a/integration/test/API/BrigInternal.hs +++ b/integration/test/API/BrigInternal.hs @@ -165,7 +165,7 @@ connectWithRemoteUser userFrom userTo = do addFederationRemoteTeam :: (HasCallStack, MakesValue domain, MakesValue remoteDomain, MakesValue team) => domain -> remoteDomain -> team -> App () addFederationRemoteTeam domain remoteDomain team = do -void $ addFederationRemoteTeam' domain remoteDomain team >>= getBody 200 + void $ addFederationRemoteTeam' domain remoteDomain team >>= getBody 200 addFederationRemoteTeam' :: (HasCallStack, MakesValue domain, MakesValue remoteDomain, MakesValue team) => domain -> remoteDomain -> team -> App Response addFederationRemoteTeam' domain remoteDomain team = do From 226c275bab6d4b606974b198b84b8f55c0c71e3e Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 27 Nov 2023 16:30:52 +0000 Subject: [PATCH 24/34] removed comment --- services/brig/src/Brig/API/Federation.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index bfdd7a54707..0d89c8accf1 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -127,7 +127,6 @@ getUserByHandle :: Handle -> ExceptT Error (AppT r) (Maybe UserProfile) getUserByHandle domain handle = do - -- TODO: should we do the team federation check here, too? searchPolicy <- lookupSearchPolicy domain let performHandleLookup = From 2032d7979b4a7855558612d88273a8cae33720cb Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 10:52:25 +0100 Subject: [PATCH 25/34] Update services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marko Dimjašević --- .../Brig/Effects/FederationConfigStore/Cassandra.hs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index 2406a933c83..bc8e2a811fd 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -93,14 +93,11 @@ maxKnownNodes :: Int maxKnownNodes = 10000 getFederationConfig' :: MonadClient m => [FederationDomainConfig] -> Domain -> m (Maybe FederationDomainConfig) -getFederationConfig' cfgs rDomain = do - let mFromCfgFile = find ((== rDomain) . domain) cfgs - mCnf <- retry x1 (query1 q (params LocalQuorum (Identity rDomain))) - case mCnf of - Nothing -> pure mFromCfgFile - Just (p, rInt) -> do - r <- toRestriction rDomain rInt - pure $ Just $ FederationDomainConfig rDomain p r +getFederationConfig' cfgs rDomain = case find ((== rDomain) . domain) cfgs of + Just cfg -> pure . Just $ cfg -- the configuration from the file has precedence + Nothing -> do + mCnf <- retry x1 (query1 q (params LocalQuorum (Identity rDomain))) + pure $ fmap (\(p, rInt) -> FederationDomainConfig rDomain p (toRestriction rDomain rInt)) mCnf where q :: PrepQuery R (Identity Domain) (FederatedUserSearchPolicy, Int32) q = "SELECT search_policy, restriction FROM federation_remotes WHERE domain = ?" From faa36891a4d678c5201c6cd04a229611b27b5c96 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 11:26:14 +0100 Subject: [PATCH 26/34] Update services/brig/src/Brig/API/Internal.hs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marko Dimjašević --- services/brig/src/Brig/API/Internal.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 9e54e500822..cf9d6013613 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -257,7 +257,7 @@ addFederationRemote fedDomConf = do AddFederationRemoteDivergingConfig cfg -> throwError . fedError . FederationUnexpectedError $ "keeping track of remote domains in the brig config file is deprecated, but as long as we \ - \do that, adding a domain with different settings than in the config file is nto allowed. want " + \do that, adding a domain with different settings than in the config file is not allowed. want " <> ( "Just " <> cs (show fedDomConf) <> "or Nothing, " From dc65ecf600bcaa8394ac0850c36da14690c934ff Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 09:48:05 +0000 Subject: [PATCH 27/34] removed test cases that are already covered --- integration/test/Test/Search.hs | 40 --------------------------------- 1 file changed, 40 deletions(-) diff --git a/integration/test/Test/Search.hs b/integration/test/Test/Search.hs index d23f0e996e1..1c8a88f343e 100644 --- a/integration/test/Test/Search.hs +++ b/integration/test/Test/Search.hs @@ -26,46 +26,6 @@ testSearchContactForExternalUsers = do -------------------------------------------------------------------------------- -- FEDERATION SEARCH -testRemoteUserSearch :: HasCallStack => App () -testRemoteUserSearch = do - startDynamicBackends [def, def] $ \[d1, d2] -> do - void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "full_search" Nothing) - void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "full_search" Nothing) - - u1 <- randomUser d1 def - u2 <- randomUser d2 def - BrigI.refreshIndex d2 - uidD2 <- objId u2 - - bindResponse (BrigP.searchContacts u1 (u2 %. "name") d2) $ \resp -> do - resp.status `shouldMatchInt` 200 - docs <- resp.json %. "documents" >>= asList - case docs of - [] -> assertFailure "Expected a non empty result, but got an empty one" - doc : _ -> doc %. "id" `shouldMatch` uidD2 - -testRemoteUserSearchExactHandle :: HasCallStack => App () -testRemoteUserSearchExactHandle = do - startDynamicBackends [def, def] $ \[d1, d2] -> do - void $ BrigI.createFedConn d2 (BrigI.FedConn d1 "exact_handle_search" Nothing) - void $ BrigI.createFedConn d1 (BrigI.FedConn d2 "exact_handle_search" Nothing) - - u1 <- randomUser d1 def - u2 <- randomUser d2 def - u2Handle <- API.randomHandle - bindResponse (BrigP.putHandle u2 u2Handle) $ assertSuccess - BrigI.refreshIndex d2 - - bindResponse (BrigP.searchContacts u1 u2Handle d2) $ \resp -> do - resp.status `shouldMatchInt` 200 - docs <- resp.json %. "documents" >>= asList - case docs of - [] -> assertFailure "Expected a non empty result, but got an empty one" - doc : _ -> objQid doc `shouldMatch` objQid u2 - --------------------------------------------------------------------------------- --- FEDERATION SEARCH WITH TEAM RESTRICTIONS - data Restriction = AllowAll | TeamAllowed | TeamNotAllowed deriving (Eq, Ord, Show) From ddafb7fdf9c2721a12b93270a7aa55340efbd767 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 10:11:16 +0000 Subject: [PATCH 28/34] clean up --- .../brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index bc8e2a811fd..3b67e905ab3 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -97,7 +97,9 @@ getFederationConfig' cfgs rDomain = case find ((== rDomain) . domain) cfgs of Just cfg -> pure . Just $ cfg -- the configuration from the file has precedence Nothing -> do mCnf <- retry x1 (query1 q (params LocalQuorum (Identity rDomain))) - pure $ fmap (\(p, rInt) -> FederationDomainConfig rDomain p (toRestriction rDomain rInt)) mCnf + case mCnf of + Just (p, r) -> Just . FederationDomainConfig rDomain p <$> toRestriction rDomain r + Nothing -> pure Nothing where q :: PrepQuery R (Identity Domain) (FederatedUserSearchPolicy, Int32) q = "SELECT search_policy, restriction FROM federation_remotes WHERE domain = ?" From 7757c3b10ab25a855208eb172e91bac0d26e1341 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 10:16:42 +0000 Subject: [PATCH 29/34] clean up --- services/brig/src/Brig/API/Internal.hs | 22 +++++----- .../src/Brig/Effects/FederationConfigStore.hs | 3 +- .../FederationConfigStore/Cassandra.hs | 41 +++++++++---------- 3 files changed, 32 insertions(+), 34 deletions(-) diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index cf9d6013613..5d6411c81d3 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -268,18 +268,20 @@ addFederationRemote fedDomConf = do updateFederationRemote :: (Member FederationConfigStore r) => Domain -> FederationDomainConfig -> (Handler r) () updateFederationRemote dom fedcfg = do - lift (liftSem (FederationConfigStore.updateFederationConfig dom fedcfg)) >>= \case - UpdateFederationSuccess -> pure () - UpdateFederationRemoteNotFound -> - throwError . fedError . FederationUnexpectedError . cs $ - "federation domain does not exist and cannot be updated: " <> show (dom, fedcfg) - UpdateFederationRemoteDivergingConfig -> - throwError . fedError . FederationUnexpectedError $ - "keeping track of remote domains in the brig config file is deprecated, but as long as we \ - \do that, removing or updating items listed in the config file is not allowed." - UpdateFederationRemoteDomainMismatch -> + if (dom /= fedcfg.domain) + then throwError . fedError . FederationUnexpectedError . cs $ "federation domain of a given peer cannot be changed from " <> show (domain fedcfg) <> " to " <> show dom <> "." + else + lift (liftSem (FederationConfigStore.updateFederationConfig fedcfg)) >>= \case + UpdateFederationSuccess -> pure () + UpdateFederationRemoteNotFound -> + throwError . fedError . FederationUnexpectedError . cs $ + "federation domain does not exist and cannot be updated: " <> show (dom, fedcfg) + UpdateFederationRemoteDivergingConfig -> + throwError . fedError . FederationUnexpectedError $ + "keeping track of remote domains in the brig config file is deprecated, but as long as we \ + \do that, removing or updating items listed in the config file is not allowed." -- | Responds with 'Nothing' if field is NULL in existing user or user does not exist. getAccountConferenceCallingConfig :: UserId -> (Handler r) (ApiFt.WithStatusNoLock ApiFt.ConferenceCallingConfig) diff --git a/services/brig/src/Brig/Effects/FederationConfigStore.hs b/services/brig/src/Brig/Effects/FederationConfigStore.hs index 2742c7d43b6..94c67b6112e 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore.hs @@ -17,7 +17,6 @@ data UpdateFederationResult = UpdateFederationSuccess | UpdateFederationRemoteNotFound | UpdateFederationRemoteDivergingConfig - | UpdateFederationRemoteDomainMismatch data AddFederationRemoteTeamResult = AddFederationRemoteTeamSuccess @@ -28,7 +27,7 @@ data FederationConfigStore m a where GetFederationConfig :: Domain -> FederationConfigStore m (Maybe FederationDomainConfig) GetFederationConfigs :: FederationConfigStore m FederationDomainConfigs AddFederationConfig :: FederationDomainConfig -> FederationConfigStore m AddFederationRemoteResult - UpdateFederationConfig :: Domain -> FederationDomainConfig -> FederationConfigStore m UpdateFederationResult + UpdateFederationConfig :: FederationDomainConfig -> FederationConfigStore m UpdateFederationResult AddFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m AddFederationRemoteTeamResult RemoveFederationRemoteTeam :: Domain -> TeamId -> FederationConfigStore m () GetFederationRemoteTeams :: Domain -> FederationConfigStore m [FederationRemoteTeam] diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index 3b67e905ab3..2a81a0d675a 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -51,7 +51,7 @@ interpretFederationDomainConfig mFedStrategy cfgs = GetFederationConfig d -> getFederationConfig' cfgs d GetFederationConfigs -> getFederationConfigs' mFedStrategy cfgs AddFederationConfig cnf -> addFederationConfig' cfgs cnf - UpdateFederationConfig d cnf -> updateFederationConfig' cfgs d cnf + UpdateFederationConfig cnf -> updateFederationConfig' cfgs cnf AddFederationRemoteTeam d t -> addFederationRemoteTeam' cfgs d t RemoveFederationRemoteTeam d t -> removeFederationRemoteTeam' d t GetFederationRemoteTeams d -> getFederationRemoteTeams' d @@ -148,27 +148,24 @@ addFederationConfig' cfgs (FederationDomainConfig rDomain searchPolicy restricti addTeams :: PrepQuery W (Domain, TeamId) () addTeams = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" -updateFederationConfig' :: MonadClient m => [FederationDomainConfig] -> Domain -> FederationDomainConfig -> m UpdateFederationResult -updateFederationConfig' cfgs dom (FederationDomainConfig rDomain searchPolicy restriction) = do - if dom /= rDomain - then pure UpdateFederationRemoteDomainMismatch - else - if dom `elem` (domain <$> cfgs) - then pure UpdateFederationRemoteDivergingConfig - else do - let configParams = - ( params - LocalQuorum - (searchPolicy, fromRestriction restriction, rDomain) - ) - { serialConsistency = Just LocalSerialConsistency - } - r <- retry x1 (trans updateConfig configParams) - updateTeams - case r of - [] -> pure UpdateFederationRemoteNotFound - [_] -> pure UpdateFederationSuccess - _ -> throwM $ ErrorCall "Primary key violation detected federation_remotes" +updateFederationConfig' :: MonadClient m => [FederationDomainConfig] -> FederationDomainConfig -> m UpdateFederationResult +updateFederationConfig' cfgs (FederationDomainConfig rDomain searchPolicy restriction) = do + if rDomain `elem` (domain <$> cfgs) + then pure UpdateFederationRemoteDivergingConfig + else do + let configParams = + ( params + LocalQuorum + (searchPolicy, fromRestriction restriction, rDomain) + ) + { serialConsistency = Just LocalSerialConsistency + } + r <- retry x1 (trans updateConfig configParams) + updateTeams + case r of + [] -> pure UpdateFederationRemoteNotFound + [_] -> pure UpdateFederationSuccess + _ -> throwM $ ErrorCall "Primary key violation detected federation_remotes" where updateConfig :: PrepQuery W (FederatedUserSearchPolicy, Int32, Domain) x updateConfig = "UPDATE federation_remotes SET search_policy = ?, restriction = ? WHERE domain = ? IF EXISTS" From 986e67ecca7a6e6cb00447e110f5df03a47e47cc Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 10:26:46 +0000 Subject: [PATCH 30/34] clean up --- services/brig/src/Brig/API/Federation.hs | 8 ++++---- services/brig/src/Brig/API/Internal.hs | 14 +++++++------- services/brig/src/Brig/User/API/Search.hs | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/services/brig/src/Brig/API/Federation.hs b/services/brig/src/Brig/API/Federation.hs index 0d89c8accf1..f956eef123c 100644 --- a/services/brig/src/Brig/API/Federation.hs +++ b/services/brig/src/Brig/API/Federation.hs @@ -32,7 +32,7 @@ import Brig.App import Brig.Data.Connection qualified as Data import Brig.Data.User qualified as Data import Brig.Effects.FederationConfigStore (FederationConfigStore) -import Brig.Effects.FederationConfigStore qualified as FederationConfigStore +import Brig.Effects.FederationConfigStore qualified as E import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.IO.Intra (notify) import Brig.Options @@ -104,7 +104,7 @@ getFederationStatus _ request = do case setFederationStrategy (cfg ^. settings) of Just AllowAll -> pure $ NonConnectedBackends mempty _ -> do - fedDomains <- fromList . fmap (.domain) . (.remotes) <$> lift (liftSem $ FederationConfigStore.getFederationConfigs) + fedDomains <- fromList . fmap (.domain) . (.remotes) <$> lift (liftSem $ E.getFederationConfigs) pure $ NonConnectedBackends (request.domains \\ fedDomains) sendConnectionAction :: Domain -> NewConnectionRequest -> Handler r NewConnectionResponse @@ -258,7 +258,7 @@ onUserDeleted origDomain udcn = lift $ do -- | If domain is not configured fall back to `NoSearch` lookupSearchPolicy :: (Member FederationConfigStore r) => Domain -> (Handler r) FederatedUserSearchPolicy lookupSearchPolicy domain = do - mConfig <- lift $ liftSem $ FederationConfigStore.getFederationConfig domain + mConfig <- lift $ liftSem $ E.getFederationConfig domain pure $ maybe NoSearch searchPolicy mConfig -- | If domain is not configured fall back to `NoSearch` @@ -268,7 +268,7 @@ lookupSearchPolicyWithTeam :: (Member FederationConfigStore r) => Domain -> Mayb lookupSearchPolicyWithTeam domain mSearcherTeamId = lift $ liftSem $ - FederationConfigStore.getFederationConfig domain <&> \case + E.getFederationConfig domain <&> \case Nothing -> NoSearch Just (FederationDomainConfig _ sp FederationRestrictionAllowAll) -> sp Just (FederationDomainConfig _ sp (FederationRestrictionByTeam teams)) -> diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 5d6411c81d3..8f84ba4b73d 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -43,7 +43,7 @@ import Brig.Effects.BlacklistPhonePrefixStore (BlacklistPhonePrefixStore) import Brig.Effects.BlacklistStore (BlacklistStore) import Brig.Effects.CodeStore (CodeStore) import Brig.Effects.FederationConfigStore (AddFederationRemoteResult (..), AddFederationRemoteTeamResult (..), FederationConfigStore, UpdateFederationResult (..)) -import Brig.Effects.FederationConfigStore qualified as FederationConfigStore +import Brig.Effects.FederationConfigStore qualified as E import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.PasswordResetStore (PasswordResetStore) import Brig.Effects.UserPendingActivationStore (UserPendingActivationStore) @@ -225,15 +225,15 @@ federationRemotesAPI = deleteFederationRemoteTeam :: (Member FederationConfigStore r) => Domain -> TeamId -> (Handler r) () deleteFederationRemoteTeam domain teamId = - lift $ liftSem $ FederationConfigStore.removeFederationRemoteTeam domain teamId + lift $ liftSem $ E.removeFederationRemoteTeam domain teamId getFederationRemoteTeams :: (Member FederationConfigStore r) => Domain -> (Handler r) [FederationRemoteTeam] getFederationRemoteTeams domain = - lift $ liftSem $ FederationConfigStore.getFederationRemoteTeams domain + lift $ liftSem $ E.getFederationRemoteTeams domain addFederationRemoteTeam :: (Member FederationConfigStore r) => Domain -> FederationRemoteTeam -> (Handler r) () addFederationRemoteTeam domain rt = - lift (liftSem $ FederationConfigStore.addFederationRemoteTeam domain rt.teamId) >>= \case + lift (liftSem $ E.addFederationRemoteTeam domain rt.teamId) >>= \case AddFederationRemoteTeamSuccess -> pure () AddFederationRemoteTeamDomainNotFound -> throwError . fedError . FederationUnexpectedError $ @@ -244,11 +244,11 @@ addFederationRemoteTeam domain rt = \remote domain is not allowed." getFederationRemotes :: (Member FederationConfigStore r) => (Handler r) FederationDomainConfigs -getFederationRemotes = lift $ liftSem $ FederationConfigStore.getFederationConfigs +getFederationRemotes = lift $ liftSem $ E.getFederationConfigs addFederationRemote :: (Member FederationConfigStore r) => FederationDomainConfig -> (Handler r) () addFederationRemote fedDomConf = do - lift (liftSem $ FederationConfigStore.addFederationConfig fedDomConf) >>= \case + lift (liftSem $ E.addFederationConfig fedDomConf) >>= \case AddFederationRemoteSuccess -> pure () AddFederationRemoteMaxRemotesReached -> throwError . fedError . FederationUnexpectedError $ @@ -273,7 +273,7 @@ updateFederationRemote dom fedcfg = do throwError . fedError . FederationUnexpectedError . cs $ "federation domain of a given peer cannot be changed from " <> show (domain fedcfg) <> " to " <> show dom <> "." else - lift (liftSem (FederationConfigStore.updateFederationConfig fedcfg)) >>= \case + lift (liftSem (E.updateFederationConfig fedcfg)) >>= \case UpdateFederationSuccess -> pure () UpdateFederationRemoteNotFound -> throwError . fedError . FederationUnexpectedError . cs $ diff --git a/services/brig/src/Brig/User/API/Search.hs b/services/brig/src/Brig/User/API/Search.hs index 4d1c3b83598..44a87a7dd50 100644 --- a/services/brig/src/Brig/User/API/Search.hs +++ b/services/brig/src/Brig/User/API/Search.hs @@ -29,7 +29,7 @@ import Brig.API.Handler import Brig.App import Brig.Data.User qualified as DB import Brig.Effects.FederationConfigStore -import Brig.Effects.FederationConfigStore qualified as FederationConfigStore +import Brig.Effects.FederationConfigStore qualified as E import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.GalleyProvider qualified as GalleyProvider import Brig.Federation.Client qualified as Federation @@ -90,7 +90,7 @@ searchRemotely domain mTid searchTerm = do msg (val "searchRemotely") ~~ field "domain" (show domain) ~~ field "searchTerm" searchTerm - mFedCnf <- lift $ liftSem $ FederationConfigStore.getFederationConfig domain + mFedCnf <- lift $ liftSem $ E.getFederationConfig domain let onlyInTeams = case restriction <$> mFedCnf of Just FederationRestrictionAllowAll -> Nothing Just (FederationRestrictionByTeam teams) -> Just teams From 8216def0d324e6613ca0b402bfc5c13f7954c547 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 11:10:53 +0000 Subject: [PATCH 31/34] added some comments --- integration/test/Test/Search.hs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/integration/test/Test/Search.hs b/integration/test/Test/Search.hs index 1c8a88f343e..99ffc440618 100644 --- a/integration/test/Test/Search.hs +++ b/integration/test/Test/Search.hs @@ -26,12 +26,15 @@ testSearchContactForExternalUsers = do -------------------------------------------------------------------------------- -- FEDERATION SEARCH +-- | Enumeration of the possible restrictions that can be applied to a federated user search data Restriction = AllowAll | TeamAllowed | TeamNotAllowed deriving (Eq, Ord, Show) data FedUserSearchTestCase = FedUserSearchTestCase { searchPolicy :: String, + -- restriction settings of the calling backend restrictionD1D2 :: Restriction, + -- restriction settings of the remote backend restrictionD2D1 :: Restriction, exactHandleSearchExpectFound :: Bool, fullSearchExpectFound :: Bool @@ -117,6 +120,7 @@ federatedUserSearch d1 d2 test = do AllowAll -> pure () TeamNotAllowed -> + -- if the team is not allowed, the restriction was set to by team earlier and we do not need to do anything pure () TeamAllowed -> do BrigI.addFederationRemoteTeam ownDomain remoteDomain remoteTeam From b2361be986009cd7b1c8d0810b6946fa93c9cdbf Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 11:21:36 +0000 Subject: [PATCH 32/34] inject map into interpreter --- .../brig/src/Brig/CanonicalInterpreter.hs | 4 +- .../FederationConfigStore/Cassandra.hs | 47 +++++++++---------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/services/brig/src/Brig/CanonicalInterpreter.hs b/services/brig/src/Brig/CanonicalInterpreter.hs index 4a2474dba6a..f53c28532a0 100644 --- a/services/brig/src/Brig/CanonicalInterpreter.hs +++ b/services/brig/src/Brig/CanonicalInterpreter.hs @@ -8,7 +8,7 @@ import Brig.Effects.BlacklistStore.Cassandra (interpretBlacklistStoreToCassandra import Brig.Effects.CodeStore (CodeStore) import Brig.Effects.CodeStore.Cassandra (codeStoreToCassandra, interpretClientToIO) import Brig.Effects.FederationConfigStore (FederationConfigStore) -import Brig.Effects.FederationConfigStore.Cassandra (interpretFederationDomainConfig) +import Brig.Effects.FederationConfigStore.Cassandra (interpretFederationDomainConfig, remotesMapFromCfgFile) import Brig.Effects.GalleyProvider (GalleyProvider) import Brig.Effects.GalleyProvider.RPC (interpretGalleyProviderToRPC) import Brig.Effects.JwtTools @@ -83,7 +83,7 @@ runBrigToIO e (AppT ma) = do . interpretJwtTools . interpretPublicKeyBundle . interpretJwk - . interpretFederationDomainConfig (e ^. settings . federationStrategy) (maybe [] (fmap (.federationDomainConfig)) (e ^. settings . federationDomainConfigs)) + . interpretFederationDomainConfig (e ^. settings . federationStrategy) (maybe mempty (remotesMapFromCfgFile . fmap (.federationDomainConfig)) (e ^. settings . federationDomainConfigs)) ) ) $ runReaderT ma e diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index 2a81a0d675a..2311b31572e 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -17,6 +17,7 @@ module Brig.Effects.FederationConfigStore.Cassandra ( interpretFederationDomainConfig, + remotesMapFromCfgFile, AddFederationRemoteResult (..), ) where @@ -42,37 +43,32 @@ interpretFederationDomainConfig :: Member (Embed m) r ) => Maybe FederationStrategy -> - [FederationDomainConfig] -> + Map Domain FederationDomainConfig -> Sem (FederationConfigStore ': r) a -> Sem r a -interpretFederationDomainConfig mFedStrategy cfgs = +interpretFederationDomainConfig mFedStrategy fedCfgs = interpret $ embed @m . \case - GetFederationConfig d -> getFederationConfig' cfgs d - GetFederationConfigs -> getFederationConfigs' mFedStrategy cfgs - AddFederationConfig cnf -> addFederationConfig' cfgs cnf - UpdateFederationConfig cnf -> updateFederationConfig' cfgs cnf - AddFederationRemoteTeam d t -> addFederationRemoteTeam' cfgs d t + GetFederationConfig d -> getFederationConfig' fedCfgs d + GetFederationConfigs -> getFederationConfigs' mFedStrategy fedCfgs + AddFederationConfig cnf -> addFederationConfig' fedCfgs cnf + UpdateFederationConfig cnf -> updateFederationConfig' fedCfgs cnf + AddFederationRemoteTeam d t -> addFederationRemoteTeam' fedCfgs d t RemoveFederationRemoteTeam d t -> removeFederationRemoteTeam' d t GetFederationRemoteTeams d -> getFederationRemoteTeams' d -- | Compile config file list into a map indexed by domains. Use this to make sure the config -- file is consistent (ie., no two entries for the same domain). -remotesMapFromCfgFile :: (Monad m) => [FederationDomainConfig] -> m (Map Domain FederationDomainConfig) -remotesMapFromCfgFile cfg = do +remotesMapFromCfgFile :: [FederationDomainConfig] -> Map Domain FederationDomainConfig +remotesMapFromCfgFile cfg = let dict = [(cnf.domain, cnf) | cnf <- cfg] merge c c' = if c == c' then c else error $ "error in config file: conflicting parameters on domain: " <> show (c, c') - pure $ Map.fromListWith merge dict + in Map.fromListWith merge dict --- | Return the config file list. Use this to make sure the config file is consistent (ie., --- no two entries for the same domain). Based on `remotesMapFromCfgFile`. -remotesListFromCfgFile :: Monad m => [FederationDomainConfig] -> m [FederationDomainConfig] -remotesListFromCfgFile cfgs = Map.elems <$> remotesMapFromCfgFile cfgs - -getFederationConfigs' :: forall m. (MonadClient m) => Maybe FederationStrategy -> [FederationDomainConfig] -> m FederationDomainConfigs +getFederationConfigs' :: forall m. (MonadClient m) => Maybe FederationStrategy -> Map Domain FederationDomainConfig -> m FederationDomainConfigs getFederationConfigs' mFedStrategy cfgs = do -- FUTUREWORK: we should solely rely on `db` in the future for remote domains; merging -- remote domains from `cfg` is just for providing an easier, more robust migration path. @@ -82,7 +78,7 @@ getFederationConfigs' mFedStrategy cfgs = do remotes <- (<>) <$> getFederationRemotes - <*> remotesListFromCfgFile cfgs + <*> pure (Map.elems cfgs) defFederationDomainConfigs & maybe id (\v cfg -> cfg {strategy = v}) mFedStrategy @@ -92,7 +88,7 @@ getFederationConfigs' mFedStrategy cfgs = do maxKnownNodes :: Int maxKnownNodes = 10000 -getFederationConfig' :: MonadClient m => [FederationDomainConfig] -> Domain -> m (Maybe FederationDomainConfig) +getFederationConfig' :: MonadClient m => Map Domain FederationDomainConfig -> Domain -> m (Maybe FederationDomainConfig) getFederationConfig' cfgs rDomain = case find ((== rDomain) . domain) cfgs of Just cfg -> pure . Just $ cfg -- the configuration from the file has precedence Nothing -> do @@ -116,10 +112,9 @@ getFederationRemotes = (\(d, p, r) -> FederationDomainConfig d p r) <$$> qry get :: PrepQuery R () (Domain, FederatedUserSearchPolicy, Int32) get = fromString $ "SELECT domain, search_policy, restriction FROM federation_remotes LIMIT " <> show maxKnownNodes -addFederationConfig' :: MonadClient m => [FederationDomainConfig] -> FederationDomainConfig -> m AddFederationRemoteResult -addFederationConfig' cfgs (FederationDomainConfig rDomain searchPolicy restriction) = do - cfg <- remotesMapFromCfgFile cfgs - conflict <- domainExistsInConfig cfg (FederationDomainConfig rDomain searchPolicy restriction) +addFederationConfig' :: MonadClient m => Map Domain FederationDomainConfig -> FederationDomainConfig -> m AddFederationRemoteResult +addFederationConfig' cfg (FederationDomainConfig rDomain searchPolicy restriction) = do + conflict <- domainExistsInConfig (FederationDomainConfig rDomain searchPolicy restriction) if conflict then pure $ AddFederationRemoteDivergingConfig cfg else do @@ -136,8 +131,8 @@ addFederationConfig' cfgs (FederationDomainConfig rDomain searchPolicy restricti where -- If remote domain is registered in config file, the version that can be added to the -- database must be the same. - domainExistsInConfig :: (Monad m) => (Map Domain FederationDomainConfig) -> FederationDomainConfig -> m Bool - domainExistsInConfig cfg fedDomConf = do + domainExistsInConfig :: (Monad m) => FederationDomainConfig -> m Bool + domainExistsInConfig fedDomConf = do pure $ case Map.lookup (domain fedDomConf) cfg of Nothing -> False Just fedDomConf' -> fedDomConf' /= fedDomConf @@ -148,7 +143,7 @@ addFederationConfig' cfgs (FederationDomainConfig rDomain searchPolicy restricti addTeams :: PrepQuery W (Domain, TeamId) () addTeams = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" -updateFederationConfig' :: MonadClient m => [FederationDomainConfig] -> FederationDomainConfig -> m UpdateFederationResult +updateFederationConfig' :: MonadClient m => Map Domain FederationDomainConfig -> FederationDomainConfig -> m UpdateFederationResult updateFederationConfig' cfgs (FederationDomainConfig rDomain searchPolicy restriction) = do if rDomain `elem` (domain <$> cfgs) then pure UpdateFederationRemoteDivergingConfig @@ -184,7 +179,7 @@ updateFederationConfig' cfgs (FederationDomainConfig rDomain searchPolicy restri insertTeam :: PrepQuery W (Domain, TeamId) () insertTeam = "INSERT INTO federation_remote_teams (domain, team) VALUES (?, ?)" -addFederationRemoteTeam' :: MonadClient m => [FederationDomainConfig] -> Domain -> TeamId -> m AddFederationRemoteTeamResult +addFederationRemoteTeam' :: MonadClient m => Map Domain FederationDomainConfig -> Domain -> TeamId -> m AddFederationRemoteTeamResult addFederationRemoteTeam' cfgs rDomain tid = do mDom <- getFederationConfig' cfgs rDomain case mDom of From c8256b9af7df417b76f349ab672917fb09ebec50 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 11:33:19 +0000 Subject: [PATCH 33/34] added comments --- .../FederationConfigStore/Cassandra.hs | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs index 2311b31572e..734a22ba170 100644 --- a/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs +++ b/services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs @@ -37,6 +37,13 @@ import Polysemy import Wire.API.Routes.FederationDomainConfig import Wire.API.User.Search +-- | Interpreter for getting the federation config from the database and the config file. +-- The config file is injected into the interpreter and has precedence over the database. +-- The config file is static and can only be changed by restarting the service. +-- If a domain is configured in the config file, it is not allowed to add it to the database. +-- If a domain is configured in the config file, it is not allowed to update it in the database. +-- If a domain is configured in the config file, it is not allowed to add a team restriction to it in the database. +-- In the future the config file will be removed and the database will be the only source of truth. interpretFederationDomainConfig :: forall m r a. ( MonadClient m, @@ -59,6 +66,7 @@ interpretFederationDomainConfig mFedStrategy fedCfgs = -- | Compile config file list into a map indexed by domains. Use this to make sure the config -- file is consistent (ie., no two entries for the same domain). +-- This is called during initialization of the interpreter and the service will fail if the config is not consistent. remotesMapFromCfgFile :: [FederationDomainConfig] -> Map Domain FederationDomainConfig remotesMapFromCfgFile cfg = let dict = [(cnf.domain, cnf) | cnf <- cfg] @@ -75,9 +83,10 @@ getFederationConfigs' mFedStrategy cfgs = do -- See -- https://docs.wire.com/understand/federation/backend-communication.html#configuring-remote-connections, -- http://docs.wire.com/developer/developer/federation-design-aspects.html#configuring-remote-connections-dev-perspective + -- (because the creation and update of a federation config is guarded, we can safely merge the two configs here) remotes <- (<>) - <$> getFederationRemotes + <$> getFederationRemotesFromDb <*> pure (Map.elems cfgs) defFederationDomainConfigs @@ -90,7 +99,7 @@ maxKnownNodes = 10000 getFederationConfig' :: MonadClient m => Map Domain FederationDomainConfig -> Domain -> m (Maybe FederationDomainConfig) getFederationConfig' cfgs rDomain = case find ((== rDomain) . domain) cfgs of - Just cfg -> pure . Just $ cfg -- the configuration from the file has precedence + Just cfg -> pure . Just $ cfg -- the configuration from the file has precedence (if exists there should not be a db entry at all) Nothing -> do mCnf <- retry x1 (query1 q (params LocalQuorum (Identity rDomain))) case mCnf of @@ -100,8 +109,8 @@ getFederationConfig' cfgs rDomain = case find ((== rDomain) . domain) cfgs of q :: PrepQuery R (Identity Domain) (FederatedUserSearchPolicy, Int32) q = "SELECT search_policy, restriction FROM federation_remotes WHERE domain = ?" -getFederationRemotes :: forall m. MonadClient m => m [FederationDomainConfig] -getFederationRemotes = (\(d, p, r) -> FederationDomainConfig d p r) <$$> qry +getFederationRemotesFromDb :: forall m. MonadClient m => m [FederationDomainConfig] +getFederationRemotesFromDb = (\(d, p, r) -> FederationDomainConfig d p r) <$$> qry where qry :: m [(Domain, FederatedUserSearchPolicy, FederationRestriction)] qry = do @@ -114,11 +123,12 @@ getFederationRemotes = (\(d, p, r) -> FederationDomainConfig d p r) <$$> qry addFederationConfig' :: MonadClient m => Map Domain FederationDomainConfig -> FederationDomainConfig -> m AddFederationRemoteResult addFederationConfig' cfg (FederationDomainConfig rDomain searchPolicy restriction) = do + -- if a domain already exists in a config, we do not allow to add it to the database conflict <- domainExistsInConfig (FederationDomainConfig rDomain searchPolicy restriction) if conflict then pure $ AddFederationRemoteDivergingConfig cfg else do - l <- length <$> getFederationRemotes + l <- length <$> getFederationRemotesFromDb if l >= maxKnownNodes then pure AddFederationRemoteMaxRemotesReached else @@ -145,6 +155,7 @@ addFederationConfig' cfg (FederationDomainConfig rDomain searchPolicy restrictio updateFederationConfig' :: MonadClient m => Map Domain FederationDomainConfig -> FederationDomainConfig -> m UpdateFederationResult updateFederationConfig' cfgs (FederationDomainConfig rDomain searchPolicy restriction) = do + -- if a domain already exists in a config, we do not allow update it if rDomain `elem` (domain <$> cfgs) then pure UpdateFederationRemoteDivergingConfig else do From ba0e75e5326f9324b709d0011872cd4fbf2e2bde Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 29 Nov 2023 11:52:23 +0000 Subject: [PATCH 34/34] lint --- services/brig/src/Brig/CanonicalInterpreter.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/brig/src/Brig/CanonicalInterpreter.hs b/services/brig/src/Brig/CanonicalInterpreter.hs index f53c28532a0..28231b06067 100644 --- a/services/brig/src/Brig/CanonicalInterpreter.hs +++ b/services/brig/src/Brig/CanonicalInterpreter.hs @@ -83,7 +83,7 @@ runBrigToIO e (AppT ma) = do . interpretJwtTools . interpretPublicKeyBundle . interpretJwk - . interpretFederationDomainConfig (e ^. settings . federationStrategy) (maybe mempty (remotesMapFromCfgFile . fmap (.federationDomainConfig)) (e ^. settings . federationDomainConfigs)) + . interpretFederationDomainConfig (e ^. settings . federationStrategy) (foldMap (remotesMapFromCfgFile . fmap (.federationDomainConfig)) (e ^. settings . federationDomainConfigs)) ) ) $ runReaderT ma e