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
This commit is contained in:
Jordan Reimer 2022-10-26 15:34:43 -06:00 committed by GitHub
parent cc96c6f470
commit 571851cee3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 154 additions and 75 deletions

3
changelog/17661.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
ui: Fixes oidc/jwt login issue with alternate mount path and jwt login via mount path tab
```

View File

@ -2,7 +2,6 @@ import Ember from 'ember';
import { next } from '@ember/runloop'; import { next } from '@ember/runloop';
import { inject as service } from '@ember/service'; import { inject as service } from '@ember/service';
import { match, alias, or } from '@ember/object/computed'; import { match, alias, or } from '@ember/object/computed';
import { assign } from '@ember/polyfills';
import { dasherize } from '@ember/string'; import { dasherize } from '@ember/string';
import Component from '@ember/component'; import Component from '@ember/component';
import { computed } from '@ember/object'; import { computed } from '@ember/object';
@ -284,25 +283,24 @@ export default Component.extend(DEFAULTS, {
}), }),
actions: { actions: {
doSubmit(passedData, event) { doSubmit(passedData, event, token) {
if (event) { if (event) {
event.preventDefault(); event.preventDefault();
} }
let data = {}; if (token) {
this.setProperties({ this.set('token', token);
error: null, }
}); this.set('error', null);
// if callback from oidc we have a token at this point // if callback from oidc or jwt we have a token at this point
let backend = const backend = token ? this.getAuthBackend('token') : this.selectedAuthBackend || {};
this.providerName === 'oidc' ? this.getAuthBackend('token') : this.selectedAuthBackend || {}; const backendMeta = BACKENDS.find(
let backendMeta = BACKENDS.find(
(b) => (b.type || '').toLowerCase() === (backend.type || '').toLowerCase() (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) { if (passedData) {
data = assign(data, passedData); Object.assign(data, passedData);
} }
if (this.customPath || backend.id) { if (this.customPath || backend.id) {
data.path = this.customPath || backend.id; data.path = this.customPath || backend.id;

View File

@ -18,6 +18,7 @@ export { ERROR_WINDOW_CLOSED, ERROR_MISSING_PARAMS, ERROR_JWT_LOGIN };
export default Component.extend({ export default Component.extend({
store: service(), store: service(),
featureFlagService: service('featureFlag'), featureFlagService: service('featureFlag'),
selectedAuthPath: null, selectedAuthPath: null,
selectedAuthType: null, selectedAuthType: null,
roleName: null, roleName: null,
@ -26,22 +27,18 @@ export default Component.extend({
onRoleName() {}, onRoleName() {},
onLoading() {}, onLoading() {},
onError() {}, onError() {},
onToken() {},
onNamespace() {}, onNamespace() {},
didReceiveAttrs() { didReceiveAttrs() {
this._super(); this._super();
let { oldSelectedAuthPath, selectedAuthPath } = this; const debounce = !this.oldSelectedAuthPath && !this.selectedAuthPath;
let shouldDebounce = !oldSelectedAuthPath && !selectedAuthPath;
if (oldSelectedAuthPath !== selectedAuthPath) { if (this.oldSelectedAuthPath !== this.selectedAuthPath || debounce) {
this.set('role', null); this.fetchRole.perform(this.roleName, { debounce });
this.onRoleName(this.roleName);
this.fetchRole.perform(null, { debounce: false });
} else if (shouldDebounce) {
this.fetchRole.perform(this.roleName);
} }
this.set('errorMessage', null); 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 // Assumes authentication using OIDC until it's known that the mount is
@ -165,9 +162,7 @@ export default Component.extend({
} catch (e) { } catch (e) {
return this.handleOIDCError(e); return this.handleOIDCError(e);
} }
let token = resp.auth.client_token; yield this.onSubmit(null, null, resp.auth.client_token);
this.onToken(token);
yield this.onSubmit();
}), }),
actions: { actions: {
@ -177,6 +172,14 @@ export default Component.extend({
e.preventDefault(); e.preventDefault();
} }
if (!this.isOIDC || !this.role || !this.role.authUrl) { 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; return;
} }
try { try {

View File

@ -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
* <OktaNumberChallenge @correctAnswer={this.oktaNumberChallengeAnswer} @hasError={this.error} @onReturnToLogin={this.returnToLoginFromOktaNumberChallenge}/>
* ```
* @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;
}
}

View File

