From ff9ae549fdc6962e9990987c54804d2570da6a12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 18 Apr 2024 15:53:19 +0200 Subject: [PATCH] fix(core): Improve browserId checks, and add logging (#9161) --- package.json | 1 + packages/@n8n/nodes-langchain/package.json | 2 +- packages/cli/src/auth/auth.service.ts | 20 +++++++---- packages/core/package.json | 2 +- packages/nodes-base/package.json | 2 +- packages/workflow/package.json | 2 +- ...s__express-serve-static-core@4.17.43.patch | 13 ++++++++ pnpm-lock.yaml | 33 ++++++++----------- 8 files changed, 46 insertions(+), 29 deletions(-) create mode 100644 patches/@types__express-serve-static-core@4.17.43.patch diff --git a/package.json b/package.json index da82e5826d389..cf5678c2b7d7c 100644 --- a/package.json +++ b/package.json @@ -94,6 +94,7 @@ "@sentry/cli@2.17.0": "patches/@sentry__cli@2.17.0.patch", "pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch", "pyodide@0.23.4": "patches/pyodide@0.23.4.patch", + "@types/express-serve-static-core@4.17.43": "patches/@types__express-serve-static-core@4.17.43.patch", "@types/ws@8.5.4": "patches/@types__ws@8.5.4.patch", "vite-plugin-checker@0.6.4": "patches/vite-plugin-checker@0.6.4.patch" } diff --git a/packages/@n8n/nodes-langchain/package.json b/packages/@n8n/nodes-langchain/package.json index 8db0154e1023d..d26b968831fe2 100644 --- a/packages/@n8n/nodes-langchain/package.json +++ b/packages/@n8n/nodes-langchain/package.json @@ -120,7 +120,7 @@ "devDependencies": { "@aws-sdk/types": "3.357.0", "@types/basic-auth": "^1.1.3", - "@types/express": "^4.17.6", + "@types/express": "^4.17.21", "@types/html-to-text": "^9.0.1", "@types/json-schema": "^7.0.15", "@types/temp": "^0.9.1", diff --git a/packages/cli/src/auth/auth.service.ts b/packages/cli/src/auth/auth.service.ts index 994e699c0386a..636c5a27abff0 100644 --- a/packages/cli/src/auth/auth.service.ts +++ b/packages/cli/src/auth/auth.service.ts @@ -41,7 +41,7 @@ const skipBrowserIdCheckEndpoints = [ `/${restEndpoint}/push`, // We need to exclude binary-data downloading endpoint because we can't send custom headers on `` tags - `/${restEndpoint}/binary-data`, + `/${restEndpoint}/binary-data/`, // oAuth callback urls aren't called by the frontend. therefore we can't send custom header on these requests `/${restEndpoint}/oauth1-credential/callback`, @@ -131,15 +131,23 @@ 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) || - // If the token was issued for another browser session - (!skipBrowserIdCheckEndpoints.includes(req.baseUrl) && - jwtPayload.browserId && - (!req.browserId || jwtPayload.browserId !== this.hash(req.browserId))) + jwtPayload.hash !== this.createJWTHash(user) ) { throw new AuthError('Unauthorized'); } + // Check if the token was issued for another browser session, ignoring the endpoints that can't send custom headers + const endpoint = req.route ? `${req.baseUrl}${req.route.path}` : req.baseUrl; + if (req.method === 'GET' && skipBrowserIdCheckEndpoints.includes(endpoint)) { + this.logger.debug(`Skipped browserId check on ${endpoint}`); + } else if ( + jwtPayload.browserId && + (!req.browserId || jwtPayload.browserId !== this.hash(req.browserId)) + ) { + this.logger.warn(`browserId check failed on ${endpoint}`); + 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, jwtPayload.browserId); diff --git a/packages/core/package.json b/packages/core/package.json index f5fd333952689..373affbb1bc7b 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -37,7 +37,7 @@ "@types/aws4": "^1.5.1", "@types/concat-stream": "^2.0.0", "@types/cron": "~1.7.1", - "@types/express": "^4.17.6", + "@types/express": "^4.17.21", "@types/lodash": "^4.14.195", "@types/mime-types": "^2.1.0", "@types/uuid": "^8.3.2", diff --git a/packages/nodes-base/package.json b/packages/nodes-base/package.json index e3eb68f7a39d0..403fa4b5e6389 100644 --- a/packages/nodes-base/package.json +++ b/packages/nodes-base/package.json @@ -812,7 +812,7 @@ "@types/cheerio": "^0.22.15", "@types/cron": "~1.7.1", "@types/eventsource": "^1.1.2", - "@types/express": "^4.17.6", + "@types/express": "^4.17.21", "@types/html-to-text": "^9.0.1", "@types/gm": "^1.25.0", "@types/js-nacl": "^1.3.0", diff --git a/packages/workflow/package.json b/packages/workflow/package.json index 7e0ec566f1bc9..699b0c11ba5f8 100644 --- a/packages/workflow/package.json +++ b/packages/workflow/package.json @@ -40,7 +40,7 @@ ], "devDependencies": { "@types/deep-equal": "^1.0.1", - "@types/express": "^4.17.6", + "@types/express": "^4.17.21", "@types/jmespath": "^0.15.0", "@types/lodash": "^4.14.195", "@types/luxon": "^3.2.0", diff --git a/patches/@types__express-serve-static-core@4.17.43.patch b/patches/@types__express-serve-static-core@4.17.43.patch new file mode 100644 index 0000000000000..05882f0b97f5d --- /dev/null +++ b/patches/@types__express-serve-static-core@4.17.43.patch @@ -0,0 +1,13 @@ +diff --git a/index.d.ts b/index.d.ts +index 5cc36f5760c806a76ee839bfb67c419c9cb48901..8ef0bf74f0f31741b564fe37f040144526e98eb5 100644 +--- a/index.d.ts ++++ b/index.d.ts +@@ -646,7 +646,7 @@ export interface Request< + + query: ReqQuery; + +- route: any; ++ route?: Pick; + + signedCookies: any; + diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 73904efe1805e..cd6d2a9438877 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -23,6 +23,9 @@ patchedDependencies: '@sentry/cli@2.17.0': hash: nchnoezkq6p37qaiku3vrpwraq path: patches/@sentry__cli@2.17.0.patch + '@types/express-serve-static-core@4.17.43': + hash: 5orrj4qleu2iko5t27vl44u4we + path: patches/@types__express-serve-static-core@4.17.43.patch '@types/ws@8.5.4': hash: nbzuqaoyqbrfwipijj5qriqqju path: patches/@types__ws@8.5.4.patch @@ -351,8 +354,8 @@ importers: specifier: ^1.1.3 version: 1.1.3 '@types/express': - specifier: ^4.17.6 - version: 4.17.14 + specifier: ^4.17.21 + version: 4.17.21 '@types/html-to-text': specifier: ^9.0.1 version: 9.0.4 @@ -907,8 +910,8 @@ importers: specifier: ~1.7.1 version: 1.7.3 '@types/express': - specifier: ^4.17.6 - version: 4.17.14 + specifier: ^4.17.21 + version: 4.17.21 '@types/lodash': specifier: ^4.14.195 version: 4.14.195 @@ -1481,8 +1484,8 @@ importers: specifier: ^1.1.2 version: 1.1.9 '@types/express': - specifier: ^4.17.6 - version: 4.17.14 + specifier: ^4.17.21 + version: 4.17.21 '@types/gm': specifier: ^1.25.0 version: 1.25.0 @@ -1614,8 +1617,8 @@ importers: specifier: ^1.0.1 version: 1.0.1 '@types/express': - specifier: ^4.17.6 - version: 4.17.14 + specifier: ^4.17.21 + version: 4.17.21 '@types/jmespath': specifier: ^0.15.0 version: 0.15.0 @@ -9636,28 +9639,20 @@ packages: resolution: {integrity: sha512-F3K4oyM12o8W9jxuJmW+1sc8kdw0Hj0t+26urwkcolPJTgkfppEfIdftdcXmUU2QPBIwcrYO6diqgIqgCDf1FA==} dev: true - /@types/express-serve-static-core@4.17.43: + /@types/express-serve-static-core@4.17.43(patch_hash=5orrj4qleu2iko5t27vl44u4we): resolution: {integrity: sha512-oaYtiBirUOPQGSWNGPWnzyAFJ0BP3cwvN4oWZQY+zUBwpVIGsKUkpBpSztp74drYcjavs7SKFZ4DX1V2QeN8rg==} dependencies: '@types/node': 18.16.16 '@types/qs': 6.9.7 '@types/range-parser': 1.2.4 '@types/send': 0.17.4 - - /@types/express@4.17.14: - resolution: {integrity: sha512-TEbt+vaPFQ+xpxFLFssxUDXj5cWCxZJjIcB7Yg0k0GMHGtgtQgpvx/MUQUeAkNbA9AAGrwkAsoeItdTgS7FMyg==} - dependencies: - '@types/body-parser': 1.19.2 - '@types/express-serve-static-core': 4.17.43 - '@types/qs': 6.9.7 - '@types/serve-static': 1.15.0 - dev: true + patched: true /@types/express@4.17.21: resolution: {integrity: sha512-ejlPM315qwLpaQlQDTjPdsUFSc6ZsP4AN6AlWnogPjQ7CVi7PYF3YVz+CY3jE2pwYf7E/7HlDAN0rV2GxTG0HQ==} dependencies: '@types/body-parser': 1.19.2 - '@types/express-serve-static-core': 4.17.43 + '@types/express-serve-static-core': 4.17.43(patch_hash=5orrj4qleu2iko5t27vl44u4we) '@types/qs': 6.9.7 '@types/serve-static': 1.15.0