feat(core): Prevent session hijacking (#9057)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2024-04-09 11:20:35 +02:00 committed by GitHub
parent 5793e5644a
commit 28261047c3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 124 additions and 53 deletions

View file

@ -62,7 +62,11 @@ Cypress.Commands.add('signinAsOwner', () => {
});
Cypress.Commands.add('signout', () => {
cy.request('POST', `${BACKEND_BASE_URL}/rest/logout`);
cy.request({
method: 'POST',
url: `${BACKEND_BASE_URL}/rest/logout`,
headers: { 'browser-id': localStorage.getItem('n8n-browserId') }
});
cy.getCookie(N8N_AUTH_COOKIE).should('not.exist');
});

View file

@ -33,7 +33,7 @@ import {
TEMPLATES_DIR,
} from '@/constants';
import { CredentialsController } from '@/credentials/credentials.controller';
import type { CurlHelper } from '@/requests';
import type { APIRequest, CurlHelper } from '@/requests';
import { registerController } from '@/decorators';
import { AuthController } from '@/controllers/auth.controller';
import { BinaryDataController } from '@/controllers/binaryData.controller';
@ -235,6 +235,13 @@ export class Server extends AbstractServer {
frontendService.settings.publicApi.latestVersion = apiLatestVersion;
}
}
// Extract BrowserId from headers
this.app.use((req: APIRequest, _, next) => {
req.browserId = req.headers['browser-id'] as string;
next();
});
// Parse cookies for easier access
this.app.use(cookieParser());

View file

