Apply feedback

This commit is contained in:
Ricardo Espinoza 2024-11-28 08:23:15 -05:00
parent a0afd32413
commit afe75f3b8f
No known key found for this signature in database
7 changed files with 27 additions and 13 deletions

View file

@ -22,7 +22,7 @@ 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'), mfaCodeOrMfaRecoveryCodeInput: () => cy.getByTestId('mfa-code-or-recovery-code-input'),
mfaSaveButton: () => cy.getByTestId('mfa-save-button'), 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'),
@ -85,10 +85,10 @@ export class PersonalSettingsPage extends BasePage {
mfaSetupModal.getters.saveButton().click(); mfaSetupModal.getters.saveButton().click();
}); });
}, },
disableMfa: (code: string) => { disableMfa: (mfaCodeOrRecoveryCode: string) => {
cy.visit(this.url); cy.visit(this.url);
this.getters.disableMfaButton().click(); this.getters.disableMfaButton().click();
this.getters.mfaCodeInput().type(code); this.getters.mfaCodeOrMfaRecoveryCodeInput().type(mfaCodeOrRecoveryCode);
this.getters.mfaSaveButton().click(); this.getters.mfaSaveButton().click();
}, },
}; };

View file

@ -79,8 +79,12 @@ export class AuthController {
throw new AuthError('MFA Error', 998); throw new AuthError('MFA Error', 998);
} }
const isMFACodeValid = await this.mfaService.validateMfa(user.id, mfaCode, mfaRecoveryCode); const isMFACodeOrMfaRecoveryCodeValid = await this.mfaService.validateMfa(
if (!isMFACodeValid) { user.id,
mfaCode,
mfaRecoveryCode,
);
if (!isMFACodeOrMfaRecoveryCodeValid) {
throw new AuthError('Invalid mfa token or recovery code'); throw new AuthError('Invalid mfa token or recovery code');
} }
} }

View file

@ -89,6 +89,10 @@ export class MFAController {
const { mfaCode, mfaRecoveryCode } = req.body; const { mfaCode, mfaRecoveryCode } = req.body;
if (!mfaCode && !mfaRecoveryCode) {
throw new BadRequestError('MFA code or recovery code is required to disable MFA feature');
}
if (mfaCode && typeof mfaCode === 'string') { if (mfaCode && typeof mfaCode === 'string') {
await this.mfaService.disableMfaWithMfaCode(userId, mfaCode); await this.mfaService.disableMfaWithMfaCode(userId, mfaCode);
} else if (mfaRecoveryCode && typeof mfaRecoveryCode === 'string') { } else if (mfaRecoveryCode && typeof mfaRecoveryCode === 'string') {

View file

@ -87,7 +87,7 @@ export class MfaService {
} }
async disableMfaWithMfaCode(userId: string, mfaCode: string) { async disableMfaWithMfaCode(userId: string, mfaCode: string) {
const isValidToken = await this.validateMfa(userId, mfaCode, ''); const isValidToken = await this.validateMfa(userId, mfaCode, undefined);
if (!isValidToken) { if (!isValidToken) {
throw new InvalidMfaCodeError(); throw new InvalidMfaCodeError();
@ -97,7 +97,7 @@ export class MfaService {
} }
async disableMfaWithRecoveryCode(userId: string, recoveryCode: string) { async disableMfaWithRecoveryCode(userId: string, recoveryCode: string) {
const isValidToken = await this.validateMfa(userId, '', recoveryCode); const isValidToken = await this.validateMfa(userId, undefined, recoveryCode);
if (!isValidToken) { if (!isValidToken) {
throw new InvalidMfaRecoveryCodeError(); throw new InvalidMfaRecoveryCodeError();

View file

@ -207,6 +207,12 @@ describe('Disable MFA setup', () => {
}) })
.expect(403); .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', () => { describe('Change password with MFA enabled', () => {

View file

@ -6,7 +6,7 @@ import { useI18n } from '@/composables/useI18n';
import { promptMfaCodeBus } from '@/event-bus'; import { promptMfaCodeBus } from '@/event-bus';
import type { IFormInputs } from '@/Interface'; import type { IFormInputs } from '@/Interface';
import { createFormEventBus } from 'n8n-design-system'; import { createFormEventBus } from 'n8n-design-system';
import { validate as uuidValidate } from 'uuid'; import { validate as validateUuid } from 'uuid';
const i18n = useI18n(); const i18n = useI18n();
@ -15,7 +15,7 @@ const readyToSubmit = ref(false);
const formFields: IFormInputs = [ const formFields: IFormInputs = [
{ {
name: 'code', name: 'mfaCodeOrMfaRecoveryCode',
initialValue: '', initialValue: '',
properties: { properties: {
label: i18n.baseText('mfa.code.recovery.input.label'), label: i18n.baseText('mfa.code.recovery.input.label'),
@ -28,7 +28,7 @@ const formFields: IFormInputs = [
]; ];
function onSubmit(values: { code: string }) { function onSubmit(values: { code: string }) {
if (uuidValidate(values.code)) { if (validateUuid(values.code)) {
promptMfaCodeBus.emit('close', { promptMfaCodeBus.emit('close', {
mfaRecoveryCode: values.code, mfaRecoveryCode: values.code,
}); });
@ -63,7 +63,7 @@ function onFormReady(isReady: boolean) {
<template #content> <template #content>
<div :class="[$style.formContainer]"> <div :class="[$style.formContainer]">
<n8n-form-inputs <n8n-form-inputs
data-test-id="mfa-code-input" data-test-id="mfa-code-or-recovery-code-input"
:inputs="formFields" :inputs="formFields"
:event-bus="formBus" :event-bus="formBus"
@submit="onSubmit" @submit="onSubmit"

View file

@ -106,7 +106,7 @@ const onSubmit = async (form: { mfaCode: string; mfaRecoveryCode: string }) => {
}; };
const onInput = ({ target: { value, name } }: { target: { value: string; name: string } }) => { const onInput = ({ target: { value, name } }: { target: { value: string; name: string } }) => {
const isSubmittingMfaCode = name === 'mfaToken'; const isSubmittingMfaCode = name === 'mfaCode';
const inputValidLength = isSubmittingMfaCode const inputValidLength = isSubmittingMfaCode
? MFA_AUTHENTICATION_TOKEN_INPUT_MAX_LENGTH ? MFA_AUTHENTICATION_TOKEN_INPUT_MAX_LENGTH
: MFA_AUTHENTICATION_RECOVERY_CODE_INPUT_MAX_LENGTH; : MFA_AUTHENTICATION_RECOVERY_CODE_INPUT_MAX_LENGTH;
@ -139,7 +139,7 @@ const mfaRecoveryCodeFieldWithDefaults = () => {
const mfaCodeFieldWithDefaults = () => { const mfaCodeFieldWithDefaults = () => {
return formField( return formField(
'mfaToken', 'mfaCode',
i18.baseText('mfa.code.input.label'), i18.baseText('mfa.code.input.label'),
i18.baseText('mfa.code.input.placeholder'), i18.baseText('mfa.code.input.placeholder'),
MFA_AUTHENTICATION_TOKEN_INPUT_MAX_LENGTH, MFA_AUTHENTICATION_TOKEN_INPUT_MAX_LENGTH,