From e693c980ea9efade196ccee963e11a159ca421f1 Mon Sep 17 00:00:00 2001
From: AllanFly120 <zheallan@amazon.com>
Date: Wed, 19 Aug 2020 11:03:19 -0700
Subject: [PATCH] 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>
---
 packages/middleware-sdk-s3/package.json          |  1 +
 .../src/validate-bucket-name.spec.ts             | 16 ++++++++++++++++
 .../src/validate-bucket-name.ts                  |  9 ++++++---
 packages/util-arn-parser/src/index.spec.ts       |  1 +
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/packages/middleware-sdk-s3/package.json b/packages/middleware-sdk-s3/package.json
index 470475982a0e..3ce9ab511d80 100644
--- 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
index f918b557705f..0f70e83b80dc 100644
--- a/packages/middleware-sdk-s3/src/validate-bucket-name.spec.ts
+++ b/packages/middleware-sdk-s3/src/validate-bucket-name.spec.ts
@@ -1,3 +1,5 @@
+const mockValidateArn = jest.fn();
+jest.mock("@aws-sdk/util-arn-parser", () => ({ validate: mockValidateArn }));
 import { validateBucketNameMiddleware } from "./validate-bucket-name";
 
 describe("validateBucketNameMiddleware", () => {
@@ -5,6 +7,7 @@ describe("validateBucketNameMiddleware", () => {
 
   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
index 35a3ccf442e9..8ef7ef821e29 100644
--- a/packages/middleware-sdk-s3/src/validate-bucket-name.ts
+++ b/packages/middleware-sdk-s3/src/validate-bucket-name.ts
@@ -7,6 +7,7 @@ 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>(
@@ -14,9 +15,11 @@ export function validateBucketNameMiddleware(): InitializeMiddleware<any, any> {
   ): 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
index f32443774e9a..f4c901008c7b 100644
--- 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);