Skip to content

Commit 48ffb40

Browse files
aspandeyaspandey
authored
Merge pull request #8776 from aspandey/access_object_version
noobaa-core: Bucket Policy Condition to access Specific VersionID
2 parents 0669471 + f7e2110 commit 48ffb40

File tree

4 files changed

+180
-19
lines changed

4 files changed

+180
-19
lines changed

‎src/endpoint/s3/s3_bucket_policy_utils.js

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,15 @@ const predicate_map = {
110110

111111
const condition_fit_functions = {
112112
's3:ExistingObjectTag': _is_object_tag_fit,
113-
's3:x-amz-server-side-encryption': _is_server_side_encryption_fit
113+
's3:x-amz-server-side-encryption': _is_server_side_encryption_fit,
114+
's3:VersionId': _is_object_version_fit
114115
};
115116

117+
//https://docs.aws.amazon.com/service-authorization/latest/reference/list_amazons3.html#amazons3-policy-keys
116118
const supported_actions = {
117119
's3:ExistingObjectTag': ['s3:DeleteObjectTagging', 's3:DeleteObjectVersionTagging', 's3:GetObject', 's3:GetObjectAcl', 's3:GetObjectTagging', 's3:GetObjectVersion', 's3:GetObjectVersionTagging', 's3:PutObjectAcl', 's3:PutObjectTagging', 's3:PutObjectVersionTagging'],
118-
's3:x-amz-server-side-encryption': ['s3:PutObject']
120+
's3:x-amz-server-side-encryption': ['s3:PutObject'],
121+
's3:VersionId': ['s3:GetObjectVersion', 's3:DeleteObjectVersion', 's3:GetObjectVersionAttributes', 's3:GetObjectVersionTagging', 's3:PutObjectVersionTagging', 's3:DeleteObjectVersionTagging']
119122
};
120123

121124
const SUPPORTED_BUCKET_POLICY_CONDITIONS = Object.keys(supported_actions);
@@ -136,6 +139,12 @@ async function _is_object_tag_fit(req, predicate, value) {
136139
dbg.log1('bucket_policy: object tag fit?', value, tag, res);
137140
return res;
138141
}
142+
async function _is_object_version_fit(req, predicate, value) {
143+
const version_id = req.query.versionId;
144+
const res = predicate(version_id, value);
145+
dbg.log1('bucket_policy: version-id fit? version-id, policy version-id, match :', version_id, value, res);
146+
return res;
147+
}
139148

140149
async function has_bucket_policy_permission(policy, account, method, arn_path, req) {
141150
const [allow_statements, deny_statements] = _.partition(policy.Statement, statement => statement.Effect === 'Allow');
@@ -210,7 +219,7 @@ async function _is_statements_fit(statements, account, method, arn_path, req) {
210219
const resource_fit = _is_resource_fit(arn_path, statement);
211220
const condition_fit = await _is_condition_fit(statement, req, method);
212221

213-
dbg.log1('bucket_policy: is_statements_fit', action_fit, principal_fit, resource_fit, condition_fit);
222+
dbg.log1('bucket_policy - is_statements_fit: action_fit, principal_fit, resource_fit, condition_fit', action_fit, principal_fit, resource_fit, condition_fit);
214223
if (action_fit && principal_fit && resource_fit && condition_fit) return true;
215224
}
216225
return false;

‎src/server/common_services/auth_server.js

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -595,16 +595,16 @@ function _prepare_auth_request(req) {
595595
dbg.log3('load auth system:', req.system && req.system._id);
596596
};
597597

598-
req.has_bucket_anonymous_permission = function(bucket, action, bucket_path) {
599-
return has_bucket_anonymous_permission(bucket, action, bucket_path);
598+
req.has_bucket_anonymous_permission = function(bucket, action, bucket_path, req_query) {
599+
return has_bucket_anonymous_permission(bucket, action, bucket_path, req_query);
600600
};
601601

602-
req.has_s3_bucket_permission = async function(bucket, action, bucket_path) {
602+
req.has_s3_bucket_permission = async function(bucket, action, bucket_path, req_query) {
603603
// Since this method can be called both authorized and unauthorized
604604
// We need to check the anonymous permission only when the bucket is configured to server anonymous requests
605605
// In case of anonymous function but with authentication flow we roll back to previous code and not return here
606606
if (req.auth_token && typeof req.auth_token === 'object') {
607-
return req.has_bucket_action_permission(bucket, action, bucket_path);
607+
return req.has_bucket_action_permission(bucket, action, bucket_path, req_query);
608608
}
609609
// If we came with a NooBaa management token then we've already checked the method permissions prior to this function
610610
// There is nothing specific to bucket permissions for the management credentials
@@ -614,20 +614,20 @@ function _prepare_auth_request(req) {
614614
}
615615

616616
if (options.anonymous === true) {
617-
return req.has_bucket_anonymous_permission(bucket, action, bucket_path);
617+
return req.has_bucket_anonymous_permission(bucket, action, bucket_path, req_query);
618618
}
619619

620620
return false;
621621
};
622622

623623
req.check_bucket_action_permission = async function(bucket, action, bucket_path) {
624-
if (!await has_bucket_action_permission(bucket, req.account, action, bucket_path)) {
624+
if (!await has_bucket_action_permission(bucket, req.account, action, undefined, bucket_path)) {
625625
throw new RpcError('UNAUTHORIZED', 'No permission to access bucket');
626626
}
627627
};
628628

629-
req.has_bucket_action_permission = async function(bucket, action, bucket_path) {
630-
return has_bucket_action_permission(bucket, req.account, action, bucket_path);
629+
req.has_bucket_action_permission = async function(bucket, action, bucket_path, req_query) {
630+
return has_bucket_action_permission(bucket, req.account, action, req_query, bucket_path);
631631
};
632632
}
633633

@@ -669,9 +669,8 @@ function _get_auth_info(account, system, authorized_by, role, extra) {
669669
* @param {string} bucket_path s3 bucket path (must start from "/")
670670
* @returns {Promise<boolean>} true if the account has permission to perform the action on the bucket
671671
*/
672-
async function has_bucket_action_permission(bucket, account, action, bucket_path = "") {
672+
async function has_bucket_action_permission(bucket, account, action, req_query, bucket_path = "") {
673673
dbg.log1('has_bucket_action_permission:', bucket.name, account.email, bucket.owner_account.email);
674-
675674
// If the system owner account wants to access the bucket, allow it
676675
if (bucket.system.owner.email.unwrap() === account.email.unwrap()) return true;
677676

@@ -689,7 +688,7 @@ async function has_bucket_action_permission(bucket, account, action, bucket_path
689688
account.email.unwrap(),
690689
action,
691690
`arn:aws:s3:::${bucket.name.unwrap()}${bucket_path}`,
692-
undefined
691+
req_query
693692
);
694693

695694
if (result === 'DENY') return false;
@@ -704,7 +703,8 @@ async function has_bucket_action_permission(bucket, account, action, bucket_path
704703
* @param {string} bucket_path bucket path
705704
* @returns {Promise<boolean>} true if the bucket is configured to serve anonymous requests
706705
*/
707-
async function has_bucket_anonymous_permission(bucket, action, bucket_path = "") {
706+
async function has_bucket_anonymous_permission(bucket, action, bucket_path, req_query) {
707+
bucket_path = bucket_path ?? "";
708708
const bucket_policy = bucket.s3_policy;
709709
if (!bucket_policy) return false;
710710
return await s3_bucket_policy_utils.has_bucket_policy_permission(
@@ -713,7 +713,7 @@ async function has_bucket_anonymous_permission(bucket, action, bucket_path = "")
713713
undefined,
714714
action || `s3:GetObject`,
715715
`arn:aws:s3:::${bucket.name.unwrap()}${bucket_path}`,
716-
undefined
716+
req_query
717717
) === 'ALLOW';
718718
}
719719

‎src/server/object_services/object_server.js

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ async function read_object_mapping(req) {
698698
const obj = await find_object_md(req);
699699

700700
// Check if the requesting account is authorized to read the object
701-
if (!await req.has_s3_bucket_permission(req.bucket, 's3:GetObject', '/' + obj.key)) {
701+
if (!await req.has_s3_bucket_permission(req.bucket, 's3:GetObject', '/' + obj.key, undefined)) {
702702
throw new RpcError('UNAUTHORIZED', 'requesting account is not authorized to read the object');
703703
}
704704

@@ -770,6 +770,26 @@ async function read_node_mapping(req) {
770770
};
771771
}
772772

773+
/**
774+
* Converts RPC parameters to a bucket policy request query.
775+
* Extracts the version ID (if present) to construct a query object.
776+
* @param {object} rpc_params - The RPC parameters object.
777+
* @returns {object|null} A query object or null.
778+
*/
779+
780+
function _convert_rpc_params_to_bucket_policy_req(rpc_params) {
781+
782+
let req_query = null;
783+
if (rpc_params && rpc_params.version_id) {
784+
req_query = {
785+
query: {
786+
versionId: rpc_params.version_id
787+
}
788+
};
789+
}
790+
return req_query;
791+
}
792+
773793
/**
774794
*
775795
* READ_OBJECT_MD
@@ -783,11 +803,13 @@ async function read_object_md(req) {
783803
throw new RpcError('UNAUTHORIZED', 'read_object_md: role should be admin');
784804
}
785805

806+
const req_query = _convert_rpc_params_to_bucket_policy_req(req.rpc_params);
786807
const obj = await find_object_md(req);
787808

788809
// Check if the requesting account is authorized to read the object
789810
const action = version_id ? 's3:GetObjectVersion' : 's3:GetObject';
790-
if (!await req.has_s3_bucket_permission(req.bucket, action, '/' + obj.key)) {
811+
812+
if (!await req.has_s3_bucket_permission(req.bucket, action, '/' + obj.key, req_query)) {
791813
throw new RpcError('UNAUTHORIZED', 'requesting account is not authorized to read the object');
792814
}
793815

‎src/test/unit_tests/test_s3_bucket_policy.js

Lines changed: 131 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/* Copyright (C) 2016 NooBaa */
22
/* eslint max-lines-per-function: ['error', 650] */
3+
/* eslint max-lines: ["error", 2500] */
34
'use strict';
45

56
// setup coretest first to prepare the env
@@ -35,6 +36,8 @@ const BKT = 'test2-bucket-policy-ops';
3536
const BKT_B = 'test2-bucket-policy-ops-1';
3637
const BKT_C = 'test2-bucket-policy-ops-2';
3738
const BKT_D = 'test2-bucket-policy-ops-3';
39+
const VER_BKT = 'test-object-ver-policy-ops';
40+
3841
const KEY = 'file1.txt';
3942
const user_a = 'alice';
4043
const user_b = 'bob';
@@ -137,6 +140,8 @@ async function setup() {
137140
await s3_owner.createBucket({ Bucket: BKT });
138141
await s3_owner.createBucket({ Bucket: BKT_C });
139142
await s3_owner.createBucket({ Bucket: BKT_D });
143+
await s3_owner.createBucket({ Bucket: VER_BKT });
144+
140145
s3_anon = new S3({
141146
...s3_creds,
142147
credentials: {
@@ -150,7 +155,7 @@ async function setup() {
150155
});
151156
}
152157

153-
/*eslint max-lines-per-function: ["error", 2000]*/
158+
/*eslint max-lines-per-function: ["error", 3000]*/
154159
mocha.describe('s3_bucket_policy', function() {
155160
mocha.before(setup);
156161
mocha.it('should fail setting bucket policy when user doesn\'t exist', async function() {
@@ -1963,4 +1968,129 @@ mocha.describe('s3_bucket_policy', function() {
19631968
assert.strictEqual(res.PolicyStatus.IsPublic, true);
19641969
});
19651970
});
1971+
mocha.describe('s3_bucket_policy: Granting access to a specific version of an object ', function() {
1972+
mocha.it('should be able to put access permission based on VersionId', async function() {
1973+
const self = this; // eslint-disable-line no-invalid-this
1974+
self.timeout(15000);
1975+
const object_key = 'allowed_file_1.txt';
1976+
1977+
await s3_owner.putBucketVersioning({
1978+
Bucket: VER_BKT,
1979+
VersioningConfiguration: {
1980+
MFADelete: 'Disabled',
1981+
Status: 'Enabled'
1982+
}
1983+
});
1984+
1985+
// Create an object and upload different copies of same object
1986+
const first_res = await s3_owner.putObject({
1987+
Body: 'Some data for the file... bla bla bla... version I',
1988+
Bucket: VER_BKT,
1989+
Key: object_key
1990+
});
1991+
1992+
await s3_owner.putObject({
1993+
Body: 'Some data for the file... bla bla bla bla... version II',
1994+
Bucket: VER_BKT,
1995+
Key: object_key
1996+
});
1997+
1998+
const third_res = await s3_owner.putObject({
1999+
Body: 'Some data for the file... bla bla bla bla... version III',
2000+
Bucket: VER_BKT,
2001+
Key: object_key
2002+
});
2003+
2004+
const version_policy = {
2005+
Version: '2012-10-17',
2006+
Statement: [
2007+
{
2008+
Sid: 'id-1',
2009+
Effect: 'Allow',
2010+
Principal: { AWS: user_b },
2011+
Action: ['s3:*'],
2012+
Resource: [`arn:aws:s3:::${VER_BKT}/${object_key}`]
2013+
},
2014+
{
2015+
Sid: 'id-2',
2016+
Effect: 'Allow',
2017+
Principal: { AWS: user_a },
2018+
Action: ['s3:*'],
2019+
Resource: [`arn:aws:s3:::${VER_BKT}/${object_key}`]
2020+
},
2021+
{
2022+
Sid: 'id-3',
2023+
Effect: 'Deny',
2024+
Principal: { AWS: user_a },
2025+
Action: ['s3:GetObjectVersion'],
2026+
Resource: [`arn:aws:s3:::${VER_BKT}/${object_key}`],
2027+
Condition: {
2028+
StringNotEquals: {
2029+
's3:VersionId': first_res.VersionId // Use one of the VersionId to set policy
2030+
}
2031+
}
2032+
},
2033+
{
2034+
Sid: 'id-4',
2035+
Effect: 'Deny',
2036+
Principal: { AWS: user_a },
2037+
Action: ['s3:DeleteObjectVersion'],
2038+
Resource: [`arn:aws:s3:::${VER_BKT}/${object_key}`],
2039+
Condition: {
2040+
StringNotEquals: {
2041+
's3:VersionId': first_res.VersionId // Use one of the VersionId to set policy
2042+
}
2043+
}
2044+
}
2045+
]
2046+
};
2047+
// Put new policy to allow user_b full access while user_a has access on one VersionId
2048+
const res_put_bucket_policy = await s3_owner.putBucketPolicy({
2049+
Bucket: VER_BKT,
2050+
Policy: JSON.stringify(version_policy)
2051+
});
2052+
assert.equal(res_put_bucket_policy.$metadata.httpStatusCode, 200);
2053+
2054+
const res_get_bucket_policy = await s3_owner.getBucketPolicy({
2055+
Bucket: VER_BKT,
2056+
});
2057+
assert.equal(res_get_bucket_policy.$metadata.httpStatusCode, 200);
2058+
2059+
// Access should be Allowed
2060+
const res1 = await s3_a.getObject({
2061+
Bucket: VER_BKT,
2062+
Key: object_key,
2063+
VersionId: first_res.VersionId
2064+
});
2065+
assert.equal(res1.$metadata.httpStatusCode, 200);
2066+
2067+
// Access should be Denied
2068+
await assert_throws_async(s3_a.getObject({
2069+
Bucket: VER_BKT,
2070+
Key: object_key,
2071+
VersionId: third_res.VersionId
2072+
}));
2073+
2074+
// All Access should be allowed : For user_b
2075+
const res2 = await s3_b.getObject({
2076+
Bucket: VER_BKT,
2077+
Key: object_key,
2078+
VersionId: first_res.VersionId
2079+
});
2080+
assert.equal(res2.$metadata.httpStatusCode, 200);
2081+
2082+
const res4 = await s3_b.getObject({
2083+
Bucket: VER_BKT,
2084+
Key: object_key,
2085+
});
2086+
assert.equal(res4.$metadata.httpStatusCode, 200);
2087+
2088+
// DELETE should be Allowed : First Version
2089+
await s3_a.deleteObject({
2090+
Bucket: VER_BKT,
2091+
Key: object_key,
2092+
VersionId: first_res.VersionId
2093+
});
2094+
});
2095+
});
19662096
});

0 commit comments

Comments
 (0)