From bea5df3c08fb7ae59a01d3811e4eb4fa4c8a98fc Mon Sep 17 00:00:00 2001 From: hender-hs Date: Fri, 24 Mar 2023 13:22:45 -0300 Subject: [PATCH 1/4] fix(sign): fix sign parameters type and usage The payload and signOptions parameters have incorrect type checking, therefore possibly breaking the jsonwebtoken source code passing invalid payload and sign options. Scenario: if "expiresIn" has been set in the nestjs module and uses payload as a string, it'll break the code, because jsonwebtoken does not allow the use of "expiresIn" option with a string payload. In order to solve the problem, it is necessary predict the developer's behavior using types. Also throw an error in case of incorrect use of the sign method of the JwtService class. --- lib/jwt.service.ts | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/lib/jwt.service.ts b/lib/jwt.service.ts index 80ca2e4c..7db54ac4 100644 --- a/lib/jwt.service.ts +++ b/lib/jwt.service.ts @@ -18,6 +18,11 @@ export class JwtService { private readonly options: JwtModuleOptions = {} ) {} + sign( + payload: string, + options?: Omit + ): string; + sign(payload: Buffer | object, options?: JwtSignOptions): string; sign(payload: string | Buffer | object, options?: JwtSignOptions): string { const signOptions = this.mergeJwtOptions( { ...options }, @@ -30,9 +35,29 @@ export class JwtService { JwtSecretRequestType.SIGN ); + const allowedSignOptKeys = ['secret', 'privateKey']; + const signOptKeys = Object.keys(signOptions); + if ( + typeof payload === 'string' && + signOptKeys.some((k) => !allowedSignOptKeys.includes(k)) + ) { + throw new Error( + 'Not allowed payload as string with these sign options: ' + + signOptKeys.join(', ') + ); + } + return jwt.sign(payload, secret, signOptions); } + signAsync( + payload: string, + options?: Omit + ): Promise; + signAsync( + payload: Buffer | object, + options?: JwtSignOptions + ): Promise; signAsync( payload: string | Buffer | object, options?: JwtSignOptions @@ -48,6 +73,18 @@ export class JwtService { JwtSecretRequestType.SIGN ); + const allowedSignOptKeys = ['secret', 'privateKey']; + const signOptKeys = Object.keys(signOptions); + if ( + typeof payload === 'string' && + signOptKeys.some((k) => !allowedSignOptKeys.includes(k)) + ) { + throw new Error( + 'Not allowed payload as string with these sign options: ' + + signOptKeys.join(', ') + ); + } + return new Promise((resolve, reject) => jwt.sign(payload, secret, signOptions, (err, encoded) => err ? reject(err) : resolve(encoded) From 42c286ca467b2394d3b15670b191d1f09e871c40 Mon Sep 17 00:00:00 2001 From: hender-hs Date: Fri, 24 Mar 2023 13:26:47 -0300 Subject: [PATCH 2/4] test(sign): JwtService sign param tests --- lib/jwt.service.spec.ts | 75 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/lib/jwt.service.spec.ts b/lib/jwt.service.spec.ts index 09151dac..253d6540 100644 --- a/lib/jwt.service.spec.ts +++ b/lib/jwt.service.spec.ts @@ -303,4 +303,79 @@ describe('JWT Service', () => { ).resolves.toBe(`verified_${testPayload}_by_customPublicKey`); }); }); + + describe('should not use invalid sign options', () => { + let jwtService: JwtService; + let testPayloadStr: string = getRandomString(); + + beforeAll(async () => { + jwtService = await setup({ secretOrKeyProvider: undefined }); + }); + + it('should not "sign" expect errors with a "payload" string and "secret"', () => { + // @ts-expect-no-error + expect(() => jwtService.sign(testPayloadStr, { secret: 'secret' })); + }); + + it('should not "signAsync" expect errors with a "payload" string and "privateKey"', () => { + // @ts-expect-no-error + expect(() => + jwtService.signAsync(testPayloadStr, { privateKey: 'privateKey' }) + ); + }); + }); + + describe('should use invalid sign options', () => { + const signOptions: jwt.SignOptions = { + expiresIn: '1d' + }; + + let jwtService: JwtService; + let testPayloadStr: string = getRandomString(); + let testPayloadObj: object = {}; + + beforeAll(async () => { + jwtService = await setup({ signOptions, secretOrKeyProvider: undefined }); + }); + + it('should "sign" expect errors with a "payload" string with "expiresIn"', () => { + expect(() => + // @ts-expect-error + jwtService.sign(testPayloadStr, { expiresIn: 60 }) + ).toThrowError( + 'Not allowed payload as string with these sign options: expiresIn' + ); + }); + + it('should "signAsync" expect errors with a "payload" string with "notBefore"', () => { + expect(() => + // @ts-expect-error + jwtService.signAsync(testPayloadStr, { notBefore: 60 }) + ).toThrowError( + 'Not allowed payload as string with these sign options: expiresIn, notBefore' + ); + }); + + it('should not "sign" expect errors with a "payload" object with "notBefore" ', () => { + // @ts-expect-no-error + expect(() => jwtService.sign(testPayloadObj, { notBefore: 60 })); + }); + + it('should not "signAsync" expect errors with a "payload" object with "notBefore" ', () => { + // @ts-expect-no-error + expect(() => jwtService.signAsync(testPayloadObj, { notBefore: 60 })); + }); + + it('should "sign" expect errors using "payload" string with already defined invalid sign options', () => { + expect(() => jwtService.sign(testPayloadStr)).toThrowError( + 'Not allowed payload as string with these sign options: expiresIn' + ); + }); + + it('should "signAsync" expect errors using "payload" string with already defined invalid sign options', () => { + expect(() => jwtService.signAsync(testPayloadStr)).toThrowError( + 'Not allowed payload as string with these sign options: expiresIn' + ); + }); + }); }); From 3b4138f7f22538bbea051098bfba0aabf233a02d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Mon, 12 Jun 2023 14:52:58 +0200 Subject: [PATCH 3/4] chore: laying the grounds for 11.0.0 --- .circleci/config.yml | 21 ++++++++++----------- package.json | 2 +- tsconfig.json | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6c91ff01..f8ed3cf6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,26 +3,26 @@ version: 2 aliases: - &restore-cache restore_cache: - key: dependency-cache-{{ checksum "package.json" }} + key: dependency-cache-{{ checksum "package.json" }} - &install-deps run: - name: Install dependencies - command: npm ci + name: Install dependencies + command: npm ci - &build-packages run: - name: Build - command: npm run build + name: Build + command: npm run build jobs: build: working_directory: ~/nest docker: - - image: cimg/node:17.9 + - image: cimg/node:20.2 steps: - checkout - run: - name: Use NPM v8 - command: npm install -g npm@^8 + name: Update NPM version + command: sudo npm install -g npm@latest - restore_cache: key: dependency-cache-{{ checksum "package.json" }} - run: @@ -34,11 +34,11 @@ jobs: - ./node_modules - run: name: Build - command: npm run build + command: npm run build test: working_directory: ~/nest docker: - - image: cimg/node:17.9 + - image: cimg/node:20.2 steps: - checkout - *restore-cache @@ -55,4 +55,3 @@ workflows: - test: requires: - build - \ No newline at end of file diff --git a/package.json b/package.json index 3780b2c8..2ec4494d 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "prepare": "husky install" }, "peerDependencies": { - "@nestjs/common": "^8.0.0 || ^9.0.0" + "@nestjs/common": "^8.0.0 || ^9.0.0 || ^10.0.0" }, "devDependencies": { "@commitlint/cli": "17.6.5", diff --git a/tsconfig.json b/tsconfig.json index 2cb7090c..e4906abd 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -7,7 +7,7 @@ "noLib": false, "emitDecoratorMetadata": true, "experimentalDecorators": true, - "target": "es6", + "target": "ES2021", "sourceMap": false, "outDir": "./dist", "rootDir": "./lib", From 60c3d6e77992d6b120eaf5d9215534e6fe90e429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Mon, 12 Jun 2023 14:54:53 +0200 Subject: [PATCH 4/4] chore: minor changes --- lib/jwt.service.spec.ts | 8 ++++---- lib/jwt.service.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/jwt.service.spec.ts b/lib/jwt.service.spec.ts index 253d6540..39b33905 100644 --- a/lib/jwt.service.spec.ts +++ b/lib/jwt.service.spec.ts @@ -343,7 +343,7 @@ describe('JWT Service', () => { // @ts-expect-error jwtService.sign(testPayloadStr, { expiresIn: 60 }) ).toThrowError( - 'Not allowed payload as string with these sign options: expiresIn' + 'Payload as string is not allowed with the following sign options: expiresIn' ); }); @@ -352,7 +352,7 @@ describe('JWT Service', () => { // @ts-expect-error jwtService.signAsync(testPayloadStr, { notBefore: 60 }) ).toThrowError( - 'Not allowed payload as string with these sign options: expiresIn, notBefore' + 'Payload as string is not allowed with the following sign options: expiresIn, notBefore' ); }); @@ -368,13 +368,13 @@ describe('JWT Service', () => { it('should "sign" expect errors using "payload" string with already defined invalid sign options', () => { expect(() => jwtService.sign(testPayloadStr)).toThrowError( - 'Not allowed payload as string with these sign options: expiresIn' + 'Payload as string is not allowed with the following sign options: expiresIn' ); }); it('should "signAsync" expect errors using "payload" string with already defined invalid sign options', () => { expect(() => jwtService.signAsync(testPayloadStr)).toThrowError( - 'Not allowed payload as string with these sign options: expiresIn' + 'Payload as string is not allowed with the following sign options: expiresIn' ); }); }); diff --git a/lib/jwt.service.ts b/lib/jwt.service.ts index 7db54ac4..5d688c7f 100644 --- a/lib/jwt.service.ts +++ b/lib/jwt.service.ts @@ -42,7 +42,7 @@ export class JwtService { signOptKeys.some((k) => !allowedSignOptKeys.includes(k)) ) { throw new Error( - 'Not allowed payload as string with these sign options: ' + + 'Payload as string is not allowed with the following sign options: ' + signOptKeys.join(', ') ); } @@ -80,7 +80,7 @@ export class JwtService { signOptKeys.some((k) => !allowedSignOptKeys.includes(k)) ) { throw new Error( - 'Not allowed payload as string with these sign options: ' + + 'Payload as string is not allowed with the following sign options: ' + signOptKeys.join(', ') ); }