From 621ba81bf0f6e9a781db0b977d70117087f7a32f Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Tue, 5 Apr 2022 11:03:29 -0600 Subject: [PATCH 1/4] fixes issue logging in with oidc from listed auth path tab --- ui/app/components/auth-form.js | 9 +++++---- ui/app/components/auth-jwt.js | 10 ++++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/ui/app/components/auth-form.js b/ui/app/components/auth-form.js index 490711636dab..8497d76ff206 100644 --- a/ui/app/components/auth-form.js +++ b/ui/app/components/auth-form.js @@ -117,7 +117,8 @@ export default Component.extend(DEFAULTS, { if (!methods && !wrappedToken) { return {}; } - if (keyIsPath) { + // if type is provided we can ignore path since we are attempting to lookup a specific backend by type + if (keyIsPath && !type) { return methods.findBy('path', selected); } return BACKENDS.findBy('type', selected); @@ -227,7 +228,7 @@ export default Component.extend(DEFAULTS, { }); this.onSuccess(authResponse, backendType, data); } catch (e) { - this.set('loading', false); + this.set('isLoading', false); if (!this.auth.mfaError) { this.set('error', `Authentication failed: ${this.auth.handleError(e)}`); } @@ -260,7 +261,7 @@ export default Component.extend(DEFAULTS, { error: null, }); // if callback from oidc or jwt we have a token at this point - let backend = ['oidc', 'jwt'].includes(this.selectedAuth) + let backend = ['oidc', 'jwt'].includes(this.providerName) ? this.getAuthBackend('token') : this.selectedAuthBackend || {}; let backendMeta = BACKENDS.find( @@ -279,7 +280,7 @@ export default Component.extend(DEFAULTS, { }, handleError(e) { this.setProperties({ - loading: false, + isLoading: false, error: e ? this.auth.handleError(e) : null, }); }, diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js index 6d200fd74744..f5ae2a44fb5a 100644 --- a/ui/app/components/auth-jwt.js +++ b/ui/app/components/auth-jwt.js @@ -178,8 +178,14 @@ export default Component.extend({ if (!this.isOIDC || !this.role || !this.role.authUrl) { return; } - - await this.fetchRole.perform(this.roleName, { debounce: false }); + 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; + } + } let win = this.getWindow(); const POPUP_WIDTH = 500; From b0f5b56e74c0f8a2eed0be5572efb4e59e3d2c7c Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Tue, 5 Apr 2022 11:17:00 -0600 Subject: [PATCH 2/4] adds changelog entry --- changelog/14916.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/14916.txt diff --git a/changelog/14916.txt b/changelog/14916.txt new file mode 100644 index 000000000000..d61065a8de48 --- /dev/null +++ b/changelog/14916.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fixes issue logging in with OIDC from a listed auth mounts tab +``` \ No newline at end of file From 69965d7c2043b60dc3c4b3b4ee75ea16de845adf Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Wed, 6 Apr 2022 14:23:01 -0600 Subject: [PATCH 3/4] adds more tests for oidc auth workflow --- ...ethod-test.js => oidc-auth-method-test.js} | 57 ++++++++++++++++--- 1 file changed, 48 insertions(+), 9 deletions(-) rename ui/tests/acceptance/{logout-auth-method-test.js => oidc-auth-method-test.js} (51%) diff --git a/ui/tests/acceptance/logout-auth-method-test.js b/ui/tests/acceptance/oidc-auth-method-test.js similarity index 51% rename from ui/tests/acceptance/logout-auth-method-test.js rename to ui/tests/acceptance/oidc-auth-method-test.js index 03a6ac85fd26..650a8d8514a0 100644 --- a/ui/tests/acceptance/logout-auth-method-test.js +++ b/ui/tests/acceptance/oidc-auth-method-test.js @@ -6,19 +6,12 @@ import { fakeWindow, buildMessage } from '../helpers/oidc-window-stub'; import sinon from 'sinon'; import { later, _cancelTimers as cancelTimers } from '@ember/runloop'; -module('Acceptance | logout auth method', function (hooks) { +module('Acceptance | oidc auth method', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); hooks.beforeEach(function () { this.openStub = sinon.stub(window, 'open').callsFake(() => fakeWindow.create()); - }); - hooks.afterEach(function () { - this.openStub.restore(); - }); - - // coverage for bug where token was selected as auth method for oidc and jwt - test('it should populate oidc auth method on logout', async function (assert) { this.server.post('/auth/oidc/oidc/auth_url', () => ({ data: { auth_url: 'http://example.com' }, })); @@ -27,13 +20,59 @@ module('Acceptance | logout auth method', function (hooks) { })); // ensure clean state sessionStorage.removeItem('selectedAuth'); + }); + hooks.afterEach(function () { + this.openStub.restore(); + }); + + const login = async (select) => { await visit('/vault/auth'); - await fillIn('[data-test-select="auth-method"]', 'oidc'); + // select from dropdown or click auth path tab + if (select) { + await fillIn('[data-test-select="auth-method"]', 'oidc'); + } else { + await click('[data-test-auth-method-link="oidc"]'); + } later(() => { window.postMessage(buildMessage().data, window.origin); cancelTimers(); }, 50); await click('[data-test-auth-submit]'); + }; + + test('it should login with oidc when selected from auth methods dropdown', async function (assert) { + assert.expect(1); + + this.server.get('/auth/token/lookup-self', (schema, req) => { + assert.ok(true, 'request made to auth/token/lookup-self after oidc callback'); + req.passthrough(); + }); + + await login(true); + }); + + test('it should login with oidc from listed auth mount tab', async function (assert) { + assert.expect(1); + + this.server.get('/sys/internal/ui/mounts', () => ({ + data: { + auth: { + 'oidc/': { description: '', options: {}, type: 'oidc' }, + }, + }, + })); + // there was a bug that would result in the /auth/:path/login endpoint hit with an empty payload rather than lookup-self + // ensure that the correct endpoint is hit after the oidc callback + this.server.get('/auth/token/lookup-self', (schema, req) => { + assert.ok(true, 'request made to auth/token/lookup-self after oidc callback'); + req.passthrough(); + }); + await login(); + }); + + // coverage for bug where token was selected as auth method for oidc and jwt + test('it should populate oidc auth method on logout', async function (assert) { + await login(true); await click('.nav-user-button button'); await click('#logout'); assert From a828260678b594d6df0906ca7d188d79466f7970 Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Wed, 6 Apr 2022 16:31:47 -0600 Subject: [PATCH 4/4] updates oidc auth method test to use non-standard path --- ui/tests/acceptance/oidc-auth-method-test.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/ui/tests/acceptance/oidc-auth-method-test.js b/ui/tests/acceptance/oidc-auth-method-test.js index 650a8d8514a0..d90304f3669a 100644 --- a/ui/tests/acceptance/oidc-auth-method-test.js +++ b/ui/tests/acceptance/oidc-auth-method-test.js @@ -52,15 +52,24 @@ module('Acceptance | oidc auth method', function (hooks) { }); test('it should login with oidc from listed auth mount tab', async function (assert) { - assert.expect(1); + assert.expect(2); this.server.get('/sys/internal/ui/mounts', () => ({ data: { auth: { - 'oidc/': { description: '', options: {}, type: 'oidc' }, + 'test-path/': { description: '', options: {}, type: 'oidc' }, }, }, })); + let didAssert; + this.server.post('/auth/test-path/oidc/auth_url', () => { + // request may be fired more than once -- we are only concerned if the endpoint is hit, not how many times + if (!didAssert) { + assert.ok(true, 'auth_url request made to correct non-standard mount path'); + didAssert = true; + } + return { data: { auth_url: 'http://example.com' } }; + }); // there was a bug that would result in the /auth/:path/login endpoint hit with an empty payload rather than lookup-self // ensure that the correct endpoint is hit after the oidc callback this.server.get('/auth/token/lookup-self', (schema, req) => {