Skip to content

Commit

Permalink
web-cli quote parsing (#6755)
Browse files Browse the repository at this point in the history 
* upgrade yargs-parser for better quote handling

* remove encoding pre&post parse, and remove wrapping quotes when pushing to data array

* add test for spaces and strings

* base64 encode policy strings in tests where we're using them with string interpolation

* improve regex to only remove wrapping single and double quotes

* don't support quotes in paths in the web cli
  • Loading branch information
@meirish
meirish committed Jun 4, 2019
1 parent 061330e commit 2eab169
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 79 deletions.
17 changes: 12 additions & 5 deletions ui/app/lib/console-helpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand All @@ -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 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 !== '')
// glue the data back together
.join('=');
data.push(strippedArg);
} else {
path = arg;
}
Expand Down
38 changes: 0 additions & 38 deletions ui/app/utils/args-tokenizer.js
View file

This file was deleted.

2 changes: 1 addition & 1 deletion ui/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,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",
Expand Down
10 changes: 5 additions & 5 deletions ui/tests/acceptance/cluster-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
]);

Expand All @@ -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();
Expand All @@ -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();
Expand Down
12 changes: 6 additions & 6 deletions ui/tests/acceptance/enterprise-control-groups-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -40,17 +40,17 @@ module('Acceptance | Enterprise | control groups', function(hooks) {
}
}
}
'`;
`;

const AUTHORIZER_POLICY = `'
const AUTHORIZER_POLICY = `
path "sys/control-group/authorize" {
capabilities = ["update"]
}
path "sys/control-group/request" {
capabilities = ["update"]
}
'`;
`;

const ADMIN_USER = 'authorizer';
const ADMIN_PASSWORD = 'test';
Expand All @@ -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',
]);
Expand Down
2 changes: 1 addition & 1 deletion ui/tests/acceptance/policies/index-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
44 changes: 23 additions & 21 deletions ui/tests/acceptance/secrets/backend/kv/secret-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -255,7 +255,7 @@ module('Acceptance | secrets/secret/create', function(hooks) {
let paths = [
'(',
')',
'"',
//'"',
//"'",
'!',
'#',
Expand Down Expand Up @@ -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) {
Expand Down
51 changes: 51 additions & 0 deletions ui/tests/unit/lib/console-helpers-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,57 @@ 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: '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`,
Expand Down
12 changes: 10 additions & 2 deletions ui/yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6210,7 +6210,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=
Expand Down Expand Up @@ -17979,13 +17979,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"
Expand Down

0 comments on commit 2eab169

Please sign in to comment.