From 3e7e4369af79e8493388613d8442448b5d098f63 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Wed, 8 Jul 2020 16:56:23 -0400 Subject: [PATCH] Implement getRedirectUrl() for the S3 backend --- libraries/object-persistor/README.md | 2 -- libraries/object-persistor/src/S3Persistor.js | 22 ++++++++++++++++--- .../test/unit/GcsPersistorTests.js | 2 +- .../test/unit/S3PersistorTests.js | 20 ++++++++++++++++- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/libraries/object-persistor/README.md b/libraries/object-persistor/README.md index add2e2a38d..1d854e3fec 100644 --- a/libraries/object-persistor/README.md +++ b/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 diff --git a/libraries/object-persistor/src/S3Persistor.js b/libraries/object-persistor/src/S3Persistor.js index fcf6c7b099..75619bd22a 100644 --- a/libraries/object-persistor/src/S3Persistor.js +++ b/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) { diff --git a/libraries/object-persistor/test/unit/GcsPersistorTests.js b/libraries/object-persistor/test/unit/GcsPersistorTests.js index 7d98053ccd..4fa52c3053 100644 --- a/libraries/object-persistor/test/unit/GcsPersistorTests.js +++ b/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 () { diff --git a/libraries/object-persistor/test/unit/S3PersistorTests.js b/libraries/object-persistor/test/unit/S3PersistorTests.js index 02a78981c5..a71282a56e 100644 --- a/libraries/object-persistor/test/unit/S3PersistorTests.js +++ b/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