Browse Source

Fixes #1818: adds reauthentication support

main
Eric Amodio 2 years ago
parent
commit
68ab8fc56e
3 changed files with 147 additions and 67 deletions
  1. +4
    -4
      src/errors.ts
  2. +98
    -46
      src/premium/github/github.ts
  3. +45
    -17
      src/premium/github/githubGitProvider.ts

+ 4
- 4
src/errors.ts View File

@ -56,7 +56,7 @@ export class AuthenticationError extends Error {
let message;
let reason: AuthenticationErrorReason | undefined;
if (messageOrReason == null) {
message = `Unable to get required authentication session for '${id}`;
message = `Unable to get required authentication session for '${id}'`;
} else if (typeof messageOrReason === 'string') {
message = messageOrReason;
reason = undefined;
@ -64,13 +64,13 @@ export class AuthenticationError extends Error {
reason = messageOrReason;
switch (reason) {
case AuthenticationErrorReason.UserDidNotConsent:
message = `'${id} authentication is required for this operation`;
message = `'${id}' authentication is required for this operation`;
break;
case AuthenticationErrorReason.Unauthorized:
message = `The provided '${id}' credentials are either invalid or expired`;
message = `Your '${id}' credentials are either invalid or expired`;
break;
case AuthenticationErrorReason.Forbidden:
message = `The provided '${id}' credentials do not have the required access`;
message = `Your '${id}' credentials do not have the required access`;
break;
}
}

+ 98
- 46
src/premium/github/github.ts View File

@ -2,7 +2,7 @@ import { Octokit } from '@octokit/core';
import { GraphqlResponseError } from '@octokit/graphql';
import { RequestError } from '@octokit/request-error';
import type { Endpoints, OctokitResponse, RequestParameters } from '@octokit/types';
import { window } from 'vscode';
import { Event, EventEmitter, window } from 'vscode';
import { fetch } from '@env/fetch';
import { isWeb } from '@env/platform';
import {
@ -32,6 +32,11 @@ const emptyPagedResult: PagedResult = Object.freeze({ values: [] });
const emptyBlameResult: GitHubBlame = Object.freeze({ ranges: [] });
export class GitHubApi {
private readonly _onDidReauthenticate = new EventEmitter<void>();
get onDidReauthenticate(): Event<void> {
return this._onDidReauthenticate.event;
}
@debug<GitHubApi['getAccountForCommit']>({ args: { 0: p => p.name, 1: '<token>' } })
async getAccountForCommit(
provider: RichRemoteProvider,
@ -102,7 +107,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -170,7 +175,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -222,7 +227,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -292,7 +297,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -392,7 +397,7 @@ export class GitHubApi {
return GitHubPullRequest.from(prs[0], provider);
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -485,7 +490,7 @@ export class GitHubApi {
return GitHubPullRequest.from(prs[0], provider);
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -561,7 +566,7 @@ export class GitHubApi {
return { ranges: ranges, viewer: rsp.viewer?.name };
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, emptyBlameResult);
return this.handleException(ex, cc, emptyBlameResult);
}
}
@ -639,7 +644,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, emptyPagedResult);
return this.handleException(ex, cc, emptyPagedResult);
}
}
@ -685,7 +690,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
// const results = await this.getCommits(token, owner, repo, ref, { limit: 1 });
@ -777,7 +782,7 @@ export class GitHubApi {
return branches;
} catch (ex) {
debugger;
return this.handleRequestError<string[]>(ex, cc, []);
return this.handleException<string[]>(ex, cc, []);
}
}
@ -824,7 +829,7 @@ export class GitHubApi {
return count;
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -893,7 +898,7 @@ export class GitHubApi {
return branches;
} catch (ex) {
debugger;
return this.handleRequestError<string[]>(ex, cc, []);
return this.handleException<string[]>(ex, cc, []);
}
}
@ -1031,7 +1036,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, emptyPagedResult);
return this.handleException(ex, cc, emptyPagedResult);
}
}
@ -1091,7 +1096,7 @@ export class GitHubApi {
return commit != null ? { values: [commit], viewer: rsp.viewer.name } : emptyPagedResult;
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, emptyPagedResult);
return this.handleException(ex, cc, emptyPagedResult);
}
}
@ -1179,7 +1184,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -1257,7 +1262,7 @@ export class GitHubApi {
return date;
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -1280,7 +1285,7 @@ export class GitHubApi {
return rsp.data;
} catch (ex) {
debugger;
return this.handleRequestError<GitHubContributor[]>(ex, cc, []);
return this.handleException<GitHubContributor[]>(ex, cc, []);
}
}
@ -1318,7 +1323,7 @@ export class GitHubApi {
return rsp.repository?.defaultBranchRef?.name ?? undefined;
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -1359,7 +1364,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -1399,7 +1404,7 @@ export class GitHubApi {
return rsp.repository.visibility === 'PUBLIC' ? RepositoryVisibility.Public : RepositoryVisibility.Private;
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -1482,7 +1487,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, emptyPagedResult);
return this.handleException(ex, cc, emptyPagedResult);
}
}
@ -1561,7 +1566,7 @@ export class GitHubApi {
return rsp?.repository?.object?.history.nodes?.[0]?.oid ?? undefined;
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -1639,7 +1644,7 @@ export class GitHubApi {
};
} catch (ex) {
debugger;
return this.handleRequestError(ex, cc, undefined);
return this.handleException(ex, cc, undefined);
}
}
@ -1704,9 +1709,14 @@ export class GitHubApi {
case 'FORBIDDEN':
throw new AuthenticationError('github', AuthenticationErrorReason.Forbidden, ex);
}
void window.showErrorMessage(`GitHub request failed: ${ex.errors?.[0]?.message ?? ex.message}`, 'OK');
} else if (ex instanceof RequestError) {
this.handleRequestError(ex);
} else {
void window.showErrorMessage(`GitHub request failed: ${ex.message}`, 'OK');
}
void window.showErrorMessage(`Unable to complete GitHub request: ${ex.message}`);
throw ex;
}
}
@ -1720,39 +1730,81 @@ export class GitHubApi {
return (await this.octokit(token).request<R>(route, options)) as any;
} catch (ex) {
if (ex instanceof RequestError) {
switch (ex.status) {
case 404: // Not found
case 410: // Gone
case 422: // Unprocessable Entity
throw new ProviderRequestNotFoundError(ex);
// case 429: //Too Many Requests
case 401: // Unauthorized
throw new AuthenticationError('github', AuthenticationErrorReason.Unauthorized, ex);
case 403: // Forbidden
throw new AuthenticationError('github', AuthenticationErrorReason.Forbidden, ex);
case 500: // Internal Server Error
if (ex.response != null) {
// TODO@eamodio: Handle GitHub down errors
}
break;
default:
if (ex.status >= 400 && ex.status < 500) throw new ProviderRequestClientError(ex);
break;
}
this.handleRequestError(ex);
} else {
void window.showErrorMessage(`GitHub request failed: ${ex.message}`, 'OK');
}
void window.showErrorMessage(`Unable to complete GitHub request: ${ex.message}`);
throw ex;
}
}
private handleRequestError<T>(ex: unknown | Error, cc: LogCorrelationContext | undefined, defaultValue: T): T {
private handleRequestError(ex: RequestError): void {
switch (ex.status) {
case 404: // Not found
case 410: // Gone
case 422: // Unprocessable Entity
throw new ProviderRequestNotFoundError(ex);
// case 429: //Too Many Requests
case 401: // Unauthorized
throw new AuthenticationError('github', AuthenticationErrorReason.Unauthorized, ex);
case 403: // Forbidden
throw new AuthenticationError('github', AuthenticationErrorReason.Forbidden, ex);
case 500: // Internal Server Error
if (ex.response != null) {
void window.showErrorMessage(
'GitHub failed to respond and might be experiencing issues. Please visit the [GitHub status page](https://githubstatus.com) for more information.',
'OK',
);
}
break;
case 502: // Bad Gateway
// GitHub seems to return this status code for timeouts
if (ex.message.includes('timeout')) {
void window.showErrorMessage('GitHub request timed out', 'OK');
return;
}
break;
default:
if (ex.status >= 400 && ex.status < 500) throw new ProviderRequestClientError(ex);
break;
}
void window.showErrorMessage(
`GitHub request failed: ${(ex.response as any)?.errors?.[0]?.message ?? ex.message}`,
'OK',
);
}
private handleException<T>(ex: unknown | Error, cc: LogCorrelationContext | undefined, defaultValue: T): T {
if (ex instanceof ProviderRequestNotFoundError) return defaultValue;
Logger.error(ex, cc);
debugger;
if (ex instanceof AuthenticationError) {
void this.showAuthenticationErrorMessage(ex);
}
throw ex;
}
private async showAuthenticationErrorMessage(ex: AuthenticationError) {
if (ex.reason === AuthenticationErrorReason.Unauthorized || ex.reason === AuthenticationErrorReason.Forbidden) {
const confirm = 'Reauthenticate';
const result = await window.showErrorMessage(
`${ex.message}. Would you like to try reauthenticating${
ex.reason === AuthenticationErrorReason.Forbidden ? ' to provide additional access' : ''
}?`,
confirm,
);
if (result === confirm) {
this._onDidReauthenticate.fire();
}
} else {
void window.showErrorMessage(ex.message, 'OK');
}
}
}
export interface GitHubBlame {

+ 45
- 17
src/premium/github/githubGitProvider.ts View File

@ -128,9 +128,13 @@ export class GitHubGitProvider implements GitProvider, Disposable {
private readonly _repoInfoCache = new Map<string, RepositoryInfo>();
private readonly _tagsCache = new Map<string, Promise<PagedResult<GitTag>>>();
private readonly _disposables: Disposable[] = [];
constructor(private readonly container: Container) {}
dispose() {}
dispose() {
this._disposables.forEach(d => d.dispose());
}
private onRepositoryChanged(repo: Repository, e: RepositoryChangeEvent) {
// if (e.changed(RepositoryChange.Config, RepositoryChangeComparisonMode.Any)) {
@ -493,7 +497,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
const ref = !uri.sha || uri.sha === 'HEAD' ? (await metadata.getRevision()).revision : uri.sha;
const blame = await github.getBlame(
session?.accessToken,
session.accessToken,
metadata.repo.owner,
metadata.repo.name,
ref,
@ -630,7 +634,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
const ref = !uri.sha || uri.sha === 'HEAD' ? (await metadata.getRevision()).revision : uri.sha;
const blame = await github.getBlame(
session?.accessToken,
session.accessToken,
metadata.repo.owner,
metadata.repo.name,
ref,
@ -796,7 +800,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
while (true) {
const result = await github.getBranches(
session?.accessToken,
session.accessToken,
metadata.repo.owner,
metadata.repo.name,
{ cursor: cursor },
@ -879,7 +883,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
try {
const { metadata, github, session } = await this.ensureRepositoryContext(repoPath);
const commit = await github.getCommit(session?.accessToken, metadata.repo.owner, metadata.repo.name, ref);
const commit = await github.getCommit(session.accessToken, metadata.repo.owner, metadata.repo.name, ref);
if (commit == null) return undefined;
const { viewer = session.account.label } = commit;
@ -942,7 +946,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
if (options?.branch) {
branches = await github.getCommitOnBranch(
session?.accessToken,
session.accessToken,
metadata.repo.owner,
metadata.repo.name,
options?.branch,
@ -951,7 +955,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
);
} else {
branches = await github.getCommitBranches(
session?.accessToken,
session.accessToken,
metadata.repo.owner,
metadata.repo.name,
ref,
@ -1008,7 +1012,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
const ref = !options?.ref || options.ref === 'HEAD' ? (await metadata.getRevision()).revision : options.ref;
const commit = await github.getCommitForFile(
session?.accessToken,
session.accessToken,
metadata.repo.owner,
metadata.repo.name,
ref,
@ -1080,7 +1084,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
try {
const { metadata, github, session } = await this.ensureRepositoryContext(repoPath);
const results = await github.getContributors(session?.accessToken, metadata.repo.owner, metadata.repo.name);
const results = await github.getContributors(session.accessToken, metadata.repo.owner, metadata.repo.name);
const currentUser = await this.getCurrentUser(repoPath);
const contributors = [];
@ -1127,7 +1131,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
try {
const { metadata, github, session } = await this.ensureRepositoryContext(repoPath);
user = await github.getCurrentUser(session?.accessToken, metadata.repo.owner, metadata.repo.name);
user = await github.getCurrentUser(session.accessToken, metadata.repo.owner, metadata.repo.name);
this._repoInfoCache.set(repoPath, { ...repo, user: user ?? null });
return user;
@ -1149,7 +1153,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
try {
const { metadata, github, session } = await this.ensureRepositoryContext(repoPath);
return await github.getDefaultBranchName(session?.accessToken, metadata.repo.owner, metadata.repo.name);
return await github.getDefaultBranchName(session.accessToken, metadata.repo.owner, metadata.repo.name);
} catch (ex) {
Logger.error(ex, cc);
debugger;
@ -1229,7 +1233,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
const { metadata, github, session } = await this.ensureRepositoryContext(repoPath);
const ref = !options?.ref || options.ref === 'HEAD' ? (await metadata.getRevision()).revision : options.ref;
const result = await github.getCommits(session?.accessToken, metadata.repo.owner, metadata.repo.name, ref, {
const result = await github.getCommits(session.accessToken, metadata.repo.owner, metadata.repo.name, ref, {
all: options?.all,
authors: options?.authors,
after: options?.cursor,
@ -1462,7 +1466,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
const { metadata, github, session } = await this.ensureRepositoryContext(repoPath);
const result = await github.searchCommits(
session?.accessToken,
session.accessToken,
`repo:${metadata.repo.owner}/${metadata.repo.name}+${query.join('+').trim()}`,
{
cursor: options?.cursor,
@ -1783,7 +1787,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
// }
const ref = !options?.ref || options.ref === 'HEAD' ? (await metadata.getRevision()).revision : options.ref;
const result = await github.getCommits(session?.accessToken, metadata.repo.owner, metadata.repo.name, ref, {
const result = await github.getCommits(session.accessToken, metadata.repo.owner, metadata.repo.name, ref, {
all: options?.all,
after: options?.cursor,
path: relativePath,
@ -2245,7 +2249,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
while (true) {
const result = await github.getTags(
session?.accessToken,
session.accessToken,
metadata.repo.owner,
metadata.repo.name,
{ cursor: cursor },
@ -2521,7 +2525,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
let github;
let session;
try {
[github, session] = await Promise.all([this.container.github, this.ensureSession()]);
[github, session] = await Promise.all([this.ensureGitHub(), this.ensureSession()]);
} catch (ex) {
debugger;
if (ex instanceof AuthenticationError) {
@ -2544,6 +2548,24 @@ export class GitHubGitProvider implements GitProvider, Disposable {
return { github: github, metadata: metadata, remotehub: remotehub, session: session };
}
private _github: GitHubApi | undefined;
@gate()
private async ensureGitHub() {
if (this._github == null) {
const github = await this.container.github;
if (github != null) {
this._disposables.push(
github.onDidReauthenticate(() => {
this._sessionPromise = undefined;
void this.ensureSession(true);
}),
);
}
this._github = github;
}
return this._github;
}
/** Only use this if you NEED non-promise access to RemoteHub */
private _remotehub: RemoteHubApi | undefined;
private _remotehubPromise: Promise<RemoteHubApi> | undefined;
@ -2570,10 +2592,16 @@ export class GitHubGitProvider implements GitProvider, Disposable {
}
private _sessionPromise: Promise<AuthenticationSession> | undefined;
private async ensureSession(): Promise<AuthenticationSession> {
private async ensureSession(force: boolean = false): Promise<AuthenticationSession> {
if (this._sessionPromise == null) {
async function getSession(): Promise<AuthenticationSession> {
try {
if (force) {
return await authentication.getSession('github', githubAuthenticationScopes, {
forceNewSession: true,
});
}
return await authentication.getSession('github', githubAuthenticationScopes, {
createIfNone: true,
});

Loading…
Cancel
Save