NSFS | NC | IAM Service - Accounts Permission When No Bucket Policy

1. Add more properties to nsfs_bucket_schema (not required) and update the new_bucket_defaults in BucketSpaceFS:
   - creator = creator is the account ID that created this bucket (internal information).
     Notes: Currently we do not allow IAM accounts to create a bucket (temporary), it will be changed after we have the config structure, therefore in the future, we could see the IAM account ID in the creator property.
2. Change the condition in authorize_request_policy (in s3_rest) and has_bucket_action_permission (in bucketspace_fs) for the same root account (alternative for ownership when there is no bucket policy).

Those next changes are not related to IAM, but were raised as a part of the code review:
3. In authorize_request_policy (in s3_rest) remove the condition req.object_sdk.nsfs_config_root from the owner condition.
4. In has_bucket_action_permission (in bucketspace_fs) change the condition of is_owner from account name to id.

Signed-off-by: shirady <[email protected]>

Author: shirady

docs/design/iam.md

@@ -89,6 +89,14 @@ Source: AccessKeys
     - root account
     - all IAM users only for themselves (except the first creation that can be done only by the root account).
 
+### No Bucket Policy
+If the resource doesn’t have a bucket policy the IAM user accounts can have access to the resources of the same root account.
+For example: 
+- root account creates 2 users (both are owned by it): user1, user2 and a bucket (bucket owner: <root-account-id>, bucket creator: <account-id-user1>).
+- user1 upload a file to the bucket 
+- user2 can delete this bucket (after it is empty): although user2 is not the creator, without a bucket policy his root account is the owner so he can delete the bucket.
+Note: Currently, we do not allow users to create a bucket.
+
 ### Root Accounts Manager
 The root accounts managers are a solution for creating root accounts using the IAM API.
 

src/endpoint/s3/s3_rest.js

