Skip to content

Commit

Permalink
updates mfa error handling (#14147)
Browse files Browse the repository at this point in the history
  • Loading branch information
zofskeez authored Feb 18, 2022
1 parent 984bf7f commit 5e54d49
Show file tree
Hide file tree
Showing 12 changed files with 99 additions and 141 deletions.
43 changes: 0 additions & 43 deletions ui/app/components/mfa-error.js

This file was deleted.

23 changes: 15 additions & 8 deletions ui/app/components/mfa-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ import { numberToWord } from 'vault/helpers/number-to-word';
* @param {function} onSuccess - fired when passcode passes validation
*/

export const VALIDATION_ERROR =
'The passcode failed to validate. If you entered the correct passcode, contact your administrator.';

export default class MfaForm extends Component {
@service auth;

@tracked passcode;
@tracked countdown;
@tracked errors;
@tracked error;

get constraints() {
return this.args.authData.mfa_requirement.mfa_constraints;
Expand Down Expand Up @@ -57,21 +59,26 @@ export default class MfaForm extends Component {

@task *validate() {
try {
this.error = null;
const response = yield this.auth.totpValidate({
clusterId: this.args.clusterId,
...this.args.authData,
});
this.args.onSuccess(response);
} catch (error) {
this.errors = error.errors;
// TODO: update if specific error can be parsed for incorrect passcode
// this.newCodeDelay.perform();
const codeUsed = (error.errors || []).find((e) => e.includes('code already used;'));
if (codeUsed) {
// parse validity period from error string to initialize countdown
const seconds = parseInt(codeUsed.split('in ')[1].split(' seconds')[0]);
this.newCodeDelay.perform(seconds);
} else {
this.error = VALIDATION_ERROR;
}
}
}

@task *newCodeDelay() {
this.passcode = null;
this.countdown = 30;
@task *newCodeDelay(timePeriod) {
this.countdown = timePeriod;
while (this.countdown) {
yield timeout(1000);
this.countdown--;
Expand Down
4 changes: 4 additions & 0 deletions ui/app/controllers/vault/cluster/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,9 @@ export default Controller.extend({
onMfaSuccess(authResponse) {
this.authSuccess(authResponse);
},
onMfaErrorDismiss() {
this.set('mfaAuthData', null);
this.auth.set('mfaErrors', null);
},
},
});
18 changes: 3 additions & 15 deletions ui/app/services/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ import { task, timeout } from 'ember-concurrency';
const TOKEN_SEPARATOR = '☃';
const TOKEN_PREFIX = 'vault-';
const ROOT_PREFIX = '_root_';
const TOTP_NOT_CONFIGURED = 'TOTP mfa required but not configured';
const BACKENDS = supportedAuthBackends();

export { TOKEN_SEPARATOR, TOKEN_PREFIX, ROOT_PREFIX, TOTP_NOT_CONFIGURED };
export { TOKEN_SEPARATOR, TOKEN_PREFIX, ROOT_PREFIX };

export default Service.extend({
permissions: service(),
Expand Down Expand Up @@ -362,21 +361,10 @@ export default Service.extend({
async authenticate(/*{clusterId, backend, data}*/) {
const [options] = arguments;
const adapter = this.clusterAdapter();
let resp;

try {
resp = await adapter.authenticate(options);
} catch (e) {
// TODO: check for totp not configured mfa error before throwing
const errors = this.handleError(e);
// stubbing error - verify once API is finalized
if (errors.includes(TOTP_NOT_CONFIGURED)) {
this.set('mfaErrors', errors);
}
throw e;
}

let resp = await adapter.authenticate(options);
const { mfa_requirement, requiresAction } = this._parseMfaResponse(resp.auth?.mfa_requirement);

if (mfa_requirement) {
if (requiresAction) {
return { mfa_requirement };
Expand Down
2 changes: 1 addition & 1 deletion ui/app/templates/components/auth-form.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="auth-form">
<div class="auth-form" data-test-auth-form>
{{#if this.hasMethodsWithPath}}
<nav class="tabs is-marginless">
<ul>
Expand Down
15 changes: 0 additions & 15 deletions ui/app/templates/components/mfa-error.hbs

This file was deleted.

2 changes: 1 addition & 1 deletion ui/app/templates/components/mfa-form.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
{{this.description}}
</p>
<form id="auth-form" {{on "submit" this.submit}}>
<MessageError @errors={{this.errors}} class="has-top-margin-s" />
<MessageError @errorMessage={{this.error}} class="has-top-margin-s" />
<div class="field has-top-margin-l">
{{#each this.constraints as |constraint index|}}
{{#if index}}
Expand Down
16 changes: 15 additions & 1 deletion ui/app/templates/vault/cluster/auth.hbs
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
<SplashPage @hasAltContent={{this.auth.mfaErrors}} as |Page|>
<Page.altContent>
<MfaError @onClose={{fn (mut this.mfaAuthData) null}} />
<div class="has-top-margin-xxl" data-test-mfa-error>
<EmptyState
@title="Unauthorized"
@message="Multi-factor authentication is required, but failed. Go back and try again, or contact your administrator."
@icon="alert-circle"
@bottomBorder={{true}}
@subTitle={{join ". " this.auth.mfaErrors}}
class="is-box-shadowless"
>
<button type="button" class="button is-ghost is-transparent" {{on "click" (action "onMfaErrorDismiss")}}>
<Icon @name="chevron-left" />
Go back
</button>
</EmptyState>
</div>
</Page.altContent>
<Page.header>
{{#if this.oidcProvider}}
Expand Down
7 changes: 6 additions & 1 deletion ui/mirage/handlers/mfa.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export default function (server) {
[mfa_constraints, methods] = generator([m('totp')], [m('okta')]); // 2 constraints 1 passcode 1 non-passcode
} else if (user === 'mfa-i') {
[mfa_constraints, methods] = generator([m('okta'), m('totp')], [m('totp')]); // 2 constraints 1 passcode/1 non-passcode 1 non-passcode
} else if (user === 'mfa-j') {
[mfa_constraints, methods] = generator([m('pingid')]); // use to test push failures
}
const numbers = (length) =>
Math.random()
Expand Down Expand Up @@ -129,7 +131,10 @@ export default function (server) {
const passcode = mfa_payload[constraintId][0];
if (method.uses_passcode) {
if (passcode !== 'test') {
const error = !passcode ? 'TOTP passcode not provided' : 'Incorrect TOTP passcode provided';
const error =
passcode === 'used'
? 'code already used; new code is available in 30 seconds'
: 'failed to validate';
return new Response(403, {}, { errors: [error] });
}
} else if (passcode) {
Expand Down
17 changes: 17 additions & 0 deletions ui/tests/acceptance/mfa-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,21 @@ module('Acceptance | mfa', function (hooks) {
await click('[data-test-mfa-validate]');
didLogin(assert);
});

test('it should render unauthorized message for push failure', async function (assert) {
await login('mfa-j');
assert.dom('[data-test-auth-form]').doesNotExist('Auth form hidden when mfa fails');
assert.dom('[data-test-empty-state-title]').hasText('Unauthorized', 'Error title renders');
assert
.dom('[data-test-empty-state-subText]')
.hasText('PingId MFA validation failed', 'Error message from server renders');
assert
.dom('[data-test-empty-state-message]')
.hasText(
'Multi-factor authentication is required, but failed. Go back and try again, or contact your administrator.',
'Error description renders'
);
await click('[data-test-mfa-error] button');
assert.dom('[data-test-auth-form]').exists('Auth form renders after mfa error dismissal');
});
});
38 changes: 0 additions & 38 deletions ui/tests/integration/components/mfa-error-test.js

This file was deleted.

55 changes: 37 additions & 18 deletions ui/tests/integration/components/mfa-form-test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { module, test, skip } from 'qunit';
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
import { setupMirage } from 'ember-cli-mirage/test-support';
import { fillIn, click, waitUntil } from '@ember/test-helpers';
import { run, later } from '@ember/runloop';
import { VALIDATION_ERROR } from 'vault/components/mfa-form';

module('Integration | Component | mfa-form', function (hooks) {
setupRenderingTest(hooks);
Expand All @@ -17,6 +18,14 @@ module('Integration | Component | mfa-form', function (hooks) {
data: { username: 'foo', password: 'bar' },
};
this.authService = this.owner.lookup('service:auth');
// setup basic totp mfa_requirement
// override in tests that require different scenarios
this.totpConstraint = this.server.create('mfa-method', { type: 'totp' });
const { mfa_requirement } = this.authService._parseMfaResponse({
mfa_request_id: 'test-mfa-id',
mfa_constraints: { test_mfa: { any: [this.totpConstraint] } },
});
this.mfaAuthData.mfa_requirement = mfa_requirement;
});

test('it should render correct descriptions', async function (assert) {
Expand Down Expand Up @@ -115,22 +124,11 @@ module('Integration | Component | mfa-form', function (hooks) {
});

test('it should validate mfa requirement', async function (assert) {
const totpConstraint = this.server.create('mfa-method', { type: 'totp' });
const { mfa_requirement } = this.authService._parseMfaResponse({
mfa_request_id: 'test-mfa-id',
mfa_constraints: {
test_mfa: {
any: [totpConstraint],
},
},
});
this.mfaAuthData.mfa_requirement = mfa_requirement;

this.server.post('/sys/mfa/validate', (schema, req) => {
const json = JSON.parse(req.requestBody);
const payload = {
mfa_request_id: 'test-mfa-id',
mfa_payload: { [totpConstraint.id]: ['test-code'] },
mfa_payload: { [this.totpConstraint.id]: ['test-code'] },
};
assert.deepEqual(json, payload, 'Correct mfa payload passed to validate endpoint');
return {};
Expand Down Expand Up @@ -164,11 +162,10 @@ module('Integration | Component | mfa-form', function (hooks) {
await click('[data-test-mfa-validate]');
});

// commented out in component until specific error code can be parsed from the api response
skip('it should show countdown on passcode validation failure', async function (assert) {
test('it should show countdown on passcode already used error', async function (assert) {
this.owner.lookup('service:auth').reopen({
totpValidate() {
throw new Error('Incorrect passcode');
throw { errors: ['code already used; new code is available in 45 seconds'] };
},
});
await render(hbs`
Expand All @@ -181,10 +178,32 @@ module('Integration | Component | mfa-form', function (hooks) {
await fillIn('[data-test-mfa-passcode]', 'test-code');
later(() => run.cancelTimers(), 50);
await click('[data-test-mfa-validate]');
assert
.dom('[data-test-mfa-countdown]')
.hasText('45', 'countdown renders with correct initial value from error response');
assert.dom('[data-test-mfa-validate]').isDisabled('Button is disabled during countdown');
assert.dom('[data-test-mfa-passcode]').isDisabled('Input is disabled during countdown');
assert.dom('[data-test-mfa-passcode]').hasNoValue('Input value is cleared on error');
assert.dom('[data-test-inline-error-message]').exists('Alert message renders');
assert.dom('[data-test-mfa-countdown]').exists('30 second countdown renders');
});

test('it should show error message for passcode invalid error', async function (assert) {
this.owner.lookup('service:auth').reopen({
totpValidate() {
throw { errors: ['failed to validate'] };
},
});
await render(hbs`
<MfaForm
@clusterId={{this.clusterId}}
@authData={{this.mfaAuthData}}
/>
`);

await fillIn('[data-test-mfa-passcode]', 'test-code');
later(() => run.cancelTimers(), 50);
await click('[data-test-mfa-validate]');
assert
.dom('[data-test-error]')
.includesText(VALIDATION_ERROR, 'Generic error message renders for passcode validation error');
});
});

0 comments on commit 5e54d49

Please sign in to comment.