From d7cd739642e9f961598f372dfebede88d9630c8c Mon Sep 17 00:00:00 2001 From: cbolles Date: Mon, 12 Feb 2024 17:20:16 -0500 Subject: [PATCH 1/4] Remove print debugs --- packages/server/src/jwt/jwt.guard.ts | 2 +- packages/server/src/jwt/jwt.module.ts | 30 ++++++++++++++- packages/server/src/jwt/jwt.service.ts | 19 +++++++++- packages/server/src/jwt/jwt.strategy.ts | 49 +++++++++++-------------- 4 files changed, 68 insertions(+), 32 deletions(-) diff --git a/packages/server/src/jwt/jwt.guard.ts b/packages/server/src/jwt/jwt.guard.ts index c431b4d1..d8da9f86 100644 --- a/packages/server/src/jwt/jwt.guard.ts +++ b/packages/server/src/jwt/jwt.guard.ts @@ -3,7 +3,7 @@ import { AuthGuard } from '@nestjs/passport'; import { GqlExecutionContext } from '@nestjs/graphql'; @Injectable() -export class JwtAuthGuard extends AuthGuard('local') { +export class JwtAuthGuard extends AuthGuard('jwt') { getRequest(context: ExecutionContext) { // Return HTTP context if (context.getType() === 'http') { diff --git a/packages/server/src/jwt/jwt.module.ts b/packages/server/src/jwt/jwt.module.ts index e59eac10..83fcd1a1 100644 --- a/packages/server/src/jwt/jwt.module.ts +++ b/packages/server/src/jwt/jwt.module.ts @@ -1,13 +1,39 @@ -import { Module, forwardRef } from '@nestjs/common'; +import { Module, UnauthorizedException, forwardRef } from '@nestjs/common'; import { JwtService } from './jwt.service'; import { HttpModule } from '@nestjs/axios'; import { JwtAuthGuard } from './jwt.guard'; import { JwtStrategy } from './jwt.strategy'; import { OrganizationModule } from '../organization/organization.module'; import { UserOrgModule } from '../userorg/userorg.module'; +import { JwtSecretRequestType, JwtModule as NestJwtModule } from '@nestjs/jwt'; @Module({ - imports: [HttpModule, forwardRef(() => OrganizationModule), UserOrgModule], + imports: [ + HttpModule, + forwardRef(() => OrganizationModule), + UserOrgModule, + NestJwtModule.registerAsync({ + imports: [forwardRef(() => JwtModule)], + inject: [JwtService], + useFactory: (jwtService: JwtService) => ({ + secretOrKeyProvider: async (requestType, rawJwtToken) => { + // Can only verify tokens via the Google public key + switch(requestType) { + case JwtSecretRequestType.SIGN: + throw new Error('Cannot sign tokens'); + case JwtSecretRequestType.VERIFY: + const publicKey = await jwtService.getPublicKey(rawJwtToken); + if (!publicKey) { + throw new UnauthorizedException('No public key found for token'); + } + return publicKey; + default: + throw new Error('Invalid request type'); + } + } + }) + }), + ], providers: [JwtService, JwtAuthGuard, JwtStrategy], exports: [JwtService] }) diff --git a/packages/server/src/jwt/jwt.service.ts b/packages/server/src/jwt/jwt.service.ts index ea651f83..b03940a2 100644 --- a/packages/server/src/jwt/jwt.service.ts +++ b/packages/server/src/jwt/jwt.service.ts @@ -21,10 +21,27 @@ export class JwtService { return response.data; } - async getPublicKey(kid: string): Promise { + async getPublicKey(rawToken: string | null | Buffer | object): Promise { + if (!rawToken) { + return null; + } + if (typeof rawToken === 'object') { + return null; + } + + const token = jwt.decode(rawToken, { complete: true }); + if (!token) { + return null; + } + const kid = token.header.kid; + if (!kid) { + return null; + } + if (!this.publicKeys || !this.publicKeys[kid]) { this.publicKeys = await this.queryForPublicKey(); } + return this.publicKeys[kid] || null; } diff --git a/packages/server/src/jwt/jwt.strategy.ts b/packages/server/src/jwt/jwt.strategy.ts index 75f597dc..df15814b 100644 --- a/packages/server/src/jwt/jwt.strategy.ts +++ b/packages/server/src/jwt/jwt.strategy.ts @@ -1,38 +1,31 @@ import { Injectable } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; -import { Strategy } from 'passport-local'; import { TokenPayload } from './token.dto'; -import { Request } from 'express'; -import { ParamsDictionary } from 'express-serve-static-core'; -import { ParsedQs } from 'qs'; import { JwtService } from './jwt.service'; +import { Strategy, ExtractJwt } from 'passport-jwt'; +import { Request } from 'express'; @Injectable() export class JwtStrategy extends PassportStrategy(Strategy) { - constructor(private readonly jwtService: JwtService) { - super(); - } - - async authenticate( - req: Request>, - _options?: any - ): Promise { - // Check if the token is present - const rawToken = req.headers.authorization; - if (!rawToken) { - this.fail({ meessage: 'Invalid Token' }, 400); - return; - } - - // Validate the token - const payload = await this.jwtService.validate(rawToken); - if (!payload) { - this.fail({ message: 'Invalid Token' }, 400); - return; - } - - const result = await this.validate(payload); - this.success(result); + constructor(jwtService: JwtService) { + super({ + jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), + ignoreExpiration: false, + secretOrKeyProvider: (_request: Request, rawJwtToken: any, done: (err: any, secretOrKey?: string | Buffer) => void) => { + // Can only verify tokens via the Google public key + jwtService.getPublicKey(rawJwtToken) + .then((publicKey) => { + if (!publicKey) { + done(new Error('No public key found for token')); + return; + } + done(null, publicKey); + }) + .catch((err) => { + done(err); + }); + } + }); } /** From 1f250a7b8115de1f491c3c1d386a72172c9d8311 Mon Sep 17 00:00:00 2001 From: cbolles Date: Mon, 12 Feb 2024 17:21:02 -0500 Subject: [PATCH 2/4] Remove incorrect comment --- packages/server/src/jwt/jwt.strategy.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/server/src/jwt/jwt.strategy.ts b/packages/server/src/jwt/jwt.strategy.ts index df15814b..3bfb75c3 100644 --- a/packages/server/src/jwt/jwt.strategy.ts +++ b/packages/server/src/jwt/jwt.strategy.ts @@ -28,11 +28,6 @@ export class JwtStrategy extends PassportStrategy(Strategy) { }); } - /** - * Need to add the organization at this step since the organization is - * queried from the database and not part of the JWT token. This allows - * the organization to then be pulled in via the organization context - */ async validate(payload: TokenPayload): Promise { return { ...payload From d7678cb711245d4d0811c9e3e5673c660e6eafd6 Mon Sep 17 00:00:00 2001 From: cbolles Date: Mon, 12 Feb 2024 17:21:18 -0500 Subject: [PATCH 3/4] Fix formatting --- packages/server/src/jwt/jwt.module.ts | 4 ++-- packages/server/src/jwt/jwt.strategy.ts | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/server/src/jwt/jwt.module.ts b/packages/server/src/jwt/jwt.module.ts index 83fcd1a1..a045b434 100644 --- a/packages/server/src/jwt/jwt.module.ts +++ b/packages/server/src/jwt/jwt.module.ts @@ -18,7 +18,7 @@ import { JwtSecretRequestType, JwtModule as NestJwtModule } from '@nestjs/jwt'; useFactory: (jwtService: JwtService) => ({ secretOrKeyProvider: async (requestType, rawJwtToken) => { // Can only verify tokens via the Google public key - switch(requestType) { + switch (requestType) { case JwtSecretRequestType.SIGN: throw new Error('Cannot sign tokens'); case JwtSecretRequestType.VERIFY: @@ -32,7 +32,7 @@ import { JwtSecretRequestType, JwtModule as NestJwtModule } from '@nestjs/jwt'; } } }) - }), + }) ], providers: [JwtService, JwtAuthGuard, JwtStrategy], exports: [JwtService] diff --git a/packages/server/src/jwt/jwt.strategy.ts b/packages/server/src/jwt/jwt.strategy.ts index 3bfb75c3..e62ebf8a 100644 --- a/packages/server/src/jwt/jwt.strategy.ts +++ b/packages/server/src/jwt/jwt.strategy.ts @@ -11,9 +11,14 @@ export class JwtStrategy extends PassportStrategy(Strategy) { super({ jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), ignoreExpiration: false, - secretOrKeyProvider: (_request: Request, rawJwtToken: any, done: (err: any, secretOrKey?: string | Buffer) => void) => { + secretOrKeyProvider: ( + _request: Request, + rawJwtToken: any, + done: (err: any, secretOrKey?: string | Buffer) => void + ) => { // Can only verify tokens via the Google public key - jwtService.getPublicKey(rawJwtToken) + jwtService + .getPublicKey(rawJwtToken) .then((publicKey) => { if (!publicKey) { done(new Error('No public key found for token')); @@ -24,8 +29,8 @@ export class JwtStrategy extends PassportStrategy(Strategy) { .catch((err) => { done(err); }); - } - }); + } + }); } async validate(payload: TokenPayload): Promise { From 19a151d8eb461fa962c6e6866f001e33c467c02d Mon Sep 17 00:00:00 2001 From: cbolles Date: Mon, 12 Feb 2024 17:24:10 -0500 Subject: [PATCH 4/4] Cleanup code --- packages/server/src/jwt/jwt.service.ts | 31 ++++++-------------------- 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/packages/server/src/jwt/jwt.service.ts b/packages/server/src/jwt/jwt.service.ts index b03940a2..a766f6fa 100644 --- a/packages/server/src/jwt/jwt.service.ts +++ b/packages/server/src/jwt/jwt.service.ts @@ -21,7 +21,9 @@ export class JwtService { return response.data; } + // TODO: Handle when key rotation has taken place async getPublicKey(rawToken: string | null | Buffer | object): Promise { + // Make sure the tokn is the correct type if (!rawToken) { return null; } @@ -29,43 +31,24 @@ export class JwtService { return null; } + // Decode the token to get the kid const token = jwt.decode(rawToken, { complete: true }); if (!token) { return null; } + + // Get the kid from the token const kid = token.header.kid; if (!kid) { return null; } + // If we don't have the public keys yet or the kid isn't in the public keys, query for the public keys if (!this.publicKeys || !this.publicKeys[kid]) { this.publicKeys = await this.queryForPublicKey(); } + // Return the public key return this.publicKeys[kid] || null; } - - async validate(rawToken: string): Promise { - // Parse out the token - const tokenString = rawToken.split(' ')[1]; - const token = jwt.decode(tokenString, { complete: true }) as any; - - // Get the kid to verify the JWT against - const kid = token.header.kid; - if (!kid) { - return null; - } - - const publicKey = await this.getPublicKey(kid); - if (!publicKey) { - return null; - } - - try { - jwt.verify(tokenString, publicKey); - return token.payload; - } catch (e) { - return null; - } - } }