Skip to content

Commit 01e7d04

Browse files
fix: racecondition in cw auth change (#4062)
Problem: In the Add Connection page we listen to auth events in the backend. When once of these events is fired the Add Connection page eventually checks if isEnterpriseSsoInUse. The issue is that this uses a variable that is separately updated by the event onDidChangeActiveConnection. There is a race condition for when onDidChangeActiveConnection updates the variable used in isEnterpriseSsoInUse() versus when isEnterpriseSsoInUse() is actually called. Also when SecondaryAuth.deleteConnection() is called the new state is not updated when the function is complete since we are relying on an event listener to do the final updating. Solution: Get rid of the variable and whenever isEnterpriseSsoInUse() is called it will pull the latest state. Now we don't have to manage it. Also in deleteConnection() do a final clearing of the saved connection state so that upon completion the user will be able to get the latest state before all the event emitters fire. Signed-off-by: nkomonen <[email protected]>
1 parent ff6b889 commit 01e7d04

File tree

2 files changed

+8
-13
lines changed

2 files changed

+8
-13
lines changed

src/auth/secondaryAuth.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,11 @@ export class SecondaryAuth<T extends Connection = Connection> {
157157
/**
158158
* Globally deletes the connection that this secondary auth is using,
159159
* effectively doing a signout.
160-
*
161-
* The deletion automatically propogates to the other users of this
162-
* connection, assuming they've configured the event listeners.
163160
*/
164161
public async deleteConnection() {
165162
if (this.activeConnection) {
166163
await this.auth.deleteConnection(this.activeConnection)
164+
await this.clearSavedConnection()
167165
}
168166
}
169167

src/codewhisperer/util/authUtil.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ export const isValidCodeWhispererConnection = (conn?: Connection): conn is Conne
4545
export class AuthUtil {
4646
static #instance: AuthUtil
4747

48-
private usingEnterpriseSSO: boolean = false
4948
private reauthenticatePromptShown: boolean = false
5049
private _isCustomizationFeatureEnabled: boolean = false
5150

@@ -79,14 +78,9 @@ export class AuthUtil {
7978
}
8079
})
8180

82-
this.secondaryAuth.onDidChangeActiveConnection(async conn => {
83-
if (conn?.type === 'sso') {
84-
this.usingEnterpriseSSO = !isBuilderIdConnection(conn)
85-
if (!this.isConnectionExpired() && this.usingEnterpriseSSO) {
86-
vscode.commands.executeCommand('aws.codeWhisperer.notifyNewCustomizations')
87-
}
88-
} else {
89-
this.usingEnterpriseSSO = false
81+
this.secondaryAuth.onDidChangeActiveConnection(async () => {
82+
if (this.isValidEnterpriseSsoInUse()) {
83+
vscode.commands.executeCommand('aws.codeWhisperer.notifyNewCustomizations')
9084
}
9185
await Promise.all([
9286
vscode.commands.executeCommand('aws.codeWhisperer.refresh'),
@@ -134,7 +128,10 @@ export class AuthUtil {
134128
}
135129

136130
public isEnterpriseSsoInUse(): boolean {
137-
return this.conn !== undefined && this.usingEnterpriseSSO
131+
const conn = this.conn
132+
// we have an sso that isn't builder id, must be IdC by process of elimination
133+
const isUsingEnterpriseSso = conn?.type === 'sso' && !isBuilderIdConnection(conn)
134+
return conn !== undefined && isUsingEnterpriseSso
138135
}
139136

140137
// If there is an active SSO connection

0 commit comments

Comments
 (0)