Merge pull request #8175 from shirady/nsfs-iam-account-implicit-policy

shirady · web-flow · commit c5026ab8e1ad · 2024-07-28T10:07:25.000+03:00
NSFS | NC | IAM Service - Accounts Permission When No Bucket Policy
diff --git a/docs/design/iam.md b/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.
 
diff --git a/src/endpoint/s3/s3_rest.js b/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(
diff --git a/src/sdk/bucketspace_fs.js b/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');
         }
diff --git a/src/server/system_services/schemas/nsfs_bucket_schema.js b/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',
         },
diff --git a/src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_schema_validation.test.js b/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', () => {
diff --git a/src/test/unit_tests/test_bucketspace_fs.js b/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: 'iam_user_1@noobaa.io',
+    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: 'iam_user_2@noobaa.io',
+    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() {
