Adapts to vscode authentication apis

src/credentials.ts

@@ -1,103 +0,0 @@
-'use strict';
-import { Event, EventEmitter } from 'vscode';
-import * as keytarType from 'keytar';
-import { extensionId } from './constants';
-import { Logger } from './logger';
-
-const CredentialKey = `${extensionId}:vscode`;
-
-// keytar depends on a native module shipped in vscode
-function getNodeModule<T>(moduleName: string): T | undefined {
-	// eslint-disable-next-line no-eval
-	const vscodeRequire = eval('require');
-	try {
-		return vscodeRequire(moduleName);
-	} catch {
-		return undefined;
-	}
-}
-
-const keychain = getNodeModule<typeof keytarType>('keytar');
-
-interface CredentialSaveEvent {
-	key: string;
-	reason: 'save';
-}
-
-interface CredentialClearEvent {
-	key: string | undefined;
-	reason: 'clear';
-}
-
-interface CredentialInvalidEvent {
-	key: string | undefined;
-	reason: 'invalid';
-}
-
-export type CredentialChangeEvent = CredentialSaveEvent | CredentialClearEvent | CredentialInvalidEvent;
-
-export namespace CredentialManager {
-	const _onDidChange = new EventEmitter<CredentialChangeEvent>();
-	export const onDidChange: Event<CredentialChangeEvent> = _onDidChange.event;
-
-	export async function addOrUpdate(key: string, value: string | object) {
-		// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
-		if (!key || !value) return;
-		if (keychain == null) {
-			Logger.log('CredentialManager.addOrUpdate: No credential store found');
-			return;
-		}
-
-		try {
-			await keychain.setPassword(CredentialKey, key, typeof value === 'string' ? value : JSON.stringify(value));
-			_onDidChange.fire({ key: key, reason: 'save' });
-		} catch (ex) {
-			Logger.error(ex, 'CredentialManager.addOrUpdate: Failed to set credentials');
-		}
-	}
-
-	export async function clear(key: string) {
-		if (!key) return;
-		if (keychain == null) {
-			Logger.log('CredentialManager.clear: No credential store found');
-			return;
-		}
-
-		try {
-			await keychain.deletePassword(CredentialKey, key);
-			_onDidChange.fire({ key: key, reason: 'clear' });
-		} catch (ex) {
-			Logger.error(ex, 'CredentialManager.clear: Failed to clear credentials');
-		}
-	}
-
-	export async function get(key: string): Promise<string | undefined> {
-		if (!key) return undefined;
-		if (keychain == null) {
-			Logger.log('CredentialManager.clear: No credential store found');
-			return undefined;
-		}
-
-		try {
-			const value = await keychain.getPassword(CredentialKey, key);
-
-			return value ?? undefined;
-		} catch (ex) {
-			Logger.error(ex, 'CredentialManager.get: Failed to get credentials');
-			return undefined;
-		}
-	}
-
-	export async function getAs<T extends object>(key: string): Promise<T | undefined> {
-		const value = await get(key);
-		if (value == null) return undefined;
-
-		return JSON.parse(value) as T;
-	}
-
-	export function invalidate(key: string) {
-		if (!key) return;
-
-		_onDidChange.fire({ key: key, reason: 'invalid' });
-	}
-}

src/git/remotes/github.ts

@@ -1,5 +1,5 @@
 'use strict';
-import { Disposable, env, QuickInputButton, Range, ThemeIcon, Uri, window } from 'vscode';
+import { AuthenticationSession, Range, Uri } from 'vscode';
 import { DynamicAutolinkReference } from '../../annotations/autolinks';
 import { AutolinkReference } from '../../config';
 import { Container } from '../../container';
@@ -13,18 +13,12 @@ const issueEnricher3rdParyRegex = /\b(\w+\\?-?\w+(?!\\?-)\/\w+\\?-?\w+(?!\\?-))\
 const fileRegex = /^\/([^/]+)\/([^/]+?)\/blob(.+)$/i;
 const rangeRegex = /^L(\d+)(?:-L(\d+))?$/;
 
