Fix v3 leak in get-conversation endpoints

changelog.d/1-api-changes/get-conversation-v3-leak

@@ -0,0 +1 @@
+The unqualified `GET /conversations/:id` endpoint has been removed from API v3, and is restored to the previous behaviour of returning a Conversation using the v2 schema. Similarly, its qualified counterpart `GET /conversations/:domain/:id` now returns a v2 Conversation when accessed through API v2.

libs/wire-api/src/Wire/API/Routes/Public/Galley/Conversation.hs

@@ -102,12 +102,13 @@ type ConversationAPI =
   Named
     "get-unqualified-conversation"
     ( Summary "Get a conversation by ID"
+        :> Until 'V3
         :> CanThrow 'ConvNotFound
         :> CanThrow 'ConvAccessDenied
         :> ZLocalUser
         :> "conversations"
         :> Capture "cnv" ConvId
-        :> Get '[Servant.JSON] Conversation
+        :> MultiVerb1 'GET '[JSON] (VersionedRespond 'V2 200 "Conversation" Conversation)
     )
     :<|> Named
            "get-unqualified-conversation-legalhold-alias"
@@ -120,18 +121,31 @@ type ConversationAPI =
                :> "legalhold"
                :> "conversations"
                :> Capture "cnv" ConvId
-               :> Get '[Servant.JSON] Conversation
+               :> MultiVerb1 'GET '[JSON] (VersionedRespond 'V2 200 "Conversation" Conversation)
+           )
+    :<|> Named
+           "get-conversation@v2"
+           ( Summary "Get a conversation by ID"
+               :> Until 'V3
+               :> MakesFederatedCall 'Galley "get-conversations"
+               :> CanThrow 'ConvNotFound
+               :> CanThrow 'ConvAccessDenied
+               :> ZLocalUser
+               :> "conversations"
+               :> QualifiedCapture "cnv" ConvId
+               :> MultiVerb1 'GET '[JSON] (VersionedRespond 'V2 200 "Conversation" Conversation)
            )
     :<|> Named
            "get-conversation"
            ( Summary "Get a conversation by ID"
+               :> From 'V3
                :> MakesFederatedCall 'Galley "get-conversations"
                :> CanThrow 'ConvNotFound
                :> CanThrow 'ConvAccessDenied
                :> ZLocalUser
                :> "conversations"
                :> QualifiedCapture "cnv" ConvId
-               :> Get '[Servant.JSON] Conversation
+               :> Get '[JSON] Conversation
            )
     :<|> Named
            "get-conversation-roles"

services/brig/src/Brig/Effects/GalleyProvider.hs

@@ -7,6 +7,7 @@ import Brig.Team.Types (ShowOrHideInvitationUrl (..))
 import qualified Data.Currency as Currency
 import Data.Id
 import Data.Json.Util (UTCTimeMillis)
+import Data.Qualified
 import qualified Galley.Types.Teams.Intra as Team
 import Imports
 import qualified Network.Wai.Utilities.Error as Wai
@@ -25,7 +26,7 @@ data GalleyProvider m a where
     GalleyProvider m ()
   GetConv ::
     UserId ->
-    ConvId ->
+    Local ConvId ->
     GalleyProvider m (Maybe Conversation)
   GetTeamConv ::
     UserId ->

services/brig/src/Brig/Effects/GalleyProvider/RPC.hs

@@ -18,6 +18,7 @@ import Data.Coerce (coerce)
 import qualified Data.Currency as Currency
 import Data.Id
 import Data.Json.Util (UTCTimeMillis)
+import Data.Qualified
 import Data.Range
 import qualified Galley.Types.Teams as Team
 import qualified Galley.Types.Teams.Intra as Team
@@ -27,8 +28,10 @@ import Network.HTTP.Types.Status
 import qualified Network.Wai.Utilities.Error as Wai
 import Polysemy
 import Polysemy.Error
+import Servant.API (toHeader)
 import System.Logger (Msg, field, msg, val)
 import Wire.API.Conversation hiding (Member)
+import Wire.API.Routes.Version
 import Wire.API.Team
 import qualified Wire.API.Team.Conversation as Conv
 import Wire.API.Team.Feature
@@ -84,7 +87,7 @@ createSelfConv u = do
   void $ ServiceRPC.request @'Galley POST req
   where
     req =
-      path "/conversations/self"
+      paths ["v" <> toHeader (maxBound :: Version), "conversations", "self"]
         . zUser u
         . expect2xx
 
@@ -97,20 +100,26 @@ getConv ::
      ]
     r =>
   UserId ->
-  ConvId ->
+  Local ConvId ->
   Sem r (Maybe Conversation)
-getConv usr cnv = do
+getConv usr lcnv = do
   debug $
     remote "galley"
-      . field "conv" (toByteString cnv)
+      . field "domain" (toByteString (tDomain lcnv))
+      . field "conv" (toByteString (tUnqualified lcnv))
       . msg (val "Getting conversation")
   rs <- ServiceRPC.request @'Galley GET req
   case Bilge.statusCode rs of
     200 -> Just <$> decodeBodyOrThrow "galley" rs
     _ -> pure Nothing
   where
     req =
-      paths ["conversations", toByteString' cnv]
+      paths
+        [ "v" <> toHeader (maxBound :: Version),
+          "conversations",
+          toByteString' (tDomain lcnv),
+          toByteString' (tUnqualified lcnv)
+        ]
         . zUser usr
         . expect [status200, status404]
 
@@ -137,7 +146,13 @@ getTeamConv usr tid cnv = do
     _ -> pure Nothing
   where
     req =
-      paths ["teams", toByteString' tid, "conversations", toByteString' cnv]
+      paths
+        [ "v" <> toHeader (maxBound :: Version),
+          "teams",
+          toByteString' tid,
+          "conversations",
+          toByteString' cnv
+        ]
         . zUser usr
         . expect [status200, status404]
 

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

@@ -897,7 +897,8 @@ addBot zuid zcon cid add = do
   let pid = addBotProvider add
   let sid = addBotService add
   -- Get the conversation and check preconditions
-  cnv <- lift (liftSem $ GalleyProvider.getConv zuid cid) >>= maybeConvNotFound
+  lcid <- qualifyLocal cid
+  cnv <- lift (liftSem $ GalleyProvider.getConv zuid lcid) >>= maybeConvNotFound
   -- Check that the user is a conversation admin and therefore is allowed to add a bot to this conversation.
   -- Note that this precondition is also checked in the internal galley API,
   -- but by having this check here we prevent any (useless) data to be written to the database
@@ -981,7 +982,8 @@ removeBotH (zusr ::: zcon ::: cid ::: bid) = do
 removeBot :: Members '[GalleyProvider] r => UserId -> ConnId -> ConvId -> BotId -> (Handler r) (Maybe Public.RemoveBotResponse)
 removeBot zusr zcon cid bid = do
   -- Get the conversation and check preconditions
-  cnv <- lift (liftSem $ GalleyProvider.getConv zusr cid) >>= maybeConvNotFound
+  lcid <- qualifyLocal cid
+  cnv <- lift (liftSem $ GalleyProvider.getConv zusr lcid) >>= maybeConvNotFound
   -- Check that the user is a conversation admin and therefore is allowed to remove a bot from the conversation.
   -- Note that this precondition is also checked in the internal galley API.
   -- However, in case we refine the roles model in the future, this check might not be granular enough.

services/brig/test/integration/API/Provider.hs

@@ -734,7 +734,7 @@ testDeleteConvBotTeam config db brig galley cannon = withTestService config db b
     svcAssertConvDelete buf quid2 qcid
   -- Check that the conversation no longer exists
   forM_ [uid1, uid2] $ \uid ->
-    getConversation galley uid cid !!! const 404 === statusCode
+    getConversationQualified galley uid qcid !!! const 404 === statusCode
   getBotConv galley bid cid !!! const 404 === statusCode
 
 testDeleteTeamBotTeam :: Config -> DB.ClientState -> Brig -> Galley -> Cannon -> Http ()
@@ -758,7 +758,7 @@ testDeleteTeamBotTeam config db brig galley cannon = withTestService config db b
   forM_ [uid1, uid2] $ \uid -> do
     void $ retryWhileN 20 (/= Intra.Deleted) (getStatus brig uid)
     chkStatus brig uid Intra.Deleted
-    aFewTimes 11 (getConversation galley uid cid) ((== 404) . statusCode)
+    aFewTimes 11 (getConversationQualified galley uid qcid) ((== 404) . statusCode)
   -- Check the bot cannot see the conversation either
   getBotConv galley bid cid !!! const 404 === statusCode
 
@@ -2088,7 +2088,7 @@ testMessageBotUtil quid uc cid pid sid sref buf brig galley cannon = do
     assertEqual "id" cid (bcnv ^. Ext.botConvId)
     assertEqual "members" [OtherMember quid Nothing roleNameWireAdmin] (bcnv ^. Ext.botConvMembers)
   -- The user can identify the bot in the member list
-  mems <- fmap cnvMembers . responseJsonError =<< getConversation galley uid cid
+  mems <- fmap cnvMembers . responseJsonError =<< getConversationQualified galley uid qcid
   let other = listToMaybe (cmOthers mems)
   liftIO $ do
     assertEqual "id" (Just buid) (qUnqualified . omQualifiedId <$> other)

services/brig/test/integration/API/User/Auth.hs

@@ -46,7 +46,7 @@ import Data.Handle (Handle (Handle))
 import Data.Id
 import Data.Misc (PlainTextPassword (..))
 import Data.Proxy
-import Data.Qualified (Qualified (qUnqualified))
+import Data.Qualified
 import Data.Range (unsafeRange)
 import qualified Data.Text as Text
 import Data.Text.Ascii (AsciiChars (validate))
@@ -256,7 +256,7 @@ testNginzLegalHold b g n = do
 
   get (apiVersion "v1" . n . paths ["legalhold", "conversations", toByteString' (qUnqualified qconv)] . header "Authorization" ("Bearer " <> toByteString' t)) !!! const 200 === statusCode
 
-  get (n . paths ["conversations", toByteString' (qUnqualified qconv)] . header "Authorization" ("Bearer " <> toByteString' t)) !!! const 200 === statusCode
+  get (apiVersion "v2" . n . paths ["conversations", toByteString' (qUnqualified qconv)] . header "Authorization" ("Bearer " <> toByteString' t)) !!! const 200 === statusCode
 
 -- | Corner case for 'testNginz': when upgrading a wire backend from the old behavior (setting
 -- cookie domain to eg. @*.wire.com@) to the new behavior (leaving cookie domain empty,

services/brig/test/integration/API/User/Connection.hs

@@ -178,11 +178,11 @@ testCreateMutualConnections brig galley = do
   assertConnections brig uid2 [ConnectionStatus uid2 uid1 Accepted]
   case responseJsonMaybe rsp >>= ucConvId of
     Nothing -> liftIO $ assertFailure "incomplete connection"
-    Just (Qualified cnv _) -> do
-      getConversation galley uid1 cnv !!! do
+    Just qcnv -> do
+      getConversationQualified galley uid1 qcnv !!! do
         const 200 === statusCode
         const (Just One2OneConv) === fmap cnvType . responseJsonMaybe
-      getConversation galley uid2 cnv !!! do
+      getConversationQualified galley uid2 qcnv !!! do
         const 200 === statusCode
         const (Just One2OneConv) === fmap cnvType . responseJsonMaybe
 
@@ -304,32 +304,32 @@ testCancelConnection2 brig galley = do
   rsp <- putConnection brig uid1 uid2 Cancelled <!! const 200 === statusCode
   assertConnections brig uid1 [ConnectionStatus uid1 uid2 Cancelled]
   assertConnections brig uid2 [ConnectionStatus uid2 uid1 Cancelled]
-  let Just (Qualified cnv _) = ucConvId =<< responseJsonMaybe rsp
+  let Just qcnv = ucConvId =<< responseJsonMaybe rsp
   -- A cannot see the conversation (due to cancelling)
-  getConversation galley uid1 cnv !!! do
+  getConversationQualified galley uid1 qcnv !!! do
     const 403 === statusCode
   -- B cannot see the conversation
-  getConversation galley uid2 cnv !!! const 403 === statusCode
+  getConversationQualified galley uid2 qcnv !!! const 403 === statusCode
   -- B initiates a connection request himself
   postConnection brig uid2 uid1 !!! const 200 === statusCode
   assertConnections brig uid2 [ConnectionStatus uid2 uid1 Sent]
   assertConnections brig uid1 [ConnectionStatus uid1 uid2 Pending]
   -- B is now a current member of the connect conversation
-  getConversation galley uid2 cnv !!! do
+  getConversationQualified galley uid2 qcnv !!! do
     const 200 === statusCode
     const (Just ConnectConv) === \rs -> do
       conv <- responseJsonMaybe rs
       Just (cnvType conv)
   -- A is a past member, cannot see the conversation
-  getConversation galley uid1 cnv !!! do
+  getConversationQualified galley uid1 qcnv !!! do
     const 403 === statusCode
   -- A finally accepts
   putConnection brig uid1 uid2 Accepted !!! const 200 === statusCode
   assertConnections brig uid1 [ConnectionStatus uid1 uid2 Accepted]
   assertConnections brig uid2 [ConnectionStatus uid2 uid1 Accepted]
-  getConversation galley uid1 cnv !!! do
+  getConversationQualified galley uid1 qcnv !!! do
     const 200 === statusCode
-  getConversation galley uid2 cnv !!! do
+  getConversationQualified galley uid2 qcnv !!! do
     const 200 === statusCode
 
 testCancelConnectionQualified2 :: Brig -> Galley -> Http ()
@@ -483,10 +483,10 @@ testBlockAndResendConnection brig galley = do
   assertConnections brig uid1 [ConnectionStatus uid1 uid2 Accepted]
   assertConnections brig uid2 [ConnectionStatus uid2 uid1 Blocked]
   -- B never accepted and thus does not see the conversation
-  let Just (Qualified cnv _) = ucConvId =<< responseJsonMaybe rsp
-  getConversation galley uid2 cnv !!! const 403 === statusCode
+  let Just qcnv = ucConvId =<< responseJsonMaybe rsp
+  getConversationQualified galley uid2 qcnv !!! const 403 === statusCode
   -- A can see the conversation and is a current member
-  getConversation galley uid1 cnv !!! do
+  getConversationQualified galley uid1 qcnv !!! do
     const 200 === statusCode
 
 testBlockAndResendConnectionQualified :: Brig -> Galley -> Http ()

services/brig/test/integration/Util.hs

@@ -710,13 +710,6 @@ getTeamMember u tid galley =
           . expect2xx
       )
 
-getConversation :: (MonadIO m, MonadHttp m) => Galley -> UserId -> ConvId -> m ResponseLBS
-getConversation galley usr cnv =
-  get $
-    galley
-      . paths ["conversations", toByteString' cnv]
-      . zAuthAccess usr "conn"
-
 getConversationQualified :: (MonadIO m, MonadHttp m) => Galley -> UserId -> Qualified ConvId -> m ResponseLBS
 getConversationQualified galley usr cnv =
   get $

services/galley/src/Galley/API/Public/Conversation.hs

@@ -32,6 +32,7 @@ conversationAPI :: API ConversationAPI GalleyEffects
 conversationAPI =
   mkNamedAPI @"get-unqualified-conversation" getUnqualifiedConversation
     <@> mkNamedAPI @"get-unqualified-conversation-legalhold-alias" getUnqualifiedConversation
+    <@> mkNamedAPI @"get-conversation@v2" (callsFed getConversation)
     <@> mkNamedAPI @"get-conversation" (callsFed getConversation)
     <@> mkNamedAPI @"get-conversation-roles" getConversationRoles
     <@> mkNamedAPI @"get-group-info" (callsFed getGroupInfo)