Fix feature flag defaults

changelog.d/3-bug-fixes/flag-defaults

@@ -0,0 +1 @@
+Fix feature flag default calculation for `mlsMigration` and `enforceFileDownloadLocation`

charts/galley/templates/configmap.yaml

@@ -106,6 +106,10 @@ data:
         fileSharing:
           {{- toYaml .settings.featureFlags.fileSharing | nindent 10 }}
         {{- end }}
+        {{- if .settings.featureFlags.enforceFileDownloadLocation }}
+        enforceFileDownloadLocation:
+          {{- toYaml .settings.featureFlags.enforceFileDownloadLocation | nindent 10 }}
+        {{- end }}
         {{- if .settings.featureFlags.sndFactorPasswordChallenge }}
         sndFactorPasswordChallenge:
           {{- toYaml .settings.featureFlags.sndFactorPasswordChallenge | nindent 10 }}

charts/galley/values.yaml

@@ -93,6 +93,12 @@ config:
         defaults:
           lockStatus: unlocked
           status: enabled
+      enforceFileDownloadLocation:
+        defaults:
+          lockStatus: locked
+          status: disabled
+          config:
+            enforcedTimeoutSeconds: ""
       legalhold: disabled-by-default
       mls:
         defaults:

hack/helm_vars/wire-server/values.yaml.gotmpl

@@ -269,6 +269,12 @@ galley:
           status: enabled
           config:
             domains: ["example.com"]
+        enforceFileDownloadLocation:
+          defaults:
+            status: disabled
+            lockStatus: unlocked
+            config:
+              enforcedDownloadLocation: "downloads"
         mlsMigration:
           defaults:
             status: enabled

integration/integration.cabal

@@ -123,8 +123,26 @@ library
     Test.Errors
     Test.ExternalPartner
     Test.FeatureFlags
+    Test.FeatureFlags.AppLock
+    Test.FeatureFlags.ClassifiedDomains
+    Test.FeatureFlags.ConferenceCalling
+    Test.FeatureFlags.DigitalSignatures
+    Test.FeatureFlags.EnforceFileDownloadLocation
+    Test.FeatureFlags.FileSharing
+    Test.FeatureFlags.GuestLinks
+    Test.FeatureFlags.LegalHold
+    Test.FeatureFlags.Mls
+    Test.FeatureFlags.MlsE2EId
+    Test.FeatureFlags.MlsMigration
+    Test.FeatureFlags.OutlookCalIntegration
+    Test.FeatureFlags.SearchVisibilityAvailable
+    Test.FeatureFlags.SearchVisibilityInbound
+    Test.FeatureFlags.SelfDeletingMessages
+    Test.FeatureFlags.SndFactorPasswordChallenge
+    Test.FeatureFlags.SSO
     Test.FeatureFlags.User
     Test.FeatureFlags.Util
+    Test.FeatureFlags.ValidateSAMLEmails
     Test.Federation
     Test.Federator
     Test.LegalHold

integration/test/Test/FeatureFlags/AppLock.hs

@@ -0,0 +1,31 @@
+module Test.FeatureFlags.AppLock where
+
+import qualified Data.Aeson as A
+import Test.FeatureFlags.Util
+import Testlib.Prelude
+
+testPatchAppLock :: (HasCallStack) => App ()
+testPatchAppLock = do
+  checkPatch OwnDomain "appLock"
+    $ object ["lockStatus" .= "locked"]
+  checkPatch OwnDomain "appLock"
+    $ object ["status" .= "disabled"]
+  checkPatch OwnDomain "appLock"
+    $ object ["lockStatus" .= "locked", "status" .= "disabled"]
+  checkPatch OwnDomain "appLock"
+    $ object
+      [ "lockStatus" .= "unlocked",
+        "config"
+          .= object
+            [ "enforceAppLock" .= True,
+              "inactivityTimeoutSecs" .= A.Number 120
+            ]
+      ]
+  checkPatch OwnDomain "appLock"
+    $ object
+      [ "config"
+          .= object
+            [ "enforceAppLock" .= True,
+              "inactivityTimeoutSecs" .= A.Number 240
+            ]
+      ]

