diff --git a/packages/core/src/auth/sso/ssoAccessTokenProvider.ts b/packages/core/src/auth/sso/ssoAccessTokenProvider.ts index 6639e9f3832..d0c8af56c3f 100644 --- a/packages/core/src/auth/sso/ssoAccessTokenProvider.ts +++ b/packages/core/src/auth/sso/ssoAccessTokenProvider.ts @@ -33,7 +33,7 @@ import { randomUUID } from '../../shared/crypto' import { getExtRuntimeContext } from '../../shared/vscode/env' import { showInputBox } from '../../shared/ui/inputPrompter' import { AmazonQPromptSettings, DevSettings, PromptSettings, ToolkitPromptSettings } from '../../shared/settings' -import { onceChanged } from '../../shared/utilities/functionUtils' +import { debounce, onceChanged } from '../../shared/utilities/functionUtils' import { NestedMap } from '../../shared/utilities/map' import { asStringifiedStack } from '../../shared/telemetry/spans' import { showViewLogsMessage } from '../../shared/utilities/messages' @@ -97,7 +97,20 @@ export abstract class SsoAccessTokenProvider { this.reAuthState.set(this.profile, { reAuthReason: `invalidate():${reason}` }) } + /** + * Sometimes we get many calls at once and this + * can trigger redundant disk reads, or token refreshes. + * We debounce to avoid this. + * + * NOTE: The property {@link getTokenDebounced()} does not work with being stubbed for tests, so + * this redundant function was created to work around that. + */ public async getToken(): Promise { + return this.getTokenDebounced() + } + private getTokenDebounced = debounce(() => this._getToken(), 50) + /** Exposed for testing purposes only */ + public async _getToken(): Promise { const data = await this.cache.token.load(this.tokenCacheKey) SsoAccessTokenProvider.logIfChanged( indent( diff --git a/packages/core/src/test/credentials/provider/sharedCredentialsProvider.test.ts b/packages/core/src/test/credentials/provider/sharedCredentialsProvider.test.ts index 8e266cc11d3..9f12d7c6c64 100644 --- a/packages/core/src/test/credentials/provider/sharedCredentialsProvider.test.ts +++ b/packages/core/src/test/credentials/provider/sharedCredentialsProvider.test.ts @@ -4,13 +4,11 @@ */ import assert from 'assert' -import * as FakeTimers from '@sinonjs/fake-timers' import * as sinon from 'sinon' import { SharedCredentialsProvider } from '../../../auth/providers/sharedCredentialsProvider' import { stripUndefined } from '../../../shared/utilities/collectionUtils' import * as process from '@aws-sdk/credential-provider-process' import { ParsedIniData } from '@smithy/shared-ini-file-loader' -import { installFakeClock } from '../../testUtil' import { SsoClient } from '../../../auth/sso/clients' import { stub } from '../../utilities/stubber' import { SsoAccessTokenProvider } from '../../../auth/sso/ssoAccessTokenProvider' @@ -19,20 +17,13 @@ import { createTestSections } from '../testUtil' const missingPropertiesFragment = 'missing properties' describe('SharedCredentialsProvider', async function () { - let clock: FakeTimers.InstalledClock let sandbox: sinon.SinonSandbox before(function () { sandbox = sinon.createSandbox() - clock = installFakeClock() - }) - - after(function () { - clock.uninstall() }) afterEach(function () { - clock.reset() sandbox.restore() }) diff --git a/packages/core/src/test/credentials/sso/ssoAccessTokenProvider.test.ts b/packages/core/src/test/credentials/sso/ssoAccessTokenProvider.test.ts index f552e8d08c8..6536095cd59 100644 --- a/packages/core/src/test/credentials/sso/ssoAccessTokenProvider.test.ts +++ b/packages/core/src/test/credentials/sso/ssoAccessTokenProvider.test.ts @@ -27,6 +27,7 @@ import { ToolkitError } from '../../../shared/errors' import * as fs from 'fs' // eslint-disable-line no-restricted-imports import * as path from 'path' import { Stub, stub } from '../../utilities/stubber' +import { globals } from '../../../shared' const hourInMs = 3600000 @@ -37,14 +38,14 @@ describe('SsoAccessTokenProvider', function () { let oidcClient: Stub let sut: SsoAccessTokenProvider let cache: ReturnType - let clock: FakeTimers.InstalledClock + let clock: FakeTimers.InstalledClock | undefined let tempDir: string let reAuthState: TestReAuthState function createToken(timeDelta: number, extras: Partial = {}) { return { accessToken: 'dummyAccessToken', - expiresAt: new clock.Date(clock.Date.now() + timeDelta), + expiresAt: new globals.clock.Date(globals.clock.Date.now() + timeDelta), ...extras, } } @@ -54,7 +55,7 @@ describe('SsoAccessTokenProvider', function () { scopes: [], clientId: 'dummyClientId', clientSecret: 'dummyClientSecret', - expiresAt: new clock.Date(clock.Date.now() + timeDelta), + expiresAt: new globals.clock.Date(globals.clock.Date.now() + timeDelta), startUrl, ...extras, } @@ -66,7 +67,7 @@ describe('SsoAccessTokenProvider', function () { deviceCode: 'dummyCode', userCode: 'dummyUserCode', verificationUri: 'dummyLink', - expiresAt: new clock.Date(clock.Date.now() + timeDelta), + expiresAt: new globals.clock.Date(globals.clock.Date.now() + timeDelta), } } @@ -77,14 +78,6 @@ describe('SsoAccessTokenProvider', function () { return cacheDir } - before(function () { - clock = installFakeClock() - }) - - after(function () { - clock.uninstall() - }) - beforeEach(async function () { oidcClient = stub(OidcClient) tempDir = await makeTemporaryTokenCacheFolder() @@ -95,7 +88,7 @@ describe('SsoAccessTokenProvider', function () { afterEach(async function () { sinon.restore() - clock.reset() + clock?.uninstall() await tryRemoveFolder(tempDir) }) @@ -163,6 +156,20 @@ describe('SsoAccessTokenProvider', function () { assert.strictEqual(cachedToken, undefined) }) + it('concurrent calls are debounced', async function () { + const validToken = createToken(hourInMs) + await cache.token.save(startUrl, { region, startUrl, token: validToken }) + const actualGetToken = sinon.spy(sut, '_getToken') + + const result = await Promise.all([sut.getToken(), sut.getToken(), sut.getToken()]) + + // Subsequent other calls were debounced so this was only called once + assert.strictEqual(actualGetToken.callCount, 1) + for (const r of result) { + assert.deepStrictEqual(r, validToken) + } + }) + describe('Exceptions', function () { it('drops expired tokens if failure was a client-fault', async function () { const exception = new UnauthorizedClientException({ message: '', $metadata: {} }) @@ -267,6 +274,7 @@ describe('SsoAccessTokenProvider', function () { }) it(`emits session duration between logins of the same startUrl`, async function () { + clock = installFakeClock() setupFlow() stubOpen() @@ -311,6 +319,7 @@ describe('SsoAccessTokenProvider', function () { }) it('respects the device authorization expiration time', async function () { + clock = installFakeClock() setupFlow() stubOpen() const exception = new AuthorizationPendingException({ message: '', $metadata: {} }) @@ -352,7 +361,7 @@ describe('SsoAccessTokenProvider', function () { const registration = { clientId: 'myExpiredClientId', clientSecret: 'myExpiredClientSecret', - expiresAt: new clock.Date(clock.Date.now() - 1), // expired date + expiresAt: new globals.clock.Date(globals.clock.Date.now() - 1), // expired date startUrl: key.startUrl, } await cache.registration.save(key, registration)