Revert "Block changes to some user data in mlsE2EId teams (WPB-6189)"

This reverts commit c715642.

integration/integration.cabal

@@ -99,7 +99,6 @@ library
     API.Gundeck
     API.GundeckInternal
     API.Nginz
-    API.Spar
     MLS.Util
     Notifications
     RunAllTests

integration/test/API/Brig.hs

@@ -109,7 +109,6 @@ addUser dom opts = do
           "password" .= fromMaybe defPassword opts.password
         ]
 
--- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/get_users__uid_domain___uid_
 getUser ::
   (HasCallStack, MakesValue user, MakesValue target) =>
   user ->
@@ -148,6 +147,12 @@ deleteUser user = do
   submit "DELETE" $
     req & addJSONObject ["password" .= defPassword]
 
+putHandle :: (HasCallStack, MakesValue user) => user -> String -> App Response
+putHandle user handle = do
+  req <- baseRequest user Brig Versioned "/self/handle"
+  submit "PUT" $
+    req & addJSONObject ["handle" .= handle]
+
 data AddClient = AddClient
   { ctype :: String,
     internal :: Bool,
@@ -393,67 +398,12 @@ replaceKeyPackages cid suites kps = do
       & addQueryParams [("ciphersuites", intercalate "," (map (.code) suites))]
       & addJSONObject ["key_packages" .= map (T.decodeUtf8 . Base64.encode) kps]
 
--- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/get_self
-getSelf :: (HasCallStack, MakesValue caller) => caller -> App Response
-getSelf caller = do
-  req <- baseRequest caller Brig Versioned "/self"
-  submit "GET" req
-
--- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/get_self
--- this is a low-level version of `getSelf` for testing some error conditions.
-getSelf' :: HasCallStack => String -> String -> App Response
-getSelf' domain uid = do
+getSelf :: HasCallStack => String -> String -> App Response
+getSelf domain uid = do
   let user = object ["domain" .= domain, "id" .= uid]
   req <- baseRequest user Brig Versioned "/self"
   submit "GET" req
 
-data PutSelf = PutSelf
-  { accent :: Maybe Int,
-    assets :: Maybe [Value], -- [{"key":"string", "size":"string", "type":"string"}]
-    name :: Maybe String,
-    picture :: Maybe [String]
-  }
-
-instance Default PutSelf where
-  def = PutSelf Nothing Nothing Nothing Nothing
-
--- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/put_self
-putSelf :: (HasCallStack, MakesValue caller) => caller -> PutSelf -> App Response
-putSelf caller body = do
-  req <- baseRequest caller Brig Versioned "/self"
-  submit "PUT" $
-    req
-      & addJSONObject
-        [ "accent_id" .= body.accent,
-          "assets" .= body.assets,
-          "name" .= body.name,
-          "picture" .= body.picture
-        ]
-
--- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/put_self_locale
-putSelfLocale :: (HasCallStack, MakesValue caller) => caller -> String -> App Response
-putSelfLocale caller locale = do
-  req <- baseRequest caller Brig Versioned "/self/locale"
-  submit "PUT" $ req & addJSONObject ["locale" .= locale]
-
--- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/put_users__uid__email
---
--- NOTE: the full process of changing (and confirming) the email address is more complicated.
--- see /services/brig/test/integration for details.
-putSelfEmail :: (HasCallStack, MakesValue caller) => caller -> String -> App Response
-putSelfEmail caller emailAddress = do
-  callerid <- asString $ caller %. "id"
-  req <- baseRequest caller Brig Versioned $ joinHttpPath ["users", callerid, "email"]
-  submit "PUT" $ req & addJSONObject ["email" .= emailAddress]
-
--- | https://staging-nginz-https.zinfra.io/v6/api/swagger-ui/#/default/put_self_handle
--- FUTUREWORK: rename to putSelfHandle for consistency
-putHandle :: (HasCallStack, MakesValue user) => user -> String -> App Response
-putHandle user handle = do
-  req <- baseRequest user Brig Versioned "/self/handle"
-  submit "PUT" $
-    req & addJSONObject ["handle" .= handle]
-
 getUserSupportedProtocols ::
   (HasCallStack, MakesValue user, MakesValue target) =>
   user ->

integration/test/API/BrigInternal.hs

@@ -52,12 +52,6 @@ createUser domain cu = do
                ]
         )
 