integration/test/Test/FeatureFlags/ClassifiedDomains.hs

@@ -0,0 +1,18 @@
+module Test.FeatureFlags.ClassifiedDomains where
+
+import SetupHelpers
+import Test.FeatureFlags.Util
+import Testlib.Prelude
+
+testClassifiedDomainsEnabled :: (HasCallStack) => App ()
+testClassifiedDomainsEnabled = do
+  (_, tid, m : _) <- createTeam OwnDomain 2
+  expected <- enabled & setField "config.domains" ["example.com"]
+  checkFeature "classifiedDomains" m tid expected
+
+testClassifiedDomainsDisabled :: (HasCallStack) => App ()
+testClassifiedDomainsDisabled = do
+  withModifiedBackend def {galleyCfg = setField "settings.featureFlags.classifiedDomains" (object ["status" .= "disabled", "config" .= object ["domains" .= ["example.com"]]])} $ \domain -> do
+    (_, tid, m : _) <- createTeam domain 2
+    expected <- disabled & setField "config.domains" ["example.com"]
+    checkFeature "classifiedDomains" m tid expected

integration/test/Test/FeatureFlags/ConferenceCalling.hs

@@ -0,0 +1,26 @@
+module Test.FeatureFlags.ConferenceCalling where
+
+import Test.FeatureFlags.Util
+import Testlib.Prelude
+
+testPatchConferenceCalling :: (HasCallStack) => App ()
+testPatchConferenceCalling = do
+  checkPatch OwnDomain "conferenceCalling"
+    $ object ["lockStatus" .= "locked"]
+  checkPatch OwnDomain "conferenceCalling"
+    $ object ["status" .= "disabled"]
+  checkPatch OwnDomain "conferenceCalling"
+    $ object ["lockStatus" .= "locked", "status" .= "disabled"]
+  checkPatch OwnDomain "conferenceCalling"
+    $ object
+      [ "lockStatus" .= "unlocked",
+        "config" .= object ["useSFTForOneToOneCalls" .= toJSON True]
+      ]
+
+testConferenceCalling :: (HasCallStack) => APIAccess -> App ()
+testConferenceCalling access = do
+  runFeatureTests OwnDomain access
+    $ mkFeatureTests "conferenceCalling"
+    & addUpdate (confCalling def {sft = toJSON True})
+    & addUpdate (confCalling def {sft = toJSON False})
+    & addInvalidUpdate (confCalling def {sft = toJSON (0 :: Int)})

integration/test/Test/FeatureFlags/DigitalSignatures.hs

@@ -0,0 +1,15 @@
+module Test.FeatureFlags.DigitalSignatures where
+
+import SetupHelpers
+import Test.FeatureFlags.Util
+import Testlib.Prelude
+
+testPatchDigitalSignatures :: (HasCallStack) => App ()
+testPatchDigitalSignatures = checkPatch OwnDomain "digitalSignatures" enabled
+
+testDigitalSignaturesInternal :: (HasCallStack) => App ()
+testDigitalSignaturesInternal = do
+  (alice, tid, _) <- createTeam OwnDomain 0
+  withWebSocket alice $ \ws -> do
+    setFlag InternalAPI ws tid "digitalSignatures" disabled
+    setFlag InternalAPI ws tid "digitalSignatures" enabled

integration/test/Test/FeatureFlags/EnforceFileDownloadLocation.hs