@ -20,6 +20,8 @@ interface AuthJwtPayload {
id: string;
/** This hash is derived from email and bcrypt of password */
hash: string;
/** This is a client generated unique string to prevent session hijacking */
browserId?: string;
}
interface IssuedJWT extends AuthJwtPayload {
@ -31,6 +33,8 @@ interface PasswordResetToken {
hash: string;
}
const pushEndpoint = `/${config.get('endpoints.rest')}/push`;
@Service()
export class AuthService {
constructor(
@ -48,7 +52,7 @@ export class AuthService {
const token = req.cookies[AUTH_COOKIE_NAME];
if (token) {
try {
req.user = await this.resolveJwt(token, res);
req.user = await this.resolveJwt(token, req, res);
} catch (error) {
if (error instanceof JsonWebTokenError || error instanceof AuthError) {
this.clearCookie(res);
@ -66,7 +70,8 @@ export class AuthService {
res.clearCookie(AUTH_COOKIE_NAME);
}
issueCookie(res: Response, user: User) {
issueCookie(res: Response, user: User, 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();
if (
@ -77,7 +82,7 @@ export class AuthService {
throw new UnauthorizedError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED);
}
const token = this.issueJWT(user);
const token = this.issueJWT(user, browserId);
res.cookie(AUTH_COOKIE_NAME, token, {
maxAge: this.jwtExpiration * Time.seconds.toMilliseconds,
httpOnly: true,
@ -86,17 +91,18 @@ export class AuthService {
});
}
issueJWT(user: User) {
issueJWT(user: User, browserId?: string) {
const payload: AuthJwtPayload = {
id: user.id,
hash: this.createJWTHash(user),
browserId: browserId && this.hash(browserId),
};
return this.jwtService.sign(payload, {
expiresIn: this.jwtExpiration,
});
}
async resolveJwt(token: string, res: Response): Promise<User> {
async resolveJwt(token: string, req: AuthenticatedRequest, res: Response): Promise<User> {
const jwtPayload: IssuedJWT = this.jwtService.verify(token, {
algorithms: ['HS256'],
});
@ -112,14 +118,20 @@ export class AuthService {
// or, If the user has been deactivated (i.e. LDAP users)
user.disabled ||
// or, If the email or password has been updated
jwtPayload.hash !== this.createJWTHash(user)
jwtPayload.hash !== this.createJWTHash(user) ||
// If the token was issued for another browser session
// NOTE: we need to exclude push endpoint from this check because we can't send custom header on websocket requests
// TODO: Implement a custom handshake for push, to avoid having to send any data on querystring or headers
(req.baseUrl !== pushEndpoint &&
jwtPayload.browserId &&
(!req.browserId || jwtPayload.browserId !== this.hash(req.browserId)))
) {
throw new AuthError('Unauthorized');
}
if (jwtPayload.exp * 1000 - Date.now() < this.jwtRefreshTimeout) {
this.logger.debug('JWT about to expire. Will be refreshed');
this.issueCookie(res, user);
this.issueCookie(res, user, jwtPayload.browserId);
}
return user;
@ -175,10 +187,11 @@ export class AuthService {
}
createJWTHash({ email, password }: User) {
const hash = createHash('sha256')
.update(email + ':' + password)
.digest('base64');
return hash.substring(0, 10);
return this.hash(email + ':' + password).substring(0, 10);
}
private hash(input: string) {
return createHash('sha256').update(input).digest('base64');
}
/** How many **milliseconds** before expiration should a JWT be renewed */

View file

@ -94,7 +94,7 @@ export class AuthController {
}
}
this.authService.issueCookie(res, user);
this.authService.issueCookie(res, user, req.browserId);
void this.internalHooks.onUserLoginSuccess({
user,
authenticationMethod: usedAuthenticationMethod,

View file

@ -164,7 +164,7 @@ export class InvitationController {
const updatedUser = await this.userRepository.save(invitee, { transaction: false });
this.authService.issueCookie(res, updatedUser);
this.authService.issueCookie(res, updatedUser, req.browserId);
void this.internalHooks.onUserSignup(updatedUser, {
user_type: 'email',

View file

@ -85,7 +85,7 @@ export class MeController {
this.logger.info('User updated successfully', { userId });
this.authService.issueCookie(res, user);
this.authService.issueCookie(res, user, req.browserId);
const updatedKeys = Object.keys(payload);
void this.internalHooks.onUserUpdate({
@ -138,7 +138,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);
this.authService.issueCookie(res, updatedUser, req.browserId);
void this.internalHooks.onUserUpdate({
user: updatedUser,

View file

@ -83,7 +83,7 @@ export class OwnerController {
this.logger.debug('Setting isInstanceOwnerSetUp updated successfully');
this.authService.issueCookie(res, owner);
this.authService.issueCookie(res, owner, req.browserId);
void this.internalHooks.onInstanceOwnerSetup({ user_id: owner.id });

View file

@ -208,7 +208,7 @@ export class PasswordResetController {
this.logger.info('User password updated successfully', { userId: user.id });
this.authService.issueCookie(res, user);
this.authService.issueCookie(res, user, req.browserId);
void this.internalHooks.onUserUpdate({
user,

View file

@ -8,7 +8,7 @@ export const corsMiddleware: RequestHandler = (req, res, next) => {
res.header('Access-Control-Allow-Methods', 'GET, POST, OPTIONS, PUT, PATCH, DELETE');
res.header(
'Access-Control-Allow-Headers',
'Origin, X-Requested-With, Content-Type, Accept, push-ref',
'Origin, X-Requested-With, Content-Type, Accept, push-ref, browser-id',
);
}

View file

@ -50,22 +50,30 @@ export class UserRoleChangePayload {
newRoleName: AssignableRole;
}
export type APIRequest<
RouteParams = {},
ResponseBody = {},
RequestBody = {},
RequestQuery = {},
> = express.Request<RouteParams, ResponseBody, RequestBody, RequestQuery> & {
browserId?: string;
};
export type AuthlessRequest<
RouteParams = {},
ResponseBody = {},
RequestBody = {},
RequestQuery = {},
> = express.Request<RouteParams, ResponseBody, RequestBody, RequestQuery>;
> = APIRequest<RouteParams, ResponseBody, RequestBody, RequestQuery> & {
user: never;
};
export type AuthenticatedRequest<
RouteParams = {},
ResponseBody = {},
RequestBody = {},
RequestQuery = {},
> = Omit<
express.Request<RouteParams, ResponseBody, RequestBody, RequestQuery>,
'user' | 'cookies'
> & {
> = Omit<APIRequest<RouteParams, ResponseBody, RequestBody, RequestQuery>, 'user' | 'cookies'> & {
user: User;
cookies: Record<string, string | undefined>;
};

View file

@ -138,7 +138,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);
this.authService.issueCookie(res, loginResult.authenticatedUser, req.browserId);
if (loginResult.onboardingRequired) {
return res.redirect(this.urlService.getInstanceBaseUrl() + SamlUrls.samlOnboarding);
} else {

View file

@ -1,6 +1,6 @@
import jwt from 'jsonwebtoken';
import { mock } from 'jest-mock-extended';
import { type NextFunction, type Response } from 'express';
import type { NextFunction, Response } from 'express';
import { AuthService } from '@/auth/auth.service';
import config from '@/config';
@ -14,6 +14,7 @@ import type { AuthenticatedRequest } from '@/requests';
describe('AuthService', () => {
config.set('userManagement.jwtSecret', 'random-secret');
const browserId = 'test-browser-id';
const userData = {
id: '123',
email: 'test@example.com',
@ -21,17 +22,18 @@ describe('AuthService', () => {
disabled: false,
mfaEnabled: false,
};
const validToken =
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjEyMyIsImhhc2giOiJtSkFZeDRXYjdrIiwiaWF0IjoxNzA2NzUwNjI1LCJleHAiOjE3MDczNTU0MjV9.JwY3doH0YrxHdX4nTOlTN4-QMaXsAu5OFOaFcIHSHBI';
const user = mock<User>(userData);
const jwtService = new JwtService(mock());
const urlService = mock<UrlService>();
const userRepository = mock<UserRepository>();
const authService = new AuthService(mock(), mock(), jwtService, urlService, userRepository);
jest.useFakeTimers();
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)`
beforeEach(() => {
jest.clearAllMocks();
jest.setSystemTime(now);
@ -54,7 +56,11 @@ describe('AuthService', () => {
});
describe('authMiddleware', () => {
const req = mock<AuthenticatedRequest>({ cookies: {}, user: undefined });
const req = mock<AuthenticatedRequest>({
cookies: {},
user: undefined,
browserId,
});
const res = mock<Response>();
const next = jest.fn() as NextFunction;
@ -99,7 +105,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);
const token = authService.issueJWT(user, browserId);
expect(authService.jwtExpiration).toBe(defaultInSeconds);
const decodedToken = jwtService.verify(token);
@ -117,7 +123,7 @@ describe('AuthService', () => {
it('should apply it to tokens', () => {
config.set('userManagement.jwtSessionDurationHours', testDurationHours);
const token = authService.issueJWT(user);
const token = authService.issueJWT(user, browserId);
const decodedToken = jwtService.verify(token);
if (decodedToken.exp === undefined || decodedToken.iat === undefined) {
@ -129,24 +135,40 @@ describe('AuthService', () => {
});
describe('resolveJwt', () => {
const req = mock<AuthenticatedRequest>({
cookies: {},
user: undefined,
browserId,
});
const res = mock<Response>();
it('should throw on invalid tokens', async () => {
await expect(authService.resolveJwt('random-string', res)).rejects.toThrow('jwt malformed');
await expect(authService.resolveJwt('random-string', req, res)).rejects.toThrow(
'jwt malformed',
);
expect(res.cookie).not.toHaveBeenCalled();
});
it('should throw on expired tokens', async () => {
jest.advanceTimersByTime(365 * Time.days.toMilliseconds);
await expect(authService.resolveJwt(validToken, res)).rejects.toThrow('jwt expired');
await expect(authService.resolveJwt(validToken, req, res)).rejects.toThrow('jwt expired');
expect(res.cookie).not.toHaveBeenCalled();
});
it('should throw on tampered tokens', async () => {
const [header, payload, signature] = validToken.split('.');
const tamperedToken = [header, payload, signature + '123'].join('.');
await expect(authService.resolveJwt(tamperedToken, res)).rejects.toThrow('invalid signature');
await expect(authService.resolveJwt(tamperedToken, req, res)).rejects.toThrow(
'invalid signature',
);
expect(res.cookie).not.toHaveBeenCalled();
});
it('should throw on hijacked tokens', async () => {
userRepository.findOne.mockResolvedValue(user);
const req = mock<AuthenticatedRequest>({ browserId: 'another-browser' });
await expect(authService.resolveJwt(validToken, req, res)).rejects.toThrow('Unauthorized');
expect(res.cookie).not.toHaveBeenCalled();
});
@ -163,17 +185,17 @@ describe('AuthService', () => {
],
])('should throw if %s', async (_, data) => {
userRepository.findOne.mockResolvedValueOnce(data && mock<User>(data));
await expect(authService.resolveJwt(validToken, res)).rejects.toThrow('Unauthorized');
await expect(authService.resolveJwt(validToken, req, res)).rejects.toThrow('Unauthorized');
expect(res.cookie).not.toHaveBeenCalled();
});
it('should refresh the cookie before it expires', async () => {
userRepository.findOne.mockResolvedValue(user);
expect(await authService.resolveJwt(validToken, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(res.cookie).not.toHaveBeenCalled();
jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days
expect(await authService.resolveJwt(validToken, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', expect.any(String), {
httpOnly: true,
maxAge: 604800000,
@ -184,15 +206,15 @@ 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, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(res.cookie).not.toHaveBeenCalled();
jest.advanceTimersByTime(5 * Time.days.toMilliseconds);
expect(await authService.resolveJwt(validToken, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(res.cookie).not.toHaveBeenCalled();
jest.advanceTimersByTime(1 * Time.days.toMilliseconds);
expect(await authService.resolveJwt(validToken, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(res.cookie).toHaveBeenCalled();
});
@ -200,11 +222,11 @@ describe('AuthService', () => {
config.set('userManagement.jwtRefreshTimeoutHours', -1);
userRepository.findOne.mockResolvedValue(user);
expect(await authService.resolveJwt(validToken, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(res.cookie).not.toHaveBeenCalled();
jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days
expect(await authService.resolveJwt(validToken, res)).toEqual(user);
expect(await authService.resolveJwt(validToken, req, res)).toEqual(user);
expect(res.cookie).not.toHaveBeenCalled();
});
});

View file

@ -16,6 +16,8 @@ import { UserRepository } from '@/databases/repositories/user.repository';
import { badPasswords } from '../shared/testData';
import { mockInstance } from '../../shared/mocking';
const browserId = 'test-browser-id';
describe('MeController', () => {
const externalHooks = mockInstance(ExternalHooks);
const internalHooks = mockInstance(InternalHooks);
@ -47,7 +49,7 @@ describe('MeController', () => {
role: 'global:owner',
});
const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' };
const req = mock<MeRequest.UserUpdate>({ user, body: reqBody });
const req = mock<MeRequest.UserUpdate>({ user, body: reqBody, browserId });
const res = mock<Response>();
userRepository.findOneOrFail.mockResolvedValue(user);
jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token');
@ -88,7 +90,7 @@ describe('MeController', () => {
role: 'global:owner',
});
const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' };
const req = mock<MeRequest.UserUpdate>({ user, body: reqBody });
const req = mock<MeRequest.UserUpdate>({ user, body: reqBody, browserId });
const res = mock<Response>();
userRepository.findOneOrFail.mockResolvedValue(user);
jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token');
@ -160,6 +162,7 @@ describe('MeController', () => {
const req = mock<MeRequest.Password>({
user: mock({ password: passwordHash }),
body: { currentPassword: 'old_password', newPassword },
browserId,
});
await expect(controller.updatePassword(req, mock())).rejects.toThrowError(
new BadRequestError(errorMessage),
@ -172,6 +175,7 @@ describe('MeController', () => {
const req = mock<MeRequest.Password>({
user: mock({ password: passwordHash }),
body: { currentPassword: 'old_password', newPassword: 'NewPassword123' },
browserId,
});
const res = mock<Response>();
userRepository.save.calledWith(req.user).mockResolvedValue(req.user);

View file

@ -82,6 +82,7 @@ describe('OwnerController', () => {
role: 'global:owner',
authIdentities: [],
});
const browserId = 'test-browser-id';
const req = mock<OwnerRequest.Post>({
body: {
email: 'valid@email.com',
@ -90,6 +91,7 @@ describe('OwnerController', () => {
lastName: 'Doe',
},
user,
browserId,
});
const res = mock<Response>();
configGetSpy.mockReturnValue(false);
@ -103,7 +105,7 @@ describe('OwnerController', () => {
where: { role: 'global:owner' },
});
expect(userRepository.save).toHaveBeenCalledWith(user, { transaction: false });
expect(authService.issueCookie).toHaveBeenCalledWith(res, user);
expect(authService.issueCookie).toHaveBeenCalledWith(res, user, browserId);
});
});
});

View file

@ -1,9 +1,16 @@
import type { AxiosRequestConfig, Method } from 'axios';
import type { AxiosRequestConfig, Method, RawAxiosRequestHeaders } from 'axios';
import axios from 'axios';
import type { IDataObject } from 'n8n-workflow';
import type { IExecutionFlattedResponse, IExecutionResponse, IRestApiContext } from '@/Interface';
import { parse } from 'flatted';
const BROWSER_ID_STORAGE_KEY = 'n8n-browserId';
let browserId = localStorage.getItem(BROWSER_ID_STORAGE_KEY);
if (!browserId && 'randomUUID' in crypto) {
browserId = crypto.randomUUID();
localStorage.setItem(BROWSER_ID_STORAGE_KEY, browserId);
}
export const NO_NETWORK_ERROR_CODE = 999;
export class ResponseError extends Error {
@ -62,7 +69,7 @@ export async function request(config: {
method: Method;
baseURL: string;
endpoint: string;
headers?: IDataObject;
headers?: RawAxiosRequestHeaders;
data?: IDataObject | IDataObject[];
withCredentials?: boolean;
}) {
@ -121,11 +128,15 @@ export async function makeRestApiRequest<T>(
endpoint: string,
data?: IDataObject | IDataObject[],
) {
const headers: RawAxiosRequestHeaders = { 'push-ref': context.pushRef };
if (browserId) {
headers['browser-id'] = browserId;
}
const response = await request({
method,
baseURL: context.baseUrl,
endpoint,
headers: { 'push-ref': context.pushRef },
headers,
data,
});
@ -137,7 +148,7 @@ export async function get(
baseURL: string,
endpoint: string,
params?: IDataObject,
headers?: IDataObject,
headers?: RawAxiosRequestHeaders,
) {
return await request({ method: 'GET', baseURL, endpoint, headers, data: params });
}
@ -146,7 +157,7 @@ export async function post(
baseURL: string,
endpoint: string,
params?: IDataObject,
headers?: IDataObject,
headers?: RawAxiosRequestHeaders,
) {
return await request({ method: 'POST', baseURL, endpoint, headers, data: params });
}