index.js

@@ -107,7 +107,6 @@ module.exports = {
           require('./lib/models/ReplicationConfiguration'),
         LifecycleConfiguration:
             require('./lib/models/LifecycleConfiguration'),
-        BucketPolicy: require('./lib/models/BucketPolicy'),
     },
     metrics: {
         StatsClient: require('./lib/metrics/StatsClient'),

lib/models/BucketInfo.js

@@ -2,7 +2,6 @@ const assert = require('assert');
 const { WebsiteConfiguration } = require('./WebsiteConfiguration');
 const ReplicationConfiguration = require('./ReplicationConfiguration');
 const LifecycleConfiguration = require('./LifecycleConfiguration');
-const BucketPolicy = require('./BucketPolicy');
 
 // WHEN UPDATING THIS NUMBER, UPDATE MODELVERSION.MD CHANGELOG
 const modelVersion = 6;
@@ -48,14 +47,12 @@ class BucketInfo {
     * @param {string[]} [cors[].exposeHeaders] - headers expose to applications
     * @param {object} [replicationConfiguration] - replication configuration
     * @param {object} [lifecycleConfiguration] - lifecycle configuration
-    * @param {object} [bucketPolicy] - bucket policy
     */
     constructor(name, owner, ownerDisplayName, creationDate,
                 mdBucketModelVersion, acl, transient, deleted,
                 serverSideEncryption, versioningConfiguration,
                 locationConstraint, websiteConfiguration, cors,
-                replicationConfiguration, lifecycleConfiguration,
+                replicationConfiguration, lifecycleConfiguration) {
-                bucketPolicy) {
         assert.strictEqual(typeof name, 'string');
         assert.strictEqual(typeof owner, 'string');
         assert.strictEqual(typeof ownerDisplayName, 'string');
@@ -115,9 +112,6 @@ class BucketInfo {
         if (lifecycleConfiguration) {
             LifecycleConfiguration.validateConfig(lifecycleConfiguration);
         }
-        if (bucketPolicy) {
-            BucketPolicy.validatePolicy(bucketPolicy);
-        }
         const aclInstance = acl || {
             Canned: 'private',
             FULL_CONTROL: [],
@@ -143,7 +137,6 @@ class BucketInfo {
         this._replicationConfiguration = replicationConfiguration || null;
         this._cors = cors || null;
         this._lifecycleConfiguration = lifecycleConfiguration || null;
-        this._bucketPolicy = bucketPolicy || null;
         return this;
     }
     /**
@@ -167,7 +160,6 @@ class BucketInfo {
             cors: this._cors,
             replicationConfiguration: this._replicationConfiguration,
             lifecycleConfiguration: this._lifecycleConfiguration,
-            bucketPolicy: this._bucketPolicy,
         };
         if (this._websiteConfiguration) {
             bucketInfos.websiteConfiguration =
@@ -188,8 +180,7 @@ class BucketInfo {
             obj.creationDate, obj.mdBucketModelVersion, obj.acl,
             obj.transient, obj.deleted, obj.serverSideEncryption,
             obj.versioningConfiguration, obj.locationConstraint, websiteConfig,
-            obj.cors, obj.replicationConfiguration, obj.lifecycleConfiguration,
+            obj.cors, obj.replicationConfiguration, obj.lifecycleConfiguration);
-            obj.bucketPolicy);
     }
 
     /**
@@ -212,8 +203,7 @@ class BucketInfo {
             data._transient, data._deleted, data._serverSideEncryption,
             data._versioningConfiguration, data._locationConstraint,
             data._websiteConfiguration, data._cors,
-            data._replicationConfiguration, data._lifecycleConfiguration,
+            data._replicationConfiguration, data._lifecycleConfiguration);
-            data._bucketPolicy);
     }
 
     /**
@@ -341,23 +331,6 @@ class BucketInfo {
         this._lifecycleConfiguration = lifecycleConfiguration;
         return this;
     }
-    /**
-     * Get bucket policy statement
-     * @return {object|null} bucket policy statement or `null` if the bucket
-     * does not have a bucket policy
-     */
-    getBucketPolicy() {
-        return this._bucketPolicy;
-    }
-    /**
-     * Set bucket policy statement
-     * @param {object} bucketPolicy - bucket policy
-     * @return {BucketInfo} - bucket info instance
-     */
-    setBucketPolicy(bucketPolicy) {
-        this._bucketPolicy = bucketPolicy;
-        return this;
-    }
     /**
      * Get cors resource
      * @return {object[]} cors

lib/models/BucketPolicy.js

@@ -1,143 +0,0 @@
-const assert = require('assert');
-
-const errors = require('../errors');
-const { validateResourcePolicy } = require('../policy/policyValidator');
-
-/**
- * Format of json policy:
- *  {
- *      "Id": "Policy id",
- *      "Version": "version date",
- *      "Statement": [
- *          {
- *              "Sid": "Statement id",
- *              "Effect": "Allow",
- *              "Principal": "*",
- *              "Action": "s3:*",
- *              "Resource": "arn:aws:s3:::examplebucket/bucket2/object"
- *          },
- *          {
- *              "Sid": "Statement id",
- *              "Effect": "Deny",
- *              "Principal": {
- *                  "AWS": ["arn:aws:iam::<account_id>", "different_account_id"]
- *              },
- *              "Action": [ "s3:*" ],
- *              "Resource": [
- *                  "arn:aws:s3:::examplebucket", "arn:aws:s3:::otherbucket/*"],
- *              "Condition": {
- *                 "StringNotLike": {
- *                      "aws:Referer": [
- *                          "http://www.example.com/", "http://example.com/*"]
- *                  }
- *              }
- *          }
- *      ]
- *  }
- */
-
-const objectActions = [
-    's3:AbortMultipartUpload',
-    's3:DeleteObject',
-    's3:DeleteObjectTagging',
-    's3:GetObject',
-    's3:GetObjectAcl',
-    's3:GetObjectTagging',
-    's3:ListMultipartUploadParts',
-    's3:PutObject',
-    's3:PutObjectAcl',
-    's3:PutObjectTagging',
-];
-
-class BucketPolicy {
-    /**
-     * Create a Bucket Policy instance
-     * @param {string} json - the json policy
-     * @return {object} - BucketPolicy instance
-     */
-    constructor(json) {
-        this._json = json;
-        this._policy = {};
-    }
-
-    /**
-     * Get the bucket policy
-     * @return {object} - the bucket policy or error
-     */
-    getBucketPolicy() {
-        const policy = this._getPolicy();
-        return policy;
-    }
-
-    /**
-     * Get the bucket policy array
-     * @return {object} - contains error if policy validation fails
-     */
-    _getPolicy() {
-        if (!this._json || this._json === '') {
-            return { error: errors.MalformedPolicy.customizeDescription(
-                'request json is empty or undefined') };
-        }
-        const validSchema = validateResourcePolicy(this._json);
-        if (validSchema.error) {
-            return validSchema;
-        }
-        this._setStatementArray();
-        const valAcRes = this._validateActionResource();
-        if (valAcRes.error) {
-            return valAcRes;
-        }
-
-        return this._policy;
-    }
-
-    _setStatementArray() {
-        this._policy = JSON.parse(this._json);
-        if (!Array.isArray(this._policy.Statement)) {
-            const statement = this._policy.Statement;
-            this._policy.Statement = [statement];
-        }
-    }
-
-    /**
-     * Validate action and resource are compatible
-     * @return {error} - contains error or empty obj
-     */
-    _validateActionResource() {
-        const invalid = this._policy.Statement.every(s => {
-            const actions = typeof s.Action === 'string' ?
-                [s.Action] : s.Action;
-            const resources = typeof s.Resource === 'string' ?
-                [s.Resource] : s.Resource;
-            const objectAction = actions.some(a =>
-                a.includes('Object') || objectActions.includes(a));
-            // wildcardObjectAction checks for actions such as 's3:*' or
-            // 's3:Put*' but will return false for actions such as
-            // 's3:PutBucket*'
-            const wildcardObjectAction = actions.some(
-                a => a.includes('*') && !a.includes('Bucket'));
-            const objectResource = resources.some(r => r.includes('/'));
-            return ((objectAction && !objectResource) ||
-                (objectResource && !objectAction && !wildcardObjectAction));
-        });
-        if (invalid) {
-            return { error: errors.MalformedPolicy.customizeDescription(
-                'Action does not apply to any resource(s) in statement') };
-        }
-        return {};
-    }
-
-    /**
-     * Call resource policy schema validation function
-     * @param {object} policy - the bucket policy object to validate
-     * @return {undefined}
-     */
-    static validatePolicy(policy) {
-        // only the BucketInfo constructor calls this function
-        // and BucketInfo will always be passed an object
-        const validated = validateResourcePolicy(JSON.stringify(policy));
-        assert.deepStrictEqual(validated, { error: null, valid: true });
-    }
-}
-
-module.exports = BucketPolicy;

lib/policyEvaluator/evaluator.js

@@ -67,7 +67,7 @@ function isResourceApplicable(requestContext, statementResource, log) {
  * @param {Object} log - logger
  * @return {boolean} true if applicable, false if not
  */
-evaluators.isActionApplicable = (requestAction, statementAction, log) => {
+function isActionApplicable(requestAction, statementAction, log) {
     if (!Array.isArray(statementAction)) {
         // eslint-disable-next-line no-param-reassign
         statementAction = [statementAction];
@@ -89,7 +89,7 @@ evaluators.isActionApplicable = (requestAction, statementAction, log) => {
         { requestAction });
     // If no match found, return false
     return false;
-};
+}
 
 /**
  * Check whether request meets policy conditions
@@ -209,14 +209,14 @@ evaluators.evaluatePolicy = (requestContext, policy, log) => {
         // If affirmative action is in policy and request action is not
         // applicable, move on to next statement
         if (currentStatement.Action &&
-            !evaluators.isActionApplicable(requestContext.getAction(),
+            !isActionApplicable(requestContext.getAction(),
             currentStatement.Action, log)) {
             continue;
         }
         // If NotAction is in policy and action matches NotAction in policy,
         // move on to next statement
         if (currentStatement.NotAction &&
-            evaluators.isActionApplicable(requestContext.getAction(),
+            isActionApplicable(requestContext.getAction(),
             currentStatement.NotAction, log)) {
             continue;
         }

lib/s3routes/routes/routeGET.js

@@ -73,9 +73,9 @@ function routerGET(request, response, api, log, statsClient, dataRetrievalFn) {
                 });
         } else if (request.query.policy !== undefined) {
             api.callApiMethod('bucketGetPolicy', request, response, log,
-                (err, xml, corsHeaders) => {
+                (err, json, corsHeaders) => {
                     routesUtils.statsReport500(err, statsClient);
-                    return routesUtils.responseXMLBody(err, xml, response,
+                    return routesUtils.responseJSONBody(err, json, response,
                         log, corsHeaders);
                 });
         } else {

tests/unit/models/BucketInfo.js

@@ -115,18 +115,6 @@ const testLifecycleConfiguration = {
         },
     ],
 };
-
-const testBucketPolicy = {
-    Version: '2012-10-17',
-    Statement: [
-        {
-            Effect: 'Allow',
-            Principal: '*',
-            Resource: 'arn:aws:s3:::examplebucket',
-            Action: 's3:*',
-        },
-    ],
-};
 // create a dummy bucket to test getters and setters
 
 Object.keys(acl).forEach(
@@ -144,8 +132,7 @@ Object.keys(acl).forEach(
             testWebsiteConfiguration,
             testCorsConfiguration,
             testReplicationConfiguration,
-            testLifecycleConfiguration,
+            testLifecycleConfiguration);
-            testBucketPolicy);
 
         describe('serialize/deSerialize on BucketInfo class', () => {
             const serialized = dummyBucket.serialize();
@@ -171,7 +158,6 @@ Object.keys(acl).forEach(
                         dummyBucket._replicationConfiguration,
                     lifecycleConfiguration:
                         dummyBucket._lifecycleConfiguration,
-                    bucketPolicy: dummyBucket._bucketPolicy,
                 };
                 assert.strictEqual(serialized, JSON.stringify(bucketInfos));
                 done();
@@ -271,10 +257,6 @@ Object.keys(acl).forEach(
                 assert.deepStrictEqual(dummyBucket.getLifecycleConfiguration(),
                     testLifecycleConfiguration);
             });
-            it('getBucketPolicy should return policy', () => {
-                assert.deepStrictEqual(
-                    dummyBucket.getBucketPolicy(), testBucketPolicy);
-            });
         });
 
         describe('setters on BucketInfo class', () => {
@@ -396,22 +378,6 @@ Object.keys(acl).forEach(
                 assert.deepStrictEqual(dummyBucket.getLifecycleConfiguration(),
                     newLifecycleConfig);
             });
-            it('setBucketPolicy should set bucket policy', () => {
-                const newBucketPolicy = {
-                    Version: '2012-10-17',
-                    Statement: [
-                        {
-                            Effect: 'Deny',
-                            Principal: '*',
-                            Resource: 'arn:aws:s3:::examplebucket',
-                            Action: 's3:*',
-                        },
-                    ],
-                };
-                dummyBucket.setBucketPolicy(newBucketPolicy);
-                assert.deepStrictEqual(
-                    dummyBucket.getBucketPolicy(), newBucketPolicy);
-            });
         });
     })
 );

tests/unit/models/BucketPolicy.js

@@ -1,105 +0,0 @@
-const assert = require('assert');
-
-const BucketPolicy = require('../../../lib/models/BucketPolicy');
-
-const testBucketPolicy = {
-    Version: '2012-10-17',
-    Statement: [
-        {
-            Effect: 'Allow',
-            Principal: '*',
-            Resource: 'arn:aws:s3:::examplebucket',
-            Action: 's3:GetBucketLocation',
-        },
-    ],
-};
-const mismatchErr = 'Action does not apply to any resource(s) in statement';
-
-function createPolicy(key, value) {
-    const newPolicy = Object.assign({}, testBucketPolicy);
-    newPolicy.Statement[0][key] = value;
-    return newPolicy;
-}
-
-function checkErr(policy, err, message) {
-    assert.strictEqual(policy.error[err], true);
-    assert.strictEqual(policy.error.description, message);
-}
-
-describe('BucketPolicy class getBucketPolicy', () => {
-    beforeEach(() => {
-        testBucketPolicy.Statement[0].Resource = 'arn:aws:s3:::examplebucket';
-        testBucketPolicy.Statement[0].Action = 's3:GetBucketLocation';
-    });
-
-    it('should return MalformedPolicy error if request json is empty', done => {
-        const bucketPolicy = new BucketPolicy('').getBucketPolicy();
-        const errMessage = 'request json is empty or undefined';
-        checkErr(bucketPolicy, 'MalformedPolicy', errMessage);
-        done();
-    });
-
-    it('should return MalformedPolicy error if request action is for objects ' +
-    'but resource refers to bucket', done => {
-        const newPolicy = createPolicy('Action', 's3:GetObject');
-        const bucketPolicy = new BucketPolicy(JSON.stringify(newPolicy))
-            .getBucketPolicy();
-        checkErr(bucketPolicy, 'MalformedPolicy', mismatchErr);
-        done();
-    });
-
-    it('should return MalformedPolicy error if request action is for objects ' +
-    'but does\'t include \'Object\' and resource refers to bucket', done => {
-        const newPolicy = createPolicy('Action', 's3:AbortMultipartUpload');
-        const bucketPolicy = new BucketPolicy(JSON.stringify(newPolicy))
-            .getBucketPolicy();
-        checkErr(bucketPolicy, 'MalformedPolicy', mismatchErr);
-        done();
-    });
-
-    it('should return MalformedPolicy error if request action is for objects ' +
-    '(with wildcard) but resource refers to bucket', done => {
-        const newPolicy = createPolicy('Action', 's3:GetObject*');
-        const bucketPolicy = new BucketPolicy(JSON.stringify(newPolicy))
-            .getBucketPolicy();
-        checkErr(bucketPolicy, 'MalformedPolicy', mismatchErr);
-        done();
-    });
-
-    it('should return MalformedPolicy error if request resource refers to ' +
-    'object but action is for buckets', done => {
-        const newPolicy = createPolicy('Resource',
-            'arn:aws:s3:::examplebucket/*');
-        const bucketPolicy = new BucketPolicy(JSON.stringify(newPolicy))
-            .getBucketPolicy();
-        checkErr(bucketPolicy, 'MalformedPolicy', mismatchErr);
-        done();
-    });
-
-    it('should return MalformedPolicy error if request resource refers to ' +
-    'object but action is for buckets (with wildcard)', done => {
-        const newPolicy = createPolicy('Resource',
-            'arn:aws:s3:::examplebucket/*');
-        newPolicy.Statement[0].Action = 's3:GetBucket*';
-        const bucketPolicy = new BucketPolicy(JSON.stringify(newPolicy))
-            .getBucketPolicy();
-        checkErr(bucketPolicy, 'MalformedPolicy', mismatchErr);
-        done();
-    });
-
-    it('should successfully get a valid policy', done => {
-        const bucketPolicy = new BucketPolicy(JSON.stringify(testBucketPolicy))
-            .getBucketPolicy();
-        assert.deepStrictEqual(bucketPolicy, testBucketPolicy);
-        done();
-    });
-
-    it('should successfully get a valid policy with wildcard in action',
-    done => {
-        const newPolicy = createPolicy('Action', 's3:Get*');
-        const bucketPolicy = new BucketPolicy(JSON.stringify(newPolicy))
-            .getBucketPolicy();
-        assert.deepStrictEqual(bucketPolicy, newPolicy);
-        done();
-    });
-});