Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

web-cli quote parsing #6755

Merged
merged 6 commits into from
May 22, 2019
Merged

web-cli quote parsing #6755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
013ef13
upgrade yargs-parser for better quote handling
meirish May 17, 2019
c761237
remove encoding pre&post parse, and remove wrapping quotes when pushi…
meirish May 17, 2019
0a76f99
add test for spaces and strings
meirish May 17, 2019
e11fb01
base64 encode policy strings in tests where we're using them with str…
meirish May 17, 2019
b02978d
improve regex to only remove wrapping single and double quotes
meirish May 21, 2019
c945dd0
don't support quotes in paths in the web cli
meirish May 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions ui/app/lib/console-helpers.js
View file
Open in desktop
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
Open in desktop

This file was deleted.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the changes in string handling, interpolation with multiple nested quotes didn't work properly any more for writing policies in tests. The work-around for this was to base64 encode the string prior to interpolation since vault's api supports writing policy in this way ✨

`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
Open in desktop
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
Open in desktop
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
58 changes: 30 additions & 28 deletions ui/tests/acceptance/secrets/backend/kv/secret-test.js
View file
Open in desktop
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 All @@ -326,16 +328,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"]
}
Expand All @@ -345,12 +347,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) {
Expand All @@ -364,7 +366,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}`,
]);

Expand Down
51 changes: 51 additions & 0 deletions ui/tests/unit/lib/console-helpers-test.js
View file
Open in desktop
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" \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The space in Service Accounts was what didn't work previously.

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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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"
Expand Down