feat(core): Allow enforcement of MFA usage on instance (#16556)

Co-authored-by: Marc Littlemore <marc@n8n.io>
Co-authored-by: Csaba Tuncsik <csaba.tuncsik@gmail.com>
This commit is contained in:
Andreas Fitzek
2025-07-02 11:03:10 +02:00
committed by GitHub
parent 060acd2db8
commit 657e5a3b3a
56 changed files with 619 additions and 88 deletions

View File

@@ -27,10 +27,12 @@ describe('ControllerRegistry', () => {
const metadata = Container.get(ControllerRegistryMetadata);
const lastActiveAtService = mock<LastActiveAtService>();
let agent: SuperAgentTest;
const authMiddleware = jest.fn().mockImplementation(async (_req, _res, next) => next());
beforeEach(() => {
jest.resetAllMocks();
const app = express();
authService.createAuthMiddleware.mockImplementation(() => authMiddleware);
new ControllerRegistry(
license,
authService,
@@ -57,7 +59,7 @@ describe('ControllerRegistry', () => {
}
beforeEach(() => {
authService.authMiddleware.mockImplementation(async (_req, _res, next) => next());
authMiddleware.mockImplementation(async (_req, _res, next) => next());
lastActiveAtService.middleware.mockImplementation(async (_req, _res, next) => next());
});
@@ -92,15 +94,15 @@ describe('ControllerRegistry', () => {
it('should not require auth if configured to skip', async () => {
await agent.get('/rest/test/no-auth').expect(200);
expect(authService.authMiddleware).not.toHaveBeenCalled();
expect(authMiddleware).not.toHaveBeenCalled();
});
it('should require auth by default', async () => {
authService.authMiddleware.mockImplementation(async (_req, res) => {
authMiddleware.mockImplementation(async (_req, res) => {
res.status(401).send();
});
await agent.get('/rest/test/auth').expect(401);
expect(authService.authMiddleware).toHaveBeenCalled();
expect(authMiddleware).toHaveBeenCalled();
});
});
@@ -116,7 +118,7 @@ describe('ControllerRegistry', () => {
}
beforeEach(() => {
authService.authMiddleware.mockImplementation(async (_req, _res, next) => next());
authMiddleware.mockImplementation(async (_req, _res, next) => next());
lastActiveAtService.middleware.mockImplementation(async (_req, _res, next) => next());
});
@@ -145,7 +147,7 @@ describe('ControllerRegistry', () => {
}
beforeEach(() => {
authService.authMiddleware.mockImplementation(async (_req, _res, next) => next());
authMiddleware.mockImplementation(async (_req, _res, next) => next());
lastActiveAtService.middleware.mockImplementation(async (_req, _res, next) => next());
});

View File

@@ -11,6 +11,7 @@ import jwt from 'jsonwebtoken';
import { AuthService } from '@/auth/auth.service';
import config from '@/config';
import { AUTH_COOKIE_NAME } from '@/constants';
import type { MfaService } from '@/mfa/mfa.service';
import { JwtService } from '@/services/jwt.service';
import type { UrlService } from '@/services/url.service';
@@ -31,6 +32,7 @@ describe('AuthService', () => {
const urlService = mock<UrlService>();
const userRepository = mock<UserRepository>();
const invalidAuthTokenRepository = mock<InvalidAuthTokenRepository>();
const mfaService = mock<MfaService>();
const authService = new AuthService(
globalConfig,
mock(),
@@ -39,13 +41,17 @@ describe('AuthService', () => {
urlService,
userRepository,
invalidAuthTokenRepository,
mfaService,
);
const now = new Date('2024-02-01T01:23:45.678Z');
jest.useFakeTimers({ now });
const validToken =
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjEyMyIsImhhc2giOiJtSkFZeDRXYjdrIiwiYnJvd3NlcklkIjoiOFpDVXE1YU1uSFhnMFZvcURLcm9hMHNaZ0NwdWlPQ1AzLzB2UmZKUXU0MD0iLCJpYXQiOjE3MDY3NTA2MjUsImV4cCI6MTcwNzM1NTQyNX0.YE-ZGGIQRNQ4DzUe9rjXvOOFFN9ufU34WibsCxAsc4o'; // Generated using `authService.issueJWT(user, browserId)`
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjEyMyIsImhhc2giOiJtSkFZeDRXYjdrIiwiYnJvd3NlcklkIjoiOFpDVXE1YU1uSFhnMFZvcURLcm9hMHNaZ0NwdWlPQ1AzLzB2UmZKUXU0MD0iLCJ1c2VkTWZhIjpmYWxzZSwiaWF0IjoxNzA2NzUwNjI1LCJleHAiOjE3MDczNTU0MjV9.N7JgwETmO41o4FUDVb4pA1HM3Clj4jyjDK-lE8Fa1Zw'; // Generated using `authService.issueJWT(user, false, browserId)`
const validTokenWithMfa =
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjEyMyIsImhhc2giOiJtSkFZeDRXYjdrIiwiYnJvd3NlcklkIjoiOFpDVXE1YU1uSFhnMFZvcURLcm9hMHNaZ0NwdWlPQ1AzLzB2UmZKUXU0MD0iLCJ1c2VkTWZhIjp0cnVlLCJpYXQiOjE3MDY3NTA2MjUsImV4cCI6MTcwNzM1NTQyNX0.9kTTue-ZdBQ0CblH0IrqW9K-k0WWfxfsWTglyPB10ko'; // Generated using `authService.issueJWT(user, true, browserId)`
beforeEach(() => {
jest.resetAllMocks();
@@ -107,7 +113,9 @@ describe('AuthService', () => {
it('should 401 if no cookie is set', async () => {
req.cookies[AUTH_COOKIE_NAME] = undefined;
await authService.authMiddleware(req, res, next);
const middleware = authService.createAuthMiddleware(true);
await middleware(req, res, next);
expect(invalidAuthTokenRepository.existsBy).not.toHaveBeenCalled();
expect(next).not.toHaveBeenCalled();
@@ -119,7 +127,9 @@ describe('AuthService', () => {
invalidAuthTokenRepository.existsBy.mockResolvedValue(false);
jest.advanceTimersByTime(365 * Time.days.toMilliseconds);
await authService.authMiddleware(req, res, next);
const middleware = authService.createAuthMiddleware(true);
await middleware(req, res, next);
expect(invalidAuthTokenRepository.existsBy).toHaveBeenCalled();
expect(userRepository.findOne).not.toHaveBeenCalled();
@@ -132,7 +142,9 @@ describe('AuthService', () => {
req.cookies[AUTH_COOKIE_NAME] = validToken;
invalidAuthTokenRepository.existsBy.mockResolvedValue(true);
await authService.authMiddleware(req, res, next);
const middleware = authService.createAuthMiddleware(true);
await middleware(req, res, next);
expect(invalidAuthTokenRepository.existsBy).toHaveBeenCalled();
expect(userRepository.findOne).not.toHaveBeenCalled();
@@ -141,13 +153,34 @@ describe('AuthService', () => {
expect(res.clearCookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME);
});
it('should 401 but not clear the cookie if 2FA is enforced and not configured for the user', async () => {
req.cookies[AUTH_COOKIE_NAME] = validToken;
userRepository.findOne.mockResolvedValue(user);
invalidAuthTokenRepository.existsBy.mockResolvedValue(false);
mfaService.isMFAEnforced.mockImplementation(() => {
return true;
});
const middleware = authService.createAuthMiddleware(false);
await middleware(req, res, next);
expect(invalidAuthTokenRepository.existsBy).toHaveBeenCalled();
expect(userRepository.findOne).toHaveBeenCalled();
expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(401);
expect(res.clearCookie).not.toHaveBeenCalledWith();
});
it('should refresh the cookie before it expires', async () => {
req.cookies[AUTH_COOKIE_NAME] = validToken;
jest.advanceTimersByTime(6 * Time.days.toMilliseconds);
invalidAuthTokenRepository.existsBy.mockResolvedValue(false);
userRepository.findOne.mockResolvedValue(user);
await authService.authMiddleware(req, res, next);
const middleware = authService.createAuthMiddleware(true);
await middleware(req, res, next);
expect(next).toHaveBeenCalled();
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', expect.any(String), {
@@ -162,7 +195,7 @@ describe('AuthService', () => {
describe('issueCookie', () => {
const res = mock<Response>();
it('should issue a cookie with the correct options', () => {
authService.issueCookie(res, user, browserId);
authService.issueCookie(res, user, false, browserId);
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', validToken, {
httpOnly: true,
@@ -172,10 +205,21 @@ describe('AuthService', () => {
});
});
it('should issue a cookie with the correct options, when 2FA was used', () => {
authService.issueCookie(res, user, true, browserId);
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', validTokenWithMfa, {
httpOnly: true,
maxAge: 604800000,
sameSite: 'lax',
secure: true,
});
});
it('should allow changing cookie options', () => {
globalConfig.auth.cookie = { secure: false, samesite: 'none' };
authService.issueCookie(res, user, browserId);
authService.issueCookie(res, user, false, browserId);
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', validToken, {
httpOnly: true,
@@ -190,7 +234,7 @@ describe('AuthService', () => {
describe('when not setting userManagement.jwtSessionDuration', () => {
it('should default to expire in 7 days', () => {
const defaultInSeconds = 7 * Time.days.toSeconds;
const token = authService.issueJWT(user, browserId);
const token = authService.issueJWT(user, false, browserId);
expect(authService.jwtExpiration).toBe(defaultInSeconds);
const decodedToken = jwtService.verify(token);
@@ -208,7 +252,7 @@ describe('AuthService', () => {
it('should apply it to tokens', () => {
config.set('userManagement.jwtSessionDurationHours', testDurationHours);
const token = authService.issueJWT(user, browserId);
const token = authService.issueJWT(user, false, browserId);
const decodedToken = jwtService.verify(token);
if (decodedToken.exp === undefined || decodedToken.iat === undefined) {
@@ -280,11 +324,17 @@ describe('AuthService', () => {
it('should refresh the cookie before it expires', async () => {
userRepository.findOne.mockResolvedValue(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual([
user,
{ usedMfa: false },
]);
expect(res.cookie).not.toHaveBeenCalled();
jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual([
user,
{ usedMfa: false },
]);
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', expect.any(String), {
httpOnly: true,
maxAge: 604800000,
@@ -294,7 +344,7 @@ describe('AuthService', () => {
const newToken = res.cookie.mock.calls[0].at(1);
expect(newToken).not.toBe(validToken);
expect(await authService.resolveJwt(newToken, req, res)).toEqual(user);
expect(await authService.resolveJwt(newToken, req, res)).toEqual([user, { usedMfa: false }]);
expect((jwt.decode(newToken) as jwt.JwtPayload).browserId).toEqual(
(jwt.decode(validToken) as jwt.JwtPayload).browserId,
);
@@ -302,15 +352,24 @@ describe('AuthService', () => {
it('should refresh the cookie only if less than 1/4th of time is left', async () => {
userRepository.findOne.mockResolvedValue(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual([
user,
{ usedMfa: false },
]);
expect(res.cookie).not.toHaveBeenCalled();
jest.advanceTimersByTime(5 * Time.days.toMilliseconds);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual([
user,
{ usedMfa: false },
]);
expect(res.cookie).not.toHaveBeenCalled();
jest.advanceTimersByTime(1 * Time.days.toMilliseconds);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual([
user,
{ usedMfa: false },
]);
expect(res.cookie).toHaveBeenCalled();
});
@@ -318,11 +377,17 @@ describe('AuthService', () => {
config.set('userManagement.jwtRefreshTimeoutHours', -1);
userRepository.findOne.mockResolvedValue(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual([
user,
{ usedMfa: false },
]);
expect(res.cookie).not.toHaveBeenCalled();
jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual([
user,
{ usedMfa: false },
]);
expect(res.cookie).not.toHaveBeenCalled();
});
});

View File

@@ -14,6 +14,7 @@ import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from '@/constants';
import { AuthError } from '@/errors/response-errors/auth.error';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
import { License } from '@/license';
import { MfaService } from '@/mfa/mfa.service';
import { JwtService } from '@/services/jwt.service';
import { UrlService } from '@/services/url.service';
@@ -24,6 +25,8 @@ interface AuthJwtPayload {
hash: string;
/** This is a client generated unique string to prevent session hijacking */
browserId?: string;
/** This indicates if mfa was used during the creation of this token */
usedMfa?: boolean;
}
interface IssuedJWT extends AuthJwtPayload {
@@ -48,10 +51,8 @@ export class AuthService {
private readonly urlService: UrlService,
private readonly userRepository: UserRepository,
private readonly invalidAuthTokenRepository: InvalidAuthTokenRepository,
private readonly mfaService: MfaService,
) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.authMiddleware = this.authMiddleware.bind(this);
const restEndpoint = globalConfig.endpoints.rest;
this.skipBrowserIdCheckEndpoints = [
// we need to exclude push endpoint because we can't send custom header on websocket requests
@@ -67,24 +68,44 @@ export class AuthService {
];
}
async authMiddleware(req: AuthenticatedRequest, res: Response, next: NextFunction) {
const token = req.cookies[AUTH_COOKIE_NAME];
if (token) {
try {
const isInvalid = await this.invalidAuthTokenRepository.existsBy({ token });
if (isInvalid) throw new AuthError('Unauthorized');
req.user = await this.resolveJwt(token, req, res);
} catch (error) {
if (error instanceof JsonWebTokenError || error instanceof AuthError) {
this.clearCookie(res);
} else {
throw error;
createAuthMiddleware(allowSkipMFA: boolean) {
return async (req: AuthenticatedRequest, res: Response, next: NextFunction) => {
const token = req.cookies[AUTH_COOKIE_NAME];
if (token) {
try {
const isInvalid = await this.invalidAuthTokenRepository.existsBy({ token });
if (isInvalid) throw new AuthError('Unauthorized');
const [user, { usedMfa }] = await this.resolveJwt(token, req, res);
const mfaEnforced = this.mfaService.isMFAEnforced();
if (mfaEnforced && !usedMfa && !allowSkipMFA) {
// If MFA is enforced, we need to check if the user has MFA enabled and used it during authentication
if (user.mfaEnabled) {
// If the user has MFA enforced, but did not use it during authentication, we need to throw an error
throw new AuthError('MFA not used during authentication');
} else {
// In this case we don't want to clear the cookie, to allow for MFA setup
res.status(401).json({ status: 'error', message: 'Unauthorized', mfaRequired: true });
return;
}
}
req.user = user;
req.authInfo = {
usedMfa,
};
} catch (error) {
if (error instanceof JsonWebTokenError || error instanceof AuthError) {
this.clearCookie(res);
} else {
throw error;
}
}
}
}
if (req.user) next();
else res.status(401).json({ status: 'error', message: 'Unauthorized' });
if (req.user) next();
else res.status(401).json({ status: 'error', message: 'Unauthorized' });
};
}
clearCookie(res: Response) {
@@ -107,7 +128,7 @@ export class AuthService {
}
}
issueCookie(res: Response, user: User, browserId?: string) {
issueCookie(res: Response, user: User, usedMfa: boolean, browserId?: string) {
// TODO: move this check to the login endpoint in AuthController
// If the instance has exceeded its user quota, prevent non-owners from logging in
const isWithinUsersLimit = this.license.isWithinUsersLimit();
@@ -119,7 +140,7 @@ export class AuthService {
throw new ForbiddenError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED);
}
const token = this.issueJWT(user, browserId);
const token = this.issueJWT(user, usedMfa, browserId);
const { samesite, secure } = this.globalConfig.auth.cookie;
res.cookie(AUTH_COOKIE_NAME, token, {
maxAge: this.jwtExpiration * Time.seconds.toMilliseconds,
@@ -129,18 +150,23 @@ export class AuthService {
});
}
issueJWT(user: User, browserId?: string) {
issueJWT(user: User, usedMfa: boolean = false, browserId?: string) {
const payload: AuthJwtPayload = {
id: user.id,
hash: this.createJWTHash(user),
browserId: browserId && this.hash(browserId),
usedMfa,
};
return this.jwtService.sign(payload, {
expiresIn: this.jwtExpiration,
});
}
async resolveJwt(token: string, req: AuthenticatedRequest, res: Response): Promise<User> {
async resolveJwt(
token: string,
req: AuthenticatedRequest,
res: Response,
): Promise<[User, { usedMfa: boolean }]> {
const jwtPayload: IssuedJWT = this.jwtService.verify(token, {
algorithms: ['HS256'],
});
@@ -175,10 +201,10 @@ export class AuthService {
if (jwtPayload.exp * 1000 - Date.now() < this.jwtRefreshTimeout) {
this.logger.debug('JWT about to expire. Will be refreshed');
this.issueCookie(res, user, req.browserId);
this.issueCookie(res, user, jwtPayload.usedMfa ?? false, req.browserId);
}
return user;
return [user, { usedMfa: jwtPayload.usedMfa ?? false }];
}
generatePasswordResetToken(user: User, expiresIn: TimeUnitValue = '20m') {

View File

@@ -8,5 +8,5 @@ import { AuthService } from './auth.service';
// DO NOT DELETE until the hooks have been updated
/** @deprecated Use `AuthService` instead */
export function issueCookie(res: Response, user: User) {
return Container.get(AuthService).issueCookie(res, user);
return Container.get(AuthService).issueCookie(res, user, user.mfaEnabled);
}

View File

@@ -87,7 +87,7 @@ export class ControllerRegistry {
...(route.skipAuth
? []
: ([
this.authService.authMiddleware.bind(this.authService),
this.authService.createAuthMiddleware(route.allowSkipMFA),
this.lastActiveAtService.middleware.bind(this.lastActiveAtService),
] as RequestHandler[])),
...(route.licenseFeature ? [this.createLicenseMiddleware(route.licenseFeature)] : []),

View File

@@ -87,13 +87,14 @@ describe('AuthController', () => {
body.password,
);
expect(authService.issueCookie).toHaveBeenCalledWith(res, member, browserId);
expect(authService.issueCookie).toHaveBeenCalledWith(res, member, false, browserId);
expect(eventsService.emit).toHaveBeenCalledWith('user-logged-in', {
user: member,
authenticationMethod: 'ldap',
});
expect(userService.toPublic).toHaveBeenCalledWith(member, {
mfaAuthenticated: false,
posthog: postHog,
withScopes: true,
});

View File

@@ -66,7 +66,7 @@ describe('OwnerController', () => {
authIdentities: [],
});
const browserId = 'test-browser-id';
const req = mock<AuthenticatedRequest>({ user, browserId });
const req = mock<AuthenticatedRequest>({ user, browserId, authInfo: { usedMfa: false } });
const res = mock<Response>();
const payload = mock<OwnerSetupRequestDto>({
email: 'valid@email.com',
@@ -85,7 +85,7 @@ describe('OwnerController', () => {
where: { role: 'global:owner' },
});
expect(userRepository.save).toHaveBeenCalledWith(user, { transaction: false });
expect(authService.issueCookie).toHaveBeenCalledWith(res, user, browserId);
expect(authService.issueCookie).toHaveBeenCalledWith(res, user, false, browserId);
expect(settingsRepository.update).toHaveBeenCalledWith(
{ key: 'userManagement.isInstanceOwnerSetUp' },
{ value: JSON.stringify(true) },

View File

@@ -97,14 +97,19 @@ export class AuthController {
}
}
this.authService.issueCookie(res, user, req.browserId);
// If user.mfaEnabled is enabled we checked for the MFA code, therefore it was used during this login execution
this.authService.issueCookie(res, user, user.mfaEnabled, req.browserId);
this.eventService.emit('user-logged-in', {
user,
authenticationMethod: usedAuthenticationMethod,
});
return await this.userService.toPublic(user, { posthog: this.postHog, withScopes: true });
return await this.userService.toPublic(user, {
posthog: this.postHog,
withScopes: true,
mfaAuthenticated: user.mfaEnabled,
});
}
this.eventService.emit('user-login-failed', {
authenticationMethod: usedAuthenticationMethod,
@@ -115,11 +120,14 @@ export class AuthController {
}
/** Check if the user is already logged in */
@Get('/login')
@Get('/login', {
allowSkipMFA: true,
})
async currentUser(req: AuthenticatedRequest): Promise<PublicUser> {
return await this.userService.toPublic(req.user, {
posthog: this.postHog,
withScopes: true,
mfaAuthenticated: req.authInfo?.usedMfa,
});
}

View File

@@ -106,6 +106,7 @@ export class E2EController {
[LICENSE_FEATURES.INSIGHTS_VIEW_HOURLY_DATA]: false,
[LICENSE_FEATURES.API_KEY_SCOPES]: false,
[LICENSE_FEATURES.OIDC]: false,
[LICENSE_FEATURES.MFA_ENFORCEMENT]: false,
};
private static readonly numericFeaturesDefaults: Record<NumericLicenseFeature, number> = {

View File

@@ -128,7 +128,7 @@ export class InvitationController {
const updatedUser = await this.userRepository.save(invitee, { transaction: false });
this.authService.issueCookie(res, updatedUser, req.browserId);
this.authService.issueCookie(res, updatedUser, false, req.browserId);
this.eventService.emit('user-signed-up', {
user: updatedUser,

View File

@@ -113,7 +113,7 @@ export class MeController {
this.logger.info('User updated successfully', { userId });
this.authService.issueCookie(res, user, req.browserId);
this.authService.issueCookie(res, user, req.authInfo?.usedMfa ?? false, req.browserId);
const changeableFields = ['email', 'firstName', 'lastName'] as const;
const fieldsChanged = changeableFields.filter(
@@ -183,7 +183,7 @@ export class MeController {
const updatedUser = await this.userRepository.save(user, { transaction: false });
this.logger.info('Password updated successfully', { userId: user.id });
this.authService.issueCookie(res, updatedUser, req.browserId);
this.authService.issueCookie(res, updatedUser, req.authInfo?.usedMfa ?? false, req.browserId);
this.eventService.emit('user-updated', { user: updatedUser, fieldsChanged: ['password'] });

View File

@@ -1,5 +1,5 @@
import { AuthenticatedRequest, UserRepository } from '@n8n/db';
import { Get, Post, RestController } from '@n8n/decorators';
import { Get, GlobalScope, Post, RestController } from '@n8n/decorators';
import { Response } from 'express';
import { AuthService } from '@/auth/auth.service';
@@ -17,13 +17,32 @@ export class MFAController {
private userRepository: UserRepository,
) {}
@Post('/can-enable')
@Post('/enforce-mfa')
@GlobalScope('user:enforceMfa')
async enforceMFA(req: MFA.Enforce) {
if (req.body.enforce && !(req.authInfo?.usedMfa ?? false)) {
// The current user tries to enforce MFA, but does not have
// MFA set up for them self. We are forbidding this, to
// help the user not lock them selfs out.
throw new BadRequestError(
'You must enable two-factor authentication on your own account before enforcing it for all users',
);
}
await this.mfaService.enforceMFA(req.body.enforce);
return;
}
@Post('/can-enable', {
allowSkipMFA: true,
})
async canEnableMFA(req: AuthenticatedRequest) {
await this.externalHooks.run('mfa.beforeSetup', [req.user]);
return;
}
@Get('/qr')
@Get('/qr', {
allowSkipMFA: true,
})
async getQRCode(req: AuthenticatedRequest) {
const { email, id, mfaEnabled } = req.user;
@@ -63,7 +82,7 @@ export class MFAController {
};
}
@Post('/enable', { rateLimit: true })
@Post('/enable', { rateLimit: true, allowSkipMFA: true })
async activateMFA(req: MFA.Activate, res: Response) {
const { mfaCode = null } = req.body;
const { id, mfaEnabled } = req.user;
@@ -88,7 +107,7 @@ export class MFAController {
const updatedUser = await this.mfaService.enableMfa(id);
this.authService.issueCookie(res, updatedUser, req.browserId);
this.authService.issueCookie(res, updatedUser, verified, req.browserId);
}
@Post('/disable', { rateLimit: true })
@@ -115,10 +134,10 @@ export class MFAController {
const updatedUser = await this.userRepository.findOneByOrFail({ id: userId });
this.authService.issueCookie(res, updatedUser, req.browserId);
this.authService.issueCookie(res, updatedUser, false, req.browserId);
}
@Post('/verify', { rateLimit: true })
@Post('/verify', { rateLimit: true, allowSkipMFA: true })
async verifyMFA(req: MFA.Verify) {
const { id } = req.user;
const { mfaCode } = req.body;

View File

@@ -67,7 +67,7 @@ export class OwnerController {
this.logger.debug('Setting isInstanceOwnerSetUp updated successfully');
this.authService.issueCookie(res, owner, req.browserId);
this.authService.issueCookie(res, owner, req.authInfo?.usedMfa ?? false, req.browserId);
this.eventService.emit('instance-owner-setup', { userId: owner.id });

View File

@@ -189,7 +189,7 @@ export class PasswordResetController {
this.logger.info('User password updated successfully', { userId: user.id });
this.authService.issueCookie(res, user, req.browserId);
this.authService.issueCookie(res, user, user.mfaEnabled, req.browserId);
this.eventService.emit('user-updated', { user, fieldsChanged: ['password'] });

View File

@@ -0,0 +1,2 @@
export const MFA_FEATURE_ENABLED = 'mfa.enabled';
export const MFA_ENFORCE_SETTING = 'mfa.enforced';

View File

@@ -1,4 +1,5 @@
import { UserRepository } from '@n8n/db';
import { LicenseState, Logger } from '@n8n/backend-common';
import { SettingsRepository, UserRepository } from '@n8n/db';
import { Service } from '@n8n/di';
import { Cipher } from 'n8n-core';
import { v4 as uuid } from 'uuid';
@@ -6,20 +7,61 @@ import { v4 as uuid } from 'uuid';
import { InvalidMfaCodeError } from '@/errors/response-errors/invalid-mfa-code.error';
import { InvalidMfaRecoveryCodeError } from '@/errors/response-errors/invalid-mfa-recovery-code-error';
import { MFA_ENFORCE_SETTING } from './constants';
import { TOTPService } from './totp.service';
@Service()
export class MfaService {
private enforceMFAValue: boolean = false;
constructor(
private userRepository: UserRepository,
private settingsRepository: SettingsRepository,
private license: LicenseState,
public totp: TOTPService,
private cipher: Cipher,
private logger: Logger,
) {}
async init() {
try {
await this.loadMFASettings();
} catch (error) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.logger.warn('Failed to load MFA settings', { error });
}
}
generateRecoveryCodes(n = 10) {
return Array.from(Array(n)).map(() => uuid());
}
private async loadMFASettings() {
const value = await this.settingsRepository.findByKey(MFA_ENFORCE_SETTING);
if (value) {
this.enforceMFAValue = value.value === 'true';
}
}
async enforceMFA(value: boolean) {
if (!this.license.isMFAEnforcementLicensed()) {
value = false; // If the license does not allow MFA enforcement, set it to false
}
await this.settingsRepository.upsert(
{
key: MFA_ENFORCE_SETTING,
value: `${value}`,
loadOnStartup: true,
},
['key'],
);
this.enforceMFAValue = value;
}
isMFAEnforced() {
return this.license.isMFAEnforcementLicensed() && this.enforceMFAValue;
}
async saveSecretAndRecoveryCodes(userId: string, secret: string, recoveryCodes: string[]) {
const { encryptedSecret, encryptedRecoveryCodes } = this.encryptSecretAndRecoveryCodes(
secret,

View File

@@ -95,7 +95,7 @@ export class Push extends TypedEmitter<PushEvents> {
app.use(
`/${restEndpoint}/push`,
// eslint-disable-next-line @typescript-eslint/unbound-method
this.authService.authMiddleware,
this.authService.createAuthMiddleware(false),
(req: SSEPushRequest | WebSocketPushRequest, res: PushResponse) =>
this.handleRequest(req, res),
);

View File

@@ -149,6 +149,7 @@ export declare namespace UserRequest {
// ----------------------------------
export declare namespace MFA {
type Enforce = AuthenticatedRequest<{}, {}, { enforce: boolean }, {}>;
type Verify = AuthenticatedRequest<{}, {}, { mfaCode: string }, {}>;
type Activate = AuthenticatedRequest<{}, {}, { mfaCode: string }, {}>;
type Disable = AuthenticatedRequest<{}, {}, { mfaCode?: string; mfaRecoveryCode?: string }, {}>;

View File

@@ -62,6 +62,7 @@ import '@/evaluation.ee/test-runs.controller.ee';
import '@/workflows/workflow-history.ee/workflow-history.controller.ee';
import '@/workflows/workflows.controller';
import '@/webhooks/webhooks.controller';
import { MfaService } from './mfa/mfa.service';
@Service()
export class Server extends AbstractServer {
@@ -125,6 +126,7 @@ export class Server extends AbstractServer {
}
if (isMfaFeatureEnabled()) {
await Container.get(MfaService).init();
await import('@/controllers/mfa.controller');
}

View File

@@ -22,6 +22,11 @@ describe('HooksService', () => {
const settingsRepository = mock<SettingsRepository>();
const workflowRepository = mock<WorkflowRepository>();
const credentialsRepository = mock<CredentialsRepository>();
const authMiddleware = jest.fn();
authService.createAuthMiddleware.mockReturnValue(authMiddleware);
const hooksService = new HooksService(
userService,
authService,
@@ -49,12 +54,13 @@ describe('HooksService', () => {
it('hooksService.issueCookie should call authService.issueCookie', async () => {
// ARRANGE
const res = mock<Response>();
mockedUser.mfaEnabled = false; // Mock mfaEnabled property
// ACT
hooksService.issueCookie(res, mockedUser);
// ASSERT
expect(authService.issueCookie).toHaveBeenCalledWith(res, mockedUser);
expect(authService.issueCookie).toHaveBeenCalledWith(res, mockedUser, false);
});
it('hooksService.findOneUser should call userRepository.findOne', async () => {
@@ -134,7 +140,7 @@ describe('HooksService', () => {
await hooksService.authMiddleware(req, res, next);
// ASSERT
expect(authService.authMiddleware).toHaveBeenCalledWith(req, res, next);
expect(authMiddleware).toHaveBeenCalledWith(req, res, next);
});
it('hooksService.dbCollections should return valid repositories', async () => {

View File

@@ -17,6 +17,7 @@ import { CredentialsOverwrites } from '@/credentials-overwrites';
import { getLdapLoginLabel } from '@/ldap.ee/helpers.ee';
import { License } from '@/license';
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
import { MfaService } from '@/mfa/mfa.service';
import { isApiEnabled } from '@/public-api';
import { PushConfig } from '@/push/push.config';
import type { CommunityPackagesService } from '@/services/community-packages.service';
@@ -51,6 +52,7 @@ export class FrontendService {
private readonly binaryDataConfig: BinaryDataConfig,
private readonly licenseState: LicenseState,
private readonly moduleRegistry: ModuleRegistry,
private readonly mfaService: MfaService,
) {
loadNodesAndCredentials.addPostProcessor(async () => await this.generateTypes());
void this.generateTypes();
@@ -195,6 +197,7 @@ export class FrontendService {
ldap: false,
saml: false,
oidc: false,
mfaEnforcement: false,
logStreaming: false,
advancedExecutionFilters: false,
variables: false,
@@ -216,6 +219,7 @@ export class FrontendService {
},
mfa: {
enabled: false,
enforced: false,
},
hideUsagePage: this.globalConfig.hideUsagePage,
license: {
@@ -321,6 +325,7 @@ export class FrontendService {
ldap: this.license.isLdapEnabled(),
saml: this.license.isSamlEnabled(),
oidc: this.licenseState.isOidcLicensed(),
mfaEnforcement: this.licenseState.isMFAEnforcementLicensed(),
advancedExecutionFilters: this.license.isAdvancedExecutionFiltersEnabled(),
variables: this.license.isVariablesEnabled(),
sourceControl: this.license.isSourceControlLicensed(),
@@ -385,6 +390,9 @@ export class FrontendService {
this.settings.mfa.enabled = this.globalConfig.mfa.enabled;
// TODO: read from settings
this.settings.mfa.enforced = this.mfaService.isMFAEnforced();
this.settings.executionMode = config.getEnv('executions.mode');
this.settings.binaryDataMode = this.binaryDataConfig.mode;

View File

@@ -28,6 +28,12 @@ import { UserService } from '@/services/user.service';
*/
@Service()
export class HooksService {
private innerAuthMiddleware: (
req: AuthenticatedRequest,
res: Response,
next: NextFunction,
) => Promise<void>;
constructor(
private readonly userService: UserService,
private readonly authService: AuthService,
@@ -35,7 +41,9 @@ export class HooksService {
private readonly settingsRepository: SettingsRepository,
private readonly workflowRepository: WorkflowRepository,
private readonly credentialsRepository: CredentialsRepository,
) {}
) {
this.innerAuthMiddleware = authService.createAuthMiddleware(false);
}
/**
* Invite users to instance during signup
@@ -49,7 +57,10 @@ export class HooksService {
* the user after instance is provisioned
*/
issueCookie(res: Response, user: User) {
return this.authService.issueCookie(res, user);
// TODO: The information on user has mfa enabled here, is missing!!
// This could be a security problem!!
// This is in just for the hackmation!!
return this.authService.issueCookie(res, user, user.mfaEnabled);
}
/**
@@ -105,7 +116,7 @@ export class HooksService {
* 1. To authenticate the /proxy routes in the hooks
*/
async authMiddleware(req: AuthenticatedRequest, res: Response, next: NextFunction) {
return await this.authService.authMiddleware(req, res, next);
return await this.innerAuthMiddleware(req, res, next);
}
getRudderStackClient(key: string, options: constructorOptions): RudderStack {

View File

@@ -61,6 +61,7 @@ export class UserService {
inviterId?: string;
posthog?: PostHogClient;
withScopes?: boolean;
mfaAuthenticated?: boolean;
},
) {
const { password, updatedAt, authIdentities, mfaRecoveryCodes, mfaSecret, ...rest } = user;
@@ -90,6 +91,8 @@ export class UserService {
publicUser.globalScopes = getGlobalScopes(user);
}
publicUser.mfaAuthenticated = options?.mfaAuthenticated ?? false;
return publicUser;
}

View File

@@ -55,7 +55,7 @@ export class OidcController {
const user = await this.oidcService.loginUser(callbackUrl);
this.authService.issueCookie(res, user);
this.authService.issueCookie(res, user, false);
res.redirect('/');
}

View File

@@ -127,7 +127,7 @@ export class SamlController {
// Only sign in user if SAML is enabled, otherwise treat as test connection
if (isSamlLicensedAndEnabled()) {
this.authService.issueCookie(res, loginResult.authenticatedUser, req.browserId);
this.authService.issueCookie(res, loginResult.authenticatedUser, false, req.browserId);
if (loginResult.onboardingRequired) {
return res.redirect(this.urlService.getInstanceBaseUrl() + '/saml/onboarding');
} else {

View File

@@ -1,7 +1,8 @@
import { randomValidPassword, uniqueId } from '@n8n/backend-test-utils';
import { testDb } from '@n8n/backend-test-utils';
import { mockInstance } from '@n8n/backend-test-utils';
import { UserRepository, type User } from '@n8n/db';
import { LICENSE_FEATURES } from '@n8n/constants';
import { SettingsRepository, UserRepository, type User } from '@n8n/db';
import { Container } from '@n8n/di';
import { randomString } from 'n8n-workflow';
@@ -9,6 +10,7 @@ import { AuthService } from '@/auth/auth.service';
import config from '@/config';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { ExternalHooks } from '@/external-hooks';
import { MFA_ENFORCE_SETTING } from '@/mfa/constants';
import { TOTPService } from '@/mfa/totp.service';
import { createOwner, createUser, createUserWithMfaEnabled } from '../shared/db/users';
@@ -22,6 +24,7 @@ const externalHooks = mockInstance(ExternalHooks);
const testServer = utils.setupTestServer({
endpointGroups: ['mfa', 'auth', 'me', 'passwordReset'],
enabledFeatures: [LICENSE_FEATURES.MFA_ENFORCEMENT],
});
beforeEach(async () => {
@@ -405,3 +408,61 @@ describe('Login', () => {
});
});
});
describe('Enforce MFA', () => {
test('Enforce MFA for the instance', async () => {
const settingsRepository = Container.get(SettingsRepository);
await settingsRepository.delete({
key: MFA_ENFORCE_SETTING,
});
let enforced = await settingsRepository.findByKey(MFA_ENFORCE_SETTING);
expect(enforced).toBe(null);
owner.mfaEnabled = true;
await testServer
.authAgentFor(owner)
.post('/mfa/enforce-mfa')
.send({ enforce: true })
.expect(200);
owner.mfaEnabled = false;
enforced = await settingsRepository.findByKey(MFA_ENFORCE_SETTING);
expect(enforced?.value).toBe('true');
await settingsRepository.delete({
key: MFA_ENFORCE_SETTING,
});
});
test('Disable MFA for the instance', async () => {
const settingsRepository = Container.get(SettingsRepository);
await settingsRepository.delete({
key: MFA_ENFORCE_SETTING,
});
let enforced = await settingsRepository.findByKey(MFA_ENFORCE_SETTING);
expect(enforced).toBe(null);
owner.mfaEnabled = true;
await testServer
.authAgentFor(owner)
.post('/mfa/enforce-mfa')
.send({ enforce: false })
.expect(200);
owner.mfaEnabled = false;
enforced = await settingsRepository.findByKey(MFA_ENFORCE_SETTING);
expect(enforced?.value).toBe('false');
await settingsRepository.delete({
key: MFA_ENFORCE_SETTING,
});
});
});

View File

@@ -58,7 +58,11 @@ function createAgent(
if (withRestSegment) void agent.use(prefix(REST_PATH_SEGMENT));
if (options?.auth && options?.user) {
const token = Container.get(AuthService).issueJWT(options.user, browserId);
const token = Container.get(AuthService).issueJWT(
options.user,
options.user.mfaEnabled,
browserId,
);
agent.jar.setCookie(`${AUTH_COOKIE_NAME}=${token}`);
}
return agent;