From 32eba1b90b091659f22ea13a01078de71c65e6bb Mon Sep 17 00:00:00 2001 From: Patrick Temple Date: Thu, 4 Jun 2020 18:30:35 -0400 Subject: [PATCH 1/6] make the credentials a promise --- src/AsyncUtils.ts | 4 ++-- src/AuthorizedSocketConnection.ts | 6 +++--- src/CredentialsManager.ts | 2 +- src/JwtCredentialsManager.ts | 25 ++++++++++++++----------- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/AsyncUtils.ts b/src/AsyncUtils.ts index ba570837..ac816388 100644 --- a/src/AsyncUtils.ts +++ b/src/AsyncUtils.ts @@ -11,10 +11,10 @@ export async function* map( export async function* filter( iterable: AsyncIterable, - predicate: (value: T) => boolean, + predicate: (value: T) => Promise | boolean, ): AsyncGenerator { for await (const value of iterable) { - if (predicate(value)) yield value; + if (await predicate(value)) yield value; } } diff --git a/src/AuthorizedSocketConnection.ts b/src/AuthorizedSocketConnection.ts index 625439e3..661c9700 100644 --- a/src/AuthorizedSocketConnection.ts +++ b/src/AuthorizedSocketConnection.ts @@ -94,11 +94,11 @@ export default class AuthorizedSocketConnection { this.socket.emit('app_error', error); } - isAuthorized( + async isAuthorized( data: any, hasPermission: (data: any, credentials: TCredentials) => boolean, ) { - const credentials = this.config.credentialsManager.getCredentials(); + const credentials = await this.config.credentialsManager.getCredentials(); const isAuthorized = !!credentials && hasPermission(data, credentials); if (!isAuthorized) { this.log('info', 'unauthorized', { @@ -239,7 +239,7 @@ export default class AuthorizedSocketConnection { const stream: AsyncIterable = resultOrStream as any; for await (const payload of stream) { - const credentials = this.config.credentialsManager.getCredentials(); + const credentials = await this.config.credentialsManager.getCredentials(); let response; try { diff --git a/src/CredentialsManager.ts b/src/CredentialsManager.ts index dbe1f82c..86bc8306 100644 --- a/src/CredentialsManager.ts +++ b/src/CredentialsManager.ts @@ -1,5 +1,5 @@ export interface CredentialsManager { - getCredentials(): TCredentials | null | undefined; + getCredentials(): Promise; authenticate(authorization: string): void | Promise; unauthenticate(): void | Promise; // allow for redis etc down the line } diff --git a/src/JwtCredentialsManager.ts b/src/JwtCredentialsManager.ts index 1ca328de..59ab52b1 100644 --- a/src/JwtCredentialsManager.ts +++ b/src/JwtCredentialsManager.ts @@ -19,7 +19,7 @@ export default abstract class JwtCredentialsManager< token: string | null | undefined; - credentials: TCredentials | null | undefined; + credentials: Promise; renewHandle: NodeJS.Timeout | null | undefined; @@ -30,8 +30,10 @@ export default abstract class JwtCredentialsManager< this.credentials = null; } - getCredentials(): TCredentials | null | undefined { - const { credentials } = this; + async getCredentials(): Promise { + if (!this.credentials) return null; + + const credentials = await this.credentials; if (credentials && Date.now() >= credentials.exp * SECONDS_TO_MS) { return null; } @@ -65,21 +67,23 @@ export default abstract class JwtCredentialsManager< throw new Error('JwtCredentialManager: Unauthenticated'); } - const credentials = await this.getCredentialsFromAuthorization(token); + // need to handle multiple promises + this.credentials = Promise.resolve( + this.getCredentialsFromAuthorization(token), + ); + const resolvedCreds = await this.credentials; // Avoid race conditions with multiple updates. if (this.token !== token) { return; } - this.credentials = credentials; - // TODO: Don't schedule renewal if the new credentials are expired or // almost expired. - this.scheduleRenewCredentials(); + this.scheduleRenewCredentials(resolvedCreds); } - scheduleRenewCredentials() { + scheduleRenewCredentials(resolvedCreds: TCredentials | null | undefined) { if (this.renewHandle) { clearTimeout(this.renewHandle); } @@ -89,12 +93,11 @@ export default abstract class JwtCredentialsManager< return; } - const { credentials } = this; - if (!credentials) { + if (!resolvedCreds) { return; } - const deltaMs = credentials.exp * SECONDS_TO_MS - Date.now(); + const deltaMs = resolvedCreds.exp * SECONDS_TO_MS - Date.now(); const deltaMsAdjusted = Math.max( 0, deltaMs - tokenExpirationMarginSeconds * SECONDS_TO_MS, From 83605284f0b8e78ccdfb930ed19ba530aa14b7bc Mon Sep 17 00:00:00 2001 From: Patrick Temple Date: Thu, 4 Jun 2020 18:38:28 -0400 Subject: [PATCH 2/6] nit --- src/JwtCredentialsManager.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/JwtCredentialsManager.ts b/src/JwtCredentialsManager.ts index 59ab52b1..c020a9f3 100644 --- a/src/JwtCredentialsManager.ts +++ b/src/JwtCredentialsManager.ts @@ -67,11 +67,10 @@ export default abstract class JwtCredentialsManager< throw new Error('JwtCredentialManager: Unauthenticated'); } - // need to handle multiple promises this.credentials = Promise.resolve( this.getCredentialsFromAuthorization(token), ); - const resolvedCreds = await this.credentials; + await this.credentials; // Avoid race conditions with multiple updates. if (this.token !== token) { @@ -80,10 +79,10 @@ export default abstract class JwtCredentialsManager< // TODO: Don't schedule renewal if the new credentials are expired or // almost expired. - this.scheduleRenewCredentials(resolvedCreds); + this.scheduleRenewCredentials(); } - scheduleRenewCredentials(resolvedCreds: TCredentials | null | undefined) { + async scheduleRenewCredentials() { if (this.renewHandle) { clearTimeout(this.renewHandle); } @@ -93,11 +92,11 @@ export default abstract class JwtCredentialsManager< return; } - if (!resolvedCreds) { - return; - } + if (!this.credentials) return; + const resolvedCredentials = await this.credentials; + if (!resolvedCredentials) return; - const deltaMs = resolvedCreds.exp * SECONDS_TO_MS - Date.now(); + const deltaMs = resolvedCredentials.exp * SECONDS_TO_MS - Date.now(); const deltaMsAdjusted = Math.max( 0, deltaMs - tokenExpirationMarginSeconds * SECONDS_TO_MS, From da2faa81b0297d5db509220e45b2f9c3ca214b8a Mon Sep 17 00:00:00 2001 From: Patrick Temple Date: Fri, 5 Jun 2020 14:49:58 -0400 Subject: [PATCH 3/6] format --- src/JwtCredentialsManager.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/JwtCredentialsManager.ts b/src/JwtCredentialsManager.ts index c020a9f3..77288871 100644 --- a/src/JwtCredentialsManager.ts +++ b/src/JwtCredentialsManager.ts @@ -88,11 +88,10 @@ export default abstract class JwtCredentialsManager< } const { tokenExpirationMarginSeconds } = this.config; - if (tokenExpirationMarginSeconds === null) { + if (tokenExpirationMarginSeconds === null || !this.credentials) { return; } - if (!this.credentials) return; const resolvedCredentials = await this.credentials; if (!resolvedCredentials) return; From 2fdbf8b49920b1cdd5b60780d289f7a90b47f177 Mon Sep 17 00:00:00 2001 From: Patrick Temple Date: Mon, 8 Jun 2020 16:10:18 -0400 Subject: [PATCH 4/6] feedback --- src/JwtCredentialsManager.ts | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/JwtCredentialsManager.ts b/src/JwtCredentialsManager.ts index 77288871..a803a4aa 100644 --- a/src/JwtCredentialsManager.ts +++ b/src/JwtCredentialsManager.ts @@ -19,7 +19,7 @@ export default abstract class JwtCredentialsManager< token: string | null | undefined; - credentials: Promise; + credentialsPromise: Promise | null; renewHandle: NodeJS.Timeout | null | undefined; @@ -27,13 +27,13 @@ export default abstract class JwtCredentialsManager< this.config = config; this.token = null; - this.credentials = null; + this.credentialsPromise = null; } async getCredentials(): Promise { - if (!this.credentials) return null; + if (!this.credentialsPromise) return null; - const credentials = await this.credentials; + const credentials = await this.credentialsPromise; if (credentials && Date.now() >= credentials.exp * SECONDS_TO_MS) { return null; } @@ -58,7 +58,7 @@ export default abstract class JwtCredentialsManager< } this.token = null; - this.credentials = null; + this.credentialsPromise = null; } async updateCredentials() { @@ -67,10 +67,10 @@ export default abstract class JwtCredentialsManager< throw new Error('JwtCredentialManager: Unauthenticated'); } - this.credentials = Promise.resolve( + this.credentialsPromise = Promise.resolve( this.getCredentialsFromAuthorization(token), ); - await this.credentials; + await this.credentialsPromise; // Avoid race conditions with multiple updates. if (this.token !== token) { @@ -88,14 +88,16 @@ export default abstract class JwtCredentialsManager< } const { tokenExpirationMarginSeconds } = this.config; - if (tokenExpirationMarginSeconds === null || !this.credentials) { + if (tokenExpirationMarginSeconds === null) { return; } - const resolvedCredentials = await this.credentials; - if (!resolvedCredentials) return; + const credentials = await this.credentialsPromise; + if (!credentials) { + return; + } - const deltaMs = resolvedCredentials.exp * SECONDS_TO_MS - Date.now(); + const deltaMs = credentials.exp * SECONDS_TO_MS - Date.now(); const deltaMsAdjusted = Math.max( 0, deltaMs - tokenExpirationMarginSeconds * SECONDS_TO_MS, From 852ca482c6e1f78b8d106552e1d904410e2b5a2e Mon Sep 17 00:00:00 2001 From: Patrick Temple Date: Mon, 8 Jun 2020 16:17:46 -0400 Subject: [PATCH 5/6] nit --- src/JwtCredentialsManager.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/JwtCredentialsManager.ts b/src/JwtCredentialsManager.ts index a803a4aa..5c2439d7 100644 --- a/src/JwtCredentialsManager.ts +++ b/src/JwtCredentialsManager.ts @@ -31,8 +31,6 @@ export default abstract class JwtCredentialsManager< } async getCredentials(): Promise { - if (!this.credentialsPromise) return null; - const credentials = await this.credentialsPromise; if (credentials && Date.now() >= credentials.exp * SECONDS_TO_MS) { return null; From af90caf29e8021b8172e03997d433547b85c0f3c Mon Sep 17 00:00:00 2001 From: Patrick Temple Date: Mon, 8 Jun 2020 16:21:17 -0400 Subject: [PATCH 6/6] remove race condition guard --- src/JwtCredentialsManager.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/JwtCredentialsManager.ts b/src/JwtCredentialsManager.ts index 5c2439d7..d32ec299 100644 --- a/src/JwtCredentialsManager.ts +++ b/src/JwtCredentialsManager.ts @@ -70,11 +70,6 @@ export default abstract class JwtCredentialsManager< ); await this.credentialsPromise; - // Avoid race conditions with multiple updates. - if (this.token !== token) { - return; - } - // TODO: Don't schedule renewal if the new credentials are expired or // almost expired. this.scheduleRenewCredentials();