Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion src/github/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ export class CredentialStore extends Disposable {
private _scopes: string[] = SCOPES_OLD;
private _scopesEnterprise: string[] = SCOPES_OLD;
private _isSamling: boolean = false;
private _handlingAuthError: Map<AuthProvider, Promise<AuthResult>> = new Map();
private _lastAuthErrorHandledAt: Map<AuthProvider, number> = new Map();
// Cooldown long enough to absorb retries from in-flight requests that were
// issued with the now-invalid token, but short enough that a token that
// is invalidated again soon after re-auth will still trigger another prompt.
private static readonly AUTH_ERROR_COOLDOWN_MS = 60_000;

private _onDidChangeSessions: vscode.EventEmitter<vscode.AuthenticationSessionsChangeEvent> = new vscode.EventEmitter();
public readonly onDidChangeSessions = this._onDidChangeSessions.event;
Expand Down Expand Up @@ -285,6 +291,51 @@ export class CredentialStore extends Disposable {
return this.doCreate({ forceNewSession: reason ? { detail: reason } : true });
}

/**
* Handles authentication errors that surface from API calls (e.g. "Bad credentials"
* or 401 Unauthorized). Triggers a re-authentication prompt for the affected
* provider, deduplicating concurrent requests so we don't show multiple prompts
* when many in-flight calls fail at once.
*/
public async handleAuthError(authProviderId: AuthProvider): Promise<AuthResult> {
// Only prompt if we currently believe we are authenticated for this provider.
// Otherwise the regular sign-in flow will handle it.
if (!this.isAuthenticated(authProviderId)) {
return { canceled: true };
}
const inFlight = this._handlingAuthError.get(authProviderId);
if (inFlight) {
return inFlight;
}
// In-flight requests that were issued with the now-invalid token may continue
// to fail with auth errors for a short period after a successful re-auth.
// Suppress re-prompting for a cooldown window to avoid repeatedly nagging the
// user.
const lastHandled = this._lastAuthErrorHandledAt.get(authProviderId);
if (lastHandled !== undefined && (Date.now() - lastHandled) < CredentialStore.AUTH_ERROR_COOLDOWN_MS) {
return { canceled: true };
}
Logger.appendLine(`Detected invalid GitHub${getGitHubSuffix(authProviderId)} credentials; prompting for re-authentication.`, CredentialStore.ID);
/* __GDPR__
"auth.badCredentials" : {}
*/
this._telemetry.sendTelemetryEvent('auth.badCredentials');
const reason = vscode.l10n.t('Your GitHub{0} authentication session is no longer valid. Please sign in again.', getGitHubSuffix(authProviderId));
const promise = (async () => {
try {
// Force re-auth only for the affected provider, not both. Going through
// recreate()/doCreate() would prompt re-auth for both GitHub.com and
// GitHub Enterprise when both are configured.
return await this.initialize(authProviderId, { forceNewSession: { detail: reason } });
} finally {
this._handlingAuthError.delete(authProviderId);
this._lastAuthErrorHandledAt.set(authProviderId, Date.now());
}
})();
this._handlingAuthError.set(authProviderId, promise);
return promise;
}

public async reset() {
this._githubAPI = undefined;
this._githubEnterpriseAPI = undefined;
Expand Down Expand Up @@ -561,7 +612,9 @@ export class CredentialStore extends Disposable {
},
});

