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

Block services whitelist for teams with default protocol MLS #4266

Merged
merged 6 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog.d/1-api-changes/WPB-10797
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Services allowlist are blocked by 409 (mls-services-not-allowed) for teams with default protocol MLS.
1 change: 1 addition & 0 deletions integration/integration.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ library
Test.MLS.Message
Test.MLS.Notifications
Test.MLS.One2One
Test.MLS.Services
Test.MLS.SubConversation
Test.MLS.Unreachable
Test.Notifications
Expand Down
23 changes: 23 additions & 0 deletions integration/test/API/Brig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -820,3 +820,26 @@ upgradePersonalToTeam :: (HasCallStack, MakesValue user) => user -> String -> Ap
upgradePersonalToTeam user name = do
req <- baseRequest user Brig Versioned $ joinHttpPath ["upgrade-personal-to-team"]
submit "POST" $ req & addJSONObject ["name" .= name, "icon" .= "default"]

postServiceWhitelist ::
( HasCallStack,
MakesValue user,
MakesValue tid,
MakesValue update
) =>
user ->
tid ->
update ->
App Response
postServiceWhitelist user tid update = do
tidStr <- asString tid
updateJson <- make update
req <-
baseRequest user Brig Versioned $
joinHttpPath
[ "teams",
tidStr,
"services",
"whitelist"
]
submit "POST" (addJSON updateJson req)
95 changes: 95 additions & 0 deletions integration/test/Test/MLS/Services.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
module Test.MLS.Services where

import API.Brig
import API.Common
import API.GalleyInternal (patchTeamFeatureConfig)
import SetupHelpers
import Testlib.JSON
import Testlib.Prelude

testWhitelistUpdatePermissions :: (HasCallStack) => App ()
testWhitelistUpdatePermissions = do
-- Create a team
(owner, tid, []) <- createTeam OwnDomain 1

-- Create a team admin
admin <- createTeamMemberWithRole owner tid "admin"

-- Create a service
email <- randomEmail
provider <- make <$> setupProvider owner def {newProviderEmail = email}
providerId <- provider %. "id" & asString
service <- make <$> newService OwnDomain providerId def

do
-- Check that a random user can't add the service to the whitelist
uid <- randomUser OwnDomain def
serviceId <- service %. "id" & asString
np <-
make
$ object
[ "id" .= serviceId,
"provider" .= providerId,
"whitelisted" .= True
]
bindResponse (postServiceWhitelist uid tid np) $ \resp -> do
resp.status `shouldMatchInt` 403
(resp.jsonBody %. "label") `shouldMatch` Just "insufficient-permissions"

do
-- Check that an admin can add the service to the whitelist
serviceId <- service %. "id" & asString
np <-
make
$ object
[ "id" .= serviceId,
"provider" .= providerId,
"whitelisted" .= True
]
postServiceWhitelist admin tid np >>= assertStatus 200

-- set team's defaultProtocol to MLS
mlsConfig <-
make
$ object
[ "config"
.= object
[ "allowedCipherSuites" .= [1 :: Int],
"defaultCipherSuite" .= (1 :: Int),
"defaultProtocol" .= "mls",
"protocolToggleUsers" .= ([] :: [String]),
"supportedProtocols" .= ["mls", "proteus"]
],
"status" .= "enabled",
"ttl" .= "unlimited"
]
patchTeamFeatureConfig OwnDomain tid "mls" mlsConfig >>= assertStatus 200

do
-- Check that a random user can't add the service to the whitelist
uid <- randomUser OwnDomain def
serviceId <- service %. "id" & asString
np <-
make
$ object
[ "id" .= serviceId,
"provider" .= providerId,
"whitelisted" .= True
]
bindResponse (postServiceWhitelist uid tid np) $ \resp -> do
resp.status `shouldMatchInt` 409
(resp.jsonBody %. "label") `shouldMatch` Just "mls-services-not-allowed"

do
-- Check that an admin can't add the service to the whitelist
serviceId <- service %. "id" & asString
np <-
make
$ object
[ "id" .= serviceId,
"provider" .= providerId,
"whitelisted" .= True
]
postServiceWhitelist admin tid np >>= \resp -> do
resp.status `shouldMatchInt` 409
(resp.jsonBody %. "label") `shouldMatch` Just "mls-services-not-allowed"
3 changes: 3 additions & 0 deletions libs/wire-api/src/Wire/API/Error/Brig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ data BrigError
| PropertyKeyTooLarge
| PropertyValueTooLarge
| UserAlreadyInATeam
| MLSServicesNotAllowed

instance (Typeable (MapError e), KnownError (MapError e)) => IsSwaggerError (e :: BrigError) where
addToOpenApi = addStaticErrorToSwagger @(MapError e)
Expand Down Expand Up @@ -298,3 +299,5 @@ type instance MapError 'PropertyKeyTooLarge = 'StaticError 403 "property-key-too
type instance MapError 'PropertyValueTooLarge = 'StaticError 403 "property-value-too-large" "The property value is too large"

type instance MapError 'UserAlreadyInATeam = 'StaticError 403 "user-already-in-a-team" "Switching teams is not allowed"

type instance MapError 'MLSServicesNotAllowed = 'StaticError 409 "mls-services-not-allowed" "Services not allowed in MLS"
4 changes: 3 additions & 1 deletion libs/wire-subsystems/src/Wire/UserSubsystem/Error.hs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ data UserSubsystemError
| UserSubsystemInvalidHandle
| UserSubsystemProfileNotFound
| UserSubsystemInsufficientTeamPermissions
| UserSubsystemMLSServicesNotAllowed
deriving (Eq, Show)

