If the keychain is unavailable, fallback to the user settings file. Fixes microsoft#366

Also, only save tokens in the keychain when going through the in editor sign-in
process. If the user fills out the token manually in the user settings file,
keep it there.

Author: Andreia Gaita

src/authentication/configuration.ts

@@ -41,13 +41,19 @@ export class Configuration implements IConfiguration {
 		this.onDidChange = this._emitter.event;
 	}
 
-	public update(username: string | undefined, token: string | undefined, raiseEvent: boolean = true): void {
+	public update(username: string | undefined, token: string | undefined, raiseEvent: boolean = true): Promise<boolean> {
 		if (username !== this.username || token !== this.token) {
 			this.username = username;
 			this.token = token;
 			if (raiseEvent) {
 				this._emitter.fire(this);
 			}
+			return Promise.resolve(true);
 		}
+		return Promise.resolve(false);
+	}
+
+	protected raiseChangedEvent(): void {
+		this._emitter.fire(this);
 	}
 }

src/authentication/vsConfiguration.ts

@@ -7,7 +7,8 @@ const HOSTS_KEY = 'hosts';
 const CREDENTIAL_SERVICE = 'vscode-pull-request-github';
 
 export class VSCodeConfiguration extends Configuration {
-	private _hosts: Map<string, IHostConfiguration>;
+	private _hosts: Map<string, IHostConfiguration> = new Map<string, IHostConfiguration>();
+	private _hostTokensInKeychain: Set<string> = new Set<string>();
 
 	constructor() {
 		super(undefined);
@@ -63,14 +64,34 @@ export class VSCodeConfiguration extends Configuration {
 		this.saveConfiguration();
 	}
 
-	public async update(username: string | undefined, token: string | undefined, raiseEvent: boolean = true): Promise<void> {
-		super.update(username, token, raiseEvent);
-		await keychain.setPassword(CREDENTIAL_SERVICE, this.host, token);
-		this.saveConfiguration();
+	public async update(username: string | undefined, token: string | undefined, raiseEvent: boolean = true): Promise<boolean> {
+		const key = this.host;
+		try {
+			// this might fail. if it does, fallback to saving the token in the user settings file
+			await keychain.setPassword(CREDENTIAL_SERVICE, key, token);
+			if (!this._hostTokensInKeychain.has(key)) {
+				this._hostTokensInKeychain.add(key);
+			}
+		} catch (e) {
+			if (this._hostTokensInKeychain.has(key)) {
+				this._hostTokensInKeychain.delete(key);
+			}
+		}
+		return super.update(username, token, false).then(hasChanged => {
+			if (hasChanged) {
+				this.saveConfiguration();
+				// raise changed events only after the host list has been roundtripped to disk
+				if (raiseEvent) {
+					this.raiseChangedEvent();
+				}
+			}
+			return hasChanged;
+		});
 	}
 
 	private reset(): void {
-		this._hosts = new Map<string, IHostConfiguration>();
+		this._hosts.clear();
+		this._hostTokensInKeychain.clear();
 	}
 
 	public async loadConfiguration(): Promise<void> {
@@ -84,12 +105,16 @@ export class VSCodeConfiguration extends Configuration {
 		return Promise.all(configHosts.map(async c => {
 			// if the token is not in the user settings file, load it from the system credential manager
 			if (c.token === 'system') {
-				c.token = await keychain.getPassword(CREDENTIAL_SERVICE, c.host) || undefined;
+				try {
+					c.token = await keychain.getPassword(CREDENTIAL_SERVICE, c.host) || undefined;
+					if (c.token) {
+						this._hostTokensInKeychain.add(c.host);
+						this._hosts.set(c.host, c);
+					}
+				} catch { } // only load the host if we can read the token, otherwise there's no point
 			} else {
-				// the token might have been filled out in the settings file, load it from there if so
-				await keychain.setPassword(CREDENTIAL_SERVICE, c.host, c.token);
+				this._hosts.set(c.host, c);
 			}
-			this._hosts.set(c.host, c);
 		})).then(_ => {
 			if (this.host && !this._hosts.has(this.host)) {
 				this._hosts.set(this.host, {
@@ -110,7 +135,10 @@ export class VSCodeConfiguration extends Configuration {
 			});
 		}
 		const config = vscode.workspace.getConfiguration(SETTINGS_NAMESPACE);
-		// don't save the token to the user settings file
-		config.update(HOSTS_KEY, Array.from(this._hosts.values()).map(x => { return { host: x.host, username: x.username, token: 'system' }; }), true);
+		// don't save the token to the user settings file if it's in the keychain
+		config.update(HOSTS_KEY, Array.from(this._hosts.values()).map(x => {
+			const token = this._hostTokensInKeychain.has(x.host) ? 'system' : x.token;
+			return { host: x.host, username: x.username, token };
+		}), true);
 	}
 }