Skip to content

UI: Add password field to static role creation page#30275

Merged
drivera258 merged 9 commits intomainfrom
ui/VAULT-35149/add-static-pass
Apr 21, 2025
Merged

UI: Add password field to static role creation page#30275
drivera258 merged 9 commits intomainfrom
ui/VAULT-35149/add-static-pass

Conversation

@drivera258
Copy link
Contributor

@drivera258 drivera258 commented Apr 16, 2025

Description

What does this PR do?

  • Adding password input field to static role creation / edit
    --- user is able to edit a set password as long as it hasn't been rotated yet
    --- user is unable to edit a password / password field is disabled once the role has been rotated
Screen.Recording.2025-04-17.at.6.51.42.PM.mov

TODO only if you're a HashiCorp employee

  • Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    of a public function, even if that change is in a CE file, double check that
    applying the patch for this PR to the ENT repo and running tests doesn't
    break any tests. Sometimes ENT only tests rely on public functions in CE
    files.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

@drivera258 drivera258 added this to the 1.20.0-rc milestone Apr 16, 2025
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Apr 16, 2025
@github-actions
Copy link

github-actions bot commented Apr 16, 2025

CI Results:
All Go tests succeeded! ✅

{{else}}
<InfoTableRow
@alwaysRender={{true}}
@alwaysRender={{(not (eq attr.name "password"))}}
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 is for conditionally hiding the password field on the details view, without removing it from the attrs

Not sure if there's a better way to do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't really need to do this. If it's not returned from the API then it shouldn't show. I suspect we're solving the issue of state problems here, is that correct (e.g. this field is removed on refresh of the details view)? If that's the case, let's do a follow up PR to address this issue.

Copy link
Contributor Author

@drivera258 drivera258 Apr 21, 2025

Choose a reason for hiding this comment

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

It still shows because its part of the attrs array of a role,
but because the API doesn't return a value, it'll just be blank so it looks like this (even on refresh):

image

i can remove this if we're okay with showing the password like that^

<ReadonlyFormField @attr={{attr}} @value={{get @model attr.name}} />
{{/if}}
{{! disable password field on edit once password has been rotated }}
{{else if (and (eq attr.name "password") (not (eq (get @model "last_vault_rotation") undefined)))}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if last_vault_rotation is present, then the password can't be edited, ie. read-only field

when the role hasnt been rotated yet, 'last_vault_rotation' isn't returned in response and will render as normal

@drivera258 drivera258 marked this pull request as ready for review April 17, 2025 17:01
@drivera258 drivera258 requested review from a team as code owners April 17, 2025 17:01
@drivera258 drivera258 requested review from clemon and removed request for clemon April 17, 2025 17:01
@github-actions
Copy link

Build Results:
All builds succeeded! ✅

@@ -135,6 +137,12 @@ export default class RoleModel extends Model {
})
skip_import_rotation;

Copy link
Contributor

Choose a reason for hiding this comment

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

I would imagine that this field is actually not editType password. Instead, it should be sensitive—see example here. The difference is that password fields (only used once that I can see) is old and not really something we should support in form-fields. Imagine a password field as something you login with. Sensitive fields by contrast are masked as you input them but allow you to toggle the view which is helpful when trying to set something.

<ReadonlyFormField @attr={{attr}} @value={{get @model attr.name}} />
{{/if}}
{{else if (and (eq @mode "edit") (eq attr.name "password"))}}
{{! password field is disabled on edit once password has been rotated }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you move this comment to the line right above @disabled so it's a tad clearer what it's referring to. Upon first read if reads as if this comment is relevant to the use of EnableInput.


* @param {object} [attr] - used to generate label for `ReadonlyFormField`, `name` key is required. Can be an attribute from a model exported with expandAttributeMeta.
* @param {string} [label] - required if no attr passed. Used to ensure a11y conformance for the readonly input.
* @param {boolean} [disabled] - to be used in specific scenarios where a user can visually see but not interact with the input field. ie. disabling a field on edit
Copy link
Contributor

Choose a reason for hiding this comment

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

To indicate it's false by default in the docs you can do something like

Suggested change
* @param {boolean} [disabled] - to be used in specific scenarios where a user can visually see but not interact with the input field. ie. disabling a field on edit
* @param {boolean} [disabled=false] - to be used in specific scenarios where a user can visually see but not interact with the input field. ie. disabling a field on edit

await render(hbs`<DatabaseRoleEdit @model={{this.modelStatic}} @mode="create"/>`);
await fillIn('[data-test-ttl-value="Rotation period"]', '2');
await click('[data-test-toggle-input="toggle-skip_import_rotation"]');
await fillIn('[data-test-input="password"]', 'testPassword'); // fill in password field
Copy link
Contributor

Choose a reason for hiding this comment

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

can you use the general selectors here? I know it's not used elsewhere, but moving forward, we are slowly (very slowly) trying to implement this pattern.


await render(hbs`<DatabaseRoleEdit @model={{this.modelStatic}} @mode="show"/>`);
assert.dom('[data-test-value-div="Rotate immediately"]').containsText('No');
assert.dom('[data-test-value="Password"]').doesNotExist(); // verify password field doesn't show on details view
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here, if these selectors already exists in general-selectors, replace them with that pattern.

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.

Left a couple of comments. Nice work!

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.

Nice work! Thank you.

@drivera258 drivera258 merged commit ceb9c6d into main Apr 21, 2025
33 checks passed
@drivera258 drivera258 deleted the ui/VAULT-35149/add-static-pass branch April 21, 2025 20:11
Erfankam pushed a commit to Erfankam/vault that referenced this pull request Sep 1, 2025
* adding password to static roles

* adding check for password rotation to disable password edit

* update field type and tests

* adding changelog

* replacing readonly with enableinput, added disable arg, test updates

* update to unless

* PR comments
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