Skip to content

Commit ed2d1b0

Browse files
authored
refactor(bearertoken): reduce re-auth by increasing polling rate. (aws#6882)
## Problem we have an [internal that runs every 5 minutes ](https://github.com/aws/aws-toolkit-vscode/blob/c3ea31dbd68cfc231b3f81795864f9dbc8a5e488/packages/amazonq/src/lsp/client.ts#L113) to take the current bearer token in the extension and send it to the language server. This is attempting to solve the problem of ensuring the bearer token used by the codewhisperer-LSP has not expired. The difficulty is that the codewhisperer-LSP does not know when it is expired so it is the responsibility of the client(us) to ensure this token has not expired. We currently do not passively refresh this token, but actively refresh it on request (see here: https://github.com/aws/aws-toolkit-vscode/blob/c3ea31dbd68cfc231b3f81795864f9dbc8a5e488/packages/core/src/auth/sso/ssoAccessTokenProvider.ts#L100-L122). Therefore, we do not know if it has refreshed until we request it. The current solution is to get and send the bearer token to the lsp every 5 minutes. However, if the token expires in the middle of these intervals, there is then a 1-5 minute period where if the customer interacts with anything using the codewhisperer-lsp, they will be prompted to re-auth unnecessarily. The issue can be observed by manually poisoning the bearerToken used by the codewhisper lsp with something like: ``` void sleep(delay).then(async () => { getLogger().info('Updating bearer token to bad token') await auth.updateBearerToken('bad Token') }) ``` Then, any lsp interaction will prompt for re-auth (until next interval 5 minute interval hits and token is refreshed). Note that manually expiring the token locally does not work, since it is still valid for requests on the LSP. ## Solution - increase polling frequency to 1 minute by making use of a local cache to avoid excessive LSP requests. This means a call is made to `getToken` (linked above) on a regular interval. However, if its cached, this is very cheap operation. - once Auth is using flare, we can implement a proper fix in Flare. Early idea is to add notifications for when Flare refreshes tokens, and use those as a signal to update the token used by the other LSP servers. - Document the problem and potential long term fixes for once Auth LSP is being used (WIP) - Refactored `auth.init -> auth.refreshConnection` to be more specific. ## Verification - Using the code above to observe the issue, we can see that after poisoning the bearer token used by the LSP, once the first `UpdateBearerToken: {request}` log message shows up (from: https://github.com/aws/aws-toolkit-vscode/blob/ce01fa85c7a97c71cd3dcc11770f6cb75b29e1c2/packages/amazonq/src/lsp/auth.ts#L83), flare interactions work again. ## Alternative Solutions - Add passive refreshing to current Auth. - This is not a great approach because it would be specialized to our current Auth logic, which is being discarded in favor of Flare. - Retry failed requests with refreshed token. - The implementation would be very awkward here and tightly couples all lsp server interactions to our auth logic. This is also against the current paradigm of the LSP responding with a re-auth prompt when authentication fails. - Update bearer token before every request. - This seems excessive and messy, and would likely be throwaway work. The number of LSP requests would double, potentially increasing latency for flare operations. Since this is an edge case, it does not seem a worthy tradeoff to degrade the common use case. --- - 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.
1 parent a97614c commit ed2d1b0

File tree

5 files changed

+56
-23
lines changed

5 files changed

+56
-23
lines changed

packages/amazonq/src/lsp/auth.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import * as crypto from 'crypto'
1414
import { LanguageClient } from 'vscode-languageclient'
1515
import { AuthUtil } from 'aws-core-vscode/codewhisperer'
1616
import { Writable } from 'stream'
17+
import { onceChanged } from 'aws-core-vscode/utils'
18+
import { getLogger, oneMinute } from 'aws-core-vscode/shared'
1719

1820
export const encryptionKey = crypto.randomBytes(32)
1921

@@ -64,7 +66,7 @@ export const notificationTypes = {
6466
export class AmazonQLspAuth {
6567
constructor(private readonly client: LanguageClient) {}
6668

67-
async init() {
69+
async refreshConnection() {
6870
const activeConnection = AuthUtil.instance.auth.activeConnection
6971
if (activeConnection?.type === 'sso') {
7072
// send the token to the language server
@@ -73,7 +75,8 @@ export class AmazonQLspAuth {
7375
}
7476
}
7577

76-
private async updateBearerToken(token: string) {
78+
public updateBearerToken = onceChanged(this._updateBearerToken.bind(this))
79+
private async _updateBearerToken(token: string) {
7780
const request = await this.createUpdateCredentialsRequest({
7881
token,
7982
})
@@ -83,6 +86,16 @@ export class AmazonQLspAuth {
8386
this.client.info(`UpdateBearerToken: ${JSON.stringify(request)}`)
8487
}
8588

89+
public startTokenRefreshInterval(pollingTime: number = oneMinute) {
90+
const interval = setInterval(async () => {
91+
await this.refreshConnection().catch((e) => {
92+
getLogger('amazonqLsp').error('Unable to update bearer token: %s', (e as Error).message)
93+
clearInterval(interval)
94+
})
95+
}, pollingTime)
96+
return interval
97+
}
98+
8699
private async createUpdateCredentialsRequest(data: any) {
87100
const payload = new TextEncoder().encode(JSON.stringify({ data }))
88101

packages/amazonq/src/lsp/client.ts

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,7 @@ import { InlineCompletionManager } from '../app/inline/completion'
1111
import { AmazonQLspAuth, encryptionKey, notificationTypes } from './auth'
1212
import { AuthUtil } from 'aws-core-vscode/codewhisperer'
1313
import { ConnectionMetadata } from '@aws/language-server-runtimes/protocol'
14-
import {
15-
Settings,
16-
oidcClientName,
17-
createServerOptions,
18-
globals,
19-
Experiments,
20-
getLogger,
21-
Commands,
22-
} from 'aws-core-vscode/shared'
14+
import { Settings, oidcClientName, createServerOptions, globals, Experiments, Commands } from 'aws-core-vscode/shared'
2315
import { activate } from './chat/activation'
2416
import { AmazonQResourcePaths } from './lspInstaller'
2517

@@ -109,7 +101,7 @@ export async function startLanguageServer(
109101
},
110102
}
111103
})
112-
await auth.init()
104+
await auth.refreshConnection()
113105

114106
if (Experiments.instance.get('amazonqLSPInline', false)) {
115107
const inlineManager = new InlineCompletionManager(client)
@@ -129,23 +121,16 @@ export async function startLanguageServer(
129121
activate(client, encryptionKey, resourcePaths.ui)
130122
}
131123

132-
// Temporary code for pen test. Will be removed when we switch to the real flare auth
133-
const authInterval = setInterval(async () => {
134-
try {
135-
await auth.init()
136-
} catch (e) {
137-
getLogger('amazonqLsp').error('Unable to update bearer token: %s', (e as Error).message)
138-
clearInterval(authInterval)
139-
}
140-
}, 300000) // every 5 minutes
124+
const refreshInterval = auth.startTokenRefreshInterval()
141125

142126
toDispose.push(
143127
AuthUtil.instance.auth.onDidChangeActiveConnection(async () => {
144-
await auth.init()
128+
await auth.refreshConnection()
145129
}),
146130
AuthUtil.instance.auth.onDidDeleteConnection(async () => {
147131
client.sendNotification(notificationTypes.deleteBearerToken.method)
148-
})
132+
}),
133+
{ dispose: () => clearInterval(refreshInterval) }
149134
)
150135
})
151136
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
import assert from 'assert'
6+
import { AmazonQLspAuth } from '../../../../src/lsp/auth'
7+
import { LanguageClient } from 'vscode-languageclient'
8+
9+
describe('AmazonQLspAuth', function () {
10+
describe('updateBearerToken', function () {
11+
it('makes request to LSP when token changes', async function () {
12+
// Note: this token will be encrypted
13+
let lastSentToken = {}
14+
const auth = new AmazonQLspAuth({
15+
sendRequest: (_method: string, param: any) => {
16+
lastSentToken = param
17+
},
18+
info: (_message: string, _data: any) => {},
19+
} as LanguageClient)
20+
21+
await auth.updateBearerToken('firstToken')
22+
assert.notDeepStrictEqual(lastSentToken, {})
23+
const encryptedFirstToken = lastSentToken
24+
25+
await auth.updateBearerToken('secondToken')
26+
assert.notDeepStrictEqual(lastSentToken, encryptedFirstToken)
27+
const encryptedSecondToken = lastSentToken
28+
29+
await auth.updateBearerToken('secondToken')
30+
assert.deepStrictEqual(lastSentToken, encryptedSecondToken)
31+
})
32+
})
33+
})

packages/core/src/shared/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,4 @@ export * from './lsp/utils/platform'
7272
export * as processUtils from './utilities/processUtils'
7373
export * as BaseLspInstaller from './lsp/baseLspInstaller'
7474
export * as collectionUtil from './utilities/collectionUtils'
75+
export * from './datetime'

packages/core/src/shared/utilities/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@
55

66
export { isExtensionInstalled, isExtensionActive } from './vsCodeUtils'
77
export { VSCODE_EXTENSION_ID } from '../extensions'
8+
export * from './functionUtils'

0 commit comments

Comments
 (0)