From 2d0587f31679fae431e5b7db5938fbd9dadc88f5 Mon Sep 17 00:00:00 2001 From: claire bontempo <68122737+hellobontempo@users.noreply.github.com> Date: Tue, 27 May 2025 16:41:01 -0700 Subject: [PATCH] UI: Improve namespace input UX (#30751) * yield namespace input and update tests * add refocus logic * more tests! --- ui/app/components/auth/form-template.hbs | 9 +- ui/app/components/auth/form-template.ts | 26 ------ ui/app/components/auth/namespace-input.hbs | 14 +-- ui/app/components/auth/namespace-input.ts | 67 +++++++++++++++ ui/app/components/auth/page.hbs | 15 +++- ui/app/components/auth/page.ts | 14 +-- ui/app/controllers/vault/cluster/auth.js | 10 ++- ui/app/routes/vault/cluster/auth.js | 2 +- ui/app/templates/vault/cluster/auth.hbs | 1 + .../components/auth/form-template-test.js | 70 ++++----------- .../components/auth/namespace-input-test.js | 85 +++++++++++++++++++ .../integration/components/auth/page-test.js | 49 ++++++----- 12 files changed, 229 insertions(+), 133 deletions(-) create mode 100644 ui/app/components/auth/namespace-input.ts create mode 100644 ui/tests/integration/components/auth/namespace-input-test.js diff --git a/ui/app/components/auth/form-template.hbs b/ui/app/components/auth/form-template.hbs index c5873a1655..d115bbab66 100644 --- a/ui/app/components/auth/form-template.hbs +++ b/ui/app/components/auth/form-template.hbs @@ -13,14 +13,7 @@ @onSuccess={{@onSuccess}} > <:namespace> - {{#if (has-feature "Namespaces")}} - - {{/if}} + {{yield}} <:back> diff --git a/ui/app/components/auth/form-template.ts b/ui/app/components/auth/form-template.ts index a0b400e5f3..18bb3d8227 100644 --- a/ui/app/components/auth/form-template.ts +++ b/ui/app/components/auth/form-template.ts @@ -8,9 +8,7 @@ import { service } from '@ember/service'; import { tracked } from '@glimmer/tracking'; import { action } from '@ember/object'; import { supportedTypes } from 'vault/utils/supported-login-methods'; -import { getRelativePath } from 'core/utils/sanitize-path'; -import type FlagsService from 'vault/services/flags'; import type VersionService from 'vault/services/version'; import type ClusterModel from 'vault/models/cluster'; import type { UnauthMountsByType } from 'vault/vault/auth/form'; @@ -31,23 +29,16 @@ import type { HTMLElementEvent } from 'vault/forms'; * @param {string} canceledMfaAuth - saved auth type from a cancelled mfa verification * @param {object} cluster - The route model which is the ember data cluster model. contains information such as cluster id, name and boolean for if the cluster is in standby * @param {object} defaultView - The `FormView` (see the interface below) data to render the initial view. - * @param {function} handleNamespaceUpdate - callback task that passes user input to the controller and updates the namespace query param in the url * @param {object} initialFormState - sets selectedAuthMethod and showAlternateView based on the login form configuration computed in parent component - * @param {string} namespaceQueryParam - namespace query param from the url - * @param {string} oidcProviderQueryParam - oidc provider query param, set in url as "?o=someprovider". if present, disables the namespace input * @param {function} onSuccess - callback after the initial authentication request, if an mfa_requirement exists the parent renders the mfa form otherwise it fires the authSuccess action in the auth controller and handles transitioning to the app * @param {array} visibleMountTypes - array of auth method types that have mounts with listing_visibility="unauth" - * * */ interface Args { alternateView: FormView | null; cluster: ClusterModel; defaultView: FormView; - handleNamespaceUpdate: CallableFunction; initialFormState: { initialAuthType: string; showAlternate: boolean }; - namespaceQueryParam: string; - oidcProviderQueryParam: string; onSuccess: CallableFunction; visibleMountTypes: string[]; } @@ -58,7 +49,6 @@ interface FormView { } export default class AuthFormTemplate extends Component { - @service declare readonly flags: FlagsService; @service declare readonly version: VersionService; supportedAuthTypes: string[]; @@ -103,17 +93,6 @@ export default class AuthFormTemplate extends Component { return this.args.visibleMountTypes?.includes(this.selectedAuthMethod); } - get namespaceInput() { - const namespaceQueryParam = this.args.namespaceQueryParam; - if (this.flags.hvdManagedNamespaceRoot) { - // When managed, the user isn't allowed to edit the prefix `admin/` - // so prefill just the relative path in the namespace input - const path = getRelativePath(namespaceQueryParam, this.flags.hvdManagedNamespaceRoot); - return path ? `/${path}` : ''; - } - return namespaceQueryParam; - } - @action setAuthType(authType: string) { this.selectedAuthMethod = authType; @@ -136,9 +115,4 @@ export default class AuthFormTemplate extends Component { handleError(message: string) { this.errorMessage = message; } - - @action - handleNamespaceUpdate(event: HTMLElementEvent) { - this.args.handleNamespaceUpdate(event.target.value); - } } diff --git a/ui/app/components/auth/namespace-input.hbs b/ui/app/components/auth/namespace-input.hbs index 30d0cc0956..aa3b70991e 100644 --- a/ui/app/components/auth/namespace-input.hbs +++ b/ui/app/components/auth/namespace-input.hbs @@ -4,14 +4,14 @@ }}
- {{#if @hvdManagedNamespace}} + {{#if this.flags.hvdManagedNamespaceRoot}} Namespace {{else}} { + @service declare readonly api: ApiService; + @service declare readonly flags: FlagsService; + + get namespaceInput() { + const namespaceQueryParam = this.args.namespaceQueryParam; + if (this.flags.hvdManagedNamespaceRoot) { + // When managed, the user isn't allowed to edit the prefix `admin/` + // so prefill just the relative path in the namespace input + const path = getRelativePath(namespaceQueryParam, this.flags.hvdManagedNamespaceRoot); + return path ? `/${path}` : ''; + } + return namespaceQueryParam; + } + + @action + async handleInput(event: HTMLElementEvent) { + // user has typed something, so input should be refocused + const value = event.target.value; + this.updateNamespace.perform(value); + } + + updateNamespace = restartableTask(async (namespace) => { + await timeout(500); + await this.args.handleNamespaceUpdate(namespace); + }); + + @action + maybeRefocus(element: HTMLElement) { + if (this.args.shouldRefocusNamespaceInput) { + element.focus(); + } + } +} diff --git a/ui/app/components/auth/page.hbs b/ui/app/components/auth/page.hbs index 174478ac19..73ffae56b1 100644 --- a/ui/app/components/auth/page.hbs +++ b/ui/app/components/auth/page.hbs @@ -53,13 +53,20 @@ @alternateView={{this.formViews.alternateView}} @cluster={{@cluster}} @defaultView={{this.formViews.defaultView}} - @handleNamespaceUpdate={{@onNamespaceUpdate}} @initialFormState={{this.initialFormState}} - @namespaceQueryParam={{@namespaceQueryParam}} - @oidcProviderQueryParam={{@oidcProviderQueryParam}} @onSuccess={{this.onAuthResponse}} @visibleMountTypes={{this.visibleMountTypes}} - /> + > + {{! yielded for accessibility so namespace submits as an input of the
element }} + {{#if (has-feature "Namespaces")}} + + {{/if}} + {{/if}} diff --git a/ui/app/components/auth/page.ts b/ui/app/components/auth/page.ts index 11bbbffeeb..677795cf60 100644 --- a/ui/app/components/auth/page.ts +++ b/ui/app/components/auth/page.ts @@ -107,6 +107,12 @@ export default class AuthPage extends Component { @tracked mfaAuthData: MfaAuthData | null = null; @tracked mfaErrors = ''; + get cspError() { + const isStandby = this.args.cluster.standby; + const hasConnectionViolations = this.csp.connectionViolations.length; + return isStandby && hasConnectionViolations ? CSP_ERROR : ''; + } + get visibleMountsByType() { const visibleAuthMounts = this.args.visibleAuthMounts; if (visibleAuthMounts) { @@ -125,13 +131,7 @@ export default class AuthPage extends Component { return Object.keys(this.visibleMountsByType || {}); } - get cspError() { - const isStandby = this.args.cluster.standby; - const hasConnectionViolations = this.csp.connectionViolations.length; - return isStandby && hasConnectionViolations ? CSP_ERROR : ''; - } - - // FORM STATE GETTERS + // AUTH FORM STATE GETTERS get formViews() { const { directLinkData, loginSettings } = this.args; diff --git a/ui/app/controllers/vault/cluster/auth.js b/ui/app/controllers/vault/cluster/auth.js index 9055d6c303..c5a8e99a3a 100644 --- a/ui/app/controllers/vault/cluster/auth.js +++ b/ui/app/controllers/vault/cluster/auth.js @@ -5,7 +5,7 @@ import { service } from '@ember/service'; import { alias } from '@ember/object/computed'; import Controller, { inject as controller } from '@ember/controller'; -import { task, timeout } from 'ember-concurrency'; +import { task } from 'ember-concurrency'; import { sanitizePath } from 'core/utils/sanitize-path'; export default Controller.extend({ @@ -22,7 +22,9 @@ export default Controller.extend({ namespaceQueryParam: alias('clusterController.namespaceQueryParam'), redirectTo: alias('vaultController.redirectTo'), hvdManagedNamespaceRoot: alias('flagsService.hvdManagedNamespaceRoot'), + shouldRefocusNamespaceInput: false, + // Query params authMount: '', oidcProvider: '', unwrapTokenError: '', @@ -36,12 +38,12 @@ export default Controller.extend({ }, updateNamespace: task(function* (value) { - // debounce - yield timeout(500); const ns = this.fullNamespaceFromInput(value); this.namespaceService.setNamespace(ns, true); - this.customMessages.fetchMessages(); + yield this.customMessages.fetchMessages(); this.set('namespaceQueryParam', ns); + // if user is inputting a namespace, maintain input focus as the param updates + this.set('shouldRefocusNamespaceInput', true); }).restartable(), actions: { diff --git a/ui/app/routes/vault/cluster/auth.js b/ui/app/routes/vault/cluster/auth.js index 596acf870c..5094335d3d 100644 --- a/ui/app/routes/vault/cluster/auth.js +++ b/ui/app/routes/vault/cluster/auth.js @@ -120,7 +120,7 @@ export default class AuthRoute extends ClusterRouteBase { // return a falsy value if the object is empty return isEmptyValue(resp.auth) ? null : resp.auth; } catch { - // swallow the error if there's an error fetching mount data (i.e. invalid namespace) + // catch error if there's a problem fetching mount data (i.e. invalid namespace) return null; } } diff --git a/ui/app/templates/vault/cluster/auth.hbs b/ui/app/templates/vault/cluster/auth.hbs index 4cec9869d3..3cf7a27176 100644 --- a/ui/app/templates/vault/cluster/auth.hbs +++ b/ui/app/templates/vault/cluster/auth.hbs @@ -22,5 +22,6 @@ @onAuthSuccess={{action "authSuccess"}} @onNamespaceUpdate={{perform this.updateNamespace}} @visibleAuthMounts={{this.model.visibleAuthMounts}} + @shouldRefocusNamespaceInput={{this.shouldRefocusNamespaceInput}} /> {{/if}} \ No newline at end of file diff --git a/ui/tests/integration/components/auth/form-template-test.js b/ui/tests/integration/components/auth/form-template-test.js index 8e16bcc6e4..484f093cf7 100644 --- a/ui/tests/integration/components/auth/form-template-test.js +++ b/ui/tests/integration/components/auth/form-template-test.js @@ -31,10 +31,7 @@ module('Integration | Component | auth | form template', function (hooks) { this.alternateView = null; this.defaultView = { view: 'dropdown', tabData: null }; - this.handleNamespaceUpdate = sinon.spy(); this.initialFormState = { initialAuthType: 'token', showAlternate: false }; - this.namespaceQueryParam = ''; - this.oidcProviderQueryParam = ''; this.onSuccess = sinon.spy(); this.visibleMountTypes = null; @@ -44,10 +41,7 @@ module('Integration | Component | auth | form template', function (hooks) { @alternateView={{this.alternateView}} @cluster={{this.cluster}} @defaultView={{this.defaultView}} - @handleNamespaceUpdate={{this.handleNamespaceUpdate}} @initialFormState={{this.initialFormState}} - @namespaceQueryParam={{this.namespaceQueryParam}} - @oidcProviderQueryParam={{this.oidcProviderQueryParam}} @onSuccess={{this.onSuccess}} @visibleMountTypes={{this.visibleMountTypes}} />`); @@ -83,6 +77,22 @@ module('Integration | Component | auth | form template', function (hooks) { authenticateStub.restore(); }); + test('dropdown does not include enterprise methods on community versions', async function (assert) { + this.version.type = 'community'; + const supported = BASE_LOGIN_METHODS.map((m) => m.type); + const unsupported = ENTERPRISE_LOGIN_METHODS.map((m) => m.type); + assert.expect(supported.length + unsupported.length); + await this.renderComponent(); + const dropdownOptions = findAll(`${GENERAL.selectByAttr('auth type')} option`).map((o) => o.value); + + supported.forEach((m) => { + assert.true(dropdownOptions.includes(m), `dropdown includes supported method: ${m}`); + }); + unsupported.forEach((m) => { + assert.false(dropdownOptions.includes(m), `dropdown does NOT include unsupported method: ${m}`); + }); + }); + module('listing visibility', function (hooks) { hooks.beforeEach(function () { const defaultTabs = { @@ -210,47 +220,14 @@ module('Integration | Component | auth | form template', function (hooks) { }); }); - module('community', function (hooks) { - hooks.beforeEach(function () { - this.version.type = 'community'; - }); - - test('it does not render the namespace input on community', async function (assert) { - await this.renderComponent(); - assert.dom(GENERAL.inputByAttr('namespace')).doesNotExist(); - }); - - test('dropdown does not include enterprise methods', async function (assert) { - const supported = BASE_LOGIN_METHODS.map((m) => m.type); - const unsupported = ENTERPRISE_LOGIN_METHODS.map((m) => m.type); - assert.expect(supported.length + unsupported.length); - await this.renderComponent(); - const dropdownOptions = findAll(`${GENERAL.selectByAttr('auth type')} option`).map((o) => o.value); - - supported.forEach((m) => { - assert.true(dropdownOptions.includes(m), `dropdown includes supported method: ${m}`); - }); - unsupported.forEach((m) => { - assert.false(dropdownOptions.includes(m), `dropdown does NOT include unsupported method: ${m}`); - }); - }); - }); - // tests with "enterprise" in the title are filtered out from CE test runs // naming the module 'ent' so these tests still run on the CE repo module('ent', function (hooks) { hooks.beforeEach(function () { this.version.type = 'enterprise'; - this.version.features = ['Namespaces']; this.namespaceQueryParam = ''; }); - test('it does not render the namespace input if version does not include feature', async function (assert) { - this.version.features = []; - await this.renderComponent(); - assert.dom(GENERAL.inputByAttr('namespace')).doesNotExist(); - }); - // in th ent module to test ALL supported login methods // iterating in tests should generally be avoided, but purposefully wanted to test the component // renders as expected as auth types change @@ -289,12 +266,6 @@ module('Integration | Component | auth | form template', function (hooks) { } }); - test('it disables namespace input when an oidc provider query param exists', async function (assert) { - this.oidcProviderQueryParam = 'myprovider'; - await this.renderComponent(); - assert.dom(GENERAL.inputByAttr('namespace')).isDisabled(); - }); - test('dropdown includes enterprise methods', async function (assert) { const supported = ALL_LOGIN_METHODS.map((m) => m.type); assert.expect(supported.length); @@ -305,15 +276,6 @@ module('Integration | Component | auth | form template', function (hooks) { assert.true(dropdownOptions.includes(m), `dropdown includes supported method: ${m}`); }); }); - - test('it sets namespace for hvd managed clusters', async function (assert) { - this.owner.lookup('service:flags').featureFlags = ['VAULT_CLOUD_ADMIN_NAMESPACE']; - this.namespaceQueryParam = 'admin/west-coast'; - await this.renderComponent(); - assert.dom(AUTH_FORM.managedNsRoot).hasValue('/admin'); - assert.dom(AUTH_FORM.managedNsRoot).hasAttribute('readonly'); - assert.dom(GENERAL.inputByAttr('namespace')).hasValue('/west-coast'); - }); }); // AUTH METHOD SPECIFIC TESTS diff --git a/ui/tests/integration/components/auth/namespace-input-test.js b/ui/tests/integration/components/auth/namespace-input-test.js new file mode 100644 index 0000000000..3321b4bd10 --- /dev/null +++ b/ui/tests/integration/components/auth/namespace-input-test.js @@ -0,0 +1,85 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { fillIn, find, render } from '@ember/test-helpers'; +import hbs from 'htmlbars-inline-precompile'; +import sinon from 'sinon'; +import { AUTH_FORM } from 'vault/tests/helpers/auth/auth-form-selectors'; +import { GENERAL } from 'vault/tests/helpers/general-selectors'; + +module('Integration | Component | auth | namespace input', function (hooks) { + setupRenderingTest(hooks); + + hooks.beforeEach(function () { + this.disabled = false; + this.oidcProviderQueryParam = ''; + this.handleNamespaceUpdate = sinon.spy(); + this.shouldRefocusNamespaceInput = false; + + this.renderComponent = () => { + return render(hbs` + `); + }; + }); + + test('it fires @handleNamespaceUpdate callback', async function (assert) { + assert.expect(1); + await this.renderComponent(); + await fillIn(GENERAL.inputByAttr('namespace'), 'ns-1'); + const [actual] = this.handleNamespaceUpdate.lastCall.args; + assert.strictEqual(actual, 'ns-1', `handleNamespaceUpdate called with: ${actual}`); + }); + + test('it disables the input if @disabled is true', async function (assert) { + this.disabled = true; + await this.renderComponent(); + assert.dom(GENERAL.inputByAttr('namespace')).isDisabled(); + }); + + test('it does not focus the input if @shouldRefocusNamespaceInput is false', async function (assert) { + await this.renderComponent(); + const element = find(GENERAL.inputByAttr('namespace')); + assert.notStrictEqual(document.activeElement, element, 'the namespace input is NOT focused'); + }); + + test('it focuses the input if @shouldRefocusNamespaceInput is true', async function (assert) { + this.shouldRefocusNamespaceInput = true; + await this.renderComponent(); + const element = find(GENERAL.inputByAttr('namespace')); + assert.strictEqual(document.activeElement, element, 'the namespace input is focused'); + }); + + module('HVD managed', function (hooks) { + hooks.beforeEach(function () { + this.owner.lookup('service:flags').featureFlags = ['VAULT_CLOUD_ADMIN_NAMESPACE']; + }); + + test('it sets namespace', async function (assert) { + this.namespaceQueryParam = 'admin/west-coast'; + await this.renderComponent(); + assert.dom(AUTH_FORM.managedNsRoot).hasValue('/admin'); + assert.dom(AUTH_FORM.managedNsRoot).hasAttribute('readonly'); + assert.dom(GENERAL.inputByAttr('namespace')).hasValue('/west-coast'); + }); + + test('it calls onNamespaceUpdate', async function (assert) { + assert.expect(2); + this.namespaceQueryParam = 'admin'; + await this.renderComponent(); + + assert.dom(GENERAL.inputByAttr('namespace')).hasValue(''); + await fillIn(GENERAL.inputByAttr('namespace'), 'ns-1'); + const [actual] = this.handleNamespaceUpdate.lastCall.args; + assert.strictEqual(actual, 'ns-1', `handleNamespaceUpdate called with: ${actual}`); + }); + }); +}); diff --git a/ui/tests/integration/components/auth/page-test.js b/ui/tests/integration/components/auth/page-test.js index 106f902902..4f4b307611 100644 --- a/ui/tests/integration/components/auth/page-test.js +++ b/ui/tests/integration/components/auth/page-test.js @@ -24,6 +24,8 @@ module('Integration | Component | auth | page', function (hooks) { this.cluster = { id: '1' }; this.directLinkData = null; this.loginSettings = null; + this.namespaceQueryParam = ''; + this.oidcProviderQueryParam = ''; this.onAuthSuccess = sinon.spy(); this.onNamespaceUpdate = sinon.spy(); this.visibleAuthMounts = false; @@ -34,8 +36,8 @@ module('Integration | Component | auth | page', function (hooks) { @cluster={{this.cluster}} @directLinkData={{this.directLinkData}} @loginSettings={{this.loginSettings}} - @namespaceQueryParam={{this.nsQp}} - @oidcProviderQueryParam={{this.providerQp}} + @namespaceQueryParam={{this.namespaceQueryParam}} + @oidcProviderQueryParam={{this.oidcProviderQueryParam}} @onAuthSuccess={{this.onAuthSuccess}} @onNamespaceUpdate={{this.onNamespaceUpdate}} @visibleAuthMounts={{this.visibleAuthMounts}} @@ -58,10 +60,12 @@ module('Integration | Component | auth | page', function (hooks) { assert.dom(GENERAL.pageError.error).hasText(CSP_ERROR); }); - test('it renders splash logo when oidc provider query param is present', async function (assert) { - this.providerQp = 'myprovider'; + test('it renders splash logo and disables namespace input when oidc provider query param is present', async function (assert) { + this.oidcProviderQueryParam = 'myprovider'; + this.version.features = ['Namespaces']; await this.renderComponent(); assert.dom(AUTH_FORM.logo).exists(); + assert.dom(GENERAL.inputByAttr('namespace')).isDisabled(); assert .dom(AUTH_FORM.helpText) .hasText( @@ -69,14 +73,6 @@ module('Integration | Component | auth | page', function (hooks) { ); }); - test('it disables namespace input when oidc provider query param is present', async function (assert) { - this.providerQp = 'myprovider'; - this.version.features = ['Namespaces']; - await this.renderComponent(); - assert.dom(AUTH_FORM.logo).exists(); - assert.dom(GENERAL.inputByAttr('namespace')).isDisabled(); - }); - test('it calls onNamespaceUpdate', async function (assert) { assert.expect(1); this.version.features = ['Namespaces']; @@ -86,21 +82,28 @@ module('Integration | Component | auth | page', function (hooks) { assert.strictEqual(actual, 'mynamespace', `onNamespaceUpdate called with: ${actual}`); }); - test('it calls onNamespaceUpdate for HVD managed clusters', async function (assert) { - assert.expect(2); + test('it passes query param to namespace input', async function (assert) { this.version.features = ['Namespaces']; - this.owner.lookup('service:flags').featureFlags = ['VAULT_CLOUD_ADMIN_NAMESPACE']; - this.nsQp = 'admin'; + this.namespaceQueryParam = 'ns-1'; await this.renderComponent(); - - assert.dom(GENERAL.inputByAttr('namespace')).hasValue(''); - await fillIn(GENERAL.inputByAttr('namespace'), 'mynamespace'); - const [actual] = this.onNamespaceUpdate.lastCall.args; - assert.strictEqual(actual, 'mynamespace', `onNamespaceUpdate called with: ${actual}`); + assert.dom(GENERAL.inputByAttr('namespace')).hasValue(this.namespaceQueryParam); }); - // DIRECT LINK tests (without any listing visibility) - test('it selects type in the dropdown if direct link is just type', async function (assert) { + test('it does not render the namespace input on community', async function (assert) { + this.version.type = 'community'; + this.version.features = []; + await this.renderComponent(); + assert.dom(GENERAL.inputByAttr('namespace')).doesNotExist(); + }); + + test('it does not render the namespace input on enterprise without the "Namespaces" feature', async function (assert) { + this.version.type = 'enterprise'; + this.version.features = []; + await this.renderComponent(); + assert.dom(GENERAL.inputByAttr('namespace')).doesNotExist(); + }); + + test('it selects type in the dropdown if direct link just has type', async function (assert) { this.directLinkData = { type: 'oidc' }; await this.renderComponent(); assert.dom(AUTH_FORM.tabBtn('oidc')).doesNotExist('tab does not render');