From 013ef133ea1626b8422e4275008b354c141f75f9 Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Fri, 17 May 2019 17:02:13 -0500 Subject: [PATCH 1/6] upgrade yargs-parser for better quote handling --- ui/package.json | 2 +- ui/yarn.lock | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/ui/package.json b/ui/package.json index 8ebf5d8afc8e..ce4735ec09f3 100644 --- a/ui/package.json +++ b/ui/package.json @@ -127,7 +127,7 @@ "text-encoder-lite": "1.0.0", "walk-sync": "^0.3.3", "xstate": "^3.3.3", - "yargs-parser": "^10.0.0" + "yargs-parser": "^13.0.0" }, "optionalDependencies": { "@babel/core": "^7.3.4", diff --git a/ui/yarn.lock b/ui/yarn.lock index 41549cff2b97..09611b1730a6 100644 --- a/ui/yarn.lock +++ b/ui/yarn.lock @@ -6488,7 +6488,7 @@ decamelize-keys@^1.0.0: decamelize "^1.1.0" map-obj "^1.0.0" -decamelize@^1.1.0, decamelize@^1.1.1, decamelize@^1.1.2: +decamelize@^1.1.0, decamelize@^1.1.1, decamelize@^1.1.2, decamelize@^1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/decamelize/-/decamelize-1.2.0.tgz#f6534d15148269b20352e7bee26f501f9a191290" integrity sha1-9lNNFRSCabIDUue+4m9QH5oZEpA= @@ -18775,13 +18775,21 @@ yam@^0.0.24: fs-extra "^4.0.2" lodash.merge "^4.6.0" -yargs-parser@^10.0.0, yargs-parser@^10.1.0: +yargs-parser@^10.1.0: version "10.1.0" resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-10.1.0.tgz#7202265b89f7e9e9f2e5765e0fe735a905edbaa8" integrity sha512-VCIyR1wJoEBZUqk5PA+oOBF6ypbwh5aNB3I50guxAL/quggdfs4TtNHQrSazFA3fYZ+tEqfs0zIGlv0c/rgjbQ== dependencies: camelcase "^4.1.0" +yargs-parser@^13.0.0: + version "13.1.0" + resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-13.1.0.tgz#7016b6dd03e28e1418a510e258be4bff5a31138f" + integrity sha512-Yq+32PrijHRri0vVKQEm+ys8mbqWjLiwQkMFNXEENutzLPP0bE4Lcd4iA3OQY5HF+GD3xXxf0MEHb8E4/SA3AA== + dependencies: + camelcase "^5.0.0" + decamelize "^1.2.0" + yargs-parser@^5.0.0: version "5.0.0" resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-5.0.0.tgz#275ecf0d7ffe05c77e64e7c86e4cd94bf0e1228a" From c7612370227e344320444978da09e759de6fd0de Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Fri, 17 May 2019 17:03:40 -0500 Subject: [PATCH 2/6] remove encoding pre&post parse, and remove wrapping quotes when pushing to data array --- ui/app/lib/console-helpers.js | 17 ++++++++++----- ui/app/utils/args-tokenizer.js | 38 ---------------------------------- 2 files changed, 12 insertions(+), 43 deletions(-) delete mode 100644 ui/app/utils/args-tokenizer.js diff --git a/ui/app/lib/console-helpers.js b/ui/app/lib/console-helpers.js index c0cb651f7496..eae59a2dfb4d 100644 --- a/ui/app/lib/console-helpers.js +++ b/ui/app/lib/console-helpers.js @@ -56,14 +56,11 @@ export function executeUICommand(command, logAndOutput, clearLog, toggleFullscre } export function parseCommand(command, shouldThrow) { - // encode everything but spaces - let cmd = encodeURIComponent(command).replace(/%20/g, decodeURIComponent); - let args = argTokenizer(cmd); + let args = argTokenizer(command); if (args[0] === 'vault') { args.shift(); } - args = args.map(decodeURIComponent); let [method, ...rest] = args; let path; let flags = []; @@ -74,7 +71,17 @@ export function parseCommand(command, shouldThrow) { flags.push(arg); } else { if (path) { - data.push(arg); + let strippedArg = arg + // we'll have arg=something or arg="lol I need spaces", so need to split on the first = + .split(/=(.+)/) + // remove " at the beginning and end of each item + .map(item => item.replace(/^"|"$/gi, '')) + // if there were quotes, there's an empty string as the last member in the array that we don't want, + // so filter it out + .filter(str => str !== '') + // glue the data back together + .join('='); + data.push(strippedArg); } else { path = arg; } diff --git a/ui/app/utils/args-tokenizer.js b/ui/app/utils/args-tokenizer.js deleted file mode 100644 index c77b0b531bac..000000000000 --- a/ui/app/utils/args-tokenizer.js +++ /dev/null @@ -1,38 +0,0 @@ -export default function(argString) { - if (Array.isArray(argString)) return argString; - - argString = argString.trim(); - - var i = 0; - var prevC = null; - var c = null; - var opening = null; - var args = []; - - for (var ii = 0; ii < argString.length; ii++) { - prevC = c; - c = argString.charAt(ii); - - // split on spaces unless we're in quotes. - if (c === ' ' && !opening) { - if (!(prevC === ' ')) { - i++; - } - continue; - } - - // don't split the string if we're in matching - // opening or closing single and double quotes. - if (c === opening) { - if (!args[i]) args[i] = ''; - opening = null; - } else if ((c === "'" || c === '"') && argString.indexOf(c, ii + 1) > 0 && !opening) { - opening = c; - } - - if (!args[i]) args[i] = ''; - args[i] += c; - } - - return args; -} From 0a76f992e504378b6dbad818ce8bfdd9ab7ae67e Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Fri, 17 May 2019 17:42:56 -0500 Subject: [PATCH 3/6] add test for spaces and strings --- ui/tests/unit/lib/console-helpers-test.js | 27 +++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/ui/tests/unit/lib/console-helpers-test.js b/ui/tests/unit/lib/console-helpers-test.js index 53c2e8b595f9..3ba448fd2585 100644 --- a/ui/tests/unit/lib/console-helpers-test.js +++ b/ui/tests/unit/lib/console-helpers-test.js @@ -26,6 +26,33 @@ module('Unit | Lib | console helpers', function() { ], ], }, + { + name: 'write with space in a value', + command: `vault write \ + auth/ldap/config \ + url=ldap://ldap.example.com:3268 \ + binddn="CN=ServiceViewDev,OU=Service Accounts,DC=example,DC=com" \ + bindpass="xxxxxxxxxxxxxxxxxxxxxxxxxx" \ + userdn="DC=example,DC=com" \ + groupdn="DC=example,DC=com" \ + insecure_tls=true \ + starttls=false + `, + expected: [ + 'write', + [], + 'auth/ldap/config', + [ + 'url=ldap://ldap.example.com:3268', + 'binddn=CN=ServiceViewDev,OU=Service Accounts,DC=example,DC=com', + 'bindpass=xxxxxxxxxxxxxxxxxxxxxxxxxx', + 'userdn=DC=example,DC=com', + 'groupdn=DC=example,DC=com', + 'insecure_tls=true', + 'starttls=false', + ], + ], + }, { name: 'read with field', command: `vault read -field=access_key aws/creds/my-role`, From e11fb01a0e258e0e7b97bdd8bbe169580110b9c7 Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Fri, 17 May 2019 17:43:45 -0500 Subject: [PATCH 4/6] base64 encode policy strings in tests where we're using them with string interpolation --- ui/tests/acceptance/cluster-test.js | 10 +++---- .../enterprise-control-groups-test.js | 12 ++++----- ui/tests/acceptance/policies/index-test.js | 2 +- .../secrets/backend/kv/secret-test.js | 26 +++++++++---------- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/ui/tests/acceptance/cluster-test.js b/ui/tests/acceptance/cluster-test.js index 8518405673c0..5c1aefeafb30 100644 --- a/ui/tests/acceptance/cluster-test.js +++ b/ui/tests/acceptance/cluster-test.js @@ -9,7 +9,7 @@ const consoleComponent = create(consoleClass); const tokenWithPolicy = async function(name, policy) { await consoleComponent.runCommands([ - `write sys/policies/acl/${name} policy=${policy}`, + `write sys/policies/acl/${name} policy=${btoa(policy)}`, `write -field=client_token auth/token/create policies=${name}`, ]); @@ -25,11 +25,11 @@ module('Acceptance | cluster', function(hooks) { }); test('hides nav item if user does not have permission', async function(assert) { - const deny_policies_policy = `' + const deny_policies_policy = ` path "sys/policies/*" { capabilities = ["deny"] }, - '`; + `; const userToken = await tokenWithPolicy('hide-policies-nav', deny_policies_policy); await logout.visit(); @@ -40,11 +40,11 @@ module('Acceptance | cluster', function(hooks) { }); test('enterprise nav item links to first route that user has access to', async function(assert) { - const read_rgp_policy = `' + const read_rgp_policy = ` path "sys/policies/rgp" { capabilities = ["read"] }, - '`; + `; const userToken = await tokenWithPolicy('show-policies-nav', read_rgp_policy); await logout.visit(); diff --git a/ui/tests/acceptance/enterprise-control-groups-test.js b/ui/tests/acceptance/enterprise-control-groups-test.js index e45e505a1c4c..68118600274d 100644 --- a/ui/tests/acceptance/enterprise-control-groups-test.js +++ b/ui/tests/acceptance/enterprise-control-groups-test.js @@ -27,7 +27,7 @@ module('Acceptance | Enterprise | control groups', function(hooks) { return logout.visit(); }); - const POLICY = `' + const POLICY = ` path "kv/foo" { capabilities = ["create", "read", "update", "delete", "list"] control_group = { @@ -40,9 +40,9 @@ module('Acceptance | Enterprise | control groups', function(hooks) { } } } - '`; + `; - const AUTHORIZER_POLICY = `' + const AUTHORIZER_POLICY = ` path "sys/control-group/authorize" { capabilities = ["update"] } @@ -50,7 +50,7 @@ module('Acceptance | Enterprise | control groups', function(hooks) { path "sys/control-group/request" { capabilities = ["update"] } - '`; + `; const ADMIN_USER = 'authorizer'; const ADMIN_PASSWORD = 'test'; @@ -67,8 +67,8 @@ module('Acceptance | Enterprise | control groups', function(hooks) { `write auth/userpass/users/${ADMIN_USER} password=${ADMIN_PASSWORD} policies=default`, `write identity/entity name=${ADMIN_USER} policies=test`, // write policies for control group + authorization - `write sys/policies/acl/kv-control-group policy=${POLICY}`, - `write sys/policies/acl/authorizer policy=${AUTHORIZER_POLICY}`, + `write sys/policies/acl/kv-control-group policy=${btoa(POLICY)}`, + `write sys/policies/acl/authorizer policy=${btoa(AUTHORIZER_POLICY)}`, // read out mount to get the accessor 'read -field=accessor sys/internal/ui/mounts/auth/userpass', ]); diff --git a/ui/tests/acceptance/policies/index-test.js b/ui/tests/acceptance/policies/index-test.js index 713b5dbf7258..85f127d49371 100644 --- a/ui/tests/acceptance/policies/index-test.js +++ b/ui/tests/acceptance/policies/index-test.js @@ -33,7 +33,7 @@ module('Acceptance | policies/acl', function(hooks) { test('it allows deletion of policies with dots in names', async function(assert) { const POLICY = 'path "*" { capabilities = ["list"]}'; let policyName = 'list.policy'; - await consoleComponent.runCommands([`write sys/policies/acl/${policyName} policy='${POLICY}'`]); + await consoleComponent.runCommands([`write sys/policies/acl/${policyName} policy=${btoa(POLICY)}`]); await page.visit({ type: 'acl' }); let policy = page.row.filterBy('name', policyName)[0]; assert.ok(policy, 'policy is shown in the list'); diff --git a/ui/tests/acceptance/secrets/backend/kv/secret-test.js b/ui/tests/acceptance/secrets/backend/kv/secret-test.js index ea2925c16a48..860fed070738 100644 --- a/ui/tests/acceptance/secrets/backend/kv/secret-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/secret-test.js @@ -192,17 +192,17 @@ module('Acceptance | secrets/secret/create', function(hooks) { test('version 2 with restricted policy still allows creation', async function(assert) { let backend = 'kv-v2'; - const V2_POLICY = `' + const V2_POLICY = ` path "kv-v2/metadata/*" { capabilities = ["list"] } path "kv-v2/data/secret" { capabilities = ["create", "read", "update"] } - '`; + `; await consoleComponent.runCommands([ `write sys/mounts/${backend} type=kv options=version=2`, - `write sys/policies/acl/kv-v2-degrade policy=${V2_POLICY}`, + `write sys/policies/acl/kv-v2-degrade policy=${btoa(V2_POLICY)}`, // delete any kv previously written here so that tests can be re-run 'delete kv-v2/metadata/secret', 'write -field=client_token auth/token/create policies=kv-v2-degrade', @@ -220,17 +220,17 @@ module('Acceptance | secrets/secret/create', function(hooks) { test('version 2 with restricted policy still allows edit', async function(assert) { let backend = 'kv-v2'; - const V2_POLICY = `' + const V2_POLICY = ` path "kv-v2/metadata/*" { capabilities = ["list"] } path "kv-v2/data/secret" { capabilities = ["create", "read", "update"] } - '`; + `; await consoleComponent.runCommands([ `write sys/mounts/${backend} type=kv options=version=2`, - `write sys/policies/acl/kv-v2-degrade policy=${V2_POLICY}`, + `write sys/policies/acl/kv-v2-degrade policy=${btoa(V2_POLICY)}`, // delete any kv previously written here so that tests can be re-run 'delete kv-v2/metadata/secret', 'write -field=client_token auth/token/create policies=kv-v2-degrade', @@ -326,16 +326,16 @@ module('Acceptance | secrets/secret/create', function(hooks) { }); let setupNoRead = async function(backend, canReadMeta = false) { - const V2_WRITE_ONLY_POLICY = `' + const V2_WRITE_ONLY_POLICY = ` path "${backend}/+/+" { capabilities = ["create", "update", "list"] } path "${backend}/+" { capabilities = ["list"] } - '`; + `; - const V2_WRITE_WITH_META_READ_POLICY = `' + const V2_WRITE_WITH_META_READ_POLICY = ` path "${backend}/+/+" { capabilities = ["create", "update", "list"] } @@ -345,12 +345,12 @@ module('Acceptance | secrets/secret/create', function(hooks) { path "${backend}/+" { capabilities = ["list"] } - '`; - const V1_WRITE_ONLY_POLICY = `' + `; + const V1_WRITE_ONLY_POLICY = ` path "${backend}/+" { capabilities = ["create", "update", "list"] } - '`; + `; let policy; if (backend === 'kv-v2' && canReadMeta) { @@ -364,7 +364,7 @@ module('Acceptance | secrets/secret/create', function(hooks) { // disable any kv previously enabled kv `delete sys/mounts/${backend}`, `write sys/mounts/${backend} type=kv options=version=${backend === 'kv-v2' ? 2 : 1}`, - `write sys/policies/acl/${backend} policy=${policy}`, + `write sys/policies/acl/${backend} policy=${btoa(policy)}`, `write -field=client_token auth/token/create policies=${backend}`, ]); From b02978d1ccf9332e3f1c2be424d1cd2d481b32c9 Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Tue, 21 May 2019 11:29:44 -0500 Subject: [PATCH 5/6] improve regex to only remove wrapping single and double quotes --- ui/app/lib/console-helpers.js | 4 ++-- ui/tests/unit/lib/console-helpers-test.js | 24 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/ui/app/lib/console-helpers.js b/ui/app/lib/console-helpers.js index eae59a2dfb4d..f68e49462715 100644 --- a/ui/app/lib/console-helpers.js +++ b/ui/app/lib/console-helpers.js @@ -74,8 +74,8 @@ export function parseCommand(command, shouldThrow) { let strippedArg = arg // we'll have arg=something or arg="lol I need spaces", so need to split on the first = .split(/=(.+)/) - // remove " at the beginning and end of each item - .map(item => item.replace(/^"|"$/gi, '')) + // remove matched wrapping " or ' from each item + .map(item => item.replace(/^("|')(.+)(\1)$/, '$2')) // if there were quotes, there's an empty string as the last member in the array that we don't want, // so filter it out .filter(str => str !== '') diff --git a/ui/tests/unit/lib/console-helpers-test.js b/ui/tests/unit/lib/console-helpers-test.js index 3ba448fd2585..d3c7fafd1c4c 100644 --- a/ui/tests/unit/lib/console-helpers-test.js +++ b/ui/tests/unit/lib/console-helpers-test.js @@ -53,6 +53,30 @@ module('Unit | Lib | console helpers', function() { ], ], }, + { + name: 'write with double quotes', + command: `vault write \ + auth/token/create \ + policies="foo" + `, + expected: ['write', [], 'auth/token/create', ['policies=foo']], + }, + { + name: 'write with single quotes', + command: `vault write \ + auth/token/create \ + policies='foo' + `, + expected: ['write', [], 'auth/token/create', ['policies=foo']], + }, + { + name: 'write with unmatched quotes', + command: `vault write \ + auth/token/create \ + policies='foo + `, + expected: ['write', [], 'auth/token/create', ["policies='foo"]], + }, { name: 'read with field', command: `vault read -field=access_key aws/creds/my-role`, From c945dd047e0a41bcc659ca0c4d431170fbc8dd50 Mon Sep 17 00:00:00 2001 From: Matthew Irish Date: Tue, 21 May 2019 12:38:01 -0500 Subject: [PATCH 6/6] don't support quotes in paths in the web cli --- .../secrets/backend/kv/secret-test.js | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/ui/tests/acceptance/secrets/backend/kv/secret-test.js b/ui/tests/acceptance/secrets/backend/kv/secret-test.js index 860fed070738..7fb54e5ced4f 100644 --- a/ui/tests/acceptance/secrets/backend/kv/secret-test.js +++ b/ui/tests/acceptance/secrets/backend/kv/secret-test.js @@ -255,7 +255,7 @@ module('Acceptance | secrets/secret/create', function(hooks) { let paths = [ '(', ')', - '"', + //'"', //"'", '!', '#', @@ -290,21 +290,23 @@ module('Acceptance | secrets/secret/create', function(hooks) { } }); - // the web cli does not handle a single quote in a path, so we test it here via the UI - test('creating a secret with a single quote works properly', async function(assert) { + // the web cli does not handle a quote as part of a path, so we test it here via the UI + test('creating a secret with a single or double quote works properly', async function(assert) { await consoleComponent.runCommands('write sys/mounts/kv type=kv'); - let path = "'some"; - await listPage.visitRoot({ backend: 'kv' }); - await listPage.create(); - await editPage.createSecret(`${path}/2`, 'foo', 'bar'); - await listPage.visit({ backend: 'kv', id: path }); - assert.ok(listPage.secrets.filterBy('text', '2')[0], `${path}: secret is displayed properly`); - await listPage.secrets.filterBy('text', '2')[0].click(); - assert.equal( - currentRouteName(), - 'vault.cluster.secrets.backend.show', - `${path}: show page renders correctly` - ); + let paths = ["'some", '"some']; + for (let path of paths) { + await listPage.visitRoot({ backend: 'kv' }); + await listPage.create(); + await editPage.createSecret(`${path}/2`, 'foo', 'bar'); + await listPage.visit({ backend: 'kv', id: path }); + assert.ok(listPage.secrets.filterBy('text', '2')[0], `${path}: secret is displayed properly`); + await listPage.secrets.filterBy('text', '2')[0].click(); + assert.equal( + currentRouteName(), + 'vault.cluster.secrets.backend.show', + `${path}: show page renders correctly` + ); + } }); test('filter clears on nav', async function(assert) {