Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WBP-5388 restrict contact search results according to team federation policy #3732

Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
00923cc
fed config store
battermann Nov 16, 2023
95558d1
wip
battermann Nov 16, 2023
1cd6bad
federation config store interpreter
battermann Nov 20, 2023
d62529c
fix
battermann Nov 20, 2023
2270672
move stuff into interpreter
battermann Nov 20, 2023
cce888d
wip
battermann Nov 20, 2023
02cf325
wip
battermann Nov 21, 2023
5c5f12a
wip
battermann Nov 21, 2023
b0a0f4a
implement team federation policy for remote search
battermann Nov 21, 2023
0b1c0e1
impl isTeamAllowed
battermann Nov 21, 2023
877ec64
wip
battermann Nov 22, 2023
26b79ea
tests
battermann Nov 22, 2023
7911d23
changelog
battermann Nov 23, 2023
171b6a7
unify API and domain models
battermann Nov 23, 2023
638956f
tagged union
battermann Nov 23, 2023
d152b1c
move everything into interpreter
battermann Nov 24, 2023
0829f1e
improve add remote team
battermann Nov 24, 2023
95dd157
handle empty list, optimize integration tests
battermann Nov 24, 2023
ff103e6
Haddocks
battermann Nov 27, 2023
dab866b
Update integration/test/API/BrigInternal.hs
battermann Nov 27, 2023
8caee10
golden test for federation restriction
battermann Nov 27, 2023
5f29b77
shorter code
battermann Nov 27, 2023
b0b1e3a
fix syntax
battermann Nov 27, 2023
226c275
removed comment
battermann Nov 27, 2023
2032d79
Update services/brig/src/Brig/Effects/FederationConfigStore/Cassandra.hs
battermann Nov 29, 2023
faa3689
Update services/brig/src/Brig/API/Internal.hs
battermann Nov 29, 2023
dc65ecf
removed test cases that are already covered
battermann Nov 29, 2023
ddafb7f
clean up
battermann Nov 29, 2023
7757c3b
clean up
battermann Nov 29, 2023
986e67e
clean up
battermann Nov 29, 2023
8216def
added some comments
battermann Nov 29, 2023
b2361be
inject map into interpreter
battermann Nov 29, 2023
c8256b9
added comments
battermann Nov 29, 2023
ba0e75e
lint
battermann Nov 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion changelog.d/2-features/WPB-5105
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions integration/integration.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ library
Test.Notifications
Test.Presence
Test.Roles
Test.Search
Test.User
Testlib.App
Testlib.Assertions
Expand Down
20 changes: 16 additions & 4 deletions integration/test/API/BrigInternal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -54,7 +56,7 @@ createUser domain cu = do
data FedConn = FedConn
{ domain :: String,
searchStrategy :: String,
restriction :: String
restriction :: Maybe [String]
}
deriving (Eq, Ord, Show)

Expand All @@ -63,7 +65,13 @@ instance ToJSON FedConn where
Aeson.object
[ "domain" .= d,
"search_policy" .= s,
"restriction" .= 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))]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the cs <$> teams part rely on the Show TeamId instance? Shouldn't it rather rely on the ToJSON TeamId instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it converts a String into a ByteString in this case. Remember that in the integration tests we do not have access to wire-api. However, Aeson.toJSON works, too.

Copy link
Contributor

@mdimjasevic mdimjasevic Nov 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I forgot this is for a [String] value, and not a [Data.Id.TeamId] value. The Data.Id module is not in wire-api, but in types-common, though. However, maybe this is not to be used in the integration package as well, but I'm not sure.

)
r
]

instance MakesValue FedConn where
Expand Down Expand Up @@ -159,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
battermann marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down
7 changes: 7 additions & 0 deletions integration/test/API/GalleyInternal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
63 changes: 11 additions & 52 deletions integration/test/Test/Brig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ 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 API.Common (randomName)
import Data.Aeson.Types hiding ((.=))
import Data.Set qualified as Set
import Data.String.Conversions
Expand All @@ -14,17 +13,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
Expand Down Expand Up @@ -54,11 +42,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)
Expand Down Expand Up @@ -139,46 +127,17 @@ 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
(_, 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]
Expand Down
28 changes: 14 additions & 14 deletions integration/test/Test/Conversation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading