Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sequence operations to credentials and clean up better when setting #175695

Merged
merged 1 commit into from
Feb 28, 2023
Merged

Sequence operations to credentials and clean up better when setting #175695

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 44 additions & 20 deletions src/vs/platform/credentials/common/credentialsMainService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Emitter } from 'vs/base/common/event';
import { Disposable } from 'vs/base/common/lifecycle';
import { ILogService } from 'vs/platform/log/common/log';
import { isWindows } from 'vs/base/common/platform';
import { retry } from 'vs/base/common/async';
import { retry, SequencerByKey } from 'vs/base/common/async';

interface ChunkedPassword {
content: string;
Expand All @@ -28,6 +28,8 @@ export abstract class BaseCredentialsMainService extends Disposable implements I

protected _keytarCache: KeytarModule | undefined;

private _sequencer = new SequencerByKey<string>();

constructor(
@ILogService protected readonly logService: ILogService,
) {
Expand Down Expand Up @@ -56,6 +58,10 @@ export abstract class BaseCredentialsMainService extends Disposable implements I
return null;
}

return this._sequencer.queue(service + account, () => this.doGetPassword(keytar, service, account));
}

private async doGetPassword(keytar: KeytarModule, service: string, account: string): Promise<string | null> {
const password = await retry(() => keytar.getPassword(service, account), 50, 3);
if (!password) {
this.logService.trace('Did not get a password from keytar for account:', account);
Expand Down Expand Up @@ -106,29 +112,43 @@ export abstract class BaseCredentialsMainService extends Disposable implements I
throw e;
}

if (isWindows && password.length > BaseCredentialsMainService.MAX_PASSWORD_LENGTH) {
let index = 0;
let chunk = 0;
let hasNextChunk = true;
while (hasNextChunk) {
const passwordChunk = password.substring(index, index + BaseCredentialsMainService.PASSWORD_CHUNK_SIZE);
index += BaseCredentialsMainService.PASSWORD_CHUNK_SIZE;
hasNextChunk = password.length - index > 0;

const content: ChunkedPassword = {
content: passwordChunk,
hasNextChunk: hasNextChunk
};
await retry(() => keytar.setPassword(service, chunk ? `${account}-${chunk}` : account, JSON.stringify(content)), 50, 3);
chunk++;
}
await this._sequencer.queue(service + account, () => this.doSetPassword(keytar, service, account, password));
}

this.logService.trace(`Got${chunk ? ` ${chunk}-chunked` : ''} password from keytar for account:`, account);
} else {
private async doSetPassword(keytar: KeytarModule, service: string, account: string, password: string): Promise<void> {
if (!isWindows) {
await retry(() => keytar.setPassword(service, account, password), 50, 3);
this.logService.trace('Got password from keytar for account:', account);
this.logService.trace('Set password from keytar for account:', account);
this._onDidChangePassword.fire({ service, account });
return;
}

// On Windows, we have to chunk the password because the Windows Credential Manager only allows passwords of a max length.
// So to make sure we can store passwords of any length, we chunk the password and store it as multiple passwords.
// To ensure we store an password correctly, we first delete any existing password chunks and then store the new ones.

await this.doDeletePassword(keytar, service, account);

let index = 0;
let chunk = 0;
let hasNextChunk = true;
const promises = [];
while (hasNextChunk) {
const passwordChunk = password.substring(index, index + BaseCredentialsMainService.PASSWORD_CHUNK_SIZE);
index += BaseCredentialsMainService.PASSWORD_CHUNK_SIZE;
hasNextChunk = password.length - index > 0;

const content: ChunkedPassword = {
content: passwordChunk,
hasNextChunk: hasNextChunk
};
promises.push(retry(() => keytar.setPassword(service, chunk ? `${account}-${chunk}` : account, JSON.stringify(content)), 50, 3));
chunk++;
}

await Promise.all(promises);

this.logService.trace(`Set${chunk ? ` ${chunk}-chunked` : ''} password from keytar for account:`, account);
this._onDidChangePassword.fire({ service, account });
}

Expand All @@ -142,6 +162,10 @@ export abstract class BaseCredentialsMainService extends Disposable implements I
throw e;
}

return await this._sequencer.queue(service + account, () => this.doDeletePassword(keytar, service, account));
}

private async doDeletePassword(keytar: KeytarModule, service: string, account: string): Promise<boolean> {
const password = await keytar.getPassword(service, account);
if (!password) {
this.logService.trace('Did not get a password to delete from keytar for account:', account);
Expand Down