diff --git a/src/api/authInterceptor.ts b/src/api/authInterceptor.ts index 68e4658c..f1b0d386 100644 --- a/src/api/authInterceptor.ts +++ b/src/api/authInterceptor.ts @@ -1,5 +1,6 @@ import { type AxiosError, isAxiosError } from "axios"; +import { OAuthError } from "../oauth/errors"; import { toSafeHost } from "../util"; import type * as vscode from "vscode"; @@ -27,6 +28,7 @@ export type AuthRequiredHandler = (hostname: string) => Promise; */ export class AuthInterceptor implements vscode.Disposable { private readonly interceptorId: number; + private authRequiredPromise: Promise | null = null; constructor( private readonly client: CoderApi, @@ -82,13 +84,21 @@ export class AuthInterceptor implements vscode.Disposable { this.logger.debug("Token refresh successful, retrying request"); return this.retryRequest(error, newTokens.access_token); } catch (refreshError) { - this.logger.error("OAuth refresh failed:", refreshError); + if (refreshError instanceof OAuthError) { + const msg = `Token refresh failed: ${refreshError.message}`; + if (refreshError.requiresReAuth) { + this.logger.warn(msg); + } else { + this.logger.error(msg); + } + } else { + this.logger.error("Token refresh failed:", refreshError); + } } } if (this.onAuthRequired) { - this.logger.debug("Triggering interactive re-authentication"); - const success = await this.onAuthRequired(hostname); + const success = await this.executeAuthRequired(hostname); if (success) { const auth = await this.secretsManager.getSessionAuth(hostname); if (auth) { @@ -101,6 +111,28 @@ export class AuthInterceptor implements vscode.Disposable { throw error; } + /** + * Execute auth required callback with deduplication. + * Multiple concurrent 401s will share the same promise. + */ + private async executeAuthRequired(hostname: string): Promise { + if (this.authRequiredPromise) { + this.logger.debug( + "Auth callback already in progress, waiting for result", + ); + return this.authRequiredPromise; + } + + this.logger.debug("Triggering re-authentication"); + this.authRequiredPromise = this.onAuthRequired!(hostname); + + try { + return await this.authRequiredPromise; + } finally { + this.authRequiredPromise = null; + } + } + private retryRequest(error: AxiosError, token: string): Promise { if (!error.config) { throw error; diff --git a/src/commands.ts b/src/commands.ts index ac4f0fdf..19daaa57 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -209,12 +209,12 @@ export class Commands { await this.deploymentManager.clearDeployment(); - void vscode.window + vscode.window .showInformationMessage("You've been logged out of Coder!", "Login") .then((action) => { if (action === "Login") { this.login().catch((error) => { - this.logger.error("Failed to login", error); + this.logger.error("Login failed", error); }); } }); diff --git a/src/core/secretsManager.ts b/src/core/secretsManager.ts index 7ad37bb0..be033689 100644 --- a/src/core/secretsManager.ts +++ b/src/core/secretsManager.ts @@ -38,7 +38,7 @@ export type CurrentDeploymentState = z.infer< */ const OAuthTokenDataSchema = z.object({ refresh_token: z.string().optional(), - scope: z.string().optional(), + scope: z.string(), expiry_timestamp: z.number(), }); diff --git a/src/deployment/deploymentManager.ts b/src/deployment/deploymentManager.ts index 1e087459..9b53751b 100644 --- a/src/deployment/deploymentManager.ts +++ b/src/deployment/deploymentManager.ts @@ -12,11 +12,6 @@ import { type Deployment, type DeploymentWithAuth } from "./types"; import type { User } from "coder/site/src/api/typesGenerated"; import type * as vscode from "vscode"; -/** - * Internal state type that allows mutation of user property. - */ -type DeploymentWithUser = Deployment & { user: User }; - /** * Manages deployment state for the extension. * @@ -35,7 +30,7 @@ export class DeploymentManager implements vscode.Disposable { private readonly contextManager: ContextManager; private readonly logger: Logger; - #deployment: DeploymentWithUser | null = null; + #deployment: Deployment | null = null; #authListenerDisposable: vscode.Disposable | undefined; #crossWindowSyncDisposable: vscode.Disposable | undefined; @@ -78,7 +73,7 @@ export class DeploymentManager implements vscode.Disposable { * Check if we have an authenticated deployment (does not guarantee that the current auth data is valid). */ public isAuthenticated(): boolean { - return this.#deployment !== null; + return this.contextManager.get("coder.authenticated"); } /** @@ -89,10 +84,10 @@ export class DeploymentManager implements vscode.Disposable { public async setDeploymentIfValid( deployment: Deployment & { token?: string }, ): Promise { - const auth = await this.secretsManager.getSessionAuth( - deployment.safeHostname, - ); - const token = deployment.token ?? auth?.token; + const token = + deployment.token ?? + (await this.secretsManager.getSessionAuth(deployment.safeHostname)) + ?.token; const tempClient = CoderApi.create(deployment.url, token, this.logger); try { @@ -132,7 +127,7 @@ export class DeploymentManager implements vscode.Disposable { // Register auth listener before setDeployment so background token refresh // can update client credentials via the listener this.registerAuthListener(); - this.updateAuthContexts(); + this.updateAuthContexts(deployment.user); this.refreshWorkspaces(); await this.oauthSessionManager.setDeployment(deployment); @@ -143,16 +138,32 @@ export class DeploymentManager implements vscode.Disposable { * Clears the current deployment. */ public async clearDeployment(): Promise { + this.suspendSession(); this.#authListenerDisposable?.dispose(); this.#authListenerDisposable = undefined; this.#deployment = null; - this.client.setCredentials(undefined, undefined); + await this.secretsManager.setCurrentDeployment(undefined); + } + + /** + * Suspend session: shows logged-out state but keeps deployment for easy re-login. + * Auth listener remains active so recovery can happen automatically if tokens update. + */ + public suspendSession(): void { this.oauthSessionManager.clearDeployment(); - this.updateAuthContexts(); - this.refreshWorkspaces(); + this.client.setCredentials(undefined, undefined); + this.updateAuthContexts(undefined); + this.clearWorkspaces(); + } - await this.secretsManager.setCurrentDeployment(undefined); + /** + * Clear all workspace providers without fetching. + */ + private clearWorkspaces(): void { + for (const provider of this.workspaceProviders) { + provider.clear(); + } } public dispose(): void { @@ -163,6 +174,7 @@ export class DeploymentManager implements vscode.Disposable { /** * Register auth listener for the current deployment. * Updates credentials when they change (token refresh, cross-window sync). + * Also handles recovery from suspended session state. */ private registerAuthListener(): void { if (!this.#deployment) { @@ -182,7 +194,18 @@ export class DeploymentManager implements vscode.Disposable { } if (auth) { - this.client.setCredentials(auth.url, auth.token); + if (this.isAuthenticated()) { + this.client.setCredentials(auth.url, auth.token); + } else { + this.logger.debug( + "Token updated after session suspended, recovering", + ); + await this.setDeploymentIfValid({ + url: auth.url, + safeHostname, + token: auth.token, + }); + } } else { await this.clearDeployment(); } @@ -210,8 +233,7 @@ export class DeploymentManager implements vscode.Disposable { /** * Update authentication-related contexts. */ - private updateAuthContexts(): void { - const user = this.#deployment?.user; + private updateAuthContexts(user: User | undefined): void { this.contextManager.set("coder.authenticated", Boolean(user)); const isOwner = user?.roles.some((r) => r.name === "owner") ?? false; this.contextManager.set("coder.isOwner", isOwner); diff --git a/src/extension.ts b/src/extension.ts index 753b4500..3544be7e 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -70,10 +70,33 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { const deployment = await secretsManager.getCurrentDeployment(); - // Create OAuth session manager with login coordinator + // Shared handler for auth failures (used by interceptor + session manager) + const handleAuthFailure = (): Promise => { + deploymentManager.suspendSession(); + vscode.window + .showWarningMessage( + "Session expired. You have been signed out.", + "Log In", + ) + .then(async (action) => { + if (action === "Log In") { + try { + await commands.login({ + url: deploymentManager.getCurrentDeployment()?.url, + }); + } catch (err) { + output.error("Login failed", err); + } + } + }); + return Promise.resolve(); + }; + + // Create OAuth session manager - callback handles background refresh failures const oauthSessionManager = OAuthSessionManager.create( deployment, serviceContainer, + handleAuthFailure, ); ctx.subscriptions.push(oauthSessionManager); @@ -94,11 +117,9 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { output, oauthSessionManager, secretsManager, - () => { - void vscode.window.showWarningMessage( - "Session expired. Please log in again using the Coder sidebar.", - ); - return Promise.resolve(false); + async () => { + await handleAuthFailure(); + return false; }, ); ctx.subscriptions.push(authInterceptor); @@ -324,7 +345,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { process.env.CODER_URL?.trim(); if (defaultUrl) { commands.login({ url: defaultUrl, autoLogin: true }).catch((error) => { - output.error("Failed to auto-login", error); + output.error("Auto-login failed", error); }); } } diff --git a/src/oauth/authorizer.ts b/src/oauth/authorizer.ts index c3ca8435..001c06c7 100644 --- a/src/oauth/authorizer.ts +++ b/src/oauth/authorizer.ts @@ -8,6 +8,7 @@ import { type Logger } from "../logging/logger"; import { AUTH_GRANT_TYPE, + DEFAULT_OAUTH_SCOPES, PKCE_CHALLENGE_METHOD, RESPONSE_TYPE, TOKEN_ENDPOINT_AUTH_METHOD, @@ -29,19 +30,6 @@ import type { User, } from "coder/site/src/api/typesGenerated"; -/** - * Minimal scopes required by the VS Code extension. - */ -const DEFAULT_OAUTH_SCOPES = [ - "workspace:read", - "workspace:update", - "workspace:start", - "workspace:ssh", - "workspace:application_connect", - "template:read", - "user:read_personal", -].join(" "); - /** * Handles the OAuth authorization code flow for authenticating with Coder deployments. * Encapsulates client registration, PKCE challenge, and token exchange. diff --git a/src/oauth/constants.ts b/src/oauth/constants.ts index f73d1536..542fa517 100644 --- a/src/oauth/constants.ts +++ b/src/oauth/constants.ts @@ -2,6 +2,17 @@ export const AUTH_GRANT_TYPE = "authorization_code"; export const REFRESH_GRANT_TYPE = "refresh_token"; +// Minimal scopes required by the VS Code extension +export const DEFAULT_OAUTH_SCOPES = [ + "workspace:read", + "workspace:update", + "workspace:start", + "workspace:ssh", + "workspace:application_connect", + "template:read", + "user:read_personal", +].join(" "); + // OAuth 2.1 Response Types export const RESPONSE_TYPE = "code"; diff --git a/src/oauth/errors.ts b/src/oauth/errors.ts index e10565b2..0c4b3d1f 100644 --- a/src/oauth/errors.ts +++ b/src/oauth/errors.ts @@ -34,6 +34,15 @@ export class OAuthError extends Error { ); this.name = "OAuthError"; } + + /** + * Returns true if this error indicates the user needs to re-authenticate. + */ + get requiresReAuth(): boolean { + return ( + this.errorCode === "invalid_grant" || this.errorCode === "invalid_client" + ); + } } export function parseOAuthError(error: unknown): OAuthError | null { @@ -57,9 +66,3 @@ function isOAuth2Error(data: unknown): data is OAuth2Error { typeof data.error === "string" ); } - -export function requiresReAuthentication(error: OAuthError): boolean { - return ( - error.errorCode === "invalid_grant" || error.errorCode === "invalid_client" - ); -} diff --git a/src/oauth/metadataClient.ts b/src/oauth/metadataClient.ts index 1eaea2ea..add545fb 100644 --- a/src/oauth/metadataClient.ts +++ b/src/oauth/metadataClient.ts @@ -18,7 +18,7 @@ import type { Logger } from "../logging/logger"; const OAUTH_DISCOVERY_ENDPOINT = "/.well-known/oauth-authorization-server"; -const REQUIRED_GRANT_TYPES: readonly string[] = [ +const REQUIRED_GRANT_TYPES: readonly OAuth2ProviderGrantType[] = [ AUTH_GRANT_TYPE, REFRESH_GRANT_TYPE, ]; diff --git a/src/oauth/sessionManager.ts b/src/oauth/sessionManager.ts index 50257ad9..a073173f 100644 --- a/src/oauth/sessionManager.ts +++ b/src/oauth/sessionManager.ts @@ -1,11 +1,7 @@ import { CoderApi } from "../api/coderApi"; -import { REFRESH_GRANT_TYPE } from "./constants"; -import { - type OAuthError, - parseOAuthError, - requiresReAuthentication, -} from "./errors"; +import { DEFAULT_OAUTH_SCOPES, REFRESH_GRANT_TYPE } from "./constants"; +import { OAuthError, parseOAuthError } from "./errors"; import { OAuthMetadataClient } from "./metadataClient"; import { buildOAuthTokenData, toUrlSearchParams } from "./utils"; @@ -23,36 +19,17 @@ import type { ServiceContainer } from "../core/container"; import type { OAuthTokenData, SecretsManager } from "../core/secretsManager"; import type { Deployment } from "../deployment/types"; import type { Logger } from "../logging/logger"; -import type { LoginCoordinator } from "../login/loginCoordinator"; /** * Token refresh threshold: refresh when token expires in less than this time. */ const TOKEN_REFRESH_THRESHOLD_MS = 10 * 60 * 1000; -/** - * Minimum time between refresh attempts to prevent thrashing. - */ -const REFRESH_THROTTLE_MS = 30 * 1000; - /** * Background token refresh check interval. */ const BACKGROUND_REFRESH_INTERVAL_MS = 5 * 60 * 1000; -/** - * Minimal scopes required by the VS Code extension. - */ -const DEFAULT_OAUTH_SCOPES = [ - "workspace:read", - "workspace:update", - "workspace:start", - "workspace:ssh", - "workspace:application_connect", - "template:read", - "user:read_personal", -].join(" "); - /** * Internal type combining access token with OAuth-specific data. * Used by getStoredTokens() for token refresh and validation. @@ -68,23 +45,25 @@ type StoredTokens = OAuthTokenData & { export class OAuthSessionManager implements vscode.Disposable { private refreshPromise: Promise | null = null; private refreshAbortController: AbortController | null = null; - private lastRefreshAttempt = 0; private refreshTimer: NodeJS.Timeout | undefined; private tokenChangeListener: vscode.Disposable | undefined; private disposed = false; /** * Create and initialize a new OAuth session manager. + * + * @param onAuthRequired Called when background refresh fails with a re-auth error. */ public static create( deployment: Deployment | null, container: ServiceContainer, + onAuthRequired: () => Promise = () => Promise.resolve(), ): OAuthSessionManager { const manager = new OAuthSessionManager( deployment, container.getSecretsManager(), container.getLogger(), - container.getLoginCoordinator(), + onAuthRequired, ); manager.setupTokenListener(); manager.scheduleNextRefresh(); @@ -95,7 +74,7 @@ export class OAuthSessionManager implements vscode.Disposable { private deployment: Deployment | null, private readonly secretsManager: SecretsManager, private readonly logger: Logger, - private readonly loginCoordinator: LoginCoordinator, + private readonly onAuthRequired: () => Promise, ) {} /** @@ -150,14 +129,13 @@ export class OAuthSessionManager implements vscode.Disposable { } /** - * Clear all refresh-related state: in-flight promise, throttle, timer, and listener. + * Clear all refresh-related state: in-flight promise, timer, and listener. * Aborts any in-flight refresh request to prevent stale token updates. */ private clearRefreshState(): void { this.refreshAbortController?.abort(); this.refreshAbortController = null; this.refreshPromise = null; - this.lastRefreshAttempt = 0; if (this.refreshTimer) { clearTimeout(this.refreshTimer); this.refreshTimer = undefined; @@ -244,10 +222,25 @@ export class OAuthSessionManager implements vscode.Disposable { .then(() => { this.logger.debug("Background token refresh succeeded"); }) - .catch((error) => { + .catch(async (error) => { if (this.disposed) { return; } + + // Re-auth errors: notify callback and stop retrying + if (error instanceof OAuthError && error.requiresReAuth) { + this.logger.warn( + "Background refresh requires re-auth:", + error.message, + ); + try { + await this.onAuthRequired(); + } catch (callbackError) { + this.logger.error("onAuthRequired callback failed:", callbackError); + } + return; + } + this.logger.warn("Background token refresh failed, will retry:", error); this.refreshTimer = setTimeout( () => this.attemptRefreshWithRetry(), @@ -260,12 +253,7 @@ export class OAuthSessionManager implements vscode.Disposable { * Check if granted scopes cover all required scopes. * Supports wildcard scopes like "workspace:*". */ - private hasRequiredScopes(grantedScope: string | undefined): boolean { - if (!grantedScope) { - // TODO server always returns empty scopes - return true; - } - + private hasRequiredScopes(grantedScope: string): boolean { const grantedScopes = new Set(grantedScope.split(" ")); const requiredScopes = DEFAULT_OAUTH_SCOPES.split(" "); @@ -381,8 +369,6 @@ export class OAuthSessionManager implements vscode.Disposable { const refreshToken = storedTokens.refresh_token; const accessToken = storedTokens.access_token; - this.lastRefreshAttempt = Date.now(); - const { axiosInstance, metadata, registration } = await this.prepareOAuthOperation(accessToken); @@ -424,8 +410,7 @@ export class OAuthSessionManager implements vscode.Disposable { return response.data; } catch (error) { - this.handleOAuthError(error); - throw error; + throw parseOAuthError(error) ?? error; } finally { if (this.refreshAbortController === abortController) { this.refreshAbortController = null; @@ -434,39 +419,6 @@ export class OAuthSessionManager implements vscode.Disposable { } } - /** - * Refreshes the token if it is approaching expiry. - */ - public async refreshIfAlmostExpired(): Promise { - if (await this.shouldRefreshToken()) { - this.logger.debug("Token approaching expiry, triggering refresh"); - await this.refreshToken(); - } - } - - /** - * Check if token should be refreshed. - * Returns true if: - * 1. Stored tokens exist with a refresh token - * 2. Token expires in less than TOKEN_REFRESH_THRESHOLD_MS - * 3. Last refresh attempt was more than REFRESH_THROTTLE_MS ago - * 4. No refresh is currently in progress - */ - private async shouldRefreshToken(): Promise { - const storedTokens = await this.getStoredTokens(); - if (!storedTokens?.refresh_token || this.refreshPromise !== null) { - return false; - } - - const now = Date.now(); - if (now - this.lastRefreshAttempt < REFRESH_THROTTLE_MS) { - return false; - } - - const timeUntilExpiry = storedTokens.expiry_timestamp - now; - return timeUntilExpiry < TOKEN_REFRESH_THRESHOLD_MS; - } - public async revokeRefreshToken(): Promise { const storedTokens = await this.getStoredTokens(); if (!storedTokens?.refresh_token) { @@ -545,51 +497,6 @@ export class OAuthSessionManager implements vscode.Disposable { return storedTokens !== undefined; } - /** - * Handle OAuth errors that may require re-authentication. - * Parses the error and triggers re-authentication modal if needed. - */ - private handleOAuthError(error: unknown): void { - const oauthError = parseOAuthError(error); - if (oauthError && requiresReAuthentication(oauthError)) { - this.logger.error( - `OAuth operation failed with error: ${oauthError.errorCode}`, - ); - // Fire and forget - don't block on showing the modal - this.showReAuthenticationModal(oauthError).catch((err) => { - this.logger.error("Failed to show re-auth modal:", err); - }); - } - } - - /** - * Show a modal dialog to the user when OAuth re-authentication is required. - * This is called when the refresh token is invalid or the client credentials are invalid. - * Clears tokens directly and lets listeners handle updates. - */ - public async showReAuthenticationModal(error: OAuthError): Promise { - const deployment = this.requireDeployment(); - const errorMessage = - error.message || - "Your session is no longer valid. This could be due to token expiration or revocation."; - - this.clearRefreshState(); - // Clear client registration and tokens to force full re-authentication - await this.secretsManager.clearOAuthClientRegistration( - deployment.safeHostname, - ); - await this.secretsManager.setSessionAuth(deployment.safeHostname, { - url: deployment.url, - token: "", - }); - - await this.loginCoordinator.ensureLoggedInWithDialog({ - safeHostname: deployment.safeHostname, - url: deployment.url, - detailPrefix: errorMessage, - }); - } - /** * Clears all in-memory state. */ diff --git a/src/oauth/utils.ts b/src/oauth/utils.ts index 9a990758..a61226ed 100644 --- a/src/oauth/utils.ts +++ b/src/oauth/utils.ts @@ -1,5 +1,7 @@ import { createHash, randomBytes } from "node:crypto"; +import { DEFAULT_OAUTH_SCOPES } from "./constants"; + import type { OAuth2TokenResponse } from "coder/site/src/api/typesGenerated"; import type { OAuthTokenData } from "../core/secretsManager"; @@ -66,7 +68,8 @@ export function buildOAuthTokenData( return { refresh_token: tokenResponse.refresh_token, - scope: tokenResponse.scope, + // Use default scopes when server returns empty, so scope changes can invalidate tokens + scope: tokenResponse.scope || DEFAULT_OAUTH_SCOPES, expiry_timestamp: getExpiryTimestamp(tokenResponse), }; } diff --git a/src/remote/remote.ts b/src/remote/remote.ts index cc553d6e..8dab3c3c 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -118,10 +118,22 @@ export class Remote { const disposables: vscode.Disposable[] = []; try { + // Shared dialog for session expiry (used by interceptor + session manager) + const showSessionExpiredDialog = () => + this.loginCoordinator.ensureLoggedInWithDialog({ + safeHostname: parts.safeHostname, + url: baseUrlRaw, + message: "Your session expired...", + detailPrefix: `You must log in to access ${workspaceName}.`, + }); + // Create OAuth session manager for this remote deployment const remoteOAuthManager = OAuthSessionManager.create( { url: baseUrlRaw, safeHostname: parts.safeHostname }, this.serviceContainer, + async () => { + await showSessionExpiredDialog(); + }, ); disposables.push(remoteOAuthManager); @@ -179,13 +191,8 @@ export class Remote { this.logger, remoteOAuthManager, this.secretsManager, - async (hostname) => { - const result = await this.loginCoordinator.ensureLoggedInWithDialog({ - safeHostname: hostname, - url: baseUrlRaw, - message: "Your session expired...", - detailPrefix: `You must log in to access ${workspaceName}.`, - }); + async () => { + const result = await showSessionExpiredDialog(); return result.success; }, ); diff --git a/src/workspace/workspacesProvider.ts b/src/workspace/workspacesProvider.ts index aa2992ab..b6e0d993 100644 --- a/src/workspace/workspacesProvider.ts +++ b/src/workspace/workspacesProvider.ts @@ -44,6 +44,7 @@ export class WorkspaceProvider private timeout: NodeJS.Timeout | undefined; private fetching = false; private visible = false; + private disposed = false; constructor( private readonly getWorkspacesQuery: WorkspaceQuery, @@ -59,8 +60,8 @@ export class WorkspaceProvider // still logged in and no errors were encountered fetching workspaces. // Calling this while already refreshing or not visible is a no-op and will // return immediately. - async fetchAndRefresh() { - if (this.fetching || !this.visible) { + public async fetchAndRefresh() { + if (this.disposed || this.fetching || !this.visible) { return; } this.fetching = true; @@ -74,7 +75,7 @@ export class WorkspaceProvider try { this.workspaces = await this.fetch(); } catch (error) { - this.logger.error("Failed to fetch workspaces", error); + this.logger.warn("Failed to fetch workspaces:", error); hadError = true; this.workspaces = []; } @@ -176,7 +177,10 @@ export class WorkspaceProvider * * If we have never fetched workspaces and are visible, fetch immediately. */ - setVisibility(visible: boolean) { + public setVisibility(visible: boolean) { + if (this.disposed) { + return; + } this.visible = visible; if (!visible) { this.cancelPendingRefresh(); @@ -214,15 +218,18 @@ export class WorkspaceProvider > = this._onDidChangeTreeData.event; // refresh causes the tree to re-render. It does not fetch fresh workspaces. - refresh(item?: vscode.TreeItem): void { + public refresh(item?: vscode.TreeItem): void { + if (this.disposed) { + return; + } this._onDidChangeTreeData.fire(item); } - getTreeItem(element: vscode.TreeItem): vscode.TreeItem { + public getTreeItem(element: vscode.TreeItem): vscode.TreeItem { return element; } - getChildren(element?: vscode.TreeItem): Thenable { + public getChildren(element?: vscode.TreeItem): Thenable { if (element) { if (element instanceof WorkspaceTreeItem) { const agents = extractAgents(element.workspace.latest_build.resources); @@ -296,12 +303,22 @@ export class WorkspaceProvider return Promise.resolve(this.workspaces ?? []); } - dispose() { + /** + * Clear all workspaces from the tree without fetching. + */ + public clear(): void { this.cancelPendingRefresh(); for (const watcher of this.agentWatchers.values()) { watcher.dispose(); } this.agentWatchers.clear(); + this.workspaces = undefined; + this.refresh(); + } + + public dispose() { + this.disposed = true; + this.clear(); } } diff --git a/test/mocks/testHelpers.ts b/test/mocks/testHelpers.ts index a6086054..6d1af77a 100644 --- a/test/mocks/testHelpers.ts +++ b/test/mocks/testHelpers.ts @@ -491,6 +491,7 @@ export class MockCoderApi implements Pick< | "setHost" | "setSessionToken" | "setCredentials" + | "getHost" | "getAuthenticatedUser" | "dispose" > { @@ -514,6 +515,8 @@ export class MockCoderApi implements Pick< }, ); + readonly getHost = vi.fn(() => this._host); + readonly getAuthenticatedUser = vi.fn((): Promise => { if (this.authenticatedUser instanceof Error) { return Promise.reject(this.authenticatedUser); @@ -570,11 +573,9 @@ export class MockOAuthSessionManager { readonly refreshToken = vi .fn() .mockResolvedValue({ access_token: "test-token" }); - readonly refreshIfAlmostExpired = vi.fn().mockResolvedValue(undefined); readonly revokeRefreshToken = vi.fn().mockResolvedValue(undefined); readonly isLoggedInWithOAuth = vi.fn().mockResolvedValue(false); readonly clearOAuthState = vi.fn().mockResolvedValue(undefined); - readonly showReAuthenticationModal = vi.fn().mockResolvedValue(undefined); readonly dispose = vi.fn(); } diff --git a/test/unit/api/authInterceptor.test.ts b/test/unit/api/authInterceptor.test.ts index b93012a7..e2b8ed3c 100644 --- a/test/unit/api/authInterceptor.test.ts +++ b/test/unit/api/authInterceptor.test.ts @@ -105,6 +105,7 @@ function createTestContext() { oauth: { refresh_token: "refresh-token", expiry_timestamp: Date.now() + ONE_HOUR_MS, + scope: "workspace:read", }, }); mockOAuthManager.isLoggedInWithOAuth.mockImplementation( diff --git a/test/unit/core/secretsManager.test.ts b/test/unit/core/secretsManager.test.ts index 308502d3..3d1f1e77 100644 --- a/test/unit/core/secretsManager.test.ts +++ b/test/unit/core/secretsManager.test.ts @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { type CurrentDeploymentState, SecretsManager, + type SessionAuth, } from "@/core/secretsManager"; import { @@ -11,6 +12,8 @@ import { createMockLogger, } from "../../mocks/testHelpers"; +import type { Deployment } from "@/deployment/types"; + describe("SecretsManager", () => { let secretStorage: InMemorySecretStorage; let memento: InMemoryMemento; @@ -286,7 +289,7 @@ describe("SecretsManager", () => { await secretsManager.setSessionAuth( "example.com", - authWithExtra as Parameters[1], + authWithExtra as SessionAuth, ); const raw = await secretStorage.get("coder.session.example.com"); @@ -300,19 +303,23 @@ describe("SecretsManager", () => { const authWithExtra = { url: "https://coder.example.com", token: "test-token", - oauth: { expiry_timestamp: 12345, extraOAuthField: "stripped" }, + oauth: { + scope: "workspace:read", + expiry_timestamp: 12345, + extraOAuthField: "stripped", + }, }; await secretsManager.setSessionAuth( "example.com", - authWithExtra as Parameters[1], + authWithExtra as SessionAuth, ); const raw = await secretStorage.get("coder.session.example.com"); expect(JSON.parse(raw!)).toEqual({ url: "https://coder.example.com", token: "test-token", - oauth: { expiry_timestamp: 12345 }, + oauth: { scope: "workspace:read", expiry_timestamp: 12345 }, }); }); @@ -324,9 +331,7 @@ describe("SecretsManager", () => { }; await secretsManager.setCurrentDeployment( - deploymentWithExtra as Parameters< - typeof secretsManager.setCurrentDeployment - >[0], + deploymentWithExtra as Deployment, ); const raw = await secretStorage.get("coder.currentDeployment"); @@ -405,12 +410,12 @@ describe("SecretsManager", () => { data: { url: "https://coder.example.com", token: "test-token", - oauth: { expiry_timestamp: 12345 }, + oauth: { scope: "workspace:read", expiry_timestamp: 12345 }, }, expected: { url: "https://coder.example.com", token: "test-token", - oauth: { expiry_timestamp: 12345 }, + oauth: { scope: "workspace:read", expiry_timestamp: 12345 }, }, }, ]; diff --git a/test/unit/deployment/deploymentManager.test.ts b/test/unit/deployment/deploymentManager.test.ts index e5fac904..fce4652f 100644 --- a/test/unit/deployment/deploymentManager.test.ts +++ b/test/unit/deployment/deploymentManager.test.ts @@ -33,6 +33,7 @@ vi.mock("@/api/coderApi", async (importOriginal) => { /** * Mock ContextManager for deployment tests. + * Mimics real ContextManager which defaults to false for boolean contexts. */ class MockContextManager { private readonly contexts = new Map(); @@ -41,8 +42,8 @@ class MockContextManager { this.contexts.set(key, value); }); - get(key: string): boolean | undefined { - return this.contexts.get(key); + get(key: string): boolean { + return this.contexts.get(key) ?? false; } } @@ -51,6 +52,7 @@ class MockContextManager { */ class MockWorkspaceProvider { readonly fetchAndRefresh = vi.fn(); + readonly clear = vi.fn(); } const TEST_URL = "https://coder.example.com"; @@ -98,6 +100,8 @@ function createTestContext() { validationMockClient, secretsManager, contextManager, + mockOAuthSessionManager, + mockWorkspaceProvider, manager, }; } @@ -396,4 +400,73 @@ describe("DeploymentManager", () => { expect(contextManager.get("coder.isOwner")).toBe(false); }); }); + + describe("suspendSession", () => { + it("clears auth state but keeps deployment for re-login", async () => { + const { + mockClient, + contextManager, + mockOAuthSessionManager, + mockWorkspaceProvider, + manager, + } = createTestContext(); + + await manager.setDeployment({ + url: TEST_URL, + safeHostname: TEST_HOSTNAME, + token: "test-token", + user: createMockUser(), + }); + expect(manager.isAuthenticated()).toBe(true); + + manager.suspendSession(); + + // Auth state is cleared + expect(mockOAuthSessionManager.clearDeployment).toHaveBeenCalled(); + expect(mockClient.host).toBeUndefined(); + expect(mockClient.token).toBeUndefined(); + expect(contextManager.get("coder.authenticated")).toBe(false); + expect(manager.isAuthenticated()).toBe(false); + expect(mockWorkspaceProvider.clear).toHaveBeenCalled(); + + // Deployment is retained for easy re-login + expect(manager.getCurrentDeployment()).toMatchObject({ + url: TEST_URL, + safeHostname: TEST_HOSTNAME, + }); + }); + }); + + describe("auth listener recovery", () => { + it("recovers from suspended state when tokens update", async () => { + const { mockClient, validationMockClient, secretsManager, manager } = + createTestContext(); + const user = createMockUser(); + validationMockClient.setAuthenticatedUserResponse(user); + + // Set up authenticated deployment + await manager.setDeployment({ + url: TEST_URL, + safeHostname: TEST_HOSTNAME, + token: "initial-token", + user, + }); + + // Suspend session (simulates session expiry) + manager.suspendSession(); + expect(manager.isAuthenticated()).toBe(false); + + // Simulate token update (e.g., from another window or re-login) + await secretsManager.setSessionAuth(TEST_HOSTNAME, { + url: TEST_URL, + token: "recovered-token", + }); + + await new Promise((resolve) => setImmediate(resolve)); + + // Should recover and be authenticated again + expect(mockClient.token).toBe("recovered-token"); + expect(manager.isAuthenticated()).toBe(true); + }); + }); }); diff --git a/test/unit/oauth/errors.test.ts b/test/unit/oauth/errors.test.ts index 3e59e6fc..ed71906f 100644 --- a/test/unit/oauth/errors.test.ts +++ b/test/unit/oauth/errors.test.ts @@ -1,37 +1,11 @@ import { AxiosError, AxiosHeaders } from "axios"; import { describe, expect, it } from "vitest"; -import { - OAuthError, - parseOAuthError, - requiresReAuthentication, -} from "@/oauth/errors"; +import { OAuthError, parseOAuthError } from "@/oauth/errors"; -import type { OAuth2ErrorCode } from "coder/site/src/api/typesGenerated"; +import { createOAuthAxiosError } from "./testUtils"; -function createOAuthAxiosError( - errorCode: string, - errorDescription?: string, -): AxiosError { - const data: Record = { error: errorCode }; - if (errorDescription) { - data.error_description = errorDescription; - } - - return new AxiosError( - "OAuth Error", - "ERR_BAD_REQUEST", - undefined, - undefined, - { - status: 400, - statusText: "Bad Request", - headers: {}, - config: { headers: new AxiosHeaders() }, - data, - }, - ); -} +import type { OAuth2ErrorCode } from "coder/site/src/api/typesGenerated"; describe("parseOAuthError", () => { it.each([ @@ -109,11 +83,11 @@ describe("parseOAuthError", () => { }); }); -describe("requiresReAuthentication", () => { +describe("OAuthError.requiresReAuth", () => { it.each(["invalid_client", "invalid_grant"])( "returns true for %s", (code) => { - expect(requiresReAuthentication(new OAuthError(code))).toBe(true); + expect(new OAuthError(code).requiresReAuth).toBe(true); }, ); @@ -125,7 +99,7 @@ describe("requiresReAuthentication", () => { "access_denied", "server_error", ])("returns false for %s", (code) => { - expect(requiresReAuthentication(new OAuthError(code))).toBe(false); + expect(new OAuthError(code).requiresReAuth).toBe(false); }); }); diff --git a/test/unit/oauth/sessionManager.test.ts b/test/unit/oauth/sessionManager.test.ts index dd7af6b7..5cb1af2f 100644 --- a/test/unit/oauth/sessionManager.test.ts +++ b/test/unit/oauth/sessionManager.test.ts @@ -6,7 +6,7 @@ import { import { describe, expect, it, vi } from "vitest"; import { type SecretsManager, type SessionAuth } from "@/core/secretsManager"; -import { OAuthError } from "@/oauth/errors"; +import { DEFAULT_OAUTH_SCOPES } from "@/oauth/constants"; import { OAuthSessionManager } from "@/oauth/sessionManager"; import { @@ -19,6 +19,7 @@ import { createMockClientRegistration, createMockOAuthMetadata, createMockTokenResponse, + createOAuthAxiosError, createTestDeployment, TEST_HOSTNAME, TEST_URL, @@ -26,7 +27,6 @@ import { import type { ServiceContainer } from "@/core/container"; import type { Deployment } from "@/deployment/types"; -import type { LoginCoordinator } from "@/login/loginCoordinator"; vi.mock("axios", async () => { const actual = await vi.importActual("axios"); @@ -56,23 +56,15 @@ vi.mock("@/api/utils", async () => { const REFRESH_BUFFER_MS = 5 * 60 * 1000; // Tokens refresh 5 minutes before expiry const ONE_HOUR_MS = 60 * 60 * 1000; - -function createMockLoginCoordinator(): LoginCoordinator { - return { - ensureLoggedIn: vi.fn(), - ensureLoggedInWithDialog: vi.fn(), - } as unknown as LoginCoordinator; -} +const BACKGROUND_REFRESH_INTERVAL_MS = 5 * 60 * 1000; function createMockServiceContainer( secretsManager: SecretsManager, logger: ReturnType, - loginCoordinator: LoginCoordinator, ): ServiceContainer { return { getSecretsManager: () => secretsManager, getLogger: () => logger, - getLoginCoordinator: () => loginCoordinator, } as ServiceContainer; } @@ -80,11 +72,9 @@ function createTestContext(deployment: Deployment = createTestDeployment()) { vi.resetAllMocks(); const base = createBaseTestContext(); - const loginCoordinator = createMockLoginCoordinator(); const container = createMockServiceContainer( base.secretsManager, base.logger, - loginCoordinator, ); const manager = OAuthSessionManager.create(deployment, container); @@ -103,20 +93,47 @@ function createTestContext(deployment: Deployment = createTestDeployment()) { oauth: { refresh_token: overrides.refreshToken ?? "refresh-token", expiry_timestamp: Date.now() + (overrides.expiryMs ?? ONE_HOUR_MS), - scope: overrides.scope ?? "", + scope: overrides.scope ?? DEFAULT_OAUTH_SCOPES, }, }); }; /** Creates a new manager (for tests that need manager created after OAuth setup) */ - const createManager = (d: Deployment = deployment) => - OAuthSessionManager.create(d, container); + const createManager = ( + d: Deployment = deployment, + onAuthRequired: () => Promise = () => Promise.resolve(), + ) => OAuthSessionManager.create(d, container, onAuthRequired); + + /** + * Sets up a complete OAuth operation test context. + * Configures session, client registration, metadata, and custom routes. + */ + const setupForOAuthOperation = async ( + routes: Record, + sessionOverrides: { + token?: string; + refreshToken?: string; + expiryMs?: number; + scope?: string; + } = {}, + ) => { + await setupOAuthSession(sessionOverrides); + await base.secretsManager.setOAuthClientRegistration( + TEST_HOSTNAME, + createMockClientRegistration(), + ); + setupAxiosMockRoutes(base.mockAdapter, { + "/.well-known/oauth-authorization-server": + createMockOAuthMetadata(TEST_URL), + ...routes, + }); + }; return { ...base, - loginCoordinator, manager, setupOAuthSession, + setupForOAuthOperation, createManager, }; } @@ -138,6 +155,7 @@ describe("OAuthSessionManager", () => { oauth: { refresh_token: "refresh-token", expiry_timestamp: Date.now() + ONE_HOUR_MS, + scope: DEFAULT_OAUTH_SCOPES, }, }, expected: true, @@ -244,74 +262,86 @@ describe("OAuthSessionManager", () => { }); describe("background refresh", () => { - it("schedules refresh before token expiry", async () => { + /** Common setup for background refresh tests with fake timers */ + const setupBackgroundRefreshTest = async ( + tokenEndpointResponse: unknown, + ) => { vi.useFakeTimers(); - - const { secretsManager, mockAdapter, setupOAuthSession, createManager } = - createTestContext(); - - await setupOAuthSession({ token: "original-token" }); - await secretsManager.setOAuthClientRegistration( - TEST_HOSTNAME, - createMockClientRegistration(), + const ctx = createTestContext(); + await ctx.setupForOAuthOperation( + { "/oauth2/token": tokenEndpointResponse }, + { token: "original-token" }, ); + return ctx; + }; - setupAxiosMockRoutes(mockAdapter, { - "/.well-known/oauth-authorization-server": - createMockOAuthMetadata(TEST_URL), - "/oauth2/token": createMockTokenResponse({ - access_token: "background-refreshed-token", - }), - }); + it("schedules refresh before token expiry", async () => { + const { secretsManager, createManager } = + await setupBackgroundRefreshTest( + createMockTokenResponse({ + access_token: "background-refreshed-token", + }), + ); - // Create manager AFTER OAuth session is set up so it schedules refresh createManager(); - - // Advance to when refresh should trigger await vi.advanceTimersByTimeAsync(ONE_HOUR_MS - REFRESH_BUFFER_MS); const auth = await secretsManager.getSessionAuth(TEST_HOSTNAME); expect(auth?.token).toBe("background-refreshed-token"); }); - }); - describe("showReAuthenticationModal", () => { - it("clears OAuth state and prompts for re-login", async () => { - const { secretsManager, loginCoordinator, manager, setupOAuthSession } = - createTestContext(); + it("calls onAuthRequired when refresh fails with re-auth error", async () => { + const { createManager } = await setupBackgroundRefreshTest( + createOAuthAxiosError("invalid_grant"), + ); - await setupOAuthSession(); - await secretsManager.setOAuthClientRegistration( - TEST_HOSTNAME, - createMockClientRegistration(), + const onAuthRequired = vi.fn().mockResolvedValue(undefined); + createManager(createTestDeployment(), onAuthRequired); + await vi.advanceTimersByTimeAsync(ONE_HOUR_MS - REFRESH_BUFFER_MS); + + expect(onAuthRequired).toHaveBeenCalledOnce(); + }); + + it("does not call onAuthRequired for non-reauth errors", async () => { + const { createManager } = await setupBackgroundRefreshTest( + createOAuthAxiosError("server_error"), ); - await manager.showReAuthenticationModal( - new OAuthError("invalid_grant", "Token expired"), + const onAuthRequired = vi.fn().mockResolvedValue(undefined); + createManager(createTestDeployment(), onAuthRequired); + await vi.advanceTimersByTimeAsync(ONE_HOUR_MS - REFRESH_BUFFER_MS); + + expect(onAuthRequired).not.toHaveBeenCalled(); + + // Should schedule retry instead + await vi.advanceTimersByTimeAsync(BACKGROUND_REFRESH_INTERVAL_MS); + expect(onAuthRequired).not.toHaveBeenCalled(); + }); + + it("continues gracefully when onAuthRequired callback throws", async () => { + const { logger, createManager } = await setupBackgroundRefreshTest( + createOAuthAxiosError("invalid_grant"), ); - const auth = await secretsManager.getSessionAuth(TEST_HOSTNAME); - expect(auth?.oauth).toBeUndefined(); - expect(auth?.token).toBe(""); - expect(loginCoordinator.ensureLoggedInWithDialog).toHaveBeenCalled(); + const callbackError = new Error("Callback failed"); + const onAuthRequired = vi.fn().mockRejectedValue(callbackError); + createManager(createTestDeployment(), onAuthRequired); + await vi.advanceTimersByTimeAsync(ONE_HOUR_MS - REFRESH_BUFFER_MS); + + expect(onAuthRequired).toHaveBeenCalledOnce(); + expect(logger.error).toHaveBeenCalledWith( + "onAuthRequired callback failed:", + callbackError, + ); }); }); describe("concurrent refresh", () => { it("deduplicates concurrent calls", async () => { - const { secretsManager, mockAdapter, manager, setupOAuthSession } = - createTestContext(); - - await setupOAuthSession(); - await secretsManager.setOAuthClientRegistration( - TEST_HOSTNAME, - createMockClientRegistration(), - ); + const { manager, setupForOAuthOperation } = createTestContext(); let callCount = 0; - setupAxiosMockRoutes(mockAdapter, { - "/.well-known/oauth-authorization-server": - createMockOAuthMetadata(TEST_URL), + await setupForOAuthOperation({ "/oauth2/token": () => { callCount++; return createMockTokenResponse({ @@ -395,4 +425,60 @@ describe("OAuthSessionManager", () => { }).not.toThrow(); }); }); + + describe("revokeRefreshToken", () => { + it("revokes token via revocation endpoint", async () => { + const { manager, setupForOAuthOperation } = createTestContext(); + + let revokedToken: string | undefined; + await setupForOAuthOperation({ + "/oauth2/revoke": (config: InternalAxiosRequestConfig) => { + const params = new URLSearchParams(config.data as string); + revokedToken = params.get("token") ?? undefined; + return {}; + }, + }); + + await manager.revokeRefreshToken(); + + expect(revokedToken).toBe("refresh-token"); + }); + }); + + describe("scope validation", () => { + it("rejects tokens with insufficient scopes", async () => { + const { secretsManager, manager } = createTestContext(); + + await secretsManager.setSessionAuth(TEST_HOSTNAME, { + url: TEST_URL, + token: "access-token", + oauth: { + refresh_token: "refresh-token", + expiry_timestamp: Date.now() + ONE_HOUR_MS, + scope: "workspace:read", // Missing other required scopes + }, + }); + + const result = await manager.isLoggedInWithOAuth(); + expect(result).toBe(false); + }); + + it("accepts wildcard scopes", async () => { + const { secretsManager, manager } = createTestContext(); + + await secretsManager.setSessionAuth(TEST_HOSTNAME, { + url: TEST_URL, + token: "access-token", + oauth: { + refresh_token: "refresh-token", + expiry_timestamp: Date.now() + ONE_HOUR_MS, + // workspace:* covers workspace:read, workspace:update, etc. + scope: "workspace:* template:read user:read_personal", + }, + }); + + const result = await manager.isLoggedInWithOAuth(); + expect(result).toBe(true); + }); + }); }); diff --git a/test/unit/oauth/testUtils.ts b/test/unit/oauth/testUtils.ts index b3a2dacb..aa64759a 100644 --- a/test/unit/oauth/testUtils.ts +++ b/test/unit/oauth/testUtils.ts @@ -1,3 +1,4 @@ +import { AxiosError, AxiosHeaders } from "axios"; import { vi } from "vitest"; import { SecretsManager } from "@/core/secretsManager"; @@ -18,6 +19,32 @@ import type { OAuth2TokenResponse, } from "coder/site/src/api/typesGenerated"; +/** + * Creates an AxiosError with OAuth error response data. + */ +export function createOAuthAxiosError( + errorCode: string, + description?: string, +): AxiosError { + const data: Record = { error: errorCode }; + if (description) { + data.error_description = description; + } + return new AxiosError( + "OAuth Error", + "ERR_BAD_REQUEST", + undefined, + undefined, + { + status: 400, + statusText: "Bad Request", + headers: {}, + config: { headers: new AxiosHeaders() }, + data, + }, + ); +} + import type { Deployment } from "@/deployment/types"; export const TEST_URL = "https://coder.example.com";