Skip to content

Commit

Permalink
OIDC Alternate Path Bug (#17661) (#17688)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
zofskeez committed Oct 27, 2022
1 parent 45b3486 commit 2a57e58
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 82 deletions.
3 changes: 3 additions & 0 deletions changelog/17661.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
ui: Fixes oidc/jwt login issue with alternate mount path and jwt login via mount path tab
```
32 changes: 12 additions & 20 deletions ui/app/components/auth-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -285,31 +284,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;
Expand Down
29 changes: 16 additions & 13 deletions ui/app/components/auth-jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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: {
Expand All @@ -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 {
Expand Down
24 changes: 0 additions & 24 deletions ui/app/components/okta-number-challenge.js

This file was deleted.

3 changes: 1 addition & 2 deletions ui/app/templates/components/auth-form.hbs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<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
@correctAnswer={{this.oktaNumberChallengeAnswer}}
@hasError={{(not-eq this.error null)}}
Expand Down Expand Up @@ -74,7 +74,6 @@
<AuthJwt
@onError={{action "handleError"}}
@onLoading={{action (mut this.isLoading)}}
@onToken={{action (mut this.token)}}
@namespace={{this.namespace}}
@onNamespace={{action (mut this.namespace)}}
@onSubmit={{action "doSubmit"}}
Expand Down
6 changes: 3 additions & 3 deletions ui/app/templates/components/okta-number-challenge.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<div class="field has-top-margin-xs">
<p data-test-okta-number-challenge-description>
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">
<MessageError @errorMessage="There was a problem" />
<button
Expand All @@ -13,15 +13,15 @@
data-test-return-from-okta-number-challenge
>Return to login</button>
</div>
{{else if this.oktaNumberChallengeCorrectAnswer}}
{{else if @correctAnswer}}
<div class="has-top-margin-s">
<p class="has-text-black has-text-weight-semibold" data-test-okta-number-challenge-verification-type>Okta
verification</p>
<p data-test-okta-number-challenge-verification-description>Select the following number to complete verification:</p>
<h1
class="title has-font-weight-normal has-top-margin-m has-bottom-margin-s"
data-test-okta-number-challenge-answer
>{{this.oktaNumberChallengeCorrectAnswer}}</h1>
>{{@correctAnswer}}</h1>
</div>
{{else}}
<div class="has-top-margin-l has-bottom-margin-m">
Expand Down
45 changes: 45 additions & 0 deletions ui/tests/integration/components/auth-form-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,4 +280,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`<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();
});
});
49 changes: 29 additions & 20 deletions ui/tests/integration/components/auth-jwt-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand All @@ -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 })];
});
});
});
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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'
);
});
});
3 changes: 3 additions & 0 deletions ui/tests/pages/components/auth-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
};
43 changes: 43 additions & 0 deletions ui/tests/unit/components/auth-form-test.js
Original file line number Diff line number Diff line change
@@ -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']);
}
});
});

0 comments on commit 2a57e58

Please sign in to comment.