UI: Consolidates form parsing logic to base.ts component (#30397)

* stuck tests

* consolidate form parsing logic

ui/app/components/auth/form/base.ts

@@ -9,10 +9,13 @@ import { service } from '@ember/service';
 import { task } from 'ember-concurrency';
 import { waitFor } from '@ember/test-waiters';
 import { sanitizePath } from 'core/utils/sanitize-path';
+import { POSSIBLE_FIELDS } from 'vault/utils/supported-login-methods';
 
 import type AuthService from 'vault/vault/services/auth';
-import type { AuthData } from 'vault/vault/services/auth';
 import type ClusterModel from 'vault/models/cluster';
+import type FlagsService from 'vault/services/flags';
+import type VersionService from 'vault/services/version';
+import type { AuthData } from 'vault/vault/services/auth';
 import type { HTMLElementEvent } from 'vault/forms';
 
 /**
@@ -33,26 +36,14 @@ interface Args {
 
 export default class AuthBase extends Component<Args> {
   @service declare readonly auth: AuthService;
+  @service declare readonly flags: FlagsService;
+  @service declare readonly version: VersionService;
 
   @action
   onSubmit(event: HTMLElementEvent<HTMLFormElement>) {
     event.preventDefault();
     const formData = new FormData(event.target as HTMLFormElement);
-    const data: Record<string, FormDataEntryValue | null> = {};
-
-    for (const key of formData.keys()) {
-      const value = formData.get(key);
-      // strip leading or trailing slashes from path for consistency
-      data[key] = key === 'path' ? sanitizePath(value) : value;
-    }
-
-    // If path is not included in the submitted form data,
-    // set it as the auth type which is the default path Vault expects.
-    // The "token" auth method does not support custom login paths.
-    if (this.args.authType !== 'token' && !Object.keys(data).includes('path')) {
-      data['path'] = this.args.authType;
-    }
-
+    const data = this.parseFormData(formData);
     this.login.unlinked().perform(data);
   }
 
@@ -84,4 +75,37 @@ export default class AuthBase extends Component<Args> {
       this.args.onError(errorMessage);
     }
   }
+
+  parseFormData(formData: FormData) {
+    const data: Record<string, FormDataEntryValue | null> = {};
+
+    // iterate over method specific fields
+    for (const field of POSSIBLE_FIELDS) {
+      const value = formData.get(field);
+      if (value) {
+        data[field] = value;
+      }
+    }
+
+    // path is supported by all auth methods except token
+    if (this.args.authType !== 'token') {
+      // strip leading or trailing slashes for consistency.
+      // fallback to auth type which is the default path Vault expects.
+      data['path'] = sanitizePath(formData?.get('path')) || this.args.authType;
+    }
+
+    if (this.version.isEnterprise) {
+      // strip leading or trailing slashes for consistency
+      let namespace = sanitizePath(formData?.get('namespace')) || '';
+
+      const hvdRootNs = this.flags.hvdManagedNamespaceRoot; // if HVD managed, this is "admin"
+      if (hvdRootNs) {
+        // HVD managed clusters can only input child namespaces, manually prepend with the hvd root
+        namespace = namespace ? `${hvdRootNs}/${namespace}` : hvdRootNs;
+      }
+      data['namespace'] = namespace;
+    }
+
+    return data;
+  }
 }

ui/app/components/auth/form/oidc-jwt.ts

@@ -9,7 +9,6 @@ import { tracked } from '@glimmer/tracking';
 import { service } from '@ember/service';
 import { restartableTask, task, timeout, waitForEvent } from 'ember-concurrency';
 import { action } from '@ember/object';
-import { sanitizePath } from 'core/utils/sanitize-path';
 import { waitFor } from '@ember/test-waiters';
 import errorMessage from 'vault/utils/error-message';
 
@@ -61,7 +60,7 @@ export default class AuthFormOidcJwt extends AuthBase {
   ];
 
   // set by form inputs
-  _formData: FormData | null = null;
+  _formData: FormData = new FormData();
 
   // set during auth prep and login workflow
   @tracked fetchedRole: RoleJwtModel | null = null;
@@ -102,9 +101,7 @@ export default class AuthFormOidcJwt extends AuthBase {
     // it will cancel and restart from the beginning.
     if (wait) await timeout(wait);
 
-    const namespace = this._formData?.get('namespace') || '';
-    const path = sanitizePath(this._formData?.get('path')) || this.args.authType;
-    const role = this._formData?.get('role') || '';
+    const { namespace = '', path = '', role = '' } = this.parseFormData(this._formData);
     const id = JSON.stringify([path, role]);
 
     // reset state

ui/app/utils/supported-login-methods.ts

@@ -56,3 +56,7 @@ export const ALL_LOGIN_METHODS = [...BASE_LOGIN_METHODS, ...ENTERPRISE_LOGIN_MET
 
 export const supportedTypes = (isEnterprise: boolean) =>
   isEnterprise ? ALL_LOGIN_METHODS.map((m) => m.type) : BASE_LOGIN_METHODS.map((m) => m.type);
+
+// this ensures no unexpected params are injected and submitted in the login form
+// 'namespace' and 'path' are intentionally omitted because they are handled explicitly
+export const POSSIBLE_FIELDS = ['role', 'jwt', 'password', 'token', 'username'];

ui/tests/helpers/auth/auth-helpers.ts

@@ -8,6 +8,7 @@ import VAULT_KEYS from 'vault/tests/helpers/vault-keys';
 import { AUTH_FORM } from 'vault/tests/helpers/auth/auth-form-selectors';
 import { GENERAL } from 'vault/tests/helpers/general-selectors';
 import { Server } from 'miragejs';
+import { POSSIBLE_FIELDS } from 'vault/utils/supported-login-methods';
 
 const { rootToken } = VAULT_KEYS;
 
@@ -38,15 +39,11 @@ export const loginNs = async (ns: string, token = rootToken) => {
   return click(AUTH_FORM.login);
 };
 
-// LOGIN WITH NON-TOKEN methods
-interface LoginFields {
-  username?: string;
-  password?: string;
-  token?: string;
-  role?: string;
+// LOGIN WITH NON-TOKEN METHODS
+type LoginFields = Partial<Record<(typeof POSSIBLE_FIELDS)[number], string>> & {
   path: string;
   namespace: string;
-}
+};
 
 interface LoginOptions {
   authType?: string;
@@ -64,7 +61,7 @@ export const loginMethod = async (loginFields: LoginFields, options: LoginOption
 export const fillInLoginFields = async (loginFields: LoginFields, { toggleOptions = false } = {}) => {
   if (toggleOptions) await click(AUTH_FORM.moreOptions);
 
-  for (const [input, value] of Object.entries(loginFields)) {
+  for (const [input, value = ''] of Object.entries(loginFields)) {
     await fillIn(GENERAL.inputByAttr(input), value);
   }
 };

ui/tests/integration/components/auth/form-template-test.js

@@ -85,10 +85,14 @@ module('Integration | Component | auth | form template', function (hooks) {
   });
 
   test('it displays errors', async function (assert) {
+    const authenticateStub = sinon.stub(this.owner.lookup('service:auth'), 'authenticate');
+    authenticateStub.throws('permission denied');
     await this.renderComponent();
     await click(AUTH_FORM.login);
-    // this error message text is because the auth service is not stubbed in this test
-    assert.dom(GENERAL.messageError).hasText('Error Authentication failed: permission denied');
+    assert
+      .dom(GENERAL.messageError)
+      .hasText('Error Authentication failed: permission denied: Sinon-provided permission denied');
+    authenticateStub.restore();
   });
 
   module('listing visibility', function (hooks) {

ui/tests/integration/components/auth/form/base-test.js

@@ -6,10 +6,11 @@
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
 import hbs from 'htmlbars-inline-precompile';
-import { find, render } from '@ember/test-helpers';
+import { click, fillIn, find, render } from '@ember/test-helpers';
 import sinon from 'sinon';
 import testHelper from './test-helper';
 import { GENERAL } from 'vault/tests/helpers/general-selectors';
+import { AUTH_FORM } from 'vault/tests/helpers/auth/auth-form-selectors';
 
 // These auth types all use the default methods in auth/form/base
 // Any auth types with custom logic should be in a separate test file, i.e. okta
@@ -24,6 +25,10 @@ module('Integration | Component | auth | form | base', function (hooks) {
     this.onSuccess = sinon.spy();
   });
 
+  hooks.afterEach(function () {
+    this.authenticateStub.restore();
+  });
+
   module('github', function (hooks) {
     hooks.beforeEach(function () {
       this.authType = 'github';
@@ -142,11 +147,6 @@ module('Integration | Component | auth | form | base', function (hooks) {
     hooks.beforeEach(function () {
       this.authType = 'token';
       this.expectedFields = ['token'];
-      this.expectedSubmit = {
-        default: { token: 'mytoken' },
-        // token doesn't support custom paths, so just test yielding functionality
-        custom: { token: 'mytoken', yield: 'yield-token' },
-      };
       this.renderComponent = ({ yieldBlock = false } = {}) => {
         if (yieldBlock) {
           return render(hbs`
@@ -157,8 +157,8 @@ module('Integration | Component | auth | form | base', function (hooks) {
               @onSuccess={{this.onSuccess}}
             >
              <:advancedSettings>
-                <label for="yield">Yielded input</label>
-                <input data-test-input="yield" id="yield" name="yield" type="text" /> 
+                <label for="path">Mount path</label>
+                <input data-test-input="path" id="path" name="path" type="text" /> 
              </:advancedSettings>
             </Auth::Form::Token>`);
         }
@@ -172,7 +172,33 @@ module('Integration | Component | auth | form | base', function (hooks) {
       };
     });
 
-    testHelper(test);
+    testHelper(test, { standardSubmit: false });
+
+    test('it submits form data with defaults', async function (assert) {
+      await this.renderComponent();
+      await fillIn(GENERAL.inputByAttr('token'), 'mytoken');
+      await click(AUTH_FORM.login);
+      const [actual] = this.authenticateStub.lastCall.args;
+      assert.propEqual(
+        actual.data,
+        { token: 'mytoken' },
+        'auth service "authenticate" method is called with form data'
+      );
+    });
+
+    test('it submits form data from yielded inputs', async function (assert) {
+      await this.renderComponent({ yieldBlock: true });
+      await fillIn(GENERAL.inputByAttr('token'), 'mytoken');
+      // token doesn't support custom paths, so testing path is not sent
+      await fillIn(GENERAL.inputByAttr('path'), `path-${this.authType}`);
+      await click(AUTH_FORM.login);
+      const [actual] = this.authenticateStub.lastCall.args;
+      assert.propEqual(
+        actual.data,
+        { token: 'mytoken' },
+        'auth service "authenticate" method is called without "path"'
+      );
+    });
   });
 
   module('userpass', function (hooks) {

ui/tests/integration/components/auth/form/oidc-jwt-test.js

@@ -331,6 +331,10 @@ module('Integration | Component | auth | form | oidc-jwt', function (hooks) {
     };
   });
 
+  hooks.afterEach(function () {
+    this.authenticateStub.restore();
+  });
+
   test('it renders helper text', async function (assert) {
     await this.renderComponent();
     const id = find(GENERAL.inputByAttr('role')).id;

ui/tests/integration/components/auth/form/okta-test.js

@@ -70,6 +70,10 @@ module('Integration | Component | auth | form | okta', function (hooks) {
     };
   });
 
+  hooks.afterEach(function () {
+    this.authenticateStub.restore();
+  });
+
   testHelper(test, { standardSubmit: false });
 
   test('it submits form data with defaults', async function (assert) {

ui/tests/integration/components/auth/form/test-helper.js

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: BUSL-1.1
  */
 
-import { click, fillIn } from '@ember/test-helpers';
+import { click, fillIn, findAll } from '@ember/test-helpers';
 import { AUTH_FORM } from 'vault/tests/helpers/auth/auth-form-selectors';
 import { AUTH_METHOD_MAP } from 'vault/tests/helpers/auth/auth-helpers';
 import { GENERAL } from 'vault/tests/helpers/general-selectors';
@@ -19,9 +19,10 @@ export default (test, { standardSubmit = true } = {}) => {
   test('it renders fields', async function (assert) {
     await this.renderComponent();
     assert.dom(AUTH_FORM.authForm(this.authType)).exists(`${this.authType}: it renders form component`);
-    this.expectedFields.forEach((field) => {
-      assert.dom(GENERAL.inputByAttr(field)).exists(`${this.authType}: it renders ${field}`);
-    });
+    const fields = findAll('input');
+    for (const field of fields) {
+      assert.true(this.expectedFields.includes(field.name), `it renders field: ${field.name}`);
+    }
   });
 
   test('it fires onError callback', async function (assert) {
@@ -75,13 +76,7 @@ export default (test, { standardSubmit = true } = {}) => {
       for (const [field, value] of Object.entries(loginData)) {
         await fillIn(GENERAL.inputByAttr(field), value);
       }
-
-      if (this.authType === 'token') {
-        // token doesn't support custom paths, so just test yielding functionality
-        await fillIn(GENERAL.inputByAttr('yield'), `yield-${this.authType}`);
-      } else {
-        await fillIn(GENERAL.inputByAttr('path'), `custom-${this.authType}`);
-      }
+      await fillIn(GENERAL.inputByAttr('path'), `custom-${this.authType}`);
 
       await click(AUTH_FORM.login);
       const [actual] = this.authenticateStub.lastCall.args;