@@ -216,6 +216,7 @@ async function authorize_request_policy(req) {
     if (!req.params.bucket) return;
     if (req.op_name === 'put_bucket') return;
 
+    // owner_account is { id: bucket.owner_account, email: bucket.bucket_owner };
     const { s3_policy, system_owner, bucket_owner, owner_account } = await req.object_sdk.read_bucket_sdk_policy_info(req.params.bucket);
     const auth_token = req.object_sdk.get_auth_token();
     const arn_path = _get_arn_from_req_path(req);
@@ -236,13 +237,17 @@ async function authorize_request_policy(req) {
 
     const is_owner = (function() {
         if (account.bucket_claim_owner && account.bucket_claim_owner.unwrap() === req.params.bucket) return true;
-        if (req.object_sdk.nsfs_config_root && account._id === owner_account.id) return true; // NC NSFS case
+        if (owner_account && owner_account.id === account._id) return true;
         if (account_identifier === bucket_owner.unwrap()) return true;
         return false;
     }());
 
     if (!s3_policy) {
-        if (is_owner) return;
+        // in case we do not have bucket policy
+        // we allow IAM account to access a bucket that is owned by their root account
+        const is_iam_account_and_same_root_account_owner = account.owner !== undefined &&
+            owner_account && account.owner === owner_account.id;
+        if (is_owner || is_iam_account_and_same_root_account_owner) return;
         throw new S3Error(S3Error.AccessDenied);
     }
     const permission = await s3_bucket_policy_utils.has_bucket_policy_permission(

src/sdk/bucketspace_fs.js

@@ -265,6 +265,11 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
             if (!sdk.requesting_account.allow_bucket_creation) {
                 throw new RpcError('UNAUTHORIZED', 'Not allowed to create new buckets');
             }
+            // currently we do not allow IAM account to create a bucket (temporary)
+            if (sdk.requesting_account.owner !== undefined) {
+            dbg.warn('create_bucket: account is IAM account (currently not allowed to create buckets)');
+                throw new RpcError('UNAUTHORIZED', 'Not allowed to create new buckets');
+            }
             if (!sdk.requesting_account.nsfs_account_config || !sdk.requesting_account.nsfs_account_config.new_buckets_path) {
                 throw new RpcError('MISSING_NSFS_ACCOUNT_CONFIGURATION');
             }
@@ -320,6 +325,7 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
             name,
             tag: js_utils.default_value(tag, undefined),
             owner_account: account._id,
+            creator: account._id,
             system_owner: new SensitiveString(account.name),
             bucket_owner: new SensitiveString(account.name),
             versioning: config.NSFS_VERSIONING_ENABLED && lock_enabled ? 'ENABLED' : 'DISABLED',
@@ -746,10 +752,16 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
 
         // If the system owner account wants to access the bucket, allow it
         if (is_system_owner) return true;
-        const is_owner = (bucket.bucket_owner.unwrap() === account_identifier);
+        const is_owner = (bucket.owner_account && bucket.owner_account.id === account._id);
         const bucket_policy = bucket.s3_policy;
 
-        if (!bucket_policy) return is_owner;
+        if (!bucket_policy) {
+        // in case we do not have bucket policy
+        // we allow IAM account to access a bucket that that is owned by their root account
+        const is_iam_and_same_root_account_owner = account.owner !== undefined &&
+            account.owner === bucket.owner_account.id;
+            return is_owner || is_iam_and_same_root_account_owner;
+        }
         if (!action) {
             throw new Error('has_bucket_action_permission: action is required');
         }

src/server/system_services/schemas/nsfs_bucket_schema.js

@@ -23,6 +23,10 @@ module.exports = {
         owner_account: {
             type: 'string',
         },
+        // creator is the account id that created this bucket (internal information)
+        creator: {
+            type: 'string',
+        },
         name: {
             type: 'string',
         },

src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_schema_validation.test.js

@@ -100,6 +100,11 @@ describe('schema validation NC NSFS bucket', () => {
             nsfs_schema_utils.validate_bucket_schema(bucket_data);
         });
 
+        it('nsfs_bucket with creator', () => {
+            const bucket_data = get_bucket_data();
+            bucket_data.creator = '65a62e22ceae5e5f1a758ab1';
+            nsfs_schema_utils.validate_bucket_schema(bucket_data);
+        });
     });
 
     describe('bucket with additional properties', () => {
@@ -434,6 +439,14 @@ describe('schema validation NC NSFS bucket', () => {
             assert_validation(bucket_data, reason, message);
         });
 
+        it('bucket with creator as a number (instead of string)', () => {
+            const bucket_data = get_bucket_data();
+            bucket_data.creator = 123; // number instead of string
+            const reason = 'Test should have failed because of wrong type for' +
+                'creator with number (instead of string)';
+            const message = 'must be string';
+            assert_validation(bucket_data, reason, message);
+        });
     });
 
     describe('skip schema check by config test', () => {

src/test/unit_tests/test_bucketspace_fs.js

@@ -25,6 +25,7 @@ const test_bucket = 'bucket1';
 const test_not_empty_bucket = 'notemptybucket';
 const test_bucket_temp_dir = 'buckettempdir';
 const test_bucket_invalid = 'bucket_invalid';
+const test_bucket_iam_account = 'bucket-iam-account-can-access';
 
 const tmp_fs_path = path.join(TMP_PATH, 'test_bucketspace_fs');
 const config_root = path.join(tmp_fs_path, 'config_root');
@@ -191,6 +192,57 @@ function make_dummy_object_sdk() {
         }
     };
 }
+
+// account_user2 (copied from the dummy sdk)
+// is the root account of account_iam_user1
+const account_iam_user1 = {
+    _id: '65a8edc9bc5d5bbf9db71b94',
+    name: 'iam_user_1',
+    email: '[email protected]',
+    owner: dummy_object_sdk.requesting_account._id,
+    allow_bucket_creation: dummy_object_sdk.requesting_account.allow_bucket_creation,
+    access_keys: [{
+        access_key: 'a-abcdefghijklmn123459',
+        secret_key: 's-abcdefghijklmn123459Example'
+    }],
+    nsfs_account_config: {
+        uid: dummy_object_sdk.requesting_account.nsfs_account_config.uid,
+        gid: dummy_object_sdk.requesting_account.nsfs_account_config.gid,
+        new_buckets_path: dummy_object_sdk.requesting_account.nsfs_account_config.new_buckets_path
+    },
+    creation_date: '2023-11-30T04:46:33.815Z',
+};
+
+// account_user2 (copied from the dummy sdk)
+// is the root account of account_iam_user2
+const account_iam_user2 = {
+    _id: '65a8edc9bc5d5bbf9db71b95',
+    name: 'iam_user_2',
+    email: '[email protected]',
+    owner: dummy_object_sdk.requesting_account._id,
+    allow_bucket_creation: dummy_object_sdk.requesting_account.allow_bucket_creation,
+    access_keys: [{
+        access_key: 'a-abcdefghijklmn123460',
+        secret_key: 's-abcdefghijklmn123460Example'
+    }],
+    nsfs_account_config: {
+        uid: dummy_object_sdk.requesting_account.nsfs_account_config.uid,
+        gid: dummy_object_sdk.requesting_account.nsfs_account_config.gid,
+        new_buckets_path: dummy_object_sdk.requesting_account.nsfs_account_config.new_buckets_path
+    },
+    creation_date: '2023-12-30T04:46:33.815Z',
+};
+
+function make_dummy_object_sdk_for_account(dummy_object_sdk_to_copy, account) {
+    const dummy_object_sdk_for_account = _.cloneDeep(dummy_object_sdk_to_copy);
+    dummy_object_sdk_for_account.requesting_account = account;
+    dummy_object_sdk_for_account.requesting_account.name = new SensitiveString(
+        dummy_object_sdk_for_account.requesting_account.name);
+    dummy_object_sdk_for_account.requesting_account.email = new SensitiveString(
+            dummy_object_sdk_for_account.requesting_account.email);
+    return dummy_object_sdk_for_account;
+}
+
 function make_invalid_dummy_object_sdk() {
     return {
         requesting_account: {
@@ -216,7 +268,7 @@ mocha.describe('bucketspace_fs', function() {
             await fs_utils.create_fresh_path(`${config_root}/${dir}`))
         );
         await fs_utils.create_fresh_path(new_buckets_path);
-        for (let account of [account_user1, account_user2, account_user3]) {
+        for (let account of [account_user1, account_user2, account_user3, account_iam_user1, account_iam_user2]) {
             account = await nc_mkm.encrypt_access_keys(account);
             const account_path = get_config_file_path(CONFIG_SUBDIRS.ACCOUNTS, account.name);
             const account_access_path = get_access_key_symlink_path(CONFIG_SUBDIRS.ACCESS_KEYS, account.access_keys[0].access_key);
@@ -312,9 +364,23 @@ mocha.describe('bucketspace_fs', function() {
                 assert.ok(err.rpc_code === 'UNAUTHORIZED');
             }
         });
+        mocha.it('should fail - create bucket by iam account', async function() {
+            // currently we do not allow IAM accounts to create buckets
+            try {
+                const param = { name: test_bucket_iam_account};
+                const dummy_object_sdk_for_iam_account = make_dummy_object_sdk_for_account(dummy_object_sdk, account_iam_user1);
+                await bucketspace_fs.create_bucket(param, dummy_object_sdk_for_iam_account);
+                assert.fail('should have failed with UNAUTHORIZED bucket creation');
+            } catch (err) {
+                assert.ok(err.rpc_code === 'UNAUTHORIZED');
+            }
+        });
         mocha.after(async function() {
             await fs_utils.folder_delete(`${new_buckets_path}/${test_bucket}`);
-            const file_path = get_config_file_path(CONFIG_SUBDIRS.BUCKETS, test_bucket);
+            await fs_utils.folder_delete(`${new_buckets_path}/${test_bucket_iam_account}`);
+            let file_path = get_config_file_path(CONFIG_SUBDIRS.BUCKETS, test_bucket);
+            await fs_utils.file_delete(file_path);
+            file_path = get_config_file_path(CONFIG_SUBDIRS.BUCKETS, test_bucket_iam_account);
             await fs_utils.file_delete(file_path);
         });
     });
@@ -333,6 +399,19 @@ mocha.describe('bucketspace_fs', function() {
             const objects = await bucketspace_fs.list_buckets(dummy_object_sdk);
             assert.equal(objects.buckets.length, 1);
         });
+        mocha.it('list buckets - iam accounts', async function() {
+            // root account created a bucket
+
+            // account_iam_user2 can list the created bucket (the implicit policy - same root account)
+            const dummy_object_sdk_for_iam_account = make_dummy_object_sdk_for_account(dummy_object_sdk, account_iam_user1);
+            const res = await bucketspace_fs.list_buckets(dummy_object_sdk_for_iam_account);
+            assert.equal(res.buckets.length, 1);
+
+            // account_iam_user2 can list the created bucket (the implicit policy - same root account)
+            const dummy_object_sdk_for_iam_account2 = make_dummy_object_sdk_for_account(dummy_object_sdk, account_iam_user2);
+            const res2 = await bucketspace_fs.list_buckets(dummy_object_sdk_for_iam_account2);
+            assert.equal(res2.buckets.length, 1);
+        });
         mocha.after(async function() {
             await fs_utils.folder_delete(`${new_buckets_path}/${test_bucket}`);
             const file_path = get_config_file_path(CONFIG_SUBDIRS.BUCKETS, test_bucket);
@@ -413,6 +492,22 @@ mocha.describe('bucketspace_fs', function() {
             }
             await fs.promises.stat(path.join(new_buckets_path, param.name));
         });
+
+        mocha.it('delete buckets - iam accounts (another IAM account deletes the bucket)', async function() {
+            // root account created the bucket
+            await create_bucket(test_bucket_iam_account);
+
+            // account_iam_user1 can see the bucket in the list
+            const dummy_object_sdk_for_account_iam_user1 = make_dummy_object_sdk_for_account(dummy_object_sdk, account_iam_user1);
+            const res = await bucketspace_fs.list_buckets(dummy_object_sdk_for_account_iam_user1);
+            assert.ok(res.buckets.length > 0);
+            assert.ok(res.buckets.some(bucket => bucket.name.unwrap() === test_bucket_iam_account));
+
+            const param = { name: test_bucket_iam_account};
+            // account_iam_user2 can delete the created bucket (the implicit policy - same root account)
+            const dummy_object_sdk_for_account_iam_user2 = make_dummy_object_sdk_for_account(dummy_object_sdk, account_iam_user2);
+            await bucketspace_fs.delete_bucket(param, dummy_object_sdk_for_account_iam_user2);
+        });
     });
     mocha.describe('set_bucket_versioning', function() {
         mocha.before(async function() {