Skip to content

Commit 02f6d0b

Browse files
fix(auth): debounce getToken() function (#6282)
## Problem: The Identity team noticed a large spike in token refreshes for specific users. One user would trigger refresh over 50 times within a few seconds. Ticket: `P180886632` ## Solution: The telemetry showed that `getChatAuthState()` was being called many times in a short period. This eventually triggered the token refresh logic many times, if the token was expired. The solution is to add a debounce to `getToken()` which calls the refresh logic. - `debounce()` only accepts functions without any args, the refresh logic requires args - `getToken()` will also load from disk is the token is not expired, so debouncing here saves disk I/O as well. The current debounce interval is 100 milliseconds, which based on telemetry should be enough to capture the barrage of calls. With some manual testing it does not feel like UX is impacted in any noticeable way. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: nkomonen-amazon <[email protected]>
1 parent 26fe595 commit 02f6d0b

File tree

3 files changed

+37
-24
lines changed

3 files changed

+37
-24
lines changed

packages/core/src/auth/sso/ssoAccessTokenProvider.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import { randomUUID } from '../../shared/crypto'
3333
import { getExtRuntimeContext } from '../../shared/vscode/env'
3434
import { showInputBox } from '../../shared/ui/inputPrompter'
3535
import { AmazonQPromptSettings, DevSettings, PromptSettings, ToolkitPromptSettings } from '../../shared/settings'
36-
import { onceChanged } from '../../shared/utilities/functionUtils'
36+
import { debounce, onceChanged } from '../../shared/utilities/functionUtils'
3737
import { NestedMap } from '../../shared/utilities/map'
3838
import { asStringifiedStack } from '../../shared/telemetry/spans'
3939
import { showViewLogsMessage } from '../../shared/utilities/messages'
@@ -97,7 +97,20 @@ export abstract class SsoAccessTokenProvider {
9797
this.reAuthState.set(this.profile, { reAuthReason: `invalidate():${reason}` })
9898
}
9999

100+
/**
101+
* Sometimes we get many calls at once and this
102+
* can trigger redundant disk reads, or token refreshes.
103+
* We debounce to avoid this.
104+
*
105+
* NOTE: The property {@link getTokenDebounced()} does not work with being stubbed for tests, so
106+
* this redundant function was created to work around that.
107+
*/
100108
public async getToken(): Promise<SsoToken | undefined> {
109+
return this.getTokenDebounced()
110+
}
111+
private getTokenDebounced = debounce(() => this._getToken(), 50)
112+
/** Exposed for testing purposes only */
113+
public async _getToken(): Promise<SsoToken | undefined> {
101114
const data = await this.cache.token.load(this.tokenCacheKey)
102115
SsoAccessTokenProvider.logIfChanged(
103116
indent(

packages/core/src/test/credentials/provider/sharedCredentialsProvider.test.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,11 @@
44
*/
55

66
import assert from 'assert'
7-
import * as FakeTimers from '@sinonjs/fake-timers'
87
import * as sinon from 'sinon'
98
import { SharedCredentialsProvider } from '../../../auth/providers/sharedCredentialsProvider'
109
import { stripUndefined } from '../../../shared/utilities/collectionUtils'
1110
import * as process from '@aws-sdk/credential-provider-process'
1211
import { ParsedIniData } from '@smithy/shared-ini-file-loader'
13-
import { installFakeClock } from '../../testUtil'
1412
import { SsoClient } from '../../../auth/sso/clients'
1513
import { stub } from '../../utilities/stubber'
1614
import { SsoAccessTokenProvider } from '../../../auth/sso/ssoAccessTokenProvider'
@@ -19,20 +17,13 @@ import { createTestSections } from '../testUtil'
1917
const missingPropertiesFragment = 'missing properties'
2018

2119
describe('SharedCredentialsProvider', async function () {
22-
let clock: FakeTimers.InstalledClock
2320
let sandbox: sinon.SinonSandbox
2421

2522
before(function () {
2623
sandbox = sinon.createSandbox()
27-
clock = installFakeClock()
28-
})
29-
30-
after(function () {
31-
clock.uninstall()
3224
})
3325

3426
afterEach(function () {
35-
clock.reset()
3627
sandbox.restore()
3728
})
3829

packages/core/src/test/credentials/sso/ssoAccessTokenProvider.test.ts

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { ToolkitError } from '../../../shared/errors'
2727
import * as fs from 'fs' // eslint-disable-line no-restricted-imports
2828
import * as path from 'path'
2929
import { Stub, stub } from '../../utilities/stubber'
30+
import { globals } from '../../../shared'
3031

3132
const hourInMs = 3600000
3233

@@ -37,14 +38,14 @@ describe('SsoAccessTokenProvider', function () {
3738
let oidcClient: Stub<OidcClient>
3839
let sut: SsoAccessTokenProvider
3940
let cache: ReturnType<typeof getCache>
40-
let clock: FakeTimers.InstalledClock
41+
let clock: FakeTimers.InstalledClock | undefined
4142
let tempDir: string
4243
let reAuthState: TestReAuthState
4344

4445
function createToken(timeDelta: number, extras: Partial<SsoToken> = {}) {
4546
return {
4647
accessToken: 'dummyAccessToken',
47-
expiresAt: new clock.Date(clock.Date.now() + timeDelta),
48+
expiresAt: new globals.clock.Date(globals.clock.Date.now() + timeDelta),
4849
...extras,
4950
}
5051
}
@@ -54,7 +55,7 @@ describe('SsoAccessTokenProvider', function () {
5455
scopes: [],
5556
clientId: 'dummyClientId',
5657
clientSecret: 'dummyClientSecret',
57-
expiresAt: new clock.Date(clock.Date.now() + timeDelta),
58+
expiresAt: new globals.clock.Date(globals.clock.Date.now() + timeDelta),
5859
startUrl,
5960
...extras,
6061
}
@@ -66,7 +67,7 @@ describe('SsoAccessTokenProvider', function () {
6667
deviceCode: 'dummyCode',
6768
userCode: 'dummyUserCode',
6869
verificationUri: 'dummyLink',
69-
expiresAt: new clock.Date(clock.Date.now() + timeDelta),
70+
expiresAt: new globals.clock.Date(globals.clock.Date.now() + timeDelta),
7071
}
7172
}
7273

@@ -77,14 +78,6 @@ describe('SsoAccessTokenProvider', function () {
7778
return cacheDir
7879
}
7980

80-
before(function () {
81-
clock = installFakeClock()
82-
})
83-
84-
after(function () {
85-
clock.uninstall()
86-
})
87-
8881
beforeEach(async function () {
8982
oidcClient = stub(OidcClient)
9083
tempDir = await makeTemporaryTokenCacheFolder()
@@ -95,7 +88,7 @@ describe('SsoAccessTokenProvider', function () {
9588

9689
afterEach(async function () {
9790
sinon.restore()
98-
clock.reset()
91+
clock?.uninstall()
9992
await tryRemoveFolder(tempDir)
10093
})
10194

@@ -163,6 +156,20 @@ describe('SsoAccessTokenProvider', function () {
163156
assert.strictEqual(cachedToken, undefined)
164157
})
165158

159+
it('concurrent calls are debounced', async function () {
160+
const validToken = createToken(hourInMs)
161+
await cache.token.save(startUrl, { region, startUrl, token: validToken })
162+
const actualGetToken = sinon.spy(sut, '_getToken')
163+
164+
const result = await Promise.all([sut.getToken(), sut.getToken(), sut.getToken()])
165+
166+
// Subsequent other calls were debounced so this was only called once
167+
assert.strictEqual(actualGetToken.callCount, 1)
168+
for (const r of result) {
169+
assert.deepStrictEqual(r, validToken)
170+
}
171+
})
172+
166173
describe('Exceptions', function () {
167174
it('drops expired tokens if failure was a client-fault', async function () {
168175
const exception = new UnauthorizedClientException({ message: '', $metadata: {} })
@@ -267,6 +274,7 @@ describe('SsoAccessTokenProvider', function () {
267274
})
268275

269276
it(`emits session duration between logins of the same startUrl`, async function () {
277+
clock = installFakeClock()
270278
setupFlow()
271279
stubOpen()
272280

@@ -311,6 +319,7 @@ describe('SsoAccessTokenProvider', function () {
311319
})
312320

313321
it('respects the device authorization expiration time', async function () {
322+
clock = installFakeClock()
314323
setupFlow()
315324
stubOpen()
316325
const exception = new AuthorizationPendingException({ message: '', $metadata: {} })
@@ -352,7 +361,7 @@ describe('SsoAccessTokenProvider', function () {
352361
const registration = {
353362
clientId: 'myExpiredClientId',
354363
clientSecret: 'myExpiredClientSecret',
355-
expiresAt: new clock.Date(clock.Date.now() - 1), // expired date
364+
expiresAt: new globals.clock.Date(globals.clock.Date.now() - 1), // expired date
356365
startUrl: key.startUrl,
357366
}
358367
await cache.registration.save(key, registration)

0 commit comments

Comments
 (0)