Skip to content

Commit

Permalink
Merge pull request #7131 from romayalon/romy-fix-account-deletion
Browse files Browse the repository at this point in the history 
Delete account should throw FORBIDDEN and not UNAUTHORIZED when it's owner of existing buckets
  • Loading branch information
@romayalon
romayalon authored Dec 14, 2022
2 parents 5f5b691 + fdecad0 commit ac07f39
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 41 deletions.
56 changes: 22 additions & 34 deletions src/server/system_services/account_server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,24 +573,7 @@ async function get_account_usage(req) {
*/
function delete_account(req) {
let account_to_delete = system_store.get_account_by_email(req.rpc_params.email);
if (!account_to_delete) {
throw new RpcError('NO_SUCH_ACCOUNT', 'No such account email: ' + req.rpc_params.email);
}
if (account_to_delete.is_support) {
throw new RpcError('BAD_REQUEST', 'Cannot delete support account');
}
if (String(account_to_delete._id) === String(req.system.owner._id)) {
throw new RpcError('BAD_REQUEST', 'Cannot delete system owner account');
}
if (!is_support_or_admin_or_me(req.system, req.account, account_to_delete)) {
throw new RpcError('UNAUTHORIZED', 'Cannot delete account');
}
for (const bucket of system_store.data.buckets) {
if (system_store.has_same_id(bucket.owner_account, account_to_delete)) {
dbg.log2('bucket', bucket.name.unwrap(), 'id is equal to the account id');
throw new RpcError('UNAUTHORIZED', 'Cannot delete account that is owner of buckets');
}
}
_verify_can_delete_account(req, account_to_delete);

let roles_to_delete = system_store.data.roles
.filter(
Expand Down Expand Up @@ -642,22 +625,7 @@ function delete_account_by_property(req) {
let roles_to_delete = [];
let accounts_to_delete = system_store.get_accounts_by_nsfs_account_config(req.rpc_params.nsfs_account_config)
.map(account_to_delete => {
if (!account_to_delete) {
throw new RpcError('NO_SUCH_ACCOUNT', 'No such account email: ' + req.rpc_params.email);
}
if (account_to_delete.is_support) {
throw new RpcError('BAD_REQUEST', 'Cannot delete support account');
}
if (String(account_to_delete._id) === String(req.system.owner._id)) {
throw new RpcError('BAD_REQUEST', 'Cannot delete system owner account');
}
if (!is_support_or_admin_or_me(req.system, req.account, account_to_delete)) {
throw new RpcError('UNAUTHORIZED', 'Cannot delete account');
}
if (system_store.data.buckets.find(b => String(b.owner_account) === String(account_to_delete._id))) {
throw new RpcError('UNAUTHORIZED', 'Cannot delete account that is owner of buckets');
}

_verify_can_delete_account(req, account_to_delete);
roles_to_delete = roles_to_delete.concat(system_store.data.roles
.filter(
role => String(role.account._id) === String(account_to_delete._id)
Expand Down Expand Up @@ -1410,6 +1378,26 @@ function _list_connection_usage(account, credentials) {
return _.concat(cloud_pool_usage, namespace_resource_usage);
}

function _verify_can_delete_account(req, account_to_delete) {
if (!account_to_delete) {
throw new RpcError('NO_SUCH_ACCOUNT', 'No such account email: ' + req.rpc_params.email);
}
if (account_to_delete.is_support) {
throw new RpcError('BAD_REQUEST', 'Cannot delete support account');
}
if (String(account_to_delete._id) === String(req.system.owner._id)) {
throw new RpcError('BAD_REQUEST', 'Cannot delete system owner account');
}
if (!is_support_or_admin_or_me(req.system, req.account, account_to_delete)) {
throw new RpcError('UNAUTHORIZED', 'Cannot delete account');
}
for (const bucket of system_store.data.buckets) {
if (system_store.has_same_id(bucket.owner_account, account_to_delete)) {
dbg.log2('bucket', bucket.name.unwrap(), 'id is equal to the account id');
throw new RpcError('FORBIDDEN', 'Cannot delete account that is owner of buckets');
}
}
}

// EXPORTS
exports.create_account = create_account;
Expand Down
36 changes: 29 additions & 7 deletions src/test/unit_tests/test_bucketspace.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ const assert = require('assert');
const coretest = require('./coretest');
const { rpc_client, EMAIL, PASSWORD, SYSTEM } = coretest;
const fs_utils = require('../../util/fs_utils');
coretest.setup({ pools_to_create: [coretest.POOL_LIST[0]] });
coretest.setup({ pools_to_create: [coretest.POOL_LIST[1]] });
const path = require('path');
const _ = require('lodash');
const P = require('../../util/promise');
const fs = require('fs');
const test_utils = require('../system_tests/test_utils');
const MAC_PLATFORM = 'darwin';

const inspect = (x, max_arr = 5) => util.inspect(x, { colors: true, depth: null, maxArrayLength: max_arr });

Expand All @@ -29,7 +30,7 @@ let new_account_params = {
mocha.describe('bucket operations - namespace_fs', function() {
const nsr = 'nsr';
const bucket_name = 'src-bucket';
const tmp_fs_root = '/tmp/test_bucket_namespace_fs';
const tmp_fs_root = get_tmp_path_by_os('/tmp/test_bucket_namespace_fs');
const bucket_path = '/src';
const other_bucket_path = '/src1';
let account_wrong_uid;
Expand Down Expand Up @@ -69,7 +70,6 @@ mocha.describe('bucket operations - namespace_fs', function() {
name: nsr,
nsfs_config: {
fs_root_path: tmp_fs_root,
fs_backend: 'GPFS'
}
});
const obj_nsr = { resource: nsr, path: bucket_path };
Expand Down Expand Up @@ -461,6 +461,25 @@ mocha.describe('bucket operations - namespace_fs', function() {
assert.strictEqual(err.code, 'AccessDenied');
}
});
mocha.it('delete account by uid, gid - account has buckets - should fail', async function() {
await s3_correct_uid_default_nsr.createBucket({ Bucket: 'bucket-to-delete123' }).promise();
try {
await rpc_client.account.delete_account({ email: '[email protected]' });
assert.fail(`delete account succeeded for account that has buckets`);
} catch (err) {
assert.equal(err.rpc_code, 'FORBIDDEN');
}

try {
await rpc_client.account.delete_account_by_property({ nsfs_account_config: { uid: process.getuid(), gid: process.getgid() } });
assert.fail(`delete account succeeded for account that has buckets`);
} catch (err) {
assert.equal(err.rpc_code, 'FORBIDDEN');
}
await s3_correct_uid_default_nsr.deleteBucket({ Bucket: 'bucket-to-delete123' }).promise();

});

mocha.it('delete bucket with uid, gid - bucket is empty', async function() {
const res = await s3_correct_uid_default_nsr.deleteBucket({ Bucket: bucket_name + '-s3' }).promise();
console.log(inspect(res));
Expand Down Expand Up @@ -578,7 +597,7 @@ async function update_account_nsfs_config(email, default_resource, new_nsfs_acco
mocha.describe('list objects - namespace_fs', function() {
const nsr = 'nsr1-list';
const bucket_name = 'bucket-to-list1';
const tmp_fs_root = '/tmp/test_bucket_namespace_fs1';
const tmp_fs_root = get_tmp_path_by_os('/tmp/test_bucket_namespace_fs1');
const bucket_path = '/bucket';
let s3_uid5;
let s3_uid26041993;
Expand Down Expand Up @@ -609,7 +628,6 @@ mocha.describe('list objects - namespace_fs', function() {
name: nsr,
nsfs_config: {
fs_root_path: tmp_fs_root,
fs_backend: 'GPFS'
}
});
const obj_nsr = { resource: nsr, path: bucket_path };
Expand Down Expand Up @@ -808,7 +826,7 @@ mocha.describe('nsfs account configurations', function() {
const non_nsfs_bucket1 = 'first.bucket';
const non_nsfs_bucket2 = 'second.bucket';
const nsr2_connection = 'nsr2_connection';
const tmp_fs_root1 = '/tmp/test_bucket_namespace_fs2';
const tmp_fs_root1 = get_tmp_path_by_os('/tmp/test_bucket_namespace_fs2');
const bucket_path = '/nsfs_accounts';
const accounts = {}; // {account_name : s3_account_object...}
const regular_bucket_name = ['regular-bucket', 'regular-bucket1', 'regular-bucket2'];
Expand Down Expand Up @@ -836,7 +854,6 @@ mocha.describe('nsfs account configurations', function() {
name: nsr1,
nsfs_config: {
fs_root_path: tmp_fs_root1,
fs_backend: 'GPFS'
}
});
const obj_nsr = { resource: nsr1, path: bucket_path };
Expand Down Expand Up @@ -1192,3 +1209,8 @@ mocha.describe('nsfs account configurations', function() {
}
});
});

function get_tmp_path_by_os(_path) {
return process.platform === MAC_PLATFORM ? '/private/' + _path : _path;
}

0 comments on commit ac07f39

Please sign in to comment.