From 7b895b35ac43f80ac0749aceee89d26d33ae7f97 Mon Sep 17 00:00:00 2001 From: Marc-Roig Date: Wed, 31 May 2023 11:26:44 +0200 Subject: [PATCH 1/2] feat: implement is url from bucket --- .../src/__tests__/getBucketFromUrl.test.ts | 41 ---------- .../src/__tests__/is-url-from-bucket.test.ts | 78 +++++++++++++++++++ .../src/__tests__/upload-aws-s3.test.ts | 2 +- packages/providers/upload-aws-s3/src/index.ts | 17 +++- packages/providers/upload-aws-s3/src/utils.ts | 53 +++++++++---- 5 files changed, 130 insertions(+), 61 deletions(-) delete mode 100644 packages/providers/upload-aws-s3/src/__tests__/getBucketFromUrl.test.ts create mode 100644 packages/providers/upload-aws-s3/src/__tests__/is-url-from-bucket.test.ts diff --git a/packages/providers/upload-aws-s3/src/__tests__/getBucketFromUrl.test.ts b/packages/providers/upload-aws-s3/src/__tests__/getBucketFromUrl.test.ts deleted file mode 100644 index c9a8a9dbf6..0000000000 --- a/packages/providers/upload-aws-s3/src/__tests__/getBucketFromUrl.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { getBucketFromUrl } from '../utils'; - -describe('Test for URLs', () => { - test('Virtual hosted style', async () => { - const url = 'https://bucket.s3.us-east-1.amazonaws.com/img.png'; - const { bucket } = getBucketFromUrl(url); - expect(bucket).toEqual('bucket'); - }); - - describe('Path style', () => { - test('No key', async () => { - const url = 'https://s3.us-east-1.amazonaws.com/bucket'; - const { bucket } = getBucketFromUrl(url); - expect(bucket).toEqual('bucket'); - }); - - test('With trailing slash', async () => { - const url = 'https://s3.us-east-1.amazonaws.com/bucket/'; - const { bucket } = getBucketFromUrl(url); - expect(bucket).toEqual('bucket'); - }); - - test('With key', async () => { - const url = 'https://s3.us-east-1.amazonaws.com/bucket/img.png'; - const { bucket } = getBucketFromUrl(url); - expect(bucket).toEqual('bucket'); - }); - }); - - test('S3 access point', async () => { - const url = 'https://bucket.s3-accesspoint.us-east-1.amazonaws.com'; - const { bucket } = getBucketFromUrl(url); - expect(bucket).toEqual('bucket'); - }); - - test('S3://', async () => { - const url = 'S3://bucket/img.png'; - const { bucket } = getBucketFromUrl(url); - expect(bucket).toEqual('bucket'); - }); -}); diff --git a/packages/providers/upload-aws-s3/src/__tests__/is-url-from-bucket.test.ts b/packages/providers/upload-aws-s3/src/__tests__/is-url-from-bucket.test.ts new file mode 100644 index 0000000000..a979e163a3 --- /dev/null +++ b/packages/providers/upload-aws-s3/src/__tests__/is-url-from-bucket.test.ts @@ -0,0 +1,78 @@ +import { isUrlFromBucket } from '../utils'; + +describe('Test for URLs', () => { + describe('AWS', () => { + test('Virtual hosted style', async () => { + const url = 'https://bucket-name-123.s3.us-east-1.amazonaws.com/img.png'; + const isFromBucket = isUrlFromBucket(url, 'bucket-name-123'); + expect(isFromBucket).toEqual(true); + }); + + describe('Path style', () => { + test('No key', async () => { + const url = 'https://s3.us-east-1.amazonaws.com/bucket'; + const isFromBucket = isUrlFromBucket(url, 'bucket'); + expect(isFromBucket).toEqual(true); + }); + + test('With trailing slash', async () => { + const url = 'https://s3.us-east-1.amazonaws.com/bucket/'; + const isFromBucket = isUrlFromBucket(url, 'bucket'); + expect(isFromBucket).toEqual(true); + }); + + test('With key', async () => { + const url = 'https://s3.us-east-1.amazonaws.com/bucket/img.png'; + const isFromBucket = isUrlFromBucket(url, 'bucket'); + expect(isFromBucket).toEqual(true); + }); + }); + + test('S3 access point', async () => { + const url = 'https://bucket.s3-accesspoint.us-east-1.amazonaws.com'; + const isFromBucket = isUrlFromBucket(url, 'bucket'); + expect(isFromBucket).toEqual(true); + }); + + test('S3://', async () => { + const url = 'S3://bucket/img.png'; + const isFromBucket = isUrlFromBucket(url, 'bucket'); + expect(isFromBucket).toEqual(true); + }); + }); + + describe('S3 Compatible', () => { + describe('DO Spaces', () => { + test('is from same bucket', async () => { + const url = 'https://bucket-name.nyc3.digitaloceanspaces.com/folder/img.png'; + const isFromBucket = isUrlFromBucket(url, 'bucket-name'); + expect(isFromBucket).toEqual(true); + }); + test('is not from same bucket', async () => { + const url = 'https://bucket-name.nyc3.digitaloceanspaces.com/folder/img.png'; + const isFromBucket = isUrlFromBucket(url, 'bucket'); + expect(isFromBucket).toEqual(false); + }); + }); + + describe('MinIO', () => { + test('is from same bucket', async () => { + const url = 'https://minio.example.com/bucket-name/folder/file'; + const isFromBucket = isUrlFromBucket(url, 'bucket-name'); + expect(isFromBucket).toEqual(true); + }); + + test('is not from same bucket', async () => { + const url = 'https://minio.example.com/bucket-name/folder/file'; + const isFromBucket = isUrlFromBucket(url, 'bucket'); + expect(isFromBucket).toEqual(false); + }); + }); + }); + + test('CDN', async () => { + const url = 'https://cdn.example.com/v1/img.png'; + const isFromBucket = isUrlFromBucket(url, 'bucket', 'https://cdn.example.com/v1/'); + expect(isFromBucket).toEqual(true); + }); +}); diff --git a/packages/providers/upload-aws-s3/src/__tests__/upload-aws-s3.test.ts b/packages/providers/upload-aws-s3/src/__tests__/upload-aws-s3.test.ts index 2d0a6cbf93..2f3cc5f839 100644 --- a/packages/providers/upload-aws-s3/src/__tests__/upload-aws-s3.test.ts +++ b/packages/providers/upload-aws-s3/src/__tests__/upload-aws-s3.test.ts @@ -122,7 +122,7 @@ describe('AWS-S3 provider', () => { }); S3InstanceMock.upload.mockImplementationOnce((params, callback) => - callback(null, { Location: 'https://validurl.test' }) + callback(null, { Location: 'validurl.test' }) ); const file: File = { diff --git a/packages/providers/upload-aws-s3/src/index.ts b/packages/providers/upload-aws-s3/src/index.ts index 5b886b06ea..1f0cf3f465 100644 --- a/packages/providers/upload-aws-s3/src/index.ts +++ b/packages/providers/upload-aws-s3/src/index.ts @@ -1,7 +1,7 @@ import type { ReadStream } from 'node:fs'; import { getOr } from 'lodash/fp'; import AWS from 'aws-sdk'; -import { getBucketFromUrl } from './utils'; +import { isUrlFromBucket } from './utils'; interface File { name: string; @@ -69,6 +69,14 @@ export = { const ACL = getOr('public-read', ['params', 'ACL'], config); + // if ACL is private and baseUrl is set, we need to warn the user + // signed url's will not have the baseUrl prefix + if (ACL === 'private' && baseUrl) { + process.emitWarning( + 'You are using a private ACL with a baseUrl. This is not recommended as the files will be accessible without the baseUrl prefix.' + ); + } + const upload = (file: File, customParams = {}): Promise => new Promise((resolve, reject) => { const fileKey = getFileKey(file); @@ -113,11 +121,12 @@ export = { }, async getSignedUrl(file: File): Promise<{ url: string }> { // Do not sign the url if it does not come from the same bucket. - const { bucket } = getBucketFromUrl(file.url); - if (bucket !== config.params.Bucket) { + if (!isUrlFromBucket(file.url, config.params.Bucket, baseUrl)) { return { url: file.url }; } + const signedUrlExpires: string = getOr(15 * 60, ['params', 'signedUrlExpires'], config); // 15 minutes + return new Promise((resolve, reject) => { const fileKey = getFileKey(file); @@ -126,7 +135,7 @@ export = { { Bucket: config.params.Bucket, Key: fileKey, - Expires: getOr(15 * 60, ['params', 'signedUrlExpires'], config), // 15 minutes + Expires: parseInt(signedUrlExpires, 10), }, (err, url) => { if (err) { diff --git a/packages/providers/upload-aws-s3/src/utils.ts b/packages/providers/upload-aws-s3/src/utils.ts index 28548a1e1b..eeb3903b4a 100644 --- a/packages/providers/upload-aws-s3/src/utils.ts +++ b/packages/providers/upload-aws-s3/src/utils.ts @@ -5,6 +5,29 @@ interface BucketInfo { err?: string; } +export function isUrlFromBucket(fileUrl: string, bucketName: string, bucketBaseUrl = '') { + const url = new URL(fileUrl); + + // Check if the file URL is using a base URL (e.g. a CDN). + // In this case, check if the file URL starts with the same base URL as the bucket URL. + if (bucketBaseUrl) { + const baseUrl = new URL(bucketBaseUrl); + return url.href.startsWith(baseUrl.href); + } + + const { bucket } = getBucketFromAwsUrl(fileUrl); + + if (bucket) { + return bucket === bucketName; + } + + // File URL might be of an S3-compatible provider. (or an invalid URL) + // In this case, check if the bucket name appears in the URL host or path. + // e.g. https://minio.example.com/bucket-name/object-key + // e.g. https://bucket.nyc3.digitaloceanspaces.com/folder/img.png + return url.host.startsWith(`${bucketName}.`) || url.pathname.includes(`/${bucketName}/`); +} + /** * Parse the bucket name from a URL. * See all URL formats in https://docs.aws.amazon.com/AmazonS3/latest/userguide/access-bucket-intro.html @@ -14,49 +37,49 @@ interface BucketInfo { * @returns {string} result.bucket - the bucket name * @returns {string} result.err - if any */ -export function getBucketFromUrl(fileUrl: string): BucketInfo { - const uri = new URL(fileUrl); +function getBucketFromAwsUrl(fileUrl: string): BucketInfo { + const url = new URL(fileUrl); // S3:/// - if (uri.protocol === 's3:') { - const bucket = uri.host; + if (url.protocol === 's3:') { + const bucket = url.host; if (!bucket) { - return { err: `Invalid S3 URI: no bucket: ${uri}` }; + return { err: `Invalid S3 url: no bucket: ${url}` }; } return { bucket }; } - if (!uri.host) { - return { err: `Invalid S3 URI: no hostname: ${uri}` }; + if (!url.host) { + return { err: `Invalid S3 url: no hostname: ${url}` }; } - const matches = uri.host.match(ENDPOINT_PATTERN); + const matches = url.host.match(ENDPOINT_PATTERN); if (!matches) { - return { err: `Invalid S3 URI: hostname does not appear to be a valid S3 endpoint: ${uri}` }; + return { err: `Invalid S3 url: hostname does not appear to be a valid S3 endpoint: ${url}` }; } const prefix = matches[1]; // https://s3.amazonaws.com/ if (!prefix) { - if (uri.pathname === '/') { + if (url.pathname === '/') { return { bucket: null }; } - const index = uri.pathname.indexOf('/', 1); + const index = url.pathname.indexOf('/', 1); // https://s3.amazonaws.com/ if (index === -1) { - return { bucket: uri.pathname.substring(1) }; + return { bucket: url.pathname.substring(1) }; } // https://s3.amazonaws.com// - if (index === uri.pathname.length - 1) { - return { bucket: uri.pathname.substring(1, index) }; + if (index === url.pathname.length - 1) { + return { bucket: url.pathname.substring(1, index) }; } // https://s3.amazonaws.com//key - return { bucket: uri.pathname.substring(1, index) }; + return { bucket: url.pathname.substring(1, index) }; } // https://.s3.amazonaws.com/ From aa7f6368d0b6b6dff6eb3302354e32dd4d392289 Mon Sep 17 00:00:00 2001 From: Marc Date: Thu, 8 Jun 2023 11:02:12 +0200 Subject: [PATCH 2/2] Update utils.ts --- packages/providers/upload-aws-s3/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/providers/upload-aws-s3/src/utils.ts b/packages/providers/upload-aws-s3/src/utils.ts index eeb3903b4a..4b21129a48 100644 --- a/packages/providers/upload-aws-s3/src/utils.ts +++ b/packages/providers/upload-aws-s3/src/utils.ts @@ -5,7 +5,7 @@ interface BucketInfo { err?: string; } -export function isUrlFromBucket(fileUrl: string, bucketName: string, bucketBaseUrl = '') { +export function isUrlFromBucket(fileUrl: string, bucketName: string, bucketBaseUrl = ''): boolean { const url = new URL(fileUrl); // Check if the file URL is using a base URL (e.g. a CDN).