From 54f956ea4e9ae96ffa0adcee6371a6eaa63d3403 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 20 Jun 2022 14:23:24 +0000 Subject: [PATCH 1/6] removed unused functions, replaced deprecated endpoint call in test --- libs/wire-api/src/Wire/API/Team/Feature.hs | 8 -------- services/spar/test-integration/Util/Email.hs | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/libs/wire-api/src/Wire/API/Team/Feature.hs b/libs/wire-api/src/Wire/API/Team/Feature.hs index 590a2d42a8..37b6070d57 100644 --- a/libs/wire-api/src/Wire/API/Team/Feature.hs +++ b/libs/wire-api/src/Wire/API/Team/Feature.hs @@ -24,8 +24,6 @@ module Wire.API.Team.Feature ( FeatureStatus (..), featureName, featureNameBS, - deprecatedFeatureName, - deprecatedFeatureNameBS, LockStatus (..), WithStatus (..), WithStatusNoLock (..), @@ -158,12 +156,6 @@ featureName = T.pack $ symbolVal (Proxy @(FeatureSymbol cfg)) featureNameBS :: forall cfg. (IsFeatureConfig cfg, KnownSymbol (FeatureSymbol cfg)) => ByteString featureNameBS = UTF8.fromString $ symbolVal (Proxy @(FeatureSymbol cfg)) -deprecatedFeatureName :: forall cfg. (HasDeprecatedFeatureName cfg, KnownSymbol (DeprecatedFeatureName cfg)) => Text -deprecatedFeatureName = T.pack $ symbolVal (Proxy @(DeprecatedFeatureName cfg)) - -deprecatedFeatureNameBS :: forall cfg. (HasDeprecatedFeatureName cfg, KnownSymbol (DeprecatedFeatureName cfg)) => ByteString -deprecatedFeatureNameBS = UTF8.fromString $ symbolVal (Proxy @(DeprecatedFeatureName cfg)) - ---------------------------------------------------------------------- -- WithStatus diff --git a/services/spar/test-integration/Util/Email.hs b/services/spar/test-integration/Util/Email.hs index 6d63868fbc..0278b9cced 100644 --- a/services/spar/test-integration/Util/Email.hs +++ b/services/spar/test-integration/Util/Email.hs @@ -159,5 +159,5 @@ setSamlEmailValidation :: HasCallStack => TeamId -> Feature.FeatureStatus -> Tes setSamlEmailValidation tid status = do galley <- view teGalley let req = put $ galley . paths p . json (Feature.WithStatusNoLock @Feature.ValidateSAMLEmailsConfig status Feature.trivialConfig) - p = ["/i/teams", toByteString' tid, "features", "validate-saml-emails"] + p = ["/i/teams", toByteString' tid, "features", Feature.featureNameBS @Feature.ValidateSAMLEmailsConfig] call req !!! const 200 === statusCode From eaed5a825415bebd2da766f4c311c3859c31d767 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Mon, 20 Jun 2022 14:27:38 +0000 Subject: [PATCH 2/6] docs for deprecated internal endpoints --- services/galley/src/Galley/API/Internal.hs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/services/galley/src/Galley/API/Internal.hs b/services/galley/src/Galley/API/Internal.hs index 87e2490495..71b59a14c7 100644 --- a/services/galley/src/Galley/API/Internal.hs +++ b/services/galley/src/Galley/API/Internal.hs @@ -130,18 +130,18 @@ type IFeatureAPI = -- SearchVisibilityAvailableConfig :<|> IFeatureStatusGet SearchVisibilityAvailableConfig :<|> IFeatureStatusPut '() SearchVisibilityAvailableConfig - :<|> IFeatureStatusDeprecatedGet "" SearchVisibilityAvailableConfig - :<|> IFeatureStatusDeprecatedPut "" SearchVisibilityAvailableConfig + :<|> IFeatureStatusDeprecatedGet "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" SearchVisibilityAvailableConfig + :<|> IFeatureStatusDeprecatedPut "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" SearchVisibilityAvailableConfig -- ValidateSAMLEmailsConfig :<|> IFeatureStatusGet ValidateSAMLEmailsConfig :<|> IFeatureStatusPut '() ValidateSAMLEmailsConfig - :<|> IFeatureStatusDeprecatedGet "" ValidateSAMLEmailsConfig - :<|> IFeatureStatusDeprecatedPut "" ValidateSAMLEmailsConfig + :<|> IFeatureStatusDeprecatedGet "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" ValidateSAMLEmailsConfig + :<|> IFeatureStatusDeprecatedPut "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" ValidateSAMLEmailsConfig -- DigitalSignaturesConfig :<|> IFeatureStatusGet DigitalSignaturesConfig :<|> IFeatureStatusPut '() DigitalSignaturesConfig - :<|> IFeatureStatusDeprecatedGet "" DigitalSignaturesConfig - :<|> IFeatureStatusDeprecatedPut "" DigitalSignaturesConfig + :<|> IFeatureStatusDeprecatedGet "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" DigitalSignaturesConfig + :<|> IFeatureStatusDeprecatedPut "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" DigitalSignaturesConfig -- AppLockConfig :<|> IFeatureStatusGet AppLockConfig :<|> IFeatureStatusPut '() AppLockConfig From e4afa92d420020bddad6788aa7d4ce168cf279e4 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Tue, 21 Jun 2022 06:59:57 +0000 Subject: [PATCH 3/6] removed deprecated internal endpoints --- services/galley/src/Galley/API/Internal.hs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/services/galley/src/Galley/API/Internal.hs b/services/galley/src/Galley/API/Internal.hs index 71b59a14c7..212b953675 100644 --- a/services/galley/src/Galley/API/Internal.hs +++ b/services/galley/src/Galley/API/Internal.hs @@ -130,18 +130,12 @@ type IFeatureAPI = -- SearchVisibilityAvailableConfig :<|> IFeatureStatusGet SearchVisibilityAvailableConfig :<|> IFeatureStatusPut '() SearchVisibilityAvailableConfig - :<|> IFeatureStatusDeprecatedGet "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" SearchVisibilityAvailableConfig - :<|> IFeatureStatusDeprecatedPut "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" SearchVisibilityAvailableConfig -- ValidateSAMLEmailsConfig :<|> IFeatureStatusGet ValidateSAMLEmailsConfig :<|> IFeatureStatusPut '() ValidateSAMLEmailsConfig - :<|> IFeatureStatusDeprecatedGet "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" ValidateSAMLEmailsConfig - :<|> IFeatureStatusDeprecatedPut "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" ValidateSAMLEmailsConfig -- DigitalSignaturesConfig :<|> IFeatureStatusGet DigitalSignaturesConfig :<|> IFeatureStatusPut '() DigitalSignaturesConfig - :<|> IFeatureStatusDeprecatedGet "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" DigitalSignaturesConfig - :<|> IFeatureStatusDeprecatedPut "This endpoint is not used internally by wire-server, nor by stern. After verification that it is not used in ibis, this can be removed" DigitalSignaturesConfig -- AppLockConfig :<|> IFeatureStatusGet AppLockConfig :<|> IFeatureStatusPut '() AppLockConfig @@ -367,10 +361,6 @@ type FeatureStatusBasePutInternal errs featureConfig = :> QueryParam "ttl" FeatureTTL :> Put '[Servant.JSON] (WithStatus featureConfig) -type IFeatureStatusDeprecatedGet d f = Named '("iget-deprecated", f) (FeatureStatusBaseDeprecatedGet d f) - -type IFeatureStatusDeprecatedPut d f = Named '("iput-deprecated", f) (FeatureStatusBaseDeprecatedPut d f) - type IFeatureStatusLockStatusPut featureName = Named '("lock", featureName) @@ -457,16 +447,10 @@ featureAPI = <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) <@> mkNamedAPI (\tid ws ttl -> setFeatureStatus @Cassandra ttl DontDoAuth tid ws) <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) - <@> mkNamedAPI (setFeatureStatus @Cassandra Nothing DontDoAuth) - <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) <@> mkNamedAPI (\tid ws ttl -> setFeatureStatus @Cassandra ttl DontDoAuth tid ws) <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) - <@> mkNamedAPI (setFeatureStatus @Cassandra Nothing DontDoAuth) - <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) <@> mkNamedAPI (\tid ws ttl -> setFeatureStatus @Cassandra ttl DontDoAuth tid ws) <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) - <@> mkNamedAPI (setFeatureStatus @Cassandra Nothing DontDoAuth) - <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) <@> mkNamedAPI (\tid ws ttl -> setFeatureStatus @Cassandra ttl DontDoAuth tid ws) <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) <@> mkNamedAPI (\tid ws ttl -> setFeatureStatus @Cassandra ttl DontDoAuth tid ws) From 51a0bf57b63b9ead4d061a823e21e01617621362 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Tue, 21 Jun 2022 07:01:30 +0000 Subject: [PATCH 4/6] changelog --- changelog.d/5-internal/pr-2496 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5-internal/pr-2496 diff --git a/changelog.d/5-internal/pr-2496 b/changelog.d/5-internal/pr-2496 new file mode 100644 index 0000000000..17a9979fd4 --- /dev/null +++ b/changelog.d/5-internal/pr-2496 @@ -0,0 +1 @@ +Removed deprecated internal feature config API endpoints From a3e7597c7c758c0a5314edaae49ba14e3fc11a5a Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Tue, 21 Jun 2022 14:57:27 +0000 Subject: [PATCH 5/6] internal all features config endpoint --- libs/galley-types/src/Galley/Types/Teams.hs | 4 +- libs/wire-api/src/Wire/API/Team/Feature.hs | 18 ++-- services/brig/src/Brig/IO/Intra.hs | 10 +- services/brig/src/Brig/Provider/API.hs | 2 +- services/galley/src/Galley/API/Internal.hs | 40 ++++---- services/galley/src/Galley/API/LegalHold.hs | 2 +- .../galley/src/Galley/API/Teams/Features.hs | 95 ++++++++----------- 7 files changed, 73 insertions(+), 98 deletions(-) diff --git a/libs/galley-types/src/Galley/Types/Teams.hs b/libs/galley-types/src/Galley/Types/Teams.hs index b978c5ef37..4f43a4cb60 100644 --- a/libs/galley-types/src/Galley/Types/Teams.hs +++ b/libs/galley-types/src/Galley/Types/Teams.hs @@ -377,7 +377,6 @@ data HiddenPerm = ChangeLegalHoldTeamSettings | ChangeLegalHoldUserSettings | ViewLegalHoldUserSettings - | ViewTeamFeature | ChangeTeamFeature | ChangeTeamSearchVisibility | ViewTeamSearchVisibility @@ -425,8 +424,7 @@ roleHiddenPermissions role = HiddenPermissions p p Set.fromList [ViewSameTeamEmails] roleHiddenPerms RoleExternalPartner = Set.fromList - [ ViewTeamFeature, - ViewLegalHoldUserSettings, + [ ViewLegalHoldUserSettings, ViewTeamSearchVisibility ] diff --git a/libs/wire-api/src/Wire/API/Team/Feature.hs b/libs/wire-api/src/Wire/API/Team/Feature.hs index 37b6070d57..84ad9d3e31 100644 --- a/libs/wire-api/src/Wire/API/Team/Feature.hs +++ b/libs/wire-api/src/Wire/API/Team/Feature.hs @@ -104,32 +104,32 @@ import Wire.API.MLS.CipherSuite (CipherSuiteTag (MLS_128_DHKEMX25519_AES128GCM_S -- -- 2. Add the config to to 'AllFeatureConfigs'. Add your feature to 'allFeatureModels'. -- --- 2. If your feature is configurable on a per-team basis, add a schema +-- 3. If your feature is configurable on a per-team basis, add a schema -- migration in galley and add 'FeatureStatusCassandra' instance in --- Galley.Cassandra.TreamFeatures together with a schema migration +-- Galley.Cassandra.TeamFeatures together with a schema migration -- --- 3. Add the feature to the config schema of galley in Galley.Types.Teams. +-- 4. Add the feature to the config schema of galley in Galley.Types.Teams. -- and extend the Arbitrary instance of FeatureConfigs in the unit tests Test.Galley.Types -- --- 4. Implement 'GetFeatureConfig' and 'SetFeatureConfig' in +-- 5. Implement 'GetFeatureConfig' and 'SetFeatureConfig' in -- Galley.API.Teams.Features which defines the main business logic for getting -- and setting (with side-effects). -- --- 5. Add public routes to Routes.Public.Galley: 'FeatureStatusGet', +-- 6. Add public routes to Routes.Public.Galley: 'FeatureStatusGet', -- 'FeatureStatusPut' (optional) and by by user: 'FeatureConfigGet'. Then -- implement them in Galley.API.Public. -- --- 6. Add internal routes in Galley.API.Internal +-- 7. Add internal routes in Galley.API.Internal -- --- 7. If the feature should be configurable via Stern add routes to Stern.API. +-- 8. If the feature should be configurable via Stern add routes to Stern.API. -- Manually check that the swagger looks okay. -- --- 8. If the feature is configured on a per-user level, see the +-- 9. If the feature is configured on a per-user level, see the -- 'ConferenceCallingConfig' as an example. -- (https://github.com/wireapp/wire-server/pull/1811, -- https://github.com/wireapp/wire-server/pull/1818) -- --- 9. Extend the integration tests with cases +-- 10. Extend the integration tests with cases class IsFeatureConfig cfg where type FeatureSymbol cfg :: Symbol defFeatureStatus :: WithStatus cfg diff --git a/services/brig/src/Brig/IO/Intra.hs b/services/brig/src/Brig/IO/Intra.hs index cf5a1d822e..b77a16cb6a 100644 --- a/services/brig/src/Brig/IO/Intra.hs +++ b/services/brig/src/Brig/IO/Intra.hs @@ -57,8 +57,8 @@ module Brig.IO.Intra getTeamLegalHoldStatus, changeTeamStatus, getTeamSearchVisibility, + getAllFeatureConfigsForUser, getVerificationCodeEnabled, - getTeamFeatureStatusSndFactorPasswordChallenge, -- * Legalhold guardLegalhold, @@ -1392,7 +1392,7 @@ getVerificationCodeEnabled tid = do paths ["i", "teams", toByteString' tid, "features", featureNameBS @SndFactorPasswordChallengeConfig] . expect2xx -getTeamFeatureStatusSndFactorPasswordChallenge :: +getAllFeatureConfigsForUser :: ( MonadReader Env m, MonadIO m, MonadMask m, @@ -1400,12 +1400,12 @@ getTeamFeatureStatusSndFactorPasswordChallenge :: HasRequestId m ) => Maybe UserId -> - m (WithStatus SndFactorPasswordChallengeConfig) -getTeamFeatureStatusSndFactorPasswordChallenge mbUserId = + m AllFeatureConfigs +getAllFeatureConfigsForUser mbUserId = responseJsonUnsafe <$> galleyRequest GET - ( paths ["i", "feature-configs", featureNameBS @SndFactorPasswordChallengeConfig] + ( paths ["i", "feature-configs"] . maybe id (queryItem "user_id" . toByteString') mbUserId ) diff --git a/services/brig/src/Brig/Provider/API.hs b/services/brig/src/Brig/Provider/API.hs index 367444b517..a78620c261 100644 --- a/services/brig/src/Brig/Provider/API.hs +++ b/services/brig/src/Brig/Provider/API.hs @@ -1087,7 +1087,7 @@ guardSecondFactorDisabled :: Maybe UserId -> ExceptT Error m () guardSecondFactorDisabled mbUserId = do - enabled <- lift $ (==) Feature.FeatureStatusEnabled . Feature.wsStatus <$> RPC.getTeamFeatureStatusSndFactorPasswordChallenge mbUserId + enabled <- lift $ (==) Feature.FeatureStatusEnabled . Feature.wsStatus . Feature.afcSndFactorPasswordChallenge <$> RPC.getAllFeatureConfigsForUser mbUserId when enabled $ throwStd accessDenied minRsaKeySize :: Int diff --git a/services/galley/src/Galley/API/Internal.hs b/services/galley/src/Galley/API/Internal.hs index 212b953675..5d96695a08 100644 --- a/services/galley/src/Galley/API/Internal.hs +++ b/services/galley/src/Galley/API/Internal.hs @@ -167,6 +167,23 @@ type IFeatureAPI = -- MLSConfig :<|> IFeatureStatusGet MLSConfig :<|> IFeatureStatusPut '() MLSConfig + -- all feature configs + :<|> Named + "feature-configs-internal" + ( Summary "Get all feature configs (for user/team; if n/a fall back to site config)." + :> "feature-configs" + :> CanThrow OperationDenied + :> CanThrow 'NotATeamMember + :> CanThrow 'TeamNotFound + :> QueryParam' + [ Optional, + Strict, + Description "Optional user id" + ] + "user_id" + UserId + :> Get '[Servant.JSON] AllFeatureConfigs + ) type InternalAPI = "i" :> InternalAPIBase @@ -220,27 +237,6 @@ type InternalAPIBase = :> ReqBody '[Servant.JSON] UpsertOne2OneConversationRequest :> Post '[Servant.JSON] UpsertOne2OneConversationResponse ) - :<|> Named - "feature-config-snd-factor-password-challenge" - -- FUTUREWORK: Introduce `/i/feature-configs` and drop this one again. The internal end-poins has the - -- same handler as the public one, plus optional user id in the query. Maybe require `DoAuth` to disable - -- access control only on the internal end-point, not on the public one. (This may also be a good oppportunity - -- to make `AllFeatureConfigs` more type-safe.) - ( Summary "Get feature config for the 2nd factor password challenge feature (for user/team; if n/a fall back to site config)." - :> "feature-configs" - :> CanThrow OperationDenied - :> CanThrow 'NotATeamMember - :> CanThrow 'TeamNotFound - :> FeatureSymbol SndFactorPasswordChallengeConfig - :> QueryParam' - [ Optional, - Strict, - Description "Optional user id" - ] - "user_id" - UserId - :> Get '[Servant.JSON] (WithStatus SndFactorPasswordChallengeConfig) - ) :<|> IFeatureAPI type ILegalholdWhitelistedTeamsAPI = @@ -398,7 +394,6 @@ internalAPI = <@> legalholdWhitelistedTeamsAPI <@> iTeamsAPI <@> mkNamedAPI @"upsert-one2one" iUpsertOne2OneConversation - <@> mkNamedAPI @"feature-config-snd-factor-password-challenge" (getFeatureStatusNoPermissionCheck @Cassandra) <@> featureAPI legalholdWhitelistedTeamsAPI :: API ILegalholdWhitelistedTeamsAPI GalleyEffects @@ -472,6 +467,7 @@ featureAPI = <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) <@> mkNamedAPI (getFeatureStatus @Cassandra DontDoAuth) <@> mkNamedAPI (\tid ws ttl -> setFeatureStatus @Cassandra ttl DontDoAuth tid ws) + <@> mkNamedAPI (maybe (getAllFeatureConfigsForServer @Cassandra) (getAllFeatureConfigsForUser @Cassandra)) internalSitemap :: Routes a (Sem GalleyEffects) () internalSitemap = do diff --git a/services/galley/src/Galley/API/LegalHold.hs b/services/galley/src/Galley/API/LegalHold.hs index 5bd899e241..60f7107ed0 100644 --- a/services/galley/src/Galley/API/LegalHold.hs +++ b/services/galley/src/Galley/API/LegalHold.hs @@ -170,7 +170,7 @@ getSettings :: getSettings lzusr tid = do let zusr = tUnqualified lzusr zusrMembership <- getTeamMember tid zusr - void $ permissionCheck ViewTeamFeature zusrMembership + void $ maybe (throwS @'NotATeamMember) pure zusrMembership isenabled <- isLegalHoldEnabledForTeam @db tid mresult <- LegalHoldData.getSettings tid pure $ case (isenabled, mresult) of diff --git a/services/galley/src/Galley/API/Teams/Features.hs b/services/galley/src/Galley/API/Teams/Features.hs index a3467ed10a..764ac11173 100644 --- a/services/galley/src/Galley/API/Teams/Features.hs +++ b/services/galley/src/Galley/API/Teams/Features.hs @@ -17,11 +17,10 @@ module Galley.API.Teams.Features ( getFeatureStatus, - getFeatureStatusNoPermissionCheck, getFeatureStatusMulti, setFeatureStatus, - -- setFeatureStatusNoTTL, getFeatureStatusForUser, + getAllFeatureConfigsForServer, getAllFeatureConfigsForTeam, getAllFeatureConfigsForUser, setLockStatus, @@ -177,34 +176,6 @@ type FeaturePersistentAllFeatures db = FeaturePersistentConstraint db MLSConfig ) -getFeatureStatusNoPermissionCheck :: - forall db cfg r. - ( GetFeatureConfig db cfg, - GetConfigForTeamConstraints db cfg r, - GetConfigForUserConstraints db cfg r, - Members - '[ ErrorS OperationDenied, - ErrorS 'NotATeamMember, - ErrorS 'TeamNotFound, - TeamStore, - Input Opts - ] - r - ) => - Maybe UserId -> - Sem r (WithStatus cfg) -getFeatureStatusNoPermissionCheck = \case - Just uid -> do - mbTeam <- getOneUserTeam uid - case mbTeam of - Nothing -> getConfigForUser @db @cfg uid - Just tid -> do - teamExists <- isJust <$> getTeam tid - if teamExists - then getConfigForTeam @db @cfg tid - else getConfigForUser @db @cfg uid - Nothing -> getConfigForServer @db @cfg - getFeatureStatus :: forall db cfg r. ( GetFeatureConfig db cfg, @@ -222,9 +193,8 @@ getFeatureStatus :: Sem r (WithStatus cfg) getFeatureStatus doauth tid = do case doauth of - DoAuth uid -> do - zusrMembership <- getTeamMember tid uid - void $ permissionCheck ViewTeamFeature zusrMembership + DoAuth uid -> + getTeamMember tid uid >>= maybe (throwS @'NotATeamMember) (const $ pure ()) DontDoAuth -> assertTeamExists tid getConfigForTeam @db @cfg tid @@ -314,16 +284,7 @@ getFeatureStatusForUser :: ) => UserId -> Sem r (WithStatus cfg) -getFeatureStatusForUser zusr = do - mbTeam <- getOneUserTeam zusr - case mbTeam of - Nothing -> - getConfigForUser @db @cfg zusr - Just tid -> do - zusrMembership <- getTeamMember tid zusr - void $ permissionCheck ViewTeamFeature zusrMembership - assertTeamExists tid - getConfigForTeam @db @cfg tid +getFeatureStatusForUser = getConfigForUser @db @cfg getAllFeatureConfigsForUser :: forall db r. @@ -345,7 +306,7 @@ getAllFeatureConfigsForUser zusr = do mbTeam <- getOneUserTeam zusr when (isJust mbTeam) $ do zusrMembership <- maybe (pure Nothing) (`getTeamMember` zusr) mbTeam - void $ permissionCheck ViewTeamFeature zusrMembership + maybe (throwS @'NotATeamMember) (const $ pure ()) zusrMembership case mbTeam of Just tid -> getAllFeatureConfigsTeam @db tid @@ -370,9 +331,40 @@ getAllFeatureConfigsForTeam :: Sem r AllFeatureConfigs getAllFeatureConfigsForTeam luid tid = do zusrMembership <- getTeamMember tid (tUnqualified luid) - void $ permissionCheck ViewTeamFeature zusrMembership + maybe (throwS @'NotATeamMember) (const $ pure ()) zusrMembership getAllFeatureConfigsTeam @db tid +getAllFeatureConfigsForServer :: + forall db r. + Members + '[ BrigAccess, + ErrorS 'NotATeamMember, + ErrorS 'TeamNotFound, + ErrorS OperationDenied, + Input Opts, + LegalHoldStore, + TeamFeatureStore db, + TeamStore + ] + r => + FeaturePersistentAllFeatures db => + Sem r AllFeatureConfigs +getAllFeatureConfigsForServer = + AllFeatureConfigs + <$> getConfigForServer @db @LegalholdConfig + <*> getConfigForServer @db @SSOConfig + <*> getConfigForServer @db @SearchVisibilityAvailableConfig + <*> getConfigForServer @db @ValidateSAMLEmailsConfig + <*> getConfigForServer @db @DigitalSignaturesConfig + <*> getConfigForServer @db @AppLockConfig + <*> getConfigForServer @db @FileSharingConfig + <*> getConfigForServer @db @ClassifiedDomainsConfig + <*> getConfigForServer @db @ConferenceCallingConfig + <*> getConfigForServer @db @SelfDeletingMessagesConfig + <*> getConfigForServer @db @GuestLinksConfig + <*> getConfigForServer @db @SndFactorPasswordChallengeConfig + <*> getConfigForServer @db @MLSConfig + getAllFeatureConfigsUser :: forall db r. Members @@ -492,7 +484,7 @@ genericGetConfigForUser uid = do getConfigForServer @db Just tid -> do zusrMembership <- getTeamMember tid uid - void $ permissionCheck ViewTeamFeature zusrMembership + maybe (throwS @'NotATeamMember) (const $ pure ()) zusrMembership assertTeamExists tid genericGetConfigForTeam @db tid @@ -633,17 +625,6 @@ instance GetFeatureConfig db LegalholdConfig where False -> FeatureStatusDisabled pure $ defFeatureStatus {wsStatus = status} - getConfigForUser uid = do - mbTeam <- getOneUserTeam uid - case mbTeam of - Nothing -> do - getConfigForServer @db - Just tid -> do - zusrMembership <- getTeamMember tid uid - void $ permissionCheck ViewTeamFeature zusrMembership - assertTeamExists tid - getConfigForTeam @db tid - instance SetFeatureConfig db LegalholdConfig where type SetConfigForTeamConstraints db LegalholdConfig (r :: EffectRow) = From 6ff69238b68b53703cd3bd5dd0d978179efae3b8 Mon Sep 17 00:00:00 2001 From: Leif Battermann Date: Wed, 22 Jun 2022 08:39:31 +0000 Subject: [PATCH 6/6] fixed self introduced bug --- services/galley/src/Galley/API/Teams/Features.hs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/services/galley/src/Galley/API/Teams/Features.hs b/services/galley/src/Galley/API/Teams/Features.hs index 764ac11173..b6c0213132 100644 --- a/services/galley/src/Galley/API/Teams/Features.hs +++ b/services/galley/src/Galley/API/Teams/Features.hs @@ -269,6 +269,9 @@ setLockStatus tid lockStatus = do pure $ LockStatusResponse lockStatus -- | For individual users to get feature config for their account (personal or team). +-- This looks supposedly redundant to the implementations of `getConfigForUser` but it's not. +-- Here we explicitly return the team setting if the user is a team member. +-- In `getConfigForUser` this is mostly also the case. But there are exceptions, e.g. `ConferenceCallingConfig` getFeatureStatusForUser :: forall (db :: *) cfg r. ( Members @@ -284,7 +287,16 @@ getFeatureStatusForUser :: ) => UserId -> Sem r (WithStatus cfg) -getFeatureStatusForUser = getConfigForUser @db @cfg +getFeatureStatusForUser zusr = do + mbTeam <- getOneUserTeam zusr + case mbTeam of + Nothing -> + getConfigForUser @db @cfg zusr + Just tid -> do + zusrMembership <- getTeamMember tid zusr + void $ maybe (throwS @'NotATeamMember) pure zusrMembership + assertTeamExists tid + getConfigForTeam @db @cfg tid getAllFeatureConfigsForUser :: forall db r.