Merge pull request #16868 from strapi/fix/s3-bucket-compat-providers

packages/providers/upload-aws-s3/src/__tests__/getBucketFromUrl.test.ts

@@ -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');
-  });
-});

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);
+  });
+});

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 = {

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<void> =>
       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 (!isUrlFromBucket(file.url, config.params.Bucket, baseUrl)) {
-        if (bucket !== config.params.Bucket) {
           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) {

packages/providers/upload-aws-s3/src/utils.ts

@@ -5,6 +5,29 @@ interface BucketInfo {
   err?: string;
 }
 
+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).
+  // 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 {
+function getBucketFromAwsUrl(fileUrl: string): BucketInfo {
-  const uri = new URL(fileUrl);
+  const url = new URL(fileUrl);
 
   // S3://<bucket-name>/<key>
-  if (uri.protocol === 's3:') {
+  if (url.protocol === 's3:') {
-    const bucket = uri.host;
+    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) {
+  if (!url.host) {
-    return { err: `Invalid S3 URI: no hostname: ${uri}` };
+    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/<bucket-name>
   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/<bucket-name>
     if (index === -1) {
-      return { bucket: uri.pathname.substring(1) };
+      return { bucket: url.pathname.substring(1) };
     }
 
     // https://s3.amazonaws.com/<bucket-name>/
-    if (index === uri.pathname.length - 1) {
+    if (index === url.pathname.length - 1) {
-      return { bucket: uri.pathname.substring(1, index) };
+      return { bucket: url.pathname.substring(1, index) };
     }
 
     // https://s3.amazonaws.com/<bucket-name>/key
-    return { bucket: uri.pathname.substring(1, index) };
+    return { bucket: url.pathname.substring(1, index) };
   }
 
   // https://<bucket-name>.s3.amazonaws.com/