Have getCurrentAuthenticationSessionInfo take in a secretStorageService (microsoft#187092)

TylerLeonhardt · web-flow · commit 322bc2d7d86e · 2023-07-06T00:09:01.000+02:00
To start the migration process, we have `getCurrentAuthenticationSessionInfo` take in a `secretStorageService` and also replaces the `LocalStorageCredentialProvider` with a `LocalStorageSecretStorageProvider`.

After this goes in we can then update all embedders (vscode.dev, github.dev, Codespaces, vscode-web-test?) and replace ICredentialProvider usages with ISecretStorageProviders and then once we do that, we can get rid of a bunch of code!
diff --git a/src/vs/code/browser/workbench/workbench.ts b/src/vs/code/browser/workbench/workbench.ts
@@ -4,38 +4,32 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { isStandalone } from 'vs/base/browser/browser';
-import { CancellationToken } from 'vs/base/common/cancellation';
 import { parse } from 'vs/base/common/marshalling';
 import { Emitter } from 'vs/base/common/event';
 import { Disposable, IDisposable } from 'vs/base/common/lifecycle';
 import { Schemas } from 'vs/base/common/network';
 import { isEqual } from 'vs/base/common/resources';
 import { URI, UriComponents } from 'vs/base/common/uri';
-import { request } from 'vs/base/parts/request/browser/request';
 import product from 'vs/platform/product/common/product';
 import { isFolderToOpen, isWorkspaceToOpen } from 'vs/platform/window/common/window';
 import { create } from 'vs/workbench/workbench.web.main';
 import { posix } from 'vs/base/common/path';
 import { ltrim } from 'vs/base/common/strings';
-import type { ICredentialsProvider } from 'vs/platform/credentials/common/credentials';
 import type { IURLCallbackProvider } from 'vs/workbench/services/url/browser/urlService';
 import type { IWorkbenchConstructionOptions } from 'vs/workbench/browser/web.api';
 import type { IWorkspace, IWorkspaceProvider } from 'vs/workbench/services/host/browser/browserHostService';
+import { ISecretStorageProvider } from 'vs/platform/secrets/common/secrets';
+import { AuthenticationSessionInfo } from 'vs/workbench/services/authentication/browser/authenticationService';
 
-interface ICredential {
-	service: string;
-	account: string;
-	password: string;
-}
-
-class LocalStorageCredentialsProvider implements ICredentialsProvider {
+class LocalStorageSecretStorageProvider implements ISecretStorageProvider {
+	private static readonly STORAGE_KEY = 'secrets.provider';
 
-	private static readonly CREDENTIALS_STORAGE_KEY = 'credentials.provider';
+	private _secrets: Record<string, string> | undefined;
 
-	private readonly authService: string | undefined;
+	type: 'in-memory' | 'persisted' | 'unknown' = 'persisted';
 
 	constructor() {
-		let authSessionInfo: { readonly id: string; readonly accessToken: string; readonly providerId: string; readonly canSignOut?: boolean; readonly scopes: string[][] } | undefined;
+		let authSessionInfo: (AuthenticationSessionInfo & { scopes: string[][] }) | undefined;
 		const authSessionElement = document.getElementById('vscode-workbench-auth-session');
 		const authSessionElementAttribute = authSessionElement ? authSessionElement.getAttribute('data-settings') : undefined;
 		if (authSessionElementAttribute) {
@@ -46,133 +40,60 @@ class LocalStorageCredentialsProvider implements ICredentialsProvider {
 
 		if (authSessionInfo) {
 			// Settings Sync Entry
-			this.setPassword(`${product.urlProtocol}.login`, 'account', JSON.stringify(authSessionInfo));
+			this.set(`${product.urlProtocol}.loginAccount`, JSON.stringify(authSessionInfo));
 
 			// Auth extension Entry
-			this.authService = `${product.urlProtocol}-${authSessionInfo.providerId}.login`;
-			this.setPassword(this.authService, 'account', JSON.stringify(authSessionInfo.scopes.map(scopes => ({
+			if (authSessionInfo.providerId !== 'github') {
+				console.error(`Unexpected auth provider: ${authSessionInfo.providerId}. Expected 'github'.`);
+				return;
+			}
+			const authAccount = JSON.stringify({ extensionId: 'vscode.github-authentication', key: 'github.auth' });
+			this.set(authAccount, JSON.stringify(authSessionInfo.scopes.map(scopes => ({
 				id: authSessionInfo!.id,
 				scopes,
 				accessToken: authSessionInfo!.accessToken
 			}))));
 		}
 	}
 
-	private _credentials: ICredential[] | undefined;
-	private get credentials(): ICredential[] {
-		if (!this._credentials) {
-			try {
-				const serializedCredentials = window.localStorage.getItem(LocalStorageCredentialsProvider.CREDENTIALS_STORAGE_KEY);
-				if (serializedCredentials) {
-					this._credentials = JSON.parse(serializedCredentials);
-				}
-			} catch (error) {
-				// ignore
-			}
-
-			if (!Array.isArray(this._credentials)) {
-				this._credentials = [];
-			}
-		}
-
-		return this._credentials;
-	}
-
-	private save(): void {
-		window.localStorage.setItem(LocalStorageCredentialsProvider.CREDENTIALS_STORAGE_KEY, JSON.stringify(this.credentials));
-	}
-
-	async getPassword(service: string, account: string): Promise<string | null> {
-		return this.doGetPassword(service, account);
+	get(key: string): Promise<string | undefined> {
+		return Promise.resolve(this.secrets[key]);
 	}
+	set(key: string, value: string): Promise<void> {
+		this.secrets[key] = value;
+		this.save();
 
-	private async doGetPassword(service: string, account?: string): Promise<string | null> {
-		for (const credential of this.credentials) {
-			if (credential.service === service) {
-				if (typeof account !== 'string' || account === credential.account) {
-					return credential.password;
-				}
-			}
-		}
-
-		return null;
+		return Promise.resolve();
 	}
-
-	async setPassword(service: string, account: string, password: string): Promise<void> {
-		this.doDeletePassword(service, account);
-
-		this.credentials.push({ service, account, password });
+	async delete(key: string): Promise<void> {
+		delete this.secrets[key];
 
 		this.save();
 
-		try {
-			if (password && service === this.authService) {
-				const value = JSON.parse(password);
-				if (Array.isArray(value) && value.length === 0) {
-					await this.logout(service);
-				}
-			}
-		} catch (error) {
-			console.log(error);
-		}
+		return Promise.resolve();
 	}
 
-	async deletePassword(service: string, account: string): Promise<boolean> {
-		const result = await this.doDeletePassword(service, account);
-
-		if (result && service === this.authService) {
+	private get secrets(): Record<string, string> {
+		if (!this._secrets) {
 			try {
-				await this.logout(service);
+				const serializedCredentials = window.localStorage.getItem(LocalStorageSecretStorageProvider.STORAGE_KEY);
+				if (serializedCredentials) {
+					this._secrets = JSON.parse(serializedCredentials);
+				}
 			} catch (error) {
-				console.log(error);
+				// ignore
 			}
-		}
-
-		return result;
-	}
 
-	private async doDeletePassword(service: string, account: string): Promise<boolean> {
-		let found = false;
-
-		this._credentials = this.credentials.filter(credential => {
-			if (credential.service === service && credential.account === account) {
-				found = true;
-
-				return false;
+			if (!(this._secrets instanceof Object)) {
+				this._secrets = {};
 			}
-
-			return true;
-		});
-
-		if (found) {
-			this.save();
 		}
 
-		return found;
-	}
-
-	async findPassword(service: string): Promise<string | null> {
-		return this.doGetPassword(service);
+		return this._secrets;
 	}
 
-	async findCredentials(service: string): Promise<Array<{ account: string; password: string }>> {
-		return this.credentials
-			.filter(credential => credential.service === service)
-			.map(({ account, password }) => ({ account, password }));
-	}
-
-	private async logout(service: string): Promise<void> {
-		const queryValues: Map<string, string> = new Map();
-		queryValues.set('logout', String(true));
-		queryValues.set('service', service);
-
-		await request({
-			url: doCreateUri('/auth/logout', queryValues).toString(true)
-		}, CancellationToken.None);
-	}
-
-	async clear(): Promise<void> {
-		window.localStorage.removeItem(LocalStorageCredentialsProvider.CREDENTIALS_STORAGE_KEY);
+	private save(): void {
+		window.localStorage.setItem(LocalStorageSecretStorageProvider.STORAGE_KEY, JSON.stringify(this.secrets));
 	}
 }
 
@@ -469,24 +390,6 @@ class WorkspaceProvider implements IWorkspaceProvider {
 	}
 }
 
-function doCreateUri(path: string, queryValues: Map<string, string>): URI {
-	let query: string | undefined = undefined;
-
-	if (queryValues) {
-		let index = 0;
-		queryValues.forEach((value, key) => {
-			if (!query) {
-				query = '';
-			}
-
-			const prefix = (index++ === 0) ? '' : '&';
-			query += `${prefix}${key}=${encodeURIComponent(value)}`;
-		});
-	}
-
-	return URI.parse(window.location.href).with({ path, query });
-}
-
 (function () {
 
 	// Find config by checking for DOM
@@ -504,6 +407,6 @@ function doCreateUri(path: string, queryValues: Map<string, string>): URI {
 		settingsSyncOptions: config.settingsSyncOptions ? { enabled: config.settingsSyncOptions.enabled, } : undefined,
 		workspaceProvider: WorkspaceProvider.create(config),
 		urlCallbackProvider: new LocalStorageURLCallbackProvider(config.callbackRoute),
-		credentialsProvider: config.remoteAuthority ? undefined /* with a remote, we don't use a local credentials provider */ : new LocalStorageCredentialsProvider()
+		secretStorageProvider: config.remoteAuthority ? undefined /* with a remote, we don't use a local secret storage provider */ : new LocalStorageSecretStorageProvider()
 	});
 })();
diff --git a/src/vs/workbench/browser/parts/activitybar/activitybarActions.ts b/src/vs/workbench/browser/parts/activitybar/activitybarActions.ts
@@ -41,6 +41,7 @@ import { ICredentialsService } from 'vs/platform/credentials/common/credentials'
 import { IUserDataProfileService } from 'vs/workbench/services/userDataProfile/common/userDataProfile';
 import { StandardMouseEvent } from 'vs/base/browser/mouseEvent';
 import { ILogService } from 'vs/platform/log/common/log';
+import { ISecretStorageService } from 'vs/platform/secrets/common/secrets';
 
 export class ViewContainerActivityAction extends ActivityAction {
 
@@ -235,7 +236,7 @@ export class AccountsActivityActionViewItem extends MenuActivityActionViewItem {
 	private readonly problematicProviders: Set<string> = new Set();
 
 	private initialized = false;
-	private sessionFromEmbedder = getCurrentAuthenticationSessionInfo(this.credentialsService, this.productService);
+	private sessionFromEmbedder = getCurrentAuthenticationSessionInfo(this.credentialsService, this.secretStorageService, this.productService);
 
 	constructor(
 		action: ActivityAction,
@@ -253,6 +254,7 @@ export class AccountsActivityActionViewItem extends MenuActivityActionViewItem {
 		@IConfigurationService configurationService: IConfigurationService,
 		@IStorageService private readonly storageService: IStorageService,
 		@IKeybindingService keybindingService: IKeybindingService,
+		@ISecretStorageService private readonly secretStorageService: ISecretStorageService,
 		@ICredentialsService private readonly credentialsService: ICredentialsService,
 		@ILogService private readonly logService: ILogService
 	) {
diff --git a/src/vs/workbench/browser/web.main.ts b/src/vs/workbench/browser/web.main.ts
@@ -92,6 +92,10 @@ import { BrowserRemoteResourceLoader } from 'vs/workbench/services/remote/browse
 import { BufferLogger } from 'vs/platform/log/common/bufferLog';
 import { FileLoggerService } from 'vs/platform/log/common/fileLog';
 import { IEmbedderTerminalService } from 'vs/workbench/services/terminal/common/embedderTerminalService';
+import { BrowserSecretStorageService } from 'vs/workbench/services/secrets/browser/secretStorageService';
+import { EncryptionService } from 'vs/workbench/services/encryption/browser/encryptionService';
+import { IEncryptionService } from 'vs/platform/encryption/common/encryptionService';
+import { ISecretStorageService } from 'vs/platform/secrets/common/secrets';
 
 export class BrowserMain extends Disposable {
 
@@ -384,9 +388,14 @@ export class BrowserMain extends Disposable {
 		const credentialsService = new BrowserCredentialsService(environmentService, remoteAgentService, productService);
 		serviceCollection.set(ICredentialsService, credentialsService);
 
+		const encryptionService = new EncryptionService();
+		serviceCollection.set(IEncryptionService, encryptionService);
+		const secretStorageService = new BrowserSecretStorageService(storageService, encryptionService, environmentService, logService);
+		serviceCollection.set(ISecretStorageService, secretStorageService);
+
 		// Userdata Initialize Service
 		const userDataInitializers: IUserDataInitializer[] = [];
-		userDataInitializers.push(new UserDataSyncInitializer(environmentService, credentialsService, userDataSyncStoreManagementService, fileService, userDataProfilesService, storageService, productService, requestService, logService, uriIdentityService));
+		userDataInitializers.push(new UserDataSyncInitializer(environmentService, secretStorageService, credentialsService, userDataSyncStoreManagementService, fileService, userDataProfilesService, storageService, productService, requestService, logService, uriIdentityService));
 		if (environmentService.options.profile) {
 			userDataInitializers.push(new UserDataProfileInitializer(environmentService, fileService, userDataProfileService, storageService, logService, uriIdentityService, requestService));
 		}
diff --git a/src/vs/workbench/contrib/editSessions/browser/editSessionsStorageService.ts b/src/vs/workbench/contrib/editSessions/browser/editSessionsStorageService.ts
@@ -25,6 +25,7 @@ import { IUserDataSyncMachinesService, UserDataSyncMachinesService } from 'vs/pl
 import { Emitter } from 'vs/base/common/event';
 import { CancellationError } from 'vs/base/common/errors';
 import { EditSessionsStoreClient } from 'vs/workbench/contrib/editSessions/common/editSessionsStorageClient';
+import { ISecretStorageService } from 'vs/platform/secrets/common/secrets';
 
 type ExistingSession = IQuickPickItem & { session: AuthenticationSession & { providerId: string } };
 type AuthenticationProviderOption = IQuickPickItem & { provider: IAuthenticationProvider };
@@ -81,6 +82,7 @@ export class EditSessionsWorkbenchService extends Disposable implements IEditSes
 		@IProductService private readonly productService: IProductService,
 		@IContextKeyService private readonly contextKeyService: IContextKeyService,
 		@IDialogService private readonly dialogService: IDialogService,
+		@ISecretStorageService private readonly secretStorageService: ISecretStorageService,
 		@ICredentialsService private readonly credentialsService: ICredentialsService
 	) {
 		super();
@@ -278,7 +280,7 @@ export class EditSessionsWorkbenchService extends Disposable implements IEditSes
 		// If settings sync is already enabled, avoid asking again to authenticate
 		if (this.shouldAttemptEditSessionInit()) {
 			this.logService.info(`Reusing user data sync enablement`);
-			const authenticationSessionInfo = await getCurrentAuthenticationSessionInfo(this.credentialsService, this.productService);
+			const authenticationSessionInfo = await getCurrentAuthenticationSessionInfo(this.credentialsService, this.secretStorageService, this.productService);
 			if (authenticationSessionInfo !== undefined) {
 				this.logService.info(`Using current authentication session with ID ${authenticationSessionInfo.id}`);
 				this.existingSessionId = authenticationSessionInfo.id;
diff --git a/src/vs/workbench/services/authentication/browser/authenticationService.ts b/src/vs/workbench/services/authentication/browser/authenticationService.ts
@@ -19,6 +19,7 @@ import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/
 import { Severity } from 'vs/platform/notification/common/notification';
 import { IProductService } from 'vs/platform/product/common/productService';
 import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput';
+import { ISecretStorageService } from 'vs/platform/secrets/common/secrets';
 import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
 import { IActivityService, NumberBadge } from 'vs/workbench/services/activity/common/activity';
 import { IAuthenticationCreateSessionOptions, AuthenticationProviderInformation, AuthenticationSession, AuthenticationSessionsChangeEvent, IAuthenticationProvider, IAuthenticationService } from 'vs/workbench/services/authentication/common/authentication';
@@ -76,9 +77,17 @@ export function addAccountUsage(storageService: IStorageService, providerId: str
 	storageService.store(accountKey, JSON.stringify(usages), StorageScope.APPLICATION, StorageTarget.MACHINE);
 }
 
+// TODO: pull this out into its own service
 export type AuthenticationSessionInfo = { readonly id: string; readonly accessToken: string; readonly providerId: string; readonly canSignOut?: boolean };
-export async function getCurrentAuthenticationSessionInfo(credentialsService: ICredentialsService, productService: IProductService): Promise<AuthenticationSessionInfo | undefined> {
-	const authenticationSessionValue = await credentialsService.getPassword(`${productService.urlProtocol}.login`, 'account');
+export async function getCurrentAuthenticationSessionInfo(
+	// TODO: Remove when all known embedders implement SecretStorageProviders instead of CredentialsProviders
+	credentialsService: ICredentialsService,
+	secretStorageService: ISecretStorageService,
+	productService: IProductService
+): Promise<AuthenticationSessionInfo | undefined> {
+	const authenticationSessionValue =
+		await secretStorageService.get(`${productService.urlProtocol}.loginAccount`)
+		?? await credentialsService.getPassword(`${productService.urlProtocol}.login`, 'account');
 	if (authenticationSessionValue) {
 		try {
 			const authenticationSessionInfo: AuthenticationSessionInfo = JSON.parse(authenticationSessionValue);
@@ -90,7 +99,8 @@ export async function getCurrentAuthenticationSessionInfo(credentialsService: IC
 				return authenticationSessionInfo;
 			}
 		} catch (e) {
-			// ignore as this is a best effort operation.
+			// This is a best effort operation.
+			console.error(`Failed parsing current auth session value: ${e}`);
 		}
 	}
 	return undefined;
diff --git a/src/vs/workbench/services/secrets/browser/secretStorageService.ts b/src/vs/workbench/services/secrets/browser/secretStorageService.ts
@@ -5,9 +5,7 @@
 
 import { IEncryptionService } from 'vs/platform/encryption/common/encryptionService';
 import { InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions';
-import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { ILogService } from 'vs/platform/log/common/log';
-import { INotificationService } from 'vs/platform/notification/common/notification';
 import { ISecretStorageProvider, ISecretStorageService, BaseSecretStorageService } from 'vs/platform/secrets/common/secrets';
 import { IStorageService } from 'vs/platform/storage/common/storage';
 import { IBrowserWorkbenchEnvironmentService } from 'vs/workbench/services/environment/browser/environmentService';
@@ -20,8 +18,6 @@ export class BrowserSecretStorageService extends BaseSecretStorageService {
 		@IStorageService storageService: IStorageService,
 		@IEncryptionService encryptionService: IEncryptionService,
 		@IBrowserWorkbenchEnvironmentService environmentService: IBrowserWorkbenchEnvironmentService,
-		@IInstantiationService instantiationService: IInstantiationService,
-		@INotificationService notificationService: INotificationService,
 		@ILogService logService: ILogService
 	) {
 		super(storageService, encryptionService, logService);
diff --git a/src/vs/workbench/services/userDataSync/browser/userDataSyncInit.ts b/src/vs/workbench/services/userDataSync/browser/userDataSyncInit.ts
@@ -35,6 +35,7 @@ import { TasksInitializer } from 'vs/platform/userDataSync/common/tasksSync';
 import { IUserDataProfilesService } from 'vs/platform/userDataProfile/common/userDataProfile';
 import { IBrowserWorkbenchEnvironmentService } from 'vs/workbench/services/environment/browser/environmentService';
 import { IUserDataInitializer } from 'vs/workbench/services/userData/browser/userDataInit';
+import { ISecretStorageService } from 'vs/platform/secrets/common/secrets';
 
 export class UserDataSyncInitializer implements IUserDataInitializer {
 
@@ -46,6 +47,7 @@ export class UserDataSyncInitializer implements IUserDataInitializer {
 
 	constructor(
 		@IBrowserWorkbenchEnvironmentService private readonly environmentService: IBrowserWorkbenchEnvironmentService,
+		@ISecretStorageService private readonly secretStorageService: ISecretStorageService,
 		@ICredentialsService private readonly credentialsService: ICredentialsService,
 		@IUserDataSyncStoreManagementService private readonly userDataSyncStoreManagementService: IUserDataSyncStoreManagementService,
 		@IFileService private readonly fileService: IFileService,
@@ -90,7 +92,7 @@ export class UserDataSyncInitializer implements IUserDataInitializer {
 
 					let authenticationSession;
 					try {
-						authenticationSession = await getCurrentAuthenticationSessionInfo(this.credentialsService, this.productService);
+						authenticationSession = await getCurrentAuthenticationSessionInfo(this.credentialsService, this.secretStorageService, this.productService);
 					} catch (error) {
 						this.logService.error(error);
 					}
diff --git a/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts b/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts
