Allow disabling MFA using recovery code

This commit is contained in:
Ricardo Espinoza 2024-11-26 17:06:50 -05:00
parent 1d80225d26
commit cd853e2ffe
No known key found for this signature in database
12 changed files with 143 additions and 52 deletions

View file

@ -68,14 +68,28 @@ describe('Two-factor authentication', { disableAutoLogin: true }, () => {
mainSidebar.actions.signout(); mainSidebar.actions.signout();
}); });
it('Should be able to disable MFA in account', () => { it('Should be able to disable MFA in account with MFA token ', () => {
const { email, password } = user; const { email, password } = user;
signinPage.actions.loginWithEmailAndPassword(email, password); signinPage.actions.loginWithEmailAndPassword(email, password);
personalSettingsPage.actions.enableMfa(); personalSettingsPage.actions.enableMfa();
mainSidebar.actions.signout(); mainSidebar.actions.signout();
const token = generateOTPToken(user.mfaSecret); const loginToken = generateOTPToken(user.mfaSecret);
mfaLoginPage.actions.loginWithMfaToken(email, password, token); mfaLoginPage.actions.loginWithMfaToken(email, password, loginToken);
personalSettingsPage.actions.disableMfa(); const disableToken = generateOTPToken(user.mfaSecret);
personalSettingsPage.actions.disableMfaWithMfaToken(disableToken);
personalSettingsPage.getters.enableMfaButton().should('exist');
mainSidebar.actions.signout();
});
it('Should be able to disable MFA in account with recovery code', () => {
const { email, password } = user;
signinPage.actions.loginWithEmailAndPassword(email, password);
personalSettingsPage.actions.enableMfa();
mainSidebar.actions.signout();
const loginToken = generateOTPToken(user.mfaSecret);
mfaLoginPage.actions.loginWithMfaToken(email, password, loginToken);
personalSettingsPage.actions.disableMfaWitRecoveryCode(user.mfaRecoveryCodes[0]);
personalSettingsPage.getters.enableMfaButton().should('exist');
mainSidebar.actions.signout(); mainSidebar.actions.signout();
}); });
}); });

View file

@ -22,6 +22,9 @@ export class PersonalSettingsPage extends BasePage {
saveSettingsButton: () => cy.getByTestId('save-settings-button'), saveSettingsButton: () => cy.getByTestId('save-settings-button'),
enableMfaButton: () => cy.getByTestId('enable-mfa-button'), enableMfaButton: () => cy.getByTestId('enable-mfa-button'),
disableMfaButton: () => cy.getByTestId('disable-mfa-button'), disableMfaButton: () => cy.getByTestId('disable-mfa-button'),
mfaCodeInput: () => cy.getByTestId('mfa-code-input'),
recoveryCodeInput: () => cy.getByTestId('recovery-code-input'),
mfaSaveButton: () => cy.getByTestId('mfa-save-button'),
themeSelector: () => cy.getByTestId('theme-select'), themeSelector: () => cy.getByTestId('theme-select'),
selectOptionsVisible: () => cy.get('.el-select-dropdown:visible .el-select-dropdown__item'), selectOptionsVisible: () => cy.get('.el-select-dropdown:visible .el-select-dropdown__item'),
}; };
@ -83,9 +86,17 @@ export class PersonalSettingsPage extends BasePage {
mfaSetupModal.getters.saveButton().click(); mfaSetupModal.getters.saveButton().click();
}); });
}, },
disableMfa: () => { disableMfaWithMfaToken: (token: string) => {
cy.visit(this.url); cy.visit(this.url);
this.getters.disableMfaButton().click(); this.getters.disableMfaButton().click();
this.getters.mfaCodeInput().type(token);
this.getters.mfaSaveButton().click();
},
disableMfaWitRecoveryCode: (recoveryCode: string) => {
cy.visit(this.url);
this.getters.disableMfaButton().click();
this.getters.recoveryCodeInput().type(recoveryCode);
this.getters.mfaSaveButton().click();
}, },
}; };
} }

View file

@ -86,13 +86,13 @@ export class MFAController {
@Post('/disable', { rateLimit: true }) @Post('/disable', { rateLimit: true })
async disableMFA(req: MFA.Disable) { async disableMFA(req: MFA.Disable) {
const { id: userId } = req.user; const { id: userId } = req.user;
const { token = null } = req.body; const { token = null, recoveryCode = null } = req.body;
if (typeof token !== 'string' || !token) { if (typeof token !== 'string' || typeof recoveryCode !== 'string') {
throw new BadRequestError('Token is required to disable MFA feature'); throw new BadRequestError('Token or recovery code should be strings');
} }
await this.mfaService.disableMfa(userId, token); await this.mfaService.disableMfa(userId, token, recoveryCode);
} }
@Post('/verify', { rateLimit: true }) @Post('/verify', { rateLimit: true })

View file

@ -2,6 +2,6 @@ import { ForbiddenError } from './forbidden.error';
export class InvalidMfaCodeError extends ForbiddenError { export class InvalidMfaCodeError extends ForbiddenError {
constructor(hint?: string) { constructor(hint?: string) {
super('Invalid two-factor code.', hint); super('Invalid two-factor code or recovery code.', hint);
} }
} }

View file

@ -85,8 +85,9 @@ export class MfaService {
return await this.authUserRepository.save(user); return await this.authUserRepository.save(user);
} }
async disableMfa(userId: string, mfaToken: string) { async disableMfa(userId: string, mfaToken: string, recoveryCode: string) {
const isValidToken = await this.validateMfa(userId, mfaToken, undefined); const isValidToken = await this.validateMfa(userId, mfaToken, recoveryCode);
if (!isValidToken) { if (!isValidToken) {
throw new InvalidMfaCodeError(); throw new InvalidMfaCodeError();
} }

View file

@ -318,7 +318,7 @@ export type LoginRequest = AuthlessRequest<
export declare namespace MFA { export declare namespace MFA {
type Verify = AuthenticatedRequest<{}, {}, { token: string }, {}>; type Verify = AuthenticatedRequest<{}, {}, { token: string }, {}>;
type Activate = AuthenticatedRequest<{}, {}, { token: string }, {}>; type Activate = AuthenticatedRequest<{}, {}, { token: string }, {}>;
type Disable = AuthenticatedRequest<{}, {}, { token: string }, {}>; type Disable = AuthenticatedRequest<{}, {}, { token?: string; recoveryCode?: string }, {}>;
type Config = AuthenticatedRequest<{}, {}, { login: { enabled: boolean } }, {}>; type Config = AuthenticatedRequest<{}, {}, { login: { enabled: boolean } }, {}>;
type ValidateRecoveryCode = AuthenticatedRequest< type ValidateRecoveryCode = AuthenticatedRequest<
{}, {},

View file

@ -24,6 +24,7 @@ export async function verifyMfaToken(
export type DisableMfaParams = { export type DisableMfaParams = {
token: string; token: string;
recoveryCode: string;
}; };
export async function disableMfa(context: IRestApiContext, data: DisableMfaParams): Promise<void> { export async function disableMfa(context: IRestApiContext, data: DisableMfaParams): Promise<void> {

View file

@ -1,50 +1,89 @@
<script setup lang="ts"> <script setup lang="ts">
import { ref } from 'vue'; import { computed, ref } from 'vue';
import Modal from '../Modal.vue'; import Modal from '../Modal.vue';
import { PROMPT_MFA_CODE_MODAL_KEY } from '@/constants'; import { PROMPT_MFA_CODE_MODAL_KEY } from '@/constants';
import { useI18n } from '@/composables/useI18n'; import { useI18n } from '@/composables/useI18n';
import { promptMfaCodeBus } from '@/event-bus'; import { promptMfaCodeBus } from '@/event-bus';
import type { IFormInputs } from '@/Interface'; import { validate as uuidValidate } from 'uuid';
import { createFormEventBus } from 'n8n-design-system';
const MFA_CODE_INPUT_NAME = 'mfaCodeInput';
const MFA_CODE_VALIDATORS = {
mfaCode: {
validate: (value: string) => {
if (value === '') {
return { message: ' ' };
}
if (value.length < 6) {
return {
message: 'Code must be 6 digits',
};
}
if (!/^\d+$/.test(value)) {
return {
message: 'Only digits are allow',
};
}
return false;
},
},
};
const RECOVERY_CODE_VALIDATORS = {
recoveryCode: {
validate: (value: string) => {
if (value === '') {
return { message: ' ' };
}
if (!uuidValidate(value)) {
return {
message: 'Must be an UUID',
};
}
return false;
},
},
};
const i18n = useI18n(); const i18n = useI18n();
const formBus = createFormEventBus(); const mfaCode = ref('');
const readyToSubmit = ref(false); const recoveryCode = ref('');
const formFields: IFormInputs = [ const isMfaCodeValid = ref(false);
{ const isRecoveryCodeValid = ref(false);
name: 'mfaCode',
initialValue: '', const anyFieldValid = computed(() => isMfaCodeValid.value || isRecoveryCodeValid.value);
properties: {
label: i18n.baseText('mfa.code.input.label'), function onClickSave() {
placeholder: i18n.baseText('mfa.code.input.placeholder'), if (!anyFieldValid.value) {
focusInitially: true, return;
capitalize: true, }
required: true,
},
},
];
function onSubmit(values: { mfaCode: string }) {
promptMfaCodeBus.emit('close', { promptMfaCodeBus.emit('close', {
mfaCode: values.mfaCode, mfaCode: mfaCode.value,
recoveryCode: recoveryCode.value,
}); });
} }
function onClickSave() { function onValidate(name: string, value: boolean) {
formBus.emit('submit'); if (name === MFA_CODE_INPUT_NAME) {
} isMfaCodeValid.value = value;
} else {
function onFormReady(isReady: boolean) { isRecoveryCodeValid.value = value;
readyToSubmit.value = isReady; }
} }
</script> </script>
<template> <template>
<Modal <Modal
width="460px" width="500px"
height="300px" height="400px"
max-height="640px" max-height="640px"
:title="i18n.baseText('mfa.prompt.code.modal.title')" :title="i18n.baseText('mfa.prompt.code.modal.title')"
:event-bus="promptMfaCodeBus" :event-bus="promptMfaCodeBus"
@ -53,12 +92,27 @@ function onFormReady(isReady: boolean) {
> >
<template #content> <template #content>
<div :class="[$style.formContainer]"> <div :class="[$style.formContainer]">
<n8n-form-inputs <n8n-form-input
data-test-id="mfa-code-form" v-model="mfaCode"
:inputs="formFields" name="mfaCode"
:event-bus="formBus" data-test-id="mfa-code-input"
@submit="onSubmit" :label="i18n.baseText('mfa.code.input.label')"
@ready="onFormReady" :placeholder="i18n.baseText('mfa.code.input.placeholder')"
:validators="MFA_CODE_VALIDATORS"
:validation-rules="[{ name: 'MAX_LENGTH', config: { maximum: 6 } }, { name: 'mfaCode' }]"
@validate="(value: boolean) => onValidate(MFA_CODE_INPUT_NAME, value)"
/>
<span> {{ i18n.baseText('mfa.prompt.code.modal.divider') }} </span>
<n8n-form-input
v-model="recoveryCode"
name="recoveryCode"
data-test-id="recovery-code-input"
:label="i18n.baseText('mfa.recoveryCode.input.label')"
:placeholder="i18n.baseText('mfa.recovery.input.placeholder')"
:validators="RECOVERY_CODE_VALIDATORS"
:validation-rules="[{ name: 'recoveryCode' }]"
@validate="(value: boolean) => onValidate('recoveryCodeInput', value)"
/> />
</div> </div>
</template> </template>
@ -66,9 +120,9 @@ function onFormReady(isReady: boolean) {
<div> <div>
<n8n-button <n8n-button
float="right" float="right"
:disabled="!readyToSubmit"
:label="i18n.baseText('settings.personal.save')" :label="i18n.baseText('settings.personal.save')"
size="large" size="large"
:disabled="!anyFieldValid"
data-test-id="mfa-save-button" data-test-id="mfa-save-button"
@click="onClickSave" @click="onClickSave"
/> />
@ -79,6 +133,12 @@ function onFormReady(isReady: boolean) {
<style lang="scss" module> <style lang="scss" module>
.formContainer { .formContainer {
padding-bottom: var(--spacing-xl); display: flex;
flex-direction: column;
gap: var(--spacing-s);
span {
font-weight: 600;
}
} }
</style> </style>

View file

@ -4,6 +4,7 @@ export const mfaEventBus = createEventBus();
export interface MfaModalClosedEventPayload { export interface MfaModalClosedEventPayload {
mfaCode: string; mfaCode: string;
recoveryCode: string;
} }
export interface MfaModalEvents { export interface MfaModalEvents {

View file

@ -2582,6 +2582,7 @@
"mfa.code.button.continue": "Continue", "mfa.code.button.continue": "Continue",
"mfa.recovery.button.verify": "Verify", "mfa.recovery.button.verify": "Verify",
"mfa.button.back": "Back", "mfa.button.back": "Back",
"mfa.recoveryCode.input.label": "Recovery code",
"mfa.code.input.label": "Two-factor code", "mfa.code.input.label": "Two-factor code",
"mfa.code.input.placeholder": "e.g. 123456", "mfa.code.input.placeholder": "e.g. 123456",
"mfa.recovery.input.label": "Recovery Code", "mfa.recovery.input.label": "Recovery Code",
@ -2608,7 +2609,8 @@
"mfa.setup.step2.toast.setupFinished.message": "Two-factor authentication enabled", "mfa.setup.step2.toast.setupFinished.message": "Two-factor authentication enabled",
"mfa.setup.step2.toast.setupFinished.error.message": "Error enabling two-factor authentication", "mfa.setup.step2.toast.setupFinished.error.message": "Error enabling two-factor authentication",
"mfa.setup.step2.toast.tokenExpired.error.message": "MFA token expired. Close the modal and enable MFA again", "mfa.setup.step2.toast.tokenExpired.error.message": "MFA token expired. Close the modal and enable MFA again",
"mfa.prompt.code.modal.title": "Two-factor code required", "mfa.prompt.code.modal.title": "Two-factor code or recovery code required",
"mfa.prompt.code.modal.divider": "Or",
"settings.personal.mfa.section.title": "Two-factor authentication (2FA)", "settings.personal.mfa.section.title": "Two-factor authentication (2FA)",
"settings.personal.personalisation": "Personalisation", "settings.personal.personalisation": "Personalisation",
"settings.personal.theme": "Theme", "settings.personal.theme": "Theme",

View file

@ -331,9 +331,10 @@ export const useUsersStore = defineStore(STORES.USERS, () => {
} }
}; };
const disableMfa = async (mfaCode: string) => { const disableMfa = async (mfaCode: string, recoveryCode: string) => {
await mfaApi.disableMfa(rootStore.restApiContext, { await mfaApi.disableMfa(rootStore.restApiContext, {
token: mfaCode, token: mfaCode,
recoveryCode,
}); });
if (currentUser.value) { if (currentUser.value) {

View file

@ -226,7 +226,7 @@ async function disableMfa(payload: MfaModalEvents['closed']) {
} }
try { try {
await usersStore.disableMfa(payload.mfaCode); await usersStore.disableMfa(payload.mfaCode, payload.recoveryCode);
showToast({ showToast({
title: i18n.baseText('settings.personal.mfa.toast.disabledMfa.title'), title: i18n.baseText('settings.personal.mfa.toast.disabledMfa.title'),