diff --git a/cypress/e2e/27-two-factor-authentication.cy.ts b/cypress/e2e/27-two-factor-authentication.cy.ts index 759964f155..05949a188c 100644 --- a/cypress/e2e/27-two-factor-authentication.cy.ts +++ b/cypress/e2e/27-two-factor-authentication.cy.ts @@ -68,16 +68,28 @@ describe('Two-factor authentication', { disableAutoLogin: true }, () => { mainSidebar.actions.signout(); }); - it('Should be able to disable MFA in account with MFA code ', () => { + it('Should be able to disable MFA in account with MFA code', () => { const { email, password } = user; signinPage.actions.loginWithEmailAndPassword(email, password); personalSettingsPage.actions.enableMfa(); mainSidebar.actions.signout(); - const loginToken = generateOTPToken(user.mfaSecret); - mfaLoginPage.actions.loginWithMfaCode(email, password, loginToken); + const mfaCode = generateOTPToken(user.mfaSecret); + mfaLoginPage.actions.loginWithMfaCode(email, password, mfaCode); const disableToken = generateOTPToken(user.mfaSecret); personalSettingsPage.actions.disableMfa(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 mfaCode = generateOTPToken(user.mfaSecret); + mfaLoginPage.actions.loginWithMfaCode(email, password, mfaCode); + personalSettingsPage.actions.disableMfa(user.mfaRecoveryCodes[0]); + personalSettingsPage.getters.enableMfaButton().should('exist'); + mainSidebar.actions.signout(); + }); }); diff --git a/packages/cli/src/controllers/mfa.controller.ts b/packages/cli/src/controllers/mfa.controller.ts index 54e67692ee..26aeca5432 100644 --- a/packages/cli/src/controllers/mfa.controller.ts +++ b/packages/cli/src/controllers/mfa.controller.ts @@ -86,13 +86,24 @@ export class MFAController { @Post('/disable', { rateLimit: true }) async disableMFA(req: MFA.Disable) { const { id: userId } = req.user; - const { mfaCode = null } = req.body; - if (typeof mfaCode !== 'string' || !mfaCode) { - throw new BadRequestError('Token is required to disable MFA feature'); + const { mfaCode, mfaRecoveryCode } = req.body; + + const mfaCodeDefined = mfaCode && typeof mfaCode === 'string'; + + const mfaRecoveryCodeDefined = mfaRecoveryCode && typeof mfaRecoveryCode === 'string'; + + if (!mfaCodeDefined === !mfaRecoveryCodeDefined) { + throw new BadRequestError( + 'Either MFA code or recovery code is required to disable MFA feature', + ); } - await this.mfaService.disableMfa(userId, mfaCode); + if (mfaCodeDefined) { + await this.mfaService.disableMfaWithMfaCode(userId, mfaCode); + } else if (mfaRecoveryCodeDefined) { + await this.mfaService.disableMfaWithRecoveryCode(userId, mfaRecoveryCode); + } } @Post('/verify', { rateLimit: true }) diff --git a/packages/cli/src/errors/response-errors/invalid-mfa-recovery-code-error.ts b/packages/cli/src/errors/response-errors/invalid-mfa-recovery-code-error.ts new file mode 100644 index 0000000000..baa0ead870 --- /dev/null +++ b/packages/cli/src/errors/response-errors/invalid-mfa-recovery-code-error.ts @@ -0,0 +1,7 @@ +import { ForbiddenError } from './forbidden.error'; + +export class InvalidMfaRecoveryCodeError extends ForbiddenError { + constructor(hint?: string) { + super('Invalid MFA recovery code', hint); + } +} diff --git a/packages/cli/src/mfa/mfa.service.ts b/packages/cli/src/mfa/mfa.service.ts index afce8927f9..84433f5f18 100644 --- a/packages/cli/src/mfa/mfa.service.ts +++ b/packages/cli/src/mfa/mfa.service.ts @@ -4,6 +4,7 @@ import { v4 as uuid } from 'uuid'; import { AuthUserRepository } from '@/databases/repositories/auth-user.repository'; import { InvalidMfaCodeError } from '@/errors/response-errors/invalid-mfa-code.error'; +import { InvalidMfaRecoveryCodeError } from '@/errors/response-errors/invalid-mfa-recovery-code-error'; import { TOTPService } from './totp.service'; @@ -85,12 +86,27 @@ export class MfaService { return await this.authUserRepository.save(user); } - async disableMfa(userId: string, mfaCode: string) { + async disableMfaWithMfaCode(userId: string, mfaCode: string) { const isValidToken = await this.validateMfa(userId, mfaCode, undefined); + if (!isValidToken) { throw new InvalidMfaCodeError(); } + await this.disableMfaForUser(userId); + } + + async disableMfaWithRecoveryCode(userId: string, recoveryCode: string) { + const isValidToken = await this.validateMfa(userId, undefined, recoveryCode); + + if (!isValidToken) { + throw new InvalidMfaRecoveryCodeError(); + } + + await this.disableMfaForUser(userId); + } + + private async disableMfaForUser(userId: string) { await this.authUserRepository.update(userId, { mfaEnabled: false, mfaSecret: null, diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index 72d6fcf135..7afb1e1bd3 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -318,7 +318,7 @@ export type LoginRequest = AuthlessRequest< export declare namespace MFA { type Verify = AuthenticatedRequest<{}, {}, { mfaCode: string }, {}>; type Activate = AuthenticatedRequest<{}, {}, { mfaCode: string }, {}>; - type Disable = AuthenticatedRequest<{}, {}, { mfaCode: string }, {}>; + type Disable = AuthenticatedRequest<{}, {}, { mfaCode?: string; mfaRecoveryCode?: string }, {}>; type Config = AuthenticatedRequest<{}, {}, { login: { enabled: boolean } }, {}>; type ValidateRecoveryCode = AuthenticatedRequest< {}, diff --git a/packages/cli/test/integration/mfa/mfa.api.test.ts b/packages/cli/test/integration/mfa/mfa.api.test.ts index 95c3334277..498be7abd5 100644 --- a/packages/cli/test/integration/mfa/mfa.api.test.ts +++ b/packages/cli/test/integration/mfa/mfa.api.test.ts @@ -184,7 +184,19 @@ describe('Disable MFA setup', () => { expect(dbUser.mfaRecoveryCodes.length).toBe(0); }); - test('POST /disable should fail if invalid mfaCode is given', async () => { + test('POST /disable should fail if invalid MFA recovery code is given', async () => { + const { user } = await createUserWithMfaEnabled(); + + await testServer + .authAgentFor(user) + .post('/mfa/disable') + .send({ + mfaRecoveryCode: 'invalid token', + }) + .expect(403); + }); + + test('POST /disable should fail if invalid MFA code is given', async () => { const { user } = await createUserWithMfaEnabled(); await testServer @@ -195,6 +207,12 @@ describe('Disable MFA setup', () => { }) .expect(403); }); + + test('POST /disable should fail if neither MFA code nor recovery code is sent', async () => { + const { user } = await createUserWithMfaEnabled(); + + await testServer.authAgentFor(user).post('/mfa/disable').send({ anotherParam: '' }).expect(400); + }); }); describe('Change password with MFA enabled', () => { diff --git a/packages/editor-ui/src/api/mfa.ts b/packages/editor-ui/src/api/mfa.ts index a28e2146b1..df5849fe2c 100644 --- a/packages/editor-ui/src/api/mfa.ts +++ b/packages/editor-ui/src/api/mfa.ts @@ -26,7 +26,8 @@ export async function verifyMfaCode( } export type DisableMfaParams = { - mfaCode: string; + mfaCode?: string; + mfaRecoveryCode?: string; }; export async function disableMfa(context: IRestApiContext, data: DisableMfaParams): Promise { diff --git a/packages/editor-ui/src/components/PromptMfaCodeModal/PromptMfaCodeModal.vue b/packages/editor-ui/src/components/PromptMfaCodeModal/PromptMfaCodeModal.vue index 0099fd37f2..fb15cee54a 100644 --- a/packages/editor-ui/src/components/PromptMfaCodeModal/PromptMfaCodeModal.vue +++ b/packages/editor-ui/src/components/PromptMfaCodeModal/PromptMfaCodeModal.vue @@ -6,6 +6,7 @@ import { useI18n } from '@/composables/useI18n'; import { promptMfaCodeBus } from '@/event-bus'; import type { IFormInputs } from '@/Interface'; import { createFormEventBus } from 'n8n-design-system'; +import { validate as validateUuid } from 'uuid'; const i18n = useI18n(); @@ -14,11 +15,11 @@ const readyToSubmit = ref(false); const formFields: IFormInputs = [ { - name: 'mfaCode', + name: 'mfaCodeOrMfaRecoveryCode', initialValue: '', properties: { - label: i18n.baseText('mfa.code.input.label'), - placeholder: i18n.baseText('mfa.code.input.placeholder'), + label: i18n.baseText('mfa.code.recovery.input.label'), + placeholder: i18n.baseText('mfa.code.recovery.input.placeholder'), focusInitially: true, capitalize: true, required: true, @@ -26,9 +27,15 @@ const formFields: IFormInputs = [ }, ]; -function onSubmit(values: { mfaCode: string }) { +function onSubmit(values: { mfaCodeOrMfaRecoveryCode: string }) { + if (validateUuid(values.mfaCodeOrMfaRecoveryCode)) { + promptMfaCodeBus.emit('close', { + mfaRecoveryCode: values.mfaCodeOrMfaRecoveryCode, + }); + return; + } promptMfaCodeBus.emit('close', { - mfaCode: values.mfaCode, + mfaCode: values.mfaCodeOrMfaRecoveryCode, }); } @@ -43,7 +50,7 @@ function onFormReady(isReady: boolean) {