--- | https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/brig/#/brig/get_i_users
-getUsersId :: (HasCallStack, MakesValue domain) => domain -> [String] -> App Response
-getUsersId domain ids = do
-  req <- baseRequest domain Brig Unversioned "/i/users"
-  submit "GET" $ req & addQueryParams [("ids", intercalate "," ids)]
-
 data FedConn = FedConn
   { domain :: String,
     searchStrategy :: String,

integration/test/Test/Demo.hs

@@ -105,7 +105,7 @@ testDynamicBackend = do
   ownDomain <- objDomain OwnDomain
   user <- randomUser OwnDomain def
   uid <- objId user
-  bindResponse (BrigP.getSelf' ownDomain uid) $ \resp -> do
+  bindResponse (BrigP.getSelf ownDomain uid) $ \resp -> do
     resp.status `shouldMatchInt` 200
     (resp.json %. "id") `shouldMatch` objId user
 
@@ -117,18 +117,18 @@ testDynamicBackend = do
         resp.json %. "setRestrictUserCreation" `shouldMatch` False
 
     -- user created in own domain should not be found in dynamic backend
-    bindResponse (BrigP.getSelf' dynDomain uid) $ \resp -> do
+    bindResponse (BrigP.getSelf dynDomain uid) $ \resp -> do
       resp.status `shouldMatchInt` 404
 
     -- now create a user in the dynamic backend
     userD1 <- randomUser dynDomain def
     uidD1 <- objId userD1
-    bindResponse (BrigP.getSelf' dynDomain uidD1) $ \resp -> do
+    bindResponse (BrigP.getSelf dynDomain uidD1) $ \resp -> do
       resp.status `shouldMatchInt` 200
       (resp.json %. "id") `shouldMatch` objId userD1
 
     -- the d1 user should not be found in the own domain
-    bindResponse (BrigP.getSelf' ownDomain uidD1) $ \resp -> do
+    bindResponse (BrigP.getSelf ownDomain uidD1) $ \resp -> do
       resp.status `shouldMatchInt` 404
 
 testStartMultipleDynamicBackends :: HasCallStack => App ()

integration/test/Test/User.hs

@@ -1,13 +1,7 @@
-{-# OPTIONS_GHC -Wno-ambiguous-fields #-}
-
 module Test.User where
 
 import API.Brig
 import API.BrigInternal
-import API.GalleyInternal
-import API.Spar
-import qualified Data.UUID as UUID
-import qualified Data.UUID.V4 as UUID
 import SetupHelpers
 import Testlib.Prelude
 
@@ -53,120 +47,3 @@ testCreateUserSupportedProtocols = do
   bindResponse (createUser OwnDomain def {supportedProtocols = Just ["proteus", "mixed"]}) $ \resp -> do
     resp.status `shouldMatchInt` 400
     resp.json %. "label" `shouldMatch` "bad-request"
-
--- | For now this only tests attempts to update /self/handle in E2EId-enabled teams.  More
--- tests can be found under `/services/brig/test/integration` (and should be moved here).
-testUpdateHandle :: HasCallStack => App ()
-testUpdateHandle = do
-  -- create team with one member, without scim, but with `mlsE2EId` enabled.
-  (owner, team, [mem1]) <- createTeam OwnDomain 2
-  mem1id <- asString $ mem1 %. "id"
-
-  let featureName = "mlsE2EId"
-  bindResponse (getTeamFeature owner featureName team) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-    resp.json %. "status" `shouldMatch` "disabled"
-  setTeamFeatureStatus owner team featureName "enabled"
-  bindResponse (getTeamFeature owner featureName team) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-    resp.json %. "status" `shouldMatch` "enabled"
-
-  -- all as expected here.  (see the second time we check this at the end of the test for an
-  -- explanation why we care.)
-  bindResponse (getSelf mem1) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-    resp.json %. "managed_by" `shouldMatch` "wire"
-  bindResponse (getUsersId owner [mem1id]) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-    mb <- (assertOne =<< asList resp.json) %. "managed_by"
-    mb `shouldMatch` "wire"
-
-  -- mem1 attempts to update handle for the first time => success
-  --
-  -- this is desired, because without SCIM users need to pick their own handles initially.
-  -- moreover it is fine, because if `handle == NULL`, no mls E2Eid client certs can be
-  -- created.
-  handle <- UUID.toString <$> liftIO UUID.nextRandom
-  bindResponse (putHandle mem1 handle) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-  bindResponse (putHandle mem1 handle) $ \resp -> do
-    -- idempotency
-    resp.status `shouldMatchInt` 200
-
-  -- mem1 attempts to update handle again => failure
-  handle2 <- UUID.toString <$> liftIO UUID.nextRandom
-  bindResponse (putHandle mem1 handle2) $ \resp -> do
-    resp.status `shouldMatchInt` 403
-    resp.json %. "label" `shouldMatch` "managed-by-scim"
-
-  -- now self thinks it is managed by "scim", so clients can block change attempts to handle,
-  -- display name without adding E2EId-specific logic.  this is just a hack, though: `GET
-  -- /self` is the only place where this is happening, other end-points still report the truth
-  -- that is still stored correctly in the DB.
-  --
-  -- details: https://wearezeta.atlassian.net/browse/WPB-6189.
-  -- FUTUREWORK: figure out a better way for clients to detect E2EId (V6?)
-  bindResponse (getSelf mem1) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-    resp.json %. "managed_by" `shouldMatch` "scim"
-  bindResponse (getUsersId owner [mem1id]) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-    mb <- (assertOne =<< asList resp.json) %. "managed_by"
-    mb `shouldMatch` "wire"
-  bindResponse (getScimTokens owner) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-    resp.json %. "tokens" `shouldMatch` ([] @String)
-
--- | For now this only tests attempts to update one's own display name, email address, or
--- language in E2EId-enabled teams (ie., everything except handle).  More tests can be found
--- under `/services/brig/test/integration` (and should be moved here).
-testUpdateSelf :: HasCallStack => TestUpdateSelfMode -> App ()
-testUpdateSelf mode = do
-  -- create team with one member, without scim, but with `mlsE2EId` enabled.
-  (owner, team, [mem1]) <- createTeam OwnDomain 2
-
-  let featureName = "mlsE2EId"
-  bindResponse (getTeamFeature owner featureName team) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-    resp.json %. "status" `shouldMatch` "disabled"
-  setTeamFeatureStatus owner team featureName "enabled"
-  bindResponse (getTeamFeature owner featureName team) $ \resp -> do
-    resp.status `shouldMatchInt` 200
-    resp.json %. "status" `shouldMatch` "enabled"
-
-  case mode of
-    TestUpdateDisplayName -> do
-      -- blocked unconditionally
-      someDisplayName <- UUID.toString <$> liftIO UUID.nextRandom
-      before <- getSelf mem1
-      bindResponse (putSelf mem1 def {name = Just someDisplayName}) $ \resp -> do
-        resp.status `shouldMatchInt` 403
-        resp.json %. "label" `shouldMatch` "managed-by-scim"
-      after <- getSelf mem1
-      void $ (before.json %. "name") `shouldMatch` (after.json %. "name")
-    TestUpdateEmailAddress -> do
-      -- allowed unconditionally *for owner* (this is a bit off-topic: team members can't
-      -- change their email addresses themselves under any conditions)
-      someEmail <- (<> "@example.com") . UUID.toString <$> liftIO UUID.nextRandom
-      bindResponse (putSelfEmail owner someEmail) $ \resp -> do
-        resp.status `shouldMatchInt` 200
-    TestUpdateLocale -> do
-      -- scim maps "User.preferredLanguage" to brig's locale field.  allowed unconditionally.
-      -- we try two languages to make sure it doesn't work because it's already the active
-      -- locale.
-      forM_ ["uk", "he"] $ \someLocale ->
-        bindResponse (putSelfLocale mem1 someLocale) $ \resp -> do
-          resp.status `shouldMatchInt` 200
-
-data TestUpdateSelfMode
-  = TestUpdateDisplayName
-  | TestUpdateEmailAddress
-  | TestUpdateLocale
-  deriving (Eq, Show, Bounded, Enum)
-
-instance HasTests x => HasTests (TestUpdateSelfMode -> x) where
-  mkTests m n s f x =
-    mconcat
-      [ mkTests m (n <> "[mode=" <> show mode <> "]") s f (x mode)
-        | mode <- [minBound ..]
-      ]

services/brig/src/Brig/API/Internal.hs

@@ -680,13 +680,13 @@ getRichInfoMultiH :: Maybe (CommaSeparatedList UserId) -> (Handler r) [(UserId,
 getRichInfoMultiH (maybe [] fromCommaSeparatedList -> uids) =
   lift $ wrapClient $ API.lookupRichInfoMultiUsers uids
 
-updateHandleH :: Member GalleyProvider r => UserId -> HandleUpdate -> (Handler r) NoContent
+updateHandleH :: UserId -> HandleUpdate -> (Handler r) NoContent
 updateHandleH uid (HandleUpdate handleUpd) =
   NoContent <$ do
     handle <- validateHandle handleUpd
     API.changeHandle uid Nothing handle API.AllowSCIMUpdates !>> changeHandleError
 
-updateUserNameH :: Member GalleyProvider r => UserId -> NameUpdate -> (Handler r) NoContent
+updateUserNameH :: UserId -> NameUpdate -> (Handler r) NoContent
 updateUserNameH uid (NameUpdate nameUpd) =
   NoContent <$ do
     name <- either (const $ throwStd (errorToWai @'E.InvalidUser)) pure $ mkName nameUpd

services/brig/src/Brig/API/Public.hs

@@ -746,11 +746,10 @@ createUser (Public.NewUserPublic new) = lift . runExceptT $ do
       Public.NewTeamMemberSSO _ ->
         Team.sendMemberWelcomeMail e t n l
 
-getSelf :: Member GalleyProvider r => UserId -> (Handler r) Public.SelfProfile
+getSelf :: UserId -> (Handler r) Public.SelfProfile
 getSelf self =
   lift (API.lookupSelfProfile self)
     >>= ifNothing (errorToWai @'E.UserNotFound)
-    >>= lift . liftSem . API.hackForBlockingHandleChangeForE2EIdTeams
 
 getUserUnqualifiedH ::
   (Member GalleyProvider r) =>
@@ -862,7 +861,7 @@ newtype GetActivationCodeResp
 instance ToJSON GetActivationCodeResp where
   toJSON (GetActivationCodeResp (k, c)) = object ["key" .= k, "code" .= c]
 
-updateUser :: Member GalleyProvider r => UserId -> ConnId -> Public.UserUpdate -> (Handler r) (Maybe Public.UpdateProfileError)
+updateUser :: UserId -> ConnId -> Public.UserUpdate -> (Handler r) (Maybe Public.UpdateProfileError)
 updateUser uid conn uu = do
   eithErr <- lift $ runExceptT $ API.updateUser uid (Just conn) uu API.ForbidSCIMUpdates
   pure $ either Just (const Nothing) eithErr
@@ -933,7 +932,7 @@ getHandleInfoUnqualifiedH self handle = do
   Public.UserHandleInfo . Public.profileQualifiedId
     <$$> Handle.getHandleInfo self (Qualified handle domain)
 
-changeHandle :: Member GalleyProvider r => UserId -> ConnId -> Public.HandleUpdate -> (Handler r) (Maybe Public.ChangeHandleError)
+changeHandle :: UserId -> ConnId -> Public.HandleUpdate -> (Handler r) (Maybe Public.ChangeHandleError)
 changeHandle u conn (Public.HandleUpdate h) = lift . exceptTToMaybe $ do
   handle <- maybe (throwError Public.ChangeHandleInvalid) pure $ parseHandle h
   API.changeHandle u (Just conn) handle API.ForbidSCIMUpdates