From 93424d6109bb84cbb82188609a314d47f5652a48 Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Fri, 19 Jan 2024 19:27:22 +0100 Subject: [PATCH 1/4] VLTCLT-39: Update getAccounts to search by name Produces a new function signature to not mess with already existing calls --- lib/IAMClient.js | 58 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/lib/IAMClient.js b/lib/IAMClient.js index 0bf0b473..bdceca4c 100644 --- a/lib/IAMClient.js +++ b/lib/IAMClient.js @@ -456,6 +456,7 @@ class VaultClient { /** * Get accounts using account ids, canonical ids or email addresses * + * @overload * @param {array|undefined} accountIds - Account ids, exclusive with * emailAddresses and canonicalIds * @param {array|undefined} emailAddresses - Email addresses, exclusive @@ -467,20 +468,55 @@ class VaultClient { * @param {function} callback - Callback(err, result) * @return {undefined} */ + // eslint-disable valid-jsdoc + /** + * Get accounts using account ids, canonical ids, email addresses + * or account names + * + * @overload - New signature that adds filter accountNames + * @param {object} filter - Contains one of the account filters + * @param {array|undefined} [filter.accountIds] - Account ids, exclusive with + * emailAddresses, canonicalIds and accountNames + * @param {array|undefined} [filter.emailAddresses] - Email addresses, exclusive + * with account ids, canonicalIds and accountNames + * @param {array|undefined} [filter.canonicalIds] - Canonical ids, exclusive with + * account ids, emailAddresses and accountNames + * @param {array|undefined} [filter.accountNames] - Account Names, exclusive with + * account ids, emailAddresses and canonicalIds. Do not use with vault2. + * @param {object} options - Options + * @param {string} [options.reqUid] - Request uid + * @param {function} callback - Callback(err, result) + * @return {undefined} + */ + // eslint-enable valid-jsdoc getAccounts(accountIds, emailAddresses, canonicalIds, options, callback) { + let accountNames; + if (arguments.length === 3) { + // check and convert args from signature getAccounts(filter, options, callback) + const filter = accountIds; + /* eslint-disable-next-line no-param-reassign */ + options = emailAddresses; + /* eslint-disable-next-line no-param-reassign */ + callback = canonicalIds; + assert(filter && typeof filter === 'object' && !Array.isArray(filter), + 'first arg (filter) must be an object containing one filter field'); + assert(options && typeof options === 'object' && !Array.isArray(options), + 'second arg (options) must be an object'); + assert(callback && typeof callback === 'function', + 'third arg (callback) must be a function'); + ({ + /* eslint-disable-next-line no-param-reassign */ + accountIds, emailAddresses, canonicalIds, accountNames + } = filter); + } assert((accountIds && Array.isArray(accountIds)) || !accountIds, 'accountIds should be an array'); assert((emailAddresses && Array.isArray(emailAddresses)) || !emailAddresses, 'emailAddresses should be an array'); assert((canonicalIds && Array.isArray(canonicalIds)) || !canonicalIds, 'canonicalIds should be an array'); - if ( - (accountIds && (emailAddresses || canonicalIds)) - || (emailAddresses && (accountIds || canonicalIds)) - || (canonicalIds && (accountIds || emailAddresses))) { - assert(false, 'accountIds, emailAddresses and canonicalIds ' - + 'ids are exclusive'); - } + assert((accountNames && Array.isArray(accountNames)) || !accountNames, + 'accountNames should be an array'); const data = { Action: 'GetAccounts', Version: '2010-05-08', @@ -494,7 +530,13 @@ class VaultClient { if (emailAddresses) { data.emailAddresses = emailAddresses; } - + if (accountNames) { + data.accountNames = accountNames; + } + if (Object.values(data).length > 3) { + assert(false, 'accountIds, emailAddresses, canonicalIds ' + + 'and accountNames ids are exclusive'); + } const verb = this.useAuthenticatedAdminRoutes ? 'POST' : 'GET'; this.request(verb, '/', this.useAuthenticatedAdminRoutes, callback, data, options.reqUid, null); From 556d816ab43872529ecad4a7b38f9f8f4b4cdefe Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Fri, 19 Jan 2024 19:38:00 +0100 Subject: [PATCH 2/4] VLTCLT-39: Test getAccounts --- tests/unit/getAccounts.js | 100 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 tests/unit/getAccounts.js diff --git a/tests/unit/getAccounts.js b/tests/unit/getAccounts.js new file mode 100644 index 00000000..1b5bda78 --- /dev/null +++ b/tests/unit/getAccounts.js @@ -0,0 +1,100 @@ +/* eslint-disable operator-linebreak */ +'use strict'; // eslint-disable-line + +const assert = require('assert'); +const IAMClient = require('../../lib/IAMClient'); + +const accountIds = ['accountId1', 'accountId2']; +const canonicalIds = ['canId1', 'canId2']; +const emailAddresses = ['email1', 'email2']; +const accountNames = ['name1', 'name2']; +const opt = { reqUid: 'test.getAccounts.reqUid' }; +const mockCB = () => {}; + +const expectedData = { + Action: 'GetAccounts', + Version: '2010-05-08', +}; + +describe('getAccounts', () => { + let client; + let spyArg = null; + + beforeEach('spy on request', done => { + client = new IAMClient('127.0.0.1', 8500); + client.request = function spy(...args) { + spyArg = args; + }; + done(); + }); + + afterEach('reset spyArg', () => { spyArg = null; }); + + describe('should send request with regular function signature (5 args)', () => { + [ + { name: 'accountIds', args: [accountIds, null, null, opt, mockCB] }, + { name: 'emailAddresses', args: [null, emailAddresses, null, opt, mockCB] }, + { name: 'canonicalIds', args: [null, null, canonicalIds, opt, mockCB] }, + ].forEach(({ name, args }) => it(`for ${name}`, () => { + client.getAccounts(...args); + const [method, path, auth, cb, data, reqUid, contentType] = spyArg; + assert.strictEqual(method, 'GET'); + assert.strictEqual(path, '/'); + assert.strictEqual(auth, false); + assert.strictEqual(cb, mockCB); + assert.strictEqual(reqUid, opt.reqUid); + assert.strictEqual(contentType, null); + assert.deepStrictEqual( + data, + Object.assign({}, expectedData, { [name]: args[0] || args[1] || args[2] }), + ); + })); + }); + + describe('should send request with new function signature (3args)', () => { + [ + { name: 'accountIds', args: [{ accountIds }, opt, mockCB] }, + { name: 'emailAddresses', args: [{ emailAddresses }, opt, mockCB] }, + { name: 'canonicalIds', args: [{ canonicalIds }, opt, mockCB] }, + { name: 'accountNames', args: [{ accountNames }, opt, mockCB] }, + ].forEach(({ name, args }) => it(`for ${name}`, () => { + client.getAccounts(...args); + const [method, path, auth, cb, data, reqUid, contentType] = spyArg; + assert.strictEqual(method, 'GET'); + assert.strictEqual(path, '/'); + assert.strictEqual(auth, false); + assert.strictEqual(cb, mockCB); + assert.strictEqual(reqUid, opt.reqUid); + assert.strictEqual(contentType, null); + assert.deepStrictEqual( + data, + Object.assign({}, expectedData, { [name]: Object.values(args[0])[0] }), + ); + })); + }); + + describe('should throw with regular function signature (5args)', () => { + it('for all values set', () => { + assert.throws(() => client.getAccounts(accountIds, emailAddresses, canonicalIds, opt, mockCB), + assert.AssertionError); + }); + }); + + describe('should throw with new function signature (3args)', () => { + it('for all values set', () => { + assert.throws(() => client.getAccounts( + { + accountIds, emailAddresses, canonicalIds, accountNames, + }, + opt, + mockCB, + ), + assert.AssertionError); + }); + + it('for wrong argument type', () => { + assert.throws(() => client.getAccounts(accountIds, opt, mockCB), + assert.AssertionError); + }); + }); +}); From f0c3aea630bf1151d1443a7839129a0782714fe4 Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Tue, 23 Jan 2024 13:40:40 +0100 Subject: [PATCH 3/4] VLTCLT-39: Update getAccounts function signature Use first arg for accountNames instead of overloading the function signature --- lib/IAMClient.js | 80 ++++++++++----------------------------- tests/unit/getAccounts.js | 46 ++-------------------- 2 files changed, 24 insertions(+), 102 deletions(-) diff --git a/lib/IAMClient.js b/lib/IAMClient.js index bdceca4c..975ba164 100644 --- a/lib/IAMClient.js +++ b/lib/IAMClient.js @@ -454,75 +454,41 @@ class VaultClient { } /** - * Get accounts using account ids, canonical ids or email addresses + * Get accounts using account ids or names, canonical ids or email addresses * - * @overload - * @param {array|undefined} accountIds - Account ids, exclusive with - * emailAddresses and canonicalIds + * @param {array|undefined} accounts - Account ids or names (depending on options.accountNames), + * exclusive with emailAddresses and canonicalIds * @param {array|undefined} emailAddresses - Email addresses, exclusive - * with account ids and canonicalIds + * with account ids or names and canonicalIds * @param {array|undefined} canonicalIds - Canonical ids, exclusive with - * account ids and emailAddresses + * account ids or names and emailAddresses * @param {object} options - Options * @param {string} [options.reqUid] - Request uid + * @param {boolean} [options.accountNames] - Flag to consider first arg `accounts` + * as `accountNames` instead of `accountIds` * @param {function} callback - Callback(err, result) * @return {undefined} */ - // eslint-disable valid-jsdoc - /** - * Get accounts using account ids, canonical ids, email addresses - * or account names - * - * @overload - New signature that adds filter accountNames - * @param {object} filter - Contains one of the account filters - * @param {array|undefined} [filter.accountIds] - Account ids, exclusive with - * emailAddresses, canonicalIds and accountNames - * @param {array|undefined} [filter.emailAddresses] - Email addresses, exclusive - * with account ids, canonicalIds and accountNames - * @param {array|undefined} [filter.canonicalIds] - Canonical ids, exclusive with - * account ids, emailAddresses and accountNames - * @param {array|undefined} [filter.accountNames] - Account Names, exclusive with - * account ids, emailAddresses and canonicalIds. Do not use with vault2. - * @param {object} options - Options - * @param {string} [options.reqUid] - Request uid - * @param {function} callback - Callback(err, result) - * @return {undefined} - */ - // eslint-enable valid-jsdoc - getAccounts(accountIds, emailAddresses, canonicalIds, options, callback) { - let accountNames; - if (arguments.length === 3) { - // check and convert args from signature getAccounts(filter, options, callback) - const filter = accountIds; - /* eslint-disable-next-line no-param-reassign */ - options = emailAddresses; - /* eslint-disable-next-line no-param-reassign */ - callback = canonicalIds; - assert(filter && typeof filter === 'object' && !Array.isArray(filter), - 'first arg (filter) must be an object containing one filter field'); - assert(options && typeof options === 'object' && !Array.isArray(options), - 'second arg (options) must be an object'); - assert(callback && typeof callback === 'function', - 'third arg (callback) must be a function'); - ({ - /* eslint-disable-next-line no-param-reassign */ - accountIds, emailAddresses, canonicalIds, accountNames - } = filter); - } - assert((accountIds && Array.isArray(accountIds)) || !accountIds, - 'accountIds should be an array'); + getAccounts(accounts, emailAddresses, canonicalIds, options, callback) { + assert((accounts && Array.isArray(accounts)) || !accounts, + 'accounts should be an array'); assert((emailAddresses && Array.isArray(emailAddresses)) || !emailAddresses, 'emailAddresses should be an array'); assert((canonicalIds && Array.isArray(canonicalIds)) || !canonicalIds, 'canonicalIds should be an array'); - assert((accountNames && Array.isArray(accountNames)) || !accountNames, - 'accountNames should be an array'); + if ( + (accounts && (emailAddresses || canonicalIds)) + || (emailAddresses && (accounts || canonicalIds)) + || (canonicalIds && (accounts || emailAddresses))) { + assert(false, 'accounts, emailAddresses and canonicalIds ' + + 'ids are exclusive'); + } const data = { Action: 'GetAccounts', Version: '2010-05-08', }; - if (accountIds) { - data.accountIds = accountIds; + if (accounts) { + data[options.accountNames ? 'accountNames' : 'accountIds'] = accounts; } if (canonicalIds) { data.canonicalIds = canonicalIds; @@ -530,13 +496,7 @@ class VaultClient { if (emailAddresses) { data.emailAddresses = emailAddresses; } - if (accountNames) { - data.accountNames = accountNames; - } - if (Object.values(data).length > 3) { - assert(false, 'accountIds, emailAddresses, canonicalIds ' - + 'and accountNames ids are exclusive'); - } + const verb = this.useAuthenticatedAdminRoutes ? 'POST' : 'GET'; this.request(verb, '/', this.useAuthenticatedAdminRoutes, callback, data, options.reqUid, null); diff --git a/tests/unit/getAccounts.js b/tests/unit/getAccounts.js index 1b5bda78..e4dce2ad 100644 --- a/tests/unit/getAccounts.js +++ b/tests/unit/getAccounts.js @@ -9,6 +9,7 @@ const canonicalIds = ['canId1', 'canId2']; const emailAddresses = ['email1', 'email2']; const accountNames = ['name1', 'name2']; const opt = { reqUid: 'test.getAccounts.reqUid' }; +const optAccountNames = Object.assign({}, opt, { accountNames: true }); const mockCB = () => {}; const expectedData = { @@ -30,11 +31,12 @@ describe('getAccounts', () => { afterEach('reset spyArg', () => { spyArg = null; }); - describe('should send request with regular function signature (5 args)', () => { + describe('should send request with correct arguments', () => { [ { name: 'accountIds', args: [accountIds, null, null, opt, mockCB] }, { name: 'emailAddresses', args: [null, emailAddresses, null, opt, mockCB] }, { name: 'canonicalIds', args: [null, null, canonicalIds, opt, mockCB] }, + { name: 'accountNames', args: [accountNames, null, null, optAccountNames, mockCB] }, ].forEach(({ name, args }) => it(`for ${name}`, () => { client.getAccounts(...args); const [method, path, auth, cb, data, reqUid, contentType] = spyArg; @@ -51,50 +53,10 @@ describe('getAccounts', () => { })); }); - describe('should send request with new function signature (3args)', () => { - [ - { name: 'accountIds', args: [{ accountIds }, opt, mockCB] }, - { name: 'emailAddresses', args: [{ emailAddresses }, opt, mockCB] }, - { name: 'canonicalIds', args: [{ canonicalIds }, opt, mockCB] }, - { name: 'accountNames', args: [{ accountNames }, opt, mockCB] }, - ].forEach(({ name, args }) => it(`for ${name}`, () => { - client.getAccounts(...args); - const [method, path, auth, cb, data, reqUid, contentType] = spyArg; - assert.strictEqual(method, 'GET'); - assert.strictEqual(path, '/'); - assert.strictEqual(auth, false); - assert.strictEqual(cb, mockCB); - assert.strictEqual(reqUid, opt.reqUid); - assert.strictEqual(contentType, null); - assert.deepStrictEqual( - data, - Object.assign({}, expectedData, { [name]: Object.values(args[0])[0] }), - ); - })); - }); - - describe('should throw with regular function signature (5args)', () => { + describe('should throw with invalid arguments', () => { it('for all values set', () => { assert.throws(() => client.getAccounts(accountIds, emailAddresses, canonicalIds, opt, mockCB), assert.AssertionError); }); }); - - describe('should throw with new function signature (3args)', () => { - it('for all values set', () => { - assert.throws(() => client.getAccounts( - { - accountIds, emailAddresses, canonicalIds, accountNames, - }, - opt, - mockCB, - ), - assert.AssertionError); - }); - - it('for wrong argument type', () => { - assert.throws(() => client.getAccounts(accountIds, opt, mockCB), - assert.AssertionError); - }); - }); }); From f2bb1a1cce26f837ebfe9a74a657c2ebc77a6929 Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Fri, 19 Jan 2024 19:38:00 +0100 Subject: [PATCH 4/4] VLTCLT-39: Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index db3d05cb..6542ad1f 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "engines": { "node": ">=16" }, - "version": "7.10.13", + "version": "7.10.14", "description": "Client library and binary for Vault, the user directory and key management service", "main": "index.js", "repository": "scality/vaultclient",