@@ -0,0 +1,55 @@
+module Test.FeatureFlags.EnforceFileDownloadLocation where
+
+import qualified API.GalleyInternal as Internal
+import SetupHelpers
+import Test.FeatureFlags.Util
+import Testlib.Prelude
+
+testPatchEnforceFileDownloadLocation :: (HasCallStack) => App ()
+testPatchEnforceFileDownloadLocation = do
+  checkPatch OwnDomain "enforceFileDownloadLocation"
+    $ object ["lockStatus" .= "unlocked"]
+  checkPatch OwnDomain "enforceFileDownloadLocation"
+    $ object ["status" .= "enabled"]
+  checkPatch OwnDomain "enforceFileDownloadLocation"
+    $ object ["lockStatus" .= "unlocked", "status" .= "enabled"]
+  checkPatch OwnDomain "enforceFileDownloadLocation"
+    $ object ["lockStatus" .= "locked", "config" .= object []]
+  checkPatch OwnDomain "enforceFileDownloadLocation"
+    $ object ["config" .= object ["enforcedDownloadLocation" .= "/tmp"]]
+
+  do
+    (user, tid, _) <- createTeam OwnDomain 0
+    bindResponse
+      ( Internal.patchTeamFeature
+          user
+          tid
+          "enforceFileDownloadLocation"
+          (object ["config" .= object ["enforcedDownloadLocation" .= ""]])
+      )
+      $ \resp -> do
+        resp.status `shouldMatchInt` 400
+        resp.json %. "label" `shouldMatch` "empty-download-location"
+
+testEnforceDownloadLocation :: (HasCallStack) => APIAccess -> App ()
+testEnforceDownloadLocation access = do
+  mkFeatureTests
+    "enforceFileDownloadLocation"
+    & addUpdate
+      ( object
+          [ "status" .= "enabled",
+            "config" .= object ["enforcedDownloadLocation" .= "/tmp"]
+          ]
+      )
+    & addUpdate
+      (object ["status" .= "disabled", "config" .= object []])
+    & addInvalidUpdate
+      ( object
+          [ "status" .= "enabled",
+            "config"
+              .= object
+                [ "enforcedDownloadLocation" .= object []
+                ]
+          ]
+      )
+    & runFeatureTests OwnDomain access

integration/test/Test/FeatureFlags/FileSharing.hs

@@ -0,0 +1,14 @@
+module Test.FeatureFlags.FileSharing where
+
+import Test.FeatureFlags.Util
+import Testlib.Prelude
+
+testPatchFileSharing :: (HasCallStack) => App ()
+testPatchFileSharing = checkPatch OwnDomain "fileSharing" disabled
+
+testFileSharing :: (HasCallStack) => APIAccess -> App ()
+testFileSharing access =
+  mkFeatureTests "fileSharing"
+    & addUpdate disabled
+    & addUpdate enabled
+    & runFeatureTests OwnDomain access

integration/test/Test/FeatureFlags/GuestLinks.hs

@@ -0,0 +1,14 @@
+module Test.FeatureFlags.GuestLinks where
+
+import Test.FeatureFlags.Util
+import Testlib.Prelude
+
+testConversationGuestLinks :: (HasCallStack) => APIAccess -> App ()
+testConversationGuestLinks access =
+  mkFeatureTests "conversationGuestLinks"
+    & addUpdate disabled
+    & addUpdate enabled
+    & runFeatureTests OwnDomain access
+
+testPatchGuestLinks :: (HasCallStack) => App ()
+testPatchGuestLinks = checkPatch OwnDomain "conversationGuestLinks" disabled

integration/test/Test/FeatureFlags/LegalHold.hs

