Skip to content

Commit bc6baaa

Browse files
fixed bucketspace_fs for list_buckets
Signed-off-by: Aayush Chouhan <achouhan@redhat.com>
1 parent e823403 commit bc6baaa

File tree

1 file changed

+57
-16
lines changed

1 file changed

+57
-16
lines changed

src/sdk/bucketspace_fs.js

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,9 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
271271
if (err.rpc_code !== 'NO_SUCH_BUCKET') throw err;
272272
}
273273
if (!bucket) return;
274-
const bucket_policy_accessible = await this.has_bucket_action_permission(bucket, account, 's3:ListBucket');
275-
dbg.log2(`list_buckets: bucket_name ${bucket_name} bucket_policy_accessible`, bucket_policy_accessible);
276-
if (!bucket_policy_accessible) return;
274+
275+
if (!this.has_list_bucket_permission(bucket, account)) return;
276+
277277
const fs_accessible = await this.validate_fs_bucket_access(bucket, object_sdk);
278278
if (!fs_accessible) return;
279279
return bucket;
@@ -921,25 +921,66 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
921921
///// UTILS /////
922922
/////////////////
923923

924-
// TODO: move the following 3 functions - has_bucket_action_permission(), validate_fs_bucket_access(), _has_access_to_nsfs_dir()
924+
// TODO: move the following 5 functions - is_system_owner(), is_bucket_owner(), has_list_bucket_permission(), has_bucket_action_permission(), validate_fs_bucket_access(), _has_access_to_nsfs_dir()
925925
// so they can be re-used
926+
927+
/**
928+
* is_system_owner checks if the account is the system owner
929+
* @param {Record<string, any>} bucket
930+
* @param {Record<string, any>} account
931+
* @returns {boolean}
932+
*/
933+
is_system_owner(bucket, account) {
934+
return Boolean(bucket?.system_owner) && bucket.system_owner.unwrap() === account?.name?.unwrap();
935+
}
936+
937+
/**
938+
* is_bucket_owner checks if the account owns the bucket
939+
* @param {Record<string, any>} bucket
940+
* @param {Record<string, any>} account
941+
* @returns {boolean}
942+
*/
943+
is_bucket_owner(bucket, account) {
944+
// check direct ownership
945+
if (bucket?.owner_account?.id === account?._id) return true;
946+
947+
// check IAM account with same root owner
948+
if (account?.owner && bucket?.owner_account?.id === account.owner) return true;
949+
950+
return false;
951+
}
952+
953+
/**
954+
* has_list_bucket_permission returns true if the account can list the bucket in ListBuckets operation
955+
* this is a synchronous function that only checks system ownership and bucket ownership
956+
*
957+
* @param {Record<string, any>} bucket
958+
* @param {Record<string, any>} account
959+
* @returns {boolean}
960+
*/
961+
has_list_bucket_permission(bucket, account) {
962+
// system owner can list all the buckets
963+
if (this.is_system_owner(bucket, account)) return true;
964+
965+
// bucket owner can list their buckets only
966+
if (this.is_bucket_owner(bucket, account)) return true;
967+
968+
// TODO: Add IAM policy support for s3:ListAllMyBuckets action
969+
970+
return false;
971+
}
972+
926973
async has_bucket_action_permission(bucket, account, action, bucket_path = "") {
927974
dbg.log1('has_bucket_action_permission:', bucket.name.unwrap(), account.name.unwrap(), account._id, bucket.bucket_owner.unwrap());
928975

929-
const is_system_owner = Boolean(bucket.system_owner) && bucket.system_owner.unwrap() === account.name.unwrap();
976+
// system owner can access all buckets
977+
if (this.is_system_owner(bucket, account)) return true;
930978

931-
// If the system owner account wants to access the bucket, allow it
932-
if (is_system_owner) return true;
933-
const is_owner = (bucket.owner_account && bucket.owner_account.id === account._id);
979+
// check bucket ownership
980+
const owner = this.is_bucket_owner(bucket, account);
934981
const bucket_policy = bucket.s3_policy;
935982

936-
if (!bucket_policy) {
937-
// in case we do not have bucket policy
938-
// we allow IAM account to access a bucket that that is owned by their root account
939-
const is_iam_and_same_root_account_owner = account.owner !== undefined &&
940-
account.owner === bucket.owner_account.id;
941-
return is_owner || is_iam_and_same_root_account_owner;
942-
}
983+
if (!bucket_policy) return owner;
943984
if (!action) {
944985
throw new Error('has_bucket_action_permission: action is required');
945986
}
@@ -967,7 +1008,7 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
9671008
);
9681009
}
9691010
if (permission_by_name === 'DENY') return false;
970-
return is_owner || (permission_by_id === 'ALLOW' || permission_by_name === 'ALLOW');
1011+
return owner || (permission_by_id === 'ALLOW' || permission_by_name === 'ALLOW');
9711012
}
9721013

9731014
async validate_fs_bucket_access(bucket, object_sdk) {

0 commit comments

Comments
 (0)