Microsoft/vscode
1
0
Fork 0
mirror of https://github.com/microsoft/vscode.git synced 2026-05-01 22:12:26 +01:00
Code Issues Packages Projects Releases Wiki Activity

Only have a single window store the session (#169356)

Browse Source 
Before this change, every single window was writing to the same secret at basically the same time because they would all refresh the token and then attempt to store that refresh token.

I believe this was causing a few race condition bugs that users were seeing.

With this change we now so our best to have only 1 window store the session by relying on the window focused state.

If the window is focused or becomes focused, we will store the refresh token.

If the window detects that another window has stored something, we will not attempt to wait for focus to store something.

If nothing has happened, and it's been 5 hrs (+/- some seconds) go ahead and store it. This is the scenario of when a user has VS Code in the background for like ages but never goes to it.

ref #165115
ref #130893
ref #168485
This commit is contained in:
Tyler James Leonhardt
2022-12-15 21:58:32 -08:00
committed by GitHub
parent edc432e920
commit 461b3f6184
1 changed files with 48 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
extensions/microsoft-authentication/src/AADHelper.ts
View File

@@ -586,7 +586,7 @@ export class AzureActiveDirectoryService {
if (token.expiresIn) {
this.setSessionTimeout(token.sessionId, token.refreshToken, scopeData, token.expiresIn * AzureActiveDirectoryService.REFRESH_TIMEOUT_MODIFIER);
}
await this.setToken(token, scopeData);
this.setToken(token, scopeData);
Logger.info(`Token refresh success for scopes: ${token.scope}`);
return token;
} catch (e) {
@@ -728,7 +728,7 @@ export class AzureActiveDirectoryService {
if (token.expiresIn) {
this.setSessionTimeout(token.sessionId, token.refreshToken, scopeData, token.expiresIn * AzureActiveDirectoryService.REFRESH_TIMEOUT_MODIFIER);
}
await this.setToken(token, scopeData);
this.setToken(token, scopeData);
Logger.info(`Login successful for scopes: ${scopeData.scopeStr}`);
return await this.convertToSession(token);
}
@@ -777,7 +777,7 @@ export class AzureActiveDirectoryService {
//#region storage operations
private async setToken(token: IToken, scopeData: IScopeData): Promise<void> {
private setToken(token: IToken, scopeData: IScopeData): void {
Logger.info(`Setting token for scopes: ${scopeData.scopeStr}`);
const existingTokenIndex = this._tokens.findIndex(t => t.sessionId === token.sessionId);
@@ -787,17 +787,54 @@ export class AzureActiveDirectoryService {
this._tokens.push(token);
}
// Don't await because setting the token is only useful for any new windows that open.
this.storeToken(token, scopeData);
}
private async storeToken(token: IToken, scopeData: IScopeData): Promise<void> {
if (!vscode.window.state.focused) {
const shouldStore = await new Promise((resolve, _) => {
// To handle the case where the window is not focused for a long time. We want to store the token
// at some point so that the next time they _do_ interact with VS Code, they don't have to sign in again.
const timer = setTimeout(
() => resolve(true),
// 5 hours + random extra 0-30 seconds so that each window doesn't try to store at the same time
(18000000) + Math.floor(Math.random() * 30000)
);
const dispose = vscode.Disposable.from(
vscode.window.onDidChangeWindowState(e => {
if (e.focused) {
resolve(true);
dispose.dispose();
clearTimeout(timer);
}
}),
this._tokenStorage.onDidChangeInOtherWindow(e => {
if (e.updated.includes(token.sessionId)) {
resolve(false);
dispose.dispose();
clearTimeout(timer);
}
})
);
});
if (!shouldStore) {
Logger.info(`Not storing token for scopes ${scopeData.scopeStr} because it was added in another window`);
return;
}
}
await this._tokenStorage.store(token.sessionId, {
id: token.sessionId,
refreshToken: token.refreshToken,
scope: token.scope,
account: token.account
});
Logger.info(`Stored token for scopes: ${scopeData.scopeStr}`);
}
private async checkForUpdates(e: IDidChangeInOtherWindowEvent<IStoredSession>): Promise<void> {
const added: vscode.AuthenticationSession[] = [];
const removed: vscode.AuthenticationSession[] = [];
for (const key of e.added) {
const session = await this._tokenStorage.get(key);
if (!session) {
@@ -834,11 +871,13 @@ export class AzureActiveDirectoryService {
for (const { value } of e.removed) {
Logger.info(`Session removed in another window with scopes: ${value.scope}`);
const session = await this.removeSessionById(value.id, false);
if (session) {
removed.push(session);
}
await this.removeSessionById(value.id, false);
}
// NOTE: We don't need to handle changed sessions because all that really would give us is a new refresh token
// because access tokens are not stored in Secret Storage due to their short lifespan. This new refresh token
// is not useful in this window because we really only care about the lifetime of the _access_ token which we
// are already managing (see usages of `setSessionTimeout`).
}
//#endregion