-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
Update marcellanz/transit_pkcs1v15 RSA encryption support #25486
Conversation
…without UI tests (yet).
…rt' into feature/transit_pkcs1v15_support
…sen by parameter.
Co-authored-by: claire bontempo <[email protected]>
Co-authored-by: claire bontempo <[email protected]>
Co-authored-by: claire bontempo <[email protected]>
Co-authored-by: Sarah Chavis <[email protected]>
Co-authored-by: Sarah Chavis <[email protected]>
Co-authored-by: Sarah Chavis <[email protected]>
Build Results: |
CI Results: |
…vault into sgm/pkcs15-padding-conflicts # Conflicts: # builtin/logical/transit/path_datakey.go # builtin/logical/transit/path_decrypt.go # builtin/logical/transit/path_encrypt.go # builtin/logical/transit/path_rewrap.go
- Fix padding scheme dropdown console error by adding values to the transit-key-actions.hbs - Populate both padding scheme drop down menus within rewrap, not just the one padding_scheme - Do not submit a padding_scheme value through POST for non-rsa keys
…_scheme - Map the appropriate API fields for the RSA padding scheme to the batch items within the rewrap API - Add the ability to create RSA keys within the encrypt API endpoint - Add test case for rewrap api that leverages the padding_scheme fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Thanks for tackling the UI side of this as well. If possible, a screenshot of this change in the UI is helpful for posterity.
Also, do you plan to add test coverage for this? I believe it'd be similar to the test written [here])https://github.com/hashicorp/vault/blob/main/ui/tests/integration/components/transit-key-actions-test.js#L69)
I'm also happy to write one if that'd be helpful! Otherwise, any of the data-test-
selectors that were added in the .hbs
files can be removed as they're not being used.
onchange={{action (mut @padding_scheme) value="target.value"}} | ||
> | ||
{{#each (array "oaep" "pkcs1v15") as |scheme|}} | ||
<option selected={{if @padding_scheme (eq @padding_scheme scheme) (eq scheme "oaep")}} value={{scheme}}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question here as above
> | ||
{{#each (array "oaep" "pkcs1v15") as |scheme|}} | ||
<option | ||
selected={{if @decrypt_padding_scheme (eq @decrypt_padding_scheme scheme) (eq scheme "oaep")}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment here about setting the default for decrypt_padding_scheme
in the transit-key-actions.js
file and simplifying this conditional to just {{eq @decrypt_padding_scheme scheme}}
Co-authored-by: claire bontempo <[email protected]>
This was a community created PR, and the author may not still be around to tackle UI testing/changes. If you have bandwidth feel free. |
- The data key api was using the incorrect parameter name for the padding scheme - Enforce that padding_scheme is only used on RSA keys, we are punting on supporting it for managed keys at the moment.
Make it compatible with recent versions of Vault's key policy engine.