feat(core): Logout should invalidate the auth token (no-changelog) (#10335)
Some checks are pending
Test Master / install-and-build (push) Waiting to run
Test Master / Unit tests (18.x) (push) Blocked by required conditions
Test Master / Unit tests (20.x) (push) Blocked by required conditions
Test Master / Unit tests (22.4) (push) Blocked by required conditions
Test Master / Lint (push) Blocked by required conditions
Test Master / Notify Slack on failure (push) Blocked by required conditions

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2024-08-22 09:33:06 +02:00 committed by GitHub
parent b805e8ddb8
commit 9fe6a71690
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 158 additions and 23 deletions

View file

@ -6,6 +6,7 @@ import { AuthService } from '@/auth/auth.service';
import config from '@/config';
import { AUTH_COOKIE_NAME, Time } from '@/constants';
import type { User } from '@db/entities/User';
import type { InvalidAuthTokenRepository } from '@db/repositories/invalidAuthToken.repository';
import type { UserRepository } from '@db/repositories/user.repository';
import { JwtService } from '@/services/jwt.service';
import type { UrlService } from '@/services/url.service';
@ -26,7 +27,15 @@ describe('AuthService', () => {
const jwtService = new JwtService(mock());
const urlService = mock<UrlService>();
const userRepository = mock<UserRepository>();
const authService = new AuthService(mock(), mock(), jwtService, urlService, userRepository);
const invalidAuthTokenRepository = mock<InvalidAuthTokenRepository>();
const authService = new AuthService(
mock(),
mock(),
jwtService,
urlService,
userRepository,
invalidAuthTokenRepository,
);
const now = new Date('2024-02-01T01:23:45.678Z');
jest.useFakeTimers({ now });
@ -70,16 +79,36 @@ describe('AuthService', () => {
it('should 401 if no cookie is set', async () => {
req.cookies[AUTH_COOKIE_NAME] = undefined;
await authService.authMiddleware(req, res, next);
expect(invalidAuthTokenRepository.existsBy).not.toHaveBeenCalled();
expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(401);
});
it('should 401 and clear the cookie if the JWT is expired', async () => {
req.cookies[AUTH_COOKIE_NAME] = validToken;
invalidAuthTokenRepository.existsBy.mockResolvedValue(false);
jest.advanceTimersByTime(365 * Time.days.toMilliseconds);
await authService.authMiddleware(req, res, next);
expect(invalidAuthTokenRepository.existsBy).toHaveBeenCalled();
expect(userRepository.findOne).not.toHaveBeenCalled();
expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(401);
expect(res.clearCookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME);
});
it('should 401 and clear the cookie if the JWT has been invalidated', async () => {
req.cookies[AUTH_COOKIE_NAME] = validToken;
invalidAuthTokenRepository.existsBy.mockResolvedValue(true);
await authService.authMiddleware(req, res, next);
expect(invalidAuthTokenRepository.existsBy).toHaveBeenCalled();
expect(userRepository.findOne).not.toHaveBeenCalled();
expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(401);
expect(res.clearCookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME);
@ -88,9 +117,11 @@ describe('AuthService', () => {
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);
expect(next).toHaveBeenCalled();
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', expect.any(String), {
httpOnly: true,
@ -302,4 +333,21 @@ describe('AuthService', () => {
expect(resolvedUser).toEqual(user);
});
});
describe('invalidateToken', () => {
const req = mock<AuthenticatedRequest>({
cookies: {
[AUTH_COOKIE_NAME]: validToken,
},
});
it('should invalidate the token', async () => {
await authService.invalidateToken(req);
expect(invalidAuthTokenRepository.insert).toHaveBeenCalledWith({
token: validToken,
expiresAt: new Date('2024-02-08T01:23:45.000Z'),
});
});
});
});

View file

@ -6,6 +6,7 @@ import { JsonWebTokenError, TokenExpiredError } from 'jsonwebtoken';
import config from '@/config';
import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES, Time } from '@/constants';
import type { User } from '@db/entities/User';
import { InvalidAuthTokenRepository } from '@db/repositories/invalidAuthToken.repository';
import { UserRepository } from '@db/repositories/user.repository';
import { AuthError } from '@/errors/response-errors/auth.error';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
@ -53,6 +54,7 @@ export class AuthService {
private readonly jwtService: JwtService,
private readonly urlService: UrlService,
private readonly userRepository: UserRepository,
private readonly invalidAuthTokenRepository: InvalidAuthTokenRepository,
) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.authMiddleware = this.authMiddleware.bind(this);
@ -62,6 +64,8 @@ export class AuthService {
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) {
@ -80,6 +84,22 @@ export class AuthService {
res.clearCookie(AUTH_COOKIE_NAME);
}
async invalidateToken(req: AuthenticatedRequest) {
const token = req.cookies[AUTH_COOKIE_NAME];
if (!token) return;
try {
const { exp } = this.jwtService.decode(token);
if (exp) {
await this.invalidAuthTokenRepository.insert({
token,
expiresAt: new Date(exp * 1000),
});
}
} catch (e) {
this.logger.warn('failed to invalidate auth token', { error: (e as Error).message });
}
}
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

View file

@ -2,6 +2,7 @@ import type { Response } from 'express';
import { Container } from 'typedi';
import jwt from 'jsonwebtoken';
import { mock, anyObject } from 'jest-mock-extended';
import type { PublicUser } from '@/Interfaces';
import type { User } from '@db/entities/User';
import { API_KEY_PREFIX, MeController } from '@/controllers/me.controller';
@ -11,14 +12,16 @@ import { UserService } from '@/services/user.service';
import { ExternalHooks } from '@/ExternalHooks';
import { License } from '@/License';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { UserRepository } from '@/databases/repositories/user.repository';
import { EventService } from '@/events/event.service';
import { badPasswords } from '@test/testData';
import { mockInstance } from '@test/mocking';
import { AuthUserRepository } from '@/databases/repositories/authUser.repository';
import { AuthUserRepository } from '@db/repositories/authUser.repository';
import { InvalidAuthTokenRepository } from '@db/repositories/invalidAuthToken.repository';
import { UserRepository } from '@db/repositories/user.repository';
import { MfaService } from '@/Mfa/mfa.service';
import { InvalidMfaCodeError } from '@/errors/response-errors/invalid-mfa-code.error';
import { badPasswords } from '@test/testData';
import { mockInstance } from '@test/mocking';
const browserId = 'test-browser-id';
describe('MeController', () => {
@ -28,6 +31,7 @@ describe('MeController', () => {
const userRepository = mockInstance(UserRepository);
const mockMfaService = mockInstance(MfaService);
mockInstance(AuthUserRepository);
mockInstance(InvalidAuthTokenRepository);
mockInstance(License).isWithinUsersLimit.mockReturnValue(true);
const controller = Container.get(MeController);

View file

@ -1,9 +1,9 @@
import validator from 'validator';
import { Response } from 'express';
import { AuthService } from '@/auth/auth.service';
import { Get, Post, RestController } from '@/decorators';
import { RESPONSE_ERROR_MESSAGES } from '@/constants';
import { Request, Response } from 'express';
import type { User } from '@db/entities/User';
import { AuthenticatedRequest, LoginRequest, UserRequest } from '@/requests';
import type { PublicUser } from '@/Interfaces';
@ -185,7 +185,8 @@ export class AuthController {
/** Log out a user */
@Post('/logout')
logout(_: Request, res: Response) {
async logout(req: AuthenticatedRequest, res: Response) {
await this.authService.invalidateToken(req);
this.authService.clearCookie(res);
return { loggedOut: true };
}

View file

@ -0,0 +1,11 @@
import { Column, Entity, PrimaryColumn } from '@n8n/typeorm';
import { datetimeColumnType } from './AbstractEntity';
@Entity()
export class InvalidAuthToken {
@PrimaryColumn()
token: string;
@Column(datetimeColumnType)
expiresAt: Date;
}

View file

@ -21,6 +21,7 @@ import { ExecutionData } from './ExecutionData';
import { WorkflowHistory } from './WorkflowHistory';
import { Project } from './Project';
import { ProjectRelation } from './ProjectRelation';
import { InvalidAuthToken } from './InvalidAuthToken';
export const entities = {
AuthIdentity,
@ -31,6 +32,7 @@ export const entities = {
ExecutionEntity,
InstalledNodes,
InstalledPackages,
InvalidAuthToken,
Settings,
SharedCredentials,
SharedWorkflow,

View file

@ -0,0 +1,16 @@
import type { MigrationContext, ReversibleMigration } from '@db/types';
const tableName = 'invalid_auth_token';
export class CreateInvalidAuthTokenTable1723627610222 implements ReversibleMigration {
async up({ schemaBuilder: { createTable, column } }: MigrationContext) {
await createTable(tableName).withColumns(
column('token').varchar(512).primary,
column('expiresAt').timestamp().notNull,
);
}
async down({ schemaBuilder: { dropTable } }: MigrationContext) {
await dropTable(tableName);
}
}

View file

@ -59,6 +59,7 @@ import { RemoveNodesAccess1712044305787 } from '../common/1712044305787-RemoveNo
import { MakeExecutionStatusNonNullable1714133768521 } from '../common/1714133768521-MakeExecutionStatusNonNullable';
import { AddActivatedAtUserSetting1717498465931 } from './1717498465931-AddActivatedAtUserSetting';
import { AddConstraintToExecutionMetadata1720101653148 } from '../common/1720101653148-AddConstraintToExecutionMetadata';
import { CreateInvalidAuthTokenTable1723627610222 } from '../common/1723627610222-CreateInvalidAuthTokenTable';
export const mysqlMigrations: Migration[] = [
InitialMigration1588157391238,
@ -121,4 +122,5 @@ export const mysqlMigrations: Migration[] = [
MakeExecutionStatusNonNullable1714133768521,
AddActivatedAtUserSetting1717498465931,
AddConstraintToExecutionMetadata1720101653148,
CreateInvalidAuthTokenTable1723627610222,
];

View file

@ -59,6 +59,7 @@ import { MakeExecutionStatusNonNullable1714133768521 } from '../common/171413376
import { AddActivatedAtUserSetting1717498465931 } from './1717498465931-AddActivatedAtUserSetting';
import { AddConstraintToExecutionMetadata1720101653148 } from '../common/1720101653148-AddConstraintToExecutionMetadata';
import { FixExecutionMetadataSequence1721377157740 } from './1721377157740-FixExecutionMetadataSequence';
import { CreateInvalidAuthTokenTable1723627610222 } from '../common/1723627610222-CreateInvalidAuthTokenTable';
export const postgresMigrations: Migration[] = [
InitialMigration1587669153312,
@ -121,4 +122,5 @@ export const postgresMigrations: Migration[] = [
AddActivatedAtUserSetting1717498465931,
AddConstraintToExecutionMetadata1720101653148,
FixExecutionMetadataSequence1721377157740,
CreateInvalidAuthTokenTable1723627610222,
];

View file

@ -56,6 +56,7 @@ import { RemoveNodesAccess1712044305787 } from '../common/1712044305787-RemoveNo
import { MakeExecutionStatusNonNullable1714133768521 } from '../common/1714133768521-MakeExecutionStatusNonNullable';
import { AddActivatedAtUserSetting1717498465931 } from './1717498465931-AddActivatedAtUserSetting';
import { AddConstraintToExecutionMetadata1720101653148 } from '../common/1720101653148-AddConstraintToExecutionMetadata';
import { CreateInvalidAuthTokenTable1723627610222 } from '../common/1723627610222-CreateInvalidAuthTokenTable';
const sqliteMigrations: Migration[] = [
InitialMigration1588102412422,
@ -115,6 +116,7 @@ const sqliteMigrations: Migration[] = [
MakeExecutionStatusNonNullable1714133768521,
AddActivatedAtUserSetting1717498465931,
AddConstraintToExecutionMetadata1720101653148,
CreateInvalidAuthTokenTable1723627610222,
];
export { sqliteMigrations };

View file

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

View file

@ -23,11 +23,15 @@ export class JwtService {
}
}
public sign(payload: object, options: jwt.SignOptions = {}): string {
sign(payload: object, options: jwt.SignOptions = {}): string {
return jwt.sign(payload, this.jwtSecret, options);
}
public verify<T = JwtPayload>(token: string, options: jwt.VerifyOptions = {}) {
decode(token: string) {
return jwt.decode(token) as JwtPayload;
}
verify<T = JwtPayload>(token: string, options: jwt.VerifyOptions = {}) {
return jwt.verify(token, this.jwtSecret, options) as T;
}
}

View file

@ -386,13 +386,19 @@ describe('GET /resolve-signup-token', () => {
describe('POST /logout', () => {
test('should log user out', async () => {
const owner = await createUser({ role: 'global:owner' });
const ownerAgent = testServer.authAgentFor(owner);
// @ts-expect-error `accessInfo` types are incorrect
const cookie = ownerAgent.jar.getCookie(AUTH_COOKIE_NAME, { path: '/' });
const response = await testServer.authAgentFor(owner).post('/logout');
const response = await ownerAgent.post('/logout');
expect(response.statusCode).toBe(200);
expect(response.body).toEqual(LOGGED_OUT_RESPONSE_BODY);
const authToken = utils.getAuthToken(response);
expect(authToken).toBeUndefined();
ownerAgent.jar.setCookie(`${AUTH_COOKIE_NAME}=${cookie!.value}`);
await ownerAgent.get('/login').expect(401);
});
});

View file

@ -859,3 +859,6 @@ export const INSECURE_CONNECTION_WARNING = `
export const CanvasNodeKey = 'canvasNode' as unknown as InjectionKey<CanvasNodeInjectionData>;
export const CanvasNodeHandleKey =
'canvasNodeHandle' as unknown as InjectionKey<CanvasNodeHandleInjectionData>;
/** Auth */
export const BROWSER_ID_STORAGE_KEY = 'n8n-browserId';

View file

@ -1,6 +1,6 @@
import type { IUpdateUserSettingsReqPayload, UpdateGlobalRolePayload } from '@/api/users';
import * as usersApi from '@/api/users';
import { PERSONALIZATION_MODAL_KEY, STORES, ROLE } from '@/constants';
import { BROWSER_ID_STORAGE_KEY, PERSONALIZATION_MODAL_KEY, STORES, ROLE } from '@/constants';
import type {
Cloud,
IPersonalizationLatestVersion,
@ -180,6 +180,8 @@ export const useUsersStore = defineStore(STORES.USERS, () => {
postHogStore.reset();
uiStore.clearBannerStack();
npsSurveyStore.resetNpsSurveyOnLogOut();
localStorage.removeItem(BROWSER_ID_STORAGE_KEY);
};
const createOwner = async (params: {

View file

@ -1,16 +1,20 @@
import type { AxiosRequestConfig, Method, RawAxiosRequestHeaders } from 'axios';
import axios from 'axios';
import { ApplicationError, jsonParse, type GenericValue, type IDataObject } from 'n8n-workflow';
import type { IExecutionFlattedResponse, IExecutionResponse, IRestApiContext } from '@/Interface';
import { parse } from 'flatted';
import { assert } from '@/utils/assert';
const BROWSER_ID_STORAGE_KEY = 'n8n-browserId';
let browserId = localStorage.getItem(BROWSER_ID_STORAGE_KEY);
if (!browserId && 'randomUUID' in crypto) {
import { BROWSER_ID_STORAGE_KEY } from '@/constants';
import type { IExecutionFlattedResponse, IExecutionResponse, IRestApiContext } from '@/Interface';
const getBrowserId = () => {
let browserId = localStorage.getItem(BROWSER_ID_STORAGE_KEY);
if (!browserId && 'randomUUID' in crypto) {
browserId = crypto.randomUUID();
localStorage.setItem(BROWSER_ID_STORAGE_KEY, browserId);
}
}
return browserId!;
};
export const NO_NETWORK_ERROR_CODE = 999;
export const STREAM_SEPERATOR = '⧉⇋⇋➽⌑⧉§§\n';
@ -82,8 +86,8 @@ export async function request(config: {
baseURL,
headers: headers ?? {},
};
if (baseURL.startsWith('/') && browserId) {
options.headers!['browser-id'] = browserId;
if (baseURL.startsWith('/')) {
options.headers!['browser-id'] = getBrowserId();
}
if (
import.meta.env.NODE_ENV !== 'production' &&
@ -204,11 +208,9 @@ export async function streamRequest<T>(
separator = STREAM_SEPERATOR,
): Promise<void> {
const headers: Record<string, string> = {
'browser-id': getBrowserId(),
'Content-Type': 'application/json',
};
if (browserId) {
headers['browser-id'] = browserId;
}
const assistantRequest: RequestInit = {
headers,
method: 'POST',