From 52f9f6ebb6da6e3b56578e4ea17379b6d0f6645e Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Sat, 22 Apr 2023 00:06:54 +0200 Subject: [PATCH] Backport/patch (#844) * backport: session regenerate * backport: session fixation refs: https://github.com/fastify/fastify-passport/commit/43c82c321db58ea3e375dd475de60befbfcf2a11 * fix: remove passport Co-authored-by: Rafael Gonzaga * fix: remove passport from clearSessionIgnoreFields Co-authored-by: Rafael Gonzaga --------- Co-authored-by: Rafael Gonzaga --- .eslintrc.js | 1 + README.md | 5 + package-lock.json | 210 +++++++++++- package.json | 5 +- src/Authenticator.ts | 15 +- src/session-managers/SecureSessionManager.ts | 63 +++- test/csrf-fixation.test.ts | 62 ++++ test/helpers.ts | 17 +- test/independent-strategy-instances.test.ts | 2 +- test/multi-instance.test.ts | 328 +++++++++++++++---- test/passport.test.ts | 20 +- test/session-isolation.test.ts | 79 +++-- 12 files changed, 665 insertions(+), 142 deletions(-) create mode 100644 test/csrf-fixation.test.ts diff --git a/.eslintrc.js b/.eslintrc.js index fdb76e70..bf611f09 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -38,5 +38,6 @@ module.exports = { '@typescript-eslint/restrict-plus-operands': 'off', '@typescript-eslint/restrict-template-expressions': 'off', '@typescript-eslint/unbound-method': 'off', + '@typescript-eslint/ban-ts-comment': ['error', { 'ts-ignore': 'allow-with-description' }], }, } diff --git a/README.md b/README.md index 047adaa6..b7c5d226 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,11 @@ server.register(fastifyPassport.secureSession()) fastifyPassport.use('test', new SomePassportStrategy()) // you'd probably use some passport strategy from npm here ``` +## Session cleanup on logIn + +For security reasons the session is cleaned after login. You can manage this configuration at your own risk by using +`clearSessionOnLogin (default: true)` and `clearSessionIgnoreFields (default: ['session'])` + ## Difference between `@fastify/secure-session` and `@fastify/session` `@fastify/secure-session` and `@fastify/session` are both session plugins for Fastify which are capable of encrypting/decrypting the session. The main difference is that `@fastify/secure-session` uses the stateless approach and stores the whole session in an encrypted cookie whereas `@fastify/session` uses the stateful approach for sessions and stores them in a session store. diff --git a/package-lock.json b/package-lock.json index 839eed7c..922d74b9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "@fastify/passport", - "version": "1.0.0", + "version": "1.0.1", "license": "MIT", "dependencies": { "@fastify/flash": "^4.0.0", @@ -14,8 +14,9 @@ }, "devDependencies": { "@fastify/cookie": "^6.0.0", - "@fastify/secure-session": "^4.0.0", - "@fastify/session": "^8.0.0", + "@fastify/csrf-protection": "^4.1.0", + "@fastify/secure-session": "^4.1.0", + "@fastify/session": "^8.2.0", "@types/jest": "^27.0.0", "@types/node": "^17.0.0", "@types/passport": "^1.0.5", @@ -716,6 +717,31 @@ "fastify-plugin": "^3.0.1" } }, + "node_modules/@fastify/csrf": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/@fastify/csrf/-/csrf-4.0.1.tgz", + "integrity": "sha512-LkMtGoj0PhnkcbB8W/oWkUKciDY8dnOYEhDUh3h8TmbzhuYd+PtVTMZjM1DcAaSNxLYTjZqe2aX6Rn7tXJO9XA==", + "dev": true, + "dependencies": { + "rndm": "^1.2.0", + "tsscmp": "^1.0.6", + "uid-safe": "^2.1.5" + }, + "engines": { + "node": ">= 10.0" + } + }, + "node_modules/@fastify/csrf-protection": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/@fastify/csrf-protection/-/csrf-protection-4.1.0.tgz", + "integrity": "sha512-OUUai2WHPfWd9TXKwKSqfYCJF/kM6WnaBqg7Td2OK4byWB4aPhCGFnavCuFK2WEiwbviElUEYb2IEsHYPpslkg==", + "dev": true, + "dependencies": { + "@fastify/csrf": "^4.0.0", + "fastify-plugin": "^3.0.0", + "http-errors": "^2.0.0" + } + }, "node_modules/@fastify/flash": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/@fastify/flash/-/flash-4.0.0.tgz", @@ -728,9 +754,9 @@ } }, "node_modules/@fastify/secure-session": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/@fastify/secure-session/-/secure-session-4.0.0.tgz", - "integrity": "sha512-kx2wuocE/EG1mOCxGJHhZLv3oltPvFeZcAlDtfQzAV1vac4XvNNYMhu4xbPwbTTCZFdkZ5E2QNUaMSU3WP5t7g==", + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/@fastify/secure-session/-/secure-session-4.1.1.tgz", + "integrity": "sha512-StnjIfENCGX6AkI2unIb2IlYlQSWujv6S7yB3jVG6JhPPxdN/8KGSTscw4vRooj1uAu8lyZpVZbh9cr/Kca2bA==", "dev": true, "dependencies": { "@fastify/cookie": "^6.0.0", @@ -738,17 +764,18 @@ "sodium-native": "^3.0.0" }, "bin": { - "fastify-secure-session": "genkey.js" + "secure-session": "genkey.js" } }, "node_modules/@fastify/session": { - "version": "8.0.0", - "resolved": "https://registry.npmjs.org/@fastify/session/-/session-8.0.0.tgz", - "integrity": "sha512-f8LbBnEu2YOGncnfUbzHe/HaaI6QekfLJV6SbiTHkbUVPsXqbFnHUdV/YnmRCnYfv4oNsSCe8GleGR1E4YoF3g==", + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/@fastify/session/-/session-8.3.0.tgz", + "integrity": "sha512-TJAbzJ7yswUR/YcSM5v3+scqL0tOMk/Yhbwbt3Z7ZK/o74GaIApTiQPSX0ZTEvzme0IBqdE9djZlWcJmcoDrYw==", "dev": true, "dependencies": { "cookie-signature": "^1.1.0", "fastify-plugin": "^3.0.0", + "safe-stable-stringify": "^2.3.1", "uid-safe": "^2.1.5" }, "engines": { @@ -2388,6 +2415,15 @@ "node": ">=0.4.0" } }, + "node_modules/depd": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/depd/-/depd-2.0.0.tgz", + "integrity": "sha512-g7nH6P6dyDioJogAAGprGpCtVImJhpPk/roCzdb3fIh61/s/nPsfR6onyMwkCAR/OlC3yBC0lESvUoQEAssIrw==", + "dev": true, + "engines": { + "node": ">= 0.8" + } + }, "node_modules/detect-newline": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/detect-newline/-/detect-newline-3.1.0.tgz", @@ -3409,6 +3445,22 @@ "integrity": "sha512-carPklcUh7ROWRK7Cv27RPtdhYhUsela/ue5/jKzjegVvXDqM2ILE9Q2BGn9JZJh1g87cp56su/FgQSzcWS8cQ==", "dev": true }, + "node_modules/http-errors": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-2.0.0.tgz", + "integrity": "sha512-FtwrG/euBzaEjYeRqOgly7G0qviiXoJWnvEH2Z1plBdXgbyjv34pHTSb9zoeHMyDy33+DWy5Wt9Wo+TURtOYSQ==", + "dev": true, + "dependencies": { + "depd": "2.0.0", + "inherits": "2.0.4", + "setprototypeof": "1.2.0", + "statuses": "2.0.1", + "toidentifier": "1.0.1" + }, + "engines": { + "node": ">= 0.8" + } + }, "node_modules/http-proxy-agent": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-4.0.1.tgz", @@ -5407,6 +5459,12 @@ "url": "https://github.com/sponsors/isaacs" } }, + "node_modules/rndm": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/rndm/-/rndm-1.2.0.tgz", + "integrity": "sha512-fJhQQI5tLrQvYIYFpOnFinzv9dwmR7hRnUz1XqP3OJ1jIweTNOd6aTO4jwQSgcBSFUB+/KHJxuGneime+FdzOw==", + "dev": true + }, "node_modules/run-parallel": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/run-parallel/-/run-parallel-1.2.0.tgz", @@ -5459,6 +5517,15 @@ "ret": "~0.2.0" } }, + "node_modules/safe-stable-stringify": { + "version": "2.4.3", + "resolved": "https://registry.npmjs.org/safe-stable-stringify/-/safe-stable-stringify-2.4.3.tgz", + "integrity": "sha512-e2bDA2WJT0wxseVd4lsDP4+3ONX6HpMXQa1ZhFQ7SU+GjvORCmShbCMltrtIDfkYhVHrOcPtj+KhmDBdPdZD1g==", + "dev": true, + "engines": { + "node": ">=10" + } + }, "node_modules/safer-buffer": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", @@ -5510,6 +5577,12 @@ "integrity": "sha512-edRH8mBKEWNVIVMKejNnuJxleqYE/ZSdcT8/Nem9/mmosx12pctd80s2Oy00KNZzrogMZS5mauK2/ymL1bvlvg==", "dev": true }, + "node_modules/setprototypeof": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/setprototypeof/-/setprototypeof-1.2.0.tgz", + "integrity": "sha512-E5LDX7Wrp85Kil5bhZv46j8jOeboKq5JMmYM3gVGdGH8xFpPWXUMsNrlODCrkoxMEeNi/XZIwuRvY4XNwYMJpw==", + "dev": true + }, "node_modules/shebang-command": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/shebang-command/-/shebang-command-2.0.0.tgz", @@ -5610,6 +5683,15 @@ "node": ">=10" } }, + "node_modules/statuses": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", + "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", + "dev": true, + "engines": { + "node": ">= 0.8" + } + }, "node_modules/string_decoder": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.3.0.tgz", @@ -5815,6 +5897,15 @@ "node": ">=8.0" } }, + "node_modules/toidentifier": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/toidentifier/-/toidentifier-1.0.1.tgz", + "integrity": "sha512-o5sSPKEkg/DIQNmH43V0/uerLrpzVedkUh8tGNvaeXpfpuwjKenlSox/2O/BTlZUtEe+JG7s5YhEz608PlAHRA==", + "dev": true, + "engines": { + "node": ">=0.6" + } + }, "node_modules/tough-cookie": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/tough-cookie/-/tough-cookie-4.0.0.tgz", @@ -5890,6 +5981,15 @@ "integrity": "sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==", "dev": true }, + "node_modules/tsscmp": { + "version": "1.0.6", + "resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.6.tgz", + "integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA==", + "dev": true, + "engines": { + "node": ">=0.6.x" + } + }, "node_modules/tsutils": { "version": "3.21.0", "resolved": "https://registry.npmjs.org/tsutils/-/tsutils-3.21.0.tgz", @@ -6763,6 +6863,28 @@ "fastify-plugin": "^3.0.1" } }, + "@fastify/csrf": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/@fastify/csrf/-/csrf-4.0.1.tgz", + "integrity": "sha512-LkMtGoj0PhnkcbB8W/oWkUKciDY8dnOYEhDUh3h8TmbzhuYd+PtVTMZjM1DcAaSNxLYTjZqe2aX6Rn7tXJO9XA==", + "dev": true, + "requires": { + "rndm": "^1.2.0", + "tsscmp": "^1.0.6", + "uid-safe": "^2.1.5" + } + }, + "@fastify/csrf-protection": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/@fastify/csrf-protection/-/csrf-protection-4.1.0.tgz", + "integrity": "sha512-OUUai2WHPfWd9TXKwKSqfYCJF/kM6WnaBqg7Td2OK4byWB4aPhCGFnavCuFK2WEiwbviElUEYb2IEsHYPpslkg==", + "dev": true, + "requires": { + "@fastify/csrf": "^4.0.0", + "fastify-plugin": "^3.0.0", + "http-errors": "^2.0.0" + } + }, "@fastify/flash": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/@fastify/flash/-/flash-4.0.0.tgz", @@ -6772,9 +6894,9 @@ } }, "@fastify/secure-session": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/@fastify/secure-session/-/secure-session-4.0.0.tgz", - "integrity": "sha512-kx2wuocE/EG1mOCxGJHhZLv3oltPvFeZcAlDtfQzAV1vac4XvNNYMhu4xbPwbTTCZFdkZ5E2QNUaMSU3WP5t7g==", + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/@fastify/secure-session/-/secure-session-4.1.1.tgz", + "integrity": "sha512-StnjIfENCGX6AkI2unIb2IlYlQSWujv6S7yB3jVG6JhPPxdN/8KGSTscw4vRooj1uAu8lyZpVZbh9cr/Kca2bA==", "dev": true, "requires": { "@fastify/cookie": "^6.0.0", @@ -6783,13 +6905,14 @@ } }, "@fastify/session": { - "version": "8.0.0", - "resolved": "https://registry.npmjs.org/@fastify/session/-/session-8.0.0.tgz", - "integrity": "sha512-f8LbBnEu2YOGncnfUbzHe/HaaI6QekfLJV6SbiTHkbUVPsXqbFnHUdV/YnmRCnYfv4oNsSCe8GleGR1E4YoF3g==", + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/@fastify/session/-/session-8.3.0.tgz", + "integrity": "sha512-TJAbzJ7yswUR/YcSM5v3+scqL0tOMk/Yhbwbt3Z7ZK/o74GaIApTiQPSX0ZTEvzme0IBqdE9djZlWcJmcoDrYw==", "dev": true, "requires": { "cookie-signature": "^1.1.0", "fastify-plugin": "^3.0.0", + "safe-stable-stringify": "^2.3.1", "uid-safe": "^2.1.5" } }, @@ -8063,6 +8186,12 @@ "integrity": "sha1-3zrhmayt+31ECqrgsp4icrJOxhk=", "dev": true }, + "depd": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/depd/-/depd-2.0.0.tgz", + "integrity": "sha512-g7nH6P6dyDioJogAAGprGpCtVImJhpPk/roCzdb3fIh61/s/nPsfR6onyMwkCAR/OlC3yBC0lESvUoQEAssIrw==", + "dev": true + }, "detect-newline": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/detect-newline/-/detect-newline-3.1.0.tgz", @@ -8841,6 +8970,19 @@ "integrity": "sha512-carPklcUh7ROWRK7Cv27RPtdhYhUsela/ue5/jKzjegVvXDqM2ILE9Q2BGn9JZJh1g87cp56su/FgQSzcWS8cQ==", "dev": true }, + "http-errors": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-2.0.0.tgz", + "integrity": "sha512-FtwrG/euBzaEjYeRqOgly7G0qviiXoJWnvEH2Z1plBdXgbyjv34pHTSb9zoeHMyDy33+DWy5Wt9Wo+TURtOYSQ==", + "dev": true, + "requires": { + "depd": "2.0.0", + "inherits": "2.0.4", + "setprototypeof": "1.2.0", + "statuses": "2.0.1", + "toidentifier": "1.0.1" + } + }, "http-proxy-agent": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-4.0.1.tgz", @@ -10357,6 +10499,12 @@ "glob": "^7.1.3" } }, + "rndm": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/rndm/-/rndm-1.2.0.tgz", + "integrity": "sha512-fJhQQI5tLrQvYIYFpOnFinzv9dwmR7hRnUz1XqP3OJ1jIweTNOd6aTO4jwQSgcBSFUB+/KHJxuGneime+FdzOw==", + "dev": true + }, "run-parallel": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/run-parallel/-/run-parallel-1.2.0.tgz", @@ -10381,6 +10529,12 @@ "ret": "~0.2.0" } }, + "safe-stable-stringify": { + "version": "2.4.3", + "resolved": "https://registry.npmjs.org/safe-stable-stringify/-/safe-stable-stringify-2.4.3.tgz", + "integrity": "sha512-e2bDA2WJT0wxseVd4lsDP4+3ONX6HpMXQa1ZhFQ7SU+GjvORCmShbCMltrtIDfkYhVHrOcPtj+KhmDBdPdZD1g==", + "dev": true + }, "safer-buffer": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/safer-buffer/-/safer-buffer-2.1.2.tgz", @@ -10423,6 +10577,12 @@ "integrity": "sha512-edRH8mBKEWNVIVMKejNnuJxleqYE/ZSdcT8/Nem9/mmosx12pctd80s2Oy00KNZzrogMZS5mauK2/ymL1bvlvg==", "dev": true }, + "setprototypeof": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/setprototypeof/-/setprototypeof-1.2.0.tgz", + "integrity": "sha512-E5LDX7Wrp85Kil5bhZv46j8jOeboKq5JMmYM3gVGdGH8xFpPWXUMsNrlODCrkoxMEeNi/XZIwuRvY4XNwYMJpw==", + "dev": true + }, "shebang-command": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/shebang-command/-/shebang-command-2.0.0.tgz", @@ -10507,6 +10667,12 @@ "escape-string-regexp": "^2.0.0" } }, + "statuses": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", + "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", + "dev": true + }, "string_decoder": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.3.0.tgz", @@ -10661,6 +10827,12 @@ "is-number": "^7.0.0" } }, + "toidentifier": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/toidentifier/-/toidentifier-1.0.1.tgz", + "integrity": "sha512-o5sSPKEkg/DIQNmH43V0/uerLrpzVedkUh8tGNvaeXpfpuwjKenlSox/2O/BTlZUtEe+JG7s5YhEz608PlAHRA==", + "dev": true + }, "tough-cookie": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/tough-cookie/-/tough-cookie-4.0.0.tgz", @@ -10703,6 +10875,12 @@ "integrity": "sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==", "dev": true }, + "tsscmp": { + "version": "1.0.6", + "resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.6.tgz", + "integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA==", + "dev": true + }, "tsutils": { "version": "3.21.0", "resolved": "https://registry.npmjs.org/tsutils/-/tsutils-3.21.0.tgz", diff --git a/package.json b/package.json index 043cb738..4d4cb256 100644 --- a/package.json +++ b/package.json @@ -48,8 +48,9 @@ }, "devDependencies": { "@fastify/cookie": "^6.0.0", - "@fastify/secure-session": "^4.0.0", - "@fastify/session": "^8.0.0", + "@fastify/csrf-protection": "^4.1.0", + "@fastify/secure-session": "^4.1.0", + "@fastify/session": "^8.2.0", "@types/jest": "^27.0.0", "@types/node": "^17.0.0", "@types/passport": "^1.0.5", diff --git a/src/Authenticator.ts b/src/Authenticator.ts index eceed1b8..d906e53e 100644 --- a/src/Authenticator.ts +++ b/src/Authenticator.ts @@ -20,6 +20,8 @@ export type InfoTransformerFunction = (info: any) => Promise export interface AuthenticatorOptions { key?: string userProperty?: string + clearSessionOnLogin?: boolean + clearSessionIgnoreFields?: string[] } export class Authenticator { @@ -33,13 +35,24 @@ export class Authenticator { private serializers: SerializeFunction[] = [] private deserializers: DeserializeFunction[] = [] private infoTransformers: InfoTransformerFunction[] = [] + private clearSessionOnLogin: boolean + private clearSessionIgnoreFields: string[] constructor(options: AuthenticatorOptions = {}) { this.key = options.key || 'passport' this.userProperty = options.userProperty || 'user' this.use(new SessionStrategy(this.deserializeUser.bind(this))) - this.sessionManager = new SecureSessionManager({ key: this.key }, this.serializeUser.bind(this)) + this.clearSessionOnLogin = options.clearSessionOnLogin ?? true + this.clearSessionIgnoreFields = ['session', ...(options.clearSessionIgnoreFields || [])] + this.sessionManager = new SecureSessionManager( + { + key: this.key, + clearSessionOnLogin: this.clearSessionOnLogin, + clearSessionIgnoreFields: this.clearSessionIgnoreFields, + }, + this.serializeUser.bind(this) + ) } use(strategy: AnyStrategy): this diff --git a/src/session-managers/SecureSessionManager.ts b/src/session-managers/SecureSessionManager.ts index a7a3173e..3b9dc4bc 100644 --- a/src/session-managers/SecureSessionManager.ts +++ b/src/session-managers/SecureSessionManager.ts @@ -2,29 +2,78 @@ import { FastifyRequest } from 'fastify' import { SerializeFunction } from '../Authenticator' +// '@ts-ignore' prevent build errors caused by types mismatch from @fastify/session and @fastify/secure-session /** Class for storing passport data in the session using `fastify-secure-session` or `@fastify/session` */ export class SecureSessionManager { key: string + clearSessionOnLogin: boolean + clearSessionIgnoreFields: string[] = ['session'] serializeUser: SerializeFunction - constructor(options: SerializeFunction | any, serializeUser?: SerializeFunction) { + constructor(serializeUser: SerializeFunction) + constructor( + options: { key?: string; clearSessionOnLogin?: boolean; clearSessionIgnoreFields?: string[] }, + serializeUser: SerializeFunction + ) + constructor( + options: SerializeFunction | { key?: string; clearSessionOnLogin?: boolean; clearSessionIgnoreFields?: string[] }, + serializeUser?: SerializeFunction + ) { if (typeof options === 'function') { - serializeUser = options - options = undefined + this.serializeUser = options + this.key = 'passport' + this.clearSessionOnLogin = true + } else if (typeof serializeUser === 'function') { + this.serializeUser = serializeUser + this.key = + (options && typeof options === 'object' && typeof options.key === 'string' && options.key) || 'passport' + this.clearSessionOnLogin = options.clearSessionOnLogin ?? true + this.clearSessionIgnoreFields = [...this.clearSessionIgnoreFields, ...(options.clearSessionIgnoreFields || [])] + } else { + throw new Error('SecureSessionManager#constructor must have a valid serializeUser-function passed as a parameter') } - options = options || {} - - this.key = options.key || 'passport' - this.serializeUser = serializeUser! } async logIn(request: FastifyRequest, user: any) { const object = await this.serializeUser(user, request) + if (this.clearSessionOnLogin && object) { + // Handle @fastify/session to prevent token/CSRF fixation + // @ts-ignore: property 'regenerate' does not exist on type 'Session'. + if (request.session.regenerate) { + // @ts-ignore: property 'regenerate' does not exist on type 'Session'. + await request.session.regenerate(this.clearSessionIgnoreFields) + } else { + // @ts-ignore: property 'data' does not exist on type 'Session' + const currentFields = request.session.data() || {} + // Handle @fastify/secure-session against CSRF fixation + // TODO: This is quite hacky. The best option would be having a regenerate method + // on secure-session as well + for (const field of Object.keys(currentFields as object)) { + if (this.clearSessionIgnoreFields.includes(field)) { + continue + } + request.session.set(field, undefined) + } + } + } + + // Handle sessions using @fastify/session + // @ts-ignore: property 'regenerate' does not exist on type 'Session'. + if (request.session.regenerate) { + // regenerate session to guard against session fixation + // @ts-ignore: Property 'regenerate' does not exist on type 'Session'. + await request.session.regenerate() + } request.session.set(this.key, object) } async logOut(request: FastifyRequest) { request.session.set(this.key, undefined) + // @ts-ignore: property 'regenerate' does not exist on type 'Session'. + if (request.session.regenerate) { + // @ts-ignore: property 'regenerate' does not exist on type 'Session'. + await request.session.regenerate() + } } getUserFromSession(request: FastifyRequest) { diff --git a/test/csrf-fixation.test.ts b/test/csrf-fixation.test.ts new file mode 100644 index 00000000..7b3a72b9 --- /dev/null +++ b/test/csrf-fixation.test.ts @@ -0,0 +1,62 @@ +/* eslint-disable @typescript-eslint/no-empty-function */ +import { getConfiguredTestServer, TestBrowserSession } from './helpers' +import fastifyCsrfProtection from '@fastify/csrf-protection' + +function createServer(sessionPluginName: '@fastify/session' | '@fastify/secure-session') { + const { server, fastifyPassport } = getConfiguredTestServer() + + void server.register(fastifyCsrfProtection, { sessionPlugin: sessionPluginName, csrfOpts: { hmacKey: 'test' } }) + + server.post( + '/login', + { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, + async () => 'success' + ) + + server.get('/csrf', async (_req, reply) => { + return reply.generateCsrf() + }) + server.get('/session', async (req) => { + return req.session.get('_csrf') || '' + }) + return server +} + +const suite = (sessionPluginName: '@fastify/session' | '@fastify/secure-session') => { + process.env.SESSION_PLUGIN = sessionPluginName + const server = createServer(sessionPluginName) + describe(`${sessionPluginName} tests`, () => { + describe('guard against fixation', () => { + let user: TestBrowserSession + + beforeEach(() => { + user = new TestBrowserSession(server) + }) + + test(`should renegerate csrf token on login`, async () => { + { + const sess = await user.inject({ method: 'GET', url: '/session' }) + expect(sess.body).toBe('') + } + await user.inject({ method: 'GET', url: '/csrf' }) + { + const sess = await user.inject({ method: 'GET', url: '/session' }) + expect(sess.body).not.toBe('') + } + await user.inject({ + method: 'POST', + url: '/login', + payload: { login: 'test', password: 'test' }, + }) + { + const sess = await user.inject({ method: 'GET', url: '/session' }) + expect(sess.body).toBe('') + } + }) + }) + }) + delete process.env.SESSION_PLUGIN +} + +suite('@fastify/session') +suite('@fastify/secure-session') diff --git a/test/helpers.ts b/test/helpers.ts index ae63b8aa..8ee8dce4 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -3,7 +3,7 @@ import fastify, { FastifyInstance } from 'fastify' import fastifySecureSession, { SecureSessionPluginOptions } from '@fastify/secure-session' import fastifyCookie from '@fastify/cookie' import fastifySession from '@fastify/session' -import Authenticator from '../src/Authenticator' +import Authenticator, { AuthenticatorOptions } from '../src/Authenticator' import { Strategy } from '../src/strategies' import { InjectOptions, Response as LightMyRequestResponse } from 'light-my-request' import * as parseCookies from 'set-cookie-parser' @@ -98,8 +98,7 @@ export const getTestServer = (sessionOptions: SessionOptions = null) => { const server = fastify() loadSessionPlugins(server, sessionOptions) - server.setErrorHandler((error, request, reply) => { - console.error(error) + server.setErrorHandler((error, _request, reply) => { void reply.status(500) void reply.send(error) }) @@ -107,8 +106,11 @@ export const getTestServer = (sessionOptions: SessionOptions = null) => { } /** Create a fastify instance with fastify-passport plugin registered but with no strategies registered yet. */ -export const getRegisteredTestServer = (sessionOptions: SessionOptions = null) => { - const fastifyPassport = new Authenticator() +export const getRegisteredTestServer = ( + sessionOptions: SessionOptions = null, + passportOptions: AuthenticatorOptions = {} +) => { + const fastifyPassport = new Authenticator(passportOptions) fastifyPassport.registerUserSerializer(async (user) => JSON.stringify(user)) fastifyPassport.registerUserDeserializer(async (serialized: string) => JSON.parse(serialized)) @@ -123,9 +125,10 @@ export const getRegisteredTestServer = (sessionOptions: SessionOptions = null) = export const getConfiguredTestServer = ( name = 'test', strategy = new TestStrategy('test'), - sessionOptions: SessionOptions = null + sessionOptions: SessionOptions = null, + passportOptions: AuthenticatorOptions = {} ) => { - const { fastifyPassport, server } = getRegisteredTestServer(sessionOptions) + const { fastifyPassport, server } = getRegisteredTestServer(sessionOptions, passportOptions) fastifyPassport.use(name, strategy) return { fastifyPassport, server } } diff --git a/test/independent-strategy-instances.test.ts b/test/independent-strategy-instances.test.ts index a6d9f2a9..62d83f70 100644 --- a/test/independent-strategy-instances.test.ts +++ b/test/independent-strategy-instances.test.ts @@ -18,7 +18,7 @@ class WelcomeStrategy extends Strategy { const suite = (sessionPluginName) => { describe(`${sessionPluginName} tests`, () => { test(`should allow passing a specific Strategy instance to an authenticate call`, async () => { - const { server, fastifyPassport } = getRegisteredTestServer() + const { server, fastifyPassport } = getRegisteredTestServer(null, { clearSessionIgnoreFields: ['messages'] }) server.get( '/', { diff --git a/test/multi-instance.test.ts b/test/multi-instance.test.ts index e94b00c9..4f92f3cc 100644 --- a/test/multi-instance.test.ts +++ b/test/multi-instance.test.ts @@ -3,89 +3,96 @@ import { getTestServer, TestBrowserSession } from './helpers' import { Authenticator } from '../src/Authenticator' import { Strategy } from '../src/strategies' +let counter: number +let authenticators: Record + +async function TestStrategyModule(instance: FastifyInstance, { namespace, clearSessionOnLogin }) { + class TestStrategy extends Strategy { + authenticate(request: any, _options?: { pauseStream?: boolean }) { + if (request.isAuthenticated()) { + return this.pass() + } + if (request.body && request.body.login === 'test' && request.body.password === 'test') { + return this.success({ namespace, id: String(counter++) }) + } + + this.fail() + } + } + + const strategyName = `test-${namespace}` + const authenticator = new Authenticator({ + key: `passport${namespace}`, + userProperty: `user${namespace}`, + clearSessionOnLogin, + }) + authenticator.use(strategyName, new TestStrategy(strategyName)) + authenticator.registerUserSerializer(async (user) => { + if (user.namespace == namespace) { + return namespace + '-' + JSON.stringify(user) + } + throw 'pass' + }) + authenticator.registerUserDeserializer(async (serialized: string) => { + if (serialized.startsWith(`${namespace}-`)) { + return JSON.parse(serialized.slice(`${namespace}-`.length)) + } + throw 'pass' + }) + + await instance.register(authenticator.initialize()) + await instance.register(authenticator.secureSession()) + authenticators[namespace] = authenticator + + instance.get( + `/${namespace}`, + { preValidation: authenticator.authenticate(strategyName, { authInfo: false }) }, + async () => `hello ${namespace}!` + ) + + instance.get( + `/user/${namespace}`, + { preValidation: authenticator.authenticate(strategyName, { authInfo: false }) }, + async (request) => JSON.stringify(request[`user${namespace}`]) + ) + + instance.post( + `/login-${namespace}`, + { + preValidation: authenticator.authenticate(strategyName, { + successRedirect: `/${namespace}`, + authInfo: false, + }), + }, + () => { + return + } + ) + + instance.post( + `/logout-${namespace}`, + { preValidation: authenticator.authenticate(strategyName, { authInfo: false }) }, + async (request, reply) => { + await request.logout() + void reply.send('logged out') + } + ) +} + const suite = (sessionPluginName) => { describe(`${sessionPluginName} tests`, () => { - describe('multiple registered instances', () => { + describe('multiple registered instances (clearSessionOnLogin: false)', () => { let server: FastifyInstance - let authenticators: Record let session: TestBrowserSession - let counter: number beforeEach(async () => { counter = 0 - server = getTestServer() authenticators = {} + server = getTestServer() session = new TestBrowserSession(server) for (const namespace of ['a', 'b']) { - await server.register(async (instance) => { - class TestStrategy extends Strategy { - authenticate(request: any, _options?: { pauseStream?: boolean }) { - if (request.isAuthenticated()) { - return this.pass() - } - if (request.body && request.body.login === 'test' && request.body.password === 'test') { - return this.success({ namespace, id: String(counter++) }) - } - - this.fail() - } - } - - const strategyName = `test-${namespace}` - const authenticator = new Authenticator({ key: `passport${namespace}`, userProperty: `user${namespace}` }) - authenticator.use(strategyName, new TestStrategy(strategyName)) - authenticator.registerUserSerializer(async (user) => { - if (user.namespace == namespace) { - return namespace + '-' + JSON.stringify(user) - } - throw 'pass' - }) - authenticator.registerUserDeserializer(async (serialized: string) => { - if (serialized.startsWith(`${namespace}-`)) { - return JSON.parse(serialized.slice(`${namespace}-`.length)) - } - throw 'pass' - }) - - await instance.register(authenticator.initialize()) - await instance.register(authenticator.secureSession()) - authenticators[namespace] = authenticator - - instance.get( - `/${namespace}`, - { preValidation: authenticator.authenticate(strategyName, { authInfo: false }) }, - async () => `hello ${namespace}!` - ) - - instance.get( - `/user/${namespace}`, - { preValidation: authenticator.authenticate(strategyName, { authInfo: false }) }, - async (request) => JSON.stringify(request[`user${namespace}`]) - ) - - instance.post( - `/login-${namespace}`, - { - preValidation: authenticator.authenticate(strategyName, { - successRedirect: `/${namespace}`, - authInfo: false, - }), - }, - () => { - return - } - ) - - instance.post( - `/logout-${namespace}`, - { preValidation: authenticator.authenticate(strategyName, { authInfo: false }) }, - async (request, reply) => { - await request.logout() - void reply.send('logged out') - } - ) - }) + await server.register(TestStrategyModule, { namespace, clearSessionOnLogin: false }) } }) @@ -252,6 +259,183 @@ const suite = (sessionPluginName) => { }) }) }) + + describe('multiple registered instances (clearSessionOnLogin: true)', () => { + let server: FastifyInstance + let session: TestBrowserSession + + beforeEach(async () => { + server = getTestServer() + session = new TestBrowserSession(server) + authenticators = {} + counter = 0 + + for (const namespace of ['a', 'b']) { + await server.register(TestStrategyModule, { namespace, clearSessionOnLogin: true }) + } + }) + + test('logging in with one instance should not log in the other instance', async () => { + let response = await session.inject({ method: 'GET', url: '/a' }) + expect(response.body).toEqual('Unauthorized') + expect(response.statusCode).toEqual(401) + + response = await session.inject({ method: 'GET', url: '/b' }) + expect(response.body).toEqual('Unauthorized') + expect(response.statusCode).toEqual(401) + + // login a + const loginResponse = await session.inject({ + method: 'POST', + url: '/login-a', + payload: { login: 'test', password: 'test' }, + }) + + expect(loginResponse.statusCode).toEqual(302) + expect(loginResponse.headers.location).toEqual('/a') + + // access protected route + response = await session.inject({ + method: 'GET', + url: '/a', + }) + expect(response.statusCode).toEqual(200) + expect(response.body).toEqual('hello a!') + + // access user data + response = await session.inject({ + method: 'GET', + url: '/user/a', + }) + expect(response.statusCode).toEqual(200) + + // try to access route protected by other instance + response = await session.inject({ + method: 'GET', + url: '/b', + }) + expect(response.statusCode).toEqual(401) + }) + + test('simultaneous login should NOT be possible', async () => { + // login a + let response = await session.inject({ + method: 'POST', + url: '/login-a', + payload: { login: 'test', password: 'test' }, + }) + + expect(response.statusCode).toEqual(302) + expect(response.headers.location).toEqual('/a') + + // login b + response = await session.inject({ + method: 'POST', + url: '/login-b', + payload: { login: 'test', password: 'test' }, + }) + + expect(response.statusCode).toEqual(302) + expect(response.headers.location).toEqual('/b') + + // access a protected route (/a) was invalidated after login /b + response = await session.inject({ + method: 'GET', + url: '/a', + }) + expect(response.statusCode).toEqual(401) + expect(response.body).toEqual('Unauthorized') + + // access b protected route + response = await session.inject({ + method: 'GET', + url: '/b', + }) + expect(response.statusCode).toEqual(200) + expect(response.body).toEqual('hello b!') + }) + + test('logging out with one instance should log out the other instance', async () => { + // login a + let response = await session.inject({ + method: 'POST', + url: '/login-a', + payload: { login: 'test', password: 'test' }, + }) + + expect(response.statusCode).toEqual(302) + expect(response.headers.location).toEqual('/a') + + // login b + response = await session.inject({ + method: 'POST', + url: '/login-b', + payload: { login: 'test', password: 'test' }, + }) + + expect(response.statusCode).toEqual(302) + expect(response.headers.location).toEqual('/b') + + // logout a + response = await session.inject({ + method: 'POST', + url: '/logout-a', + }) + expect(response.statusCode).toEqual(401) + + // try to access route protected by now logged out instance + response = await session.inject({ + method: 'GET', + url: '/a', + }) + expect(response.statusCode).toEqual(401) + + // access b protected route which should still be logged in + response = await session.inject({ + method: 'GET', + url: '/b', + }) + expect(response.statusCode).toEqual(200) + expect(response.body).toEqual('hello b!') + }) + + test('user objects from different instances should be different', async () => { + // login a + let response = await session.inject({ + method: 'POST', + url: '/login-a', + payload: { login: 'test', password: 'test' }, + }) + expect(response.statusCode).toEqual(302) + expect(response.headers.location).toEqual('/a') + + response = await session.inject({ + method: 'GET', + url: '/user/a', + }) + expect(response.statusCode).toEqual(200) + const userA = JSON.parse(response.body) + + // login b + response = await session.inject({ + method: 'POST', + url: '/login-b', + payload: { login: 'test', password: 'test' }, + }) + + expect(response.statusCode).toEqual(302) + expect(response.headers.location).toEqual('/b') + + response = await session.inject({ + method: 'GET', + url: '/user/b', + }) + expect(response.statusCode).toEqual(200) + const userB = JSON.parse(response.body) + + expect(userA.id).not.toEqual(userB.id) + }) + }) } suite('@fastify/session') diff --git a/test/passport.test.ts b/test/passport.test.ts index 9ffefe42..c047e98e 100644 --- a/test/passport.test.ts +++ b/test/passport.test.ts @@ -24,7 +24,9 @@ const suite = (sessionPluginName) => { }) test(`should allow login, and add successMessage to session upon logged in`, async () => { - const { server, fastifyPassport } = getConfiguredTestServer() + const { server, fastifyPassport } = getConfiguredTestServer('test', new TestStrategy('test'), null, { + clearSessionIgnoreFields: ['messages'], + }) server.get( '/', @@ -79,7 +81,9 @@ const suite = (sessionPluginName) => { } } - const { server, fastifyPassport } = getConfiguredTestServer('test', new WelcomeStrategy('test')) + const { server, fastifyPassport } = getConfiguredTestServer('test', new WelcomeStrategy('test'), null, { + clearSessionIgnoreFields: ['messages'], + }) server.get( '/', { @@ -121,7 +125,7 @@ const suite = (sessionPluginName) => { test(`should throw error if pauseStream is being used`, async () => { jest.spyOn(console, 'error').mockImplementation(jest.fn()) - const fastifyPassport = new Authenticator() + const fastifyPassport = new Authenticator({ clearSessionIgnoreFields: ['messages'] }) fastifyPassport.use('test', new TestStrategy('test')) fastifyPassport.registerUserSerializer(async (user) => JSON.stringify(user)) fastifyPassport.registerUserDeserializer(async (serialized: string) => JSON.parse(serialized)) @@ -164,7 +168,9 @@ const suite = (sessionPluginName) => { }) test(`should execute successFlash if logged in`, async () => { - const { server, fastifyPassport } = getConfiguredTestServer() + const { server, fastifyPassport } = getConfiguredTestServer('test', new TestStrategy('test'), null, { + clearSessionIgnoreFields: ['flash'], + }) server.get( '/', { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, @@ -299,7 +305,9 @@ const suite = (sessionPluginName) => { }) test(`should redirect to the returnTo set in the session upon login`, async () => { - const { server, fastifyPassport } = getConfiguredTestServer() + const { server, fastifyPassport } = getConfiguredTestServer('test', new TestStrategy('test'), null, { + clearSessionIgnoreFields: ['returnTo'], + }) server.addHook('preValidation', async (request, _reply) => { request.session.set('returnTo', '/success') }) @@ -602,7 +610,7 @@ const suite = (sessionPluginName) => { }) test(`should allow registering strategies after creating routes referring to those strategies by name`, async () => { - const { server, fastifyPassport } = getRegisteredTestServer() + const { server, fastifyPassport } = getRegisteredTestServer(null, { clearSessionIgnoreFields: ['messages'] }) server.get( '/', diff --git a/test/session-isolation.test.ts b/test/session-isolation.test.ts index 919e1148..e8a4591e 100644 --- a/test/session-isolation.test.ts +++ b/test/session-isolation.test.ts @@ -1,38 +1,44 @@ /* eslint-disable @typescript-eslint/no-empty-function */ import { getConfiguredTestServer, TestBrowserSession, generateTestUser } from './helpers' -const { server, fastifyPassport } = getConfiguredTestServer() - -server.get( - '/protected', - { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, - async () => 'hello!' -) -server.get('/my-id', { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, async (request) => - String((request.user as any).id) -) -server.post( - '/login', - { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, - async () => 'success' -) - -server.post('/force-login', async (request, reply) => { - await request.logIn(generateTestUser()) - void reply.send('logged in') -}) - -server.post( - '/logout', - { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, - async (request, reply) => { - await request.logout() - void reply.send('logged out') - } -) +function createServer() { + const { server, fastifyPassport } = getConfiguredTestServer() + + server.get( + '/protected', + { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, + async () => 'hello!' + ) + server.get('/my-id', { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, async (request) => + String((request.user as any).id) + ) + server.post( + '/login', + { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, + async () => 'success' + ) + + server.post('/force-login', async (request, reply) => { + await request.logIn(generateTestUser()) + void reply.send('logged in') + }) + + server.post( + '/logout', + { preValidation: fastifyPassport.authenticate('test', { authInfo: false }) }, + async (request, reply) => { + await request.logout() + void reply.send('logged out') + } + ) + return server +} const suite = (sessionPluginName) => { + process.env.SESSION_PLUGIN = sessionPluginName + const server = createServer() describe(`${sessionPluginName} tests`, () => { + const sessionOnlyTest = sessionPluginName === '@fastify/session' ? test : test.skip describe('session isolation', () => { let userA, userB, userC @@ -171,12 +177,25 @@ const suite = (sessionPluginName) => { return response.body }) ) - // expect each returned ID to be unique expect(Array.from(new Set(ids)).sort()).toEqual(ids.sort()) }) + + sessionOnlyTest('should regenerate session on login', async () => { + expect(userA.cookies['sessionId']).toBeUndefined() + await userA.inject({ method: 'GET', url: '/protected' }) + expect(userA.cookies['sessionId']).not.toBeUndefined() + const prevSessionId = userA.cookies.sessionId + await userA.inject({ + method: 'POST', + url: '/login', + payload: { login: 'test', password: 'test' }, + }) + expect(userA.cookies.sessionId).not.toBe(prevSessionId) + }) }) }) + delete process.env.SESSION_PLUGIN } suite('@fastify/session')