Fix: Ensure asset download URL uses S3DownloadEndpoint when provided (#…

…2163)

changelog.d/5-internal/bump-aeson-amazonka

@@ -1 +1 @@
-Bump aeson to v2.0.3.0 and update amazonka fork from upstream repository.  (#2153, #2157)
+Bump aeson to v2.0.3.0 and update amazonka fork from upstream repository.  (#2153, #2157, #2163)

charts/cargohold/templates/tests/cargohold-integration.yaml

@@ -23,11 +23,17 @@ spec:
     - name: "cargohold-config"
       mountPath: "/etc/wire/cargohold/conf"
     env:
-    # these dummy values are necessary for Amazonka's "Discover"
+    # these values are necessary for Amazonka's "Discover"
     - name: AWS_ACCESS_KEY_ID
-      value: "dummy"
+      valueFrom:
+        secretKeyRef:
+          name: cargohold
+          key: awsKeyId
     - name: AWS_SECRET_ACCESS_KEY
-      value: "dummy"
+      valueFrom:
+        secretKeyRef:
+          name: cargohold
+          key: awsSecretKey
     - name: AWS_REGION
-      value: "eu-west-1"
+      value: "{{ .Values.config.aws.region }}"
   restartPolicy: Never

services/cargohold/src/CargoHold/AWS.hs

@@ -22,7 +22,7 @@ module CargoHold.AWS
   ( -- * Monad
     Env,
     mkEnv,
-    useDownloadEndpoint,
+    amazonkaEnvWithDownloadEndpoint,
     Amazon,
     amazonkaEnv,
     execute,
@@ -70,9 +70,9 @@ data Env = Env
 makeLenses ''Env
 
 -- | Override the endpoint in the '_amazonkaEnv' with '_amazonkaDownloadEndpoint'.
-useDownloadEndpoint :: Env -> Env
-useDownloadEndpoint e =
-  e & amazonkaEnv %~ AWS.override (setAWSEndpoint (e ^. amazonkaDownloadEndpoint))
+amazonkaEnvWithDownloadEndpoint :: Env -> AWS.Env
+amazonkaEnvWithDownloadEndpoint e =
+  AWS.override (setAWSEndpoint (e ^. amazonkaDownloadEndpoint)) (e ^. amazonkaEnv)
 
 setAWSEndpoint :: AWSEndpoint -> AWS.Service -> AWS.Service
 setAWSEndpoint e = AWS.setEndpoint (_awsSecure e) (_awsHost e) (_awsPort e)

services/cargohold/src/CargoHold/S3.hs

@@ -41,7 +41,7 @@ import Amazonka hiding (Error, ToByteString, (.=))
 import Amazonka.S3
 import Amazonka.S3.Lens
 import CargoHold.API.Error
-import CargoHold.AWS (amazonkaEnv)
+import CargoHold.AWS (amazonkaEnvWithDownloadEndpoint)
 import qualified CargoHold.AWS as AWS
 import CargoHold.App hiding (Env, Handler)
 import CargoHold.Options
@@ -215,8 +215,7 @@ signedURL path = do
   ttl <- view (settings . setDownloadLinkTTL)
   let req = newGetObject (BucketName b) (ObjectKey . Text.decodeLatin1 $ toByteString' path)
   signed <-
-    AWS.execute (AWS.useDownloadEndpoint e) $
-      presignURL (e ^. amazonkaEnv) now (Seconds (fromIntegral ttl)) req
+    presignURL (amazonkaEnvWithDownloadEndpoint e) now (Seconds (fromIntegral ttl)) req
   toUri signed
   where
     toUri x = case parseURI strictURIParserOptions x of

services/cargohold/test/integration/API.hs

@@ -23,6 +23,7 @@ import API.Util
 import Bilge hiding (body)
 import Bilge.Assert
 import CargoHold.API.Error
+import CargoHold.Options (awsS3DownloadEndpoint, optAws)
 import CargoHold.Types
 import qualified CargoHold.Types.V3 as V3
 import qualified Codec.MIME.Type as MIME
@@ -43,13 +44,15 @@ import Data.UUID.V4
 import Federator.MockServer
 import Imports hiding (head)
 import Network.HTTP.Client (parseUrlThrow)
+import qualified Network.HTTP.Client as HTTP
 import Network.HTTP.Media ((//))
 import qualified Network.HTTP.Types as HTTP
 import Network.Wai.Utilities (Error (label))
 import qualified Network.Wai.Utilities.Error as Wai
 import Test.Tasty
 import Test.Tasty.HUnit
 import TestSetup
+import Util.Options
 import Wire.API.Federation.API.Cargohold
 import Wire.API.Federation.Component
 
@@ -63,7 +66,8 @@ tests s =
           test s "download with accept header" testDownloadWithAcceptHeader,
           test s "tokens" testSimpleTokens,
           test s "s3-upstream-closed" testSimpleS3ClosedConnectionReuse,
-          test s "client-compatibility" testUploadCompatibility
+          test s "client-compatibility" testUploadCompatibility,
+          test s "download url override" testDownloadURLOverride
         ],
       testGroup
         "remote"
@@ -219,6 +223,35 @@ testSimpleS3ClosedConnectionReuse = go >> wait >> go
       uploadSimple (path "/assets/v3") uid sets part2
         !!! const 201 === statusCode
 
+testDownloadURLOverride :: TestM ()
+testDownloadURLOverride = do
+  -- This is a .example domain, it shouldn't resolve. But it is also not
+  -- supposed to be used by cargohold to make connections.
+  let downloadEndpoint = "external-s3-url.example"
+  withSettingsOverrides (optAws . awsS3DownloadEndpoint ?~ AWSEndpoint downloadEndpoint True 443) $ do
+    uid <- liftIO $ Id <$> nextRandom
+
+    -- Upload, should work, shouldn't try to use the S3DownloadEndpoint
+    let bdy = (applicationText, "Hello World")
+    uploadRes <-
+      uploadSimple (path "/assets/v3") uid V3.defAssetSettings bdy
+        <!! const 201 === statusCode
+    let loc = decodeHeaderOrFail "Location" uploadRes :: ByteString
+    let Just ast = responseJsonMaybe @V3.Asset uploadRes
+    let Just tok = view V3.assetToken ast
+
+    -- Lookup with token and get download URL. Should return the
+    -- S3DownloadEndpoint, but not try to use it.
+    downloadURLRes <-
+      downloadAsset uid loc (Just tok) <!! do
+        const 302 === statusCode
+        const Nothing === responseBody
+    downloadURL <- parseUrlThrow (C8.unpack (getHeader' "Location" downloadURLRes))
+    liftIO $ do
+      assertEqual "download host" downloadEndpoint (HTTP.host downloadURL)
+      assertEqual "download port" 443 (HTTP.port downloadURL)
+      assertEqual "download secure" True (HTTP.secure downloadURL)
+
 --------------------------------------------------------------------------------
 -- Client compatibility tests