feat(middleware-sdk-s3): bucket name validator skips ARN bucket name (#…

…1442) * feat(middleware-sdk-s3): bucket name validator skips ARN bucket name * feat(middleware-sdk-s3): address feedbacks Co-authored-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
aws · Aug 19, 2020 · e693c98 · e693c98
1 parent b5da159
commit e693c98
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 3 deletions.
diff --git a/packages/middleware-sdk-s3/package.json b/packages/middleware-sdk-s3/package.json
@@ -18,6 +18,7 @@
   },
   "license": "Apache-2.0",
   "dependencies": {
+    "@aws-sdk/util-arn-parser": "1.0.0-gamma.1",
     "tslib": "^1.8.0"
   },
   "devDependencies": {

diff --git a/packages/middleware-sdk-s3/src/validate-bucket-name.spec.ts b/packages/middleware-sdk-s3/src/validate-bucket-name.spec.ts
@@ -1,10 +1,13 @@
+const mockValidateArn = jest.fn();
+jest.mock("@aws-sdk/util-arn-parser", () => ({ validate: mockValidateArn }));
 import { validateBucketNameMiddleware } from "./validate-bucket-name";
 
 describe("validateBucketNameMiddleware", () => {
   const mockNextHandler = jest.fn();
 
   beforeEach(() => {
     jest.clearAllMocks();
+    mockValidateArn.mockReturnValue(false);
   });
 
   it("throws error if Bucket parameter contains '/'", async () => {
@@ -36,4 +39,17 @@ describe("validateBucketNameMiddleware", () => {
     expect(mockNextHandler.mock.calls.length).toBe(1);
     expect(mockNextHandler.mock.calls[0][0]).toEqual(args);
   });
+
+  it("should not validate bucket name if the bucket name is an ARN", async () => {
+    mockValidateArn.mockReturnValue(true);
+    const handler = validateBucketNameMiddleware()(mockNextHandler, {} as any);
+    const args = {
+      input: {
+        Bucket: "arn:aws:s3:us-east-1:123456789012:accesspoint/myendpoint",
+      },
+    };
+    await handler(args);
+    expect(mockNextHandler.mock.calls.length).toBe(1);
+    expect(mockNextHandler.mock.calls[0][0]).toEqual(args);
+  });
 });
diff --git a/packages/middleware-sdk-s3/src/validate-bucket-name.ts b/packages/middleware-sdk-s3/src/validate-bucket-name.ts
@@ -7,16 +7,19 @@ import {
   MetadataBearer,
   Pluggable,
 } from "@aws-sdk/types";
+import { validate as validateArn } from "@aws-sdk/util-arn-parser";
 
 export function validateBucketNameMiddleware(): InitializeMiddleware<any, any> {
   return <Output extends MetadataBearer>(
     next: InitializeHandler<any, Output>
   ): InitializeHandler<any, Output> => async (
     args: InitializeHandlerArguments<any>
   ): Promise<InitializeHandlerOutput<Output>> => {
-    const { input } = args;
-    if (typeof input.Bucket === "string" && input.Bucket.indexOf("/") >= 0) {
-      const err = new Error(`Bucket name shouldn't contain '/', received '${input.Bucket}'`);
+    const {
+      input: { Bucket },
+    } = args;
+    if (typeof Bucket === "string" && !validateArn(Bucket) && Bucket.indexOf("/") >= 0) {
+      const err = new Error(`Bucket name shouldn't contain '/', received '${Bucket}'`);
       err.name = "InvalidBucketName";
       throw err;
     }

diff --git a/packages/util-arn-parser/src/index.spec.ts b/packages/util-arn-parser/src/index.spec.ts
@@ -4,6 +4,7 @@ describe("validate", () => {
   it("should validate whether input is a qualified resource ARN", () => {
     expect(validate("arn:aws:s3:us-west-2:123456789012:accesspoint:myendpoint")).toBe(true);
     expect(validate("arn:aws:s3:us-east-1:123456789012:accesspoint:myendpoint")).toBe(true);
+    expect(validate("arn:aws:s3:us-east-1:123456789012:accesspoint/myendpoint")).toBe(true);
     expect(validate("arn:aws-cn:s3:cn-north-1:123456789012:accesspoint:myendpoint")).toBe(true);
     expect(validate("arn:aws:sns:us-west-2:123456789012:myTopic")).toBe(true);
     expect(validate("some:random:string:separated:by:colons")).toBe(false);