bf: S3C-2440 get policy xml error and tests

(cherry picked from commit cc30a2eb42)

constants.js

@@ -122,6 +122,29 @@ const constants = {
         '(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$'),
     // user metadata applied on zenko objects
     zenkoIDHeader: 'x-amz-meta-zenko-instance-id',
+    bucketOwnerActions: [
+        'bucketDeleteCors',
+        'bucketDeleteLifecycle',
+        'bucketDeletePolicy',
+        'bucketDeleteReplication',
+        'bucketDeleteWebsite',
+        'bucketGetCors',
+        'bucketGetLifecycle',
+        'bucketGetLocation',
+        'bucketGetPolicy',
+        'bucketGetReplication',
+        'bucketGetVersioning',
+        'bucketGetWebsite',
+        'bucketPutCors',
+        'bucketPutLifecycle',
+        'bucketPutPolicy',
+        'bucketPutReplication',
+        'bucketPutVersioning',
+        'bucketPutWebsite',
+        'objectDeleteTagging',
+        'objectGetTagging',
+        'objectPutTagging',
+    ],
 };
 
 module.exports = constants;

docs/BUCKET_POLICIES.md

@@ -0,0 +1,146 @@
+# Bucket Policy Documentation
+
+## Description
+
+Bucket policy is a method of controlling access to a user's account at the
+resource level.
+There are three associated APIs:
+
+- PUT Bucket policy (see https://docs.aws.amazon.com/AmazonS3/latest/API/RESTBucketPUTpolicy.html)
+- GET Bucket policy (see https://docs.aws.amazon.com/AmazonS3/latest/API/RESTBucketGETpolicy.html)
+- DELETE Bucket policy (see https://docs.aws.amazon.com/AmazonS3/latest/API/RESTBucketDELETEpolicy.html)
+
+More information on bucket policies in general can be found at
+https://docs.aws.amazon.com/AmazonS3/latest/dev/using-iam-policies.html.
+
+## Requirements
+
+To prevent loss of access to a bucket, the root owner of a bucket will always
+be able to perform any of the three bucket policy-related operations, even
+if permission is explicitly denied.
+All other users must have permission to perform the desired operation.
+
+## Design
+
+On a PUTBucketPolicy request, the user provides a policy in JSON format.
+The policy is evaluated against our policy schema in Arsenal and, once
+validated, is stored as part of the bucket's metadata.
+On a GETBucketPolicy request, the policy is retrieved from the bucket's
+metadata.
+On a DELETEBucketPolicy request, the policy is deleted from the bucket's
+metadata.
+
+All other APIs are updated to check if a bucket policy is attached to the bucket
+the request is made on. If there is a policy, user authorization to perform
+the requested action is checked.
+
+### Differences Between Bucket and IAM Policies
+
+IAM policies are attached to an IAM identity and define what actions that
+identity is allowed to or denied from doing on what resource.
+Bucket policies attach only to buckets and define what actions are allowed or
+denied for which principles on that bucket. Permissions specified in a bucket
+policy apply to all objects in that bucket unless otherwise specified.
+
+Besides their attachment origins, the main structural difference between
+IAM policy and bucket policy is the requirement of a "Principal" element in
+bucket policies. This field is redundant in IAM policies.
+
+### Policy Validation
+
+For general guidelines for bucket policy structure, see examples here:
+https://docs.aws.amazon.com/AmazonS3/latest/dev//example-bucket-policies.html.
+
+Each bucket policy statement object requires at least four keys:
+"Effect", "Principle", "Resource", and "Action".
+
+"Effect" defines the effect of the policy and can have a string value of either
+"Allow" or "Deny".
+"Resource" defines to which bucket or list of buckets a policy is attached.
+An object within the bucket is also a valid resource. The element value can be
+either a single bucket or object ARN string or an array of ARNs.
+"Action" lists which action(s) the policy controls. Its value can also be either
+a string or array of S3 APIs. Each action is the API name prepended by "s3:".
+"Principle" specifies which user(s) are granted or denied access to the bucket
+resource. Its value can be a string or an object containing an array of users.
+Valid users can be identified with an account ARN, account id, or user ARN.
+
+There are also two optional bucket policy statement keys: Sid and Condition.
+
+"Sid" stands for "statement id". If this key is not included, one will be
+generated for the statement.
+"Condition" lists the condition under which a statement will take affect.
+The possibilities are as follows:
+
+- ArnEquals
+- ArnEqualsIfExists
+- ArnLike
+- ArnLikeIfExists
+- ArnNotEquals
+- ArnNotEqualsIfExists
+- ArnNotLike
+- ArnNotLikeIfExists
+- BinaryEquals
+- BinaryEqualsIfExists
+- BinaryNotEquals
+- BinaryNotEqualsIfExists
+- Bool
+- BoolIfExists
+- DateEquals
+- DateEqualsIfExists
+- DateGreaterThan
+- DateGreaterThanEquals
+- DateGreaterThanEqualsIfExists
+- DateGreaterThanIfExists
+- DateLessThan
+- DateLessThanEquals
+- DateLessThanEqualsIfExists
+- DateLessThanIfExists
+- DateNotEquals
+- DateNotEqualsIfExists
+- IpAddress
+- IpAddressIfExists
+- NotIpAddress
+- NotIpAddressIfExists
+- Null
+- NumericEquals
+- NumericEqualsIfExists
+- NumericGreaterThan
+- NumericGreaterThanEquals
+- NumericGreaterThanEqualsIfExists
+- NumericGreaterThanIfExists
+- NumericLessThan
+- NumericLessThanEquals
+- NumericLessThanEqualsIfExists
+- NumericLessThanIfExists
+- NumericNotEquals
+- NumericNotEqualsIfExists
+- StringEquals
+- StringEqualsIfExists
+- StringEqualsIgnoreCase
+- StringEqualsIgnoreCaseIfExists
+- StringLike
+- StringLikeIfExists
+- StringNotEquals
+- StringNotEqualsIfExists
+- StringNotEqualsIgnoreCase
+- StringNotEqualsIgnoreCaseIfExists
+- StringNotLike
+- StringNotLikeIfExists
+
+The value of the Condition key will be an object containing the desired
+condition name as that key. The value of inner object can be a string, boolean,
+number, or object, depending on the condition.
+
+## Authorization with Multiple Access Control Mechanisms
+
+In the case where multiple access control mechanisms (such as IAM policies,
+bucket policies, and ACLs) refer to the same resource, the principle of
+least-privilege is applied. Unless an action is explicitly allowed, access will
+by default be denied. An explicit DENY in any policy will trump another
+policy's ALLOW for an action. The request will only be allowed if at least one
+policy specifies an ALLOW, and there is no overriding DENY.
+
+The following diagram illustrates this logic:
+
+![Access_Control_Authorization_Chart](./images/access_control_authorization.png)

lib/api/apiUtils/authorization/aclChecks.js

@@ -1,138 +0,0 @@
-const constants = require('../../../../constants');
-
-// whitelist buckets to allow public read on objects
-const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS ?
-    process.env.ALLOW_PUBLIC_READ_BUCKETS.split(',') : [];
-
-function isBucketAuthorized(bucket, requestType, canonicalID) {
-    // Check to see if user is authorized to perform a
-    // particular action on bucket based on ACLs.
-    // TODO: Add IAM checks and bucket policy checks.
-    if (bucket.getOwner() === canonicalID) {
-        return true;
-    } else if (requestType === 'bucketOwnerAction') {
-        // only bucket owner can modify or retrieve this property of a bucket
-        return false;
-    }
-    const bucketAcl = bucket.getAcl();
-    if (requestType === 'bucketGet' || requestType === 'bucketHead') {
-        if (bucketAcl.Canned === 'public-read'
-            || bucketAcl.Canned === 'public-read-write'
-            || (bucketAcl.Canned === 'authenticated-read'
-                && canonicalID !== constants.publicId)) {
-            return true;
-        } else if (bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
-            || bucketAcl.READ.indexOf(canonicalID) > -1) {
-            return true;
-        }
-    }
-
-    if (requestType === 'bucketGetACL') {
-        if ((bucketAcl.Canned === 'log-delivery-write'
-                && canonicalID === constants.logId)
-            || bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
-            || bucketAcl.READ_ACP.indexOf(canonicalID) > -1) {
-            return true;
-        }
-    }
-
-    if (requestType === 'bucketPutACL') {
-        if (bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
-            || bucketAcl.WRITE_ACP.indexOf(canonicalID) > -1) {
-            return true;
-        }
-    }
-
-    if (requestType === 'bucketDelete' && bucket.getOwner() === canonicalID) {
-        return true;
-    }
-
-    if (requestType === 'objectDelete' || requestType === 'objectPut') {
-        if (bucketAcl.Canned === 'public-read-write'
-            || bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
-            || bucketAcl.WRITE.indexOf(canonicalID) > -1) {
-            return true;
-        }
-    }
-    // Note that an account can have the ability to do objectPutACL,
-    // objectGetACL, objectHead or objectGet even if the account has no rights
-    // to the bucket holding the object.  So, if the request type is
-    // objectPutACL, objectGetACL, objectHead or objectGet, the bucket
-    // authorization check should just return true so can move on to check
-    // rights at the object level.
-
-    return (requestType === 'objectPutACL' || requestType === 'objectGetACL' ||
-    requestType === 'objectGet' || requestType === 'objectHead');
-}
-
-function isObjAuthorized(bucket, objectMD, requestType, canonicalID) {
-    const bucketOwner = bucket.getOwner();
-    if (!objectMD) {
-        return false;
-    }
-    if (objectMD['owner-id'] === canonicalID) {
-        return true;
-    }
-    // account is authorized if:
-    // - requesttype is "bucketOwnerAction" (example: for objectTagging) and
-    // - account is the bucket owner
-    if (requestType === 'bucketOwnerAction' && bucketOwner === canonicalID) {
-        return true;
-    }
-    if (requestType === 'objectGet' || requestType === 'objectHead') {
-        if (objectMD.acl.Canned === 'public-read'
-            || objectMD.acl.Canned === 'public-read-write'
-            || (objectMD.acl.Canned === 'authenticated-read'
-                && canonicalID !== constants.publicId)) {
-            return true;
-        } else if (objectMD.acl.Canned === 'bucket-owner-read'
-                && bucketOwner === canonicalID) {
-            return true;
-        } else if ((objectMD.acl.Canned === 'bucket-owner-full-control'
-                && bucketOwner === canonicalID)
-            || objectMD.acl.FULL_CONTROL.indexOf(canonicalID) > -1
-            || objectMD.acl.READ.indexOf(canonicalID) > -1) {
-            return true;
-        }
-    }
-
-    // User is already authorized on the bucket for FULL_CONTROL or WRITE or
-    // bucket has canned ACL public-read-write
-    if (requestType === 'objectPut' || requestType === 'objectDelete') {
-        return true;
-    }
-
-    if (requestType === 'objectPutACL') {
-        if ((objectMD.acl.Canned === 'bucket-owner-full-control'
-                && bucketOwner === canonicalID)
-            || objectMD.acl.FULL_CONTROL.indexOf(canonicalID) > -1
-            || objectMD.acl.WRITE_ACP.indexOf(canonicalID) > -1) {
-            return true;
-        }
-    }
-
-    if (requestType === 'objectGetACL') {
-        if ((objectMD.acl.Canned === 'bucket-owner-full-control'
-                && bucketOwner === canonicalID)
-            || objectMD.acl.FULL_CONTROL.indexOf(canonicalID) > -1
-            || objectMD.acl.READ_ACP.indexOf(canonicalID) > -1) {
-            return true;
-        }
-    }
-
-    // allow public reads on buckets that are whitelisted for anonymous reads
-    // TODO: remove this after bucket policies are implemented
-    const bucketAcl = bucket.getAcl();
-    const allowPublicReads = publicReadBuckets.includes(bucket.getName()) &&
-        bucketAcl.Canned === 'public-read' &&
-        (requestType === 'objectGet' || requestType === 'objectHead');
-    if (allowPublicReads) {
-        return true;
-    }
-    return false;
-}
-
-module.exports = {
-    isBucketAuthorized,
-    isObjAuthorized,
-};

lib/api/apiUtils/authorization/permissionChecks.js

@@ -0,0 +1,313 @@
+const { evaluators } = require('arsenal').policies;
+const constants = require('../../../../constants');
+
+const actionMap = {
+    's3:AbortMultipartUpload': 'multipartDelete',
+    's3:DeleteBucket': 'bucketDelete',
+    's3:DeleteBucketPolicy': 'bucketDeletePolicy',
+    's3:DeleteBucketWebsite': 'bucketDeleteWebsite',
+    's3:DeleteObject': 'objectDelete',
+    's3:DeleteObjectTagging': 'objectDeleteTagging',
+    's3:GetBucketAcl': 'bucketGetACL',
+    's3:GetBucketCORS': 'bucketGetCors',
+    's3:GetBucketLocation': 'bucketGetLocation',
+    's3:GetBucketPolicy': 'bucketGetPolicy',
+    's3:GetBucketVersioning': 'bucketGetVersioning',
+    's3:GetBucketWebsite': 'bucketGetWebsite',
+    's3:GetLifecycleConfiguration': 'bucketGetLifecycle',
+    's3:GetObject': 'objectGet',
+    's3:GetObjectAcl': 'objectGetACL',
+    's3:GetObjectTagging': 'objectGetTagging',
+    's3:GetReplicationConfiguration': 'bucketGetReplication',
+    's3:ListBucket': 'bucketHead',
+    's3:ListBucketMultipartUploads': 'listMultipartUploads',
+    's3:ListMultipartUploadParts': 'listParts',
+    's3:PutBucketAcl': 'bucketPutACL',
+    's3:PutBucketCORS': 'bucketPutCors',
+    's3:PutBucketPolicy': 'bucketPutPolicy',
+    's3:PutBucketVersioning': 'bucketPutVersioning',
+    's3:PutBucketWebsite': 'bucketPutWebsite',
+    's3:PutLifecycleConfiguration': 'bucketPutLifecycle',
+    's3:PutObject': 'objectPut',
+    's3:PutObjectAcl': 'objectPutACL',
+    's3:PutObjectTagging': 'objectPutTagging',
+    's3:PutReplicationConfiguration': 'bucketPutReplication',
+};
+
+// whitelist buckets to allow public read on objects
+const publicReadBuckets = process.env.ALLOW_PUBLIC_READ_BUCKETS ?
+    process.env.ALLOW_PUBLIC_READ_BUCKETS.split(',') : [];
+
+function checkBucketAcls(bucket, requestType, canonicalID) {
+    if (constants.bucketOwnerActions.includes(requestType)) {
+        // only bucket owner can modify or retrieve this property of a bucket
+        return false;
+    }
+
+    const bucketAcl = bucket.getAcl();
+    if (requestType === 'bucketGet' || requestType === 'bucketHead') {
+        if (bucketAcl.Canned === 'public-read'
+            || bucketAcl.Canned === 'public-read-write'
+            || (bucketAcl.Canned === 'authenticated-read'
+                && canonicalID !== constants.publicId)) {
+            return true;
+        } else if (bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
+            || bucketAcl.READ.indexOf(canonicalID) > -1) {
+            return true;
+        }
+    }
+    if (requestType === 'bucketGetACL') {
+        if ((bucketAcl.Canned === 'log-delivery-write'
+                && canonicalID === constants.logId)
+            || bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
+            || bucketAcl.READ_ACP.indexOf(canonicalID) > -1) {
+            return true;
+        }
+    }
+
+    if (requestType === 'bucketPutACL') {
+        if (bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
+            || bucketAcl.WRITE_ACP.indexOf(canonicalID) > -1) {
+            return true;
+        }
+    }
+
+    if (requestType === 'bucketDelete' && bucket.getOwner() === canonicalID) {
+        return true;
+    }
+
+    if (requestType === 'objectDelete' || requestType === 'objectPut') {
+        if (bucketAcl.Canned === 'public-read-write'
+            || bucketAcl.FULL_CONTROL.indexOf(canonicalID) > -1
+            || bucketAcl.WRITE.indexOf(canonicalID) > -1) {
+            return true;
+        }
+    }
+    // Note that an account can have the ability to do objectPutACL,
+    // objectGetACL, objectHead or objectGet even if the account has no rights
+    // to the bucket holding the object.  So, if the request type is
+    // objectPutACL, objectGetACL, objectHead or objectGet, the bucket
+    // authorization check should just return true so can move on to check
+    // rights at the object level.
+    return (requestType === 'objectPutACL' || requestType === 'objectGetACL' ||
+    requestType === 'objectGet' || requestType === 'objectHead');
+}
+
+function checkObjectAcls(bucket, objectMD, requestType, canonicalID) {
+    if (!objectMD.acl) {
+        return false;
+    }
+    const bucketOwner = bucket.getOwner();
+
+    if (requestType === 'objectGet' || requestType === 'objectHead') {
+        if (objectMD.acl.Canned === 'public-read'
+            || objectMD.acl.Canned === 'public-read-write'
+            || (objectMD.acl.Canned === 'authenticated-read'
+                && canonicalID !== constants.publicId)) {
+            return true;
+        } else if (objectMD.acl.Canned === 'bucket-owner-read'
+                && bucketOwner === canonicalID) {
+            return true;
+        } else if ((objectMD.acl.Canned === 'bucket-owner-full-control'
+                && bucketOwner === canonicalID)
+            || objectMD.acl.FULL_CONTROL.indexOf(canonicalID) > -1
+            || objectMD.acl.READ.indexOf(canonicalID) > -1) {
+            return true;
+        }
+    }
+
+    // User is already authorized on the bucket for FULL_CONTROL or WRITE or
+    // bucket has canned ACL public-read-write
+    if (requestType === 'objectPut' || requestType === 'objectDelete') {
+        return true;
+    }
+
+    if (requestType === 'objectPutACL') {
+        if ((objectMD.acl.Canned === 'bucket-owner-full-control'
+                && bucketOwner === canonicalID)
+            || objectMD.acl.FULL_CONTROL.indexOf(canonicalID) > -1
+            || objectMD.acl.WRITE_ACP.indexOf(canonicalID) > -1) {
+            return true;
+        }
+    }
+
+    if (requestType === 'objectGetACL') {
+        if ((objectMD.acl.Canned === 'bucket-owner-full-control'
+                && bucketOwner === canonicalID)
+            || objectMD.acl.FULL_CONTROL.indexOf(canonicalID) > -1
+            || objectMD.acl.READ_ACP.indexOf(canonicalID) > -1) {
+            return true;
+        }
+    }
+
+    // allow public reads on buckets that are whitelisted for anonymous reads
+    // TODO: remove this after bucket policies are implemented
+    const bucketAcl = bucket.getAcl();
+    const allowPublicReads = publicReadBuckets.includes(bucket.getName()) &&
+        bucketAcl.Canned === 'public-read' &&
+        (requestType === 'objectGet' || requestType === 'objectHead');
+    if (allowPublicReads) {
+        return true;
+    }
+    return false;
+}
+
+function _checkActions(requestType, actions, log) {
+    // if requestType isn't in list of controlled actions
+    if (!Object.values(actionMap).includes(requestType)) {
+        return true;
+    }
+    const mappedAction = Object.keys(actionMap)
+        [Object.values(actionMap).indexOf(requestType)];
+    return evaluators.isActionApplicable(mappedAction, actions, log);
+}
+
+function _getAccountId(arn) {
+    // account or user arn is of format 'arn:aws:iam::<12-digit-acct-id>:etc...
+    return arn.substr(13, 12);
+}
+
+function _isAccountId(principal) {
+    return (principal.length === 12 && /^\d+$/.test(principal));
+}
+
+function _checkPrincipal(requester, principal) {
+    if (principal === '*') {
+        return true;
+    }
+    if (principal === requester) {
+        return true;
+    }
+    if (_isAccountId(principal)) {
+        return _getAccountId(requester) === principal;
+    }
+    if (principal.endsWith('root')) {
+        return _getAccountId(requester) === _getAccountId(principal);
+    }
+    return false;
+}
+
+function _checkPrincipals(canonicalID, arn, principal) {
+    if (principal === '*') {
+        return true;
+    }
+    if (principal.CanonicalUser) {
+        if (Array.isArray(principal.CanonicalUser)) {
+            return principal.CanonicalUser.some(p => _checkPrincipal(canonicalID, p));
+        }
+        return _checkPrincipal(canonicalID, principal.CanonicalUser);
+    }
+    if (principal.AWS) {
+        if (Array.isArray(principal.AWS)) {
+            return principal.AWS.some(p => _checkPrincipal(arn, p));
+        }
+        return _checkPrincipal(arn, principal.AWS);
+    }
+    return false;
+}
+
+function checkBucketPolicy(policy, requestType, canonicalID, arn, log) {
+    let permission = 'defaultDeny';
+    let copiedStatement = JSON.parse(JSON.stringify(policy.Statement));
+    while (copiedStatement.length > 0) {
+        const s = copiedStatement[0];
+        const principalMatch = _checkPrincipals(canonicalID, arn, s.Principal);
+        const actionMatch = _checkActions(requestType, s.Action, log);
+
+        if (principalMatch && actionMatch && s.Effect === 'Deny') {
+            // explicit deny trumps any allows, so return immediately
+            return 'explicitDeny';
+        }
+        if (principalMatch && actionMatch && s.Effect === 'Allow') {
+            permission = 'allow';
+        }
+        copiedStatement = copiedStatement.splice(1);
+    }
+    return permission;
+}
+
+function isBucketAuthorized(bucket, requestType, canonicalID, arn, log) {
+    // Check to see if user is authorized to perform a
+    // particular action on bucket based on ACLs.
+    // TODO: Add IAM checks
+    if (bucket.getOwner() === canonicalID) {
+        return true;
+    }
+    const aclPermission = checkBucketAcls(bucket, requestType, canonicalID);
+    const bucketPolicy = bucket.getBucketPolicy();
+    if (!bucketPolicy) {
+        if (constants.bucketOwnerActions.includes(requestType)) {
+            return false;
+        }
+        return aclPermission;
+    }
+    const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType,
+        canonicalID, arn, log);
+    if (bucketPolicyPermission === 'explicitDeny') {
+        return false;
+    }
+    return (aclPermission || (bucketPolicyPermission === 'allow'));
+}
+
+function isObjAuthorized(bucket, objectMD, requestType, canonicalID, arn, log) {
+    const bucketOwner = bucket.getOwner();
+    if (!objectMD) {
+        return false;
+    }
+    if (objectMD['owner-id'] === canonicalID) {
+        return true;
+    }
+    // account is authorized if:
+    // - requesttype is included in bucketOwnerActions and
+    // - account is the bucket owner
+    if (constants.bucketOwnerActions.includes(requestType)
+    && bucketOwner === canonicalID) {
+        return true;
+    }
+    const aclPermission = checkObjectAcls(bucket, objectMD, requestType,
+        canonicalID);
+    const bucketPolicy = bucket.getBucketPolicy();
+    if (!bucketPolicy) {
+        return aclPermission;
+    }
+    const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType,
+        canonicalID, arn, log);
+    if (bucketPolicyPermission === 'explicitDeny') {
+        return false;
+    }
+    return (aclPermission || (bucketPolicyPermission === 'allow'));
+}
+
+function _checkResource(resource, bucketArn) {
+    if (resource === bucketArn) {
+        return true;
+    }
+    if (resource.includes('/')) {
+        const rSubs = resource.split('/');
+        return rSubs[0] === bucketArn;
+    }
+    return false;
+}
+
+// the resources specified in the bucket policy should contain the bucket name
+function validatePolicyResource(bucketName, policy) {
+    const bucketArn = `arn:aws:s3:::${bucketName}`;
+
+    return policy.Statement.every(s => {
+        if (Array.isArray(s.Resource)) {
+            return s.Resource.every(r => _checkResource(r, bucketArn));
+        }
+        if (typeof s.Resource === 'string') {
+            return _checkResource(s.Resource, bucketArn);
+        }
+        return false;
+    });
+}
+
+module.exports = {
+    isBucketAuthorized,
+    isObjAuthorized,
+    checkBucketAcls,
+    checkObjectAcls,
+    validatePolicyResource,
+};

lib/api/apiUtils/bucket/bucketShield.js

@@ -1,4 +1,5 @@
 const invisiblyDelete = require('./invisiblyDelete');
+const constants = require('../../../../constants');
 
 /**
  * Checks whether to proceed with a request based on the bucket flags
@@ -8,9 +9,17 @@ const invisiblyDelete = require('./invisiblyDelete');
  * @return {boolean} true if the bucket should be shielded, false otherwise
  */
 function bucketShield(bucket, requestType) {
-    const invisiblyDeleteRequests = ['bucketGet', 'bucketHead',
-        'bucketGetACL', 'bucketOwnerAction', 'objectGet', 'objectGetACL',
-        'objectHead', 'objectPutACL', 'objectDelete'];
+    const invisiblyDeleteRequests = constants.bucketOwnerActions.concat(
+        [
+            'bucketGet',
+            'bucketHead',
+            'bucketGetACL',
+            'objectGet',
+            'objectGetACL',
+            'objectHead',
+            'objectPutACL',
+            'objectDelete',
+        ]);
     if (invisiblyDeleteRequests.indexOf(requestType) > -1 &&
         bucket.hasDeletedFlag()) {
         invisiblyDelete(bucket.getName(), bucket.getOwner());

lib/api/apiUtils/object/abortMultipartUpload.js

@@ -16,7 +16,7 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
         bucketName,
         objectKey,
         uploadId,
-        requestType: 'deleteMPU',
+        preciseRequestType: 'multipartDelete',
     };
     // For validating the request at the destinationBucket level
     // params are the same as validating at the MPU level

lib/api/bucketDeleteCors.js

@@ -2,11 +2,12 @@ const { errors } = require('arsenal');
 
 const bucketShield = require('./apiUtils/bucket/bucketShield');
 const collectCorsHeaders = require('../utilities/collectCorsHeaders');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const metadata = require('../metadata/wrapper');
 const { pushMetric } = require('../utapi/utilities');
 
-const requestType = 'bucketOwnerAction';
+const requestType = 'bucketDeleteCors';
 
 /**
  * Bucket Delete CORS - Delete bucket cors configuration
@@ -19,6 +20,7 @@ const requestType = 'bucketOwnerAction';
 function bucketDeleteCors(authInfo, request, log, callback) {
     const bucketName = request.bucketName;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
 
     return metadata.getBucket(bucketName, log, (err, bucket) => {
         const corsHeaders = collectCorsHeaders(request.headers.origin,
@@ -32,7 +34,8 @@ function bucketDeleteCors(authInfo, request, log, callback) {
         }
         log.trace('found bucket in metadata');
 
-        if (!isBucketAuthorized(bucket, requestType, canonicalID)) {
+        if (!isBucketAuthorized(bucket, requestType, canonicalID, requestArn,
+        log)) {
             log.debug('access denied for user on bucket', {
                 requestType,
                 method: 'bucketDeleteCors',

lib/api/bucketDeleteLifecycle.js

@@ -17,7 +17,7 @@ function bucketDeleteLifecycle(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketDeleteLifecycle',
     };
     return metadataValidateBucket(metadataValParams, log, (err, bucket) => {
         const corsHeaders = collectCorsHeaders(headers.origin, method, bucket);

lib/api/bucketDeletePolicy.js

@@ -16,7 +16,7 @@ function bucketDeletePolicy(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketDeletePolicy',
     };
     return metadataValidateBucket(metadataValParams, log, (err, bucket) => {
         const corsHeaders = collectCorsHeaders(headers.origin, method, bucket);

lib/api/bucketDeleteReplication.js

@@ -17,7 +17,7 @@ function bucketDeleteReplication(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketDeleteReplication',
     };
     return metadataValidateBucket(metadataValParams, log, (err, bucket) => {
         const corsHeaders = collectCorsHeaders(headers.origin, method, bucket);

lib/api/bucketDeleteWebsite.js

@@ -2,15 +2,17 @@ const { errors } = require('arsenal');
 
 const bucketShield = require('./apiUtils/bucket/bucketShield');
 const collectCorsHeaders = require('../utilities/collectCorsHeaders');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const metadata = require('../metadata/wrapper');
 const { pushMetric } = require('../utapi/utilities');
 
-const requestType = 'bucketOwnerAction';
+const requestType = 'bucketDeleteWebsite';
 
 function bucketDeleteWebsite(authInfo, request, log, callback) {
     const bucketName = request.bucketName;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
 
     return metadata.getBucket(bucketName, log, (err, bucket) => {
         const corsHeaders = collectCorsHeaders(request.headers.origin,
@@ -24,7 +26,8 @@ function bucketDeleteWebsite(authInfo, request, log, callback) {
         }
         log.trace('found bucket in metadata');
 
-        if (!isBucketAuthorized(bucket, requestType, canonicalID)) {
+        if (!isBucketAuthorized(bucket, requestType, canonicalID, requestArn,
+        log)) {
             log.debug('access denied for user on bucket', {
                 requestType,
                 method: 'bucketDeleteWebsite',

lib/api/bucketGetCors.js

@@ -3,11 +3,12 @@ const { errors } = require('arsenal');
 const bucketShield = require('./apiUtils/bucket/bucketShield');
 const collectCorsHeaders = require('../utilities/collectCorsHeaders');
 const { convertToXml } = require('./apiUtils/bucket/bucketCors');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const metadata = require('../metadata/wrapper');
 const { pushMetric } = require('../utapi/utilities');
 
-const requestType = 'bucketOwnerAction';
+const requestType = 'bucketGetCors';
 
 /**
  * Bucket Get CORS - Get bucket cors configuration
@@ -20,6 +21,7 @@ const requestType = 'bucketOwnerAction';
 function bucketGetCors(authInfo, request, log, callback) {
     const bucketName = request.bucketName;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
 
     metadata.getBucket(bucketName, log, (err, bucket) => {
         if (err) {
@@ -33,7 +35,8 @@ function bucketGetCors(authInfo, request, log, callback) {
         const corsHeaders = collectCorsHeaders(request.headers.origin,
             request.method, bucket);
 
-        if (!isBucketAuthorized(bucket, requestType, canonicalID)) {
+        if (!isBucketAuthorized(bucket, requestType, canonicalID, requestArn,
+        log)) {
             log.debug('access denied for user on bucket', {
                 requestType,
                 method: 'bucketGetCors',

lib/api/bucketGetLifecycle.js

@@ -20,7 +20,7 @@ function bucketGetLifecycle(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketGetLifecycle',
     };
     return metadataValidateBucket(metadataValParams, log, (err, bucket) => {
         const corsHeaders = collectCorsHeaders(headers.origin, method, bucket);

lib/api/bucketGetLocation.js

@@ -1,13 +1,14 @@
 const { errors, s3middleware } = require('arsenal');
 
 const bucketShield = require('./apiUtils/bucket/bucketShield');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const metadata = require('../metadata/wrapper');
 const { pushMetric } = require('../utapi/utilities');
 const escapeForXml = s3middleware.escapeForXml;
 const collectCorsHeaders = require('../utilities/collectCorsHeaders');
 
-const requestType = 'bucketOwnerAction';
+const requestType = 'bucketGetLocation';
 
 /**
  * Bucket Get Location - Get bucket locationConstraint configuration
@@ -21,6 +22,7 @@ const requestType = 'bucketOwnerAction';
 function bucketGetLocation(authInfo, request, log, callback) {
     const bucketName = request.bucketName;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
 
     return metadata.getBucket(bucketName, log, (err, bucket) => {
         if (err) {
@@ -35,7 +37,8 @@ function bucketGetLocation(authInfo, request, log, callback) {
         const corsHeaders = collectCorsHeaders(request.headers.origin,
             request.method, bucket);
 
-        if (!isBucketAuthorized(bucket, requestType, canonicalID)) {
+        if (!isBucketAuthorized(bucket, requestType, canonicalID, requestArn,
+        log)) {
             log.debug('access denied for account on bucket', {
                 requestType,
                 method: 'bucketGetLocation',

lib/api/bucketGetPolicy.js

@@ -17,11 +17,8 @@ function bucketGetPolicy(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketGetPolicy',
     };
-    if (!process.env.BUCKET_POLICY) {
-        return callback(errors.NotImplemented);
-    }
 
     return metadataValidateBucket(metadataValParams, log, (err, bucket) => {
         const corsHeaders = collectCorsHeaders(headers.origin, method, bucket);
@@ -42,7 +39,9 @@ function bucketGetPolicy(authInfo, request, log, callback) {
                 corsHeaders);
         }
         // TODO: implement Utapi metric support
-        return callback(null, bucketPolicy, corsHeaders);
+        // bucketPolicy needs to be JSON stringified on return for proper
+        // parsing on return to caller function
+        return callback(null, JSON.stringify(bucketPolicy), corsHeaders);
     });
 }
 

lib/api/bucketGetReplication.js

@@ -20,7 +20,7 @@ function bucketGetReplication(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketGetReplication',
     };
     return metadataValidateBucket(metadataValParams, log, (err, bucket) => {
         const corsHeaders = collectCorsHeaders(headers.origin, method, bucket);

lib/api/bucketGetVersioning.js

@@ -53,7 +53,7 @@ function bucketGetVersioning(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketGetVersioning',
     };
 
     metadataValidateBucket(metadataValParams, log, (err, bucket) => {

lib/api/bucketGetWebsite.js

@@ -3,11 +3,12 @@ const { errors } = require('arsenal');
 const bucketShield = require('./apiUtils/bucket/bucketShield');
 const { convertToXml } = require('./apiUtils/bucket/bucketWebsite');
 const collectCorsHeaders = require('../utilities/collectCorsHeaders');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const metadata = require('../metadata/wrapper');
 const { pushMetric } = require('../utapi/utilities');
 
-const requestType = 'bucketOwnerAction';
+const requestType = 'bucketGetWebsite';
 
 /**
  * Bucket Get Website - Get bucket website configuration
@@ -20,6 +21,7 @@ const requestType = 'bucketOwnerAction';
 function bucketGetWebsite(authInfo, request, log, callback) {
     const bucketName = request.bucketName;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
 
     metadata.getBucket(bucketName, log, (err, bucket) => {
         if (err) {
@@ -33,7 +35,8 @@ function bucketGetWebsite(authInfo, request, log, callback) {
 
         const corsHeaders = collectCorsHeaders(request.headers.origin,
             request.method, bucket);
-        if (!isBucketAuthorized(bucket, requestType, canonicalID)) {
+        if (!isBucketAuthorized(bucket, requestType, canonicalID, requestArn,
+        log)) {
             log.debug('access denied for user on bucket', {
                 requestType,
                 method: 'bucketGetWebsite',

lib/api/bucketPutCors.js

@@ -4,12 +4,13 @@ const { errors } = require('arsenal');
 
 const bucketShield = require('./apiUtils/bucket/bucketShield');
 const collectCorsHeaders = require('../utilities/collectCorsHeaders');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const metadata = require('../metadata/wrapper');
 const { parseCorsXml } = require('./apiUtils/bucket/bucketCors');
 const { pushMetric } = require('../utapi/utilities');
 
-const requestType = 'bucketOwnerAction';
+const requestType = 'bucketPutCors';
 
 /**
  * Bucket Put Cors - Adds cors rules to bucket
@@ -23,6 +24,7 @@ function bucketPutCors(authInfo, request, log, callback) {
     log.debug('processing request', { method: 'bucketPutCors' });
     const bucketName = request.bucketName;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
 
     if (!request.post) {
         log.debug('CORS xml body is missing',
@@ -65,7 +67,8 @@ function bucketPutCors(authInfo, request, log, callback) {
             });
         },
         function validateBucketAuthorization(bucket, rules, corsHeaders, next) {
-            if (!isBucketAuthorized(bucket, requestType, canonicalID)) {
+            if (!isBucketAuthorized(bucket, requestType, canonicalID,
+            requestArn, log)) {
                 log.debug('access denied for account on bucket', {
                     requestType,
                 });

lib/api/bucketPutLifecycle.js

@@ -24,7 +24,7 @@ function bucketPutLifecycle(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketPutLifecycle',
     };
     return waterfall([
         next => parseXML(request.post, log, next),

lib/api/bucketPutPolicy.js

@@ -1,10 +1,12 @@
 const async = require('async');
-const { BucketPolicy } = require('arsenal').models;
+const { errors, models } = require('arsenal');
 
 const collectCorsHeaders = require('../utilities/collectCorsHeaders');
 const metadata = require('../metadata/wrapper');
 const { metadataValidateBucket } = require('../metadata/metadataUtils');
-const { errors } = require('arsenal');
+const { validatePolicyResource } =
+    require('./apiUtils/authorization/permissionChecks');
+const { BucketPolicy } = models;
 
 /**
  * bucketPutPolicy - create or update a bucket policy
@@ -21,27 +23,33 @@ function bucketPutPolicy(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketPutPolicy',
     };
 
-    if (!process.env.BUCKET_POLICY) {
-        return callback(errors.NotImplemented);
-    }
-
     return async.waterfall([
         next => {
-            const policyJSON = JSON.parse(request.post);
-            const bucketPolicy = new BucketPolicy(policyJSON);
+            const bucketPolicy = new BucketPolicy(request.post);
             // if there was an error getting bucket policy,
             // returned policyObj will contain 'error' key
             process.nextTick(() => {
                 const policyObj = bucketPolicy.getBucketPolicy();
                 if (policyObj.error) {
-                    return next(policyObj.error);
+                    const err = errors.MalformedPolicy.customizeDescription(
+                        policyObj.error.description);
+                    return next(err);
                 }
                 return next(null, policyObj);
             });
         },
+        (bucketPolicy, next) => {
+            process.nextTick(() => {
+                if (!validatePolicyResource(bucketName, bucketPolicy)) {
+                    return next(errors.MalformedPolicy.customizeDescription(
+                        'Policy has invalid resource'));
+                }
+                return next(null, bucketPolicy);
+            });
+        },
         (bucketPolicy, next) => metadataValidateBucket(metadataValParams, log,
             (err, bucket) => {
                 if (err) {

lib/api/bucketPutReplication.js

@@ -27,7 +27,7 @@ function bucketPutReplication(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketPutReplication',
     };
     return waterfall([
         // Validate the request XML and return the replication configuration.

lib/api/bucketPutVersioning.js

@@ -81,7 +81,7 @@ function bucketPutVersioning(authInfo, request, log, callback) {
     const metadataValParams = {
         authInfo,
         bucketName,
-        requestType: 'bucketOwnerAction',
+        requestType: 'bucketPutVersioning',
     };
 
     return waterfall([

lib/api/bucketPutWebsite.js

@@ -3,12 +3,13 @@ const { errors } = require('arsenal');
 
 const bucketShield = require('./apiUtils/bucket/bucketShield');
 const collectCorsHeaders = require('../utilities/collectCorsHeaders');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const metadata = require('../metadata/wrapper');
 const { parseWebsiteConfigXml } = require('./apiUtils/bucket/bucketWebsite');
 const { pushMetric } = require('../utapi/utilities');
 
-const requestType = 'bucketOwnerAction';
+const requestType = 'bucketPutWebsite';
 
 /**
  * Bucket Put Website - Create bucket website configuration
@@ -22,6 +23,7 @@ function bucketPutWebsite(authInfo, request, log, callback) {
     log.debug('processing request', { method: 'bucketPutWebsite' });
     const bucketName = request.bucketName;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
 
     if (!request.post) {
         return callback(errors.MissingRequestBodyError);
@@ -45,7 +47,8 @@ function bucketPutWebsite(authInfo, request, log, callback) {
             });
         },
         function validateBucketAuthorization(bucket, config, next) {
-            if (!isBucketAuthorized(bucket, requestType, canonicalID)) {
+            if (!isBucketAuthorized(bucket, requestType, canonicalID,
+            requestArn, log)) {
                 log.debug('access denied for user on bucket', {
                     requestType,
                     method: 'bucketPutWebsite',

lib/api/listMultipartUploads.js

@@ -96,6 +96,7 @@ function listMultipartUploads(authInfo, request, log, callback) {
         // the authorization to list multipart uploads is the same
         // as listing objects in a bucket.
         requestType: 'bucketGet',
+        preciseRequestType: 'listMultipartUploads',
     };
 
     async.waterfall([

lib/api/listParts.js

@@ -95,7 +95,7 @@ function listParts(authInfo, request, log, callback) {
         bucketName,
         objectKey,
         uploadId,
-        requestType: 'listParts',
+        preciseRequestType: 'listParts',
     };
     // For validating the request at the destinationBucket level
     // params are the same as validating at the MPU level

lib/api/multiObjectDelete.js

@@ -11,7 +11,8 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders');
 const metadata = require('../metadata/wrapper');
 const services = require('../services');
 const vault = require('../auth/vault');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const { preprocessingVersioningDelete }
     = require('./apiUtils/object/versioning');
 const createAndStoreObject = require('./apiUtils/object/createAndStoreObject');
@@ -316,6 +317,7 @@ function multiObjectDelete(authInfo, request, log, callback) {
 
     const bucketName = request.bucketName;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
 
     return async.waterfall([
         function parseXML(next) {
@@ -448,7 +450,7 @@ function multiObjectDelete(authInfo, request, log, callback) {
                         bucketMD);
                 }
                 if (!isBucketAuthorized(bucketMD, 'objectDelete',
-                    canonicalID)) {
+                    canonicalID, requestArn, log)) {
                     log.trace("access denied due to bucket acl's");
                     // if access denied at the bucket level, no access for
                     // any of the objects so all results will be error results

lib/api/objectDeleteTagging.js

@@ -41,7 +41,7 @@ function objectDeleteTagging(authInfo, request, log, callback) {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'bucketOwnerAction',
+        requestType: 'objectDeleteTagging',
         versionId: reqVersionId,
     };
 

lib/api/objectGetTagging.js

@@ -37,7 +37,7 @@ function objectGetTagging(authInfo, request, log, callback) {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'bucketOwnerAction',
+        requestType: 'objectGetTagging',
         versionId: reqVersionId,
     };
 

lib/api/objectPutPart.js

@@ -7,7 +7,8 @@ const { BackendInfo } = require('./apiUtils/object/BackendInfo');
 const constants = require('../../constants');
 const data = require('../data/wrapper');
 const { dataStore } = require('./apiUtils/object/storeObject');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const kms = require('../kms/wrapper');
 const metadata = require('../metadata/wrapper');
 const { pushMetric } = require('../utapi/utilities');
@@ -81,6 +82,7 @@ function objectPutPart(authInfo, request, streamingV4Params, log,
     log.trace('owner canonicalid to send to data', {
         canonicalID: authInfo.getCanonicalID,
     });
+    const requestArn = authInfo.getArn();
     // Note that keys in the query object retain their case, so
     // `request.query.uploadId` must be called with that exact capitalization.
     const uploadId = request.query.uploadId;
@@ -109,7 +111,7 @@ function objectPutPart(authInfo, request, streamingV4Params, log,
             // `requestType` is the general 'objectPut'.
             const requestType = 'objectPut';
             if (!isBucketAuthorized(destinationBucket, requestType,
-                canonicalID)) {
+                canonicalID, requestArn, log)) {
                 log.debug('access denied for user on bucket', { requestType });
                 return next(errors.AccessDenied, destinationBucket);
             }

lib/api/objectPutTagging.js

@@ -42,7 +42,7 @@ function objectPutTagging(authInfo, request, log, callback) {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'bucketOwnerAction',
+        requestType: 'objectPutTagging',
         versionId: reqVersionId,
     };
 

lib/api/websiteGet.js

@@ -7,10 +7,10 @@ const metadata = require('../metadata/wrapper');
 const bucketShield = require('./apiUtils/bucket/bucketShield');
 const { findRoutingRule, extractRedirectInfo } =
     require('./apiUtils/object/websiteServing');
-const { isObjAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isObjAuthorized, isBucketAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 const collectResponseHeaders = require('../utilities/collectResponseHeaders');
 const { pushMetric } = require('../utapi/utilities');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
 
 /**
  * _errorActions - take a number of actions once have error getting obj
@@ -47,7 +47,7 @@ function _errorActions(err, errorDocument, routingRules,
                 // return the default error message if the object is private
                 // rather than sending a stored error file
                 if (!isObjAuthorized(bucket, errObjMD, 'objectGet',
-                    constants.publicId)) {
+                    constants.publicId, null, log)) {
                     log.trace('errorObj not authorized', { error: err });
                     return callback(err, true, null, corsHeaders);
                 }
@@ -144,7 +144,7 @@ function websiteGet(request, log, callback) {
                     { error: err });
                     let returnErr = err;
                     const bucketAuthorized = isBucketAuthorized(bucket,
-                      'bucketGet', constants.publicId);
+                      'bucketGet', constants.publicId, null, log);
                     // if index object does not exist and bucket is private AWS
                     // returns 403 - AccessDenied error.
                     if (err === errors.NoSuchKey && !bucketAuthorized) {
@@ -156,7 +156,7 @@ function websiteGet(request, log, callback) {
                       callback);
                 }
                 if (!isObjAuthorized(bucket, objMD, 'objectGet',
-                    constants.publicId)) {
+                    constants.publicId, null, log)) {
                     const err = errors.AccessDenied;
                     log.trace('request not authorized', { error: err });
                     return _errorActions(err, websiteConfig.getErrorDocument(),

lib/api/websiteHead.js

@@ -7,10 +7,10 @@ const metadata = require('../metadata/wrapper');
 const bucketShield = require('./apiUtils/bucket/bucketShield');
 const { findRoutingRule, extractRedirectInfo } =
     require('./apiUtils/object/websiteServing');
-const { isObjAuthorized } = require('./apiUtils/authorization/aclChecks');
 const collectResponseHeaders = require('../utilities/collectResponseHeaders');
 const { pushMetric } = require('../utapi/utilities');
-const { isBucketAuthorized } = require('./apiUtils/authorization/aclChecks');
+const { isBucketAuthorized, isObjAuthorized } =
+    require('./apiUtils/authorization/permissionChecks');
 
 
 /**
@@ -104,7 +104,7 @@ function websiteHead(request, log, callback) {
                     { error: err });
                     let returnErr = err;
                     const bucketAuthorized = isBucketAuthorized(bucket,
-                      'bucketGet', constants.publicId);
+                      'bucketGet', constants.publicId, null, log);
                     // if index object does not exist and bucket is private AWS
                     // returns 403 - AccessDenied error.
                     if (err === errors.NoSuchKey && !bucketAuthorized) {
@@ -114,7 +114,7 @@ function websiteHead(request, log, callback) {
                         reqObjectKey, corsHeaders, log, callback);
                 }
                 if (!isObjAuthorized(bucket, objMD, 'objectGet',
-                    constants.publicId)) {
+                    constants.publicId, null, log)) {
                     const err = errors.AccessDenied;
                     log.trace('request not authorized', { error: err });
                     return _errorActions(err, routingRules, reqObjectKey,

lib/metadata/metadataUtils.js

@@ -4,7 +4,7 @@ const { errors } = require('arsenal');
 const metadata = require('./wrapper');
 const BucketInfo = require('arsenal').models.BucketInfo;
 const { isBucketAuthorized, isObjAuthorized } =
-    require('../api/apiUtils/authorization/aclChecks');
+    require('../api/apiUtils/authorization/permissionChecks');
 const bucketShield = require('../api/apiUtils/bucket/bucketShield');
 
 /** _parseListEntries - parse the values returned in a listing by metadata
@@ -174,15 +174,23 @@ function metadataGetObject(bucketName, objectKey, versionId, log, cb) {
  * @return {undefined} - and call callback with params err, bucket md
  */
 function metadataValidateBucketAndObj(params, log, callback) {
-    const { authInfo, bucketName, objectKey, versionId, requestType } = params;
+    const { authInfo, bucketName, objectKey, versionId, requestType, preciseRequestType } = params;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
     async.waterfall([
         function getBucketAndObjectMD(next) {
             return metadataGetBucketAndObject(requestType, bucketName,
                 objectKey, versionId, log, next);
         },
         function checkBucketAuth(bucket, objMD, next) {
-            if (!isBucketAuthorized(bucket, requestType, canonicalID)) {
+            // if requester is not bucket owner, bucket policy actions should be denied with
+            // MethodNotAllowed error
+            const onlyOwnerAllowed = ['bucketDeletePolicy', 'bucketGetPolicy', 'bucketPutPolicy'];
+            if (bucket.getOwner() !== canonicalID && onlyOwnerAllowed.includes(requestType)) {
+                return next(errors.MethodNotAllowed, bucket);
+            }
+            if (!isBucketAuthorized(bucket, (preciseRequestType || requestType), canonicalID,
+            requestArn, log)) {
                 log.debug('access denied for user on bucket', { requestType });
                 return next(errors.AccessDenied, bucket);
             }
@@ -199,7 +207,8 @@ function metadataValidateBucketAndObj(params, log, callback) {
             if (!objMD) {
                 return next(null, bucket);
             }
-            if (!isObjAuthorized(bucket, objMD, requestType, canonicalID)) {
+            if (!isObjAuthorized(bucket, objMD, requestType, canonicalID,
+            requestArn, log)) {
                 log.debug('access denied for user on object', { requestType });
                 return next(errors.AccessDenied, bucket);
             }
@@ -251,14 +260,21 @@ function metadataGetBucket(requestType, bucketName, log, cb) {
  * @return {undefined} - and call callback with params err, bucket md
  */
 function metadataValidateBucket(params, log, callback) {
-    const { authInfo, bucketName, requestType } = params;
+    const { authInfo, bucketName, requestType, preciseRequestType } = params;
     const canonicalID = authInfo.getCanonicalID();
+    const requestArn = authInfo.getArn();
     return metadataGetBucket(requestType, bucketName, log, (err, bucket) => {
         if (err) {
             return callback(err);
         }
+        // if requester is not bucket owner, bucket policy actions should be denied with
+        // MethodNotAllowed error
+        const onlyOwnerAllowed = ['bucketDeletePolicy', 'bucketGetPolicy', 'bucketPutPolicy'];
+        if (bucket.getOwner() !== canonicalID && onlyOwnerAllowed.includes(requestType)) {
+            return callback(errors.MethodNotAllowed, bucket);
+        }
         // still return bucket for cors headers
-        if (!isBucketAuthorized(bucket, requestType, canonicalID)) {
+        if (!isBucketAuthorized(bucket, (preciseRequestType || requestType), canonicalID, requestArn, log)) {
             log.debug('access denied for user on bucket', { requestType });
             return callback(errors.AccessDenied, bucket);
         }

package.json

@@ -19,7 +19,7 @@
   },
   "homepage": "https://github.com/scality/S3#readme",
   "dependencies": {
-    "arsenal": "github:scality/Arsenal#98737a69",
+    "arsenal": "github:scality/Arsenal#61d7790",
     "async": "~2.5.0",
     "aws-sdk": "2.28.0",
     "azure-storage": "^2.1.0",

tests/functional/aws-node-sdk/test/bucket/deleteBucketPolicy.js

@@ -9,11 +9,11 @@ const bucket = 'deletebucketpolicy-test-bucket';
 const bucketPolicy = {
     Version: '2012-10-17',
     Statement: [{
-        Sid: 'test-id',
+        Sid: 'testid',
         Effect: 'Allow',
-        Principle: '*',
+        Principal: '*',
         Action: 's3:putBucketPolicy',
-        Resource: `arn:aws:s3::${bucket}`,
+        Resource: `arn:aws:s3:::${bucket}`,
     }],
 };
 
@@ -31,10 +31,7 @@ function assertError(err, expectedErr, cb) {
     cb();
 }
 
-const describeSkipUntilImpl =
-    process.env.BUCKET_POLICY ? describe : describe.skip;
-
-describeSkipUntilImpl('aws-sdk test delete bucket policy', () => {
+describe('aws-sdk test delete bucket policy', () => {
     let s3;
     let otherAccountS3;
 
@@ -55,9 +52,9 @@ describeSkipUntilImpl('aws-sdk test delete bucket policy', () => {
 
         afterEach(done => s3.deleteBucket({ Bucket: bucket }, done));
 
-        it('should return AccessDenied if user is not bucket owner', done => {
+        it('should return MethodNotAllowed if user is not bucket owner', done => {
             otherAccountS3.deleteBucketPolicy({ Bucket: bucket },
-            err => assertError(err, 'AccessDenied', done));
+            err => assertError(err, 'MethodNotAllowed', done));
         });
 
         it('should return no error if no policy on bucket', done => {
@@ -66,7 +63,7 @@ describeSkipUntilImpl('aws-sdk test delete bucket policy', () => {
         });
 
         it('should delete policy from bucket', done => {
-            const params = { Bucket: bucket, Policy: bucketPolicy };
+            const params = { Bucket: bucket, Policy: JSON.stringify(bucketPolicy) };
             s3.putBucketPolicy(params, err => {
                 assert.equal(err, null);
                 s3.deleteBucketPolicy({ Bucket: bucket }, err => {

tests/functional/aws-node-sdk/test/bucket/getBucketPolicy.js

@@ -9,19 +9,19 @@ const bucket = 'getbucketpolicy-testbucket';
 const bucketPolicy = {
     Version: '2012-10-17',
     Statement: [{
-        Sid: 'test-id',
+        Sid: 'testid',
         Effect: 'Allow',
-        Principle: '*',
+        Principal: '*',
         Action: 's3:putBucketPolicy',
-        Resource: 'arn:aws:s3::getbucketpolicy-testbucket',
+        Resource: `arn:aws:s3:::${bucket}`,
     }],
 };
 const expectedPolicy = {
-    Sid: 'test-id',
+    Sid: 'testid',
     Effect: 'Allow',
-    Principle: '*',
+    Principal: '*',
     Action: 's3:putBucketPolicy',
-    Resource: 'arn:aws:s3::getbucketpolicy-testbucket',
+    Resource: `arn:aws:s3:::${bucket}`,
 };
 
 // Check for the expected error response code and status code.
@@ -38,10 +38,7 @@ function assertError(err, expectedErr, cb) {
     cb();
 }
 
-const describeSkipUntilImpl =
-    process.env.BUCKET_POLICY ? describe : describe.skip;
-
-describeSkipUntilImpl('aws-sdk test get bucket policy', () => {
+describe('aws-sdk test get bucket policy', () => {
     const config = getConfig('default', { signatureVersion: 'v4' });
     const s3 = new S3(config);
     const otherAccountS3 = new BucketUtility('lisa', {}).s3;
@@ -56,9 +53,9 @@ describeSkipUntilImpl('aws-sdk test get bucket policy', () => {
 
         afterEach(done => s3.deleteBucket({ Bucket: bucket }, done));
 
-        it('should return AccessDenied if user is not bucket owner', done => {
+        it('should return MethodNotAllowed if user is not bucket owner', done => {
             otherAccountS3.getBucketPolicy({ Bucket: bucket },
-            err => assertError(err, 'AccessDenied', done));
+            err => assertError(err, 'MethodNotAllowed', done));
         });
 
         it('should return NoSuchBucketPolicy error if no policy put to bucket',
@@ -71,14 +68,15 @@ describeSkipUntilImpl('aws-sdk test get bucket policy', () => {
         it('should get bucket policy', done => {
             s3.putBucketPolicy({
                 Bucket: bucket,
-                Policy: bucketPolicy,
+                Policy: JSON.stringify(bucketPolicy),
             }, err => {
                 assert.equal(err, null, `Err putting bucket policy: ${err}`);
                 s3.getBucketPolicy({ Bucket: bucket },
                 (err, res) => {
+                    const parsedRes = JSON.parse(res.Policy);
                     assert.equal(err, null, 'Error getting bucket policy: ' +
                         `${err}`);
-                    assert.deepStrictEqual(res.Statement[0], expectedPolicy);
+                    assert.deepStrictEqual(parsedRes.Statement[0], expectedPolicy);
                     done();
                 });
             });

tests/functional/aws-node-sdk/test/bucket/putBucketPolicy.js

@@ -7,11 +7,11 @@ const BucketUtility = require('../../lib/utility/bucket-util');
 
 const bucket = 'policyputtestbucket';
 const basicStatement = {
-    Sid: 'statement-id',
+    Sid: 'statementid',
     Effect: 'Allow',
     Principal: '*',
     Action: ['s3:putBucketPolicy'],
-    Resource: 'aws:arn:s3::example-bucket',
+    Resource: `arn:aws:s3:::${bucket}`,
 };
 
 function getPolicyParams(paramToChange) {
@@ -26,7 +26,7 @@ function getPolicyParams(paramToChange) {
     }
     return {
         Bucket: bucket,
-        Policy: bucketPolicy,
+        Policy: JSON.stringify(bucketPolicy),
     };
 }
 
@@ -44,10 +44,7 @@ function assertError(err, expectedErr, cb) {
     cb();
 }
 
-const describeSkipUntilImpl =
-    process.env.BUCKET_POLICY ? describe : describe.skip;
-
-describeSkipUntilImpl('aws-sdk test put bucket policy', () => {
+describe('aws-sdk test put bucket policy', () => {
     let s3;
     let otherAccountS3;
 
@@ -69,10 +66,10 @@ describeSkipUntilImpl('aws-sdk test put bucket policy', () => {
 
         afterEach(done => s3.deleteBucket({ Bucket: bucket }, done));
 
-        it('should return AccessDenied if user is not bucket owner', done => {
+        it('should return MethodNotAllowed if user is not bucket owner', done => {
             const params = getPolicyParams();
             otherAccountS3.putBucketPolicy(params,
-                err => assertError(err, 'AccessDenied', done));
+                err => assertError(err, 'MethodNotAllowed', done));
         });
 
         it('should put a bucket policy on bucket', done => {

tests/unit/api/bucketACLauth.js

@@ -2,13 +2,15 @@ const assert = require('assert');
 const BucketInfo = require('arsenal').models.BucketInfo;
 const constants = require('../../../constants');
 const { isBucketAuthorized }
-    = require('../../../lib/api/apiUtils/authorization/aclChecks');
+    = require('../../../lib/api/apiUtils/authorization/permissionChecks');
+const { DummyRequestLogger } = require('../helpers');
 
 const ownerCanonicalId = 'ownerCanonicalId';
 const creationDate = new Date().toJSON();
 const bucket = new BucketInfo('niftyBucket', ownerCanonicalId,
     'iAmTheOwnerDisplayName', creationDate);
 const accountToVet = 'accountToVetId';
+const log = new DummyRequestLogger();
 
 describe('bucket authorization for bucketGet, bucketHead, ' +
     'objectGet, and objectHead', () => {
@@ -92,7 +94,7 @@ describe('bucket authorization for bucketGet, bucketHead, ' +
             }
             bucket.setCannedAcl(value.canned);
             const results = requestTypes.map(type =>
-                isBucketAuthorized(bucket, type, value.id));
+                isBucketAuthorized(bucket, type, value.id, null, log));
             assert.deepStrictEqual(results, value.response);
             done();
         });
@@ -199,7 +201,7 @@ describe('bucket authorization for bucketOwnerAction', () => {
     });
 
     it('should allow access to bucket owner', () => {
-        const result = isBucketAuthorized(bucket, 'bucketOwnerAction',
+        const result = isBucketAuthorized(bucket, 'bucketDeleteCors',
             ownerCanonicalId);
         assert.strictEqual(result, true);
     });
@@ -221,7 +223,7 @@ describe('bucket authorization for bucketOwnerAction', () => {
                 bucket.setSpecificAcl(value.aclParam[1], value.aclParam[0]);
             }
             bucket.setCannedAcl(value.canned);
-            const result = isBucketAuthorized(bucket, 'bucketOwnerAction',
+            const result = isBucketAuthorized(bucket, 'bucketDeleteCors',
                 value.id);
             assert.strictEqual(result, false);
             done();

tests/unit/api/bucketDeletePolicy.js

@@ -22,14 +22,13 @@ function _makeRequest(includePolicy) {
     };
     if (includePolicy) {
         const examplePolicy = {
-            version: '2012-10-17',
-            statements: [
+            Version: '2012-10-17',
+            Statement: [
                 {
-                    sid: '',
-                    effect: 'Allow',
-                    resource: 'arn:aws:s3::bucketname',
-                    principal: '*',
-                    actions: ['s3:sampleAction'],
+                    Effect: 'Allow',
+                    Resource: `arn:aws:s3:::${bucketName}`,
+                    Principal: '*',
+                    Action: ['s3:GetBucketLocation'],
                 },
             ],
         };
@@ -38,10 +37,7 @@ function _makeRequest(includePolicy) {
     return request;
 }
 
-const describeSkipUntilImpl =
-    process.env.BUCKET_POLICY ? describe : describe.skip;
-
-describeSkipUntilImpl('deleteBucketPolicy API', () => {
+describe('deleteBucketPolicy API', () => {
     before(() => cleanup());
     beforeEach(done => bucketPut(authInfo, _makeRequest(), log, done));
     afterEach(() => cleanup());

tests/unit/api/bucketGetPolicy.js

@@ -19,14 +19,13 @@ const testBasicRequest = {
 };
 
 const expectedBucketPolicy = {
-    version: '2012-10-17',
-    statements: [
+    Version: '2012-10-17',
+    Statement: [
         {
-            sid: '',
-            effect: '',
-            resource: '',
-            principal: '',
-            actions: '',
+            Effect: 'Allow',
+            Resource: `arn:aws:s3:::${bucketName}`,
+            Principal: '*',
+            Action: ['s3:GetBucketLocation'],
         },
     ],
 };
@@ -37,10 +36,7 @@ const testPutPolicyRequest = {
     post: JSON.stringify(expectedBucketPolicy),
 };
 
-const describeSkipUntilImpl =
-    process.env.BUCKET_POLICY ? describe : describe.skip;
-
-describeSkipUntilImpl('getBucketPolicy API', () => {
+describe('getBucketPolicy API', () => {
     before(() => cleanup());
     beforeEach(done => bucketPut(authInfo, testBasicRequest, log, done));
     afterEach(() => cleanup());
@@ -64,7 +60,7 @@ describeSkipUntilImpl('getBucketPolicy API', () => {
         it('should return bucket policy', done => {
             bucketGetPolicy(authInfo, testBasicRequest, log, (err, res) => {
                 assert.equal(err, null);
-                assert.deepStrictEqual(expectedBucketPolicy, res);
+                assert.deepStrictEqual(expectedBucketPolicy, JSON.parse(res));
                 done();
             });
         });

tests/unit/api/bucketPolicyAuth.js

@@ -0,0 +1,358 @@
+const assert = require('assert');
+const { BucketInfo, BucketPolicy } = require('arsenal').models;
+const constants = require('../../../constants');
+const { isBucketAuthorized, isObjAuthorized, validatePolicyResource }
+    = require('../../../lib/api/apiUtils/authorization/permissionChecks');
+const { DummyRequestLogger } = require('../helpers');
+
+const bucketOwnerCanonicalId = 'bucketOwnerCanonicalId';
+const creationDate = new Date().toJSON();
+const bucket = new BucketInfo('policyBucketAuthTester', bucketOwnerCanonicalId,
+    'iAmTheOwnerDisplayName', creationDate);
+const objectOwnerCanonicalId = 'objectOwnerCanonicalId';
+const object = { 'owner-id': objectOwnerCanonicalId };
+const canonicalIdToVet = 'canonicalIdToVet';
+const userArn = 'arn:aws:iam::123456789012:user/user';
+const otherUserArn = 'arn:aws:iam::123456789012:user/other';
+const otherAccountUserArn = 'arn:aws:iam:987654321098:user/other';
+const accountArn = 'arn:aws:iam::123456789012:root';
+const accountId = '123456789012';
+const bucAction = 'bucketHead';
+const objAction = 'objectPut';
+const basePolicyObj = {
+    Version: '2012-10-17',
+    Statement: {
+        Effect: 'Allow',
+        Principal: '*',
+        Resource: `arn:aws:s3:::${bucket.getName()}`,
+        Action: 's3:*',
+    },
+};
+const bucketName = 'matchme';
+const log = new DummyRequestLogger();
+
+const authTests = [
+    {
+        name: 'should allow access if canonical user principal matches non-',
+        bucketId: canonicalIdToVet,
+        objectId: canonicalIdToVet,
+        keyToChange: 'Principal',
+        bucketValue: { CanonicalUser: [canonicalIdToVet] },
+        objectValue: { CanonicalUser: [canonicalIdToVet] },
+        expected: true,
+    },
+    {
+        name: 'should allow access if user arn principal matches non-',
+        bucketId: userArn,
+        objectId: userArn,
+        keyToChange: 'Principal',
+        bucketValue: { AWS: userArn },
+        objectValue: { AWS: userArn },
+        expected: true,
+    },
+    {
+        name: 'should allow access if account arn principal matches non-',
+        bucketId: accountArn,
+        objectId: accountArn,
+        keyToChange: 'Principal',
+        bucketValue: { AWS: accountArn },
+        objectValue: { AWS: accountArn },
+        expected: true,
+    },
+    {
+        name: 'should allow access if account id principal matches non-',
+        bucketId: accountId,
+        objectId: accountId,
+        keyToChange: 'Principal',
+        bucketValue: { AWS: accountId },
+        objectValue: { AWS: accountId },
+        expected: true,
+    },
+    {
+        name: 'should allow access if account id principal is contained in ' +
+            'user arn of non-',
+        bucketId: userArn,
+        objectId: userArn,
+        keyToChange: 'Principal',
+        bucketValue: { AWS: accountId },
+        objectValue: { AWS: accountId },
+        expected: true,
+    },
+    {
+        name: 'should allow access if account id principal is contained in ' +
+            'account arn of non-',
+        bucketId: accountArn,
+        objectId: accountArn,
+        keyToChange: 'Principal',
+        bucketValue: { AWS: accountId },
+        objectValue: { AWS: accountId },
+        expected: true,
+    },
+    {
+        name: 'should allow access if account arn principal is contained in ' +
+            'user arn of non-',
+        bucketId: userArn,
+        objectId: userArn,
+        keyToChange: 'Principal',
+        bucketValue: { AWS: accountArn },
+        objectValue: { AWS: accountArn },
+        expected: true,
+    },
+    {
+        name: 'should deny access if account arn principal doesn\'t match ' +
+            'user arn of non-',
+        bucketId: otherAccountUserArn,
+        objectId: otherAccountUserArn,
+        keyToChange: 'Principal',
+        bucketValue: { AWS: accountArn },
+        objectValue: { AWS: accountArn },
+        expected: false,
+    },
+    {
+        name: 'should deny access if user arn principal doesn\'t match ' +
+            'user arn of non-',
+        bucketId: userArn,
+        objectId: userArn,
+        keyToChange: 'Principal',
+        bucketValue: { AWS: otherUserArn },
+        objectValue: { AWS: otherUserArn },
+        expected: false,
+    },
+    {
+        name: 'should deny access if principal doesn\'t match non-',
+        bucketId: canonicalIdToVet,
+        objectId: canonicalIdToVet,
+        keyToChange: 'Principal',
+        bucketValue: { CanonicalUser: [bucketOwnerCanonicalId] },
+        objectValue: { CanonicalUser: [objectOwnerCanonicalId] },
+        expected: false,
+    },
+    {
+        name: 'should allow access if principal and action match policy for ' +
+            'non-',
+        bucketId: canonicalIdToVet,
+        objectId: canonicalIdToVet,
+        keyToChange: 'Action',
+        bucketValue: ['s3:ListBucket'],
+        objectValue: ['s3:PutObject'],
+        expected: true,
+    },
+    {
+        name: 'should deny access if principal matches but action does not ' +
+            'match policy for non-',
+        bucketId: canonicalIdToVet,
+        objectId: canonicalIdToVet,
+        keyToChange: 'Action',
+        bucketValue: ['s3:GetBucketLocation'],
+        objectValue: ['s3:GetObject'],
+        expected: false,
+    },
+    {
+        name: 'should allow access even if bucket policy denies for ',
+        bucketId: bucketOwnerCanonicalId,
+        objectId: objectOwnerCanonicalId,
+        keyToChange: 'Effect',
+        bucketValue: 'Deny',
+        objectValue: 'Deny',
+        expected: true,
+    },
+];
+
+const resourceTests = [
+    {
+        name: 'true if policy resource matches bucket arn',
+        rValue: `arn:aws:s3:::${bucketName}`,
+        expected: true,
+    },
+    {
+        name: 'true if policy resource matches obj in bucket',
+        rValue: `arn:aws:s3:::${bucketName}/*`,
+        expected: true,
+    },
+    {
+        name: 'false if policy resource is bucket name',
+        rValue: bucketName,
+        expected: false,
+    },
+    {
+        name: 'false if policy resource does not match bucket arn',
+        rValue: 'arn:aws:s3:::nomatch',
+        expected: false,
+    },
+    {
+        name: 'false if policy resource is array and any elements do not ' +
+            'match bucket arn',
+        rValue: [`arn:aws:s3:::${bucketName}`, 'arn:aws:s3:::nomatch'],
+        expected: false,
+    },
+];
+
+describe('bucket policy authorization', () => {
+    describe('isBucketAuthorized with no policy set', () => {
+        it('should allow access to bucket owner', done => {
+            const allowed = isBucketAuthorized(bucket, 'bucketPut',
+                bucketOwnerCanonicalId, null, log);
+            assert.equal(allowed, true);
+            done();
+        });
+
+        it('should deny access to non-bucket owner',
+        done => {
+            const allowed = isBucketAuthorized(bucket, 'bucketPut',
+                canonicalIdToVet, null, log);
+            assert.equal(allowed, false);
+            done();
+        });
+    });
+
+    describe('isBucketAuthorized with bucket policy set', () => {
+        beforeEach(function beFn() {
+            this.currentTest.basePolicy = new BucketPolicy(JSON.stringify(
+                basePolicyObj)).getBucketPolicy();
+            bucket.setBucketPolicy(this.currentTest.basePolicy);
+        });
+
+        it('should allow access to non-bucket owner if principal is set to "*"',
+        done => {
+            const allowed = isBucketAuthorized(bucket, bucAction,
+                canonicalIdToVet, null, log);
+            assert.equal(allowed, true);
+            done();
+        });
+
+        it('should allow access to public user if principal is set to "*"',
+        done => {
+            const allowed = isBucketAuthorized(bucket, bucAction,
+                constants.publicId, null, log);
+            assert.equal(allowed, true);
+            done();
+        });
+
+        authTests.forEach(t => {
+            it(`${t.name}bucket owner`, function itFn(done) {
+                const newPolicy = this.test.basePolicy;
+                newPolicy.Statement[0][t.keyToChange] = t.bucketValue;
+                bucket.setBucketPolicy(newPolicy);
+                const allowed = isBucketAuthorized(bucket, bucAction,
+                    t.bucketId, t.bucketId, log);
+                assert.equal(allowed, t.expected);
+                done();
+            });
+        });
+
+        it('should deny access to non-bucket owner if two statements apply ' +
+        'to principal but one denies access', function itFn(done) {
+            const newPolicy = this.test.basePolicy;
+            newPolicy.Statement[1] = {
+                Effect: 'Deny',
+                Principal: { CanonicalUser: [canonicalIdToVet] },
+                Resource: `arn:aws:s3:::${bucket.getName()}`,
+                Action: 's3:*',
+            };
+            bucket.setBucketPolicy(newPolicy);
+            const allowed = isBucketAuthorized(bucket, bucAction,
+                canonicalIdToVet, null, log);
+            assert.equal(allowed, false);
+            done();
+        });
+    });
+
+    describe('isObjAuthorized with no policy set', () => {
+        before(() => {
+            bucket.setBucketPolicy(null);
+        });
+
+        it('should allow access to object owner', done => {
+            const allowed = isObjAuthorized(bucket, object, objAction,
+                objectOwnerCanonicalId, null, log);
+            assert.equal(allowed, true);
+            done();
+        });
+
+        it('should deny access to non-object owner',
+        done => {
+            const allowed = isObjAuthorized(bucket, object, objAction,
+                canonicalIdToVet, null, log);
+            assert.equal(allowed, false);
+            done();
+        });
+    });
+
+    describe('isObjAuthorized with bucket policy set', () => {
+        beforeEach(function beFn() {
+            const newPolicyObj = basePolicyObj;
+            newPolicyObj.Statement.Resource =
+                `arn:aws:s3:::${bucket.getName()}/*`;
+            this.currentTest.basePolicy = new BucketPolicy(JSON.stringify(
+                newPolicyObj)).getBucketPolicy();
+            bucket.setBucketPolicy(this.currentTest.basePolicy);
+        });
+
+        it('should allow access to non-object owner if principal is set to "*"',
+        done => {
+            const allowed = isObjAuthorized(bucket, object, objAction,
+                canonicalIdToVet, null, log);
+            assert.equal(allowed, true);
+            done();
+        });
+
+        it('should allow access to public user if principal is set to "*"',
+        done => {
+            const allowed = isObjAuthorized(bucket, object, objAction,
+                constants.publicId, null, log);
+            assert.equal(allowed, true);
+            done();
+        });
+
+        authTests.forEach(t => {
+            it(`${t.name}object owner`, function itFn(done) {
+                const newPolicy = this.test.basePolicy;
+                newPolicy.Statement[0][t.keyToChange] = t.objectValue;
+                bucket.setBucketPolicy(newPolicy);
+                const allowed = isObjAuthorized(bucket, object, objAction,
+                    t.objectId, t.objectId, log);
+                assert.equal(allowed, t.expected);
+                done();
+            });
+        });
+
+        it('should deny access to non-object owner if two statements apply ' +
+        'to principal but one denies access', function itFn(done) {
+            const newPolicy = this.test.basePolicy;
+            newPolicy.Statement[1] = {
+                Effect: 'Deny',
+                Principal: { CanonicalUser: [canonicalIdToVet] },
+                Resource: `arn:aws:s3:::${bucket.getName()}/*`,
+                Action: 's3:*',
+            };
+            bucket.setBucketPolicy(newPolicy);
+            const allowed = isObjAuthorized(bucket, object, objAction,
+                canonicalIdToVet, null, log);
+            assert.equal(allowed, false);
+            done();
+        });
+    });
+
+    describe('validate policy resource', () => {
+        resourceTests.forEach(t => {
+            it(`should return ${t.name}`, done => {
+                const newPolicy = basePolicyObj;
+                newPolicy.Statement.Resource = t.rValue;
+                newPolicy.Statement = [newPolicy.Statement];
+                assert.equal(
+                    validatePolicyResource(bucketName, newPolicy), t.expected);
+                done();
+            });
+        });
+
+        it('should return false if any statement resource does not match ' +
+        'bucket arn', done => {
+            const newPolicy = basePolicyObj;
+            newPolicy.Statement = [newPolicy.Statement];
+            newPolicy.Statement[1] = basePolicyObj.Statement;
+            newPolicy.Statement[0].Resource = `arn:aws:s3:::${bucketName}`;
+            assert.equal(validatePolicyResource(bucketName, newPolicy), false);
+            done();
+        });
+    });
+});

tests/unit/api/bucketPutPolicy.js

@@ -18,36 +18,35 @@ const testBucketPutRequest = {
 };
 
 const expectedBucketPolicy = {
-    version: '2012-10-17',
-    statements: [
+    Version: '2012-10-17',
+    Statement: [
         {
-            sid: '',
-            effect: 'Allow',
-            resource: 'arn:aws:s3::bucketname',
-            principal: '*',
-            actions: ['s3:sampleAction'],
+            Effect: 'Allow',
+            Resource: `arn:aws:s3:::${bucketName}`,
+            Principal: '*',
+            Action: ['s3:GetBucketLocation'],
         },
     ],
 };
 
-const testPutPolicyRequest = {
+function getPolicyRequest(policy) {
+    return {
         bucketName,
         headers: {
             host: `${bucketName}.s3.amazonaws.com`,
         },
-    post: JSON.stringify(expectedBucketPolicy),
-};
+        post: JSON.stringify(policy),
+    };
+}
 
-const describeSkipUntilImpl =
-    process.env.BUCKET_POLICY ? describe : describe.skip;
-
-describeSkipUntilImpl('putBucketPolicy API', () => {
+describe('putBucketPolicy API', () => {
     before(() => cleanup());
     beforeEach(done => bucketPut(authInfo, testBucketPutRequest, log, done));
     afterEach(() => cleanup());
 
     it('should update a bucket\'s metadata with bucket policy obj', done => {
-        bucketPutPolicy(authInfo, testPutPolicyRequest, log, err => {
+        bucketPutPolicy(authInfo, getPolicyRequest(expectedBucketPolicy),
+        log, err => {
             if (err) {
                 process.stdout.write(`Err putting bucket policy ${err}`);
                 return done(err);
@@ -63,4 +62,15 @@ describeSkipUntilImpl('putBucketPolicy API', () => {
             });
         });
     });
+
+    it('should return error if policy resource does not include bucket name',
+    done => {
+        expectedBucketPolicy.Statement[0].Resource = 'arn:aws::s3:::badname';
+        bucketPutPolicy(authInfo, getPolicyRequest(expectedBucketPolicy),
+        log, err => {
+            assert.strictEqual(err.MalformedPolicy, true);
+            assert.strictEqual(err.description, 'Policy has invalid resource');
+            return done();
+        });
+    });
 });

tests/unit/api/objectACLauth.js

@@ -3,7 +3,8 @@ const assert = require('assert');
 const BucketInfo = require('arsenal').models.BucketInfo;
 const constants = require('../../../constants');
 const { isObjAuthorized }
-    = require('../../../lib/api/apiUtils/authorization/aclChecks');
+    = require('../../../lib/api/apiUtils/authorization/permissionChecks');
+const { DummyRequestLogger } = require('../helpers');
 
 const bucketOwnerCanonicalId = 'bucketOwnerCanonicalId';
 const creationDate = new Date().toJSON();
@@ -21,6 +22,7 @@ const object = {
         READ_ACP: [],
     },
 };
+const log = new DummyRequestLogger();
 
 describe('object acl authorization for objectGet and objectHead', () => {
     // Reset the object ACLs
@@ -38,21 +40,22 @@ describe('object acl authorization for objectGet and objectHead', () => {
 
     it('should allow access to object owner', () => {
         const results = requestTypes.map(type =>
-                isObjAuthorized(bucket, object, type, objectOwnerCanonicalId));
+                isObjAuthorized(bucket, object, type, objectOwnerCanonicalId,
+                    null, log));
         assert.deepStrictEqual(results, [true, true]);
     });
 
     it('should allow access to anyone if canned public-read ACL', () => {
         object.acl.Canned = 'public-read';
         const results = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(results, [true, true]);
     });
 
     it('should allow access to anyone if canned public-read-write ACL', () => {
         object.acl.Canned = 'public-read-write';
         const results = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(results, [true, true]);
     });
 
@@ -60,7 +63,7 @@ describe('object acl authorization for objectGet and objectHead', () => {
         'authenticated-read ACL', () => {
         object.acl.Canned = 'authenticated-read';
         const publicResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, constants.publicId));
+            isObjAuthorized(bucket, object, type, constants.publicId, null, log));
         assert.deepStrictEqual(publicResults, [false, false]);
     });
 
@@ -68,63 +71,67 @@ describe('object acl authorization for objectGet and objectHead', () => {
         'authenticated-read ACL', () => {
         object.acl.Canned = 'authenticated-read';
         const results = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(results, [true, true]);
     });
 
     it('should allow access to bucker owner if ' +
         'bucket-owner-read ACL', () => {
         const noAuthResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId));
+            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId, null,
+                log));
         assert.deepStrictEqual(noAuthResults, [false, false]);
         object.acl.Canned = 'bucket-owner-read';
         const authResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId));
+            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId, null,
+                log));
         assert.deepStrictEqual(authResults, [true, true]);
     });
 
     it('should allow access to bucker owner if ' +
         'bucket-owner-full-control ACL', () => {
         const noAuthResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId));
+            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId, null,
+                log));
         assert.deepStrictEqual(noAuthResults, [false, false]);
         object.acl.Canned = 'bucket-owner-full-control';
         const authResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId));
+            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId, null,
+                log));
         assert.deepStrictEqual(authResults, [true, true]);
     });
 
     it('should allow access to account if ' +
         'account was granted FULL_CONTROL', () => {
         const noAuthResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(noAuthResults, [false, false]);
         object.acl.FULL_CONTROL = [accountToVet];
         const authResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(authResults, [true, true]);
     });
 
     it('should allow access to account if ' +
         'account was granted READ right', () => {
         const noAuthResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(noAuthResults, [false, false]);
         object.acl.READ = [accountToVet];
         const authResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(authResults, [true, true]);
     });
 
     it('should not allow access to public user if private canned ACL', () => {
         const results = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(results, [false, false]);
     });
 
     it('should not allow access to just any user if private canned ACL', () => {
         const results = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(results, [false, false]);
     });
 });
@@ -134,10 +141,11 @@ describe('object authorization for objectPut and objectDelete', () => {
         'are done at bucket level', () => {
         const requestTypes = ['objectPut', 'objectDelete'];
         const results = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(results, [true, true]);
         const publicUserResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, constants.publicId));
+            isObjAuthorized(bucket, object, type, constants.publicId, null,
+                log));
         assert.deepStrictEqual(publicUserResults, [true, true]);
     });
 });
@@ -159,51 +167,54 @@ describe('object authorization for objectPutACL and objectGetACL', () => {
 
     it('should allow access to object owner', () => {
         const results = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, objectOwnerCanonicalId));
+            isObjAuthorized(bucket, object, type, objectOwnerCanonicalId,
+                null, log));
         assert.deepStrictEqual(results, [true, true]);
     });
 
     it('should allow access to bucket owner if ' +
         'bucket-owner-full-control canned ACL set on object', () => {
         const noAuthResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId));
+            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId, null,
+                log));
         assert.deepStrictEqual(noAuthResults, [false, false]);
         object.acl.Canned = 'bucket-owner-full-control';
         const authorizedResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId));
+            isObjAuthorized(bucket, object, type, bucketOwnerCanonicalId,
+                null, log));
         assert.deepStrictEqual(authorizedResults, [true, true]);
     });
 
     it('should allow access to account if ' +
         'account was granted FULL_CONTROL right', () => {
         const noAuthResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(noAuthResults, [false, false]);
         object.acl.FULL_CONTROL = [accountToVet];
         const authorizedResults = requestTypes.map(type =>
-            isObjAuthorized(bucket, object, type, accountToVet));
+            isObjAuthorized(bucket, object, type, accountToVet, null, log));
         assert.deepStrictEqual(authorizedResults, [true, true]);
     });
 
     it('should allow objectPutACL access to account if ' +
         'account was granted WRITE_ACP right', () => {
         const noAuthResult = isObjAuthorized(bucket, object, 'objectPutACL',
-            accountToVet);
+            accountToVet, null, log);
         assert.strictEqual(noAuthResult, false);
         object.acl.WRITE_ACP = [accountToVet];
         const authorizedResult = isObjAuthorized(bucket, object, 'objectPutACL',
-            accountToVet);
+            accountToVet, null, log);
         assert.strictEqual(authorizedResult, true);
     });
 
     it('should allow objectGetACL access to account if ' +
         'account was granted READ_ACP right', () => {
         const noAuthResult = isObjAuthorized(bucket, object, 'objectGetACL',
-            accountToVet);
+            accountToVet, null, log);
         assert.strictEqual(noAuthResult, false);
         object.acl.READ_ACP = [accountToVet];
         const authorizedResult = isObjAuthorized(bucket, object, 'objectGetACL',
-            accountToVet);
+            accountToVet, null, log);
         assert.strictEqual(authorizedResult, true);
     });
 });