From 4fa3f6c2d3f92f5a32223127452b9278d31d97e6 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 | 32 +++++------- ui/app/components/auth-jwt.js | 29 ++++++----- ui/app/templates/components/auth-form.hbs | 1 - .../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 ++++++++++++++++ 8 files changed, 151 insertions(+), 54 deletions(-) create mode 100644 changelog/17661.txt 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 000000000000..5dfb8ea76b47 --- /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 6d4208ce28c6..67d0246a4d76 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'; @@ -246,31 +245,24 @@ export default Component.extend(DEFAULTS, { }), actions: { - doSubmit() { - let passedData, e; - if (arguments.length > 1) { - [passedData, e] = arguments; - } else { - [e] = arguments; + doSubmit(passedData, event, token) { + if (event) { + event.preventDefault(); } - if (e) { - e.preventDefault(); + if (token) { + this.set('token', token); } - 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( + 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 f5ae2a44fb5a..48bebe99b6ce 100644 --- a/ui/app/components/auth-jwt.js +++ b/ui/app/components/auth-jwt.js @@ -17,6 +17,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, @@ -25,22 +26,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 @@ -164,9 +161,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: { @@ -176,6 +171,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/templates/components/auth-form.hbs b/ui/app/templates/components/auth-form.hbs index 149fb0bec9ca..80b697b5c914 100644 --- a/ui/app/templates/components/auth-form.hbs +++ b/ui/app/templates/components/auth-form.hbs @@ -65,7 +65,6 @@ { + 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 08f21119a750..c0e30cb6c498 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={{selectedAuthPath}} @onError={{action (mut error)}} @onLoading={{action (mut isLoading)}} - @onToken={{action (mut token)}} @onNamespace={{action (mut namespace)}} @onSelectedAuth={{action (mut selectedAuth)}} @onSubmit={{action 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 })]; }); }); }); @@ -205,8 +193,7 @@ module('Integration | Component | auth jwt', function (hooks) { }); this.window.trigger('message', buildMessage()); await settled(); - assert.equal(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) { @@ -236,4 +223,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 4c10de51558b..07d5efc5f0ca 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 000000000000..d7b07d460acd --- /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']); + } + }); +});