Conversation
|
@jcrowthe if I can change something, just let me know :) |
jcrowthe
left a comment
jcrowthe
left a comment
There was a problem hiding this comment.
Hey, thanks for the fix. I've noted below an ordering issue.
To support older versions of Vault as well, would you modify this PR to make safer checks that identity_policies exists prior to each usage? With that added I will have no problem merging. Thanks!
app/elements/secrets-init.html
Outdated
| // Wait for all policy requests to return. Once complete, trigger _parseAccess() | ||
| if (this.loginResponse && this.loginResponse.policies && this.policyRequests.length == this.loginResponse.policies.length) { | ||
| var policies = this.loginResponse.policies.concat(this.loginResponse.identity_policies); | ||
| if (this.loginResponse && this.loginResponse.policies && this.policyRequests.length == policies.length) { |
There was a problem hiding this comment.
This checks for the existence of values prior to using them. The creation of a new variable policies happens prior to this check.
|
Good hint. I chaned some stuff to make it much more secure. |
jcrowthe
left a comment
jcrowthe
left a comment
There was a problem hiding this comment.
Thanks! Feel free to provide additional PR's if you find issues :)
2 participants
Proposed changes
It looks like tokens from the CLI/UI are not working as expected. After some investigations I figured out, that these token look a little bit diffrent to tokens generated by the LDAP / UserPass sigin in Crypter itself.
"External" tokens are not including the policies in
policies. They include the same values in theidentity_policiesinstead.The last changelogs (1.2.3 to to 1.3) did not include any information about a change in these. Also the documentation of the
identity_policiesis not very detailed.To fix this, I aggregated together
policyandidentity_policies. In my local tests this workin with the token as well with the existing userpass signin.If I missied some test etc. please just give me a hint,
Types of changes
What types of changes does your code introduce to Cryptr?
Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
N/A