From 571851cee30e1c07f53c9b5584e0d21029c49073 Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Wed, 26 Oct 2022 15:34:43 -0600 Subject: [PATCH] OIDC Alternate Path Bug (#17661) * adds error handling to auth-jwt component for missing roles and fixes bug where role wasn't being retained when using alternate oidc mount path at login * fixes jwt login bug from auth mount tabs and adds test * updates okta-number-challenge success value to arg in template * adds changelog entry * fixes issues logging in manually with jwt * reverts mistaken change --- changelog/17661.txt | 3 ++ ui/app/components/auth-form.js | 24 +++++---- ui/app/components/auth-jwt.js | 29 ++++++----- ui/app/components/okta-number-challenge.js | 24 --------- ui/app/templates/components/auth-form.hbs | 3 +- .../components/okta-number-challenge.hbs | 6 +-- .../integration/components/auth-form-test.js | 45 +++++++++++++++++ .../integration/components/auth-jwt-test.js | 49 +++++++++++-------- ui/tests/pages/components/auth-form.js | 3 ++ ui/tests/unit/components/auth-form-test.js | 43 ++++++++++++++++ 10 files changed, 154 insertions(+), 75 deletions(-) create mode 100644 changelog/17661.txt delete mode 100644 ui/app/components/okta-number-challenge.js create mode 100644 ui/tests/unit/components/auth-form-test.js diff --git a/changelog/17661.txt b/changelog/17661.txt new file mode 100644 index 000000000..5dfb8ea76 --- /dev/null +++ b/changelog/17661.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Fixes oidc/jwt login issue with alternate mount path and jwt login via mount path tab +``` \ No newline at end of file diff --git a/ui/app/components/auth-form.js b/ui/app/components/auth-form.js index 9b753b2bd..c5ebc4c7b 100644 --- a/ui/app/components/auth-form.js +++ b/ui/app/components/auth-form.js @@ -2,7 +2,6 @@ import Ember from 'ember'; import { next } from '@ember/runloop'; import { inject as service } from '@ember/service'; import { match, alias, or } from '@ember/object/computed'; -import { assign } from '@ember/polyfills'; import { dasherize } from '@ember/string'; import Component from '@ember/component'; import { computed } from '@ember/object'; @@ -284,25 +283,24 @@ export default Component.extend(DEFAULTS, { }), actions: { - doSubmit(passedData, event) { + doSubmit(passedData, event, token) { if (event) { event.preventDefault(); } - let data = {}; - this.setProperties({ - error: null, - }); - // if callback from oidc we have a token at this point - let backend = - this.providerName === 'oidc' ? this.getAuthBackend('token') : this.selectedAuthBackend || {}; - let backendMeta = BACKENDS.find( + if (token) { + this.set('token', token); + } + this.set('error', null); + // if callback from oidc or jwt we have a token at this point + const backend = token ? this.getAuthBackend('token') : this.selectedAuthBackend || {}; + const backendMeta = BACKENDS.find( (b) => (b.type || '').toLowerCase() === (backend.type || '').toLowerCase() ); - let attributes = (backendMeta || {}).formAttributes || []; + const attributes = (backendMeta || {}).formAttributes || []; + const data = this.getProperties(...attributes); - data = assign(data, this.getProperties(...attributes)); if (passedData) { - data = assign(data, passedData); + Object.assign(data, passedData); } if (this.customPath || backend.id) { data.path = this.customPath || backend.id; diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js index 1605aadea..9e6e5ae13 100644 --- a/ui/app/components/auth-jwt.js +++ b/ui/app/components/auth-jwt.js @@ -18,6 +18,7 @@ export { ERROR_WINDOW_CLOSED, ERROR_MISSING_PARAMS, ERROR_JWT_LOGIN }; export default Component.extend({ store: service(), featureFlagService: service('featureFlag'), + selectedAuthPath: null, selectedAuthType: null, roleName: null, @@ -26,22 +27,18 @@ export default Component.extend({ onRoleName() {}, onLoading() {}, onError() {}, - onToken() {}, onNamespace() {}, didReceiveAttrs() { this._super(); - let { oldSelectedAuthPath, selectedAuthPath } = this; - let shouldDebounce = !oldSelectedAuthPath && !selectedAuthPath; - if (oldSelectedAuthPath !== selectedAuthPath) { - this.set('role', null); - this.onRoleName(this.roleName); - this.fetchRole.perform(null, { debounce: false }); - } else if (shouldDebounce) { - this.fetchRole.perform(this.roleName); + const debounce = !this.oldSelectedAuthPath && !this.selectedAuthPath; + + if (this.oldSelectedAuthPath !== this.selectedAuthPath || debounce) { + this.fetchRole.perform(this.roleName, { debounce }); } + this.set('errorMessage', null); - this.set('oldSelectedAuthPath', selectedAuthPath); + this.set('oldSelectedAuthPath', this.selectedAuthPath); }, // Assumes authentication using OIDC until it's known that the mount is @@ -165,9 +162,7 @@ export default Component.extend({ } catch (e) { return this.handleOIDCError(e); } - let token = resp.auth.client_token; - this.onToken(token); - yield this.onSubmit(); + yield this.onSubmit(null, null, resp.auth.client_token); }), actions: { @@ -177,6 +172,14 @@ export default Component.extend({ e.preventDefault(); } if (!this.isOIDC || !this.role || !this.role.authUrl) { + let message = this.errorMessage; + if (!this.role) { + message = 'Invalid role. Please try again.'; + } else if (!this.role.authUrl) { + message = + 'Missing auth_url. Please check that allowed_redirect_uris for the role include this mount path.'; + } + this.onError(message); return; } try { diff --git a/ui/app/components/okta-number-challenge.js b/ui/app/components/okta-number-challenge.js deleted file mode 100644 index 7c9566e11..000000000 --- a/ui/app/components/okta-number-challenge.js +++ /dev/null @@ -1,24 +0,0 @@ -import Component from '@glimmer/component'; - -/** - * @module OktaNumberChallenge - * OktaNumberChallenge components are used to display loading screen and correct answer for Okta Number Challenge when signing in through Okta - * - * @example - * ```js - * - * ``` - * @param {number} correctAnswer - The correct answer to click for the okta number challenge. - * @param {boolean} hasError - Determines if there is an error being thrown. - * @param {function} onReturnToLogin - Sets waitingForOktaNumberChallenge to false if want to return to main login. - */ - -export default class OktaNumberChallenge extends Component { - get oktaNumberChallengeCorrectAnswer() { - return this.args.correctAnswer; - } - - get errorThrown() { - return this.args.hasError; - } -} diff --git a/ui/app/templates/components/auth-form.hbs b/ui/app/templates/components/auth-form.hbs index 4924ac01c..b17c19b01 100644 --- a/ui/app/templates/components/auth-form.hbs +++ b/ui/app/templates/components/auth-form.hbs @@ -1,5 +1,5 @@
- {{#if (or (this.error) (and this.waitingForOktaNumberChallenge (not this.cancelAuthForOktaNumberChallenge)))}} + {{#if (and this.waitingForOktaNumberChallenge (not this.cancelAuthForOktaNumberChallenge))}}

To finish signing in, you will need to complete an additional MFA step.

- {{#if this.errorThrown}} + {{#if @hasError}}
- {{else if this.oktaNumberChallengeCorrectAnswer}} + {{else if @correctAnswer}}

Okta verification

@@ -21,7 +21,7 @@

{{this.oktaNumberChallengeCorrectAnswer}}

+ >{{@correctAnswer}}
{{else}}
diff --git a/ui/tests/integration/components/auth-form-test.js b/ui/tests/integration/components/auth-form-test.js index 451e75758..7a3ef6ef4 100644 --- a/ui/tests/integration/components/auth-form-test.js +++ b/ui/tests/integration/components/auth-form-test.js @@ -269,4 +269,49 @@ module('Integration | Component | auth form', function (hooks) { ); server.shutdown(); }); + + test('it should retain oidc role when mount path is changed', async function (assert) { + assert.expect(1); + + const auth_url = 'http://dev-foo-bar.com'; + const server = new Pretender(function () { + this.post('/v1/auth/:path/oidc/auth_url', (req) => { + const { role, redirect_uri } = JSON.parse(req.requestBody); + const goodRequest = + req.params.path === 'foo-oidc' && + role === 'foo' && + redirect_uri.includes('/auth/foo-oidc/oidc/callback'); + + return [ + goodRequest ? 200 : 400, + { 'Content-Type': 'application/json' }, + JSON.stringify( + goodRequest ? { data: { auth_url } } : { errors: [`role "${role}" could not be found`] } + ), + ]; + }); + this.get('/v1/sys/internal/ui/mounts', this.passthrough); + }); + + window.open = (url) => { + assert.strictEqual(url, auth_url, 'auth_url is returned when required params are passed'); + }; + + this.owner.lookup('service:router').reopen({ + urlFor(route, { auth_path }) { + return `/auth/${auth_path}/oidc/callback`; + }, + }); + + this.set('cluster', EmberObject.create({})); + await render(hbs``); + + await component.selectMethod('oidc'); + await component.oidcRole('foo'); + await component.oidcMoreOptions(); + await component.oidcMountPath('foo-oidc'); + await component.login(); + + server.shutdown(); + }); }); diff --git a/ui/tests/integration/components/auth-jwt-test.js b/ui/tests/integration/components/auth-jwt-test.js index a974250cf..ab4254378 100644 --- a/ui/tests/integration/components/auth-jwt-test.js +++ b/ui/tests/integration/components/auth-jwt-test.js @@ -50,7 +50,6 @@ const renderIt = async (context, path = 'jwt') => { @selectedAuthPath={{this.selectedAuthPath}} @onError={{action (mut this.error)}} @onLoading={{action (mut this.isLoading)}} - @onToken={{action (mut this.token)}} @onNamespace={{action (mut this.namespace)}} @onSelectedAuth={{action (mut this.selectedAuth)}} @onSubmit={{action this.handler}} @@ -73,30 +72,19 @@ module('Integration | Component | auth jwt', function (hooks) { return [200, { 'Content-Type': 'application/json' }, JSON.stringify(OIDC_AUTH_RESPONSE)]; }); this.post('/v1/auth/:path/oidc/auth_url', (request) => { - let body = JSON.parse(request.requestBody); - if (body.role === 'test') { + const { role } = JSON.parse(request.requestBody); + if (['test', 'okta', 'bar'].includes(role)) { + const auth_url = role === 'test' ? 'http://example.com' : role === 'okta' ? 'http://okta.com' : ''; return [ 200, { 'Content-Type': 'application/json' }, JSON.stringify({ - data: { - auth_url: 'http://example.com', - }, + data: { auth_url }, }), ]; } - if (body.role === 'okta') { - return [ - 200, - { 'Content-Type': 'application/json' }, - JSON.stringify({ - data: { - auth_url: 'http://okta.com', - }, - }), - ]; - } - return [400, { 'Content-Type': 'application/json' }, JSON.stringify({ errors: [ERROR_JWT_LOGIN] })]; + const errors = role === 'foo' ? ['role "foo" could not be found'] : [ERROR_JWT_LOGIN]; + return [400, { 'Content-Type': 'application/json' }, JSON.stringify({ errors })]; }); }); }); @@ -209,8 +197,7 @@ module('Integration | Component | auth jwt', function (hooks) { }); this.window.trigger('message', buildMessage()); await settled(); - assert.strictEqual(this.token, 'token', 'calls onToken with token'); - assert.ok(this.handler.calledOnce, 'calls the onSubmit handler'); + assert.ok(this.handler.withArgs(null, null, 'token').calledOnce, 'calls the onSubmit handler with token'); }); test('oidc: fails silently when event origin does not match window origin', async function (assert) { @@ -240,4 +227,26 @@ module('Integration | Component | auth jwt', function (hooks) { await settled(); assert.notOk(this.handler.called, 'should not call the submit handler'); }); + + test('oidc: it should trigger error callback when role is not found', async function (assert) { + await renderIt(this, 'oidc'); + await component.role('foo'); + await component.login(); + assert.strictEqual( + this.error, + 'Invalid role. Please try again.', + 'Error message is returned when role is not found' + ); + }); + + test('oidc: it should trigger error callback when role is returned without auth_url', async function (assert) { + await renderIt(this, 'oidc'); + await component.role('bar'); + await component.login(); + assert.strictEqual( + this.error, + 'Missing auth_url. Please check that allowed_redirect_uris for the role include this mount path.', + 'Error message is returned when role is returned without auth_url' + ); + }); }); diff --git a/ui/tests/pages/components/auth-form.js b/ui/tests/pages/components/auth-form.js index 4c10de515..07d5efc5f 100644 --- a/ui/tests/pages/components/auth-form.js +++ b/ui/tests/pages/components/auth-form.js @@ -14,4 +14,7 @@ export default { errorMessagePresent: isPresent('[data-test-auth-error]'), descriptionText: text('[data-test-description]'), login: clickable('[data-test-auth-submit]'), + oidcRole: fillable('[data-test-role]'), + oidcMoreOptions: clickable('[data-test-yield-content] button'), + oidcMountPath: fillable('#custom-path'), }; diff --git a/ui/tests/unit/components/auth-form-test.js b/ui/tests/unit/components/auth-form-test.js new file mode 100644 index 000000000..d7b07d460 --- /dev/null +++ b/ui/tests/unit/components/auth-form-test.js @@ -0,0 +1,43 @@ +import { module, test } from 'qunit'; +import { setupTest } from 'ember-qunit'; +import { settled } from '@ember/test-helpers'; + +module('Unit | Component | auth-form', function (hooks) { + setupTest(hooks); + + test('it should use token for oidc and jwt auth method types when processing form submit', async function (assert) { + assert.expect(4); + + const component = this.owner.lookup('component:auth-form'); + component.reopen({ + methods: [], // eslint-disable-line + // eslint-disable-next-line + authenticate: { + unlinked() { + return { + perform(type, data) { + assert.deepEqual( + type, + 'token', + `Token type correctly passed to authenticate method for ${component.providerName}` + ); + assert.deepEqual( + data, + { token: component.token }, + `Token passed to authenticate method for ${component.providerName}` + ); + }, + }; + }, + }, + }); + + const event = new Event('submit'); + + for (const type of ['oidc', 'jwt']) { + component.set('selectedAuth', type); + await settled(); + await component.actions.doSubmit.apply(component, [undefined, event, 'foo-bar']); + } + }); +});