userSubsystemErrorToHttpError :: UserSubsystemError -> HttpError
Expand All @@ -29,6 +30,7 @@ userSubsystemErrorToHttpError =
UserSubsystemHandleExists -> errorToWai @E.HandleExists
UserSubsystemInvalidHandle -> errorToWai @E.InvalidHandle
UserSubsystemHandleManagedByScim -> errorToWai @E.HandleManagedByScim
UserSubsystemInsufficientTeamPermissions -> errorToWai @'E.InsufficientTeamPermissions
UserSubsystemInsufficientTeamPermissions -> errorToWai @E.InsufficientTeamPermissions
UserSubsystemMLSServicesNotAllowed -> errorToWai @E.MLSServicesNotAllowed

instance Exception UserSubsystemError
13 changes: 12 additions & 1 deletion services/brig/src/Brig/Provider/API.hs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import UnliftIO.Async (pooledMapConcurrentlyN_)
import Wire.API.Conversation hiding (Member)
import Wire.API.Conversation.Bot
import Wire.API.Conversation.Bot qualified as Public
import Wire.API.Conversation.Protocol
import Wire.API.Conversation.Role
import Wire.API.Error
import Wire.API.Error.Brig qualified as E
Expand Down Expand Up @@ -289,6 +290,7 @@ beginPasswordReset (Public.PasswordReset target) = do
let gen = mkVerificationCodeGen target
(lift . liftSem $ createCode gen VerificationCode.PasswordReset (Retries 3) (Timeout 3600) (Just (toUUID pid))) >>= \case
Left (CodeAlreadyExists code) ->
-- FUTUREWORK: use subsystem error
throwE $ pwResetError (PasswordResetInProgress $ Just code.codeTTL)
Right code ->
lift $ sendPasswordResetMail target (code.codeKey) (code.codeValue)
Expand Down Expand Up @@ -609,11 +611,12 @@ updateServiceWhitelist ::
Public.UpdateServiceWhitelist ->
(Handler r) UpdateServiceWhitelistResp
updateServiceWhitelist uid con tid upd = do
-- Preconditions
guardSecondFactorDisabled (Just uid)
guardMLSNotDefault
let pid = updateServiceWhitelistProvider upd
sid = updateServiceWhitelistService upd
newWhitelisted = updateServiceWhitelistStatus upd
-- Preconditions
lift . liftSem $ ensurePermissions uid tid (Set.toList serviceWhitelistPermissions)
_ <- wrapClientE (DB.lookupService pid sid) >>= maybeServiceNotFound
-- Add to various tables
Expand All @@ -640,6 +643,14 @@ updateServiceWhitelist uid con tid upd = do
)
wrapClientE $ DB.deleteServiceWhitelist (Just tid) pid sid
pure UpdateServiceWhitelistRespChanged
where
guardMLSNotDefault = lift . liftSem $ do
feat <- GalleyAPIAccess.getFeatureConfigForTeam @_ @Feature.MLSConfig tid
let defProtocol = feat.config.mlsDefaultProtocol
case defProtocol of
ProtocolProteusTag -> pure ()
ProtocolMLSTag -> throw UserSubsystemMLSServicesNotAllowed
ProtocolMixedTag -> throw UserSubsystemMLSServicesNotAllowed

--------------------------------------------------------------------------------
-- Bot API
Expand Down
28 changes: 0 additions & 28 deletions services/brig/test/integration/API/Provider.hs
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,6 @@ tests dom conf p db b c g n = do
"service whitelist"
[ test p "search permissions" $
testWhitelistSearchPermissions conf db b g,
test p "update permissions" $
testWhitelistUpdatePermissions conf db b g,
test p "basic functionality" $
testWhitelistBasic conf db b g,
test p "search" $ testSearchWhitelist conf db b g,
Expand Down Expand Up @@ -879,32 +877,6 @@ testWhitelistSearchPermissions _config _db brig galley = do
listTeamServiceProfilesByPrefix brig member tid Nothing True 20
!!! const 200 === statusCode

testWhitelistUpdatePermissions :: Config -> DB.ClientState -> Brig -> Galley -> Http ()
testWhitelistUpdatePermissions config db brig galley = do
-- Create a team
(owner, tid) <- Team.createUserWithTeam brig
-- Create a team admin
let Just adminPermissions = newPermissions serviceWhitelistPermissions mempty
admin <- userId <$> Team.createTeamMember brig galley owner tid adminPermissions
-- Create a service
pid <- providerId <$> randomProvider db brig
new <- defNewService config
sid <- serviceId <$> addGetService brig pid new
enableService brig pid sid
-- Check that a random user can't add it to the whitelist
_uid <- userId <$> randomUser brig
updateServiceWhitelist brig _uid tid (UpdateServiceWhitelist pid sid True) !!! do
const 403 === statusCode
const (Just "insufficient-permissions") === fmap Error.label . responseJsonMaybe
-- Check that a member who's not a team admin also can't add it to the whitelist
_uid <- userId <$> Team.createTeamMember brig galley owner tid noPermissions
updateServiceWhitelist brig _uid tid (UpdateServiceWhitelist pid sid True) !!! do
const 403 === statusCode
const (Just "insufficient-permissions") === fmap Error.label . responseJsonMaybe
-- Check that a team admin can add and remove from the whitelist
whitelistService brig admin tid pid sid
dewhitelistService brig admin tid pid sid

testSearchWhitelist :: Config -> DB.ClientState -> Brig -> Galley -> Http ()
testSearchWhitelist config db brig galley = do
-- Create a team, a team owner, and a team member with no permissions
Expand Down
Loading