diff --git a/changelog.d/1-api-changes/WPB-27169-do-not-count-apps-and-collaborators-as-members-in-get-team-size b/changelog.d/1-api-changes/WPB-27169-do-not-count-apps-and-collaborators-as-members-in-get-team-size new file mode 100644 index 00000000000..a052dda2913 --- /dev/null +++ b/changelog.d/1-api-changes/WPB-27169-do-not-count-apps-and-collaborators-as-members-in-get-team-size @@ -0,0 +1 @@ +Do not count apps and collaborators as members in get-team-size. New schema: `{"teamSize": num, "apps": num, "collaborators": num}` (non-overlapping). `teamSize` has been the label since the dawn of time, only the other two have changed. The protobuf schema for TeamEvents changed accordingly. diff --git a/integration/test/Test/Apps.hs b/integration/test/Test/Apps.hs index 4428f5e732f..fffa1376106 100644 --- a/integration/test/Test/Apps.hs +++ b/integration/test/Test/Apps.hs @@ -68,6 +68,13 @@ testCreateGetApp sameOrOtherDomain = do void $ assertNoEvent 5 wsRegularMember pure (appId, cookie) + -- team size counts apps separately. (they are not members.) + bindResponse (getTeamSize owner tid) $ \resp -> do + resp.status `shouldMatchInt` 200 + resp.json %. "teamSize" `shouldMatchInt` 2 + resp.json %. "apps" `shouldMatchInt` 1 + resp.json %. "collaborators" `shouldMatchInt` 0 + -- Verify that the team.member-join event is in the team notifications queue bindResponse (getTeamNotifications regularMember (Just lastTeamNotif)) $ \resp -> do resp.status `shouldMatchInt` 200 @@ -189,6 +196,7 @@ testDeleteAppFromTeam = do appId <- bindResponse (createApp owner tid new) $ \resp -> do resp.status `shouldMatchInt` 200 resp.json %. "user.id" & asString + BrigI.refreshIndex domain let appIdObject = object ["domain" .= domain, "id" .= appId] @@ -572,30 +580,29 @@ testAppReceivesMemberJoinNotification = do testTeamSizeWithApps :: (HasCallStack) => TaggedBool "test internal api" -> App () testTeamSizeWithApps (TaggedBool testInternalApi) = do domain <- make OwnDomain - numRegulars <- liftIO $ randomRIO (1 :: Int, 3) + numRegulars <- liftIO $ randomRIO (2 :: Int, 4) numApps <- liftIO $ randomRIO (1 :: Int, 3) - (owner, tid, extraMembers) <- createTeam domain (numRegulars + 1) + (owner, tid, extraMembers) <- createTeam domain numRegulars apps <- replicateM numApps $ bindResponse (createApp owner tid def) $ \resp -> do resp.status `shouldMatchInt` 200 resp.json %. "user" let checkSize :: (HasCallStack) => Int -> Int -> App () - checkSize wantRegulars wantApps = - (if testInternalApi then BrigI.getTeamSize else Brig.getTeamSize) owner tid `bindResponse` \resp -> do - resp.status `shouldMatchInt` 200 - resp.json %. "teamSize" `shouldMatchInt` (1 + wantRegulars + wantApps) - resp.json %. "teamSizeRegulars" `shouldMatchInt` (1 + wantRegulars) - resp.json %. "teamSizeApps" `shouldMatchInt` wantApps + checkSize wantRegulars wantApps = do + BrigI.refreshIndex domain + eventually $ do + (if testInternalApi then BrigI.getTeamSize else Brig.getTeamSize) owner tid `bindResponse` \resp -> do + resp.status `shouldMatchInt` 200 + resp.json %. "teamSize" `shouldMatchInt` wantRegulars + resp.json %. "apps" `shouldMatchInt` wantApps + resp.json %. "collaborators" `shouldMatchInt` 0 - BrigI.refreshIndex domain - eventually $ do - checkSize numRegulars numApps + checkSize numRegulars numApps deleteTeamMember tid owner (head apps) >>= assertSuccess - deleteTeamMember tid owner (head extraMembers) >>= assertSuccess + checkSize numRegulars (numApps - 1) - BrigI.refreshIndex domain - eventually $ do - checkSize (numRegulars - 1) (numApps - 1) + deleteTeamMember tid owner (head extraMembers) >>= assertSuccess + checkSize (numRegulars - 1) (numApps - 1) diff --git a/integration/test/Test/TeamCollaborators.hs b/integration/test/Test/TeamCollaborators.hs index 642dad537e8..45642d45e3e 100644 --- a/integration/test/Test/TeamCollaborators.hs +++ b/integration/test/Test/TeamCollaborators.hs @@ -19,7 +19,7 @@ module Test.TeamCollaborators where -import qualified API.Brig as BrigP +import API.Brig as BrigP import qualified API.BrigInternal as BrigI import API.Common (randomName) import API.Galley @@ -63,6 +63,14 @@ testCreateTeamCollaborator = do res %. "team" `shouldMatch` team res %. "permissions" `shouldMatch` ["create_team_conversation", "implicit_connection"] + -- team size counts collaborators separately + BrigI.refreshIndex OwnDomain + bindResponse (getTeamSize owner team) $ \resp -> do + resp.status `shouldMatchInt` 200 + resp.json %. "teamSize" `shouldMatchInt` 2 + resp.json %. "apps" `shouldMatchInt` 0 + resp.json %. "collaborators" `shouldMatchInt` 1 + testTeamCollaboratorEndpointsForbiddenForOtherTeams :: (HasCallStack) => App () testTeamCollaboratorEndpointsForbiddenForOtherTeams = do (owner, _team, _members) <- createTeam OwnDomain 2 diff --git a/libs/types-common-journal/proto/TeamEvents.proto b/libs/types-common-journal/proto/TeamEvents.proto index 8bd25c21cc8..6e2aed96213 100644 --- a/libs/types-common-journal/proto/TeamEvents.proto +++ b/libs/types-common-journal/proto/TeamEvents.proto @@ -22,10 +22,11 @@ message TeamEvent { // are guaranteed to be present). // // for backwards compatibility, clients should make these - // fields optional, and fall back to using `member_count` if - // they are missing. - required int32 member_count_regular = 4; - required int32 member_count_app = 5; + // fields optional, and assume '0' if missing. wire-server + // always sets these fields and never receives team events + // from clients, so we make the fields as "required". + required int32 apps = 4; + required int32 collaborators = 5; } enum EventType { diff --git a/libs/wire-api/src/Wire/API/Routes/Public/Galley/TeamMember.hs b/libs/wire-api/src/Wire/API/Routes/Public/Galley/TeamMember.hs index 33044bfcc0a..e5d10d10977 100644 --- a/libs/wire-api/src/Wire/API/Routes/Public/Galley/TeamMember.hs +++ b/libs/wire-api/src/Wire/API/Routes/Public/Galley/TeamMember.hs @@ -213,6 +213,7 @@ type TeamMemberAPI = "add-team-collaborator" ( Summary "Add a collaborator to the team." :> From 'V10 + :> CanThrow 'TooManyTeamMembersOnTeamWithLegalhold :> ZLocalUser :> "teams" :> Capture "tid" TeamId diff --git a/libs/wire-api/src/Wire/API/Team/Size.hs b/libs/wire-api/src/Wire/API/Team/Size.hs index d751769a903..86467737075 100644 --- a/libs/wire-api/src/Wire/API/Team/Size.hs +++ b/libs/wire-api/src/Wire/API/Team/Size.hs @@ -17,66 +17,33 @@ module Wire.API.Team.Size ( TeamSize (..), - teamSizeTotal, - updateTeamSize, ) where import Control.Lens ((?~)) import Data.Aeson qualified as A -import Data.Aeson.Types qualified as A import Data.OpenApi qualified as S import Data.Schema import Imports import Numeric.Natural import Test.QuickCheck (arbitrarySizedNatural) -import Wire.API.User.Search import Wire.Arbitrary data TeamSize = TeamSize - { regulars :: Natural, - apps :: Natural + { teamSize :: Natural, + apps :: Natural, + collaborators :: Natural } deriving (Show, Eq) deriving (A.ToJSON, A.FromJSON, S.ToSchema) via (Schema TeamSize) --- | Total team members (regulars + apps). -teamSizeTotal :: TeamSize -> Natural -teamSizeTotal ts = ts.regulars + ts.apps - --- Increase or decrease a team size component, depending on user type. - --- If the result of a decrease is <0, it is set to 1 (regulars) or 0 --- (apps). This handles corner cases where ES reports lower numbers --- from the past. -updateTeamSize :: UserTypeFilter -> TeamSize -> Int -> TeamSize -updateTeamSize = go - where - go :: UserTypeFilter -> TeamSize -> Int -> TeamSize - go UserTypeFilterRegular (TeamSize rs as) n = TeamSize (upd 1 rs n) as - go UserTypeFilterApp (TeamSize rs as) n = TeamSize rs (upd 0 as n) - - upd :: Int -> Natural -> Int -> Natural - upd low n i = fromIntegral . max low $ fromIntegral n + i - instance ToSchema TeamSize where schema = - objectWithDocModifier (description ?~ "Team member counts broken down by user type.") $ - fromTeamSize .= tripleSchema `withParser` validate - where - fromTeamSize :: TeamSize -> (Natural, Natural, Maybe Natural) - fromTeamSize ts = (ts.regulars, ts.apps, Just (teamSizeTotal ts)) - tripleSchema :: ObjectSchema SwaggerDoc (Natural, Natural, Maybe Natural) - tripleSchema = - (,,) - <$> (\(r, _, _) -> r) .= fieldWithDocModifier "teamSizeRegulars" (description ?~ "Number of regular users in team.") schema - <*> (\(_, a, _) -> a) .= fieldWithDocModifier "teamSizeApps" (description ?~ "Number of apps in team.") schema - <*> (\(_, _, t) -> t) .= maybe_ (optFieldWithDocModifier "teamSize" (description ?~ "Total team members (teamSizeRegulars + teamSizeApps).") schema) - validate :: (Natural, Natural, Maybe Natural) -> A.Parser TeamSize - validate (r, a, Nothing) = pure TeamSize {regulars = r, apps = a} - validate (r, a, Just t) - | r + a == t = pure TeamSize {regulars = r, apps = a} - | otherwise = fail $ "teamSize (" <> show t <> ") != regulars + apps (" <> show (r + a) <> ")" + objectWithDocModifier (description ?~ "Team member counts: paid seats (regular users), apps, and collaborators.") $ + TeamSize + <$> (.teamSize) .= field "teamSize" schema + <*> (.apps) .= field "apps" schema + <*> (.collaborators) .= field "collaborators" schema instance Arbitrary TeamSize where - arbitrary = TeamSize <$> arbitrarySizedNatural <*> arbitrarySizedNatural + arbitrary = TeamSize <$> arbitrarySizedNatural <*> arbitrarySizedNatural <*> arbitrarySizedNatural diff --git a/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/TeamSize.hs b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/TeamSize.hs index 8137fe05501..82757e4e94d 100644 --- a/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/TeamSize.hs +++ b/libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/TeamSize.hs @@ -21,10 +21,10 @@ import Imports import Wire.API.Team.Size testObject_TeamSize_1 :: TeamSize -testObject_TeamSize_1 = TeamSize 0 0 +testObject_TeamSize_1 = TeamSize 0 0 0 testObject_TeamSize_2 :: TeamSize -testObject_TeamSize_2 = TeamSize 100 400 +testObject_TeamSize_2 = TeamSize 100 400 7 testObject_TeamSize_3 :: TeamSize -testObject_TeamSize_3 = TeamSize (fromIntegral $ maxBound @Word64) (fromIntegral $ maxBound @Word64) +testObject_TeamSize_3 = TeamSize (fromIntegral $ maxBound @Word64) (fromIntegral $ maxBound @Word64) (fromIntegral $ maxBound @Word64) diff --git a/libs/wire-api/test/golden/testObject_Event_meeting_create_manual_1.json b/libs/wire-api/test/golden/testObject_Event_meeting_create_manual_1.json index faaf77d4b6a..7aac908df04 100644 --- a/libs/wire-api/test/golden/testObject_Event_meeting_create_manual_1.json +++ b/libs/wire-api/test/golden/testObject_Event_meeting_create_manual_1.json @@ -1,9 +1,5 @@ { "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e", - "qualified_id": { - "domain": "example.com", - "id": "00000001-0000-0000-0000-000000000001" - }, "from": "a471447c-aa30-4592-81b0-dec6c1c02bca", "qualified_conversation": { "domain": "example.com", @@ -13,6 +9,10 @@ "domain": "example.com", "id": "a471447c-aa30-4592-81b0-dec6c1c02bca" }, + "qualified_id": { + "domain": "example.com", + "id": "00000001-0000-0000-0000-000000000001" + }, "time": "2018-01-01T00:00:00.000Z", "type": "meeting.create", "via": "user" diff --git a/libs/wire-api/test/golden/testObject_Event_meeting_delete_manual_1.json b/libs/wire-api/test/golden/testObject_Event_meeting_delete_manual_1.json index 5bae8ab62d0..6ff021670ec 100644 --- a/libs/wire-api/test/golden/testObject_Event_meeting_delete_manual_1.json +++ b/libs/wire-api/test/golden/testObject_Event_meeting_delete_manual_1.json @@ -1,9 +1,5 @@ { "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e", - "qualified_id": { - "domain": "example.com", - "id": "00000001-0000-0000-0000-000000000001" - }, "from": "a471447c-aa30-4592-81b0-dec6c1c02bca", "qualified_conversation": { "domain": "example.com", @@ -13,6 +9,10 @@ "domain": "example.com", "id": "a471447c-aa30-4592-81b0-dec6c1c02bca" }, + "qualified_id": { + "domain": "example.com", + "id": "00000001-0000-0000-0000-000000000001" + }, "time": "2018-01-01T00:00:00.000Z", "type": "meeting.delete", "via": "user" diff --git a/libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_1.json b/libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_1.json index 8d40ebe09a2..cad3c3e2768 100644 --- a/libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_1.json +++ b/libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_1.json @@ -1,9 +1,5 @@ { "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e", - "qualified_id": { - "domain": "example.com", - "id": "00000001-0000-0000-0000-000000000001" - }, "from": "a471447c-aa30-4592-81b0-dec6c1c02bca", "qualified_conversation": { "domain": "example.com", @@ -13,6 +9,10 @@ "domain": "example.com", "id": "a471447c-aa30-4592-81b0-dec6c1c02bca" }, + "qualified_id": { + "domain": "example.com", + "id": "00000001-0000-0000-0000-000000000001" + }, "time": "2018-01-01T00:00:00.000Z", "type": "meeting.member-add", "via": "user" diff --git a/libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_2.json b/libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_2.json index 628f1bf141e..00f1ce4f3bc 100644 --- a/libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_2.json +++ b/libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_2.json @@ -1,9 +1,5 @@ { "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e", - "qualified_id": { - "domain": "example.com", - "id": "00000001-0000-0000-0000-000000000001" - }, "from": "a471447c-aa30-4592-81b0-dec6c1c02bca", "qualified_conversation": { "domain": "example.com", @@ -13,6 +9,10 @@ "domain": "example.com", "id": "a471447c-aa30-4592-81b0-dec6c1c02bca" }, + "qualified_id": { + "domain": "example.com", + "id": "00000001-0000-0000-0000-000000000001" + }, "team": "00000002-0000-0000-0000-000000000002", "time": "2018-01-01T00:00:00.000Z", "type": "meeting.member-add", diff --git a/libs/wire-api/test/golden/testObject_Event_meeting_update_manual_1.json b/libs/wire-api/test/golden/testObject_Event_meeting_update_manual_1.json index e1754d221ef..42c3c2c3780 100644 --- a/libs/wire-api/test/golden/testObject_Event_meeting_update_manual_1.json +++ b/libs/wire-api/test/golden/testObject_Event_meeting_update_manual_1.json @@ -1,9 +1,5 @@ { "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e", - "qualified_id": { - "domain": "example.com", - "id": "00000001-0000-0000-0000-000000000001" - }, "from": "a471447c-aa30-4592-81b0-dec6c1c02bca", "qualified_conversation": { "domain": "example.com", @@ -13,6 +9,10 @@ "domain": "example.com", "id": "a471447c-aa30-4592-81b0-dec6c1c02bca" }, + "qualified_id": { + "domain": "example.com", + "id": "00000001-0000-0000-0000-000000000001" + }, "time": "2018-01-01T00:00:00.000Z", "type": "meeting.update", "via": "user" diff --git a/libs/wire-api/test/golden/testObject_TeamSize_1.json b/libs/wire-api/test/golden/testObject_TeamSize_1.json index 92dda71f2da..e76772592dc 100644 --- a/libs/wire-api/test/golden/testObject_TeamSize_1.json +++ b/libs/wire-api/test/golden/testObject_TeamSize_1.json @@ -1,5 +1,5 @@ { - "teamSize": 0, - "teamSizeApps": 0, - "teamSizeRegulars": 0 + "apps": 0, + "collaborators": 0, + "teamSize": 0 } diff --git a/libs/wire-api/test/golden/testObject_TeamSize_2.json b/libs/wire-api/test/golden/testObject_TeamSize_2.json index 5b9794591db..293cfd45e5f 100644 --- a/libs/wire-api/test/golden/testObject_TeamSize_2.json +++ b/libs/wire-api/test/golden/testObject_TeamSize_2.json @@ -1,5 +1,5 @@ { - "teamSize": 500, - "teamSizeApps": 400, - "teamSizeRegulars": 100 + "apps": 400, + "collaborators": 7, + "teamSize": 100 } diff --git a/libs/wire-api/test/golden/testObject_TeamSize_3.json b/libs/wire-api/test/golden/testObject_TeamSize_3.json index 421801b4b47..2145f501bf3 100644 --- a/libs/wire-api/test/golden/testObject_TeamSize_3.json +++ b/libs/wire-api/test/golden/testObject_TeamSize_3.json @@ -1,5 +1,5 @@ { - "teamSize": 3.689348814741910323e19, - "teamSizeApps": 1.8446744073709551615e19, - "teamSizeRegulars": 1.8446744073709551615e19 + "apps": 1.8446744073709551615e19, + "collaborators": 1.8446744073709551615e19, + "teamSize": 1.8446744073709551615e19 } diff --git a/libs/wire-subsystems/src/Wire/IndexedUserStore/ElasticSearch.hs b/libs/wire-subsystems/src/Wire/IndexedUserStore/ElasticSearch.hs index 07572a29851..68e0f8a8649 100644 --- a/libs/wire-subsystems/src/Wire/IndexedUserStore/ElasticSearch.hs +++ b/libs/wire-subsystems/src/Wire/IndexedUserStore/ElasticSearch.hs @@ -90,7 +90,7 @@ getTeamSizeImpl cfg tid = do result <- either (embed . throwIO . IndexLookupError) pure (r :: Either ES.EsError (ES.SearchResult UserDoc)) let aggs = fromMaybe mempty (ES.aggregations result) getCount name = maybe 0 (.filterDocCount) $ M.lookup name aggs >>= parseMaybe (parseJSON @FilterResult) - pure $ TeamSize (getCount "regulars") (getCount "apps") + pure $ TeamSize (getCount "teamSize") (getCount "apps") (getCount "collaborators") where teamQ = termQ "team" (idToText tid) @@ -119,13 +119,18 @@ getTeamSizeImpl cfg tid = do { ES.boolQueryMustMatch = [teamQ, termQ "type" "app"] } + -- Collaborators are not members of the team, they are users (of other teams + -- or of no team) that collaborate with it. + collaboratorQuery = termQ "collaborating_teams" (idToText tid) + search = (ES.mkSearch Nothing Nothing) { ES.size = ES.Size 0, ES.aggBody = Just $ - ES.mkAggregations "regulars" (ES.FilterAgg (ES.FilterAggregation (ES.Filter regularQuery) Nothing)) + ES.mkAggregations "teamSize" (ES.FilterAgg (ES.FilterAggregation (ES.Filter regularQuery) Nothing)) <> ES.mkAggregations "apps" (ES.FilterAgg (ES.FilterAggregation (ES.Filter appQuery) Nothing)) + <> ES.mkAggregations "collaborators" (ES.FilterAgg (ES.FilterAggregation (ES.Filter collaboratorQuery) Nothing)) } upsertImpl :: diff --git a/libs/wire-subsystems/src/Wire/TeamJournal.hs b/libs/wire-subsystems/src/Wire/TeamJournal.hs index 9ae5ec1044a..bd65923296d 100644 --- a/libs/wire-subsystems/src/Wire/TeamJournal.hs +++ b/libs/wire-subsystems/src/Wire/TeamJournal.hs @@ -112,14 +112,13 @@ journalEvent typ tid dat tim = do -- utils evData :: TeamSize -> [UserId] -> Maybe Currency.Alpha -> TeamEvent'EventData -evData teamSize@(TeamSize regulars apps) billingUserIds cur = - defMessage - & T.memberCount .~ memberCountTotal - & T.billingUser .~ (toBytes <$> billingUserIds) - & T.maybe'currency .~ (pack . show <$> cur) - & T.memberCountRegular .~ memberCountRegulars - & T.memberCountApp .~ memberCountApps - where - memberCountTotal, memberCountRegulars, memberCountApps :: Int32 - (memberCountTotal, memberCountRegulars, memberCountApps) = - (fromIntegral $ teamSizeTotal teamSize, fromIntegral regulars, fromIntegral apps) +evData + (TeamSize (fromIntegral -> teamSize) (fromIntegral -> apps) (fromIntegral -> collaborators)) + billingUserIds + cur = + defMessage + & T.memberCount .~ teamSize + & T.billingUser .~ (toBytes <$> billingUserIds) + & T.maybe'currency .~ (pack . show <$> cur) + & T.apps .~ apps + & T.collaborators .~ collaborators diff --git a/libs/wire-subsystems/src/Wire/TeamSubsystem.hs b/libs/wire-subsystems/src/Wire/TeamSubsystem.hs index cd4fa9a7cac..8674077cde5 100644 --- a/libs/wire-subsystems/src/Wire/TeamSubsystem.hs +++ b/libs/wire-subsystems/src/Wire/TeamSubsystem.hs @@ -26,13 +26,22 @@ import Data.Qualified import Data.Range import Data.Singletons (Demote, Sing, SingKind, fromSing) import Imports +import Numeric.Natural import Polysemy +import Polysemy.Input (Input, input) import Wire.API.Error import Wire.API.Error.Galley +import Wire.API.Team.Feature (FeatureStatus (FeatureStatusEnabled), LegalholdConfig) +import Wire.API.Team.FeatureFlags (FanoutLimit, FeatureDefaults (..)) import Wire.API.Team.LegalHold (UserLegalHoldStatusResponse) import Wire.API.Team.Member import Wire.API.Team.Member.Error import Wire.API.Team.Member.Info (TeamMemberInfoList) +import Wire.API.Team.Size (TeamSize (..)) +import Wire.BrigAPIAccess (BrigAPIAccess, getSize) +import Wire.FeaturesConfigSubsystem (FeaturesConfigSubsystem, getDbFeatureRawInternal) +import Wire.LegalHold (computeLegalHoldFeatureStatus) +import Wire.LegalHoldStore (LegalHoldStore) data PermissionCheckArgs teamAssociation where PermissionCheckArgs :: @@ -144,3 +153,74 @@ checkConsent :: Sem r ConsentGiven checkConsent teamsOfUsers other = do consentGiven <$> getLHStatus (Map.lookup other teamsOfUsers) other + +-- | Ensure that a team has fewer members than the given limit (usually +-- @settings.maxTeamSize@). Returns the team size as it was before adding +-- anybody. +ensureNotTooLarge :: + ( Member BrigAPIAccess r, + Member (ErrorS 'TooManyTeamMembers) r + ) => + Word32 -> + TeamId -> + Sem r TeamSize +ensureNotTooLarge maxSize tid = do + tSize <- getSize tid + unless (tSize.teamSize < fromIntegral maxSize) $ + throwS @'TooManyTeamMembers + pure tSize + +-- | Ensure that a team doesn't exceed the member count limit for the LegalHold +-- feature. A team with more members than the fanout limit is too large, because +-- the fanout limit would prevent turning LegalHold feature _off_ again (for +-- details see 'Galley.API.LegalHold.removeSettings'). +-- +-- If LegalHold is configured for whitelisted teams only we consider the team +-- size unlimited, because we make the assumption that these teams won't turn +-- LegalHold off after activation. +-- FUTUREWORK: Find a way around the fanout limit. +ensureNotTooLargeForLegalHold :: + forall r. + ( Member LegalHoldStore r, + Member (ErrorS 'TooManyTeamMembersOnTeamWithLegalhold) r, + Member (Input FanoutLimit) r, + Member (Input (FeatureDefaults LegalholdConfig)) r, + Member FeaturesConfigSubsystem r + ) => + TeamId -> + Natural -> + Sem r () +ensureNotTooLargeForLegalHold tid teamSize = + whenM (isLegalHoldEnabledForTeam tid) $ + unlessM (teamSizeBelowLimit teamSize) $ + throwS @'TooManyTeamMembersOnTeamWithLegalhold + +isLegalHoldEnabledForTeam :: + forall r. + ( Member LegalHoldStore r, + Member FeaturesConfigSubsystem r, + Member (Input (FeatureDefaults LegalholdConfig)) r + ) => + TeamId -> + Sem r Bool +isLegalHoldEnabledForTeam tid = do + dbFeature <- getDbFeatureRawInternal tid + status <- computeLegalHoldFeatureStatus tid dbFeature + pure $ status == FeatureStatusEnabled + +teamSizeBelowLimit :: + ( Member (Input FanoutLimit) r, + Member (Input (FeatureDefaults LegalholdConfig)) r + ) => + Natural -> + Sem r Bool +teamSizeBelowLimit teamSize = do + limit <- fromIntegral . fromRange <$> input @FanoutLimit + let withinLimit = teamSize <= limit + featureLegalHold <- input @(FeatureDefaults LegalholdConfig) + case featureLegalHold of + FeatureLegalHoldDisabledPermanently -> pure withinLimit + FeatureLegalHoldDisabledByDefault -> pure withinLimit + FeatureLegalHoldWhitelistTeamsAndImplicitConsent -> + -- unlimited, see docs of 'ensureNotTooLargeForLegalHold' + pure True diff --git a/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs b/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs index f78029cebde..a05edddfe72 100644 --- a/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs +++ b/libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs @@ -313,8 +313,8 @@ internalFindTeamInvitationImpl (Just e) c = NotAllowed -> throwGuardFailed TeamInviteSetToNotAllowed maxSize <- maxTeamSize <$> input - teamSize <- teamSizeTotal <$> IndexedUserStore.getTeamSize tid - when (teamSize >= fromIntegral maxSize) $ + tSize <- (.teamSize) <$> IndexedUserStore.getTeamSize tid + when (tSize >= fromIntegral maxSize) $ throw UserSubsystemTooManyTeamMembers -- FUTUREWORK: The above can easily be done/tested in the intra call. -- Remove after the next release. diff --git a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/IndexedUserStore.hs b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/IndexedUserStore.hs index b77869840fe..1fefcaeeb36 100644 --- a/libs/wire-subsystems/test/unit/Wire/MockInterpreters/IndexedUserStore.hs +++ b/libs/wire-subsystems/test/unit/Wire/MockInterpreters/IndexedUserStore.hs @@ -87,12 +87,16 @@ inMemoryIndexedUserStoreInterpreter = error "IndexedUserStore: unimplemented in memory interpreter" GetTeamSize tid -> gets $ \index -> - let regulars = help [Just UserTypeRegular, Nothing] + let teamSize = help [Just UserTypeRegular, Nothing] apps = help [Just UserTypeApp] help allowedTypes = fromIntegral . length $ Map.filter (\(doc, _) -> doc.udTeam == Just tid && doc.udType `elem` allowedTypes) index.docs + collaborators = + fromIntegral + . length + $ Map.filter (\(doc, _) -> tid `elem` doc.udCollaboratingTeams) index.docs in TeamSize {..} upsertImpl :: (Member (State UserIndex) r) => ES.DocId -> UserDoc -> ES.VersionControl -> Sem r () diff --git a/services/brig/test/integration/API/Team.hs b/services/brig/test/integration/API/Team.hs index 66e1ead9e28..708fc1c7c46 100644 --- a/services/brig/test/integration/API/Team.hs +++ b/services/brig/test/integration/API/Team.hs @@ -154,7 +154,7 @@ testTeamSize brig req = do void $ get (req tid uid) - TeamId -> - Sem r Bool -isLegalHoldEnabledForTeam tid = do - dbFeature <- getDbFeatureRawInternal tid - status <- computeLegalHoldFeatureStatus tid dbFeature - pure $ status == FeatureStatusEnabled - ensureNotTooLargeToActivateLegalHold :: ( Member BrigAPIAccess r, Member (ErrorS 'CannotEnableLegalHoldServiceLargeTeam) r, @@ -80,27 +67,10 @@ ensureNotTooLargeToActivateLegalHold :: TeamId -> Sem r () ensureNotTooLargeToActivateLegalHold tid = do - teamSize <- getSize tid - unlessM (teamSizeBelowLimit teamSize) $ + tSize <- (.teamSize) <$> getSize tid + unlessM (teamSizeBelowLimit tSize) $ throwS @'CannotEnableLegalHoldServiceLargeTeam -teamSizeBelowLimit :: - ( Member (Input FanoutLimit) r, - Member (Input (FeatureDefaults LegalholdConfig)) r - ) => - TeamSize -> - Sem r Bool -teamSizeBelowLimit (fromIntegral . teamSizeTotal -> teamSize) = do - limit :: Int <- fromIntegral . fromRange <$> input @FanoutLimit - let withinLimit = teamSize <= limit - featureLegalHold <- input @(FeatureDefaults LegalholdConfig) - case featureLegalHold of - FeatureLegalHoldDisabledPermanently -> pure withinLimit - FeatureLegalHoldDisabledByDefault -> pure withinLimit - FeatureLegalHoldWhitelistTeamsAndImplicitConsent -> - -- unlimited, see docs of 'ensureNotTooLargeForLegalHold' - pure True - ensureReAuthorised :: ( Member BrigAPIAccess r, Member (Error AuthenticationError) r diff --git a/services/galley/src/Galley/API/Public/TeamMember.hs b/services/galley/src/Galley/API/Public/TeamMember.hs index 5c56816012e..87b467cbf0e 100644 --- a/services/galley/src/Galley/API/Public/TeamMember.hs +++ b/services/galley/src/Galley/API/Public/TeamMember.hs @@ -20,10 +20,14 @@ module Galley.API.Public.TeamMember where import Galley.API.Teams import Galley.API.Teams.Export qualified as Export import Galley.App +import Imports import Wire.API.Routes.API import Wire.API.Routes.Public.Galley.TeamMember import Wire.API.Team.Collaborator +import Wire.API.Team.Size +import Wire.BrigAPIAccess (getSize) import Wire.TeamCollaboratorsSubsystem +import Wire.TeamSubsystem qualified as TeamSubsystem teamMemberAPI :: API TeamMemberAPI GalleyEffects teamMemberAPI = @@ -36,7 +40,11 @@ teamMemberAPI = <@> mkNamedAPI @"update-team-member" updateTeamMember <@> mkNamedAPI @"get-team-members-csv" Export.getTeamMembersCSV <@> mkNamedAPI @"add-team-collaborator" - (\zuid tid (NewTeamCollaborator uid perms) -> createTeamCollaborator zuid uid tid perms) + ( \zuid tid (NewTeamCollaborator uid perms) -> do + n <- getSize tid + TeamSubsystem.ensureNotTooLargeForLegalHold tid (n.teamSize + n.apps + n.collaborators + 1) + createTeamCollaborator zuid uid tid perms + ) <@> mkNamedAPI @"get-team-collaborators" getAllTeamCollaborators <@> mkNamedAPI @"update-team-collaborator" updateTeamCollaborator <@> mkNamedAPI @"remove-team-collaborator" removeTeamCollaborator diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index 181a720ae8f..e84a0275da8 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -49,7 +49,6 @@ module Galley.API.Teams uncheckedUpdateTeamMember, userIsTeamOwner, canUserJoinTeam, - ensureNotTooLargeForLegalHold, ensureNotTooLargeToActivateLegalHold, internalDeleteBindingTeam, updateTeamCollaborator, @@ -279,8 +278,8 @@ updateTeamStatus tid (TeamStatusUpdate newStatus cur) = do -- We could also write `updateTeamSize 1 size 0` here, but it seems clearer to do it -- inline. teamSize <- do - (TeamSize numRegulars numApps) <- E.getSize tid - pure $ TeamSize (max 1 numRegulars) numApps + (TeamSize numRegulars numApps numCollaborators) <- E.getSize tid + pure $ TeamSize (max 1 numRegulars) numApps numCollaborators Journal.teamActivate tid teamSize c teamCreationTime runJournal _ _ = throwS @'InvalidTeamStatusUpdate validateTransition :: (Member (ErrorS 'InvalidTeamStatusUpdate) r) => (TeamStatus, TeamStatus) -> Sem r Bool @@ -787,8 +786,26 @@ deleteTeamMember' lusr zcon tid remove mBody = do Just u | u.userType == U.UserTypeApp -> UserTypeFilterApp _ -> UserTypeFilterRegular teamSizeAfterDelete <- do - before <- E.getSize tid - pure $ updateTeamSize uType before (-1) + before <- + -- ES may not be in sync with cassandra/postgres, eg., if we + -- add and remove a member very quickly. So, if we call + -- E.getSize here, we get the wrong answer, and this may + -- result in the `TeamSize` naturals to underflow (5xx error). + -- + -- Two solutions: (1) force-sync the index here (it doesn't + -- drift, the approximate value is only used for the + -- journal); (2) accept that `E.getSize` gives us an + -- approximation and circumvent the 5xx errors by + -- lower-bounding the fields before the substraction. + -- + -- We apply (2). + E.getSize tid <&> \s -> case uType of + UserTypeFilterRegular -> s {teamSize = max 1 s.teamSize} + UserTypeFilterApp -> s {apps = max 1 s.apps} + + pure case uType of + UserTypeFilterRegular -> before {teamSize = before.teamSize - 1} + UserTypeFilterApp -> before {apps = before.apps - 1} E.deleteUser remove case uType of UserTypeFilterRegular -> pure () @@ -1017,31 +1034,6 @@ ensureNotElevated targetPermissions member = ) $ throwS @'InvalidPermissions --- | Ensure that a team doesn't exceed the member count limit for the LegalHold --- feature. A team with more members than the fanout limit is too large, because --- the fanout limit would prevent turning LegalHold feature _off_ again (for --- details see 'Galley.API.LegalHold.removeSettings'). --- --- If LegalHold is configured for whitelisted teams only we consider the team --- size unlimited, because we make the assumption that these teams won't turn --- LegalHold off after activation. --- FUTUREWORK: Find a way around the fanout limit. -ensureNotTooLargeForLegalHold :: - forall r. - ( Member LegalHoldStore r, - Member (ErrorS 'TooManyTeamMembersOnTeamWithLegalhold) r, - Member (Input FanoutLimit) r, - Member (Input (FeatureDefaults LegalholdConfig)) r, - Member FeaturesConfigSubsystem r - ) => - TeamId -> - TeamSize -> - Sem r () -ensureNotTooLargeForLegalHold tid teamSize = - whenM (isLegalHoldEnabledForTeam tid) $ - unlessM (teamSizeBelowLimit teamSize) $ - throwS @'TooManyTeamMembersOnTeamWithLegalhold - addTeamMemberInternal :: ( Member E.BrigAPIAccess r, Member (ErrorS 'TooManyTeamMembers) r, @@ -1068,13 +1060,21 @@ addTeamMemberInternal tid origin originConn (ntmNewTeamMember -> new) = do Log.field "targets" (toByteString (new ^. userId)) . Log.field "action" (Log.val "Teams.addTeamMemberInternal") sizeAfterAdd <- do - n <- ensureNotTooLarge tid + maxSize <- inputs @Opts (^. settings . maxTeamSize) + n <- TeamSubsystem.ensureNotTooLarge maxSize tid uType <- E.getUser (new ^. userId) <&> \case Just u | u.userType == U.UserTypeApp -> UserTypeFilterApp _ -> UserTypeFilterRegular - pure $ updateTeamSize uType n 1 - ensureNotTooLargeForLegalHold tid sizeAfterAdd + pure case uType of + UserTypeFilterRegular -> n {teamSize = n.teamSize + 1} + UserTypeFilterApp -> + -- FUTUREWORK: this shouldn't happen, apps are not team + -- members! See also: + -- https://wearezeta.atlassian.net/browse/WPB-28095 + -- https://wearezeta.atlassian.net/browse/WPB-25521 + n {apps = n.apps + 1} + TeamSubsystem.ensureNotTooLargeForLegalHold tid (sizeAfterAdd.teamSize + sizeAfterAdd.apps + sizeAfterAdd.collaborators) admins <- E.getTeamAdmins tid let admins' = [new ^. userId | isAdminOrOwner (new ^. M.permissions)] <> admins @@ -1100,20 +1100,6 @@ addTeamMemberInternal tid origin originConn (ntmNewTeamMember -> new) = do APITeamQueue.pushTeamEvent tid e pure sizeAfterAdd - where - ensureNotTooLarge :: - ( Member E.BrigAPIAccess r, - Member (ErrorS 'TooManyTeamMembers) r, - Member (Input Opts) r - ) => - TeamId -> - Sem r TeamSize - ensureNotTooLarge teamid = do - o <- input - teamSize <- E.getSize teamid - unless (teamSizeTotal teamSize < fromIntegral (o ^. settings . maxTeamSize)) $ - throwS @'TooManyTeamMembers - pure teamSize getBindingTeamMembers :: ( Member (ErrorS 'TeamNotFound) r, @@ -1155,14 +1141,8 @@ canUserJoinTeam tid = do lhEnabled <- isLegalHoldEnabledForTeam tid when lhEnabled $ do sizeBeforeJoin <- E.getSize tid - let uType = - -- We do not have a `UserId` to check here. Also, - -- `canUserJoinTeam` is called by Brig during user - -- registration via invitation (POST /register), where apps - -- never go. So it is safe to assume "regular" - UserTypeFilterRegular - let sizeAfterJoin = updateTeamSize uType sizeBeforeJoin 1 - ensureNotTooLargeForLegalHold tid sizeAfterJoin + let sizeAfterJoin = sizeBeforeJoin {teamSize = sizeBeforeJoin.teamSize + 1} + TeamSubsystem.ensureNotTooLargeForLegalHold tid (sizeAfterJoin.teamSize + sizeAfterJoin.apps + sizeAfterJoin.collaborators) -- | Modify and get visibility type for a team (internal, no user permission checks) getSearchVisibilityInternal ::