Skip to content

Commit

Permalink
bugfix: CLDSRV-282 fix regression from CLDSRV-280
Browse files Browse the repository at this point in the history
* auth results can be an array of arrays
* Object.assign needs an empty object or it modifies the original
* should be `_isObjectLockEnabled` not `!_isObjectLockEnabled`
* add unit tests

(cherry picked from commit bbb3e2f)
  • Loading branch information
miniscruff authored and alexanderchan-scality committed Nov 24, 2022
1 parent f9e695b commit 0a432f2
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 25 deletions.
65 changes: 40 additions & 25 deletions lib/api/bucketPut.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,12 @@ function _handleAuthResults(locationConstraint, log, cb) {
if (err) {
return cb(err);
}
if (!authorizationResults.every(res => res.isAllowed)) {
if (!authorizationResults.every(res => {
if (Array.isArray(res)) {
return res.every(subRes => subRes.isAllowed);
}
return res.isAllowed;
})) {
log.trace(
'authorization check failed for user',
{ locationConstraint },
Expand All @@ -140,6 +145,36 @@ function _isObjectLockEnabled(headers) {
return header !== undefined && header.toLowerCase() === 'true';
}

function authBucketPut(authParams, bucketName, locationConstraint, request, authInfo) {
const ip = requestUtils.getClientIp(request, config);
const baseParams = {
authParams,
ip,
bucketName,
request,
authInfo,
locationConstraint,
};
const requestConstantParams = [Object.assign(
baseParams,
{ apiMethod: 'bucketPut' },
)];

if (_isObjectLockEnabled(request.headers)) {
requestConstantParams.push(Object.assign(
{},
baseParams,
{ apiMethod: 'bucketPutObjectLock' },
));
requestConstantParams.push(Object.assign(
{},
baseParams,
{ apiMethod: 'bucketPutVersioning' },
));
}
return requestConstantParams;
}

/**
* PUT Service - Create bucket for the user
* @param {AuthInfo} authInfo - Instance of AuthInfo class with requester's info
Expand Down Expand Up @@ -176,30 +211,9 @@ function bucketPut(authInfo, request, log, callback) {
}

const authParams = auth.server.extractParams(request, log, 's3', request.query);
const ip = requestUtils.getClientIp(request, config);
const baseParams = {
authParams,
ip,
bucketName,
request,
authInfo,
locationConstraint,
};
const requestConstantParams = [Object.assign(
baseParams,
{ apiMethod: 'bucketPut' },
)];

if (!_isObjectLockEnabled(request.headers)) {
requestConstantParams.push(Object.assign(
baseParams,
{ apiMethod: 'bucketPutObjectLock' },
));
requestConstantParams.push(Object.assign(
baseParams,
{ apiMethod: 'bucketPutVersioning' },
));
}
const requestConstantParams = authBucketPut(
authParams, bucketName, locationConstraint, request, authInfo
);

return vault.checkPolicies(
requestConstantParams.map(_buildConstantParams),
Expand Down Expand Up @@ -230,4 +244,5 @@ module.exports = {
checkLocationConstraint,
bucketPut,
_handleAuthResults,
authBucketPut,
};
22 changes: 22 additions & 0 deletions tests/unit/api/bucketPut.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,28 @@ describe('bucketPut API', () => {
],
calledWith: [null, constraint],
},
{
description: 'array of arrays allowed auth',
error: undefined,
results: [
{ isAllowed: true },
{ isAllowed: true },
[{ isAllowed: true }, { isAllowed: true }],
{ isAllowed: true },
],
calledWith: [null, constraint],
},
{
description: 'array of arrays not allowed auth',
error: undefined,
results: [
{ isAllowed: true },
{ isAllowed: true },
[{ isAllowed: true }, { isAllowed: false }],
{ isAllowed: true },
],
calledWith: [errors.AccessDenied],
},
{
description: 'single not allowed auth',
error: undefined,
Expand Down
39 changes: 39 additions & 0 deletions tests/unit/policies.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
const assert = require('assert');
const DummyRequest = require('./DummyRequest');
const { authBucketPut } = require('../../lib/api/bucketPut');

function prepareDummyRequest(headers = {}) {
const request = new DummyRequest({
hostname: 'localhost',
port: 80,
headers,
socket: {
remoteAddress: '0.0.0.0',
},
});
return request;
}

describe('Policies: permission checks for S3 APIs', () => {
describe('PutBucket', () => {
function putBucketApiMethods(headers) {
const request = prepareDummyRequest(headers);
const result = authBucketPut(null, 'name', null, request, null);
return result.map(req => req.apiMethod);
}

it('should return s3:PutBucket without any provided header', () => {
assert.deepStrictEqual(
putBucketApiMethods(),
['bucketPut'],
);
});

it('should return s3:PutBucket and s3:PutBucketObjectLockConfiguration with ACL headers', () => {
assert.deepStrictEqual(
putBucketApiMethods({ 'x-amz-bucket-object-lock-enabled': 'true' }),
['bucketPut', 'bucketPutObjectLock', 'bucketPutVersioning'],
);
});
});
});

0 comments on commit 0a432f2

Please sign in to comment.