github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-10-17 21:49:36 +02:00
Code Issues Actions 13 Packages Projects Releases Wiki Activity Graph

perf(core): Cut redundant work in WebAuthn and MFA hot paths

Browse Source 
- `verifyAuthenticationResponse` already loads the credential row to
  derive UV requirements; pass it through to `verifyAssertion` instead
  of repeating the lookup.
- `getAvailableMfaMethods` ran two independent reads sequentially —
  fan them out with `Promise.all`. Hit on every MFA-required login.
- `deleteWebAuthnCredential` only needed to know whether TOTP is set,
  but called `getSecretAndRecoveryCodes` which AES-decrypts the secret
  plus every recovery code. Add a lightweight `hasTotpSecret(userId)`
  and use that instead.
- Replace bare `throw new Error` with `OperationalError`, drop the
  WebAuthn section banner and a couple of comments that just
  paraphrased the surrounding code.
This commit is contained in:
Ricardo Espinoza 2026-05-14 16:45:05 -04:00
parent c21c709b8a
commit db983f0e53
No known key found for this signature in database
3 changed files with 23 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
packages/cli/src/controllers/__tests__/mfa.controller.test.ts
View File

@ -80,10 +80,7 @@ describe('MFAController', () => {
mfaService.validateProof.mockResolvedValue(true);
webauthnService.getUserCredentials.mockResolvedValue([{ id: credId } as never]);
mfaService.isMFAEnforced.mockResolvedValue(true);
mfaService.getSecretAndRecoveryCodes.mockResolvedValue({
decryptedSecret: '',
decryptedRecoveryCodes: [],
});
mfaService.hasTotpSecret.mockResolvedValue(false);
await expect(
controller.deleteWebAuthnCredential(mkReq({ mfaCode: '123456' }), mock<Response>()),
@ -95,10 +92,7 @@ describe('MFAController', () => {
mfaService.validateProof.mockResolvedValue(true);
webauthnService.getUserCredentials.mockResolvedValue([{ id: credId } as never]);
mfaService.isMFAEnforced.mockResolvedValue(true);
mfaService.getSecretAndRecoveryCodes.mockResolvedValue({
decryptedSecret: 'totp-secret',
decryptedRecoveryCodes: ['rc'],
});
mfaService.hasTotpSecret.mockResolvedValue(true);
webauthnService.deleteCredential.mockResolvedValue(true);
const res = mock<Response>();
@ -114,10 +108,7 @@ describe('MFAController', () => {
mfaService.validateProof.mockResolvedValue(true);
webauthnService.getUserCredentials.mockResolvedValue([{ id: credId } as never]);
mfaService.isMFAEnforced.mockResolvedValue(false);
mfaService.getSecretAndRecoveryCodes.mockResolvedValue({
decryptedSecret: '',
decryptedRecoveryCodes: [],
});
mfaService.hasTotpSecret.mockResolvedValue(false);
webauthnService.deleteCredential.mockResolvedValue(true);
const res = mock<Response>();
@ -142,10 +133,7 @@ describe('MFAController', () => {
{ id: 'cred-2' },
] as never);
mfaService.isMFAEnforced.mockResolvedValue(false);
mfaService.getSecretAndRecoveryCodes.mockResolvedValue({
decryptedSecret: '',
decryptedRecoveryCodes: [],
});
mfaService.hasTotpSecret.mockResolvedValue(false);
webauthnService.deleteCredential.mockResolvedValue(true);
const res = mock<Response>();
@ -159,10 +147,7 @@ describe('MFAController', () => {
mfaService.validateProof.mockResolvedValue(true);
webauthnService.getUserCredentials.mockResolvedValue([{ id: credId } as never]);
mfaService.isMFAEnforced.mockResolvedValue(false);
mfaService.getSecretAndRecoveryCodes.mockResolvedValue({
decryptedSecret: 'totp-secret',
decryptedRecoveryCodes: ['rc'],
});
mfaService.hasTotpSecret.mockResolvedValue(true);
webauthnService.deleteCredential.mockResolvedValue(false);
await expect(

16
packages/cli/src/controllers/mfa.controller.ts
View File

@ -206,10 +206,6 @@ export class MFAController {
if (!verified) throw new BadRequestError('MFA secret could not be verified');
}
// ---------------------------------------------------------------
// WebAuthn endpoints
// ---------------------------------------------------------------
@Get('/webauthn/registration-options', {
allowSkipMFA: true,
})
@ -361,16 +357,13 @@ export class MFAController {
throw new BadRequestError('Invalid two-factor proof');
}
const [credentialsBefore, isMFAEnforced, { decryptedSecret }] = await Promise.all([
const [credentialsBefore, isMFAEnforced, hasTotpSecret] = await Promise.all([
this.mfaService.webauthn.getUserCredentials(userId),
this.mfaService.isMFAEnforced(),
this.mfaService.getSecretAndRecoveryCodes(userId),
this.mfaService.hasTotpSecret(userId),
]);
const remainingWebauthn = credentialsBefore.filter((c) => c.id !== id).length;
const hasTotpSecret = !!decryptedSecret;
// Refuse if removing this credential would leave the user with no
// authentication factor on an instance that enforces MFA.
if (isMFAEnforced && remainingWebauthn === 0 && !hasTotpSecret) {
throw new BadRequestError(
'You can’t remove your last MFA credential while two-factor authentication is required on this instance',
@ -382,9 +375,8 @@ export class MFAController {
throw new BadRequestError('Credential not found');
}
// If this was the last MFA factor, mirror disable-MFA semantics so the
// user's session reflects the new state. The pre-delete read above
// already told us what's left — no need to re-query.
// Mirror disable-MFA semantics when the last factor is gone so the
// session reflects the new state.
if (remainingWebauthn === 0 && !hasTotpSecret) {
await this.userRepository.update(userId, {
mfaEnabled: false,

22
packages/cli/src/mfa/mfa.service.ts
View File

@ -121,6 +121,14 @@ export class MfaService {
return await this.decryptSecretAndRecoveryCodes(mfaSecret ?? '', mfaRecoveryCodes ?? []);
}
async hasTotpSecret(userId: string): Promise<boolean> {
const user = await this.userRepository.findOne({
where: { id: userId },
select: { mfaSecret: true },
});
return !!user?.mfaSecret;
}
async validateMfa(
userId: string,
mfaCode: string | undefined,
@ -164,18 +172,16 @@ export class MfaService {
async getAvailableMfaMethods(
userId: string,
): Promise<Array<'totp' | 'passkey' | 'security_key'>> {
const [user, credentials] = await Promise.all([
this.userRepository.findOneByOrFail({ id: userId }),
this.webauthn.getUserCredentials(userId),
]);
const methods = new Set<'totp' | 'passkey' | 'security_key'>();
const user = await this.userRepository.findOneByOrFail({ id: userId });
if (user.mfaSecret) {
methods.add('totp');
}
const credentials = await this.webauthn.getUserCredentials(userId);
if (user.mfaSecret) methods.add('totp');
for (const cred of credentials) {
methods.add(isPlatformCredential(cred) ? 'passkey' : 'security_key');
}
return Array.from(methods);
}