@@ -0,0 +1,142 @@
+module Test.FeatureFlags.LegalHold where
+
+import qualified API.Galley as Public
+import qualified API.GalleyInternal as Internal
+import Control.Monad.Codensity (Codensity (runCodensity))
+import Control.Monad.Reader
+import SetupHelpers
+import Test.FeatureFlags.Util
+import Testlib.Prelude
+import Testlib.ResourcePool (acquireResources)
+
+testLegalholdDisabledByDefault :: (HasCallStack) => App ()
+testLegalholdDisabledByDefault = do
+  let put uid tid st = Internal.setTeamFeatureConfig uid tid "legalhold" (object ["status" .= st]) >>= assertSuccess
+  let patch uid tid st = Internal.setTeamFeatureStatus uid tid "legalhold" st >>= assertSuccess
+  forM_ [put, patch] $ \setFeatureStatus -> do
+    withModifiedBackend
+      def {galleyCfg = setField "settings.featureFlags.legalhold" "disabled-by-default"}
+      $ \domain -> do
+        (owner, tid, m : _) <- createTeam domain 2
+        nonMember <- randomUser domain def
+        assertForbidden =<< Public.getTeamFeature nonMember tid "legalhold"
+        -- Test default
+        checkFeature "legalhold" m tid disabled
+        -- Test override
+        setFeatureStatus owner tid "enabled"
+        checkFeature "legalhold" owner tid enabled
+        setFeatureStatus owner tid "disabled"
+        checkFeature "legalhold" owner tid disabled
+
+-- always disabled
+testLegalholdDisabledPermanently :: (HasCallStack) => App ()
+testLegalholdDisabledPermanently = do
+  let cfgLhDisabledPermanently =
+        def
+          { galleyCfg = setField "settings.featureFlags.legalhold" "disabled-permanently"
+          }
+      cfgLhDisabledByDefault =
+        def
+          { galleyCfg = setField "settings.featureFlags.legalhold" "disabled-by-default"
+          }
+  resourcePool <- asks (.resourcePool)
+  runCodensity (acquireResources 1 resourcePool) $ \[testBackend] -> do
+    let domain = testBackend.berDomain
+
+    -- Happy case: DB has no config for the team
+    runCodensity (startDynamicBackend testBackend cfgLhDisabledPermanently) $ \_ -> do
+      (owner, tid, _) <- createTeam domain 1
+      checkFeature "legalhold" owner tid disabled
+      assertStatus 403 =<< Internal.setTeamFeatureStatus domain tid "legalhold" "enabled"
+      assertStatus 403 =<< Internal.setTeamFeatureConfig domain tid "legalhold" (object ["status" .= "enabled"])
+
+    -- Interesting case: The team had LH enabled before backend config was
+    -- changed to disabled-permanently
+    (owner, tid) <- runCodensity (startDynamicBackend testBackend cfgLhDisabledByDefault) $ \_ -> do
+      (owner, tid, _) <- createTeam domain 1
+      checkFeature "legalhold" owner tid disabled
+      assertSuccess =<< Internal.setTeamFeatureStatus domain tid "legalhold" "enabled"
+      checkFeature "legalhold" owner tid enabled
+      pure (owner, tid)
+
+    runCodensity (startDynamicBackend testBackend cfgLhDisabledPermanently) $ \_ -> do
+      checkFeature "legalhold" owner tid disabled
+
+-- enabled if team is allow listed, disabled in any other case
+testLegalholdWhitelistTeamsAndImplicitConsent :: (HasCallStack) => App ()
+testLegalholdWhitelistTeamsAndImplicitConsent = do
+  let cfgLhWhitelistTeamsAndImplicitConsent =
+        def
+          { galleyCfg = setField "settings.featureFlags.legalhold" "whitelist-teams-and-implicit-consent"
+          }
+      cfgLhDisabledByDefault =
+        def
+          { galleyCfg = setField "settings.featureFlags.legalhold" "disabled-by-default"
+          }
+  resourcePool <- asks (.resourcePool)
+  runCodensity (acquireResources 1 resourcePool) $ \[testBackend] -> do
+    let domain = testBackend.berDomain
+
+    -- Happy case: DB has no config for the team
+    (owner, tid) <- runCodensity (startDynamicBackend testBackend cfgLhWhitelistTeamsAndImplicitConsent) $ \_ -> do
+      (owner, tid, _) <- createTeam domain 1
+      checkFeature "legalhold" owner tid disabled
+      Internal.legalholdWhitelistTeam tid owner >>= assertSuccess
+      checkFeature "legalhold" owner tid enabled
+
+      -- Disabling it doesn't work
+      assertStatus 403 =<< Internal.setTeamFeatureStatus domain tid "legalhold" "disabled"
+      assertStatus 403 =<< Internal.setTeamFeatureConfig domain tid "legalhold" (object ["status" .= "disabled"])
+      checkFeature "legalhold" owner tid enabled
+      pure (owner, tid)
+
+    -- Interesting case: The team had LH disabled before backend config was
+    -- changed to "whitelist-teams-and-implicit-consent". It should still show
+    -- enabled when the config gets changed.
+    runCodensity (startDynamicBackend testBackend cfgLhDisabledByDefault) $ \_ -> do
+      checkFeature "legalhold" owner tid disabled
+      assertSuccess =<< Internal.setTeamFeatureStatus domain tid "legalhold" "disabled"
+      checkFeature "legalhold" owner tid disabled
+
+    runCodensity (startDynamicBackend testBackend cfgLhWhitelistTeamsAndImplicitConsent) $ \_ -> do
+      checkFeature "legalhold" owner tid enabled
+
+testExposeInvitationURLsToTeamAdminConfig :: (HasCallStack) => App ()
+testExposeInvitationURLsToTeamAdminConfig = do
+  let cfgExposeInvitationURLsTeamAllowlist tids =
+        def
+          { galleyCfg = setField "settings.exposeInvitationURLsTeamAllowlist" tids
+          }
+  resourcePool <- asks (.resourcePool)
+  runCodensity (acquireResources 1 resourcePool) $ \[testBackend] -> do
+    let domain = testBackend.berDomain
+
+    let testNoAllowlistEntry = runCodensity (startDynamicBackend testBackend $ cfgExposeInvitationURLsTeamAllowlist ([] :: [String])) $ \_ -> do
+          (owner, tid, _) <- createTeam domain 1
+          checkFeature "exposeInvitationURLsToTeamAdmin" owner tid disabledLocked
+          -- here we get a response with HTTP status 200 and feature status unchanged (disabled), which we find weird, but we're just testing the current behavior
+          -- a team that is not in the allow list cannot enable the feature, it will always be disabled and locked
+          -- even though the internal API request to enable it succeeds
+          assertSuccess =<< Internal.setTeamFeatureStatus domain tid "exposeInvitationURLsToTeamAdmin" "enabled"
+          checkFeature "exposeInvitationURLsToTeamAdmin" owner tid disabledLocked
+          -- however, a request to the public API will fail
+          assertStatus 409 =<< Public.setTeamFeatureConfig owner tid "exposeInvitationURLsToTeamAdmin" (object ["status" .= "enabled"])
+          assertSuccess =<< Internal.setTeamFeatureStatus domain tid "exposeInvitationURLsToTeamAdmin" "disabled"
+          pure (owner, tid)
+
+    -- Happy case: DB has no config for the team
+    (owner, tid) <- testNoAllowlistEntry
+
+    -- Interesting case: The team is in the allow list
+    runCodensity (startDynamicBackend testBackend $ cfgExposeInvitationURLsTeamAllowlist [tid]) $ \_ -> do
+      -- when the team is in the allow list the lock status is implicitly unlocked
+      checkFeature "exposeInvitationURLsToTeamAdmin" owner tid disabled
+      assertSuccess =<< Internal.setTeamFeatureStatus domain tid "exposeInvitationURLsToTeamAdmin" "enabled"
+      checkFeature "exposeInvitationURLsToTeamAdmin" owner tid enabled
+      assertSuccess =<< Internal.setTeamFeatureStatus domain tid "exposeInvitationURLsToTeamAdmin" "disabled"
+      checkFeature "exposeInvitationURLsToTeamAdmin" owner tid disabled
+      assertSuccess =<< Internal.setTeamFeatureStatus domain tid "exposeInvitationURLsToTeamAdmin" "enabled"
+      checkFeature "exposeInvitationURLsToTeamAdmin" owner tid enabled
+
+    -- Interesting case: The team had the feature enabled but is not in allow list
+    void testNoAllowlistEntry