Skip to content

Commit ac9a78d

Browse files
authored
fix(auth): login state updates across windows (#5547)
Problem: Users can open multiple VSC sessions. If they sign out or the connection is lost in another window, then interacting with chat or in-line suggestions in the original window will only produce cryptic "Profile does not exist" errors, with no clear way to get back to a working state. Solution: Log out once this state is detected (uses chat, invokes manual or auto inline suggestion). 3 commits: [fix(amazonq): user stays signed in if they log out in other windows](270a321) Problem: Users can open multiple VSC sessions. If they sign out or the connection is lost in another window, then interacting with chat or in-line suggestions in the original window will only produce cryptic "Profile does not exist" errors, with no clear way to get back to a working state. Solution: Log out once this state is detected (uses chat, invokes manual or auto inline suggestion). [fix(amazonq): disable 'connection already exists' error for login page](5593784) Error would appear due to certain race conditions. If the auth page is visible then we know that we have no connections. (for amazon q only). [fix(auth): login state updates across windows](ced109f) - Uses a vscode filwatcher to detect changes to `.aws/sso/cache`. - Try to 'reconnect' on new changes based on stored state key. - Global state is stored across windows. - Login or log out will happen in all vsc instances. --- <!--- REMINDER: Ensure that your PR meets the guidelines in CONTRIBUTING.md --> License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent fdda3ca commit ac9a78d

File tree

10 files changed

+130
-21
lines changed

10 files changed

+130
-21
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "Auth: Login state not updating across multiple VS Code windows."
4+
}

packages/core/src/auth/auth.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { Credentials } from '@aws-sdk/types'
1414
import { SsoAccessTokenProvider } from './sso/ssoAccessTokenProvider'
1515
import { Timeout } from '../shared/utilities/timeoutUtils'
1616
import { errorCode, isAwsError, isNetworkError, ToolkitError, UnknownError } from '../shared/errors'
17-
import { getCache } from './sso/cache'
17+
import { getCache, getCacheFileWatcher } from './sso/cache'
1818
import { isNonNullable, Mutable } from '../shared/utilities/tsUtils'
1919
import { builderIdStartUrl, SsoToken, truncateStartUrl } from './sso/model'
2020
import { SsoClient } from './sso/clients'
@@ -58,6 +58,7 @@ import {
5858
scopesSsoAccountAccess,
5959
AwsConnection,
6060
scopesCodeWhispererCore,
61+
ProfileNotFoundError,
6162
} from './connection'
6263
import { isSageMaker, isCloud9, isAmazonQ } from '../shared/extensionUtilities'
6364
import { telemetry } from '../shared/telemetry/telemetry'
@@ -132,6 +133,7 @@ const authClassName = 'Auth'
132133

