diff --git a/jest.config.js b/jest.config.js index 679a18e0..ba7f4609 100755 --- a/jest.config.js +++ b/jest.config.js @@ -2,7 +2,8 @@ module.exports = { preset: "ts-jest/presets/default-esm", moduleNameMapper: { - "^(\\.{1,2}/.*)\\.js$": "$1" + "^(\\.{1,2}/.*)\\.js$": "$1", + "^vscode$": "/src/test/vscode.ts" }, transform: { "^.+\\.tsx?$": [ diff --git a/package.json b/package.json index 8b4e7dd9..d9ddadb5 100644 --- a/package.json +++ b/package.json @@ -310,10 +310,15 @@ "contents": "Sign in to GitHub to display runs, workflows, and configure Actions settings.\n[Sign in to GitHub](command:github-actions.sign-in)", "when": "!github-actions.signed-in" }, + { + "view": "github-actions.empty-view", + "contents": "GitHub rejected the saved sign-in. It may have expired or been revoked.\n[Sign in again](command:github-actions.sign-in)", + "when": "github-actions.signed-in && github-actions.auth-invalid" + }, { "view": "github-actions.empty-view", "contents": "Unable to connect to the GitHub API, please check your internet connection.\n[Refresh](command:github-actions.explorer.refresh)", - "when": "github-actions.signed-in && !github-actions.internet-access" + "when": "github-actions.signed-in && !github-actions.internet-access && !github-actions.auth-invalid" }, { "view": "github-actions.empty-view", diff --git a/src/api/api.ts b/src/api/api.ts index 1ac4b7b4..abd05d89 100644 --- a/src/api/api.ts +++ b/src/api/api.ts @@ -1,13 +1,24 @@ import {Octokit} from "@octokit/rest"; import {version} from "../../package.json"; import {getGitHubApiUri} from "../configuration/configuration"; +import {isUnauthorizedError, notifyUnauthorized} from "./authErrors"; export const userAgent = `VS Code GitHub Actions (${version})`; export function getClient(token: string): Octokit { - return new Octokit({ + const client = new Octokit({ auth: token, userAgent: userAgent, baseUrl: getGitHubApiUri() }); + + // Surface rejected tokens wherever they happen, so the user is offered a way to sign in again + client.hook.error("request", error => { + if (isUnauthorizedError(error)) { + notifyUnauthorized(); + } + throw error; + }); + + return client; } diff --git a/src/api/authErrors.test.ts b/src/api/authErrors.test.ts new file mode 100644 index 00000000..36bc84e5 --- /dev/null +++ b/src/api/authErrors.test.ts @@ -0,0 +1,33 @@ +import {describe, expect, it, jest} from "@jest/globals"; +import {isUnauthorizedError, notifyUnauthorized, onUnauthorized} from "./authErrors"; + +describe("isUnauthorizedError", () => { + it("detects a 401 response", () => { + expect(isUnauthorizedError(Object.assign(new Error("Bad credentials"), {status: 401}))).toBe(true); + }); + + it("ignores other response statuses", () => { + expect(isUnauthorizedError(Object.assign(new Error("Forbidden"), {status: 403}))).toBe(false); + expect(isUnauthorizedError(Object.assign(new Error("Server error"), {status: 500}))).toBe(false); + }); + + it("ignores errors without a status", () => { + expect(isUnauthorizedError(new Error("getaddrinfo ENOTFOUND api.github.com"))).toBe(false); + expect(isUnauthorizedError(undefined)).toBe(false); + expect(isUnauthorizedError("401")).toBe(false); + }); +}); + +describe("onUnauthorized", () => { + it("notifies listeners until they are disposed", () => { + const listener = jest.fn(); + const subscription = onUnauthorized(listener); + + notifyUnauthorized(); + expect(listener).toHaveBeenCalledTimes(1); + + subscription.dispose(); + notifyUnauthorized(); + expect(listener).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/api/authErrors.ts b/src/api/authErrors.ts new file mode 100644 index 00000000..b11687c0 --- /dev/null +++ b/src/api/authErrors.ts @@ -0,0 +1,26 @@ +type UnauthorizedListener = () => void; + +const listeners: UnauthorizedListener[] = []; + +/** + * Whether the error is a GitHub API response rejecting the session token (e.g., it was revoked or has expired) + */ +export function isUnauthorizedError(error: unknown): boolean { + return typeof error === "object" && error !== null && (error as {status?: unknown}).status === 401; +} + +export function onUnauthorized(listener: UnauthorizedListener): {dispose: () => void} { + listeners.push(listener); + return { + dispose: () => { + const index = listeners.indexOf(listener); + if (index !== -1) { + listeners.splice(index, 1); + } + } + }; +} + +export function notifyUnauthorized() { + listeners.forEach(l => l()); +} diff --git a/src/api/canReachGitHubAPI.test.ts b/src/api/canReachGitHubAPI.test.ts new file mode 100644 index 00000000..15fdf94d --- /dev/null +++ b/src/api/canReachGitHubAPI.test.ts @@ -0,0 +1,54 @@ +import {beforeEach, describe, expect, it, jest} from "@jest/globals"; + +const getSession = jest.fn<() => Promise>(); +const request = jest.fn<() => Promise>(); + +jest.unstable_mockModule("../auth/auth", () => ({getSession})); +jest.unstable_mockModule("../log", () => ({logError: jest.fn()})); +jest.unstable_mockModule("./api", () => ({getClient: () => ({request})})); + +const {getGitHubAPIStatus, resetGitHubAPIStatus} = await import("./canReachGitHubAPI"); + +describe("getGitHubAPIStatus", () => { + beforeEach(() => { + resetGitHubAPIStatus(); + getSession.mockReset().mockResolvedValue({accessToken: "token"}); + request.mockReset(); + }); + + it("returns signed-out without a session", async () => { + getSession.mockResolvedValue(undefined); + + expect(await getGitHubAPIStatus()).toBe("signed-out"); + expect(request).not.toHaveBeenCalled(); + }); + + it("returns ok when the API accepts the token", async () => { + request.mockResolvedValue({status: 200}); + + expect(await getGitHubAPIStatus()).toBe("ok"); + }); + + it("returns unauthorized when the API rejects the token", async () => { + request.mockRejectedValue(Object.assign(new Error("Bad credentials"), {status: 401})); + + expect(await getGitHubAPIStatus()).toBe("unauthorized"); + }); + + it("returns unreachable for other failures", async () => { + request.mockRejectedValue(new Error("getaddrinfo ENOTFOUND api.github.com")); + + expect(await getGitHubAPIStatus()).toBe("unreachable"); + }); + + it("caches the status until it is reset", async () => { + request.mockRejectedValue(Object.assign(new Error("Bad credentials"), {status: 401})); + expect(await getGitHubAPIStatus()).toBe("unauthorized"); + + request.mockResolvedValue({status: 200}); + expect(await getGitHubAPIStatus()).toBe("unauthorized"); + + resetGitHubAPIStatus(); + expect(await getGitHubAPIStatus()).toBe("ok"); + }); +}); diff --git a/src/api/canReachGitHubAPI.ts b/src/api/canReachGitHubAPI.ts index 96af6d6c..6738e35a 100644 --- a/src/api/canReachGitHubAPI.ts +++ b/src/api/canReachGitHubAPI.ts @@ -3,23 +3,40 @@ import {TTLCache} from "@actions/languageserver/utils/cache"; import {getSession} from "../auth/auth"; import {logError} from "../log"; import {getClient} from "./api"; +import {isUnauthorizedError} from "./authErrors"; const API_ACCESS_TTL_MS = 10 * 1000; const cache = new TTLCache(API_ACCESS_TTL_MS); -export async function canReachGitHubAPI() { +/** + * - `ok`: the API accepted the session token + * - `signed-out`: there is no session + * - `unauthorized`: the API rejected the session token (e.g., it was revoked or has expired) + * - `unreachable`: any other failure, e.g., no network connection + */ +export type GitHubAPIStatus = "ok" | "signed-out" | "unauthorized" | "unreachable"; + +export async function getGitHubAPIStatus(): Promise { const session = await getSession(); if (!session) { - return false; + return "signed-out"; } - return await cache.get("canReachGitHubAPI", undefined, async () => { + return await cache.get("gitHubAPIStatus", undefined, async (): Promise => { try { const octokit = getClient(session.accessToken); await octokit.request("GET /"); } catch (e) { logError(e as Error, "Error getting GitHub context"); - return false; + return isUnauthorizedError(e) ? "unauthorized" : "unreachable"; } - return true; + return "ok"; }); } + +export async function canReachGitHubAPI() { + return (await getGitHubAPIStatus()) === "ok"; +} + +export function resetGitHubAPIStatus() { + cache.clear(); +} diff --git a/src/auth/auth.ts b/src/auth/auth.ts index d35e7355..38d647b8 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -69,8 +69,11 @@ async function getSessionInternal( typeof createOrForceMessage === "string" ? {forceNewSession: {detail: createOrForceMessage}} : {createIfNone: createOrForceMessage}; - const authProviderId = useEnterprise() ? AUTH_PROVIDER_ID_ENTERPRISE : AUTH_PROVIDER_ID; - return await vscode.authentication.getSession(authProviderId, getScopes(), options); + return await vscode.authentication.getSession(getAuthProviderId(), getScopes(), options); +} + +export function getAuthProviderId(): string { + return useEnterprise() ? AUTH_PROVIDER_ID_ENTERPRISE : AUTH_PROVIDER_ID; } function getScopes(): string[] { diff --git a/src/auth/authState.ts b/src/auth/authState.ts new file mode 100644 index 00000000..202b38b8 --- /dev/null +++ b/src/auth/authState.ts @@ -0,0 +1,85 @@ +import * as vscode from "vscode"; + +import {onUnauthorized} from "../api/authErrors"; +import {GitHubAPIStatus, getGitHubAPIStatus, resetGitHubAPIStatus} from "../api/canReachGitHubAPI"; +import {getGitHubContext, resetGitHubContext} from "../git/repository"; +import {logError} from "../log"; +import {restartLanguageServer} from "../workflow/languageServer"; +import {getAuthProviderId} from "./auth"; + +let unauthorizedWarningShown = false; +let sessionChange: Promise = Promise.resolve(); + +/** + * Checks the session against the GitHub API and updates the context keys that control which views are shown + */ +export async function updateContextKeys(): Promise<{status: GitHubAPIStatus; hasGitHubRepos: boolean}> { + const status = await getGitHubAPIStatus(); + const ghContext = status === "ok" ? await getGitHubContext() : undefined; + const hasGitHubRepos = !!ghContext && ghContext.repos.length > 0; + + await Promise.all([ + vscode.commands.executeCommand("setContext", "github-actions.signed-in", status !== "signed-out"), + vscode.commands.executeCommand("setContext", "github-actions.internet-access", status === "ok"), + vscode.commands.executeCommand("setContext", "github-actions.auth-invalid", status === "unauthorized"), + vscode.commands.executeCommand("setContext", "github-actions.has-repos", hasGitHubRepos) + ]); + + if (status !== "unauthorized") { + unauthorizedWarningShown = false; + } + + return {status, hasGitHubRepos}; +} + +export function initAuthStateTracking(context: vscode.ExtensionContext) { + context.subscriptions.push( + onUnauthorized(() => { + handleUnauthorized().catch(e => logError(e as Error, "Error handling rejected GitHub session")); + }) + ); + + context.subscriptions.push( + vscode.authentication.onDidChangeSessions(e => { + if (e.provider.id !== getAuthProviderId()) { + return; + } + + // Serialize handling so overlapping events don't restart the language server concurrently + sessionChange = sessionChange + .then(() => handleSessionChange(context)) + .catch(e => logError(e as Error, "Error handling GitHub session change")); + }) + ); +} + +async function handleUnauthorized() { + await Promise.all([ + vscode.commands.executeCommand("setContext", "github-actions.auth-invalid", true), + vscode.commands.executeCommand("setContext", "github-actions.internet-access", false) + ]); + + if (unauthorizedWarningShown) { + return; + } + unauthorizedWarningShown = true; + + const signInAction = "Sign in again"; + const result = await vscode.window.showWarningMessage( + "GitHub rejected the saved sign-in for GitHub Actions. It may have expired or been revoked.", + signInAction + ); + if (result === signInAction) { + await vscode.commands.executeCommand("github-actions.sign-in"); + } +} + +async function handleSessionChange(context: vscode.ExtensionContext) { + resetGitHubAPIStatus(); + resetGitHubContext(); + + // The language server only receives the token on initialization + await restartLanguageServer(context); + + await vscode.commands.executeCommand("github-actions.explorer.refresh"); +} diff --git a/src/commands/signIn.test.ts b/src/commands/signIn.test.ts new file mode 100644 index 00000000..fe41d449 --- /dev/null +++ b/src/commands/signIn.test.ts @@ -0,0 +1,90 @@ +import {beforeEach, describe, expect, it, jest} from "@jest/globals"; + +type Handler = () => Promise; + +const handlers = new Map(); +const getSession = jest.fn<(createIfNone?: boolean) => Promise>(); +const newSession = jest.fn<(message: string) => Promise>(); +const getGitHubAPIStatus = jest.fn<() => Promise>(); +const resetGitHubAPIStatus = jest.fn(); +const resetGitHubContext = jest.fn(); +const updateContextKeys = jest.fn<() => Promise>(); + +jest.unstable_mockModule("vscode", () => ({ + commands: { + registerCommand: (id: string, handler: Handler) => { + handlers.set(id, handler); + return {dispose: jest.fn()}; + } + } +})); +jest.unstable_mockModule("../auth/auth", () => ({getSession, newSession})); +jest.unstable_mockModule("../auth/authState", () => ({updateContextKeys})); +jest.unstable_mockModule("../api/canReachGitHubAPI", () => ({getGitHubAPIStatus, resetGitHubAPIStatus})); +jest.unstable_mockModule("../git/repository", () => ({resetGitHubContext})); +jest.unstable_mockModule("../log", () => ({logError: jest.fn()})); + +const {registerSignIn} = await import("./signIn"); + +async function signIn() { + registerSignIn({subscriptions: []} as never); + await handlers.get("github-actions.sign-in")?.(); +} + +describe("github-actions.sign-in", () => { + beforeEach(() => { + jest.clearAllMocks(); + getSession.mockResolvedValue({accessToken: "token"}); + newSession.mockResolvedValue({accessToken: "new-token"}); + updateContextKeys.mockResolvedValue({status: "ok", hasGitHubRepos: true}); + }); + + it("keeps a session the API accepts", async () => { + getGitHubAPIStatus.mockResolvedValue("ok"); + + await signIn(); + + expect(getSession).toHaveBeenCalledWith(true); + expect(newSession).not.toHaveBeenCalled(); + expect(updateContextKeys).toHaveBeenCalled(); + }); + + it("forces a new session when the API rejects the token", async () => { + getGitHubAPIStatus.mockResolvedValue("unauthorized"); + + await signIn(); + + expect(newSession).toHaveBeenCalled(); + expect(resetGitHubAPIStatus).toHaveBeenCalled(); + expect(resetGitHubContext).toHaveBeenCalled(); + expect(updateContextKeys).toHaveBeenCalled(); + }); + + it("does not force a new session when the API is unreachable", async () => { + getGitHubAPIStatus.mockResolvedValue("unreachable"); + + await signIn(); + + expect(newSession).not.toHaveBeenCalled(); + expect(updateContextKeys).toHaveBeenCalled(); + }); + + it("stops when the user cancels signing in again", async () => { + getGitHubAPIStatus.mockResolvedValue("unauthorized"); + newSession.mockRejectedValue(new Error("Cancelled")); + + await signIn(); + + expect(resetGitHubAPIStatus).not.toHaveBeenCalled(); + expect(updateContextKeys).not.toHaveBeenCalled(); + }); + + it("does nothing without a session", async () => { + getSession.mockResolvedValue(undefined); + + await signIn(); + + expect(getGitHubAPIStatus).not.toHaveBeenCalled(); + expect(updateContextKeys).not.toHaveBeenCalled(); + }); +}); diff --git a/src/commands/signIn.ts b/src/commands/signIn.ts index 7d09d1fb..2fdc438c 100644 --- a/src/commands/signIn.ts +++ b/src/commands/signIn.ts @@ -1,21 +1,33 @@ import * as vscode from "vscode"; -import {getSession} from "../auth/auth"; -import {canReachGitHubAPI} from "../api/canReachGitHubAPI"; -import {getGitHubContext} from "../git/repository"; +import {getSession, newSession} from "../auth/auth"; +import {updateContextKeys} from "../auth/authState"; +import {getGitHubAPIStatus, resetGitHubAPIStatus} from "../api/canReachGitHubAPI"; +import {resetGitHubContext} from "../git/repository"; +import {logError} from "../log"; export function registerSignIn(context: vscode.ExtensionContext) { context.subscriptions.push( vscode.commands.registerCommand("github-actions.sign-in", async () => { const session = await getSession(true); - if (session) { - const canReachAPI = await canReachGitHubAPI(); - const ghContext = await getGitHubContext(); - const hasGitHubRepos = ghContext && ghContext.repos.length > 0; + if (!session) { + return; + } + + // A cached session can hold a token that GitHub no longer accepts, so asking for a session + // again would return the same one. Force a new session instead. + if ((await getGitHubAPIStatus()) === "unauthorized") { + try { + await newSession("GitHub rejected the saved sign-in. Sign in again to continue."); + } catch (e) { + logError(e as Error, "Error signing in again"); + return; + } - await vscode.commands.executeCommand("setContext", "github-actions.signed-in", true); - await vscode.commands.executeCommand("setContext", "github-actions.internet-access", canReachAPI); - await vscode.commands.executeCommand("setContext", "github-actions.has-repos", hasGitHubRepos); + resetGitHubAPIStatus(); + resetGitHubContext(); } + + await updateContextKeys(); }) ); } diff --git a/src/configuration/configuration.ts b/src/configuration/configuration.ts index 567a1981..5b3a8c64 100644 --- a/src/configuration/configuration.ts +++ b/src/configuration/configuration.ts @@ -1,5 +1,5 @@ import * as vscode from "vscode"; -import {deactivateLanguageServer, initLanguageServer} from "../workflow/languageServer"; +import {restartLanguageServer} from "../workflow/languageServer"; import {resetGitHubContext} from "../git/repository"; const settingsKey = "github-actions"; @@ -21,7 +21,7 @@ export function initConfiguration(context: vscode.ExtensionContext) { (useEnterprise() && (e.affectsConfiguration("github-enterprise.uri") || e.affectsConfiguration(getSettingsKey("remote-name")))) ) { - await updateLanguageServerApiUrl(context); + await restartLanguageServer(context); resetGitHubContext(); await vscode.commands.executeCommand("github-actions.explorer.refresh"); } @@ -96,12 +96,6 @@ export function getGitHubApiUri(): string { } } -async function updateLanguageServerApiUrl(context: vscode.ExtensionContext) { - await deactivateLanguageServer(); - - await initLanguageServer(context); -} - async function promptToReloadForDebuggerSettingChange(context: vscode.ExtensionContext) { if (vscode.env.uiKind !== vscode.UIKind.Desktop) { return; diff --git a/src/extension.ts b/src/extension.ts index be9a157f..36d5f595 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1,7 +1,6 @@ import * as vscode from "vscode"; -import {canReachGitHubAPI} from "./api/canReachGitHubAPI"; -import {getSession} from "./auth/auth"; +import {initAuthStateTracking, updateContextKeys} from "./auth/authState"; import {registerCancelWorkflowRun} from "./commands/cancelWorkflowRun"; import {registerOpenWorkflowFile} from "./commands/openWorkflowFile"; import {registerOpenWorkflowJobLogs} from "./commands/openWorkflowJobLogs"; @@ -20,7 +19,6 @@ import {registerCopyVariable} from "./commands/variables/copyVariable"; import {registerDeleteVariable} from "./commands/variables/deleteVariable"; import {registerUpdateVariable} from "./commands/variables/updateVariable"; import {initConfiguration, isDebuggerEnabled} from "./configuration/configuration"; -import {getGitHubContext} from "./git/repository"; import {init as initLogger, log, revealLog} from "./log"; import {LogScheme} from "./logs/constants"; import {WorkflowStepLogProvider} from "./logs/fileProvider"; @@ -41,20 +39,13 @@ export async function activate(context: vscode.ExtensionContext) { log("Activating GitHub Actions extension..."); - const hasSession = !!(await getSession()); - const canReachAPI = hasSession && (await canReachGitHubAPI()); + // Register before the first API request, so a rejected token is reported to the user + initAuthStateTracking(context); - // Prefetch git repository origin url - const ghContext = hasSession && (await getGitHubContext()); - const hasGitHubRepos = ghContext && ghContext.repos.length > 0; + // Also prefetches the git repository origin url + await updateContextKeys(); const debuggerEnabled = vscode.env.uiKind === vscode.UIKind.Desktop && isDebuggerEnabled(); - await Promise.all([ - vscode.commands.executeCommand("setContext", "github-actions.signed-in", hasSession), - vscode.commands.executeCommand("setContext", "github-actions.internet-access", canReachAPI), - vscode.commands.executeCommand("setContext", "github-actions.has-repos", hasGitHubRepos) - ]); - initResources(context); initConfiguration(context); diff --git a/src/test/vscode.ts b/src/test/vscode.ts new file mode 100644 index 00000000..ea69327d --- /dev/null +++ b/src/test/vscode.ts @@ -0,0 +1,3 @@ +// Stand-in for the `vscode` module, which only exists inside the extension host. Tests replace it with +// `jest.unstable_mockModule("vscode", ...)`. +export {}; diff --git a/src/treeViews/treeViews.ts b/src/treeViews/treeViews.ts index 6261b1b2..debc2792 100644 --- a/src/treeViews/treeViews.ts +++ b/src/treeViews/treeViews.ts @@ -1,6 +1,6 @@ import * as vscode from "vscode"; -import {canReachGitHubAPI} from "../api/canReachGitHubAPI"; +import {updateContextKeys} from "../auth/authState"; import {executeCacheClearCommand} from "../workflow/languageServer"; import {getGitHubContext} from "../git/repository"; import {logDebug} from "../log"; @@ -23,14 +23,9 @@ export async function initTreeViews(context: vscode.ExtensionContext, store: Run context.subscriptions.push( vscode.commands.registerCommand("github-actions.explorer.refresh", async () => { - const canReachAPI = await canReachGitHubAPI(); - await vscode.commands.executeCommand("setContext", "github-actions.internet-access", canReachAPI); + const {status, hasGitHubRepos} = await updateContextKeys(); - const ghContext = await getGitHubContext(); - const hasGitHubRepos = ghContext && ghContext.repos.length > 0; - await vscode.commands.executeCommand("setContext", "github-actions.has-repos", hasGitHubRepos); - - if (canReachAPI && hasGitHubRepos) { + if (status === "ok" && hasGitHubRepos) { await workflowTreeProvider.refresh(); await settingsTreeProvider.refresh(); } diff --git a/src/workflow/languageServer.ts b/src/workflow/languageServer.ts index 1e5e98b3..6c2f9094 100644 --- a/src/workflow/languageServer.ts +++ b/src/workflow/languageServer.ts @@ -90,6 +90,12 @@ export function deactivateLanguageServer(): Promise { return client.stop(); } +export async function restartLanguageServer(context: vscode.ExtensionContext): Promise { + await deactivateLanguageServer(); + + await initLanguageServer(context); +} + export function executeCacheClearCommand(): Promise { if (!client) { return Promise.resolve();