diff --git a/src/credentials.ts b/src/credentials.ts deleted file mode 100644 index c001b90..0000000 --- a/src/credentials.ts +++ /dev/null @@ -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(moduleName: string): T | undefined { - // eslint-disable-next-line no-eval - const vscodeRequire = eval('require'); - try { - return vscodeRequire(moduleName); - } catch { - return undefined; - } -} - -const keychain = getNodeModule('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(); - export const onDidChange: Event = _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 { - 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(key: string): Promise { - 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' }); - } -} diff --git a/src/git/remotes/github.ts b/src/git/remotes/github.ts index 43a5653..b06d570 100644 --- a/src/git/remotes/github.ts +++ b/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(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 { 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 { 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, }); } diff --git a/src/git/remotes/provider.ts b/src/git/remotes/provider.ts index 6c4e78d..130a34b 100644 --- a/src/git/remotes/provider.ts +++ b/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 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; + async connect(): Promise { + try { + const session = await this.ensureSession(true); + return Boolean(session); + } catch (ex) { + return false; + } + } - disconnect(): Promise { + 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 { - 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; - protected abstract onGetPullRequestForCommit(credentials: T, ref: string): Promise; - - protected _credentials: T | null | undefined; - protected credentials() { - if (this._credentials === undefined) { - return CredentialManager.getAs(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; + protected abstract onGetPullRequestForCommit( + session: AuthenticationSession, + ref: string, + ): Promise; + + 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(); } } } diff --git a/src/github/github.ts b/src/github/github.ts index b02e454..3913413 100644 --- a/src/github/github.ts +++ b/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; }