Merge pull request #5 from overleaf/em-s3-signed-url

Implement getRedirectUrl() for the S3 backend

libraries/object-persistor/README.md

@@ -96,8 +96,6 @@ A `string` containing the signed link, or `null` if a link cannot be generated.
 
 In the case of `null`, you should fall back to `getObjectStream` as sometimes signed links cannot be generated.
 
-Signed links are currently only supported on the `gcs` backend.
-
 Do not use this method if you are using a secondary persistor, as this mechanism does not check to see if the object actually exists - so cannot provide a fallback.
 
 #### getObjectSize

libraries/object-persistor/src/S3Persistor.js

@@ -114,9 +114,25 @@ module.exports = class S3Persistor extends AbstractPersistor {
     }
   }
 
-  async getRedirectUrl() {
-    // not implemented
-    return null
+  async getRedirectUrl(bucketName, key) {
+    const expiresSeconds = Math.round(this.settings.signedUrlExpiryInMs / 1000)
+    try {
+      const url = await this._getClientForBucket(
+        bucketName
+      ).getSignedUrlPromise('getObject', {
+        Bucket: bucketName,
+        Key: key,
+        Expires: expiresSeconds
+      })
+      return url
+    } catch (err) {
+      throw PersistorHelper.wrapError(
+        err,
+        'error generating signed url for S3 file',
+        { bucketName, key },
+        ReadError
+      )
+    }
   }
 
   async deleteDirectory(bucketName, key, continuationToken) {

libraries/object-persistor/test/unit/GcsPersistorTests.js

@@ -250,7 +250,7 @@ describe('GcsPersistorTests', function () {
     })
   })
 
-  describe('getFile', function () {
+  describe('getRedirectUrl', function () {
     let signedUrl
 
     beforeEach(async function () {

libraries/object-persistor/test/unit/S3PersistorTests.js

@@ -27,6 +27,7 @@ describe('S3PersistorTests', function () {
   ]
   const filesSize = 33
   const md5 = 'ffffffff00000000ffffffff00000000'
+  const redirectUrl = 'https://wombat.potato/giraffe'
 
   let Logger,
     Transform,
@@ -126,7 +127,8 @@ describe('S3PersistorTests', function () {
         .returns({ promise: sinon.stub().resolves({ ETag: `"${md5}"` }) }),
       copyObject: sinon.stub().returns(EmptyPromise),
       deleteObject: sinon.stub().returns(EmptyPromise),
-      deleteObjects: sinon.stub().returns(EmptyPromise)
+      deleteObjects: sinon.stub().returns(EmptyPromise),
+      getSignedUrlPromise: sinon.stub().resolves(redirectUrl)
     }
     S3 = sinon.stub().returns(S3Client)
 
@@ -354,6 +356,22 @@ describe('S3PersistorTests', function () {
     })
   })
 
+  describe('getRedirectUrl', function () {
+    let signedUrl
+
+    beforeEach(async function () {
+      signedUrl = await S3Persistor.getRedirectUrl(bucket, key)
+    })
+
+    it('should request a signed URL', function () {
+      expect(S3Client.getSignedUrlPromise).to.have.been.called
+    })
+
+    it('should return the url', function () {
+      expect(signedUrl).to.equal(redirectUrl)
+    })
+  })
+
   describe('getObjectSize', function () {
     describe('when called with valid parameters', function () {
       let size