bucket notification - harden bad name scenario. Also, don't reply internal '_' field. (#8796)

Signed-off-by: Amit Prinz Setter <alphaprinz@gmail.com>

Author: alphaprinz

src/cmd/manage_nsfs.js

@@ -758,40 +758,48 @@ async function notification_management() {
 async function connection_management(action, user_input) {
     manage_nsfs_validations.validate_connection_args(user_input, action);
 
+    //don't reply the internal '_' field.
+    delete user_input._;
+
     let response = {};
     let data;
 
-    switch (action) {
-        case ACTIONS.ADD:
-            data = await notifications_util.add_connect_file(user_input, config_fs);
-            response = { code: ManageCLIResponse.ConnectionCreated, detail: data };
-            break;
-        case ACTIONS.DELETE:
-            await config_fs.delete_connection_config_file(user_input.name);
-            response = { code: ManageCLIResponse.ConnectionDeleted, detail: {name: user_input.name} };
-            break;
-        case ACTIONS.UPDATE:
-            await notifications_util.update_connect_file(user_input.name, user_input.key,
-                user_input.value, user_input.remove_key, config_fs);
-            response = { code: ManageCLIResponse.ConnectionUpdated, detail: {name: user_input.name} };
-            break;
-        case ACTIONS.STATUS:
-            data = await new notifications_util.Notificator({
-                fs_context: config_fs.fs_context,
-                connect_files_dir: config_fs.connections_dir_path,
-                nc_config_fs: config_fs,
-            }).parse_connect_file(user_input.name, user_input.decrypt);
-            response = { code: ManageCLIResponse.ConnectionStatus, detail: data };
-            break;
-        case ACTIONS.LIST:
-            data = await list_connections();
-            response = { code: ManageCLIResponse.ConnectionList, detail: data };
-            break;
-        default:
-            throw_cli_error(ManageCLIError.InvalidAction);
+    try {
+        switch (action) {
+            case ACTIONS.ADD:
+                data = await notifications_util.add_connect_file(user_input, config_fs);
+                response = { code: ManageCLIResponse.ConnectionCreated, detail: data };
+                break;
+            case ACTIONS.DELETE:
+                await config_fs.delete_connection_config_file(user_input.name);
+                response = { code: ManageCLIResponse.ConnectionDeleted, detail: {name: user_input.name} };
+                break;
+            case ACTIONS.UPDATE:
+                await notifications_util.update_connect_file(user_input.name, user_input.key,
+                    user_input.value, user_input.remove_key, config_fs);
+                response = { code: ManageCLIResponse.ConnectionUpdated, detail: {name: user_input.name} };
+                break;
+            case ACTIONS.STATUS:
+                data = await new notifications_util.Notificator({
+                    fs_context: config_fs.fs_context,
+                    connect_files_dir: config_fs.connections_dir_path,
+                    nc_config_fs: config_fs,
+                }).parse_connect_file(user_input.name, user_input.decrypt);
+                response = { code: ManageCLIResponse.ConnectionStatus, detail: data };
+                break;
+            case ACTIONS.LIST:
+                data = await list_connections();
+                response = { code: ManageCLIResponse.ConnectionList, detail: data };
+                break;
+            default:
+                throw_cli_error(ManageCLIError.InvalidAction);
+        }
+        write_stdout_response(response.code, response.detail, response.event_arg);
+    } catch (err) {
+        if (err.code === 'EEXIST') throw_cli_error(ManageCLIError.ConnectionAlreadyExists, user_input.name);
+        if (err.code === 'ENOENT') throw_cli_error(ManageCLIError.NoSuchConnection, user_input.name);
+        throw err;
     }
-
-    write_stdout_response(response.code, response.detail, response.event_arg);
 }
 
 ////////////////////

src/manage_nsfs/manage_nsfs_cli_errors.js

@@ -528,6 +528,18 @@ ManageCLIError.LifecycleWorkerReachedTimeout = Object.freeze({
     http_code: 400,
 });
 
+ManageCLIError.ConnectionAlreadyExists = Object.freeze({
+    code: 'ConnectionAlreadyExists',
+    message: 'The requested connection name is not available. Please select a different name and try again.',
+    http_code: 409,
+});
+
+ManageCLIError.NoSuchConnection = Object.freeze({
+    code: 'NoSuchConnection',
+    message: 'Connection does not exist.',
+    http_code: 404,
+});
+
 ///////////////////////////////
 //       ERRORS MAPPING      //
 ///////////////////////////////

src/test/unit_tests/jest_tests/test_nc_nsfs_connection_cli.test.js

@@ -6,6 +6,7 @@
 // disabling init_rand_seed as it takes longer than the actual test execution
 process.env.DISABLE_INIT_RANDOM_SEED = "true";
 
+const _ = require('lodash');
 const fs = require('fs');
 const path = require('path');
 const os_util = require('../../../util/os_utils');
@@ -14,7 +15,10 @@ const { ConfigFS } = require('../../../sdk/config_fs');
 const { TMP_PATH, set_nc_config_dir_in_config } = require('../../system_tests/test_utils');
 const { TYPES, ACTIONS } = require('../../../manage_nsfs/manage_nsfs_constants');
 
-const tmp_fs_path = path.join(TMP_PATH, 'test_nc_nsfs_account_cli');
+const ManageCLIError = require('../../../manage_nsfs/manage_nsfs_cli_errors').ManageCLIError;
+
+const tmp_fs_path = path.join(TMP_PATH, 'test_nc_connection_cli.test');
+
 const timeout = 5000;
 
 // eslint-disable-next-line max-lines-per-function
@@ -58,7 +62,9 @@ describe('manage nsfs cli connection flow', () => {
         it('cli create connection from cli', async () => {
             const conn_options = {...defaults, config_root};
             conn_options.name = "fromcli";
-            await exec_manage_cli(TYPES.CONNECTION, ACTIONS.ADD, conn_options);
+            const res = await exec_manage_cli(TYPES.CONNECTION, ACTIONS.ADD, conn_options);
+            const res_json = JSON.parse(res.trim());
+            expect(_.isEqual(new Set(Object.keys(res_json.response)), new Set(['reply', 'code']))).toBe(true);
             const connection = await config_fs.get_connection_by_name(conn_options.name);
             assert_connection(connection, conn_options, true);
         }, timeout);
@@ -96,6 +102,26 @@ describe('manage nsfs cli connection flow', () => {
             assert_connection(res.response.reply, defaults, false);
         }, timeout);
 
+        it('conn already exists', async () => {
+            const action = ACTIONS.ADD;
+            const { name, agent_request_object, request_options_object, notification_protocol } = defaults;
+            const conn_options = { config_root, name, agent_request_object, request_options_object, notification_protocol };
+            await exec_manage_cli(TYPES.CONNECTION, action, conn_options);
+            const actual = await config_fs.get_connection_by_name(name);
+            assert_connection(actual, defaults, true);
+
+            const res = await exec_manage_cli(TYPES.CONNECTION, action, conn_options, true);
+            const res_json = JSON.parse(res.trim());
+            expect(res_json.error.code).toBe(ManageCLIError.ConnectionAlreadyExists.code);
+        });
+
+        it('conn does not exist', async () => {
+            const conn_options = { config_root, name: "badname" };
+            const res = await exec_manage_cli(TYPES.CONNECTION, ACTIONS.DELETE, conn_options, true);
+            const res_json = JSON.parse(res.trim());
+            expect(res_json.error.code).toBe(ManageCLIError.NoSuchConnection.code);
+        });
+
     });
 });
 
@@ -123,13 +149,17 @@ function assert_connection(connection, connection_options, is_encrypted) {
  * @param {string} action
  * @param {object} options
  */
-async function exec_manage_cli(type, action, options) {
+async function exec_manage_cli(type, action, options, expect_failure = false) {
     const command = create_command(type, action, options);
     let res;
     try {
         res = await os_util.exec(command, { return_stdout: true });
     } catch (e) {
-        res = e;
+        if (expect_failure) {
+            res = e.stdout;
+        } else {
+            res = e;
+        }
     }
     return res;
 }