From 566a29ee23ed96aa26f0a3ec228776ef0554f39f Mon Sep 17 00:00:00 2001 From: claire bontempo <68122737+hellobontempo@users.noreply.github.com> Date: Tue, 21 Mar 2023 07:51:15 -0600 Subject: [PATCH] UI/update auth form to fetchRoles after a namespace is inputted, prior to OIDC auth (#19541) * re-fetch roles if there is a namespace * remove redundant conditional * reorder oidc auth operations * add test * test cleanup * add changelog --- changelog/19541.txt | 3 + ui/app/components/auth-jwt.js | 18 ++-- ui/app/templates/components/auth-form.hbs | 24 +++-- ui/app/templates/vault/cluster/auth.hbs | 1 + .../acceptance/enterprise-namespaces-test.js | 89 ++++++++++++++++++- ui/tests/pages/auth.js | 14 +++ 6 files changed, 126 insertions(+), 23 deletions(-) create mode 100644 changelog/19541.txt diff --git a/changelog/19541.txt b/changelog/19541.txt new file mode 100644 index 000000000..9bdecc358 --- /dev/null +++ b/changelog/19541.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: fixes oidc tabs in auth form submitting with the root's default_role value after a namespace has been inputted +``` diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js index 72f4eac85..ec4c7b2c5 100644 --- a/ui/app/components/auth-jwt.js +++ b/ui/app/components/auth-jwt.js @@ -139,7 +139,7 @@ export default Component.extend({ let { namespace, path, state, code } = oidcState; - // The namespace can be either be passed as a query paramter, or be embedded + // The namespace can be either be passed as a query parameter, or be embedded // in the state param in the format `,ns=`. So if // `namespace` is empty, check for namespace in state as well. if (namespace === '' || this.featureFlagService.managedNamespaceRoot) { @@ -176,6 +176,14 @@ export default Component.extend({ if (e && e.preventDefault) { e.preventDefault(); } + try { + await this.fetchRole.perform(this.roleName, { debounce: false }); + } catch (error) { + // this task could be cancelled if the instances in didReceiveAttrs resolve after this was started + if (error?.name !== 'TaskCancelation') { + throw error; + } + } if (!this.isOIDC || !this.role || !this.role.authUrl) { let message = this.errorMessage; if (!this.role) { @@ -187,14 +195,6 @@ export default Component.extend({ this.onError(message); return; } - try { - await this.fetchRole.perform(this.roleName, { debounce: false }); - } catch (error) { - // this task could be cancelled if the instances in didReceiveAttrs resolve after this was started - if (error?.name !== 'TaskCancelation') { - throw error; - } - } const win = this.getWindow(); const POPUP_WIDTH = 500; diff --git a/ui/app/templates/components/auth-form.hbs b/ui/app/templates/components/auth-form.hbs index b17c19b01..2753b1d57 100644 --- a/ui/app/templates/components/auth-form.hbs +++ b/ui/app/templates/components/auth-form.hbs @@ -19,7 +19,7 @@ "is-active" "" }} - data-test-auth-method + data-test-auth-method={{method.id}} > {{/let}} {{/each}} - {{#if this.hasMethodsWithPath}} -
  • - - Other - -
    • - {{/if}} +
  • + + Other + +
    • {{/if}} diff --git a/ui/app/templates/vault/cluster/auth.hbs b/ui/app/templates/vault/cluster/auth.hbs index 442a1f471..6902db07b 100644 --- a/ui/app/templates/vault/cluster/auth.hbs +++ b/ui/app/templates/vault/cluster/auth.hbs @@ -82,6 +82,7 @@
    `[data-test-auth-method="${path}"] a`, + authSubmit: '[data-test-auth-submit]', + }; + hooks.beforeEach(async function () { + this.namespace = 'test-ns'; + this.rootOidc = 'root-oidc'; + this.nsOidc = 'ns-oidc'; + + const enableOidc = async (path, role = '') => { + this.server.post(`/auth/${path}/config`, () => {}); + await shell.runCommands([ + `write sys/auth/${path} type=oidc`, + `write auth/${path}/config default_role="${role}" oidc_discovery_url="https://example.com"`, + // show method as tab + `write sys/auth/${path}/tune listing_visibility="unauth"`, + ]); + }; + await authPage.login(); + // enable oidc in root namespace, without default role + await enableOidc(this.rootOidc); + // create child namespace to enable oidc + await createNS(this.namespace); + await logout.visit(); + + // enable oidc in child namespace with default role + await authPage.loginNs(this.namespace); + await enableOidc(this.nsOidc, `${this.nsOidc}-role`); + await authPage.logout(); + + // end by logging in/out of root so query params are cleared out and don't include namespace + await authPage.login(); + return await authPage.logout(); + }); + + hooks.afterEach(async function () { + const disableOidc = async (path) => { + await shell.runCommands([`delete /sys/auth/${path}`]); + }; + + await authPage.loginNs(this.namespace); + await visit(`/vault/access?namespace=${this.namespace}`); + // disable methods to cleanup test state for re-running + await disableOidc(this.rootOidc); + await disableOidc(this.nsOidc); + await authPage.logout(); + + await authPage.login(); + await shell.runCommands([`delete /sys/auth/${this.namespace}`]); + this.server.shutdown(); + }); + + test('oidc: request is made to auth_url when a namespace is inputted', async function (assert) { + assert.expect(5); + this.server.post(`/auth/${this.rootOidc}/oidc/auth_url`, (schema, req) => { + const { redirect_uri } = JSON.parse(req.requestBody); + const { pathname, search } = parseURL(redirect_uri); + assert.strictEqual( + pathname + search, + `/ui/vault/auth/${this.rootOidc}/oidc/callback`, + 'request made to auth_url when the login page is visited' + ); + }); + this.server.post(`/auth/${this.nsOidc}/oidc/auth_url`, (schema, req) => { + const { redirect_uri } = JSON.parse(req.requestBody); + const { pathname, search } = parseURL(redirect_uri); + assert.strictEqual( + pathname + search, + `/ui/vault/auth/${this.nsOidc}/oidc/callback?namespace=${this.namespace}`, + 'request made to correct auth_url when namespace is filled in' + ); + }); + await visit('/vault/auth?with=oidc%2F'); + assert.dom(SELECTORS.authTab(this.rootOidc)).exists('renders oidc method tab for root'); + await authPage.namespaceInput(this.namespace); + assert.strictEqual( + currentURL(), + `/vault/auth?namespace=${this.namespace}&with=${this.nsOidc}%2F`, + 'url updates with namespace value' + ); + assert.dom(SELECTORS.authTab(this.nsOidc)).exists('renders oidc method tab for child namespace'); + }); + }); }); diff --git a/ui/tests/pages/auth.js b/ui/tests/pages/auth.js index e915fe9ef..d8e7a2340 100644 --- a/ui/tests/pages/auth.js +++ b/ui/tests/pages/auth.js @@ -16,6 +16,7 @@ export default create({ tokenInput: fillable('[data-test-token]'), usernameInput: fillable('[data-test-username]'), passwordInput: fillable('[data-test-password]'), + namespaceInput: fillable('[data-test-auth-form-ns-input]'), login: async function (token) { // make sure we're always logged out and logged back in await this.logout(); @@ -44,4 +45,17 @@ export default create({ await this.passwordInput(password).submit(); return; }, + loginNs: async function (ns) { + // make sure we're always logged out and logged back in + await this.logout(); + await settled(); + // clear session storage to ensure we have a clean state + window.localStorage.clear(); + await this.visit({ with: 'token' }); + await settled(); + await this.namespaceInput(ns); + await settled(); + await this.tokenInput(rootToken).submit(); + return; + }, });