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

Mask obfuscated Secret sync create/edit fields #27348

Merged
merged 10 commits into from
Jun 18, 2024

Conversation

Monkeychip
Copy link
Contributor

@Monkeychip Monkeychip commented Jun 4, 2024

This PR masks while typing the obfuscated fields on the destinations create/edit form for Secrets sync. Screenshots for the changes are included on the file changes view.

Notes:

  • Not backporting to 1.17.x. This is a feature and too many changes to safely backport.

  • GCP json credentials field is the only param using the enable-input wrap and not the masked-input because it's not a simple one-to-one replacement and enable-input is doing a great job at handling it's use case.

  • Enterprise test pass

  • Tested that patch still works as expected. There used to be an issue where we'd send ** values if a obfuscated field had been changed previously and then not changed again. I will link to the internal JIRA ticket within slack.

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jun 4, 2024
Copy link

github-actions bot commented Jun 4, 2024

CI Results:
All Go tests succeeded! ✅

@@ -247,7 +247,7 @@
<MaskedInput
@name={{@attr.name}}
@value={{or (get @model this.valuePath) @attr.options.defaultValue}}
@allowCopy="true"
@allowCopy={{if @attr.options.noCopy false true}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should default to true. not ideal (rather it default to false) but working with an old pattern and trying to keep backporting/scope simple.

@Monkeychip Monkeychip changed the title WIP: masked input on access token GH destinations WIP: masked input on all obfuscated Secret sync create/edit fields Jun 11, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CREATE:
image

EDIT:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CREATE:
image

EDIT:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CREATE:
image

EDIT:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CREATE:
image

EDIT:
image

@Monkeychip Monkeychip modified the milestones: 1.17.1, 1.18.0-rc Jun 18, 2024
@Monkeychip Monkeychip marked this pull request as ready for review June 18, 2024 16:28
@Monkeychip Monkeychip requested a review from a team as a code owner June 18, 2024 16:28
@Monkeychip Monkeychip changed the title WIP: masked input on all obfuscated Secret sync create/edit fields Mask obfuscated Secret sync create/edit fields Jun 18, 2024
Copy link

Build Results:
All builds succeeded! ✅

Copy link
Contributor

@hashishaw hashishaw left a comment

Choose a reason for hiding this comment

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

Nice work, and elegant solution! 🚢

@@ -32,7 +32,7 @@
aria-label={{or @name "masked input"}}
{{on "change" this.onChange}}
{{on "keyup" (fn this.handleKeyUp @name)}}
data-test-textarea
data-test-textarea={{or @name ""}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need or if the default is an empty string

Suggested change
data-test-textarea={{or @name ""}}
data-test-textarea={{@name}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that as well, but without the conditional it broke any test selector that didn't pass in a name. I suspect because it comes in as undefined?

@@ -131,6 +131,28 @@ module('Integration | Component | sync | Secrets::Page::Destinations::CreateAndE
await click(PAGE.saveButton);
});

test('edit: payload only contains masked inputs when they have changed', async function (assert) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

@Monkeychip Monkeychip merged commit 66e78db into main Jun 18, 2024
52 of 56 checks passed
@Monkeychip Monkeychip deleted the ui/VAULT-27324/add-gh-destination-secrets-sync branch June 18, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants