Detect stale GitHub credentials and prompt re-authentication (#8728)

* Initial plan

* Detect "Bad credentials"/401 errors and trigger re-auth

Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/d7437db6-d6f8-4e32-80f2-d39c6660a4f2

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>

* handleAuthError: re-auth only the affected provider

Agent-Logs-Url: https://github.com/microsoft/vscode-pull-request-github/sessions/56935e9d-de2d-4ee5-a5b6-44ba4fb409b4

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>

Author: Copilot

src/github/credentials.ts

@@ -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;
@@ -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;
@@ -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),

src/github/loggingOctokit.ts

@@ -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;
@@ -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');
 	}
 
@@ -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.
+				}
+			}
 		});
 	}
 

src/test/github/loggingOctokit.test.ts

@@ -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));
+		});
+	});
+});