web-cli quote parsing #6755
Merged
web-cli quote parsing #6755
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
meirish
commented
May 17, 2019
ui/app/lib/console-helpers.js
Outdated
| // 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, '')) |
There was a problem hiding this comment.
🤔 maybe we should strip single quotes here too...
There was a problem hiding this comment.
updated this so it 1) also strips single quotes, and 2) will only do so if the quotes are matching
meirish
commented
May 17, 2019
| @@ -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)}`, | |||
There was a problem hiding this comment.
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 ✨
meirish
commented
May 17, 2019
| command: `vault write \ | ||
| auth/ldap/config \ | ||
| url=ldap://ldap.example.com:3268 \ | ||
| binddn="CN=ServiceViewDev,OU=Service Accounts,DC=example,DC=com" \ |
There was a problem hiding this comment.
The space in Service Accounts was what didn't work previously.
andaley
previously approved these changes
May 20, 2019
…ing interpolation
meirish
force-pushed
the
ui-cli-quote-parse
branch
from
fbdeb6a to
b02978d
Compare
meirish
force-pushed
the
ui-cli-quote-parse
branch
from
51a1bad to
c945dd0
Compare
andaley
approved these changes
May 22, 2019
meirish
added a commit
that referenced
this pull request
Jun 4, 2019
* 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously if there was a space in a quoted value, the web cli would parse that incorrectly and break on the space, causing a whole set of wrong values to be written as this would also change any values after it. Additionally, there was an issue when quoting items in the web cli - the quotes would get written as part of the value.
This PR should fix both of these issues.
NOTE: this removes support for using
"in paths in the web-cli, while this was nice, I think the use case of quoting attributes will be more common. Creating secrets from the GUI does still support"and'in paths if that is needed.