-export class GitHubRemote extends RemoteProviderWithApi<{ token: string }> {
-	private readonly Buttons = class {
-		// static readonly Help: QuickInputButton = {
-		// 	iconPath: new ThemeIcon('question'),
-		// 	tooltip: 'Help',
-		// };
+const authProvider = Object.freeze({ id: 'github', scopes: ['repo'] });
 
-		static readonly OpenPATs: QuickInputButton = {
-			iconPath: new ThemeIcon('globe'),
-			tooltip: 'Open Personal Access Tokens on GitHub',
-		};
-	};
+export class GitHubRemote extends RemoteProviderWithApi {
+	protected get authProvider() {
+		return authProvider;
+	}
 
 	constructor(domain: string, path: string, protocol?: string, name?: string, custom: boolean = false) {
 		super(domain, path, protocol, name, custom);
@@ -69,56 +63,6 @@ export class GitHubRemote extends RemoteProviderWithApi<{ token: string }> {
 		return this.formatName('GitHub');
 	}
 
-	async connect() {
-		const input = window.createInputBox();
-		input.ignoreFocusOut = true;
-
-		let disposable: Disposable | undefined;
-		let token: string | undefined;
-
-		try {
-			token = await new Promise<string | undefined>(resolve => {
-				disposable = Disposable.from(
-					input.onDidHide(() => resolve(undefined)),
-					input.onDidTriggerButton(e => {
-						if (e === this.Buttons.OpenPATs) {
-							void env.openExternal(Uri.parse('https://github.com/settings/tokens'));
-						}
-
-						// if (e === this.Buttons.Help) {
-						// 	// TODO@eamodio link to proper wiki
-						// 	void env.openExternal(Uri.parse('https://github.com/eamodio/vscode-gitlens/wiki'));
-						// }
-					}),
-					input.onDidChangeValue(
-						e =>
-							(input.validationMessage =
-								e == null || e.length === 0
-									? 'Must be a valid GitHub personal access token'
-									: undefined),
-					),
-					input.onDidAccept(() => resolve(input.value)),
-				);
-
-				// TODO@eamodio add this button once we have a valid help link above
-				input.buttons = [this.Buttons.OpenPATs]; // [this.Buttons.Help];
-				input.title = `Connect to ${this.name}`;
-				input.prompt = 'Enter a GitHub personal access token';
-				input.placeholder = 'Generate a personal access token (with repo access) from github.com (required)';
-
-				input.show();
-			});
-		} finally {
-			input.dispose();
-			disposable?.dispose();
-		}
-
-		if (token == null || token.length === 0) return false;
-
-		await this.saveCredentials({ token: token });
-		return true;
-	}
-
 	async getLocalInfoFromRemoteUri(
 		repository: Repository,
 		uri: Uri,
@@ -211,21 +155,21 @@ export class GitHubRemote extends RemoteProviderWithApi<{ token: string }> {
 	}
 
 	protected async onGetIssueOrPullRequest(
-		{ token }: { token: string },
+		{ accessToken }: AuthenticationSession,
 		id: string,
 	): Promise<IssueOrPullRequest | undefined> {
 		const [owner, repo] = this.splitPath();
-		return (await Container.github)?.getIssueOrPullRequest(this.name, token, owner, repo, Number(id), {
+		return (await Container.github)?.getIssueOrPullRequest(this.name, accessToken, owner, repo, Number(id), {
 			baseUrl: this.apiBaseUrl,
 		});
 	}
 
 	protected async onGetPullRequestForCommit(
-		{ token }: { token: string },
+		{ accessToken }: AuthenticationSession,
 		ref: string,
 	): Promise<PullRequest | undefined> {
 		const [owner, repo] = this.splitPath();
-		return (await Container.github)?.getPullRequestForCommit(this.name, token, owner, repo, ref, {
+		return (await Container.github)?.getPullRequestForCommit(this.name, accessToken, owner, repo, ref, {
 			baseUrl: this.apiBaseUrl,
 		});
 	}

src/git/remotes/provider.ts

@@ -1,9 +1,18 @@
 'use strict';
-import { env, Event, EventEmitter, Range, Uri, window } from 'vscode';
+import {
+	authentication,
+	AuthenticationSession,
+	AuthenticationSessionsChangeEvent,
+	env,
+	Event,
+	EventEmitter,
+	Range,
+	Uri,
+	window,
+} from 'vscode';
 import { DynamicAutolinkReference } from '../../annotations/autolinks';
 import { AutolinkReference } from '../../config';
 import { Container } from '../../container';
-import { CredentialChangeEvent, CredentialManager } from '../../credentials';
 import { Logger } from '../../logger';
 import { Messages } from '../../messages';
 import { IssueOrPullRequest } from '../models/issue';
@@ -12,11 +21,11 @@ import { PullRequest } from '../models/pullRequest';
 import { Repository } from '../models/repository';
 import { debug, gate, Promises } from '../../system';
 
-export class CredentialError extends Error {
+export class AuthenticationError extends Error {
 	constructor(private original: Error) {
 		super(original.message);
 
-		Error.captureStackTrace(this, CredentialError);
+		Error.captureStackTrace(this, AuthenticationError);
 	}
 }
 
@@ -202,7 +211,7 @@ export abstract class RemoteProvider {
 	}
 }
 
-export abstract class RemoteProviderWithApi<T extends object = any> extends RemoteProvider {
+export abstract class RemoteProviderWithApi extends RemoteProvider {
 	static is(provider: RemoteProvider | undefined): provider is RemoteProviderWithApi {
 		return provider instanceof RemoteProviderWithApi;
 	}
@@ -212,46 +221,39 @@ export abstract class RemoteProviderWithApi extends Remo
 		return this._onDidChange.event;
 	}
 
-	private badCredentialsCount = 0;
+	private invalidAuthenticationCount = 0;
 
 	constructor(domain: string, path: string, protocol?: string, name?: string, custom?: boolean) {
 		super(domain, path, protocol, name, custom);
 
-		Container.context.subscriptions.push(CredentialManager.onDidChange(this.onCredentialsChanged, this));
+		Container.context.subscriptions.push(
+			authentication.onDidChangeSessions(this.onAuthenticationSessionsChanged, this),
+		);
 	}
 
-	private onCredentialsChanged(e: CredentialChangeEvent) {
-		if (e.reason === 'invalid' && e.key === this.credentialsKey) {
-			this._credentials = null;
-			this._onDidChange.fire();
-
-			return;
-		}
-
-		if (e.reason === 'save' && e.key === this.credentialsKey) {
-			if (this._credentials === null) {
-				this._credentials = undefined;
-			}
-			this._onDidChange.fire();
-
-			return;
-		}
-
-		if (e.reason === 'clear' && (e.key === undefined || e.key === this.credentialsKey)) {
-			this._credentials = undefined;
-			this._prsByCommit.clear();
-
+	private onAuthenticationSessionsChanged(e: AuthenticationSessionsChangeEvent) {
+		if (e.provider.id === this.authProvider.id) {
+			this._session = null;
 			this._onDidChange.fire();
 		}
 	}
 
 	abstract get apiBaseUrl(): string;
 
-	abstract async connect(): Promise<boolean>;
+	async connect(): Promise<boolean> {
+		try {
+			const session = await this.ensureSession(true);
+			return Boolean(session);
+		} catch (ex) {
+			return false;
+		}
+	}
 
-	disconnect(): Promise<void> {
+	disconnect(): void {
 		this._prsByCommit.clear();
-		return this.clearCredentials();
+		this.invalidAuthenticationCount = 0;
+		this._session = null;
+		this._onDidChange.fire();
 	}
 
 	@gate()
@@ -259,13 +261,13 @@ export abstract class RemoteProviderWithApi extends Remo
 		exit: connected => `returned ${connected}`,
 	})
 	async isConnected(): Promise<boolean> {
-		return (await this.credentials()) != null;
+		return (await this.session()) != null;
 	}
 
 	get maybeConnected(): boolean | undefined {
-		if (this._credentials === undefined) return undefined;
+		if (this._session === undefined) return undefined;
 
-		return this._credentials !== null;
+		return this._session !== null;
 	}
 
 	@gate()
@@ -277,14 +279,14 @@ export abstract class RemoteProviderWithApi extends Remo
 		if (!connected) return undefined;
 
 		try {
-			const issueOrPullRequest = await this.onGetIssueOrPullRequest(this._credentials!, id);
-			this.badCredentialsCount = 0;
+			const issueOrPullRequest = await this.onGetIssueOrPullRequest(this._session!, id);
+			this.invalidAuthenticationCount = 0;
 			return issueOrPullRequest;
 		} catch (ex) {
 			Logger.error(ex, cc);
 
-			if (ex instanceof CredentialError) {
-				this.handleBadCredentials();
+			if (ex instanceof AuthenticationError) {
+				this.handleAuthenticationException();
 			}
 			return undefined;
 		}
@@ -305,42 +307,45 @@ export abstract class RemoteProviderWithApi extends Remo
 		return pr.then(pr => pr ?? undefined);
 	}
 
-	protected abstract onGetIssueOrPullRequest(credentials: T, id: string): Promise<IssueOrPullRequest | undefined>;
-	protected abstract onGetPullRequestForCommit(credentials: T, ref: string): Promise<PullRequest | undefined>;
-
-	protected _credentials: T | null | undefined;
-	protected credentials() {
-		if (this._credentials === undefined) {
-			return CredentialManager.getAs<T>(this.credentialsKey).then(c => {
-				this.badCredentialsCount = 0;
-				this._credentials = c ?? null;
-				return c ?? undefined;
-			});
+	protected abstract get authProvider(): { id: string; scopes: string[] };
+
+	protected abstract onGetIssueOrPullRequest(
+		session: AuthenticationSession,
+		id: string,
+	): Promise<IssueOrPullRequest | undefined>;
+	protected abstract onGetPullRequestForCommit(
+		session: AuthenticationSession,
+		ref: string,
+	): Promise<PullRequest | undefined>;
+
+	protected _session: AuthenticationSession | null | undefined;
+	protected session() {
+		if (this._session === undefined) {
+			return this.ensureSession(false);
 		}
-		return this._credentials ?? undefined;
+		return this._session ?? undefined;
 	}
 
-	protected async clearCredentials() {
-		this.badCredentialsCount = 0;
-		this._credentials = undefined;
-		await CredentialManager.clear(this.credentialsKey);
-		this._credentials = undefined;
-	}
+	private async ensureSession(createIfNone: boolean) {
+		if (this._session != null) return this._session;
 
-	protected invalidateCredentials() {
-		this.badCredentialsCount = 0;
-		this._credentials = null;
-		CredentialManager.invalidate(this.credentialsKey);
-	}
+		let session;
+		try {
+			session = await authentication.getSession(this.authProvider.id, this.authProvider.scopes, {
+				createIfNone: createIfNone,
+			});
+		} catch (ex) {
+			// TODO@eamodio	save that the user rejected auth?
+		}
 
-	protected saveCredentials(credentials: T) {
-		this.badCredentialsCount = 0;
-		this._credentials = credentials;
-		return CredentialManager.addOrUpdate(this.credentialsKey, credentials);
-	}
+		this._session = session ?? null;
+		this.invalidAuthenticationCount = 0;
+
+		if (session != null) {
+			this._onDidChange.fire();
+		}
 
-	private get credentialsKey() {
-		return this.custom ? `${this.name}:${this.domain}` : this.name;
+		return session ?? undefined;
 	}
 
 	@gate()
@@ -352,27 +357,27 @@ export abstract class RemoteProviderWithApi extends Remo
 		if (!connected) return null;
 
 		try {
-			const pr = (await this.onGetPullRequestForCommit(this._credentials!, ref)) ?? null;
+			const pr = (await this.onGetPullRequestForCommit(this._session!, ref)) ?? null;
 			this._prsByCommit.set(ref, pr);
-			this.badCredentialsCount = 0;
+			this.invalidAuthenticationCount = 0;
 			return pr;
 		} catch (ex) {
 			Logger.error(ex, cc);
 
 			this._prsByCommit.delete(ref);
 
-			if (ex instanceof CredentialError) {
-				this.handleBadCredentials();
+			if (ex instanceof AuthenticationError) {
+				this.handleAuthenticationException();
 			}
 			return null;
 		}
 	}
 
-	private handleBadCredentials() {
-		this.badCredentialsCount++;
+	private handleAuthenticationException() {
+		this.invalidAuthenticationCount++;
 
-		if (this.badCredentialsCount >= 5) {
-			this.invalidateCredentials();
+		if (this.invalidAuthenticationCount >= 5) {
+			this.disconnect();
 		}
 	}
 }

src/github/github.ts

@@ -2,7 +2,7 @@
 import { graphql } from '@octokit/graphql';
 import { Logger } from '../logger';
 import { debug } from '../system';
-import { CredentialError, IssueOrPullRequest, PullRequest, PullRequestState } from '../git/git';
+import { AuthenticationError, IssueOrPullRequest, PullRequest, PullRequestState } from '../git/git';
 
 export class GitHubApi {
 	@debug({
@@ -48,9 +48,6 @@ export class GitHubApi {
 	}
 }`;
 
-			const variables = { owner: owner, repo: repo, sha: ref };
-			// Logger.debug(cc, `variables: ${JSON.stringify(variables)}`);
-
 			const rsp = await graphql<{
 				repository?: {
 					object?: {
@@ -60,7 +57,9 @@ export class GitHubApi {
 					};
 				};
 			}>(query, {
-				...variables,
+				owner: owner,
+				repo: repo,
+				sha: ref,
 				headers: { authorization: `Bearer ${token}` },
 				...options,
 			});
@@ -88,7 +87,7 @@ export class GitHubApi {
 			Logger.error(ex, cc);
 
 			if (ex.code === 401) {
-				throw new CredentialError(ex);
+				throw new AuthenticationError(ex);
 			}
 			throw ex;
 		}
@@ -132,11 +131,10 @@ export class GitHubApi {
 	}
 }`;
 
-			const variables = { owner: owner, repo: repo, number: number };
-			// Logger.debug(cc, `variables: ${JSON.stringify(variables)}`);
-
 			const rsp = await graphql<{ repository?: { issueOrPullRequest?: GitHubIssueOrPullRequest } }>(query, {
-				...variables,
+				owner: owner,
+				repo: repo,
+				number: number,
 				headers: { authorization: `Bearer ${token}` },
 				...options,
 			});
@@ -157,7 +155,7 @@ export class GitHubApi {
 			Logger.error(ex, cc);
 
 			if (ex.code === 401) {
-				throw new CredentialError(ex);
+				throw new AuthenticationError(ex);
 			}
 			throw ex;
 		}