refactor(core): Extract all Auth-related User columns into a separate entity (#9557)

Co-authored-by: Ricardo Espinoza <ricardo@n8n.io>

packages/cli/src/Interfaces.ts

@@ -624,7 +624,6 @@ export interface PublicUser {
 	passwordResetToken?: string;
 	createdAt: Date;
 	isPending: boolean;
-	hasRecoveryCodesLeft: boolean;
 	role?: GlobalRole;
 	globalScopes?: Scope[];
 	signInType: AuthProviderType;

packages/cli/src/Mfa/mfa.service.ts

@@ -1,43 +1,34 @@
 import { v4 as uuid } from 'uuid';
 import { Service } from 'typedi';
 import { Cipher } from 'n8n-core';
-import { UserRepository } from '@db/repositories/user.repository';
+import { AuthUserRepository } from '@db/repositories/authUser.repository';
 import { TOTPService } from './totp.service';
 
 @Service()
 export class MfaService {
 	constructor(
-		private userRepository: UserRepository,
+		private authUserRepository: AuthUserRepository,
 		public totp: TOTPService,
 		private cipher: Cipher,
 	) {}
 
-	public generateRecoveryCodes(n = 10) {
+	generateRecoveryCodes(n = 10) {
 		return Array.from(Array(n)).map(() => uuid());
 	}
 
-	public generateEncryptedRecoveryCodes() {
-		return this.generateRecoveryCodes().map((code) => this.cipher.encrypt(code));
-	}
-
-	public async saveSecretAndRecoveryCodes(userId: string, secret: string, recoveryCodes: string[]) {
+	async saveSecretAndRecoveryCodes(userId: string, secret: string, recoveryCodes: string[]) {
 		const { encryptedSecret, encryptedRecoveryCodes } = this.encryptSecretAndRecoveryCodes(
 			secret,
 			recoveryCodes,
 		);
 
-		const user = await this.userRepository.findOneBy({ id: userId });
-		if (user) {
-			Object.assign(user, {
-				mfaSecret: encryptedSecret,
-				mfaRecoveryCodes: encryptedRecoveryCodes,
-			});
-
-			await this.userRepository.save(user);
-		}
+		const user = await this.authUserRepository.findOneByOrFail({ id: userId });
+		user.mfaSecret = encryptedSecret;
+		user.mfaRecoveryCodes = encryptedRecoveryCodes;
+		await this.authUserRepository.save(user);
 	}
 
-	public encryptSecretAndRecoveryCodes(rawSecret: string, rawRecoveryCodes: string[]) {
+	encryptSecretAndRecoveryCodes(rawSecret: string, rawRecoveryCodes: string[]) {
 		const encryptedSecret = this.cipher.encrypt(rawSecret),
 			encryptedRecoveryCodes = rawRecoveryCodes.map((code) => this.cipher.encrypt(code));
 		return {
@@ -53,37 +44,46 @@ export class MfaService {
 		};
 	}
 
-	public async getSecretAndRecoveryCodes(userId: string) {
-		const { mfaSecret, mfaRecoveryCodes } = await this.userRepository.findOneOrFail({
-			where: { id: userId },
-			select: ['id', 'mfaSecret', 'mfaRecoveryCodes'],
+	async getSecretAndRecoveryCodes(userId: string) {
+		const { mfaSecret, mfaRecoveryCodes } = await this.authUserRepository.findOneByOrFail({
+			id: userId,
 		});
 		return this.decryptSecretAndRecoveryCodes(mfaSecret ?? '', mfaRecoveryCodes ?? []);
 	}
 
-	public async enableMfa(userId: string) {
-		const user = await this.userRepository.findOneBy({ id: userId });
-		if (user) {
-			user.mfaEnabled = true;
-
-			await this.userRepository.save(user);
+	async validateMfa(
+		userId: string,
+		mfaToken: string | undefined,
+		mfaRecoveryCode: string | undefined,
+	) {
+		const user = await this.authUserRepository.findOneByOrFail({ id: userId });
+		if (mfaToken) {
+			const decryptedSecret = this.cipher.decrypt(user.mfaSecret!);
+			return this.totp.verifySecret({ secret: decryptedSecret, token: mfaToken });
+		} else if (mfaRecoveryCode) {
+			const validCodes = user.mfaRecoveryCodes.map((code) => this.cipher.decrypt(code));
+			const index = validCodes.indexOf(mfaRecoveryCode);
+			if (index === -1) return false;
+			// remove used recovery code
+			validCodes.splice(index, 1);
+			user.mfaRecoveryCodes = validCodes.map((code) => this.cipher.encrypt(code));
+			await this.authUserRepository.save(user);
+			return true;
 		}
+		return false;
 	}
 
-	public encryptRecoveryCodes(mfaRecoveryCodes: string[]) {
-		return mfaRecoveryCodes.map((code) => this.cipher.encrypt(code));
+	async enableMfa(userId: string) {
+		const user = await this.authUserRepository.findOneByOrFail({ id: userId });
+		user.mfaEnabled = true;
+		return await this.authUserRepository.save(user);
 	}
 
-	public async disableMfa(userId: string) {
-		const user = await this.userRepository.findOneBy({ id: userId });
-
-		if (user) {
-			Object.assign(user, {
-				mfaEnabled: false,
-				mfaSecret: null,
-				mfaRecoveryCodes: [],
-			});
-			await this.userRepository.save(user);
-		}
+	async disableMfa(userId: string) {
+		const user = await this.authUserRepository.findOneByOrFail({ id: userId });
+		user.mfaEnabled = false;
+		user.mfaSecret = null;
+		user.mfaRecoveryCodes = [];
+		return await this.authUserRepository.save(user);
 	}
 }

packages/cli/src/commands/mfa/disable.ts

@@ -1,6 +1,6 @@
 import Container from 'typedi';
 import { Flags } from '@oclif/core';
-import { UserRepository } from '@db/repositories/user.repository';
+import { AuthUserRepository } from '@db/repositories/authUser.repository';
 import { BaseCommand } from '../BaseCommand';
 
 export class DisableMFACommand extends BaseCommand {
@@ -27,7 +27,8 @@ export class DisableMFACommand extends BaseCommand {
 			return;
 		}
 
-		const user = await Container.get(UserRepository).findOneBy({ email: flags.email });
+		const repository = Container.get(AuthUserRepository);
+		const user = await repository.findOneBy({ email: flags.email });
 
 		if (!user) {
 			this.reportUserDoesNotExistError(flags.email);
@@ -46,7 +47,7 @@ export class DisableMFACommand extends BaseCommand {
 
 		Object.assign(user, { mfaSecret: null, mfaRecoveryCodes: [], mfaEnabled: false });
 
-		await Container.get(UserRepository).save(user);
+		await repository.save(user);
 
 		this.reportSuccess(flags.email);
 	}

packages/cli/src/controllers/auth.controller.ts

@@ -79,16 +79,11 @@ export class AuthController {
 					throw new AuthError('MFA Error', 998);
 				}
 
-				const { decryptedRecoveryCodes, decryptedSecret } =
-					await this.mfaService.getSecretAndRecoveryCodes(user.id);
-
-				user.mfaSecret = decryptedSecret;
-				user.mfaRecoveryCodes = decryptedRecoveryCodes;
-
-				const isMFATokenValid =
-					(await this.validateMfaToken(user, mfaToken)) ||
-					(await this.validateMfaRecoveryCode(user, mfaRecoveryCode));
-
+				const isMFATokenValid = await this.mfaService.validateMfa(
+					user.id,
+					mfaToken,
+					mfaRecoveryCode,
+				);
 				if (!isMFATokenValid) {
 					throw new AuthError('Invalid mfa token or recovery code');
 				}
@@ -193,27 +188,4 @@ export class AuthController {
 		this.authService.clearCookie(res);
 		return { loggedOut: true };
 	}
-
-	private async validateMfaToken(user: User, token?: string) {
-		if (!!!token) return false;
-		return this.mfaService.totp.verifySecret({
-			secret: user.mfaSecret ?? '',
-			token,
-		});
-	}
-
-	private async validateMfaRecoveryCode(user: User, mfaRecoveryCode?: string) {
-		if (!!!mfaRecoveryCode) return false;
-		const index = user.mfaRecoveryCodes.indexOf(mfaRecoveryCode);
-		if (index === -1) return false;
-
-		// remove used recovery code
-		user.mfaRecoveryCodes.splice(index, 1);
-
-		await this.userService.update(user.id, {
-			mfaRecoveryCodes: this.mfaService.encryptRecoveryCodes(user.mfaRecoveryCodes),
-		});
-
-		return true;
-	}
 }

packages/cli/src/controllers/e2e.controller.ts

@@ -16,6 +16,7 @@ import { CacheService } from '@/services/cache/cache.service';
 import { PasswordUtility } from '@/services/password.utility';
 import Container from 'typedi';
 import { Logger } from '@/Logger';
+import { AuthUserRepository } from '@/databases/repositories/authUser.repository';
 
 if (!inE2ETests) {
 	Container.get(Logger).error('E2E endpoints only allowed during E2E tests');
@@ -106,6 +107,7 @@ export class E2EController {
 		private readonly passwordUtility: PasswordUtility,
 		private readonly eventBus: MessageEventBus,
 		private readonly userRepository: UserRepository,
+		private readonly authUserRepository: AuthUserRepository,
 	) {
 		license.isFeatureEnabled = (feature: BooleanLicenseFeature) =>
 			this.enabledFeatures[feature] ?? false;
@@ -185,13 +187,6 @@ export class E2EController {
 		members: UserSetupPayload[],
 		admin: UserSetupPayload,
 	) {
-		if (owner?.mfaSecret && owner.mfaRecoveryCodes?.length) {
-			const { encryptedRecoveryCodes, encryptedSecret } =
-				this.mfaService.encryptSecretAndRecoveryCodes(owner.mfaSecret, owner.mfaRecoveryCodes);
-			owner.mfaSecret = encryptedSecret;
-			owner.mfaRecoveryCodes = encryptedRecoveryCodes;
-		}
-
 		const userCreatePromises = [
 			this.userRepository.createUserWithProject({
 				id: uuid(),
@@ -221,7 +216,17 @@ export class E2EController {
 			);
 		}
 
-		await Promise.all(userCreatePromises);
+		const [newOwner] = await Promise.all(userCreatePromises);
+
+		if (owner?.mfaSecret && owner.mfaRecoveryCodes?.length) {
+			const { encryptedRecoveryCodes, encryptedSecret } =
+				this.mfaService.encryptSecretAndRecoveryCodes(owner.mfaSecret, owner.mfaRecoveryCodes);
+
+			await this.authUserRepository.update(newOwner.user.id, {
+				mfaSecret: encryptedSecret,
+				mfaRecoveryCodes: encryptedRecoveryCodes,
+			});
+		}
 
 		await this.settingsRepo.update(
 			{ key: 'userManagement.isInstanceOwnerSetUp' },

packages/cli/src/controllers/users.controller.ts

@@ -77,7 +77,6 @@ export class UsersController {
 				delete user.isOwner;
 				delete user.isPending;
 				delete user.signInType;
-				delete user.hasRecoveryCodesLeft;
 			}
 		}
 

packages/cli/src/databases/entities/AuthUser.ts

@@ -0,0 +1,19 @@
+import { Column, Entity, PrimaryColumn } from '@n8n/typeorm';
+
+@Entity({ name: 'user' })
+export class AuthUser {
+	@PrimaryColumn({ type: 'uuid', update: false })
+	id: string;
+
+	@Column({ type: String, update: false })
+	email: string;
+
+	@Column({ type: Boolean, default: false })
+	mfaEnabled: boolean;
+
+	@Column({ type: String, nullable: true })
+	mfaSecret?: string | null;
+
+	@Column({ type: 'simple-array', default: '' })
+	mfaRecoveryCodes: string[];
+}

packages/cli/src/databases/entities/User.ts

@@ -109,12 +109,6 @@ export class User extends WithTimestamps implements IUser {
 	@Column({ type: Boolean, default: false })
 	mfaEnabled: boolean;
 
-	@Column({ type: String, nullable: true, select: false })
-	mfaSecret?: string | null;
-
-	@Column({ type: 'simple-array', default: '', select: false })
-	mfaRecoveryCodes: string[];
-
 	/**
 	 * Whether the user is pending setup completion.
 	 */
@@ -152,7 +146,7 @@ export class User extends WithTimestamps implements IUser {
 	}
 
 	toJSON() {
-		const { password, apiKey, mfaSecret, mfaRecoveryCodes, ...rest } = this;
+		const { password, apiKey, ...rest } = this;
 		return rest;
 	}
 

packages/cli/src/databases/entities/index.ts

@@ -1,6 +1,7 @@
 /* eslint-disable @typescript-eslint/naming-convention */
 import { AuthIdentity } from './AuthIdentity';
 import { AuthProviderSyncHistory } from './AuthProviderSyncHistory';
+import { AuthUser } from './AuthUser';
 import { CredentialsEntity } from './CredentialsEntity';
 import { EventDestinations } from './EventDestinations';
 import { ExecutionEntity } from './ExecutionEntity';
@@ -25,6 +26,7 @@ import { ProjectRelation } from './ProjectRelation';
 export const entities = {
 	AuthIdentity,
 	AuthProviderSyncHistory,
+	AuthUser,
 	CredentialsEntity,
 	EventDestinations,
 	ExecutionEntity,

packages/cli/src/databases/repositories/authUser.repository.ts

@@ -0,0 +1,10 @@
+import { Service } from 'typedi';
+import { DataSource, Repository } from '@n8n/typeorm';
+import { AuthUser } from '../entities/AuthUser';
+
+@Service()
+export class AuthUserRepository extends Repository<AuthUser> {
+	constructor(dataSource: DataSource) {
+		super(AuthUser, dataSource.manager);
+	}
+}

packages/cli/src/databases/repositories/user.repository.ts

@@ -6,6 +6,7 @@ import type { ListQuery } from '@/requests';
 import { type GlobalRole, User } from '../entities/User';
 import { Project } from '../entities/Project';
 import { ProjectRelation } from '../entities/ProjectRelation';
+
 @Service()
 export class UserRepository extends Repository<User> {
 	constructor(dataSource: DataSource) {

packages/cli/src/services/user.service.ts

@@ -57,15 +57,13 @@ export class UserService {
 			withScopes?: boolean;
 		},
 	) {
-		const { password, updatedAt, apiKey, authIdentities, mfaRecoveryCodes, mfaSecret, ...rest } =
-			user;
+		const { password, updatedAt, apiKey, authIdentities, ...rest } = user;
 
 		const ldapIdentity = authIdentities?.find((i) => i.providerType === 'ldap');
 
 		let publicUser: PublicUser = {
 			...rest,
 			signInType: ldapIdentity ? 'ldap' : 'email',
-			hasRecoveryCodesLeft: !!user.mfaRecoveryCodes?.length,
 		};
 
 		if (options?.withInviteUrl && !options?.inviterId) {