Skip to content

Commit 4b54db3

Browse files
authored
Merge pull request #9291 from shirady/iam-no-bucket-policy
IAM | No Bucket Policy Authorization for IAM Users
2 parents 7e70923 + 640cdd3 commit 4b54db3

File tree

5 files changed

+22
-6
lines changed

5 files changed

+22
-6
lines changed

src/api/bucket_api.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,9 @@ module.exports = {
12071207
bucket_owner: {
12081208
$ref: 'common_api#/definitions/email'
12091209
},
1210+
bucket_owner_id: {
1211+
type: 'string'
1212+
},
12101213
website: {
12111214
$ref: 'common_api#/definitions/bucket_website'
12121215
},

src/endpoint/s3/s3_rest.js

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ async function authorize_request_policy(req) {
238238
s3_policy,
239239
system_owner,
240240
bucket_owner,
241+
bucket_owner_id,
241242
owner_account,
242243
public_access_block,
243244
} = await req.object_sdk.read_bucket_sdk_policy_info(req.params.bucket);
@@ -253,8 +254,9 @@ async function authorize_request_policy(req) {
253254
}
254255

255256
const account = req.object_sdk.requesting_account;
256-
const account_identifier_name = req.object_sdk.nsfs_config_root ? account.name.unwrap() : account.email.unwrap();
257-
const account_identifier_id = req.object_sdk.nsfs_config_root ? account._id : undefined;
257+
const is_nc_deployment = req.object_sdk.nsfs_config_root;
258+
const account_identifier_name = is_nc_deployment ? account.name.unwrap() : account.email.unwrap();
259+
const account_identifier_id = is_nc_deployment ? account._id : undefined;
258260
const account_identifier_arn = s3_bucket_policy_utils.get_bucket_policy_principal_arn(account);
259261

260262
// deny delete_bucket permissions from bucket_claim_owner accounts (accounts that were created by OBC from openshift\k8s)
@@ -286,8 +288,11 @@ async function authorize_request_policy(req) {
286288
if (!s3_policy) {
287289
// in case we do not have bucket policy
288290
// we allow IAM account to access a bucket that is owned by their root account
289-
const is_iam_account_and_same_root_account_owner = account.owner !== undefined &&
290-
owner_account && account.owner === owner_account.id;
291+
let is_iam_account_and_same_root_account_owner = false;
292+
if (account.owner !== undefined) {
293+
const owner_account_to_compare = is_nc_deployment ? (owner_account && owner_account.id) : bucket_owner_id;
294+
is_iam_account_and_same_root_account_owner = account.owner === owner_account_to_compare;
295+
}
291296
if (is_owner || is_iam_account_and_same_root_account_owner) return;
292297
throw new S3Error(S3Error.AccessDenied);
293298
}

src/sdk/object_sdk.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,8 @@ class ObjectSDK {
218218
s3_policy: bucket.s3_policy,
219219
system_owner: bucket.system_owner, // note that bucketspace_fs currently doesn't return system_owner
220220
bucket_owner: bucket.bucket_owner,
221-
owner_account: bucket.owner_account, // in NC NSFS this is the account id that owns the bucket
221+
bucket_owner_id: bucket.bucket_owner_id, // in containerized this is the account id that owns the bucket
222+
owner_account: bucket.owner_account, // in NC NSFS this is an object of account id and name that owns the bucket
222223
public_access_block: bucket.public_access_block,
223224
};
224225
return policy_info;

src/server/common_services/auth_server.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,13 @@ async function has_bucket_action_permission(bucket, account, action, req_query,
545545
(account.bucket_claim_owner && account.bucket_claim_owner.name.unwrap() === bucket.name.unwrap());
546546
const bucket_policy = bucket.s3_policy;
547547

548-
if (!bucket_policy) return is_owner;
548+
if (!bucket_policy) {
549+
// in case we do not have bucket policy
550+
// we allow IAM account to access a bucket that that is owned by their root account
551+
const is_iam_and_same_root_account_owner = account.owner !== undefined &&
552+
account.owner._id.toString() === bucket.owner_account._id.toString();
553+
return is_owner || is_iam_and_same_root_account_owner;
554+
}
549555
if (!action) {
550556
throw new Error('has_bucket_action_permission: action is required');
551557
}

src/server/system_services/bucket_server.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,7 @@ async function read_bucket_sdk_info(req) {
733733
s3_policy: bucket.s3_policy,
734734
system_owner: bucket.system.owner.email,
735735
bucket_owner: bucket.owner_account.email,
736+
bucket_owner_id: bucket.owner_account._id.toString(),
736737
bucket_info: await P.map_props({
737738
bucket,
738739
nodes_aggregate_pool: bucket.tiering && nodes_client.instance().aggregate_nodes_by_pool(pool_names, system._id),

0 commit comments

Comments
 (0)