const rateLogger = new RateLogger(this._telemetry, isEnterprise(authProviderId));
const rateLogger = new RateLogger(this._telemetry, isEnterprise(authProviderId), (_e) => {
void this.handleAuthError(authProviderId);
});
const github: GitHub = {
octokit: new LoggingOctokit(octokit, rateLogger),
graphql: new LoggingApolloClient(graphql, rateLogger),
Expand Down
39 changes: 38 additions & 1 deletion src/github/loggingOctokit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,35 @@ function isObject(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null;
}

/**
* Detects whether an error from a REST (Octokit) or GraphQL (Apollo) call
* indicates that the GitHub authentication token is no longer valid. This
* happens when the token has been revoked or has expired and surfaces as
* either a 401 status, a "Bad credentials" message (REST), or a
* "401 Unauthorized" network error (GraphQL).
*/
export function isAuthError(e: unknown): boolean {
if (!isObject(e)) {
return false;
}
if (e.status === 401) {
return true;
}
const networkError = e.networkError;
if (isObject(networkError) && networkError.statusCode === 401) {
return true;
}
if (typeof e.message === 'string') {
if (e.message.includes('Bad credentials')) {
return true;
}
if (e.message.includes('401 Unauthorized')) {
return true;
}
}
return false;
}

export function getErrorCode(e: unknown): string | undefined {
if (!isObject(e)) {
return undefined;
Expand Down Expand Up @@ -94,7 +123,7 @@ export class RateLogger {
private hasLoggedLowRateLimit: boolean = false;
private readonly _isInsiders: boolean;

constructor(private readonly telemetry: ITelemetry, private readonly errorOnFlood: boolean) {
constructor(private readonly telemetry: ITelemetry, private readonly errorOnFlood: boolean, private readonly authErrorHandler?: (e: unknown) => void) {
this._isInsiders = vscode.env.appName.toLowerCase().includes('insider');
}

Expand Down Expand Up @@ -187,6 +216,14 @@ export class RateLogger {
}
*/
this.telemetry.sendTelemetryErrorEvent('pr.apiCallFailed', properties);

if (this.authErrorHandler && isAuthError(e)) {
try {
this.authErrorHandler(e);
} catch {
// Ignore errors from the auth error handler so they don't propagate.
}
}
});
}

Expand Down
99 changes: 99 additions & 0 deletions src/test/github/loggingOctokit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { default as assert } from 'assert';
import { isAuthError, RateLogger } from '../../github/loggingOctokit';
import { MockTelemetry } from '../mocks/mockTelemetry';

describe('loggingOctokit', () => {
describe('isAuthError', () => {
it('returns true for an Octokit-style 401 with "Bad credentials" message', () => {
const e: any = new Error('Bad credentials');
e.status = 401;
assert.strictEqual(isAuthError(e), true);
});

it('returns true for an error whose message is exactly "Bad credentials"', () => {
assert.strictEqual(isAuthError(new Error('Bad credentials')), true);
});

it('returns true for an error whose message includes "Bad credentials"', () => {
assert.strictEqual(isAuthError(new Error('HttpError: Bad credentials - https://docs.github.com/rest')), true);
});

it('returns true for a GraphQL networkError with statusCode 401', () => {
const e: any = new Error('Network error');
e.networkError = { statusCode: 401 };
assert.strictEqual(isAuthError(e), true);
});

it('returns true for a GraphQL error message including "401 Unauthorized"', () => {
assert.strictEqual(isAuthError(new Error('Response not successful: Received status code 401 Unauthorized')), true);
});

it('returns false for unrelated errors', () => {
const e: any = new Error('Not Found');
e.status = 404;
assert.strictEqual(isAuthError(e), false);
assert.strictEqual(isAuthError(new Error('Server Error')), false);
assert.strictEqual(isAuthError(undefined), false);
assert.strictEqual(isAuthError(null), false);
assert.strictEqual(isAuthError('Bad credentials'), false);
});
});

describe('RateLogger.logApiError', () => {
it('invokes the auth error handler when the API call fails with a 401/Bad credentials', async () => {
const telemetry = new MockTelemetry();
let handlerCalls = 0;
const handler = () => { handlerCalls++; };
const rateLogger = new RateLogger(telemetry, false, handler);

const e: any = new Error('Bad credentials');
e.status = 401;
rateLogger.logApiError('/test', Promise.reject(e));

// allow microtasks to run
await new Promise(resolve => setImmediate(resolve));
assert.strictEqual(handlerCalls, 1);
});

it('does not invoke the auth error handler for non-auth errors', async () => {
const telemetry = new MockTelemetry();
let handlerCalls = 0;
const handler = () => { handlerCalls++; };
const rateLogger = new RateLogger(telemetry, false, handler);

const e: any = new Error('Not Found');
e.status = 404;
rateLogger.logApiError('/test', Promise.reject(e));

await new Promise(resolve => setImmediate(resolve));
assert.strictEqual(handlerCalls, 0);
});

it('swallows exceptions thrown by the auth error handler', async () => {
const telemetry = new MockTelemetry();
const handler = () => { throw new Error('handler failure'); };
const rateLogger = new RateLogger(telemetry, false, handler);

const e: any = new Error('Bad credentials');
e.status = 401;
// Should not throw.
rateLogger.logApiError('/test', Promise.reject(e));
await new Promise(resolve => setImmediate(resolve));
});

it('works without an auth error handler', async () => {
const telemetry = new MockTelemetry();
const rateLogger = new RateLogger(telemetry, false);

const e: any = new Error('Bad credentials');
e.status = 401;
rateLogger.logApiError('/test', Promise.reject(e));
await new Promise(resolve => setImmediate(resolve));
});
});
});
Loading