133134
export class Auth implements AuthService, ConnectionManager {
134135
readonly #ssoCache = getCache()
136+
readonly #ssoCacheWatcher = getCacheFileWatcher()
135137
readonly #validationErrors = new Map<Connection['id'], Error>()
136138
readonly #invalidCredentialsTimeouts = new Map<Connection['id'], Timeout>()
137139
readonly #onDidChangeActiveConnection = new vscode.EventEmitter<StatefulConnection | undefined>()
@@ -160,6 +162,10 @@ export class Auth implements AuthService, ConnectionManager {
160162
return this.store.listProfiles().length !== 0
161163
}
162164

165+
public get cacheWatcher() {
166+
return this.#ssoCacheWatcher
167+
}
168+
163169
/**
164170
* Map startUrl -> declared connections
165171
*/
@@ -878,8 +884,22 @@ export class Auth implements AuthService, ConnectionManager {
878884
@withTelemetryContext({ name: 'handleInvalidCredentials', class: authClassName })
879885
private async handleInvalidCredentials<T>(id: Connection['id'], refresh: () => Promise<T>): Promise<T> {
880886
getLogger().info(`auth: Handling invalid credentials of connection: ${id}`)
881-
const profile = this.store.getProfile(id)
882-
const previousState = profile?.metadata.connectionState
887+
888+
let profile: StoredProfile
889+
try {
890+
profile = this.store.getProfileOrThrow(id)
891+
} catch (err) {
892+
if (err instanceof ProfileNotFoundError) {
893+
getLogger().info(
894+
`Auth: deleting connection '${id}' due to error encountered while fetching auth token: %s`,
895+
err
896+
)
897+
await this.deleteConnection({ id })
898+
}
899+
throw err
900+
}
901+
902+
const previousState = profile.metadata.connectionState
883903
await this.updateConnectionState(id, 'invalid')
884904

885905
if (previousState === 'invalid') {
@@ -895,8 +915,7 @@ export class Auth implements AuthService, ConnectionManager {
895915
const timeout = new Timeout(60000)
896916
this.#invalidCredentialsTimeouts.set(id, timeout)
897917

898-
const connLabel =
899-
profile?.metadata.label ?? (profile?.type === 'sso' ? this.getSsoProfileLabel(profile) : id)
918+
const connLabel = profile.metadata.label ?? (profile.type === 'sso' ? this.getSsoProfileLabel(profile) : id)
900919
const message = localize(
901920
'aws.auth.invalidConnection',
902921
'Connection "{0}" is invalid or expired, login again?',

packages/core/src/auth/connection.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,12 @@ export interface ProfileMetadata {
201201

202202
export type StoredProfile<T extends Profile = Profile> = T & { readonly metadata: ProfileMetadata }
203203

204+
export class ProfileNotFoundError extends Error {
205+
public constructor(id: string) {
206+
super(`Profile does not exist: ${id}`)
207+
}
208+
}
209+
204210
function getTelemetryForProfile(profile: StoredProfile<Profile> | undefined) {
205211
if (!profile) {
206212
return {}
@@ -245,7 +251,7 @@ export class ProfileStore {
245251
try {
246252
profile = this.getProfile(id)
247253
if (profile === undefined) {
248-
throw new Error(`Profile does not exist: ${id}`)
254+
throw new ProfileNotFoundError(id)
249255
}
250256
} catch (err) {
251257
// Always emit failures

packages/core/src/auth/secondaryAuth.ts

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import * as vscode from 'vscode'
99
import { getLogger } from '../shared/logger'
1010
import { cast, Optional } from '../shared/utilities/typeConstructors'
1111
import { Auth } from './auth'
12-
import { once, onceChanged } from '../shared/utilities/functionUtils'
12+
import { onceChanged } from '../shared/utilities/functionUtils'
1313
import { isNonNullable } from '../shared/utilities/tsUtils'
1414
import { ToolIdStateKey } from '../shared/globalState'
1515
import { Connection, getTelemetryMetadataForConn, SsoConnection, StatefulConnection } from './connection'
@@ -147,6 +147,40 @@ export class SecondaryAuth<T extends Connection = Connection> {
147147
await this.clearSavedConnection()
148148
}
149149
})
150+
151+
const refreshConn = (event: string) => {
152+
getLogger().debug(`secondaryAuth: detected ${event} event in sso cache, refreshing auth.`)
153+
globals.clock.setTimeout(
154+
telemetry.function_call.run(
155+
() => async () => {
156+
if (this.#savedConnection?.id === this.getStateConnectionId()) {
157+
// Someone updated our cache but the global state doesn't indicate anything new, so do nothing.
158+
// (it could have been us that updated the cache)
159+
getLogger().debug(
160+
`secondaryAuth: cache event did not update global state, no refresh is needed.`
161+
)
162+
return
163+
}
164+
await this.auth.restorePreviousSession()
165+
await this.restoreConnection(true)
166+
},
167+
{
168+
emit: false,
169+
functionId: { name: 'cacheWatchCallback', class: secondaryAuthClassName },
170+
}
171+
),
172+
/**
173+
* The connection is first created and stored on disk, then it is stored in global state.
174+
* This creates a race condition for the callback, who listens to the disk event but
175+
* depends on the global state to make a connection decision. The time is arbitrary.
176+
*
177+
* TODO: fix this race condition.
178+
*/
179+
3000
180+
)
181+
}
182+
this.auth.cacheWatcher.onDidCreate(() => refreshConn('create'))
183+
this.auth.cacheWatcher.onDidDelete(() => refreshConn('delete'))
150184
}
151185

152186
public get activeConnection(): T | undefined {
@@ -250,11 +284,17 @@ export class SecondaryAuth<T extends Connection = Connection> {
250284
return await addScopes(conn, extraScopes, this.auth)
251285
}
252286

287+
private hasRunRestoreConnection = false
288+
253289
// Used to lazily restore persisted connections.
254290
// Kind of clunky. We need an async module loader layer to make things ergonomic.
255-
public readonly restoreConnection: typeof this._restoreConnection = once(async () => this._restoreConnection())
256291
@withTelemetryContext({ name: 'restoreConnection', class: secondaryAuthClassName })
257-
private async _restoreConnection(): Promise<T | undefined> {
292+
public async restoreConnection(force: boolean = false): Promise<T | undefined> {
293+
if (!force && (this.activeConnection !== undefined || this.hasRunRestoreConnection)) {
294+
return
295+
}
296+
this.hasRunRestoreConnection = true
297+
258298
try {
259299
return await telemetry.auth_modifyConnection.run(async (span) => {
260300
span.record({
@@ -289,7 +329,7 @@ export class SecondaryAuth<T extends Connection = Connection> {
289329
// TODO: fix this
290330
// eslint-disable-next-line aws-toolkits/no-banned-usages
291331
const globalState = globals.context.globalState
292-
const id = cast(globalState.get(this.key), Optional(String))
332+
const id = this.getStateConnectionId()
293333
if (id === undefined) {
294334
return
295335
}
@@ -335,6 +375,10 @@ export class SecondaryAuth<T extends Connection = Connection> {
335375
return conn
336376
}
337377
}
378+
379+
private getStateConnectionId() {
380+
return cast(globals.globalState.get(this.key), Optional(String))
381+
}
338382
}
339383

340384
/**

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6+
import * as vscode from 'vscode'
67
import * as crypto from 'crypto'
78
import * as path from 'path'
89
import { getLogger } from '../../shared/logger/logger'
@@ -13,6 +14,7 @@ import { hasProps, selectFrom } from '../../shared/utilities/tsUtils'
1314
import { SsoToken, ClientRegistration } from './model'
1415
import { DevSettings } from '../../shared/settings'
1516
import { onceChanged } from '../../shared/utilities/functionUtils'
17+
import globals from '../../shared/extensionGlobals'
1618

1719
interface RegistrationKey {
1820
readonly startUrl: string
@@ -42,6 +44,12 @@ export function getCache(directory = getCacheDir()): SsoCache {
4244
}
4345
}
4446

47+
export function getCacheFileWatcher(directory = getCacheDir()) {
48+
const watcher = vscode.workspace.createFileSystemWatcher(new vscode.RelativePattern(directory, '*.json'))
49+
globals.context.subscriptions.push(watcher)
50+
return watcher
51+
}
52+
4553
export function getRegistrationCache(directory = getCacheDir()): KeyedCache<ClientRegistration, RegistrationKey> {
4654
// Compatability for older Toolkit versions (format on disk is unchanged)
4755
type StoredRegistration = Omit<ClientRegistration, 'expiresAt'> & { readonly expiresAt: string }

packages/core/src/codewhisperer/util/authUtil.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ import {
2626
isIdcSsoConnection,
2727
hasExactScopes,
2828
getTelemetryMetadataForConn,
29+
ProfileNotFoundError,
2930
} from '../../auth/connection'
3031
import { getLogger } from '../../shared/logger'
31-
import { Commands } from '../../shared/vscode/commands2'
32+
import { Commands, placeholder } from '../../shared/vscode/commands2'
3233
import { vsCodeState } from '../models/model'
3334
import { onceChanged, once } from '../../shared/utilities/functionUtils'
3435
import { indent } from '../../shared/utilities/textUtilities'
@@ -42,6 +43,7 @@ const localize = nls.loadMessageBundle()
4243
import { telemetry } from '../../shared/telemetry/telemetry'
4344
import { asStringifiedStack } from '../../shared/telemetry/spans'
4445
import { withTelemetryContext } from '../../shared/telemetry/util'
46+
import { focusAmazonQPanel } from '../../codewhispererChat/commands/registerCommands'
4547

4648
/** Backwards compatibility for connections w pre-chat scopes */
4749
export const codeWhispererCoreScopes = [...scopesCodeWhispererCore]
@@ -134,14 +136,12 @@ export class AuthUtil {
134136
Commands.tryExecute('aws.amazonq.updateReferenceLog'),
135137
])
136138

137-
await setContext('aws.codewhisperer.connected', this.isConnected())
139+
await this.setVscodeContextProps()
138140

139141
// To check valid connection
140142
if (this.isValidEnterpriseSsoInUse() || (this.isBuilderIdInUse() && !this.isConnectionExpired())) {
141-
// start the feature config polling job
142143
await showAmazonQWalkthroughOnce()
143144
}
144-
await this.setVscodeContextProps()
145145
})
146146
})
147147

@@ -254,8 +254,16 @@ export class AuthUtil {
254254
throw new ToolkitError('Connection is not an SSO connection', { code: 'BadConnectionType' })
255255
}
256256

257-
const bearerToken = await this.conn.getToken()
258-
return bearerToken.accessToken
257+
try {
258+
const bearerToken = await this.conn.getToken()
259+
return bearerToken.accessToken
260+
} catch (err) {
261+
if (err instanceof ProfileNotFoundError) {
262+
// Expected that connection would be deleted by conn.getToken()
263+
void focusAmazonQPanel.execute(placeholder, 'profileNotFoundSignout')
264+
}
265+
throw err
266+
}
259267
}
260268

261269
@withTelemetryContext({ name: 'getCredentials', class: authClassName })

packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55
import * as vscode from 'vscode'
6-
import { AwsConnection, Connection, getTelemetryMetadataForConn, isSsoConnection } from '../../../../auth/connection'
6+
import {
7+
AwsConnection,
8+
Connection,
9+
SsoConnection,
10+
getTelemetryMetadataForConn,
11+
isSsoConnection,
12+
} from '../../../../auth/connection'
713
import { AuthUtil } from '../../../../codewhisperer/util/authUtil'
814
import { CommonAuthWebview } from '../backend'
915
import { awsIdSignIn } from '../../../../codewhisperer/util/showSsoPrompt'
@@ -178,6 +184,12 @@ export class AmazonQLoginWebview extends CommonAuthWebview {
178184
this.emitAuthMetric()
179185
}
180186

187+
async listSsoConnections(): Promise<SsoConnection[]> {
188+
// Amazon Q only supports 1 connection at a time,
189+
// so there isn't a need to de-duplicate connections.
190+
return []
191+
}
192+
181193
override startIamCredentialSetup(
182194
profileName: string,
183195
accessKey: string,

packages/core/src/login/webview/vue/backend.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import {
1616
AwsConnection,
1717
Connection,
1818
hasScopes,
19-
isSsoConnection,
2019
scopesCodeCatalyst,
2120
scopesCodeWhispererChat,
2221
scopesSsoAccountAccess,
@@ -189,9 +188,8 @@ export abstract class CommonAuthWebview extends VueWebview {
189188

190189
abstract signout(): Promise<void>
191190

192-
async listSsoConnections(): Promise<SsoConnection[]> {
193-
return (await Auth.instance.listConnections()).filter((conn) => isSsoConnection(conn)) as SsoConnection[]
194-
}
191+
/** List current connections known by the extension for the purpose of preventing duplicates. */
192+
abstract listSsoConnections(): Promise<SsoConnection[]>
195193

196194
/**
197195
* Emit stored metric metadata. Does not reset the stored metric metadata, because it

packages/core/src/login/webview/vue/toolkit/backend_toolkit.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ import { CommonAuthWebview } from '../backend'
1010
import {
1111
AwsConnection,
1212
Connection,
13+
SsoConnection,
1314
TelemetryMetadata,
1415
createSsoProfile,
1516
getTelemetryMetadataForConn,
17+
isSsoConnection,
1618
} from '../../../../auth/connection'
1719
import { Auth } from '../../../../auth/auth'
1820
import { CodeCatalystAuthenticationProvider } from '../../../../codecatalyst/auth'
@@ -145,6 +147,10 @@ export class ToolkitLoginWebview extends CommonAuthWebview {
145147
return connections
146148
}
147149

150+
async listSsoConnections(): Promise<SsoConnection[]> {
151+
return (await Auth.instance.listConnections()).filter((conn) => isSsoConnection(conn)) as SsoConnection[]
152+
}
153+
148154
override reauthenticateConnection(): Promise<undefined> {
149155
throw new Error('Method not implemented.')
150156
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "Login state not updating across multiple VS Code windows."
4+
}

0 commit comments

Comments
 (0)