Sqservices 1529 remove redundant endpoints internal and view feature permissions

changelog.d/5-internal/pr-2496

@@ -0,0 +1 @@
+Removed deprecated internal feature config API endpoints

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
         ]
 

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 (..),
@@ -106,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
@@ -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
 

services/brig/src/Brig/IO/Intra.hs

@@ -57,8 +57,8 @@ module Brig.IO.Intra
     getTeamLegalHoldStatus,
     changeTeamStatus,
     getTeamSearchVisibility,
+    getAllFeatureConfigsForUser,
     getVerificationCodeEnabled,
-    getTeamFeatureStatusSndFactorPasswordChallenge,
 
     -- * Legalhold
     guardLegalhold,
@@ -1392,20 +1392,20 @@ getVerificationCodeEnabled tid = do
       paths ["i", "teams", toByteString' tid, "features", featureNameBS @SndFactorPasswordChallengeConfig]
         . expect2xx
 
-getTeamFeatureStatusSndFactorPasswordChallenge ::
+getAllFeatureConfigsForUser ::
   ( MonadReader Env m,
     MonadIO m,
     MonadMask m,
     MonadHttp m,
     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
       )
 

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

services/galley/src/Galley/API/Internal.hs

@@ -130,18 +130,12 @@ type IFeatureAPI =
     -- SearchVisibilityAvailableConfig
     :<|> IFeatureStatusGet SearchVisibilityAvailableConfig
     :<|> IFeatureStatusPut '() SearchVisibilityAvailableConfig
-    :<|> IFeatureStatusDeprecatedGet "" SearchVisibilityAvailableConfig
-    :<|> IFeatureStatusDeprecatedPut "" SearchVisibilityAvailableConfig
     -- ValidateSAMLEmailsConfig
     :<|> IFeatureStatusGet ValidateSAMLEmailsConfig
     :<|> IFeatureStatusPut '() ValidateSAMLEmailsConfig
-    :<|> IFeatureStatusDeprecatedGet "" ValidateSAMLEmailsConfig
-    :<|> IFeatureStatusDeprecatedPut "" ValidateSAMLEmailsConfig
     -- DigitalSignaturesConfig
     :<|> IFeatureStatusGet DigitalSignaturesConfig
     :<|> IFeatureStatusPut '() DigitalSignaturesConfig
-    :<|> IFeatureStatusDeprecatedGet "" DigitalSignaturesConfig
-    :<|> IFeatureStatusDeprecatedPut "" DigitalSignaturesConfig
     -- AppLockConfig
     :<|> IFeatureStatusGet AppLockConfig
     :<|> IFeatureStatusPut '() AppLockConfig
@@ -173,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
 
@@ -226,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 =
@@ -367,10 +357,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)
@@ -408,7 +394,6 @@ internalAPI =
       <@> legalholdWhitelistedTeamsAPI
       <@> iTeamsAPI
       <@> mkNamedAPI @"upsert-one2one" iUpsertOne2OneConversation
-      <@> mkNamedAPI @"feature-config-snd-factor-password-challenge" (getFeatureStatusNoPermissionCheck @Cassandra)
       <@> featureAPI
 
 legalholdWhitelistedTeamsAPI :: API ILegalholdWhitelistedTeamsAPI GalleyEffects
@@ -457,16 +442,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)
@@ -488,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

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