From 5e54d49d9b319786e3e22f8e6c176c2216e2c172 Mon Sep 17 00:00:00 2001 From: Jordan Reimer Date: Fri, 18 Feb 2022 08:22:39 -0700 Subject: [PATCH] updates mfa error handling (#14147) --- ui/app/components/mfa-error.js | 43 --------------- ui/app/components/mfa-form.js | 23 +++++--- ui/app/controllers/vault/cluster/auth.js | 4 ++ ui/app/services/auth.js | 18 +----- ui/app/templates/components/auth-form.hbs | 2 +- ui/app/templates/components/mfa-error.hbs | 15 ----- ui/app/templates/components/mfa-form.hbs | 2 +- ui/app/templates/vault/cluster/auth.hbs | 16 +++++- ui/mirage/handlers/mfa.js | 7 ++- ui/tests/acceptance/mfa-test.js | 17 ++++++ .../integration/components/mfa-error-test.js | 38 ------------- .../integration/components/mfa-form-test.js | 55 +++++++++++++------ 12 files changed, 99 insertions(+), 141 deletions(-) delete mode 100644 ui/app/components/mfa-error.js delete mode 100644 ui/app/templates/components/mfa-error.hbs delete mode 100644 ui/tests/integration/components/mfa-error-test.js diff --git a/ui/app/components/mfa-error.js b/ui/app/components/mfa-error.js deleted file mode 100644 index ed894a051a4c..000000000000 --- a/ui/app/components/mfa-error.js +++ /dev/null @@ -1,43 +0,0 @@ -import Component from '@glimmer/component'; -import { inject as service } from '@ember/service'; -import { action } from '@ember/object'; -import { TOTP_NOT_CONFIGURED } from 'vault/services/auth'; - -const TOTP_NA_MSG = - 'Multi-factor authentication is required, but you have not set it up. In order to do so, please contact your administrator.'; -const MFA_ERROR_MSG = - 'Multi-factor authentication is required, but failed. Go back and try again, or contact your administrator.'; - -export { TOTP_NA_MSG, MFA_ERROR_MSG }; - -/** - * @module MfaError - * MfaError components are used to display mfa errors - * - * @example - * ```js - * - * ``` - */ - -export default class MfaError extends Component { - @service auth; - - get isTotp() { - return this.auth.mfaErrors.includes(TOTP_NOT_CONFIGURED); - } - get title() { - return this.isTotp ? 'TOTP not set up' : 'Unauthorized'; - } - get description() { - return this.isTotp ? TOTP_NA_MSG : MFA_ERROR_MSG; - } - - @action - onClose() { - this.auth.set('mfaErrors', null); - if (this.args.onClose) { - this.args.onClose(); - } - } -} diff --git a/ui/app/components/mfa-form.js b/ui/app/components/mfa-form.js index 6ba69b32035e..b88410580041 100644 --- a/ui/app/components/mfa-form.js +++ b/ui/app/components/mfa-form.js @@ -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; @@ -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--; diff --git a/ui/app/controllers/vault/cluster/auth.js b/ui/app/controllers/vault/cluster/auth.js index 3e98db58ee73..0959cbb920ce 100644 --- a/ui/app/controllers/vault/cluster/auth.js +++ b/ui/app/controllers/vault/cluster/auth.js @@ -80,5 +80,9 @@ export default Controller.extend({ onMfaSuccess(authResponse) { this.authSuccess(authResponse); }, + onMfaErrorDismiss() { + this.set('mfaAuthData', null); + this.auth.set('mfaErrors', null); + }, }, }); diff --git a/ui/app/services/auth.js b/ui/app/services/auth.js index b25e502828c8..ec1db73f06fd 100644 --- a/ui/app/services/auth.js +++ b/ui/app/services/auth.js @@ -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(), @@ -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 }; diff --git a/ui/app/templates/components/auth-form.hbs b/ui/app/templates/components/auth-form.hbs index 654a6b43fd1d..9bc02fa8289e 100644 --- a/ui/app/templates/components/auth-form.hbs +++ b/ui/app/templates/components/auth-form.hbs @@ -1,4 +1,4 @@ -
+
{{#if this.hasMethodsWithPath}}