From 637b8547447bdfb5f1cac8718d007e665b433f70 Mon Sep 17 00:00:00 2001 From: Peter Perlepes Date: Thu, 13 Jan 2022 11:41:06 +0200 Subject: [PATCH 1/2] feat: Add injectable loadCryptoKeyFunction fix: Revisit middleware and instance tests, deepmerge fetcher options --- .eslintrc.js | 5 +- .pnp.cjs | 1 + packages/backend-core/src/Base.ts | 30 +++-- packages/sdk-node/package.json | 3 +- packages/sdk-node/src/Clerk.ts | 124 +++++++++++------- packages/sdk-node/src/__tests__/Clerk.test.ts | 80 +---------- .../sdk-node/src/__tests__/middleware.test.ts | 28 ++-- packages/sdk-node/src/info.ts | 2 +- yarn.lock | 1 + 9 files changed, 126 insertions(+), 148 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index c768f42349e..8ea44b22689 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -16,6 +16,7 @@ module.exports = { '@typescript-eslint/no-unsafe-assignment': 'warn', 'simple-import-sort/imports': 'error', '@typescript-eslint/no-unsafe-call': 'off', - '@typescript-eslint/no-unsafe-member-access': 'off' - } + '@typescript-eslint/no-unsafe-member-access': 'off', + '@typescript-eslint/no-unsafe-return': 'warn', + }, }; diff --git a/.pnp.cjs b/.pnp.cjs index 1a23c7e764c..e89af70780a 100755 --- a/.pnp.cjs +++ b/.pnp.cjs @@ -902,6 +902,7 @@ function $$SETUP_STATE(hydrateRuntimeState, basePath) { ["@typescript-eslint/parser", "virtual:741cead45f55954a45f37a2caecbcc0a526ec48f52e0bc8ed655e34523095ff3680820084e6476ab975a5a3d0d84297111a55441cb139ee8a44706c2be539ec6#npm:5.7.0"], ["camelcase-keys", "npm:6.2.2"], ["cookies", "npm:0.8.0"], + ["deepmerge", "npm:4.2.2"], ["eslint", "npm:8.5.0"], ["eslint-config-prettier", "virtual:741cead45f55954a45f37a2caecbcc0a526ec48f52e0bc8ed655e34523095ff3680820084e6476ab975a5a3d0d84297111a55441cb139ee8a44706c2be539ec6#npm:8.3.0"], ["got", "npm:11.8.3"], diff --git a/packages/backend-core/src/Base.ts b/packages/backend-core/src/Base.ts index 5baf2f2c7dc..22660f874cd 100644 --- a/packages/backend-core/src/Base.ts +++ b/packages/backend-core/src/Base.ts @@ -11,6 +11,7 @@ export const API_KEY = process.env.CLERK_API_KEY || ''; type ImportKeyFunction = ( ...args: any[] ) => Promise; +type LoadCryptoKeyFunction = (token: string) => Promise; type DecodeBase64Function = (base64Encoded: string) => string; type VerifySignatureFunction = (...args: any[]) => Promise; @@ -29,6 +30,7 @@ type AuthState = { status: AuthStatus; session?: Session; interstitial?: string; + sessionClaims?: JWTPayload; }; type AuthStateParams = { @@ -58,20 +60,25 @@ export class Base { importKeyFunction: ImportKeyFunction; verifySignatureFunction: VerifySignatureFunction; decodeBase64Function: DecodeBase64Function; + loadCryptoKeyFunction?: LoadCryptoKeyFunction; + /** * Creates an instance of a Clerk Base. * @param {ImportKeyFunction} importKeyFunction Function to import a PEM. Should have a similar result to crypto.subtle.importKey + * @param {LoadCryptoKeyFunction} loadCryptoKeyFunction Function load a PK CryptoKey from the host environment. Used for JWK clients etc. * @param {VerifySignatureFunction} verifySignatureFunction Function to verify a CryptoKey or a similar structure later on. Should have a similar result to crypto.subtle.verify * @param {DecodeBase64Function} decodeBase64Function Function to decode a Base64 string. Similar to atob */ constructor( importKeyFunction: ImportKeyFunction, verifySignatureFunction: VerifySignatureFunction, - decodeBase64Function: DecodeBase64Function + decodeBase64Function: DecodeBase64Function, + loadCryptoKeyFunction?: LoadCryptoKeyFunction ) { this.importKeyFunction = importKeyFunction; this.verifySignatureFunction = verifySignatureFunction; this.decodeBase64Function = decodeBase64Function; + this.loadCryptoKeyFunction = loadCryptoKeyFunction; } /** @@ -81,14 +88,16 @@ export class Base { * The public key will be supplied in the form of CryptoKey or will be loaded from the CLERK_JWT_KEY environment variable. * * @param {string} token - * @param {CryptoKey | null} [key] * @return {Promise} claims */ - verifySessionToken = async ( - token: string, - key?: CryptoKey | null - ): Promise => { - const availableKey = key || (await this.loadPublicKey()); + verifySessionToken = async (token: string): Promise => { + // Try to load the PK from supplied function and + // if there is no custom load function + // try to load from the environment. + const availableKey = this.loadCryptoKeyFunction + ? await this.loadCryptoKeyFunction(token) + : await this.loadCryptoKeyFromEnv(); + const claims = await this.verifyJwt(availableKey, token); checkClaims(claims); return claims; @@ -96,11 +105,12 @@ export class Base { /** * - * Construct the RSA public key from the PEM retrieved from the CLERK_JWT_KEY environment variable. + * Modify the RSA public key from the PEM retrieved from the CLERK_JWT_KEY environment variable + * and return a contructed CryptoKey. * You will find that at your application dashboard (https://dashboard.clerk.dev) under Settings -> API keys * */ - loadPublicKey = async (): Promise => { + loadCryptoKeyFromEnv = async (): Promise => { const key = process.env.CLERK_JWT_KEY; if (!key) { throw new Error('Missing jwt key'); @@ -214,6 +224,7 @@ export class Base { id: sessionClaims.sid as string, userId: sessionClaims.sub as string, }, + sessionClaims, }; } @@ -267,6 +278,7 @@ export class Base { id: sessionClaims.sid as string, userId: sessionClaims.sub as string, }, + sessionClaims, }; } diff --git a/packages/sdk-node/package.json b/packages/sdk-node/package.json index f56ee37d237..7157e770670 100644 --- a/packages/sdk-node/package.json +++ b/packages/sdk-node/package.json @@ -57,6 +57,7 @@ "@peculiar/webcrypto": "^1.2.3", "camelcase-keys": "^6.2.2", "cookies": "^0.8.0", + "deepmerge": "^4.2.2", "got": "^11.8.2", "jsonwebtoken": "^8.5.1", "jwks-rsa": "^2.0.4", @@ -80,4 +81,4 @@ "publishConfig": { "access": "public" } -} \ No newline at end of file +} diff --git a/packages/sdk-node/src/Clerk.ts b/packages/sdk-node/src/Clerk.ts index 735ac224834..4e29bc0bac4 100644 --- a/packages/sdk-node/src/Clerk.ts +++ b/packages/sdk-node/src/Clerk.ts @@ -5,10 +5,11 @@ Session, } from '@clerk/backend-core'; import Cookies from 'cookies'; +import deepmerge from 'deepmerge'; import type { NextFunction, Request, Response } from 'express'; -import got from 'got'; +import got, { OptionsOfJSONResponseBody } from 'got'; import jwt, { JwtPayload } from 'jsonwebtoken'; -import jwks, { JwksClient } from 'jwks-rsa'; +import jwks from 'jwks-rsa'; import querystring from 'querystring'; import { SupportMessages } from './constants/SupportMessages'; @@ -21,7 +22,7 @@ const defaultApiKey = process.env.CLERK_API_KEY || ''; const defaultApiVersion = process.env.CLERK_API_VERSION || 'v1'; const defaultServerApiUrl = process.env.CLERK_API_URL || 'https://api.clerk.dev'; -const defaultJWKSCacheMaxAge = 3600000; // 1 hour +const JWKS_MAX_AGE = 3600000; // 1 hour const packageRepo = 'https://github.com/clerkinc/clerk-sdk-node'; export type MiddlewareOptions = { @@ -54,13 +55,8 @@ const verifySignature = async ( return await crypto.subtle.verify(algorithm, key, signature, data); }; -/** Base initialization */ - -const nodeBase = new Base(importKey, verifySignature, decodeBase64); - export default class Clerk extends ClerkBackendAPI { - // private _restClient: RestClient; - private _jwksClient: JwksClient; + base: Base; // singleton instance static _instance: Clerk; @@ -70,7 +66,7 @@ export default class Clerk extends ClerkBackendAPI { serverApiUrl = defaultServerApiUrl, apiVersion = defaultApiVersion, httpOptions = {}, - jwksCacheMaxAge = defaultJWKSCacheMaxAge, + jwksCacheMaxAge = JWKS_MAX_AGE, }: { apiKey?: string; serverApiUrl?: string; @@ -82,16 +78,22 @@ export default class Clerk extends ClerkBackendAPI { url, { method, authorization, contentType, userAgent, body } ) => { - return got(url, { - method, - responseType: 'json', - headers: { - authorization, - 'Content-Type': contentType, - 'User-Agent': userAgent, + const finalHTTPOptions = deepmerge( + { + method, + responseType: 'json', + headers: { + authorization, + 'Content-Type': contentType, + 'User-Agent': userAgent, + }, + // @ts-ignore + ...(body && { body: querystring.stringify(body) }), }, - ...(body && { body: querystring.stringify(body) }), - }); + httpOptions + ) as OptionsOfJSONResponseBody; + + return got(url, finalHTTPOptions); }; super({ @@ -108,21 +110,48 @@ export default class Clerk extends ClerkBackendAPI { throw Error(SupportMessages.API_KEY_NOT_FOUND); } - // TBD: Add jwk client as an argument to getAuthState ? - // this._jwksClient = jwks({ - // jwksUri: `${serverApiUrl}/${apiVersion}/jwks`, - // requestHeaders: { - // Authorization: `Bearer ${apiKey}`, - // }, - // timeout: 5000, - // cache: true, - // cacheMaxAge: jwksCacheMaxAge, - // }); - - // const key = await this._jwksClient.getSigningKey(decoded.header.kid); - // const verified = jwt.verify(token, key.getPublicKey(), { - // algorithms: algorithms as jwt.Algorithm[], - // }) as JwtPayload; + const loadCryptoKey = async (token: string) => { + const decoded = jwt.decode(token, { complete: true }); + if (!decoded) { + throw new Error(`Failed to decode token: ${token}`); + } + + const jwksClient = jwks({ + jwksUri: `${serverApiUrl}/${apiVersion}/jwks`, + requestHeaders: { + Authorization: `Bearer ${defaultApiKey}`, + }, + timeout: 5000, + cache: true, + cacheMaxAge: jwksCacheMaxAge, + }); + + const encoder = new TextEncoder(); + + return await crypto.subtle.importKey( + 'raw', + encoder.encode( + ( + await jwksClient.getSigningKey(decoded.header.kid) + ).getPublicKey() as string + ), + { + name: 'RSASSA-PKCS1-v1_5', + hash: 'SHA-256', + }, + true, + ['verify'] + ); + }; + + /** Base initialization */ + + this.base = new Base( + importKey, + verifySignature, + decodeBase64, + loadCryptoKey + ); } // For use as singleton, always returns the same instance @@ -172,18 +201,19 @@ export default class Clerk extends ClerkBackendAPI { const cookies = new Cookies(req, res); try { - const { status, session, interstitial } = await nodeBase.getAuthState({ - cookieToken: cookies.get('__session') as string, - clientUat: cookies.get('__client_uat') as string, - headerToken: req.headers.authorization?.replace('Bearer ', ''), - origin: req.headers.origin, - host: req.headers.host, - forwardedPort: req.headers['x-forwarded-port'] as string, - forwardedHost: req.headers['x-forwarded-host'] as string, - referrer: req.headers.referer, - userAgent: req.headers['user-agent'] as string, - fetchInterstitial: () => this.fetchInterstitial(), - }); + const { status, session, interstitial, sessionClaims } = + await this.base.getAuthState({ + cookieToken: cookies.get('__session') as string, + clientUat: cookies.get('__client_uat') as string, + headerToken: req.headers.authorization?.replace('Bearer ', ''), + origin: req.headers.origin, + host: req.headers.host, + forwardedPort: req.headers['x-forwarded-port'] as string, + forwardedHost: req.headers['x-forwarded-host'] as string, + referrer: req.headers.referer, + userAgent: req.headers['user-agent'] as string, + fetchInterstitial: () => this.fetchInterstitial(), + }); if (status === AuthStatus.SignedOut) { return signedOut(); @@ -192,6 +222,8 @@ export default class Clerk extends ClerkBackendAPI { if (status === AuthStatus.SignedIn) { // @ts-ignore req.session = session; + // @ts-ignore + req.sessionClaims = sessionClaims; return next(); } diff --git a/packages/sdk-node/src/__tests__/Clerk.test.ts b/packages/sdk-node/src/__tests__/Clerk.test.ts index d70c3e41a33..a11cb0aaaf0 100644 --- a/packages/sdk-node/src/__tests__/Clerk.test.ts +++ b/packages/sdk-node/src/__tests__/Clerk.test.ts @@ -1,10 +1,3 @@ -import { AllowlistIdentifierApi } from '../apis/AllowlistIdentifierApi'; -import { ClientApi } from '../apis/ClientApi'; -import { EmailApi } from '../apis/EmailApi'; -import { InvitationApi } from '../apis/InvitationApi'; -import { SessionApi } from '../apis/SessionApi'; -import { SMSMessageApi } from '../apis/SMSMessageApi'; -import { UserApi } from '../apis/UserApi'; import Clerk from '../Clerk'; test('getInstance() getter returns a Clerk instance', () => { @@ -24,79 +17,8 @@ test('separate Clerk instances are not the same object', () => { expect(clerk2).not.toBe(clerk); }); -test('allowlistIdentifiers getter returns an AllowlistIdentifier API instance', () => { - const allowlistIdentifiers = Clerk.getInstance().allowlistIdentifiers; - expect(allowlistIdentifiers).toBeInstanceOf(AllowlistIdentifierApi); -}); - -test('allowlistIdentifiers getter returns the same instance every time', () => { +test('clerkInstance getter returns the same instance of a resource every time', () => { const allowlistIdentifiers = Clerk.getInstance().allowlistIdentifiers; const allowlistIdentifiers2 = Clerk.getInstance().allowlistIdentifiers; expect(allowlistIdentifiers2).toBe(allowlistIdentifiers); }); - -test('clients getter returns a Client API instance', () => { - const clients = Clerk.getInstance().clients; - expect(clients).toBeInstanceOf(ClientApi); -}); - -test('clients getter returns the same instance every time', () => { - const clients = Clerk.getInstance().clients; - const clients2 = Clerk.getInstance().clients; - expect(clients2).toBe(clients); -}); - -test('emails getter returns a Email API instance', () => { - const emails = Clerk.getInstance().emails; - expect(emails).toBeInstanceOf(EmailApi); -}); - -test('emails getter returns the same instance every time', () => { - const emails = Clerk.getInstance().emails; - const emails2 = Clerk.getInstance().emails; - expect(emails2).toBe(emails); -}); - -test('invitations getter returns an Invation API instance', () => { - const invitations = Clerk.getInstance().invitations; - expect(invitations).toBeInstanceOf(InvitationApi); -}); - -test('invitations getter returns the same instance every time', () => { - const invitations = Clerk.getInstance().invitations; - const invitations2 = Clerk.getInstance().invitations; - expect(invitations2).toBe(invitations); -}); - -test('sessions getter returns a Session API instance', () => { - const sessions = Clerk.getInstance().sessions; - expect(sessions).toBeInstanceOf(SessionApi); -}); - -test('sessions getter returns the same instance every time', () => { - const sessions = Clerk.getInstance().sessions; - const sessions2 = Clerk.getInstance().sessions; - expect(sessions2).toBe(sessions); -}); - -test('smsMessages getter returns an smsMessage API instance', () => { - const smsMessages = Clerk.getInstance().smsMessages; - expect(smsMessages).toBeInstanceOf(SMSMessageApi); -}); - -test('smsMessages getter returns the same instance every time', () => { - const smsMessages = Clerk.getInstance().smsMessages; - const smsMessages2 = Clerk.getInstance().smsMessages; - expect(smsMessages2).toBe(smsMessages); -}); - -test('users getter returns a User api instance', () => { - const users = Clerk.getInstance().users; - expect(users).toBeInstanceOf(UserApi); -}); - -test('users getter returns the same instance every time', () => { - const users = Clerk.getInstance().users; - const users2 = Clerk.getInstance().users; - expect(users2).toBe(users); -}); diff --git a/packages/sdk-node/src/__tests__/middleware.test.ts b/packages/sdk-node/src/__tests__/middleware.test.ts index cf43b231b44..f828e6e38bd 100644 --- a/packages/sdk-node/src/__tests__/middleware.test.ts +++ b/packages/sdk-node/src/__tests__/middleware.test.ts @@ -1,3 +1,4 @@ +import { AuthStatus } from '@clerk/backend-core'; import type { NextFunction, Request, Response } from 'express'; import jwt from 'jsonwebtoken'; @@ -5,12 +6,19 @@ import Clerk from '../Clerk'; const mockNext = jest.fn(); -const mockClaims = { +const mockAuthStateClaims = { iss: 'https://clerk.issuer', sub: 'subject', sid: 'session_id', }; -const mockToken = jwt.sign(mockClaims, 'mock-secret'); + +const mockAuthState = { + sessionClaims: mockAuthStateClaims, + session: { id: mockAuthStateClaims.sid, userId: mockAuthStateClaims.sub }, + status: AuthStatus.SignedIn, +}; + +const mockToken = jwt.sign(mockAuthStateClaims, 'mock-secret'); afterEach(() => { mockNext.mockReset(); @@ -53,16 +61,16 @@ test('expressWithSession with Authorization header', async () => { const res = {} as Response; const clerk = Clerk.getInstance(); - clerk.verifyToken = jest.fn().mockReturnValue(mockClaims); + clerk.base.getAuthState = jest.fn().mockReturnValueOnce(mockAuthState); await clerk.expressWithSession()(req, res, mockNext as NextFunction); // @ts-ignore - expect(req.sessionClaims).toEqual(mockClaims); + expect(req.sessionClaims).toEqual(mockAuthStateClaims); // @ts-ignore - expect(req.session.id).toEqual(mockClaims.sid); + expect(req.session.id).toEqual(mockAuthStateClaims.sid); // @ts-ignore - expect(req.session.userId).toEqual(mockClaims.sub); + expect(req.session.userId).toEqual(mockAuthStateClaims.sub); expect(mockNext).toHaveBeenCalledWith(); // 0 args }); @@ -73,16 +81,16 @@ test('expressWithSession with Authorization header in Bearer format', async () = const res = {} as Response; const clerk = Clerk.getInstance(); - clerk.verifyToken = jest.fn().mockReturnValue(mockClaims); + clerk.base.getAuthState = jest.fn().mockReturnValueOnce(mockAuthState); await clerk.expressWithSession()(req, res, mockNext as NextFunction); // @ts-ignore - expect(req.sessionClaims).toEqual(mockClaims); + expect(req.sessionClaims).toEqual(mockAuthStateClaims); // @ts-ignore - expect(req.session.id).toEqual(mockClaims.sid); + expect(req.session.id).toEqual(mockAuthStateClaims.sid); // @ts-ignore - expect(req.session.userId).toEqual(mockClaims.sub); + expect(req.session.userId).toEqual(mockAuthStateClaims.sub); expect(mockNext).toHaveBeenCalledWith(); // 0 args }); diff --git a/packages/sdk-node/src/info.ts b/packages/sdk-node/src/info.ts index be7eb79f676..ae33cbcf6ea 100644 --- a/packages/sdk-node/src/info.ts +++ b/packages/sdk-node/src/info.ts @@ -1,4 +1,4 @@ /** DO NOT EDIT: This file is automatically generated by ../scripts/info.js */ -export const LIB_VERSION="2.2.1"; +export const LIB_VERSION="2.6.0"; export const LIB_NAME="@clerk/clerk-sdk-node"; diff --git a/yarn.lock b/yarn.lock index e9816b03d43..3feff9f2b7a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -465,6 +465,7 @@ __metadata: "@typescript-eslint/parser": ^5.5.0 camelcase-keys: ^6.2.2 cookies: ^0.8.0 + deepmerge: ^4.2.2 eslint: ^8.3.0 eslint-config-prettier: ^8.3.0 got: ^11.8.2 From 7699fa938190994f114aa4d258f15569e4dd9e2d Mon Sep 17 00:00:00 2001 From: Peter Perlepes Date: Tue, 18 Jan 2022 11:06:48 +0200 Subject: [PATCH 2/2] chore(clerk-sdk-node): Some latest additions --- packages/sdk-node/package.json | 2 +- packages/sdk-node/src/Clerk.ts | 31 ++++++++++++++++--------------- packages/sdk-node/src/info.ts | 2 +- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/packages/sdk-node/package.json b/packages/sdk-node/package.json index 7157e770670..27e882c26a1 100644 --- a/packages/sdk-node/package.json +++ b/packages/sdk-node/package.json @@ -1,5 +1,5 @@ { - "version": "2.6.0", + "version": "2.6.2", "license": "MIT", "main": "dist/index.js", "module": "esm/index.js", diff --git a/packages/sdk-node/src/Clerk.ts b/packages/sdk-node/src/Clerk.ts index 4e29bc0bac4..ba7537d5c29 100644 --- a/packages/sdk-node/src/Clerk.ts +++ b/packages/sdk-node/src/Clerk.ts @@ -71,27 +71,24 @@ export default class Clerk extends ClerkBackendAPI { apiKey?: string; serverApiUrl?: string; apiVersion?: string; - httpOptions?: object; + httpOptions?: OptionsOfJSONResponseBody; jwksCacheMaxAge?: number; } = {}) { const fetcher: ClerkFetcher = ( url, { method, authorization, contentType, userAgent, body } ) => { - const finalHTTPOptions = deepmerge( - { - method, - responseType: 'json', - headers: { - authorization, - 'Content-Type': contentType, - 'User-Agent': userAgent, - }, - // @ts-ignore - ...(body && { body: querystring.stringify(body) }), + const finalHTTPOptions = deepmerge(httpOptions, { + method, + responseType: 'json', + headers: { + authorization, + 'Content-Type': contentType, + 'User-Agent': userAgent, }, - httpOptions - ) as OptionsOfJSONResponseBody; + // @ts-ignore + ...(body && { body: querystring.stringify(body) }), + }) as OptionsOfJSONResponseBody; return got(url, finalHTTPOptions); }; @@ -282,7 +279,7 @@ export default class Clerk extends ClerkBackendAPI { return async ( req: WithSessionProp | WithSessionClaimsProp, res: Response, - next: NextFunction + next?: NextFunction ) => { try { await this._runMiddleware( @@ -313,4 +310,8 @@ export default class Clerk extends ClerkBackendAPI { ) { return this.withSession(handler, { onError }); } + + set httpOptions(value: OptionsOfJSONResponseBody) { + this.httpOptions = value; + } } diff --git a/packages/sdk-node/src/info.ts b/packages/sdk-node/src/info.ts index ae33cbcf6ea..e2b38426038 100644 --- a/packages/sdk-node/src/info.ts +++ b/packages/sdk-node/src/info.ts @@ -1,4 +1,4 @@ /** DO NOT EDIT: This file is automatically generated by ../scripts/info.js */ -export const LIB_VERSION="2.6.0"; +export const LIB_VERSION="2.6.2"; export const LIB_NAME="@clerk/clerk-sdk-node";