Skip to content

Commit 2bf5725

Browse files
author
Kerkesni
committed
always evaluate policies in backbeat routes
Issue: CLDSRV-731
1 parent 4d7d1b3 commit 2bf5725

File tree

2 files changed

+56
-31
lines changed

2 files changed

+56
-31
lines changed

lib/routes/routeBackbeat.js

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,17 +1596,8 @@ function routeBackbeat(clientIP, request, response, log) {
15961596
request.accountQuotas = infos?.accountQuota;
15971597
return next(err, userInfo, authorizationResults);
15981598
}, 's3', requestContexts),
1599-
(userInfo, authorizationResults, next) => {
1600-
// Using the same flag used to bypass user bucket policies
1601-
// for internal mode, so that we can bypass policy evaluation
1602-
// on backbeat routes. This ensures we don't break operations
1603-
// coming from internal services like backbeat
1604-
if (request.bypassUserBucketPolicies) {
1605-
return next(null, userInfo);
1606-
}
1607-
return handleAuthorizationResults(
1608-
request, authorizationResults, apiMethods[0], undefined, log, err => next(err, userInfo));
1609-
},
1599+
(userInfo, authorizationResults, next) => handleAuthorizationResults(
1600+
request, authorizationResults, apiMethods[0], undefined, log, err => next(err, userInfo)),
16101601
(userInfo, next) => {
16111602
// TODO: understand why non-object requests (batchdelete) were not authenticated
16121603
if (!isObjectRequest) {

tests/unit/routes/routeBackbeat.js

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const bucketDelete = require('../../../lib/api/bucketDelete');
1818
const bucketPutVersioning = require('../../../lib/api/bucketPutVersioning');
1919
const objectPut = require('../../../lib/api/objectPut');
2020
const { objectDelete } = require('../../../lib/api/objectDelete');
21+
const bucketPutPolicy = require('../../../lib/api/bucketPutPolicy');
2122

2223
const log = new DummyRequestLogger();
2324

@@ -707,9 +708,10 @@ describe('routeBackbeat authorization', () => {
707708
let endPromise;
708709
let resolveEnd;
709710
const bucketName = 'bucketname';
710-
const authInfo = makeAuthInfo('cannonicalID',);
711+
const authInfo = makeAuthInfo('cannonicalID');
711712
const namespace = 'default';
712713
const objectName = 'objectName';
714+
const bucketPutPolicyPromise = promisify(bucketPutPolicy);
713715

714716
const testBucket = {
715717
bucketName,
@@ -801,6 +803,7 @@ describe('routeBackbeat authorization', () => {
801803
target: `${bucketName}/${objectName}`,
802804
operation: null,
803805
versionId: true,
806+
expect: { code: 200 },
804807
},
805808
{
806809
method: 'PUT',
@@ -1013,26 +1016,57 @@ describe('routeBackbeat authorization', () => {
10131016
assert.strictEqual(err.code, 'AccessDenied');
10141017
});
10151018

1016-
it('should bypass policy evaluation', async () => {
1017-
sinon.stub(auth.server, 'doAuth').yields(null, new AuthInfo({
1018-
canonicalID: 'abcdef/lifecycle',
1019-
accountDisplayName: 'Lifecycle Service Account',
1020-
}), [{
1021-
isAllowed: false,
1022-
implicitDeny: true,
1023-
action: 'objectReplicate',
1024-
}], undefined, undefined);
1025-
1026-
request.bypassUserBucketPolicies = true;
1027-
1028-
routeBackbeat('127.0.0.1', request, response, log);
1029-
1030-
void await endPromise;
1031-
1032-
if (testCase.expect) {
1033-
const errCode = response.writeHead.getCall(0).args[0];
1034-
assert.strictEqual(errCode, testCase.expect.code);
1019+
[true, false].forEach(bypass => {
1020+
// Bucket policies only affect APIs that call standardMetadataValidateBucketAndObj
1021+
if (testCase.resourceType !== 'metadata' && testCase.resourceType !== 'data') {
1022+
return;
10351023
}
1024+
it(`should ${bypass ? '' : 'not '}bypass bucket policy evaluation`, async () => {
1025+
const policyRequest = {
1026+
bucketName,
1027+
headers: {
1028+
host: `${bucketName}.s3.amazonaws.com`,
1029+
},
1030+
post: JSON.stringify({
1031+
Version: '2012-10-17',
1032+
Statement: [{
1033+
Effect: 'Deny',
1034+
Principal: '*',
1035+
Action: '*',
1036+
Resource: `arn:aws:s3:::${bucketName}/*`,
1037+
}],
1038+
}),
1039+
actionImplicitDenies: false,
1040+
};
1041+
await bucketPutPolicyPromise(authInfo, policyRequest, log);
1042+
1043+
// simulate assume role session user
1044+
const sessionAuthInfo = new AuthInfo({
1045+
arn: 'arn:aws:sts::000000000000:assumed-role/session',
1046+
canonicalID: authInfo.getCanonicalID(),
1047+
accountDisplayName: authInfo.getAccountDisplayName(),
1048+
});
1049+
1050+
sinon.stub(auth.server, 'doAuth').yields(null, sessionAuthInfo, [{
1051+
isAllowed: true,
1052+
implicitDeny: false,
1053+
action: 'objectReplicate',
1054+
}], undefined, undefined);
1055+
1056+
request.bypassUserBucketPolicies = bypass;
1057+
1058+
routeBackbeat('127.0.0.1', request, response, log);
1059+
1060+
void await endPromise;
1061+
1062+
if (bypass) {
1063+
const errCode = response.writeHead.getCall(0).args[0];
1064+
assert.strictEqual(errCode, testCase.expect.code);
1065+
} else {
1066+
const err = JSON.parse(response.end.getCall(0).args[0]);
1067+
assert.strictEqual(err.code, 'AccessDenied');
1068+
}
1069+
});
10361070
});
10371071
});
10381072
});

0 commit comments

Comments
 (0)