From 996dc56f08f4920cbab09cb05db24bbda3383cc7 Mon Sep 17 00:00:00 2001 From: claire bontempo <68122737+hellobontempo@users.noreply.github.com> Date: Thu, 23 Mar 2023 01:13:03 -0600 Subject: [PATCH] Backport 1.13.x: UI/update auth form to fetchRoles after a namespace is inputted, prior to OIDC auth #19541 (#19661) * 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 * UI: fix enterprise test failures (#19671) * move oidc tests into new file * remove module from namespace test * remove entered line * add logout to afterEach hook * remove ns test * move test setup to within test * use logout.visit() instead * updates oidc auth namespaces test * reverts to authPage logout --------- Co-authored-by: Jordan Reimer --------- Co-authored-by: Jordan Reimer --- 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 | 1 - .../enterprise-oidc-namespace-test.js | 91 +++++++++++++++++++ ui/tests/pages/auth.js | 14 +++ 7 files changed, 129 insertions(+), 23 deletions(-) create mode 100644 changelog/19541.txt create mode 100644 ui/tests/acceptance/enterprise-oidc-namespace-test.js diff --git a/changelog/19541.txt b/changelog/19541.txt new file mode 100644 index 000000000000..9bdecc35832d --- /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 41110ebbf259..724ed90c0664 100644 --- a/ui/app/components/auth-jwt.js +++ b/ui/app/components/auth-jwt.js @@ -134,7 +134,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) { @@ -171,6 +171,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) { @@ -182,14 +190,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 b17c19b010c6..2753b1d57ad2 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 442a1f4711e5..6902db07b17e 100644 --- a/ui/app/templates/vault/cluster/auth.hbs +++ b/ui/app/templates/vault/cluster/auth.hbs @@ -82,6 +82,7 @@
    { + await shell.runCommands(`write sys/namespaces/${name} -force`); +}; +const SELECTORS = { + authTab: (path) => `[data-test-auth-method="${path}"] a`, +}; + +module('Acceptance | Enterprise | oidc auth namespace test', function (hooks) { + setupApplicationTest(hooks); + setupMirage(hooks); + + hooks.beforeEach(async function () { + this.namespace = 'test-ns'; + this.rootOidc = 'root-oidc'; + this.nsOidc = 'ns-oidc'; + + this.server.post(`/auth/:path/config`, () => {}); + + this.enableOidc = (path, role = '') => { + return 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"`, + ]); + }; + + this.disableOidc = (path) => shell.runCommands([`delete /sys/auth/${path}`]); + }); + + 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 authPage.login(); + // enable oidc in root namespace, without default role + await this.enableOidc(this.rootOidc); + // create child namespace to enable oidc + await createNS(this.namespace); + // enable oidc in child namespace with default role + await authPage.loginNs(this.namespace); + await this.enableOidc(this.nsOidc, `${this.nsOidc}-role`); + await authPage.logout(); + + await visit('/vault/auth'); + 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'); + + // disable methods to cleanup test state for re-running + await authPage.login(); + await this.disableOidc(this.rootOidc); + await this.disableOidc(this.nsOidc); + await shell.runCommands([`delete /sys/auth/${this.namespace}`]); + await authPage.logout(); + }); +}); diff --git a/ui/tests/pages/auth.js b/ui/tests/pages/auth.js index 99515e9afb12..ca5864ec7447 100644 --- a/ui/tests/pages/auth.js +++ b/ui/tests/pages/auth.js @@ -11,6 +11,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(); @@ -39,4 +40,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; + }, });