From 8b591a1eadac8af16dc33fdb6110889569f100bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 5 Jan 2024 16:36:51 +0100 Subject: [PATCH 01/16] Introduce the feature flag This commit implements no business logic around the flag, but merely sets up the very basics needed to use the flag. --- cassandra-schema.cql | 1 + charts/galley/templates/configmap.yaml | 4 +++ charts/galley/values.yaml | 3 ++ hack/helm_vars/wire-server/values.yaml.gotmpl | 3 ++ libs/galley-types/src/Galley/Types/Teams.hs | 9 +++-- .../test/unit/Test/Galley/Types.hs | 1 + .../src/Wire/API/Routes/Internal/Galley.hs | 4 +++ .../Wire/API/Routes/Public/Galley/Feature.hs | 1 + libs/wire-api/src/Wire/API/Team/Feature.hs | 33 ++++++++++++++++++- services/galley/galley.cabal | 1 + services/galley/galley.integration.yaml | 3 ++ services/galley/src/Galley/API/Internal.hs | 3 ++ .../galley/src/Galley/API/Public/Feature.hs | 1 + .../galley/src/Galley/API/Teams/Features.hs | 2 ++ .../src/Galley/API/Teams/Features/Get.hs | 7 ++++ .../src/Galley/Cassandra/TeamFeatures.hs | 4 +++ services/galley/src/Galley/Schema/Run.hs | 4 ++- ...V91_TeamMemberDeletedLimitedEventFanout.hs | 30 +++++++++++++++++ .../test/integration/API/Teams/Feature.hs | 4 ++- 19 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 services/galley/src/Galley/Schema/V91_TeamMemberDeletedLimitedEventFanout.hs diff --git a/cassandra-schema.cql b/cassandra-schema.cql index 596495b57c..efcf342403 100644 --- a/cassandra-schema.cql +++ b/cassandra-schema.cql @@ -1200,6 +1200,7 @@ CREATE TABLE galley_test.team_features ( guest_links_lock_status int, guest_links_status int, legalhold_status int, + limited_event_fanout_status int, mls_allowed_ciphersuites set, mls_default_ciphersuite int, mls_default_protocol int, diff --git a/charts/galley/templates/configmap.yaml b/charts/galley/templates/configmap.yaml index 2e839ce1c5..1b8da54d73 100644 --- a/charts/galley/templates/configmap.yaml +++ b/charts/galley/templates/configmap.yaml @@ -141,5 +141,9 @@ data: mlsMigration: {{- toYaml .settings.featureFlags.mlsMigration | nindent 10 }} {{- end }} + {{- if .settings.featureFlags.limitedEventFanout }} + limitedEventFanout: + {{- toYaml .settings.featureFlags.limitedEventFanout | nindent 10 }} + {{- end }} {{- end }} {{- end }} diff --git a/charts/galley/values.yaml b/charts/galley/values.yaml index b81aa2df63..9cfcc287d1 100644 --- a/charts/galley/values.yaml +++ b/charts/galley/values.yaml @@ -132,6 +132,9 @@ config: usersThreshold: 100 clientsThreshold: 100 lockStatus: locked + limitedEventFanout: + defaults: + status: disabled aws: region: "eu-west-1" diff --git a/hack/helm_vars/wire-server/values.yaml.gotmpl b/hack/helm_vars/wire-server/values.yaml.gotmpl index 02c8c47499..538a9df985 100644 --- a/hack/helm_vars/wire-server/values.yaml.gotmpl +++ b/hack/helm_vars/wire-server/values.yaml.gotmpl @@ -235,6 +235,9 @@ galley: usersThreshold: 100 clientsThreshold: 50 lockStatus: locked + limitedEventFanout: + defaults: + status: disabled journal: endpoint: http://fake-aws-sqs:4568 queueName: integration-team-events.fifo diff --git a/libs/galley-types/src/Galley/Types/Teams.hs b/libs/galley-types/src/Galley/Types/Teams.hs index 38d16b768a..ef54abdd5d 100644 --- a/libs/galley-types/src/Galley/Types/Teams.hs +++ b/libs/galley-types/src/Galley/Types/Teams.hs @@ -43,6 +43,7 @@ module Galley.Types.Teams flagMlsE2EId, flagMlsMigration, flagEnforceFileDownloadLocation, + flagLimitedEventFanout, Defaults (..), ImplicitLockStatus (..), unImplicitLockStatus, @@ -167,7 +168,8 @@ data FeatureFlags = FeatureFlags _flagOutlookCalIntegration :: !(Defaults (WithStatus OutlookCalIntegrationConfig)), _flagMlsE2EId :: !(Defaults (WithStatus MlsE2EIdConfig)), _flagMlsMigration :: !(Defaults (WithStatus MlsMigrationConfig)), - _flagEnforceFileDownloadLocation :: !(Defaults (WithStatus EnforceFileDownloadLocationConfig)) + _flagEnforceFileDownloadLocation :: !(Defaults (WithStatus EnforceFileDownloadLocationConfig)), + _flagLimitedEventFanout :: !(Defaults (ImplicitLockStatus LimitedEventFanoutConfig)) } deriving (Eq, Show, Generic) @@ -221,6 +223,7 @@ instance FromJSON FeatureFlags where <*> (fromMaybe (Defaults (defFeatureStatus @MlsE2EIdConfig)) <$> (obj .:? "mlsE2EId")) <*> (fromMaybe (Defaults (defFeatureStatus @MlsMigrationConfig)) <$> (obj .:? "mlsMigration")) <*> (fromMaybe (Defaults (defFeatureStatus @EnforceFileDownloadLocationConfig)) <$> (obj .:? "enforceFileDownloadLocation")) + <*> withImplicitLockStatusOrDefault obj "limitedEventFanout" where withImplicitLockStatusOrDefault :: forall cfg. (IsFeatureConfig cfg, Schema.ToSchema cfg) => Object -> Key -> A.Parser (Defaults (ImplicitLockStatus cfg)) withImplicitLockStatusOrDefault obj fieldName = fromMaybe (Defaults (ImplicitLockStatus (defFeatureStatus @cfg))) <$> obj .:? fieldName @@ -245,6 +248,7 @@ instance ToJSON FeatureFlags where mlsE2EId mlsMigration enforceFileDownloadLocation + teamMemberDeletedLimitedEventFanout ) = object [ "sso" .= sso, @@ -263,7 +267,8 @@ instance ToJSON FeatureFlags where "outlookCalIntegration" .= outlookCalIntegration, "mlsE2EId" .= mlsE2EId, "mlsMigration" .= mlsMigration, - "enforceFileDownloadLocation" .= enforceFileDownloadLocation + "enforceFileDownloadLocation" .= enforceFileDownloadLocation, + "limitedEventFanout" .= teamMemberDeletedLimitedEventFanout ] instance FromJSON FeatureSSO where diff --git a/libs/galley-types/test/unit/Test/Galley/Types.hs b/libs/galley-types/test/unit/Test/Galley/Types.hs index 1e6c740d01..09e8ab6c38 100644 --- a/libs/galley-types/test/unit/Test/Galley/Types.hs +++ b/libs/galley-types/test/unit/Test/Galley/Types.hs @@ -100,6 +100,7 @@ instance Arbitrary FeatureFlags where <*> arbitrary <*> arbitrary <*> arbitrary + <*> arbitrary where unlocked :: ImplicitLockStatus a -> ImplicitLockStatus a unlocked = ImplicitLockStatus . Public.setLockStatus Public.LockStatusUnlocked . _unImplicitLockStatus diff --git a/libs/wire-api/src/Wire/API/Routes/Internal/Galley.hs b/libs/wire-api/src/Wire/API/Routes/Internal/Galley.hs index cc930367b0..f03b19870b 100644 --- a/libs/wire-api/src/Wire/API/Routes/Internal/Galley.hs +++ b/libs/wire-api/src/Wire/API/Routes/Internal/Galley.hs @@ -165,6 +165,10 @@ type IFeatureAPI = :<|> IFeatureStatusPutWithDesc '[] '() EnforceFileDownloadLocationConfig "

Custom feature: only supported for some decidated on-prem systems.

" :<|> IFeatureStatusPatchWithDesc '[] '() EnforceFileDownloadLocationConfig "

Custom feature: only supported for some decidated on-prem systems.

" :<|> IFeatureStatusLockStatusPutWithDesc EnforceFileDownloadLocationConfig "

Custom feature: only supported for some decidated on-prem systems.

" + -- LimitedEventFanoutConfig + :<|> IFeatureStatusGet LimitedEventFanoutConfig + :<|> IFeatureStatusPut '[] '() LimitedEventFanoutConfig + :<|> IFeatureStatusPatch '[] '() LimitedEventFanoutConfig -- all feature configs :<|> Named "feature-configs-internal" diff --git a/libs/wire-api/src/Wire/API/Routes/Public/Galley/Feature.hs b/libs/wire-api/src/Wire/API/Routes/Public/Galley/Feature.hs index 69a40c0312..3dab419273 100644 --- a/libs/wire-api/src/Wire/API/Routes/Public/Galley/Feature.hs +++ b/libs/wire-api/src/Wire/API/Routes/Public/Galley/Feature.hs @@ -101,6 +101,7 @@ type FeatureAPI = '() EnforceFileDownloadLocationConfig "

Custom feature: only supported for some decidated on-prem systems.

" + :<|> From 'V5 ::> FeatureStatusGet LimitedEventFanoutConfig :<|> AllFeatureConfigsUserGet :<|> AllFeatureConfigsTeamGet :<|> FeatureConfigDeprecatedGet "The usage of this endpoint was removed in iOS in version 3.101. It is not used by team management, or webapp, and is potentially used by the old Android client as of June 2022" LegalholdConfig diff --git a/libs/wire-api/src/Wire/API/Team/Feature.hs b/libs/wire-api/src/Wire/API/Team/Feature.hs index 98cca4dffc..8dd48682f7 100644 --- a/libs/wire-api/src/Wire/API/Team/Feature.hs +++ b/libs/wire-api/src/Wire/API/Team/Feature.hs @@ -82,6 +82,7 @@ module Wire.API.Team.Feature MlsE2EIdConfig (..), MlsMigrationConfig (..), EnforceFileDownloadLocationConfig (..), + LimitedEventFanoutConfig (..), AllFeatureConfigs (..), unImplicitLockStatus, ImplicitLockStatus (..), @@ -211,6 +212,7 @@ data FeatureSingleton cfg where -- all other constructors) FeatureSingleton MlsMigrationConfig FeatureSingletonEnforceFileDownloadLocationConfig :: FeatureSingleton EnforceFileDownloadLocationConfig + FeatureSingletonLimitedEventFanoutConfig :: FeatureSingleton LimitedEventFanoutConfig class FeatureTrivialConfig cfg where trivialConfig :: cfg @@ -1118,6 +1120,32 @@ instance IsFeatureConfig EnforceFileDownloadLocationConfig where featureSingleton = FeatureSingletonEnforceFileDownloadLocationConfig objectSchema = field "config" schema +---------------------------------------------------------------------- +-- Guarding the fanout of events when a team member is deleted. +-- +-- FUTUREWORK: This is a transient flag that is to be removed after about 6 +-- months of its introduction, namely once all clients get a chance to adapt to +-- a limited event fanout. + +data LimitedEventFanoutConfig = LimitedEventFanoutConfig + deriving stock (Eq, Show, Generic) + deriving (Arbitrary) via (GenericUniform LimitedEventFanoutConfig) + +instance RenderableSymbol LimitedEventFanoutConfig where + renderSymbol = "LimitedEventFanoutConfig" + +instance IsFeatureConfig LimitedEventFanoutConfig where + type FeatureSymbol LimitedEventFanoutConfig = "limitedEventFanout" + defFeatureStatus = withStatus FeatureStatusEnabled LockStatusUnlocked LimitedEventFanoutConfig FeatureTTLUnlimited + featureSingleton = FeatureSingletonLimitedEventFanoutConfig + objectSchema = pure LimitedEventFanoutConfig + +instance ToSchema LimitedEventFanoutConfig where + schema = object "LimitedEventFanoutConfig" objectSchema + +instance FeatureTrivialConfig LimitedEventFanoutConfig where + trivialConfig = LimitedEventFanoutConfig + ---------------------------------------------------------------------- -- FeatureStatus @@ -1196,7 +1224,8 @@ data AllFeatureConfigs = AllFeatureConfigs afcOutlookCalIntegration :: WithStatus OutlookCalIntegrationConfig, afcMlsE2EId :: WithStatus MlsE2EIdConfig, afcMlsMigration :: WithStatus MlsMigrationConfig, - afcEnforceFileDownloadLocation :: WithStatus EnforceFileDownloadLocationConfig + afcEnforceFileDownloadLocation :: WithStatus EnforceFileDownloadLocationConfig, + afcLimitedEventFanout :: WithStatus LimitedEventFanoutConfig } deriving stock (Eq, Show) deriving (FromJSON, ToJSON, S.ToSchema) via (Schema AllFeatureConfigs) @@ -1224,6 +1253,7 @@ instance ToSchema AllFeatureConfigs where <*> afcMlsE2EId .= featureField <*> afcMlsMigration .= featureField <*> afcEnforceFileDownloadLocation .= featureField + <*> afcLimitedEventFanout .= featureField where featureField :: forall cfg. @@ -1253,5 +1283,6 @@ instance Arbitrary AllFeatureConfigs where <*> arbitrary <*> arbitrary <*> arbitrary + <*> arbitrary makeLenses ''ImplicitLockStatus diff --git a/services/galley/galley.cabal b/services/galley/galley.cabal index 941488970c..1c40172da3 100644 --- a/services/galley/galley.cabal +++ b/services/galley/galley.cabal @@ -276,6 +276,7 @@ library Galley.Schema.V88_RemoveMemberClientAndTruncateMLSGroupMemberClient Galley.Schema.V89_MlsLockStatus Galley.Schema.V90_EnforceFileDownloadLocationConfig + Galley.Schema.V91_TeamMemberDeletedLimitedEventFanout Galley.Types.Clients Galley.Types.ToUserRole Galley.Types.UserList diff --git a/services/galley/galley.integration.yaml b/services/galley/galley.integration.yaml index cb4a014a1e..f73ed5ca06 100644 --- a/services/galley/galley.integration.yaml +++ b/services/galley/galley.integration.yaml @@ -93,6 +93,9 @@ settings: usersThreshold: 100 clientsThreshold: 50 lockStatus: locked + limitedEventFanout: + defaults: + status: disabled logLevel: Warn logNetStrings: false diff --git a/services/galley/src/Galley/API/Internal.hs b/services/galley/src/Galley/API/Internal.hs index 7f3bcf4de4..ff038a0b39 100644 --- a/services/galley/src/Galley/API/Internal.hs +++ b/services/galley/src/Galley/API/Internal.hs @@ -239,6 +239,9 @@ featureAPI = <@> mkNamedAPI @'("iput", EnforceFileDownloadLocationConfig) setFeatureStatusInternal <@> mkNamedAPI @'("ipatch", EnforceFileDownloadLocationConfig) patchFeatureStatusInternal <@> mkNamedAPI @'("ilock", EnforceFileDownloadLocationConfig) (updateLockStatus @EnforceFileDownloadLocationConfig) + <@> mkNamedAPI @'("iget", LimitedEventFanoutConfig) (getFeatureStatus DontDoAuth) + <@> mkNamedAPI @'("iput", LimitedEventFanoutConfig) setFeatureStatusInternal + <@> mkNamedAPI @'("ipatch", LimitedEventFanoutConfig) patchFeatureStatusInternal <@> mkNamedAPI @"feature-configs-internal" (maybe getAllFeatureConfigsForServer getAllFeatureConfigsForUser) waiInternalSitemap :: Routes a (Sem GalleyEffects) () diff --git a/services/galley/src/Galley/API/Public/Feature.hs b/services/galley/src/Galley/API/Public/Feature.hs index f2d2cd19e2..61fc38c87b 100644 --- a/services/galley/src/Galley/API/Public/Feature.hs +++ b/services/galley/src/Galley/API/Public/Feature.hs @@ -67,6 +67,7 @@ featureAPI = <@> mkNamedAPI @'("put", MlsMigrationConfig) (setFeatureStatus . DoAuth) <@> mkNamedAPI @'("get", EnforceFileDownloadLocationConfig) (getFeatureStatus . DoAuth) <@> mkNamedAPI @'("put", EnforceFileDownloadLocationConfig) (setFeatureStatus . DoAuth) + <@> mkNamedAPI @'("get", LimitedEventFanoutConfig) (getFeatureStatus . DoAuth) <@> mkNamedAPI @"get-all-feature-configs-for-user" getAllFeatureConfigsForUser <@> mkNamedAPI @"get-all-feature-configs-for-team" getAllFeatureConfigsForTeam <@> mkNamedAPI @'("get-config", LegalholdConfig) getFeatureStatusForUser diff --git a/services/galley/src/Galley/API/Teams/Features.hs b/services/galley/src/Galley/API/Teams/Features.hs index 56e6ea7168..b00b2c6f1d 100644 --- a/services/galley/src/Galley/API/Teams/Features.hs +++ b/services/galley/src/Galley/API/Teams/Features.hs @@ -395,3 +395,5 @@ instance SetFeatureConfig MlsMigrationConfig where persistAndPushEvent tid wsnl instance SetFeatureConfig EnforceFileDownloadLocationConfig + +instance SetFeatureConfig LimitedEventFanoutConfig diff --git a/services/galley/src/Galley/API/Teams/Features/Get.hs b/services/galley/src/Galley/API/Teams/Features/Get.hs index d9525c1386..c2d61f92aa 100644 --- a/services/galley/src/Galley/API/Teams/Features/Get.hs +++ b/services/galley/src/Galley/API/Teams/Features/Get.hs @@ -239,6 +239,7 @@ getAllFeatureConfigsForServer = <*> getConfigForServer @MlsE2EIdConfig <*> getConfigForServer @MlsMigrationConfig <*> getConfigForServer @EnforceFileDownloadLocationConfig + <*> getConfigForServer @LimitedEventFanoutConfig getAllFeatureConfigsUser :: forall r. @@ -274,6 +275,7 @@ getAllFeatureConfigsUser uid = <*> getConfigForUser @MlsE2EIdConfig uid <*> getConfigForUser @MlsMigrationConfig uid <*> getConfigForUser @EnforceFileDownloadLocationConfig uid + <*> getConfigForUser @LimitedEventFanoutConfig uid getAllFeatureConfigsTeam :: forall r. @@ -305,6 +307,7 @@ getAllFeatureConfigsTeam tid = <*> getConfigForTeam @MlsE2EIdConfig tid <*> getConfigForTeam @MlsMigrationConfig tid <*> getConfigForTeam @EnforceFileDownloadLocationConfig tid + <*> getConfigForTeam @LimitedEventFanoutConfig tid -- | Note: this is an internal function which doesn't cover all features, e.g. LegalholdConfig genericGetConfigForTeam :: @@ -497,6 +500,10 @@ instance GetFeatureConfig EnforceFileDownloadLocationConfig where getConfigForServer = input <&> view (settings . featureFlags . flagEnforceFileDownloadLocation . unDefaults) +instance GetFeatureConfig LimitedEventFanoutConfig where + getConfigForServer = + input <&> view (settings . featureFlags . flagLimitedEventFanout . unDefaults . unImplicitLockStatus) + -- | If second factor auth is enabled, make sure that end-points that don't support it, but -- should, are blocked completely. (This is a workaround until we have 2FA for those -- end-points as well.) diff --git a/services/galley/src/Galley/Cassandra/TeamFeatures.hs b/services/galley/src/Galley/Cassandra/TeamFeatures.hs index 21eb3e0d06..8e33a267a0 100644 --- a/services/galley/src/Galley/Cassandra/TeamFeatures.hs +++ b/services/galley/src/Galley/Cassandra/TeamFeatures.hs @@ -169,6 +169,8 @@ getFeatureConfig FeatureSingletonEnforceFileDownloadLocationConfig tid = do where select :: PrepQuery R (Identity TeamId) (Maybe FeatureStatus, Maybe Text) select = "select enforce_file_download_location_status, enforce_file_download_location from team_features where team_id = ?" +getFeatureConfig FeatureSingletonLimitedEventFanoutConfig tid = + getTrivialConfigC "limited_event_fanout_status" tid setFeatureConfig :: MonadClient m => FeatureSingleton cfg -> TeamId -> WithStatusNoLock cfg -> m () setFeatureConfig FeatureSingletonLegalholdConfig tid statusNoLock = setFeatureStatusC "legalhold_status" tid (wssStatus statusNoLock) @@ -265,6 +267,8 @@ setFeatureConfig FeatureSingletonEnforceFileDownloadLocationConfig tid status = insert :: PrepQuery W (TeamId, FeatureStatus, Maybe Text) () insert = "insert into team_features (team_id, enforce_file_download_location_status, enforce_file_download_location) values (?, ?, ?)" +setFeatureConfig FeatureSingletonLimitedEventFanoutConfig tid statusNoLock = + setFeatureStatusC "limited_event_fanout_status" tid (wssStatus statusNoLock) getFeatureLockStatus :: MonadClient m => FeatureSingleton cfg -> TeamId -> m (Maybe LockStatus) getFeatureLockStatus FeatureSingletonFileSharingConfig tid = getLockStatusC "file_sharing_lock_status" tid diff --git a/services/galley/src/Galley/Schema/Run.hs b/services/galley/src/Galley/Schema/Run.hs index 4905617fa6..51e2941703 100644 --- a/services/galley/src/Galley/Schema/Run.hs +++ b/services/galley/src/Galley/Schema/Run.hs @@ -91,6 +91,7 @@ import Galley.Schema.V87_TeamFeatureSupportedProtocols qualified as V87_TeamFeat import Galley.Schema.V88_RemoveMemberClientAndTruncateMLSGroupMemberClient qualified as V88_RemoveMemberClientAndTruncateMLSGroupMemberClient import Galley.Schema.V89_MlsLockStatus qualified as V89_MlsLockStatus import Galley.Schema.V90_EnforceFileDownloadLocationConfig qualified as V90_EnforceFileDownloadLocationConfig +import Galley.Schema.V91_TeamMemberDeletedLimitedEventFanout qualified as V91_TeamMemberDeletedLimitedEventFanout import Imports import Options.Applicative import System.Logger.Extended qualified as Log @@ -182,7 +183,8 @@ migrations = V87_TeamFeatureSupportedProtocols.migration, V88_RemoveMemberClientAndTruncateMLSGroupMemberClient.migration, V89_MlsLockStatus.migration, - V90_EnforceFileDownloadLocationConfig.migration + V90_EnforceFileDownloadLocationConfig.migration, + V91_TeamMemberDeletedLimitedEventFanout.migration -- FUTUREWORK: once #1726 has made its way to master/production, -- the 'message' field in connections table can be dropped. -- See also https://github.com/wireapp/wire-server/pull/1747/files diff --git a/services/galley/src/Galley/Schema/V91_TeamMemberDeletedLimitedEventFanout.hs b/services/galley/src/Galley/Schema/V91_TeamMemberDeletedLimitedEventFanout.hs new file mode 100644 index 0000000000..3ad5f31439 --- /dev/null +++ b/services/galley/src/Galley/Schema/V91_TeamMemberDeletedLimitedEventFanout.hs @@ -0,0 +1,30 @@ +-- This file is part of the Wire Server implementation. +-- +-- 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 +-- 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 Galley.Schema.V91_TeamMemberDeletedLimitedEventFanout where + +import Cassandra.Schema +import Imports +import Text.RawString.QQ + +migration :: Migration +migration = + Migration 91 "Add a field for the team member deleted limited event fanout" $ + schema' + [r| ALTER TABLE team_features ADD ( + limited_event_fanout_status int + ) + |] diff --git a/services/galley/test/integration/API/Teams/Feature.hs b/services/galley/test/integration/API/Teams/Feature.hs index a6d8955e38..f0a522d365 100644 --- a/services/galley/test/integration/API/Teams/Feature.hs +++ b/services/galley/test/integration/API/Teams/Feature.hs @@ -1063,7 +1063,9 @@ testAllFeatures = do afcOutlookCalIntegration = withStatus FeatureStatusDisabled LockStatusLocked OutlookCalIntegrationConfig FeatureTTLUnlimited, afcMlsE2EId = withStatus FeatureStatusDisabled LockStatusUnlocked (wsConfig defFeatureStatus) FeatureTTLUnlimited, afcMlsMigration = defaultMlsMigrationConfig, - afcEnforceFileDownloadLocation = defaultEnforceFileDownloadLocationConfig + afcEnforceFileDownloadLocation = defaultEnforceFileDownloadLocationConfig, + afcLimitedEventFanout = + withStatus FeatureStatusDisabled LockStatusLocked LimitedEventFanoutConfig FeatureTTLUnlimited } testFeatureConfigConsistency :: TestM () From 9bb42c2c2fb7f1488d716eced3841a4347f757be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 9 Jan 2024 11:21:19 +0100 Subject: [PATCH 02/16] Document the feature flag --- .../src/developer/reference/config-options.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/src/developer/reference/config-options.md b/docs/src/developer/reference/config-options.md index a5bc057a45..6e5fd477d8 100644 --- a/docs/src/developer/reference/config-options.md +++ b/docs/src/developer/reference/config-options.md @@ -425,7 +425,7 @@ federator: clientPrivateKey: client-key.pem ``` -### Outlook calalendar integration +### Outlook calendar integration This feature setting only applies to the Outlook Calendar extension for Wire. As it is an external service, it should only be configured through this feature flag and otherwise ignored by the backend. @@ -450,6 +450,23 @@ config: GuestLinkTTLSeconds: 604800 ``` +### Limited Event Fanout + +To maintain compatibility with clients and their versions that do not implement +the limited event fanout when a team member is deleted, the limited event fanout +flag is used. Its default value `disabled` means that the old-style full event +fanout will take place when a team member is deleted. Set the flag to `enabled` +to send team events only to team owners and administrators. + +Example configuration: + +```yaml +# galley.yaml +limitedEventFanout: + defaults: + status: disabled +``` + ## Settings in brig Some features (as of the time of writing this: only From 4460b55340f0678f9a22261b0cf6484e8e31d01e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 9 Jan 2024 14:43:36 +0100 Subject: [PATCH 03/16] Guard member deleted event fanout --- services/galley/src/Galley/API/Internal.hs | 2 +- services/galley/src/Galley/API/Teams.hs | 40 +++++++++++++++++++--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/services/galley/src/Galley/API/Internal.hs b/services/galley/src/Galley/API/Internal.hs index ff038a0b39..1ed073b333 100644 --- a/services/galley/src/Galley/API/Internal.hs +++ b/services/galley/src/Galley/API/Internal.hs @@ -344,7 +344,7 @@ rmUser lusr conn = do leaveTeams page = for_ (pageItems page) $ \tid -> do admins <- E.getTeamAdmins tid - uncheckedDeleteTeamMember lusr conn tid (tUnqualified lusr) admins + uncheckedDeleteTeamMember lusr conn tid (tUnqualified lusr) (Left admins) page' <- listTeams @p2 (tUnqualified lusr) (Just (pageState page)) maxBound leaveTeams page' diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index e907e4883d..2dfd60e3d8 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -86,6 +86,7 @@ import Data.Time.Clock (UTCTime) import Galley.API.Action import Galley.API.Error as Galley import Galley.API.LegalHold.Team +import Galley.API.Teams.Features.Get import Galley.API.Teams.Notifications qualified as APITeamQueue import Galley.API.Update qualified as API import Galley.API.Util @@ -141,6 +142,7 @@ import Wire.API.Team qualified as Public import Wire.API.Team.Conversation import Wire.API.Team.Conversation qualified as Public import Wire.API.Team.Export (TeamExportUser (..)) +import Wire.API.Team.Feature import Wire.API.Team.Member import Wire.API.Team.Member qualified as M import Wire.API.Team.Member qualified as Public @@ -889,9 +891,11 @@ deleteTeamMember :: Member (ErrorS 'NotATeamMember) r, Member (ErrorS OperationDenied) r, Member ExternalAccess r, + Member (Input Opts) r, Member (Input UTCTime) r, Member GundeckAccess r, Member MemberStore r, + Member TeamFeatureStore r, Member TeamStore r, Member P.TinyLog r ) => @@ -915,9 +919,11 @@ deleteNonBindingTeamMember :: Member (ErrorS 'NotATeamMember) r, Member (ErrorS OperationDenied) r, Member ExternalAccess r, + Member (Input Opts) r, Member (Input UTCTime) r, Member GundeckAccess r, Member MemberStore r, + Member TeamFeatureStore r, Member TeamStore r, Member P.TinyLog r ) => @@ -941,9 +947,11 @@ deleteTeamMember' :: Member (ErrorS 'NotATeamMember) r, Member (ErrorS OperationDenied) r, Member ExternalAccess r, + Member (Input Opts) r, Member (Input UTCTime) r, Member GundeckAccess r, Member MemberStore r, + Member TeamFeatureStore r, Member TeamStore r, Member P.TinyLog r ) => @@ -982,8 +990,13 @@ deleteTeamMember' lusr zcon tid remove mBody = do Journal.teamUpdate tid sizeAfterDelete $ filter (/= remove) owners pure TeamMemberDeleteAccepted else do - admins <- E.getTeamAdmins tid - uncheckedDeleteTeamMember lusr (Just zcon) tid remove admins + wsStatus <$> getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid >>= \case + FeatureStatusEnabled -> do + admins <- E.getTeamAdmins tid + uncheckedDeleteTeamMember lusr (Just zcon) tid remove (Left admins) + FeatureStatusDisabled -> do + mems <- getTeamMembersForFanout tid + uncheckedDeleteTeamMember lusr (Just zcon) tid remove (Right mems) pure TeamMemberDeleteCompleted -- This function is "unchecked" because it does not validate that the user has the `RemoveTeamMember` permission. @@ -1002,9 +1015,9 @@ uncheckedDeleteTeamMember :: Maybe ConnId -> TeamId -> UserId -> - [UserId] -> + Either [UserId] TeamMemberList -> Sem r () -uncheckedDeleteTeamMember lusr zcon tid remove admins = do +uncheckedDeleteTeamMember lusr zcon tid remove (Left admins) = do now <- input pushMemberLeaveEvent now E.deleteTeamMember tid remove @@ -1022,6 +1035,25 @@ uncheckedDeleteTeamMember lusr zcon tid remove admins = do (filter (/= (tUnqualified lusr)) admins) E.push1 $ newPushLocal1 ListComplete (tUnqualified lusr) (TeamEvent e) r & pushConn .~ zcon & pushTransient .~ True +uncheckedDeleteTeamMember lusr zcon tid remove (Right mems) = do + now <- input + pushMemberLeaveEventToAll now + E.deleteTeamMember tid remove + -- notify all conversation members not in this team. + removeFromConvsAndPushConvLeaveEvent lusr zcon tid remove + where + -- notify all team members. This is to maintain compatibility with clients + -- relying on these events, but eventually they will catch up and this + -- function, and the corresponding feature flag, will be ready for removal. + pushMemberLeaveEventToAll :: UTCTime -> Sem r () + pushMemberLeaveEventToAll now = do + let e = newEvent tid now (EdMemberLeave remove) + let r = + list1 + (userRecipient (tUnqualified lusr)) + (membersToRecipients (Just (tUnqualified lusr)) (mems ^. teamMembers)) + E.push1 $ + newPushLocal1 (mems ^. teamMemberListType) (tUnqualified lusr) (TeamEvent e) r & pushConn .~ zcon removeFromConvsAndPushConvLeaveEvent :: forall r. From cdb87d2c491154329926d0e6de941b4c9b32063a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Wed, 10 Jan 2024 10:35:23 +0100 Subject: [PATCH 04/16] Test: Limited event fanout This extends an existing test case that deletes a team member, but now explicitly enabling the limited event fanout feature flag. --- integration/test/Test/Conversation.hs | 69 ++++++++++++++++++------- services/galley/src/Galley/API/Teams.hs | 13 ++++- 2 files changed, 60 insertions(+), 22 deletions(-) diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index 601bf33dda..2fbbdcd383 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -660,36 +660,65 @@ testDeleteTeamConversationWithUnreachableRemoteMembers = do notif <- awaitNotification bob bobClient noValue isConvDeleteNotif assertNotification notif -testDeleteTeamMember :: HasCallStack => App () -testDeleteTeamMember = do - (alice, team, [alex]) <- createTeam OwnDomain 2 +testDeleteTeamMemberLimitedEventFanout :: HasCallStack => App () +testDeleteTeamMemberLimitedEventFanout = do + -- Alex will get removed from the team + (alice, team, [alex, alison]) <- createTeam OwnDomain 3 + ana <- createTeamMemberWithRole alice team "admin" [amy, bob] <- for [OwnDomain, OtherDomain] $ flip randomUser def forM_ [amy, bob] $ connectTwoUsers alice - [aliceId, alexId, amyId, bobId] <- - forM [alice, alex, amy, bob] (%. "qualified_id") - let nc = (defProteus {qualifiedUsers = [alexId, amyId, bobId], team = Just team}) + [aliceId, alexId, amyId, alisonId, anaId, bobId] <- do + forM [alice, alex, amy, alison, ana, bob] (%. "qualified_id") + let nc = + ( defProteus + { qualifiedUsers = + [alexId, amyId, alisonId, anaId, bobId], + team = Just team + } + ) conv <- postConversation alice nc >>= getJSON 201 - withWebSockets [alice, amy, bob] $ \[wsAlice, wsAmy, wsBob] -> do - void $ deleteTeamMember team alice alex >>= getBody 202 - assertConvLeaveNotif wsAmy alexId - do - n <- awaitMatch isTeamMemberLeaveNotif wsAlice + memsBefore <- getMembers team aliceId + + -- Only the team admins will get the team-level event about Alex being removed + -- from the team + setTeamFeatureStatus OwnDomain team "limitedEventFanout" "enabled" + + withWebSockets [alice, amy, bob, alison, ana] $ + \[wsAlice, wsAmy, wsBob, wsAlison, wsAna] -> do + void $ deleteTeamMember team alice alex >>= getBody 202 + + memsAfter <- getMembers team aliceId + memsAfter `shouldNotMatch` memsBefore + + assertConvLeaveNotif wsAmy alexId + assertConvLeaveNotif wsAlison alexId + alexUId <- alex %. "id" - nPayload n %. "data.user" `shouldMatch` alexUId - assertConvLeaveNotif wsAlice alexId - do - bindResponse (getConversation bob conv) $ \resp -> do - resp.status `shouldMatchInt` 200 - mems <- resp.json %. "members.others" & asList - memIds <- forM mems (%. "qualified_id") - memIds `shouldMatchSet` [aliceId, amyId] - assertConvLeaveNotif wsBob alexId + do + n <- awaitMatch isTeamMemberLeaveNotif wsAlice + nPayload n %. "data.user" `shouldMatch` alexUId + assertConvLeaveNotif wsAlice alexId + do + n <- awaitMatch isTeamMemberLeaveNotif wsAna + nPayload n %. "data.user" `shouldMatch` alexUId + assertConvLeaveNotif wsAna alexId + do + bindResponse (getConversation bob conv) $ \resp -> do + resp.status `shouldMatchInt` 200 + mems <- resp.json %. "members.others" & asList + memIds <- forM mems (%. "qualified_id") + memIds `shouldMatchSet` [aliceId, alisonId, amyId, anaId] + assertConvLeaveNotif wsBob alexId where assertConvLeaveNotif :: MakesValue leaverId => WebSocket -> leaverId -> App () assertConvLeaveNotif ws leaverId = do n <- awaitMatch isConvLeaveNotif ws nPayload n %. "data.qualified_user_ids.0" `shouldMatch` leaverId nPayload n %. "data.reason" `shouldMatch` "user-deleted" + getMembers tid usr = bindResponse (getTeamMembers usr tid) $ \resp -> do + resp.status `shouldMatchInt` 200 + ms <- resp.json %. "members" & asList + forM ms $ (%. "user") testLeaveConversationSuccess :: HasCallStack => App () testLeaveConversationSuccess = do diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index 2dfd60e3d8..2ceabc963f 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -986,8 +986,17 @@ deleteTeamMember' lusr zcon tid remove mBody = do then 0 else sizeBeforeDelete - 1 E.deleteUser remove - owners <- E.getBillingTeamMembers tid - Journal.teamUpdate tid sizeAfterDelete $ filter (/= remove) owners + toNotify <- + wsStatus <$> getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid >>= \case + FeatureStatusEnabled -> E.getBillingTeamMembers tid + FeatureStatusDisabled -> do + let filterFromMembers list = + view userId <$> filter (`hasPermission` SetBilling) (list ^. teamMembers) + -- fmap (view userId) $ (list ^. teamMembers) + mems <- getTeamMembersForFanout tid + let res = filterFromMembers mems + pure res + Journal.teamUpdate tid sizeAfterDelete $ filter (/= remove) toNotify pure TeamMemberDeleteAccepted else do wsStatus <$> getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid >>= \case From 1c05b8309890f45806c2385a67d235daf3bc7284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 12 Jan 2024 11:59:08 +0100 Subject: [PATCH 05/16] Test: future-port a test from a branch from July 14, 2023 --- integration/test/Test/Conversation.hs | 30 +++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index 2fbbdcd383..d8aa1e4123 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -742,6 +742,36 @@ testLeaveConversationSuccess = do assertLeaveNotification chad conv bob bClient chad assertLeaveNotification chad conv eve eClient chad +testDeleteTeamMemberFullEventFanout :: HasCallStack => App () +testDeleteTeamMemberFullEventFanout = do + (alice, team, [alex, alison]) <- createTeam OwnDomain 3 + [amy, bob] <- for [OwnDomain, OtherDomain] $ flip randomUser def + forM_ [amy, bob] $ connectTwoUsers alice + [aliceId, alexId, alisonId, amyId, bobId] <- + forM [alice, alex, alison, amy, bob] (%. "qualified_id") + let nc = (defProteus {qualifiedUsers = [alexId, alisonId, amyId, bobId], team = Just team}) + conv <- postConversation alice nc >>= getJSON 201 + withWebSockets [alice, alison, amy, bob] $ \[wsAlice, wsAlison, wsAmy, wsBob] -> do + void $ deleteTeamMember team alice alex >>= getBody 202 + alexUId <- alex %. "id" + do + n <- awaitMatch isTeamMemberLeaveNotif wsAlice + nPayload n %. "data.user" `shouldMatch` alexUId + do + n <- awaitMatch isTeamMemberLeaveNotif wsAlison + nPayload n %. "data.user" `shouldMatch` alexUId + do + n <- awaitMatch isConvLeaveNotif wsAmy + nPayload n %. "data.qualified_user_ids" `shouldMatch` [alexId] + do + bindResponse (getConversation bob conv) $ \resp -> do + resp.status `shouldMatchInt` 200 + mems <- resp.json %. "members.others" & asList + memIds <- forM mems (%. "qualified_id") + memIds `shouldMatchSet` [aliceId, alisonId, amyId] + n <- awaitMatch isConvLeaveNotif wsBob + nPayload n %. "data.qualified_user_ids.0" `shouldMatch` alexId + testOnUserDeletedConversations :: HasCallStack => App () testOnUserDeletedConversations = do startDynamicBackends [def] $ \[dynDomain] -> do From 4979bf395089b38b1ecf9fc2734e5e59ea8fab41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 12 Jan 2024 15:25:14 +0100 Subject: [PATCH 06/16] Fix the team event fanout for the unlimited case --- integration/test/Test/Conversation.hs | 10 ++- services/galley/src/Galley/API/Internal.hs | 67 +++++++++++++------ services/galley/src/Galley/API/Teams.hs | 5 +- .../galley/src/Galley/API/Teams/Features.hs | 1 - .../src/Galley/API/Teams/Features/Get.hs | 3 +- 5 files changed, 55 insertions(+), 31 deletions(-) diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index d8aa1e4123..b105cff479 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -742,6 +742,10 @@ testLeaveConversationSuccess = do assertLeaveNotification chad conv bob bClient chad assertLeaveNotification chad conv eve eClient chad +-- The test relies on the default value for the 'limitedEventFanout' flag, which +-- is disabled by default. The counterpart test +-- 'testDeleteTeamMemberLimitedEventFanout' enables the flag and tests the +-- limited fanout. testDeleteTeamMemberFullEventFanout :: HasCallStack => App () testDeleteTeamMemberFullEventFanout = do (alice, team, [alex, alison]) <- createTeam OwnDomain 3 @@ -758,8 +762,10 @@ testDeleteTeamMemberFullEventFanout = do n <- awaitMatch isTeamMemberLeaveNotif wsAlice nPayload n %. "data.user" `shouldMatch` alexUId do - n <- awaitMatch isTeamMemberLeaveNotif wsAlison - nPayload n %. "data.user" `shouldMatch` alexUId + t <- awaitMatch isTeamMemberLeaveNotif wsAlison + nPayload t %. "data.user" `shouldMatch` alexUId + c <- awaitMatch isConvLeaveNotif wsAlison + nPayload c %. "data.qualified_user_ids" `shouldMatch` [alexId] do n <- awaitMatch isConvLeaveNotif wsAmy nPayload n %. "data.qualified_user_ids" `shouldMatch` [alexId] diff --git a/services/galley/src/Galley/API/Internal.hs b/services/galley/src/Galley/API/Internal.hs index 1ed073b333..9d065a5131 100644 --- a/services/galley/src/Galley/API/Internal.hs +++ b/services/galley/src/Galley/API/Internal.hs @@ -298,26 +298,26 @@ rmUser :: forall p1 p2 r. ( p1 ~ CassandraPaging, p2 ~ InternalPaging, - ( Member BackendNotificationQueueAccess r, - Member BrigAccess r, - Member ClientStore r, - Member ConversationStore r, - Member (Error InternalError) r, - Member ExternalAccess r, - Member FederatorAccess r, - Member GundeckAccess r, - Member (Input Env) r, - Member (Input (Local ())) r, - Member (Input UTCTime) r, - Member (ListItems p1 ConvId) r, - Member (ListItems p1 (Remote ConvId)) r, - Member (ListItems p2 TeamId) r, - Member MemberStore r, - Member ProposalStore r, - Member P.TinyLog r, - Member SubConversationStore r, - Member TeamStore r - ) + Member BackendNotificationQueueAccess r, + Member ClientStore r, + Member ConversationStore r, + Member (Error DynError) r, + Member (Error InternalError) r, + Member ExternalAccess r, + Member FederatorAccess r, + Member GundeckAccess r, + Member (Input Env) r, + Member (Input Opts) r, + Member (Input UTCTime) r, + Member (ListItems p1 ConvId) r, + Member (ListItems p1 (Remote ConvId)) r, + Member (ListItems p2 TeamId) r, + Member MemberStore r, + Member ProposalStore r, + Member P.TinyLog r, + Member SubConversationStore r, + Member TeamFeatureStore r, + Member TeamStore r ) => Local UserId -> Maybe ConnId -> @@ -343,11 +343,34 @@ rmUser lusr conn = do goConvPages range newCids leaveTeams page = for_ (pageItems page) $ \tid -> do - admins <- E.getTeamAdmins tid - uncheckedDeleteTeamMember lusr conn tid (tUnqualified lusr) (Left admins) + toNotify <- + handleImpossibleErrors $ + getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid + >>= ( \case + FeatureStatusEnabled -> Left <$> E.getTeamAdmins tid + FeatureStatusDisabled -> Right <$> getTeamMembersForFanout tid + ) + . wsStatus + uncheckedDeleteTeamMember lusr conn tid (tUnqualified lusr) toNotify page' <- listTeams @p2 (tUnqualified lusr) (Just (pageState page)) maxBound leaveTeams page' + -- The @'NotATeamMember@ and @'TeamNotFound@ errors cannot happen at this + -- point: the user is a team member because we fetched the list of teams + -- they are member of, and conversely the list of teams was fetched exactly + -- for this user so it cannot be that the team is not found. Therefore, this + -- helper just drops the errors. + handleImpossibleErrors :: + Sem + ( ErrorS 'NotATeamMember + ': ErrorS 'TeamNotFound + ': r + ) + a -> + Sem r a + handleImpossibleErrors action = + mapToDynamicError @'TeamNotFound (mapToDynamicError @'NotATeamMember action) + leaveLocalConversations :: [ConvId] -> Sem r () leaveLocalConversations ids = do let qUser = tUntagged lusr diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index 2ceabc963f..655f0ca83d 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -992,10 +992,7 @@ deleteTeamMember' lusr zcon tid remove mBody = do FeatureStatusDisabled -> do let filterFromMembers list = view userId <$> filter (`hasPermission` SetBilling) (list ^. teamMembers) - -- fmap (view userId) $ (list ^. teamMembers) - mems <- getTeamMembersForFanout tid - let res = filterFromMembers mems - pure res + filterFromMembers <$> getTeamMembersForFanout tid Journal.teamUpdate tid sizeAfterDelete $ filter (/= remove) toNotify pure TeamMemberDeleteAccepted else do diff --git a/services/galley/src/Galley/API/Teams/Features.hs b/services/galley/src/Galley/API/Teams/Features.hs index b00b2c6f1d..f18b1fe6c5 100644 --- a/services/galley/src/Galley/API/Teams/Features.hs +++ b/services/galley/src/Galley/API/Teams/Features.hs @@ -81,7 +81,6 @@ patchFeatureStatusInternal :: GetConfigForTeamConstraints cfg r, SetConfigForTeamConstraints cfg r, Member (ErrorS 'NotATeamMember) r, - Member (ErrorS OperationDenied) r, Member (ErrorS 'TeamNotFound) r, Member TeamStore r, Member TeamFeatureStore r, diff --git a/services/galley/src/Galley/API/Teams/Features/Get.hs b/services/galley/src/Galley/API/Teams/Features/Get.hs index c2d61f92aa..c0ca2b2c9c 100644 --- a/services/galley/src/Galley/API/Teams/Features/Get.hs +++ b/services/galley/src/Galley/API/Teams/Features/Get.hs @@ -115,8 +115,7 @@ getFeatureStatus :: forall cfg r. ( GetFeatureConfig cfg, GetConfigForTeamConstraints cfg r, - ( Member (ErrorS OperationDenied) r, - Member (ErrorS 'NotATeamMember) r, + ( Member (ErrorS 'NotATeamMember) r, Member (ErrorS 'TeamNotFound) r, Member TeamStore r ) From 9a014ab2f0958f6b29629eea54a3502171dba1f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 12 Jan 2024 17:04:04 +0100 Subject: [PATCH 07/16] Test: getting and setting the feature flag --- integration/integration.cabal | 1 + integration/test/Test/FeatureFlags.hs | 35 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 integration/test/Test/FeatureFlags.hs diff --git a/integration/integration.cabal b/integration/integration.cabal index 26428be73c..6f9148fd06 100644 --- a/integration/integration.cabal +++ b/integration/integration.cabal @@ -119,6 +119,7 @@ library Test.Demo Test.Errors Test.ExternalPartner + Test.FeatureFlags Test.Federation Test.Federator Test.MessageTimer diff --git a/integration/test/Test/FeatureFlags.hs b/integration/test/Test/FeatureFlags.hs new file mode 100644 index 0000000000..f31e1ed425 --- /dev/null +++ b/integration/test/Test/FeatureFlags.hs @@ -0,0 +1,35 @@ +-- This file is part of the Wire Server implementation. +-- +-- 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 +-- 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.FeatureFlags where + +import API.GalleyInternal +import SetupHelpers +import Testlib.Prelude + +testLimitedEventFanout :: HasCallStack => App () +testLimitedEventFanout = do + let featureName = "limitedEventFanout" + (_alice, team, _) <- createTeam OwnDomain 1 + -- getTeamFeatureStatus OwnDomain team "limitedEventFanout" "enabled" + bindResponse (getTeamFeature OwnDomain featureName team) $ \resp -> do + resp.status `shouldMatchInt` 200 + resp.json %. "status" `shouldMatch` "disabled" + setTeamFeatureStatus OwnDomain team featureName "enabled" + bindResponse (getTeamFeature OwnDomain featureName team) $ \resp -> do + resp.status `shouldMatchInt` 200 + resp.json %. "status" `shouldMatch` "enabled" From bc12939b1da34c00960f1f295f0bc4257d5050b5 Mon Sep 17 00:00:00 2001 From: Stefan Berthold Date: Mon, 15 Jan 2024 13:10:39 +0000 Subject: [PATCH 08/16] fix linter --- services/galley/src/Galley/API/Teams.hs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index 655f0ca83d..d43d2e0bd9 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -987,22 +987,22 @@ deleteTeamMember' lusr zcon tid remove mBody = do else sizeBeforeDelete - 1 E.deleteUser remove toNotify <- - wsStatus <$> getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid >>= \case + getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid >>= (\case FeatureStatusEnabled -> E.getBillingTeamMembers tid FeatureStatusDisabled -> do let filterFromMembers list = view userId <$> filter (`hasPermission` SetBilling) (list ^. teamMembers) - filterFromMembers <$> getTeamMembersForFanout tid + filterFromMembers <$> getTeamMembersForFanout tid) . wsStatus Journal.teamUpdate tid sizeAfterDelete $ filter (/= remove) toNotify pure TeamMemberDeleteAccepted else do - wsStatus <$> getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid >>= \case + getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid >>= (\case FeatureStatusEnabled -> do admins <- E.getTeamAdmins tid uncheckedDeleteTeamMember lusr (Just zcon) tid remove (Left admins) FeatureStatusDisabled -> do mems <- getTeamMembersForFanout tid - uncheckedDeleteTeamMember lusr (Just zcon) tid remove (Right mems) + uncheckedDeleteTeamMember lusr (Just zcon) tid remove (Right mems)) . wsStatus pure TeamMemberDeleteCompleted -- This function is "unchecked" because it does not validate that the user has the `RemoveTeamMember` permission. From d272978b1aa30875fbbbad7f53afdf181511459a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Mon, 15 Jan 2024 14:28:26 +0100 Subject: [PATCH 09/16] Add a changelog --- changelog.d/2-features/WPB-5883 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/2-features/WPB-5883 diff --git a/changelog.d/2-features/WPB-5883 b/changelog.d/2-features/WPB-5883 new file mode 100644 index 0000000000..5fd8d32672 --- /dev/null +++ b/changelog.d/2-features/WPB-5883 @@ -0,0 +1 @@ +Introduce a feature flag that controls whether the limited event fanout should be used when a team member is deleted From e030a6e9814312471ce495807ed56ce4ff1ce51a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Mon, 15 Jan 2024 14:57:11 +0100 Subject: [PATCH 10/16] Fix more linting --- services/galley/src/Galley/API/Teams.hs | 32 +++++++++++++++---------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index d43d2e0bd9..33c5fea1fe 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -987,22 +987,28 @@ deleteTeamMember' lusr zcon tid remove mBody = do else sizeBeforeDelete - 1 E.deleteUser remove toNotify <- - getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid >>= (\case - FeatureStatusEnabled -> E.getBillingTeamMembers tid - FeatureStatusDisabled -> do - let filterFromMembers list = - view userId <$> filter (`hasPermission` SetBilling) (list ^. teamMembers) - filterFromMembers <$> getTeamMembersForFanout tid) . wsStatus + getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid + >>= ( \case + FeatureStatusEnabled -> E.getBillingTeamMembers tid + FeatureStatusDisabled -> do + let filterFromMembers list = + view userId <$> filter (`hasPermission` SetBilling) (list ^. teamMembers) + filterFromMembers <$> getTeamMembersForFanout tid + ) + . wsStatus Journal.teamUpdate tid sizeAfterDelete $ filter (/= remove) toNotify pure TeamMemberDeleteAccepted else do - getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid >>= (\case - FeatureStatusEnabled -> do - admins <- E.getTeamAdmins tid - uncheckedDeleteTeamMember lusr (Just zcon) tid remove (Left admins) - FeatureStatusDisabled -> do - mems <- getTeamMembersForFanout tid - uncheckedDeleteTeamMember lusr (Just zcon) tid remove (Right mems)) . wsStatus + getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid + >>= ( \case + FeatureStatusEnabled -> do + admins <- E.getTeamAdmins tid + uncheckedDeleteTeamMember lusr (Just zcon) tid remove (Left admins) + FeatureStatusDisabled -> do + mems <- getTeamMembersForFanout tid + uncheckedDeleteTeamMember lusr (Just zcon) tid remove (Right mems) + ) + . wsStatus pure TeamMemberDeleteCompleted -- This function is "unchecked" because it does not validate that the user has the `RemoveTeamMember` permission. From 550ac204e8802cc060d279edc9050fc05b543587 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Mon, 15 Jan 2024 15:03:32 +0100 Subject: [PATCH 11/16] Move a test within a module --- integration/test/Test/Conversation.hs | 44 +++++++++++++-------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index b105cff479..21f619dcf4 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -720,28 +720,6 @@ testDeleteTeamMemberLimitedEventFanout = do ms <- resp.json %. "members" & asList forM ms $ (%. "user") -testLeaveConversationSuccess :: HasCallStack => App () -testLeaveConversationSuccess = do - [alice, bob, chad, dee] <- createUsers [OwnDomain, OwnDomain, OtherDomain, OtherDomain] - [aClient, bClient] <- forM [alice, bob] $ \user -> - objId $ bindResponse (addClient user def) $ getJSON 201 - startDynamicBackends [def] $ \[dynDomain] -> do - eve <- randomUser dynDomain def - eClient <- objId $ bindResponse (addClient eve def) $ getJSON 201 - forM_ [bob, chad, dee, eve] $ connectTwoUsers alice - conv <- - postConversation - alice - ( defProteus - { qualifiedUsers = [bob, chad, dee, eve] - } - ) - >>= getJSON 201 - void $ removeMember chad conv chad >>= getBody 200 - assertLeaveNotification chad conv alice aClient chad - assertLeaveNotification chad conv bob bClient chad - assertLeaveNotification chad conv eve eClient chad - -- The test relies on the default value for the 'limitedEventFanout' flag, which -- is disabled by default. The counterpart test -- 'testDeleteTeamMemberLimitedEventFanout' enables the flag and tests the @@ -778,6 +756,28 @@ testDeleteTeamMemberFullEventFanout = do n <- awaitMatch isConvLeaveNotif wsBob nPayload n %. "data.qualified_user_ids.0" `shouldMatch` alexId +testLeaveConversationSuccess :: HasCallStack => App () +testLeaveConversationSuccess = do + [alice, bob, chad, dee] <- createUsers [OwnDomain, OwnDomain, OtherDomain, OtherDomain] + [aClient, bClient] <- forM [alice, bob] $ \user -> + objId $ bindResponse (addClient user def) $ getJSON 201 + startDynamicBackends [def] $ \[dynDomain] -> do + eve <- randomUser dynDomain def + eClient <- objId $ bindResponse (addClient eve def) $ getJSON 201 + forM_ [bob, chad, dee, eve] $ connectTwoUsers alice + conv <- + postConversation + alice + ( defProteus + { qualifiedUsers = [bob, chad, dee, eve] + } + ) + >>= getJSON 201 + void $ removeMember chad conv chad >>= getBody 200 + assertLeaveNotification chad conv alice aClient chad + assertLeaveNotification chad conv bob bClient chad + assertLeaveNotification chad conv eve eClient chad + testOnUserDeletedConversations :: HasCallStack => App () testOnUserDeletedConversations = do startDynamicBackends [def] $ \[dynDomain] -> do From ac5e1e49b5af0b78d189e52f57ebe61f65c77cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 16 Jan 2024 09:29:37 +0100 Subject: [PATCH 12/16] Fix a galley-types unit test --- libs/galley-types/test/unit/Test/Galley/Types.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/galley-types/test/unit/Test/Galley/Types.hs b/libs/galley-types/test/unit/Test/Galley/Types.hs index 09e8ab6c38..3381fe49ef 100644 --- a/libs/galley-types/test/unit/Test/Galley/Types.hs +++ b/libs/galley-types/test/unit/Test/Galley/Types.hs @@ -100,7 +100,7 @@ instance Arbitrary FeatureFlags where <*> arbitrary <*> arbitrary <*> arbitrary - <*> arbitrary + <*> fmap (fmap unlocked) arbitrary where unlocked :: ImplicitLockStatus a -> ImplicitLockStatus a unlocked = ImplicitLockStatus . Public.setLockStatus Public.LockStatusUnlocked . _unImplicitLockStatus From cf3d08df7837e84377d051ef689c86624fce0cfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 16 Jan 2024 09:33:29 +0100 Subject: [PATCH 13/16] Fix a galley-integration test --- services/galley/test/integration/API/Teams/Feature.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/galley/test/integration/API/Teams/Feature.hs b/services/galley/test/integration/API/Teams/Feature.hs index f0a522d365..a3f07368c5 100644 --- a/services/galley/test/integration/API/Teams/Feature.hs +++ b/services/galley/test/integration/API/Teams/Feature.hs @@ -1065,7 +1065,7 @@ testAllFeatures = do afcMlsMigration = defaultMlsMigrationConfig, afcEnforceFileDownloadLocation = defaultEnforceFileDownloadLocationConfig, afcLimitedEventFanout = - withStatus FeatureStatusDisabled LockStatusLocked LimitedEventFanoutConfig FeatureTTLUnlimited + withStatus FeatureStatusDisabled LockStatusUnlocked LimitedEventFanoutConfig FeatureTTLUnlimited } testFeatureConfigConsistency :: TestM () From 7baf56d846c593c5e81c8ff9fd886bc365b09981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 16 Jan 2024 10:15:18 +0100 Subject: [PATCH 14/16] Make a notification push transient --- services/galley/src/Galley/API/Teams.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index 33c5fea1fe..a55c85d062 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -1065,7 +1065,7 @@ uncheckedDeleteTeamMember lusr zcon tid remove (Right mems) = do (userRecipient (tUnqualified lusr)) (membersToRecipients (Just (tUnqualified lusr)) (mems ^. teamMembers)) E.push1 $ - newPushLocal1 (mems ^. teamMemberListType) (tUnqualified lusr) (TeamEvent e) r & pushConn .~ zcon + newPushLocal1 (mems ^. teamMemberListType) (tUnqualified lusr) (TeamEvent e) r & pushTransient .~ True removeFromConvsAndPushConvLeaveEvent :: forall r. From ed05cc2a61b88ff58674b0f3a5ff2291b8ebab9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 16 Jan 2024 10:28:20 +0100 Subject: [PATCH 15/16] Revert the change to the billing team update notification --- services/galley/src/Galley/API/Teams.hs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/services/galley/src/Galley/API/Teams.hs b/services/galley/src/Galley/API/Teams.hs index a55c85d062..de6d448650 100644 --- a/services/galley/src/Galley/API/Teams.hs +++ b/services/galley/src/Galley/API/Teams.hs @@ -986,17 +986,8 @@ deleteTeamMember' lusr zcon tid remove mBody = do then 0 else sizeBeforeDelete - 1 E.deleteUser remove - toNotify <- - getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid - >>= ( \case - FeatureStatusEnabled -> E.getBillingTeamMembers tid - FeatureStatusDisabled -> do - let filterFromMembers list = - view userId <$> filter (`hasPermission` SetBilling) (list ^. teamMembers) - filterFromMembers <$> getTeamMembersForFanout tid - ) - . wsStatus - Journal.teamUpdate tid sizeAfterDelete $ filter (/= remove) toNotify + owners <- E.getBillingTeamMembers tid + Journal.teamUpdate tid sizeAfterDelete $ filter (/= remove) owners pure TeamMemberDeleteAccepted else do getFeatureStatus @LimitedEventFanoutConfig DontDoAuth tid From 31afbb5e9c05744589374ab0b2fb78ed1d45abfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 16 Jan 2024 10:36:32 +0100 Subject: [PATCH 16/16] Reuse a notification assertion helper --- integration/test/Notifications.hs | 6 ++++++ integration/test/Test/Conversation.hs | 27 ++++++++++----------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/integration/test/Notifications.hs b/integration/test/Notifications.hs index ec92dd390b..9ea5370622 100644 --- a/integration/test/Notifications.hs +++ b/integration/test/Notifications.hs @@ -139,3 +139,9 @@ assertLeaveNotification fromUser conv user client leaver = isNotifFromUser fromUser ] ) + +assertConvUserDeletedNotif :: MakesValue leaverId => WebSocket -> leaverId -> App () +assertConvUserDeletedNotif ws leaverId = do + n <- awaitMatch isConvLeaveNotif ws + nPayload n %. "data.qualified_user_ids.0" `shouldMatch` leaverId + nPayload n %. "data.reason" `shouldMatch` "user-deleted" diff --git a/integration/test/Test/Conversation.hs b/integration/test/Test/Conversation.hs index 21f619dcf4..558c47a30f 100644 --- a/integration/test/Test/Conversation.hs +++ b/integration/test/Test/Conversation.hs @@ -690,31 +690,26 @@ testDeleteTeamMemberLimitedEventFanout = do memsAfter <- getMembers team aliceId memsAfter `shouldNotMatch` memsBefore - assertConvLeaveNotif wsAmy alexId - assertConvLeaveNotif wsAlison alexId + assertConvUserDeletedNotif wsAmy alexId + assertConvUserDeletedNotif wsAlison alexId alexUId <- alex %. "id" do n <- awaitMatch isTeamMemberLeaveNotif wsAlice nPayload n %. "data.user" `shouldMatch` alexUId - assertConvLeaveNotif wsAlice alexId + assertConvUserDeletedNotif wsAlice alexId do n <- awaitMatch isTeamMemberLeaveNotif wsAna nPayload n %. "data.user" `shouldMatch` alexUId - assertConvLeaveNotif wsAna alexId + assertConvUserDeletedNotif wsAna alexId do bindResponse (getConversation bob conv) $ \resp -> do resp.status `shouldMatchInt` 200 mems <- resp.json %. "members.others" & asList memIds <- forM mems (%. "qualified_id") memIds `shouldMatchSet` [aliceId, alisonId, amyId, anaId] - assertConvLeaveNotif wsBob alexId + assertConvUserDeletedNotif wsBob alexId where - assertConvLeaveNotif :: MakesValue leaverId => WebSocket -> leaverId -> App () - assertConvLeaveNotif ws leaverId = do - n <- awaitMatch isConvLeaveNotif ws - nPayload n %. "data.qualified_user_ids.0" `shouldMatch` leaverId - nPayload n %. "data.reason" `shouldMatch` "user-deleted" getMembers tid usr = bindResponse (getTeamMembers usr tid) $ \resp -> do resp.status `shouldMatchInt` 200 ms <- resp.json %. "members" & asList @@ -742,19 +737,17 @@ testDeleteTeamMemberFullEventFanout = do do t <- awaitMatch isTeamMemberLeaveNotif wsAlison nPayload t %. "data.user" `shouldMatch` alexUId - c <- awaitMatch isConvLeaveNotif wsAlison - nPayload c %. "data.qualified_user_ids" `shouldMatch` [alexId] - do - n <- awaitMatch isConvLeaveNotif wsAmy - nPayload n %. "data.qualified_user_ids" `shouldMatch` [alexId] + assertConvUserDeletedNotif wsAlison alexId + + assertConvUserDeletedNotif wsAmy alexId + do bindResponse (getConversation bob conv) $ \resp -> do resp.status `shouldMatchInt` 200 mems <- resp.json %. "members.others" & asList memIds <- forM mems (%. "qualified_id") memIds `shouldMatchSet` [aliceId, alisonId, amyId] - n <- awaitMatch isConvLeaveNotif wsBob - nPayload n %. "data.qualified_user_ids.0" `shouldMatch` alexId + assertConvUserDeletedNotif wsBob alexId testLeaveConversationSuccess :: HasCallStack => App () testLeaveConversationSuccess = do