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.

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

UI: glimmerize masked input #20431

Merged
merged 6 commits into from
May 1, 2023
Merged

UI: glimmerize masked input #20431

merged 6 commits into from
May 1, 2023

Conversation

hashishaw
Copy link
Contributor

This PR glimmerizes the MaskedInput component, and adds an allowDownload attribute which adds a download button next to the copy button. As part of this update, the parent component must set the value in the onChange callback rather than the component updating the passed argument (which is deprecated behavior)

allowDownload has been enabled for KV V2 secret engine data, which fixes #6364

Screenshot 2023-04-28 at 2 46 15 PM

@hashishaw hashishaw added the ui label Apr 28, 2023
@hashishaw hashishaw added this to the 1.14 milestone Apr 28, 2023
@hashishaw hashishaw marked this pull request as ready for review April 28, 2023 20:14
constructor() {
super(...arguments);
if (!this.args.onChange && !this.args.displayOnly) {
debug('onChange is required for editable Masked Input!');
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

assert.dom('.masked-value').hasClass('masked-font');
test('it renders a download button when allowDownload is true', async function (assert) {
await render(hbs`<MaskedInput @allowDownload={{true}} />`);
assert.ok(component.downloadButtonIsPresent);
});

test('it shortens all outputs when displayOnly and masked', async function (assert) {
this.set('value', '123456789-123456789-123456789');
Copy link
Contributor

Choose a reason for hiding this comment

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

thank you for updating the tests. an easy miss when you glimmerize a component.

Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

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

Very very cool. Thank you for tackling. I'll go ahead and update the glimmerize document and assign you to the jira ticket from the glimmer epic.

@hashishaw hashishaw force-pushed the ui-glimmerize-masked-input branch from 339084f to 163aa61 Compare May 1, 2023 15:27
@hashishaw hashishaw merged commit b5e82f5 into main May 1, 2023
@hashishaw hashishaw deleted the ui-glimmerize-masked-input branch May 1, 2023 16:43
@Noobgam
Copy link

Noobgam commented Sep 16, 2023

Hello folks, for those not super familiar with Vault configuration, is there an option to opt out of this feature?

I've noticed that in our case people just tend to missclick where they expected to see the copy button, so that's some learning curve after upgrading for a feature we're not using at all

@dhumphries-sainsburys
Copy link

We are also interested in disabling the download button as it downloads the secrets in plaintext and we have a lot of contractors who we can't trust to misplace these plaintext files

@andyspiers
Copy link

andyspiers commented Sep 20, 2023

This is an undesirable and surprising feature IMO.

One click and you have downloaded a sensitive value to your local disk unencrypted.

Of course it's possible to view/copy and then save locally but that requires a specific action whereas this is easy to do by accident, almost without noticing.

It would be great to be able to disable this please, or at least have some kind of UI warning like "are you sure you want to download this data unencrypted"?

@hellobontempo
Copy link
Contributor

Thank you for the feedback on this. At this time, we do not have a way for operators to disable/enable specific UI features (I have added the request above to our internal planning for UI specific configurations). To more immediately resolve this issue we've added a warning prior to downloading to prevent accidental downloads. This will be released in 1.14.4 and 1.15.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: Download button on each KV row
6 participants