@ -1,5 +1,5 @@
<div class="auth-form" data-test-auth-form> <div class="auth-form" data-test-auth-form>
{{#if (or (this.error) (and this.waitingForOktaNumberChallenge (not this.cancelAuthForOktaNumberChallenge)))}} {{#if (and this.waitingForOktaNumberChallenge (not this.cancelAuthForOktaNumberChallenge))}}
<OktaNumberChallenge <OktaNumberChallenge
@correctAnswer={{this.oktaNumberChallengeAnswer}} @correctAnswer={{this.oktaNumberChallengeAnswer}}
@hasError={{(not-eq this.error null)}} @hasError={{(not-eq this.error null)}}
@ -74,7 +74,6 @@
<AuthJwt <AuthJwt
@onError={{action "handleError"}} @onError={{action "handleError"}}
@onLoading={{action (mut this.isLoading)}} @onLoading={{action (mut this.isLoading)}}
@onToken={{action (mut this.token)}}
@namespace={{this.namespace}} @namespace={{this.namespace}}
@onNamespace={{action (mut this.namespace)}} @onNamespace={{action (mut this.namespace)}}
@onSubmit={{action "doSubmit"}} @onSubmit={{action "doSubmit"}}

View File

@ -3,7 +3,7 @@
<div class="field has-top-margin-xs"> <div class="field has-top-margin-xs">
<p data-test-okta-number-challenge-description> <p data-test-okta-number-challenge-description>
To finish signing in, you will need to complete an additional MFA step.</p> To finish signing in, you will need to complete an additional MFA step.</p>
{{#if this.errorThrown}} {{#if @hasError}}
<div class="has-top-margin-s"> <div class="has-top-margin-s">
<MessageError @errorMessage="There was a problem" /> <MessageError @errorMessage="There was a problem" />
<button <button
@ -13,7 +13,7 @@
data-test-return-from-okta-number-challenge data-test-return-from-okta-number-challenge
>Return to login</button> >Return to login</button>
</div> </div>
{{else if this.oktaNumberChallengeCorrectAnswer}} {{else if @correctAnswer}}
<div class="has-top-margin-s"> <div class="has-top-margin-s">
<p class="has-text-black has-text-weight-semibold" data-test-okta-number-challenge-verification-type>Okta <p class="has-text-black has-text-weight-semibold" data-test-okta-number-challenge-verification-type>Okta
verification</p> verification</p>
@ -21,7 +21,7 @@
<h1 <h1
class="title has-font-weight-normal has-top-margin-m has-bottom-margin-s" class="title has-font-weight-normal has-top-margin-m has-bottom-margin-s"
data-test-okta-number-challenge-answer data-test-okta-number-challenge-answer
>{{this.oktaNumberChallengeCorrectAnswer}}</h1> >{{@correctAnswer}}</h1>
</div> </div>
{{else}} {{else}}
<div class="has-top-margin-l has-bottom-margin-m"> <div class="has-top-margin-l has-bottom-margin-m">

View File

@ -269,4 +269,49 @@ module('Integration | Component | auth form', function (hooks) {
); );
server.shutdown(); 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`<AuthForm @cluster={{this.cluster}} />`);
await component.selectMethod('oidc');
await component.oidcRole('foo');
await component.oidcMoreOptions();
await component.oidcMountPath('foo-oidc');
await component.login();
server.shutdown();
});
}); });

View File

@ -50,7 +50,6 @@ const renderIt = async (context, path = 'jwt') => {
@selectedAuthPath={{this.selectedAuthPath}} @selectedAuthPath={{this.selectedAuthPath}}
@onError={{action (mut this.error)}} @onError={{action (mut this.error)}}
@onLoading={{action (mut this.isLoading)}} @onLoading={{action (mut this.isLoading)}}
@onToken={{action (mut this.token)}}
@onNamespace={{action (mut this.namespace)}} @onNamespace={{action (mut this.namespace)}}
@onSelectedAuth={{action (mut this.selectedAuth)}} @onSelectedAuth={{action (mut this.selectedAuth)}}
@onSubmit={{action this.handler}} @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)]; return [200, { 'Content-Type': 'application/json' }, JSON.stringify(OIDC_AUTH_RESPONSE)];
}); });
this.post('/v1/auth/:path/oidc/auth_url', (request) => { this.post('/v1/auth/:path/oidc/auth_url', (request) => {
let body = JSON.parse(request.requestBody); const { role } = JSON.parse(request.requestBody);
if (body.role === 'test') { if (['test', 'okta', 'bar'].includes(role)) {
const auth_url = role === 'test' ? 'http://example.com' : role === 'okta' ? 'http://okta.com' : '';
return [ return [
200, 200,
{ 'Content-Type': 'application/json' }, { 'Content-Type': 'application/json' },
JSON.stringify({ JSON.stringify({
data: { data: { auth_url },
auth_url: 'http://example.com',
},
}), }),
]; ];
} }
if (body.role === 'okta') { const errors = role === 'foo' ? ['role "foo" could not be found'] : [ERROR_JWT_LOGIN];
return [ return [400, { 'Content-Type': 'application/json' }, JSON.stringify({ errors })];
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] })];
}); });
}); });
}); });
@ -209,8 +197,7 @@ module('Integration | Component | auth jwt', function (hooks) {
}); });
this.window.trigger('message', buildMessage()); this.window.trigger('message', buildMessage());
await settled(); await settled();
assert.strictEqual(this.token, 'token', 'calls onToken with token'); assert.ok(this.handler.withArgs(null, null, 'token').calledOnce, 'calls the onSubmit handler with token');
assert.ok(this.handler.calledOnce, 'calls the onSubmit handler');
}); });
test('oidc: fails silently when event origin does not match window origin', async function (assert) { 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(); await settled();
assert.notOk(this.handler.called, 'should not call the submit handler'); 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'
);
});
}); });

View File

@ -14,4 +14,7 @@ export default {
errorMessagePresent: isPresent('[data-test-auth-error]'), errorMessagePresent: isPresent('[data-test-auth-error]'),
descriptionText: text('[data-test-description]'), descriptionText: text('[data-test-description]'),
login: clickable('[data-test-auth-submit]'), login: clickable('[data-test-auth-submit]'),
oidcRole: fillable('[data-test-role]'),
oidcMoreOptions: clickable('[data-test-yield-content] button'),
oidcMountPath: fillable('#custom-path'),
}; };

View File

@ -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']);
}
});
});