From 122d89a831e01ce2c8ee252400e711b0f803dfca Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 8 Nov 2024 09:33:22 +0100 Subject: [PATCH] Merge pull request #21660 from overleaf/jpa-s3-https [object-persistor] s3: simplify using a custom CA for HTTPS endpoints GitOrigin-RevId: 2c6a5312a842582e5e40e917ccc586392087cb7a --- libraries/object-persistor/src/S3Persistor.js | 10 +++++- package-lock.json | 2 -- services/filestore/package.json | 1 - .../test/acceptance/js/FilestoreTests.js | 31 +++++-------------- .../test/acceptance/js/TestConfig.js | 8 +---- 5 files changed, 18 insertions(+), 34 deletions(-) diff --git a/libraries/object-persistor/src/S3Persistor.js b/libraries/object-persistor/src/S3Persistor.js index 1a8c326be4..b34afcf1e9 100644 --- a/libraries/object-persistor/src/S3Persistor.js +++ b/libraries/object-persistor/src/S3Persistor.js @@ -523,7 +523,7 @@ class S3Persistor extends AbstractPersistor { if (this.settings.endpoint) { const endpoint = new URL(this.settings.endpoint) options.endpoint = this.settings.endpoint - options.sslEnabled = endpoint.protocol === 'https' + options.sslEnabled = endpoint.protocol === 'https:' } // path-style access is only used for acceptance tests @@ -537,6 +537,14 @@ class S3Persistor extends AbstractPersistor { } } + if (options.sslEnabled && this.settings.ca && !options.httpOptions?.agent) { + options.httpOptions = options.httpOptions || {} + options.httpOptions.agent = new https.Agent({ + rejectUnauthorized: true, + ca: this.settings.ca, + }) + } + return options } diff --git a/package-lock.json b/package-lock.json index 613e000184..02a10978e9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -39546,7 +39546,6 @@ }, "devDependencies": { "@google-cloud/storage": "^6.10.1", - "aws-sdk": "^2.718.0", "chai": "^4.3.6", "chai-as-promised": "^7.1.1", "mocha": "^10.2.0", @@ -50110,7 +50109,6 @@ "@overleaf/object-persistor": "*", "@overleaf/settings": "*", "@overleaf/stream-utils": "^0.1.0", - "aws-sdk": "^2.718.0", "body-parser": "^1.20.3", "bunyan": "^1.8.15", "chai": "^4.3.6", diff --git a/services/filestore/package.json b/services/filestore/package.json index 4bfe446bf6..ddf4bf3405 100644 --- a/services/filestore/package.json +++ b/services/filestore/package.json @@ -36,7 +36,6 @@ }, "devDependencies": { "@google-cloud/storage": "^6.10.1", - "aws-sdk": "^2.718.0", "chai": "^4.3.6", "chai-as-promised": "^7.1.1", "mocha": "^10.2.0", diff --git a/services/filestore/test/acceptance/js/FilestoreTests.js b/services/filestore/test/acceptance/js/FilestoreTests.js index bdab787393..2317fd515f 100644 --- a/services/filestore/test/acceptance/js/FilestoreTests.js +++ b/services/filestore/test/acceptance/js/FilestoreTests.js @@ -7,7 +7,6 @@ const Path = require('path') const FilestoreApp = require('./FilestoreApp') const TestHelper = require('./TestHelper') const fetch = require('node-fetch') -const S3 = require('aws-sdk/clients/s3') const { promisify } = require('util') const { Storage } = require('@google-cloud/storage') const streamifier = require('streamifier') @@ -43,6 +42,7 @@ const { PerProjectEncryptedS3Persistor, RootKeyEncryptionKey, } = require('@overleaf/object-persistor/src/PerProjectEncryptedS3Persistor') +const { S3Persistor } = require('@overleaf/object-persistor/src/S3Persistor') const crypto = require('crypto') describe('Filestore', function () { @@ -521,18 +521,11 @@ describe('Filestore', function () { bucketName = `random-bucket-${new ObjectId().toString()}` fileUrl = `${filestoreUrl}/bucket/${bucketName}/key/${fileId}` - const cfg = s3Config() - const s3ClientSettings = { - credentials: { - accessKeyId: process.env.MINIO_ROOT_USER, - secretAccessKey: process.env.MINIO_ROOT_PASSWORD, - }, - endpoint: cfg.endpoint, - httpOptions: cfg.httpOptions, - s3ForcePathStyle: cfg.pathStyle, - } - - const s3 = new S3(s3ClientSettings) + const s3 = new S3Persistor({ + ...s3Config(), + key: process.env.MINIO_ROOT_USER, + secret: process.env.MINIO_ROOT_PASSWORD, + })._getClientForBucket(bucketName) await s3 .createBucket({ Bucket: bucketName, @@ -1263,16 +1256,8 @@ describe('Filestore', function () { }) let s3Client - before('create s3Client', function () { - const cfg = s3Config() - const s3ClientSettings = { - accessKeyId: cfg.key, - secretAccessKey: cfg.secret, - endpoint: cfg.endpoint, - httpOptions: cfg.httpOptions, - s3ForcePathStyle: cfg.pathStyle, - } - s3Client = new S3(s3ClientSettings) + before('create s3 client', function () { + s3Client = new S3Persistor(s3Config())._getClientForBucket('') }) async function checkDEKStorage({ diff --git a/services/filestore/test/acceptance/js/TestConfig.js b/services/filestore/test/acceptance/js/TestConfig.js index 07ad3aa1bf..dfedad7dcf 100644 --- a/services/filestore/test/acceptance/js/TestConfig.js +++ b/services/filestore/test/acceptance/js/TestConfig.js @@ -1,7 +1,6 @@ const fs = require('fs') const Path = require('path') const crypto = require('crypto') -const https = require('https') const { RootKeyEncryptionKey, } = require('@overleaf/object-persistor/src/PerProjectEncryptedS3Persistor') @@ -12,12 +11,7 @@ function s3BaseConfig() { endpoint: process.env.AWS_S3_ENDPOINT, pathStyle: true, partSize: 100 * 1024 * 1024, - httpOptions: { - agent: new https.Agent({ - rejectUnauthorized: true, - ca: [fs.readFileSync('/certs/public.crt')], - }), - }, + ca: [fs.readFileSync('/certs/public.crt')], } }