Fix - list_buckets allowing unauthorized bucket access

Signed-off-by: Aayush Chouhan <achouhan@redhat.com>

Author: aayushchouhan09

src/server/common_services/auth_server.js

@@ -536,7 +536,12 @@ function _get_auth_info(account, system, authorized_by, role, extra) {
  * @returns  {Promise<boolean>} true if the account has permission to perform the action on the bucket
  */
 async function has_bucket_action_permission(bucket, account, action, req_query, bucket_path = "") {
+    if (!account || !account.email) {
+        dbg.warn('has_bucket_action_permission: account or account.email is missing');
+        return false;
+    }
     dbg.log1('has_bucket_action_permission:', bucket.name, account.email, bucket.owner_account.email);
+
     // If the system owner account wants to access the bucket, allow it
     if (bucket.system.owner.email.unwrap() === account.email.unwrap()) return true;
 
@@ -558,7 +563,9 @@ async function has_bucket_action_permission(bucket, account, action, req_query,
     );
 
     if (result === 'DENY') return false;
-    return is_owner || result === 'ALLOW';
+    if (result === 'ALLOW') return true;
+
+    return is_owner;
 }
 
 /**

src/server/system_services/bucket_server.js

@@ -1062,8 +1062,13 @@ async function list_buckets(req) {
     let continuation_token = req.rpc_params?.continuation_token;
     const max_buckets = req.rpc_params?.max_buckets;
 
-    const accessible_bucket_list = system_store.data.buckets.filter(
-        async bucket => await req.has_s3_bucket_permission(bucket, "s3:ListBucket", req) && !bucket.deleting
+    const accessible_bucket_list = [];
+    await Promise.all(
+        system_store.data.buckets.map(async bucket => {
+            if (bucket.deleting) return;
+            const has_permission = await req.has_s3_bucket_permission(bucket, "s3:ListBucket");
+            if (has_permission) accessible_bucket_list.push(bucket);
+        })
     );
 
     accessible_bucket_list.sort((a, b) => a.name.unwrap().localeCompare(b.name.unwrap()));

src/test/integration_tests/api/s3/test_s3_list_buckets.js

@@ -127,5 +127,78 @@ mocha.describe('s3_ops', function() {
         });
     });
 
+    mocha.describe('list_buckets permissions', function() {
+        this.timeout(60000);
+        let s3_user;
+        const test_user = 'test-user';
+        const user_bucket = 'user-buck';
+        const admin_bucket = 'admin-buck';
+
+        mocha.before(async function() {
+            const account = await rpc_client.account.create_account({
+                name: test_user,
+                email: test_user,
+                has_login: false,
+                s3_access: true,
+                default_resource: coretest.POOL_LIST[0].name
+            });
+
+            s3_user = new S3Client({
+                ...s3_client_params,
+                credentials: {
+                    accessKeyId: account.access_keys[0].access_key.unwrap(),
+                    secretAccessKey: account.access_keys[0].secret_key.unwrap(),
+                }
+            });
+
+            await s3_user.send(new CreateBucketCommand({ Bucket: user_bucket }));
+            await s3.send(new CreateBucketCommand({ Bucket: admin_bucket }));
+        });
+
+        mocha.after(async function() {
+            await s3_user.send(new DeleteBucketCommand({ Bucket: user_bucket }));
+            await s3.send(new DeleteBucketCommand({ Bucket: admin_bucket }));
+            await rpc_client.account.delete_account({ email: test_user });
+        });
+
+        mocha.it('user should list only owned buckets', async function() {
+            const buckets = (await s3_user.send(new ListBucketsCommand())).Buckets.map(b => b.Name);
+            assert(buckets.includes(user_bucket) && !buckets.includes(admin_bucket));
+        });
+
+        mocha.it('admin should list all buckets', async function() {
+            const buckets = (await s3.send(new ListBucketsCommand())).Buckets.map(b => b.Name);
+            assert(buckets.length >= 2);
+            assert(buckets.includes(user_bucket));
+            assert(buckets.includes(admin_bucket));
+        });
+
+        mocha.it('bucket policy grants list access', async function() {
+            let buckets = (await s3_user.send(new ListBucketsCommand())).Buckets.map(b => b.Name);
+            assert(!buckets.includes(admin_bucket));
+
+            await rpc_client.bucket.put_bucket_policy({
+                name: admin_bucket,
+                policy: {
+                    Version: '2012-10-17',
+                    Statement: [{
+                        Effect: 'Allow',
+                        Principal: { AWS: test_user },
+                        Action: ['s3:ListBucket'],
+                        Resource: [`arn:aws:s3:::${admin_bucket}`]
+                    }]
+                }
+            });
+
+            buckets = (await s3_user.send(new ListBucketsCommand())).Buckets.map(b => b.Name);
+            assert(buckets.includes(admin_bucket));
+
+            await rpc_client.bucket.delete_bucket_policy({ name: admin_bucket });
+
+            buckets = (await s3_user.send(new ListBucketsCommand())).Buckets.map(b => b.Name);
+            assert(!buckets.includes(admin_bucket));
+        });
+    });
+
 });