diff --git a/package-lock.json b/package-lock.json index b3730077..fbae912b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,11 +13,13 @@ "abort-controller": "^3.0.0", "agentkeepalive": "^4.5.0", "json-bigint": "^1.0.0", - "node-fetch": "^2.6.6" + "node-fetch": "^2.6.6", + "rwlock": "^5.0.0" }, "devDependencies": { "@types/jest": "^29.5.12", "@types/node-fetch": "^2.5.12", + "@types/rwlock": "^5.0.6", "@typescript-eslint/eslint-plugin": "^5.4.0", "@typescript-eslint/parser": "^5.4.0", "allure-commandline": "^2.29.0", @@ -1459,6 +1461,13 @@ "integrity": "sha512-Gj7cI7z+98M282Tqmp2K5EIsoouUEzbBJhQQzDE3jSIRk6r9gsz0oUokqIUR4u1R3dMHo0pDHM7sNOHyhulypw==", "dev": true }, + "node_modules/@types/rwlock": { + "version": "5.0.6", + "resolved": "https://registry.npmjs.org/@types/rwlock/-/rwlock-5.0.6.tgz", + "integrity": "sha512-JcjZjGATVC+LoLd910jIEkbSVQpvxRvAhHqIYCl9sf0qvKp+m2ps/WfthQRqWUcE38pgiD6YqEQ5tK4oSNK1XA==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/set-cookie-parser": { "version": "2.4.2", "resolved": "https://registry.npmjs.org/@types/set-cookie-parser/-/set-cookie-parser-2.4.2.tgz", @@ -7649,6 +7658,12 @@ "queue-microtask": "^1.2.2" } }, + "node_modules/rwlock": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/rwlock/-/rwlock-5.0.0.tgz", + "integrity": "sha512-XgzRqLMfCcm9QfZuPav9cV3Xin5TRcIlp4X/SH3CvB+x5D2AakdlEepfJKDd8ByncvfpcxNWdRZVUl38PS6ZJg==", + "license": "MIT" + }, "node_modules/rxjs": { "version": "7.5.6", "resolved": "https://registry.npmjs.org/rxjs/-/rxjs-7.5.6.tgz", @@ -9930,6 +9945,12 @@ "integrity": "sha512-Gj7cI7z+98M282Tqmp2K5EIsoouUEzbBJhQQzDE3jSIRk6r9gsz0oUokqIUR4u1R3dMHo0pDHM7sNOHyhulypw==", "dev": true }, + "@types/rwlock": { + "version": "5.0.6", + "resolved": "https://registry.npmjs.org/@types/rwlock/-/rwlock-5.0.6.tgz", + "integrity": "sha512-JcjZjGATVC+LoLd910jIEkbSVQpvxRvAhHqIYCl9sf0qvKp+m2ps/WfthQRqWUcE38pgiD6YqEQ5tK4oSNK1XA==", + "dev": true + }, "@types/set-cookie-parser": { "version": "2.4.2", "resolved": "https://registry.npmjs.org/@types/set-cookie-parser/-/set-cookie-parser-2.4.2.tgz", @@ -14480,6 +14501,11 @@ "queue-microtask": "^1.2.2" } }, + "rwlock": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/rwlock/-/rwlock-5.0.0.tgz", + "integrity": "sha512-XgzRqLMfCcm9QfZuPav9cV3Xin5TRcIlp4X/SH3CvB+x5D2AakdlEepfJKDd8ByncvfpcxNWdRZVUl38PS6ZJg==" + }, "rxjs": { "version": "7.5.6", "resolved": "https://registry.npmjs.org/rxjs/-/rxjs-7.5.6.tgz", diff --git a/package.json b/package.json index fc6e1e88..6c5d2ad8 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "devDependencies": { "@types/jest": "^29.5.12", "@types/node-fetch": "^2.5.12", + "@types/rwlock": "^5.0.6", "@typescript-eslint/eslint-plugin": "^5.4.0", "@typescript-eslint/parser": "^5.4.0", "allure-commandline": "^2.29.0", @@ -46,6 +47,7 @@ "abort-controller": "^3.0.0", "agentkeepalive": "^4.5.0", "json-bigint": "^1.0.0", - "node-fetch": "^2.6.6" + "node-fetch": "^2.6.6", + "rwlock": "^5.0.0" } } diff --git a/src/auth/index.ts b/src/auth/index.ts index 9e288f6f..f8dc1ae2 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -7,6 +7,7 @@ import { UsernamePasswordAuth } from "../types"; import { TokenKey, inMemoryCache, noneCache } from "../common/tokenCache"; +import ReadWriteLock from "rwlock"; type Login = { access_token: string; @@ -22,11 +23,16 @@ export class Authenticator { options: ConnectionOptions; accessToken?: string; + private rwlock = new ReadWriteLock(); constructor(context: Context, options: ConnectionOptions) { - context.httpClient.authenticator = this; this.context = context; this.options = options; + if (context.httpClient.authenticator) { + return context.httpClient.authenticator; + } else { + context.httpClient.authenticator = this; + } } private getCacheKey(): TokenKey | undefined { @@ -157,21 +163,66 @@ export class Authenticator { ); } - async authenticate() { - const options = this.options.auth || this.options; - const cachedToken = this.getCachedToken(); + async authenticate(): Promise { + // Try to get token from cache using read lock + const cachedToken = await this.tryGetCachedToken(); if (cachedToken) { this.accessToken = cachedToken; return; } + // No cached token, acquire write lock and authenticate + await this.acquireWriteLockAndAuthenticate(); + } + + private async tryGetCachedToken(): Promise { + return new Promise((resolve, reject) => { + this.rwlock.readLock(releaseReadLock => { + try { + const cachedToken = this.getCachedToken(); + releaseReadLock(); + resolve(cachedToken); + } catch (error) { + releaseReadLock(); + reject(error); + } + }); + }); + } + + private async acquireWriteLockAndAuthenticate(): Promise { + return new Promise((resolve, reject) => { + this.rwlock.writeLock(async releaseWriteLock => { + try { + // Double-check cache in case another thread authenticated while waiting + const cachedToken = this.getCachedToken(); + if (cachedToken) { + this.accessToken = cachedToken; + releaseWriteLock(); + return resolve(); + } + + await this.performAuthentication(); + + releaseWriteLock(); + resolve(); + } catch (error) { + releaseWriteLock(); + reject(error); + } + }); + }); + } + + private async performAuthentication(): Promise { + const options = this.options.auth || this.options; + if (this.isUsernamePassword()) { - await this.authenticateUsernamePassword(options as UsernamePasswordAuth); - return; + return this.authenticateUsernamePassword(options as UsernamePasswordAuth); } + if (this.isServiceAccount()) { - await this.authenticateServiceAccount(options as ServiceAccountAuth); - return; + return this.authenticateServiceAccount(options as ServiceAccountAuth); } throw new Error("Please provide valid auth credentials"); diff --git a/test/unit/http.test.ts b/test/unit/http.test.ts index 6228ae76..f47985b9 100644 --- a/test/unit/http.test.ts +++ b/test/unit/http.test.ts @@ -136,7 +136,7 @@ describe.each([ ] ])("token caching %s", (_, authUrl: string, auth: AuthOptions) => { const server = setupServer(); - const httpClient = new NodeHttpClient(); + let httpClient = new NodeHttpClient(); beforeAll(() => { server.listen(); @@ -144,6 +144,9 @@ describe.each([ afterAll(() => { server.close(); }); + beforeEach(() => { + httpClient = new NodeHttpClient(); + }); it("caches and reuses access token", async () => { const authenticator = new Authenticator( @@ -295,10 +298,11 @@ describe.each([ useCache: true } ); - + // Different Authenticator has to have different httpClient + const httpClient2 = new NodeHttpClient(); const authenticator2 = new Authenticator( { - httpClient, + httpClient: httpClient2, apiEndpoint: "api.fake2.firebolt.io", logger }, @@ -342,4 +346,37 @@ describe.each([ expect(authenticator2.accessToken).toEqual("fake_access_token"); expect(calls).toEqual(2); }); + it("does not overwrite token when run concurrently", async () => { + const authenticator = new Authenticator( + { httpClient, apiEndpoint, logger }, + { + auth, + account: "my_account", + useCache: true + } + ); + + let calls = 0; + + server.use( + rest.post(authUrl, (req, res, ctx) => { + calls++; + return res( + ctx.json({ + access_token: "fake_access_token", + expires_in: 2 ^ 30 + }) + ); + }) + ); + + authenticator.clearCache(); + + const promises = Array(10) + .fill(null) + .map(() => authenticator.authenticate()); + + await Promise.all(promises); + expect(calls).toEqual(1); + }); });