fix: force securetransport traffic only for buckets with dynamic buck…

…et policies (#832)

Co-authored-by: Jeet <68876606+jn1119@users.noreply.github.com>

addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/__tests__/__snapshots__/aws-accounts-service.test.js.snap

@@ -12,8 +12,11 @@ Object {
       },
       "Effect": "Deny",
       "Principal": "*",
-      "Resource": "arn:aws:s3:::dummyBucket/*",
-      "Sid": "Deny requests that do not use TLS",
+      "Resource": Array [
+        "arn:aws:s3:::dummyBucket/*",
+        "arn:aws:s3:::dummyBucket",
+      ],
+      "Sid": "Deny requests that do not use TLS/HTTPS",
     },
     Object {
       "Action": "s3:*",

addons/addon-base-raas/packages/base-raas-services/lib/aws-accounts/aws-accounts-service.js

@@ -98,11 +98,11 @@ class AwsAccountsService extends Service {
 
       const securityStatements = [
         {
-          Sid: 'Deny requests that do not use TLS',
+          Sid: 'Deny requests that do not use TLS/HTTPS',
           Effect: 'Deny',
           Principal: '*',
           Action: 's3:*',
-          Resource: `arn:aws:s3:::${s3BucketName}/*`,
+          Resource: [`arn:aws:s3:::${s3BucketName}/*`, `arn:aws:s3:::${s3BucketName}`],
           Condition: { Bool: { 'aws:SecureTransport': false } },
         },
         {

addons/addon-base-raas/packages/base-raas-services/lib/environment/__tests__/environment-mount-service.test.js

@@ -67,6 +67,55 @@ describe('EnvironmentMountService', () => {
     iamService = await container.find('iamService');
   });
 
+  describe('getAllStatements helper method', () => {
+    it('should return listStatement, getStatement, and putStatement', async () => {
+      // BUILD
+      const s3BucketName = 'somebucket';
+      const s3Prefix = 'someprefix';
+      const expectedStatements = [
+        {
+          Sid: `List:${s3Prefix}`,
+          Effect: 'Allow',
+          Principal: { AWS: [] },
+          Action: 's3:ListBucket',
+          Resource: `arn:aws:s3:::${s3BucketName}`,
+          Condition: {
+            StringLike: {
+              's3:prefix': [`${s3Prefix}*`],
+            },
+          },
+        },
+        {
+          Sid: `Get:${s3Prefix}`,
+          Effect: 'Allow',
+          Principal: { AWS: [] },
+          Action: ['s3:GetObject'],
+          Resource: [`arn:aws:s3:::${s3BucketName}/${s3Prefix}*`],
+        },
+        {
+          Sid: `Put:${s3Prefix}`,
+          Effect: 'Allow',
+          Principal: { AWS: [] },
+          Action: [
+            's3:AbortMultipartUpload',
+            's3:ListMultipartUploadParts',
+            's3:PutObject',
+            's3:PutObjectAcl',
+            's3:DeleteObject',
+          ],
+          Resource: [`arn:aws:s3:::${s3BucketName}/${s3Prefix}*`],
+        },
+      ];
+
+      // OPERATE
+      // eslint-disable-next-line prefer-const
+      let allStatements = service.getAllStatements(s3BucketName, s3Prefix);
+
+      // CHECK
+      expect(allStatements).toEqual(expectedStatements);
+    });
+  });
+
   describe('Update paths', () => {
     it('should call nothing if all are admin changes', async () => {
       // BUILD

addons/addon-base-raas/packages/base-raas-services/lib/environment/environment-mount-service.js

@@ -170,46 +170,12 @@ class EnvironmentMountService extends Service {
         // Get statements for listing and reading study data, respectively
         const statements = s3Policy.Statement;
         s3Prefixes.forEach(prefix => {
+          const allStatements = this.getAllStatements(s3BucketName, prefix);
+          let [listStatement, getStatement, putStatement] = allStatements;
           const listSid = `List:${prefix}`;
           const getSid = `Get:${prefix}`;
           const putSid = `Put:${prefix}`;
 
-          // Define default statements to be used if we can't find existing ones
-          let listStatement = {
-            Sid: listSid,
-            Effect: 'Allow',
-            Principal: { AWS: [] },
-            Action: 's3:ListBucket',
-            Resource: `arn:aws:s3:::${s3BucketName}`,
-            Condition: {
-              StringLike: {
-                's3:prefix': [`${prefix}*`],
-              },
-            },
-          };
-          // Read Permission
-          let getStatement = {
-            Sid: getSid,
-            Effect: 'Allow',
-            Principal: { AWS: [] },
-            Action: ['s3:GetObject'],
-            Resource: [`arn:aws:s3:::${s3BucketName}/${prefix}*`],
-          };
-          // Write Permission
-          let putStatement = {
-            Sid: putSid,
-            Effect: 'Allow',
-            Principal: { AWS: [] },
-            Action: [
-              's3:AbortMultipartUpload',
-              's3:ListMultipartUploadParts',
-              's3:PutObject',
-              's3:PutObjectAcl',
-              's3:DeleteObject',
-            ],
-            Resource: [`arn:aws:s3:::${s3BucketName}/${prefix}*`],
-          };
-
           // Pull out existing statements if available
           statements.forEach(statement => {
             if (statement.Sid === listSid) {
@@ -230,7 +196,7 @@ class EnvironmentMountService extends Service {
           s3Policy.Statement = s3Policy.Statement.filter(
             statement => ![listSid, getSid, putSid].includes(statement.Sid),
           );
-          [listStatement, getStatement, putStatement].forEach(statement => {
+          allStatements.forEach(statement => {
             // Only add updated statement if it contains principals (otherwise leave it out)
             if (statement.Principal.AWS.length > 0) {
               s3Policy.Statement.push(statement);
@@ -281,6 +247,58 @@ class EnvironmentMountService extends Service {
     ]);
   }
 
+  /**
+   * Helper method to generate the default statements to be used to update the S3 policy.
+   *
+   * @param {String} s3BucketName - name of the bucket that these statements apply to
+   * @param {String} prefix - S3 prefix to apply to these statements
+   * @returns {Array<Object>} - the generic list statement, read statement, and write statement as well as the security
+   *                            statement to require TLS/HTTPS traffic
+   */
+  getAllStatements(s3BucketName, prefix) {
+    const listSid = `List:${prefix}`;
+    const getSid = `Get:${prefix}`;
+    const putSid = `Put:${prefix}`;
+
+    // Define default statements to be used if we can't find existing ones
+    const listStatement = {
+      Sid: listSid,
+      Effect: 'Allow',
+      Principal: { AWS: [] },
+      Action: 's3:ListBucket',
+      Resource: `arn:aws:s3:::${s3BucketName}`,
+      Condition: {
+        StringLike: {
+          's3:prefix': [`${prefix}*`],
+        },
+      },
+    };
+    // Read Permission
+    const getStatement = {
+      Sid: getSid,
+      Effect: 'Allow',
+      Principal: { AWS: [] },
+      Action: ['s3:GetObject'],
+      Resource: [`arn:aws:s3:::${s3BucketName}/${prefix}*`],
+    };
+    // Write Permission
+    const putStatement = {
+      Sid: putSid,
+      Effect: 'Allow',
+      Principal: { AWS: [] },
+      Action: [
+        's3:AbortMultipartUpload',
+        's3:ListMultipartUploadParts',
+        's3:PutObject',
+        's3:PutObjectAcl',
+        's3:DeleteObject',
+      ],
+      Resource: [`arn:aws:s3:::${s3BucketName}/${prefix}*`],
+    };
+
+    return [listStatement, getStatement, putStatement];
+  }
+
   /**
    * Function that calculates which users need should and should not have access permissions for the given studyId
    * Accordingly calls methods that ensure/remove/update permissions for said users

addons/addon-edit-s3-bucket-policy/README.md

@@ -0,0 +1,7 @@
+# Addon for performing S3 Bucket Policy update
+
+The purpose of this add-on is to edit S3 Bucket Policies
+
+## npm packages
+
+- @aws-ee/edit-s3-bucket-policy

addons/addon-edit-s3-bucket-policy/packages/edit-s3-bucket-policy/.eslintrc.json

@@ -0,0 +1,26 @@
+{
+  "extends": ["plugin:jest/recommended", "airbnb-base", "prettier"],
+  "plugins": ["jest", "prettier"],
+  "rules": {
+    "prettier/prettier": ["error"],
+    "no-unused-vars": [
+      "error",
+      {
+        "varsIgnorePattern": "^_.+",
+        "argsIgnorePattern": "^_",
+        "caughtErrorsIgnorePattern": "^_",
+        "args": "after-used",
+        "ignoreRestSiblings": true
+      }
+    ],
+    "prefer-destructuring": 0,
+    "no-underscore-dangle": 0,
+    "no-param-reassign": 0,
+    "class-methods-use-this": 0,
+    "no-use-before-define": 0,
+    "import/no-unresolved": 0
+  },
+  "env": {
+    "jest/globals": true
+  }
+}

addons/addon-edit-s3-bucket-policy/packages/edit-s3-bucket-policy/.gitignore

@@ -0,0 +1,46 @@
+# See https://help.github.com/ignore-files/ for more about ignoring files.
+
+**/.class
+**/.DS_Store
+**/coverage
+**/node_modules
+
+**/npm-debug.log
+**/pnpm-debug.log
+**/npm-debug.log*
+
+# Serverless directories
+.serverless
+
+# VisualStudioCode.gitignore
+.vscode/*
+!.vscode/settings.json
+!.vscode/tasks.json
+!.vscode/launch.json
+!.vscode/extensions.json
+
+# dependencies
+/.pnp
+.pnp.js
+
+# testing
+/coverage
+
+# production
+/build
+
+# transpiled code
+dist
+
+# misc
+.env.local
+.env.development
+.env.development.local
+.env.test.local
+.env.production
+.env.production.local
+
+yarn-debug.log*
+yarn-error.log*
+
+# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.

addons/addon-edit-s3-bucket-policy/packages/edit-s3-bucket-policy/.prettierrc.json

@@ -0,0 +1,7 @@
+{
+  "tabWidth": 2,
+  "printWidth": 120,
+  "singleQuote": true,
+  "quoteProps": "consistent",
+  "trailingComma": "all"
+}

addons/addon-edit-s3-bucket-policy/packages/edit-s3-bucket-policy/jest.config.js

@@ -0,0 +1,25 @@
+/*
+ *  Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License").
+ *  You may not use this file except in compliance with the License.
+ *  A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ *  or in the "license" file accompanying this file. This file is distributed
+ *  on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *  express or implied. See the License for the specific language governing
+ *  permissions and limitations under the License.
+ */
+
+// jest.config.js
+module.exports = {
+  // verbose: true,
+  notify: false,
+  testEnvironment: 'node',
+  testPathIgnorePatterns: ['/node_modules/', '/.history/', '/__tests__/__fixtures__/'],
+  // Configure JUnit reporter as CodeBuild currently only supports JUnit or Cucumber reports
+  // See https://docs.aws.amazon.com/codebuild/latest/userguide/test-reporting.html
+  reporters: ['default', ['jest-junit', { suiteName: 'jest tests', outputDirectory: './.build/test' }]],
+};

addons/addon-edit-s3-bucket-policy/packages/edit-s3-bucket-policy/jsconfig.json

@@ -0,0 +1,6 @@
+{
+  "exclude": [
+    "node_modules",
+    "**/node_modules/*"
+  ]
+}

addons/addon-edit-s3-bucket-policy/packages/edit-s3-bucket-policy/lib/plugins/edit-s3-bucket-policy-plugin.js

@@ -0,0 +1,36 @@
+/*
+ *  Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License").
+ *  You may not use this file except in compliance with the License.
+ *  A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ *  or in the "license" file accompanying this file. This file is distributed
+ *  on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *  express or implied. See the License for the specific language governing
+ *  permissions and limitations under the License.
+ */
+
+const EditS3BucketPolicyService = require('../steps/edit-s3-bucket-policy-service');
+
+/**
+ * Returns a map of post deployment steps
+ *
+ * @param existingStepsMap Map of existing post deployment steps
+ * @param pluginRegistry A registry that provides plugins registered by various addons for the specified extension point.
+ *
+ * @returns {Promise<*>}
+ */
+// eslint-disable-next-line no-unused-vars
+async function getSteps(existingStepsMap, pluginRegistry) {
+  const stepsMap = new Map([['EditS3BucketPolicyService', new EditS3BucketPolicyService()], ...existingStepsMap]);
+  return stepsMap;
+}
+
+const plugin = {
+  getSteps,
+};
+
+module.exports = plugin;