Skip to content

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

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.

Sign up for GitHub

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

Merged
drivera258 merged 9 commits into from
Apr 21, 2025
Merged

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8972d77
adding password to static roles
drivera258 Apr 15, 2025
68d2d07
adding check for password rotation to disable password edit
drivera258 Apr 16, 2025
f26fab5
Merge branch 'main' into ui/VAULT-35149/add-static-pass
drivera258 Apr 16, 2025
4de0211
update field type and tests
drivera258 Apr 16, 2025
db752b8
adding changelog
drivera258 Apr 17, 2025
0657a9a
replacing readonly with enableinput, added disable arg, test updates
drivera258 Apr 17, 2025
b50c6e0
update to unless
drivera258 Apr 18, 2025
a5ed769
Merge branch 'main' into ui/VAULT-35149/add-static-pass
drivera258 Apr 21, 2025
cc45fb9
PR comments
drivera258 Apr 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/30275.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui/database: Adding password input field for creating a static role
```
2 changes: 1 addition & 1 deletion ui/app/components/database-role-edit.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
/>
{{else}}
<InfoTableRow
@alwaysRender={{true}}
@alwaysRender={{(not (eq attr.name "password"))}}
Copy link
Contributor Author

@drivera258 drivera258 Apr 17, 2025

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

@Monkeychip Monkeychip Apr 21, 2025

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 •
edited
Loading

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^

@defaultShown={{defaultDisplay}}
@label={{capitalize (or attr.options.label (humanize (dasherize attr.name)))}}
@value={{get @model attr.name}}
Expand Down
13 changes: 12 additions & 1 deletion ui/app/models/database/role.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ export default class RoleModel extends Model {
})
revocation_statement;

@attr('string', { readOnly: true }) last_vault_rotation;

// ENTERPRISE ONLY
@attr({
label: 'Rotate immediately',
Expand All @@ -135,6 +137,12 @@ export default class RoleModel extends Model {
})
skip_import_rotation;

Copy link
Contributor

@Monkeychip Monkeychip Apr 21, 2025

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.

@attr('string', {
editType: 'password',
subText: 'The database password that this Vault role corresponds to.',
})
password;

/* FIELD ATTRIBUTES */
get fieldAttrs() {
// Main fields on edit/create form
Expand All @@ -156,6 +164,7 @@ export default class RoleModel extends Model {
'default_ttl',
'max_ttl',
'username',
'password',
'rotation_period',
'skip_import_rotation',
'creation_statements',
Expand All @@ -169,7 +178,9 @@ export default class RoleModel extends Model {

// remove enterprise-only attrs if on community
if (!this.version.isEnterprise) {
allRoleSettingFields = allRoleSettingFields.filter((role) => role !== 'skip_import_rotation');
allRoleSettingFields = allRoleSettingFields.filter(
(role) => !['skip_import_rotation', 'password'].includes(role)
);
}

return expandAttributeMeta(this, allRoleSettingFields);
Expand Down
10 changes: 10 additions & 0 deletions ui/app/templates/components/database-role-setting-form.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@
{{else}}
<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

@Monkeychip Monkeychip Apr 21, 2025

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.

<EnableInput
data-test-enable-field={{attr.name}}
class="field"
@attr={{attr}}
@disabled={{(not (eq (get @model "last_vault_rotation") undefined))}}
>
<FormField @attr={{attr}} @model={{@model}} />
</EnableInput>
{{else}}
<FormField data-test-field={{true}} @attr={{attr}} @model={{@model}} @modelValidations={{@modelValidations}} />
{{#if (and (eq attr.name "skip_import_rotation") this.isOverridden)}}
Expand Down
2 changes: 1 addition & 1 deletion ui/app/utils/model-helpers/database-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export const AVAILABLE_PLUGIN_TYPES = [
];

export const ROLE_FIELDS = {
static: ['username', 'rotation_period', 'skip_import_rotation'],
static: ['username', 'password', 'rotation_period', 'skip_import_rotation'],
dynamic: ['default_ttl', 'max_ttl'],
};

Expand Down
20 changes: 11 additions & 9 deletions ui/lib/core/addon/components/enable-input.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@
<Input aria-label={{@label}} readonly class="input" @type="text" @value="**********" />
{{/if}}
</div>
<div class="align-self-end">
<Hds::Button
@text="Enable input"
@icon="edit"
@isIconOnly={{true}}
@color="tertiary"
{{on "click" (fn (mut this.enable))}}
/>
</div>
{{#unless @disabled}}
<div class="align-self-end">
<Hds::Button
@text="Enable input"
@icon="edit"
@isIconOnly={{true}}
@color="tertiary"
{{on "click" (fn (mut this.enable))}}
/>
</div>
{{/unless}}
</div>
{{/if}}
2 changes: 2 additions & 0 deletions ui/lib/core/addon/components/enable-input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { tracked } from '@glimmer/tracking';
interface Args {
attr?: AttrData;
label?: string;
disabled?: boolean; // specifically used for disabling on edit
}
interface AttrData {
name: string; // required if @attr is passed
Expand Down Expand Up @@ -38,6 +39,7 @@ interface AttrData {

* @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

@Monkeychip Monkeychip Apr 21, 2025

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

*/

export default class EnableInputComponent extends Component<Args> {
Expand Down
9 changes: 9 additions & 0 deletions ui/tests/integration/components/database-role-edit-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ module('Integration | Component | database-role-edit', function (hooks) {
{
path: 'static-roles',
username: 'staticTestUser',
password: 'testPassword',
rotation_period: '172800s', // 2 days in seconds
skip_import_rotation: true,
},
Expand All @@ -97,11 +98,16 @@ module('Integration | Component | database-role-edit', function (hooks) {
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

@Monkeychip Monkeychip Apr 21, 2025

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 click('[data-test-secret-save]');

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

@Monkeychip Monkeychip Apr 21, 2025

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.


await render(hbs`<DatabaseRoleEdit @model={{this.modelStatic}} @mode="edit"/>`);
assert.dom('[data-test-icon="edit"]').exists(); // verify password field is enabled for edit & enable button is rendered bc role hasn't been rotated
});

test('enterprise: it should successfully create user that does rotate immediately & verify warning modal pops up', async function (assert) {
Expand All @@ -116,6 +122,9 @@ module('Integration | Component | database-role-edit', function (hooks) {

await render(hbs`<DatabaseRoleEdit @model={{this.modelStatic}} @mode="show"/>`);
assert.dom('[data-test-value-div="Rotate immediately"]').containsText('Yes');

await render(hbs`<DatabaseRoleEdit @model={{this.modelStatic}} @mode="edit"/>`);
assert.dom('[data-test-icon="edit"]').doesNotExist(); // verify password field is disabled for edit & enable button isn't rendered bc role has already been rotated
});

test('it should show Get credentials button when a user has the correct policy', async function (assert) {
Expand Down
Loading