diff --git a/src/commands.ts b/src/commands.ts index 1a219148f..e0aee6936 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -14,6 +14,11 @@ import * as cliExec from "./core/cliExec"; import { CertificateError } from "./error/certificateError"; import { toError } from "./error/errorUtils"; import { type FeatureSet, featureSetForVersion } from "./featureSet"; +import { + AuthTelemetry, + type AuthLoginOutcome, + type AuthLogoutOutcome, +} from "./instrumentation/auth"; import { reportElapsedProgress, withCancellableProgress, @@ -49,7 +54,7 @@ import type { PathResolver } from "./core/pathResolver"; import type { SecretsManager } from "./core/secretsManager"; import type { DeploymentManager } from "./deployment/deploymentManager"; import type { Logger } from "./logging/logger"; -import type { LoginCoordinator } from "./login/loginCoordinator"; +import type { LoginCoordinator, LoginMethod } from "./login/loginCoordinator"; import type { TelemetryService } from "./telemetry/service"; import type { SpeedtestPanelFactory } from "./webviews/speedtest/speedtestPanelFactory"; import type { @@ -68,6 +73,11 @@ interface OpenOptions { useDefaultDirectory?: boolean; } +interface LoginArgs { + readonly url?: string; + readonly autoLogin?: boolean; +} + const openDefaults = { openRecent: false, useDefaultDirectory: true, @@ -83,6 +93,7 @@ export class Commands { private readonly duplicateWorkspaceIpc: DuplicateWorkspaceIpc; private readonly speedtestPanelFactory: SpeedtestPanelFactory; private readonly telemetryService: TelemetryService; + private readonly authTelemetry: AuthTelemetry; // These will only be populated when actively connected to a workspace and are // used in commands. Because commands can be executed by the user, it is not @@ -109,6 +120,7 @@ export class Commands { this.loginCoordinator = serviceContainer.getLoginCoordinator(); this.duplicateWorkspaceIpc = serviceContainer.getDuplicateWorkspaceIpc(); this.speedtestPanelFactory = serviceContainer.getSpeedtestPanelFactory(); + this.authTelemetry = new AuthTelemetry(this.telemetryService); } /** @@ -137,20 +149,20 @@ export class Commands { * Log into a deployment. If already authenticated, this is a no-op. * If no URL is provided, shows a menu of recent URLs plus defaults. */ - public async login(args?: { - url?: string; - autoLogin?: boolean; - }): Promise { + public async login(args?: LoginArgs): Promise { if (this.deploymentManager.isAuthenticated()) { return; } - await this.performLogin(args); + await this.authTelemetry.traceLogin( + args?.autoLogin ? "auto_login" : "command", + (trace) => this.performLogin(args, trace.setMethod), + ); } - private async performLogin(args?: { - url?: string; - autoLogin?: boolean; - }): Promise { + private async performLogin( + args: LoginArgs | undefined, + setMethod: (method: LoginMethod) => void, + ): Promise { this.logger.debug("Logging in"); const currentDeployment = await this.secretsManager.getCurrentDeployment(); @@ -160,7 +172,7 @@ export class Commands { currentDeployment?.url, ); if (!url) { - return; // The user aborted. + return { success: false, reason: "no_url_provided" }; } const safeHostname = toSafeHost(url); @@ -173,19 +185,26 @@ export class Commands { }); if (!result.success) { - return; + return result; } + // Record the method eagerly so it is captured even if persistence throws. + setMethod(result.method); await this.deploymentManager.setDeployment({ url, safeHostname, token: result.token, user: result.user, }); + this.showWelcomeMessage(result.user.username); + this.logger.debug("Login complete to deployment:", url); + return { success: true, method: result.method }; + } + private showWelcomeMessage(username: string): void { vscode.window .showInformationMessage( - `Welcome to Coder, ${result.user.username}!`, + `Welcome to Coder, ${username}!`, { detail: "You can now use the Coder extension to manage your Coder instance.", @@ -197,7 +216,6 @@ export class Commands { vscode.commands.executeCommand("coder.open"); } }); - this.logger.debug("Login complete to deployment:", url); } /** @@ -418,21 +436,30 @@ export class Commands { * Log out and clear stored credentials, requiring re-authentication on next login. */ public async logout(): Promise { + await this.authTelemetry.traceLogout(() => this.performLogout()); + } + + private async performLogout(): Promise { if (!this.deploymentManager.isAuthenticated()) { - return; + return { success: false, reason: "not_authenticated" }; } this.logger.debug("Logging out"); const deployment = this.deploymentManager.getCurrentDeployment(); - - await this.deploymentManager.clearDeployment(); + await this.deploymentManager.clearDeployment("logout"); if (deployment) { await this.cliManager.clearCredentials(deployment.url); await this.secretsManager.clearAllAuthData(deployment.safeHostname); } + this.showLogoutMessage(); + this.logger.debug("Logout complete"); + return { success: true }; + } + + private showLogoutMessage(): void { vscode.window .showInformationMessage("You've been logged out of Coder!", "Login") .then((action) => { @@ -442,8 +469,6 @@ export class Commands { }); } }); - - this.logger.debug("Logout complete"); } /** @@ -452,7 +477,9 @@ export class Commands { */ public async switchDeployment(): Promise { this.logger.debug("Switching deployment"); - await this.performLogin(); + await this.authTelemetry.traceLogin("switch_deployment", (trace) => + this.performLogin(undefined, trace.setMethod), + ); } /** diff --git a/src/core/cliCredentialManager.ts b/src/core/cliCredentialManager.ts index c338c6387..a6399cc1a 100644 --- a/src/core/cliCredentialManager.ts +++ b/src/core/cliCredentialManager.ts @@ -7,8 +7,14 @@ import * as semver from "semver"; import { isAbortError } from "../error/errorUtils"; import { featureSetForVersion } from "../featureSet"; +import { + CredentialCliError, + CredentialFileError, + CredentialTelemetry, +} from "../instrumentation/credentials"; import { isKeyringEnabled } from "../settings/cli"; import { getHeaderArgs } from "../settings/headers"; +import { type TelemetryReporter } from "../telemetry/reporter"; import { toSafeHost } from "../util"; import { writeAtomically } from "../util/fs"; @@ -17,6 +23,7 @@ import { version } from "./cliExec"; import type { WorkspaceConfiguration } from "vscode"; import type { Logger } from "../logging/logger"; +import type { Span } from "../telemetry/span"; import type { PathResolver } from "./pathResolver"; @@ -46,11 +53,16 @@ export function isKeyringSupported(): boolean { * persistence: keyring-backed (via CLI) and file-based (plaintext). */ export class CliCredentialManager { + private readonly credentialTelemetry: CredentialTelemetry; + constructor( private readonly logger: Logger, private readonly resolveBinary: BinaryResolver, private readonly pathResolver: PathResolver, - ) {} + telemetry: TelemetryReporter, + ) { + this.credentialTelemetry = new CredentialTelemetry(telemetry); + } /** * Store credentials for a deployment URL. Uses the OS keyring when the @@ -59,22 +71,35 @@ export class CliCredentialManager { * * Keyring and files are mutually exclusive, never both. */ - public async storeToken( + public storeToken( url: string, token: string, configs: Pick, options?: { signal?: AbortSignal }, ): Promise { - const binPath = await this.resolveKeyringBinary( - url, - configs, - "keyringAuth", - ); - if (!binPath) { - await this.writeCredentialFiles(url, token); - return; - } + return this.credentialTelemetry.traceStore(configs, async (span) => { + const binPath = await this.resolveKeyringBinary( + url, + configs, + "keyringAuth", + ); + if (!binPath) { + span.setProperty("category", "file"); + await this.writeCredentialFiles(url, token); + return; + } + span.setProperty("category", "keyring"); + await this.storeKeyringToken(binPath, url, token, configs, options); + }); + } + private async storeKeyringToken( + binPath: string, + url: string, + token: string, + configs: Pick, + options?: { signal?: AbortSignal }, + ): Promise { const args = [ ...getHeaderArgs(configs), "login", @@ -89,7 +114,10 @@ export class CliCredentialManager { this.logger.info("Stored token via CLI for", url); } catch (error) { this.logger.warn("Failed to store token via CLI:", error); - throw error; + if (isAbortError(error)) { + throw error; + } + throw new CredentialCliError(error); } } @@ -139,15 +167,20 @@ export class CliCredentialManager { * deletion in parallel, both best-effort. Throws AbortError when the * signal is aborted. */ - public async deleteToken( + public deleteToken( url: string, configs: Pick, options?: { signal?: AbortSignal }, ): Promise { - await Promise.all([ - this.deleteCredentialFiles(url), - this.deleteKeyringToken(url, configs, options?.signal), - ]); + return this.credentialTelemetry.traceClear(configs, async (span) => { + await Promise.all([ + this.deleteCredentialFiles(url), + this.deleteKeyringToken(url, configs, { + signal: options?.signal, + span, + }), + ]); + }); } /** @@ -201,14 +234,18 @@ export class CliCredentialManager { url: string, token: string, ): Promise { - const safeHostname = toSafeHost(url); - await Promise.all([ - this.atomicWriteFile(this.pathResolver.getUrlPath(safeHostname), url), - this.atomicWriteFile( - this.pathResolver.getSessionTokenPath(safeHostname), - token, - ), - ]); + try { + const safeHostname = toSafeHost(url); + await Promise.all([ + this.atomicWriteFile(this.pathResolver.getUrlPath(safeHostname), url), + this.atomicWriteFile( + this.pathResolver.getSessionTokenPath(safeHostname), + token, + ), + ]); + } catch (error) { + throw new CredentialFileError(error); + } } /** @@ -230,18 +267,22 @@ export class CliCredentialManager { } /** - * Delete keyring token via `coder logout`. Best-effort: never throws. + * Delete keyring token via `coder logout`. Best-effort: records the failure + * on the span instead of throwing (except on abort), so it is tagged where + * it occurs. */ private async deleteKeyringToken( url: string, configs: Pick, - signal?: AbortSignal, + { signal, span }: { signal?: AbortSignal; span: Span }, ): Promise { let binPath: string | undefined; try { binPath = await this.resolveKeyringBinary(url, configs, "keyringAuth"); } catch (error) { this.logger.warn("Could not resolve keyring binary for delete:", error); + span.setProperty("failure_category", "binary"); + span.markFailure(); return; } if (!binPath) { @@ -257,6 +298,8 @@ export class CliCredentialManager { throw error; } this.logger.warn("Failed to delete token via CLI:", error); + span.setProperty("failure_category", "cli"); + span.markFailure(); } } diff --git a/src/core/container.ts b/src/core/container.ts index 858686924..b18b54d87 100644 --- a/src/core/container.ts +++ b/src/core/container.ts @@ -90,6 +90,7 @@ export class ServiceContainer implements vscode.Disposable { } }, this.pathResolver, + this.telemetryService, ); this.cliManager = new CliManager( this.logger, diff --git a/src/deployment/deploymentManager.ts b/src/deployment/deploymentManager.ts index 5868ecb91..8ef71d56e 100644 --- a/src/deployment/deploymentManager.ts +++ b/src/deployment/deploymentManager.ts @@ -7,6 +7,11 @@ import { type ServiceContainer } from "../core/container"; import { type ContextManager } from "../core/contextManager"; import { type MementoManager } from "../core/mementoManager"; import { type SecretsManager } from "../core/secretsManager"; +import { + DeploymentTelemetry, + type DeploymentRecoveryTrigger, + type DeploymentSuspendReason, +} from "../instrumentation/deployment"; import { type Logger } from "../logging/logger"; import { type OAuthSessionManager } from "../oauth/sessionManager"; import { getAuthConfigWatchSettings } from "../settings/authConfig"; @@ -40,6 +45,7 @@ export class DeploymentManager implements vscode.Disposable { private readonly contextManager: ContextManager; private readonly logger: Logger; private readonly telemetryService: TelemetryService; + private readonly deploymentTelemetry: DeploymentTelemetry; #deployment: Deployment | null = null; #disposed = false; @@ -60,6 +66,7 @@ export class DeploymentManager implements vscode.Disposable { this.contextManager = serviceContainer.getContextManager(); this.logger = serviceContainer.getLogger(); this.telemetryService = serviceContainer.getTelemetryService(); + this.deploymentTelemetry = new DeploymentTelemetry(this.telemetryService); } public static create( @@ -175,9 +182,9 @@ export class DeploymentManager implements vscode.Disposable { /** * Clears the current deployment. */ - public async clearDeployment(): Promise { + public async clearDeployment(reason: DeploymentSuspendReason): Promise { this.logger.debug("Clearing deployment", this.#deployment?.safeHostname); - this.suspendSession(); + this.suspendSession(reason); this.#authListenerDisposable?.dispose(); this.#authListenerDisposable = undefined; this.#deployment = null; @@ -190,11 +197,15 @@ export class DeploymentManager implements vscode.Disposable { * 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 { + public suspendSession(reason: DeploymentSuspendReason): void { + const wasAuthenticated = this.isAuthenticated(); this.oauthSessionManager.clearDeployment(); this.client.setCredentials(undefined, undefined); this.updateAuthContexts(undefined); this.clearWorkspaces(); + if (wasAuthenticated) { + this.deploymentTelemetry.suspended(reason); + } } /** @@ -242,14 +253,17 @@ export class DeploymentManager implements vscode.Disposable { this.logger.debug( "Token updated after session suspended, recovering", ); - await this.verifyAndApplyDeployment({ - url: auth.url, - safeHostname, - token: auth.token, - }); + await this.recoverDeployment( + { + url: auth.url, + safeHostname, + token: auth.token, + }, + "token_update", + ); } } else { - await this.clearDeployment(); + await this.clearDeployment("credentials_removed"); } }, ); @@ -285,7 +299,10 @@ export class DeploymentManager implements vscode.Disposable { this.logger.debug( "Authentication settings changed after session suspended, recovering", ); - await this.verifyAndApplyDeployment(snapshot); + const recovered = await this.recoverDeployment(snapshot, "auth_config"); + if (!recovered) { + this.deploymentTelemetry.authConfigRecoveryFailed(); + } } while (this.#recoveryPending); } catch (err) { this.logger.warn( @@ -307,12 +324,24 @@ export class DeploymentManager implements vscode.Disposable { if (deployment) { this.logger.info("Deployment changed from another window"); - await this.verifyAndApplyDeployment(deployment); + this.deploymentTelemetry.crossWindowDetected(); + await this.recoverDeployment(deployment, "cross_window"); } }, ); } + private async recoverDeployment( + deployment: Deployment & { token?: string }, + trigger: DeploymentRecoveryTrigger, + ): Promise { + const recovered = await this.verifyAndApplyDeployment(deployment); + if (recovered) { + this.deploymentTelemetry.recovered(trigger); + } + return recovered; + } + /** * Update authentication-related contexts. */ diff --git a/src/extension.ts b/src/extension.ts index c8e7fb8f3..5903d79cf 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -103,7 +103,7 @@ async function doActivate( // Shared handler for auth failures (used by interceptor + session manager) const handleAuthFailure = (): Promise => { - deploymentManager.suspendSession(); + deploymentManager.suspendSession("auth_failure"); vscode.window .showWarningMessage( "Session expired. You have been signed out.", diff --git a/src/instrumentation/auth.ts b/src/instrumentation/auth.ts index c6cba325c..c4d5f2018 100644 --- a/src/instrumentation/auth.ts +++ b/src/instrumentation/auth.ts @@ -1,8 +1,15 @@ +import type { LoginMethod } from "../login/loginCoordinator"; import type { TelemetryReporter } from "../telemetry/reporter"; +import type { Span } from "../telemetry/span"; export type AuthTokenRefreshTrigger = "background" | "reactive"; export type AuthRecoveryAction = "refresh_success" | "login_required" | "none"; export type AuthLoginPromptTrigger = "auth_required" | "missing_session"; +export type AuthLoginSource = + | "auto_login" + | "command" + | "switch_deployment" + | "uri"; export type LoginPromptReason = | "user_dismissed" @@ -12,6 +19,16 @@ export type LoginPromptReason = export type LoginPromptOutcome = | { success: true } | { success: false; reason: LoginPromptReason }; +export type AuthLoginOutcome = + | { success: true; method: LoginMethod } + | { success: false; method?: LoginMethod; reason: LoginPromptReason }; +export type AuthLogoutOutcome = + | { success: true } + | { success: false; reason: "not_authenticated" }; + +interface AuthLoginTrace { + setMethod: (method: LoginMethod) => void; +} interface AuthRecoveryRecorder { logReceived(): void; @@ -22,6 +39,51 @@ interface AuthRecoveryRecorder { export class AuthTelemetry { public constructor(private readonly telemetry: TelemetryReporter) {} + public traceLogin( + source: AuthLoginSource, + fn: (trace: AuthLoginTrace) => Promise, + ): Promise { + return this.telemetry.trace( + "auth.login", + async (span) => { + try { + const result = await fn({ + setMethod: (method) => span.setProperty("method", method), + }); + if (result.method) { + span.setProperty("method", result.method); + } + if (!result.success) { + recordReason(span, result.reason); + } + return result; + } catch (error) { + span.setProperty("reason", "exception"); + throw error; + } + }, + { source, method: "unknown" }, + ); + } + + public traceLogout( + fn: () => Promise, + ): Promise { + return this.telemetry.trace("auth.logout", async (span) => { + try { + const result = await fn(); + if (!result.success) { + span.setProperty("reason", result.reason); + span.markAborted(); + } + return result; + } catch (error) { + span.setProperty("reason", "exception"); + throw error; + } + }); + } + public traceTokenRefresh( trigger: AuthTokenRefreshTrigger, fn: () => Promise, @@ -68,12 +130,7 @@ export class AuthTelemetry { async (span) => { const result = await fn(); if (!result.success) { - span.setProperty("reason", result.reason); - if (result.reason === "auth_failed") { - span.markFailure(); - } else { - span.markAborted(); - } + recordReason(span, result.reason); } return result; }, @@ -81,3 +138,13 @@ export class AuthTelemetry { ); } } + +/** `auth_failed` is a real failure; user/URL dismissals are intentional aborts. */ +function recordReason(span: Span, reason: LoginPromptReason): void { + span.setProperty("reason", reason); + if (reason === "auth_failed") { + span.markFailure(); + } else { + span.markAborted(); + } +} diff --git a/src/instrumentation/credentials.ts b/src/instrumentation/credentials.ts new file mode 100644 index 000000000..754b26e1a --- /dev/null +++ b/src/instrumentation/credentials.ts @@ -0,0 +1,98 @@ +import { isAbortError } from "../error/errorUtils"; +import { isKeyringEnabled } from "../settings/cli"; + +import type { WorkspaceConfiguration } from "vscode"; + +import type { TelemetryReporter } from "../telemetry/reporter"; +import type { Span } from "../telemetry/span"; + +export type CredentialFailureCategory = "aborted" | "binary" | "cli" | "file"; + +type CredentialEvent = "auth.credential.store" | "auth.credential.clear"; + +/** + * Wraps credential store/clear in a span carrying `keyring_enabled`, the + * `category` of storage involved, and a `failure_category` on failure. The + * traced operation sets `category` on the span and reports failures by + * throwing a categorized error (store) or recording on the span (clear, which + * is best-effort). Aborts are recorded and re-thrown so callers still unwind. + */ +export class CredentialTelemetry { + public constructor(private readonly telemetry: TelemetryReporter) {} + + public traceStore( + configs: Pick, + fn: (span: Span) => Promise, + ): Promise { + return this.trace("auth.credential.store", configs, fn); + } + + public traceClear( + configs: Pick, + fn: (span: Span) => Promise, + ): Promise { + return this.trace("auth.credential.clear", configs, fn); + } + + private async trace( + eventName: CredentialEvent, + configs: Pick, + fn: (span: Span) => Promise, + ): Promise { + const keyringEnabled = isKeyringEnabled(configs); + let aborted: Error | undefined; + await this.telemetry.trace( + eventName, + async (span) => { + try { + await fn(span); + } catch (error) { + span.setProperty( + "failure_category", + categorizeCredentialError(error), + ); + if (isAbortError(error)) { + span.markAborted(); + aborted = error; + return; + } + throw error; + } + }, + { + keyring_enabled: keyringEnabled, + category: keyringEnabled ? "keyring" : "file", + }, + ); + if (aborted) { + throw aborted; + } + } +} + +function categorizeCredentialError(error: unknown): CredentialFailureCategory { + if (isAbortError(error)) { + return "aborted"; + } + if (error instanceof CredentialFileError) { + return "file"; + } + if (error instanceof CredentialCliError) { + return "cli"; + } + return "binary"; +} + +export class CredentialCliError extends Error { + public constructor(cause: unknown) { + super("Credential CLI operation failed", { cause }); + this.name = "CredentialCliError"; + } +} + +export class CredentialFileError extends Error { + public constructor(cause: unknown) { + super("Credential file operation failed", { cause }); + this.name = "CredentialFileError"; + } +} diff --git a/src/instrumentation/deployment.ts b/src/instrumentation/deployment.ts new file mode 100644 index 000000000..738b9ff91 --- /dev/null +++ b/src/instrumentation/deployment.ts @@ -0,0 +1,31 @@ +import type { TelemetryReporter } from "../telemetry/reporter"; + +export type DeploymentSuspendReason = + | "auth_config_change" + | "auth_failure" + | "credentials_removed" + | "logout"; +export type DeploymentRecoveryTrigger = + | "auth_config" + | "cross_window" + | "token_update"; + +export class DeploymentTelemetry { + public constructor(private readonly telemetry: TelemetryReporter) {} + + public suspended(reason: DeploymentSuspendReason): void { + this.telemetry.log("deployment.suspended", { reason }); + } + + public recovered(trigger: DeploymentRecoveryTrigger): void { + this.telemetry.log("deployment.recovered", { trigger }); + } + + public crossWindowDetected(): void { + this.telemetry.log("deployment.cross_window.detected"); + } + + public authConfigRecoveryFailed(): void { + this.telemetry.log("deployment.auth_config.recovery_failed"); + } +} diff --git a/src/login/loginCoordinator.ts b/src/login/loginCoordinator.ts index baac3f28d..cb02627a9 100644 --- a/src/login/loginCoordinator.ts +++ b/src/login/loginCoordinator.ts @@ -27,10 +27,28 @@ import type { import type { Logger } from "../logging/logger"; import type { OAuthCallback } from "../oauth/oauthCallback"; -type LoginResult = +export type LoginMethod = + | "mtls" + | "provided_token" + | "stored_token" + | "keyring_token" + | "cli_token" + | "oauth"; + +type LoginAttemptResult = | { success: false; reason: LoginPromptReason } | { success: true; user: User; token: string; oauth?: OAuthTokenData }; +export type LoginResult = + | { success: false; method?: LoginMethod; reason: LoginPromptReason } + | { + success: true; + method: LoginMethod; + user: User; + token: string; + oauth?: OAuthTokenData; + }; + export interface LoginOptions { safeHostname: string; url: string | undefined; @@ -63,10 +81,10 @@ export class LoginCoordinator implements vscode.Disposable { } /** - * Direct login - for user-initiated login via commands. + * Direct login for callers that already have a deployment URL. * Stores session auth and URL history on success. */ - public async ensureLoggedIn( + public ensureLoggedIn( options: LoginOptions & { url: string }, ): Promise { const { safeHostname, url } = options; @@ -92,7 +110,7 @@ export class LoginCoordinator implements vscode.Disposable { detailPrefix?: string; trigger: AuthLoginPromptTrigger; }, - ): Promise { + ): Promise { return this.authTelemetry.traceLoginPrompt(options.trigger, () => this.performLoginDialog(options), ); @@ -100,7 +118,7 @@ export class LoginCoordinator implements vscode.Disposable { private async performLoginDialog( options: LoginOptions & { message?: string; detailPrefix?: string }, - ): Promise { + ): Promise { const { safeHostname, url, detailPrefix, message } = options; return this.executeWithGuard(async () => { // Show dialog promise @@ -116,7 +134,7 @@ export class LoginCoordinator implements vscode.Disposable { }, "Login", ) - .then(async (action): Promise => { + .then(async (action): Promise => { if (action === "Login") { // Proceed with the login flow, handling logging in from another window const storedAuth = @@ -158,7 +176,7 @@ export class LoginCoordinator implements vscode.Disposable { } private async persistSessionAuth( - result: LoginResult, + result: LoginAttemptResult, safeHostname: string, url: string, ): Promise { @@ -184,9 +202,9 @@ export class LoginCoordinator implements vscode.Disposable { /** * Chains login attempts to prevent overlapping UI. */ - private executeWithGuard( - executeFn: () => Promise, - ): Promise { + private executeWithGuard( + executeFn: () => Promise, + ): Promise { const result = this.loginQueue.then(executeFn); this.loginQueue = result.catch(() => { /* Keep chain going on error */ @@ -199,11 +217,11 @@ export class LoginCoordinator implements vscode.Disposable { * Returns a promise and a dispose function to clean up the listener. */ private waitForCrossWindowLogin(safeHostname: string): { - promise: Promise; + promise: Promise; dispose: () => void; } { let disposable: vscode.Disposable | undefined; - const promise = new Promise((resolve) => { + const promise = new Promise((resolve) => { disposable = this.secretsManager.onDidChangeSessionAuth( safeHostname, async (auth) => { @@ -234,9 +252,9 @@ export class LoginCoordinator implements vscode.Disposable { /** * Attempt to authenticate using OAuth, token, or mTLS. If necessary, prompts - * for authentication method and credentials. Returns the token and user upon - * successful authentication. Null means the user aborted or authentication - * failed (in which case an error notification will have been displayed). + * for authentication method and credentials. Returns the token, user, and + * method on success, or a bounded reason when the user aborts or + * authentication fails. */ private async attemptLogin( deployment: Deployment, @@ -265,7 +283,10 @@ export class LoginCoordinator implements vscode.Disposable { // mTLS authentication (no token needed) if (!needToken(vscode.workspace.getConfiguration())) { this.logger.debug("Attempting mTLS authentication (no token required)"); - return this.tryMtlsAuth(client, isAutoLogin); + return withLoginMethod( + "mtls", + await this.tryMtlsAuth(client, isAutoLogin), + ); } // Try provided token first @@ -277,7 +298,7 @@ export class LoginCoordinator implements vscode.Disposable { isAutoLogin, ); if (result !== "unauthorized") { - return result; + return withLoginMethod("provided_token", result); } } @@ -289,7 +310,7 @@ export class LoginCoordinator implements vscode.Disposable { this.logger.debug("Trying stored session token"); const result = await this.tryTokenAuth(client, auth.token, isAutoLogin); if (result !== "unauthorized") { - return result; + return withLoginMethod("stored_token", result); } } @@ -316,7 +337,7 @@ export class LoginCoordinator implements vscode.Disposable { this.logger.debug("Trying token from OS keyring"); const result = await this.tryTokenAuth(client, keyringToken, isAutoLogin); if (result !== "unauthorized") { - return result; + return withLoginMethod("keyring_token", result); } } @@ -324,9 +345,9 @@ export class LoginCoordinator implements vscode.Disposable { const authMethod = await maybeAskAuthMethod(client); switch (authMethod) { case "oauth": - return this.loginWithOAuth(deployment); + return withLoginMethod("oauth", await this.loginWithOAuth(deployment)); case "legacy": - return this.loginWithToken(client); + return withLoginMethod("cli_token", await this.loginWithToken(client)); case undefined: return { success: false, reason: "user_dismissed" }; } @@ -335,7 +356,7 @@ export class LoginCoordinator implements vscode.Disposable { private async tryMtlsAuth( client: CoderApi, isAutoLogin: boolean, - ): Promise { + ): Promise { try { const user = await client.getAuthenticatedUser(); return { success: true, token: "", user }; @@ -352,7 +373,7 @@ export class LoginCoordinator implements vscode.Disposable { client: CoderApi, token: string, isAutoLogin: boolean, - ): Promise { + ): Promise { client.setSessionToken(token); try { const user = await client.getAuthenticatedUser(); @@ -392,7 +413,7 @@ export class LoginCoordinator implements vscode.Disposable { /** * Session token authentication flow. */ - private async loginWithToken(client: CoderApi): Promise { + private async loginWithToken(client: CoderApi): Promise { const url = client.getAxiosInstance().defaults.baseURL; if (!url) { throw new Error("No base URL set on REST client"); @@ -450,7 +471,9 @@ export class LoginCoordinator implements vscode.Disposable { /** * OAuth authentication flow. */ - private async loginWithOAuth(deployment: Deployment): Promise { + private async loginWithOAuth( + deployment: Deployment, + ): Promise { try { this.logger.debug("Starting OAuth authentication"); @@ -492,3 +515,10 @@ export class LoginCoordinator implements vscode.Disposable { this.oauthAuthorizer.dispose(); } } + +function withLoginMethod( + method: LoginMethod, + result: LoginAttemptResult, +): LoginResult { + return { ...result, method }; +} diff --git a/src/uri/uriHandler.ts b/src/uri/uriHandler.ts index a846b4ef4..861eeff24 100644 --- a/src/uri/uriHandler.ts +++ b/src/uri/uriHandler.ts @@ -1,6 +1,7 @@ import * as vscode from "vscode"; import { errToStr } from "../api/api-helper"; +import { AuthTelemetry } from "../instrumentation/auth"; import { CALLBACK_PATH } from "../oauth/utils"; import { maybeAskUrl } from "../promptUtils"; import { toSafeHost } from "../util"; @@ -131,6 +132,9 @@ async function setupDeployment( const secretsManager = serviceContainer.getSecretsManager(); const mementoManager = serviceContainer.getMementoManager(); const loginCoordinator = serviceContainer.getLoginCoordinator(); + const authTelemetry = new AuthTelemetry( + serviceContainer.getTelemetryService(), + ); const currentDeployment = await secretsManager.getCurrentDeployment(); @@ -151,11 +155,9 @@ async function setupDeployment( const safeHostname = toSafeHost(url); const token: string | undefined = params.get("token") ?? undefined; - const result = await loginCoordinator.ensureLoggedIn({ - safeHostname, - url, - token, - }); + const result = await authTelemetry.traceLogin("uri", () => + loginCoordinator.ensureLoggedIn({ safeHostname, url, token }), + ); if (!result.success) { throw new Error("Failed to login to deployment from URI"); diff --git a/test/unit/commands.telemetry.test.ts b/test/unit/commands.telemetry.test.ts new file mode 100644 index 000000000..aa0a41637 --- /dev/null +++ b/test/unit/commands.telemetry.test.ts @@ -0,0 +1,275 @@ +import { describe, expect, it, vi } from "vitest"; + +import { Commands } from "@/commands"; +import { maybeAskUrl } from "@/promptUtils"; + +import { createTelemetryHarness } from "../mocks/telemetry"; +import { + createMockLogger, + createMockUser, + MockUserInteraction, +} from "../mocks/testHelpers"; + +import type { CoderApi } from "@/api/coderApi"; +import type { CliManager } from "@/core/cliManager"; +import type { ServiceContainer } from "@/core/container"; +import type { MementoManager } from "@/core/mementoManager"; +import type { PathResolver } from "@/core/pathResolver"; +import type { SecretsManager } from "@/core/secretsManager"; +import type { DeploymentManager } from "@/deployment/deploymentManager"; +import type { Deployment } from "@/deployment/types"; +import type { LoginCoordinator, LoginResult } from "@/login/loginCoordinator"; +import type { SpeedtestPanelFactory } from "@/webviews/speedtest/speedtestPanelFactory"; +import type { DuplicateWorkspaceIpc } from "@/workspace/duplicateWorkspaceIpc"; + +vi.mock("@/promptUtils", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, maybeAskUrl: vi.fn() }; +}); + +vi.mock("@/workspace/workspacesProvider", () => { + class AgentTreeItem {} + class WorkspaceTreeItem {} + return { AgentTreeItem, WorkspaceTreeItem }; +}); + +const TEST_URL = "https://coder.example.com"; +const TEST_HOSTNAME = "coder.example.com"; + +type LoginResultForTest = LoginResult & { + readonly user?: ReturnType; +}; + +interface SetupOptions { + readonly authenticated?: boolean; + readonly loginResult?: LoginResultForTest; + readonly clearAllAuthDataError?: Error; +} + +function setup(options: SetupOptions = {}) { + vi.clearAllMocks(); + new MockUserInteraction(); + vi.mocked(maybeAskUrl).mockResolvedValue(TEST_URL); + + const { sink, service } = createTelemetryHarness(); + const deployment: Deployment = { + url: TEST_URL, + safeHostname: TEST_HOSTNAME, + }; + const loginResult = + options.loginResult ?? + ({ + success: true, + method: "stored_token", + user: createMockUser(), + token: "test-token", + } satisfies LoginResultForTest); + const loginCoordinator: Pick = { + ensureLoggedIn: vi.fn(() => Promise.resolve(loginResult)), + }; + + const deploymentManager: Pick< + DeploymentManager, + | "isAuthenticated" + | "getCurrentDeployment" + | "setDeployment" + | "clearDeployment" + > = { + isAuthenticated: vi.fn(() => options.authenticated ?? false), + getCurrentDeployment: vi.fn(() => deployment), + setDeployment: vi.fn(() => Promise.resolve()), + clearDeployment: vi.fn(() => Promise.resolve()), + }; + + const cliManager: Pick = { + clearCredentials: vi.fn(() => Promise.resolve()), + }; + + const secretsManager: Pick< + SecretsManager, + "getCurrentDeployment" | "clearAllAuthData" + > = { + getCurrentDeployment: vi.fn(() => Promise.resolve(null)), + clearAllAuthData: vi.fn(() => { + if (options.clearAllAuthDataError) { + return Promise.reject(options.clearAllAuthDataError); + } + return Promise.resolve(); + }), + }; + + const serviceContainer = { + getTelemetryService: () => service, + getLogger: () => createMockLogger(), + getPathResolver: () => ({}) as PathResolver, + getMementoManager: () => ({}) as MementoManager, + getSecretsManager: () => secretsManager, + getCliManager: () => cliManager, + getLoginCoordinator: () => loginCoordinator, + getDuplicateWorkspaceIpc: () => ({}) as DuplicateWorkspaceIpc, + getSpeedtestPanelFactory: () => ({}) as SpeedtestPanelFactory, + } as ServiceContainer; + + const extensionClient = { + getAxiosInstance: () => ({ defaults: { baseURL: TEST_URL } }), + } as unknown as CoderApi; + + const commands = new Commands( + serviceContainer, + extensionClient, + deploymentManager as DeploymentManager, + ); + + return { + commands, + sink, + mocks: { cliManager, deploymentManager, loginCoordinator, secretsManager }, + }; +} + +describe("Commands", () => { + describe("login telemetry", () => { + it("emits one auth.login for command login success", async () => { + const { commands, mocks, sink } = setup(); + + await commands.login(); + + const events = sink.eventsNamed("auth.login"); + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ + properties: { + source: "command", + method: "stored_token", + result: "success", + }, + }); + expect(events[0].measurements.durationMs).toEqual(expect.any(Number)); + expect(mocks.loginCoordinator.ensureLoggedIn).toHaveBeenCalledWith( + expect.objectContaining({ + safeHostname: TEST_HOSTNAME, + url: TEST_URL, + }), + ); + expect(mocks.deploymentManager.setDeployment).toHaveBeenCalled(); + }); + + it("uses auto_login source when requested", async () => { + const { commands, mocks, sink } = setup({ + loginResult: { + success: true, + method: "provided_token", + user: createMockUser(), + token: "test-token", + }, + }); + + await commands.login({ url: TEST_URL, autoLogin: true }); + + expect(sink.expectOne("auth.login").properties).toMatchObject({ + source: "auto_login", + method: "provided_token", + result: "success", + }); + expect(mocks.loginCoordinator.ensureLoggedIn).toHaveBeenCalledWith( + expect.objectContaining({ autoLogin: true }), + ); + }); + + it("records URL cancellation without attempting login", async () => { + const { commands, mocks, sink } = setup(); + vi.mocked(maybeAskUrl).mockResolvedValueOnce(undefined); + + await commands.login(); + + expect(sink.expectOne("auth.login")).toMatchObject({ + properties: { + source: "command", + method: "unknown", + result: "aborted", + reason: "no_url_provided", + }, + }); + expect(mocks.loginCoordinator.ensureLoggedIn).not.toHaveBeenCalled(); + }); + + it("records auth failures as auth.login errors", async () => { + const { commands, mocks, sink } = setup({ + loginResult: { + success: false, + method: "cli_token", + reason: "auth_failed", + }, + }); + + await commands.login(); + + expect(sink.expectOne("auth.login")).toMatchObject({ + properties: { + method: "cli_token", + result: "error", + reason: "auth_failed", + }, + }); + expect(mocks.deploymentManager.setDeployment).not.toHaveBeenCalled(); + }); + + it("uses switch_deployment source", async () => { + const { commands, sink } = setup(); + + await commands.switchDeployment(); + + expect(sink.expectOne("auth.login").properties).toMatchObject({ + source: "switch_deployment", + result: "success", + }); + }); + }); + + describe("logout telemetry", () => { + it("records not_authenticated as aborted", async () => { + const { commands, mocks, sink } = setup({ authenticated: false }); + + await commands.logout(); + + expect(sink.expectOne("auth.logout")).toMatchObject({ + properties: { + result: "aborted", + reason: "not_authenticated", + }, + }); + expect(mocks.cliManager.clearCredentials).not.toHaveBeenCalled(); + }); + + it("records successful logout", async () => { + const { commands, mocks, sink } = setup({ authenticated: true }); + + await commands.logout(); + + const event = sink.expectOne("auth.logout"); + expect(event.properties).toMatchObject({ result: "success" }); + expect(event.properties.reason).toBeUndefined(); + expect(event.measurements.durationMs).toEqual(expect.any(Number)); + expect(mocks.deploymentManager.clearDeployment).toHaveBeenCalledWith( + "logout", + ); + expect(mocks.cliManager.clearCredentials).toHaveBeenCalledWith(TEST_URL); + expect(mocks.secretsManager.clearAllAuthData).toHaveBeenCalledWith( + TEST_HOSTNAME, + ); + }); + + it("records logout exceptions", async () => { + const { commands, sink } = setup({ + authenticated: true, + clearAllAuthDataError: new Error("secret clear failed"), + }); + + await expect(commands.logout()).rejects.toThrow("secret clear failed"); + + expect(sink.expectOne("auth.logout")).toMatchObject({ + properties: { result: "error", reason: "exception" }, + error: { message: "secret clear failed" }, + }); + }); + }); +}); diff --git a/test/unit/core/cliCredentialManager.test.ts b/test/unit/core/cliCredentialManager.test.ts index 1167654bd..200eb8801 100644 --- a/test/unit/core/cliCredentialManager.test.ts +++ b/test/unit/core/cliCredentialManager.test.ts @@ -12,7 +12,11 @@ import * as cliExec from "@/core/cliExec"; import { PathResolver } from "@/core/pathResolver"; import { isKeyringEnabled } from "@/settings/cli"; -import { createMockLogger } from "../../mocks/testHelpers"; +import { createTestTelemetryService, TestSink } from "../../mocks/telemetry"; +import { + createMockLogger, + MockConfigurationProvider, +} from "../../mocks/testHelpers"; import type * as nodeFs from "node:fs"; @@ -136,12 +140,15 @@ function writeCredentialFiles(url: string, token: string) { function setup(resolver?: BinaryResolver) { const r = resolver ?? successResolver(); + const sink = new TestSink(); return { resolver: r, + sink, manager: new CliCredentialManager( createMockLogger(), r, TEST_PATH_RESOLVER, + createTestTelemetryService(sink), ), }; } @@ -160,6 +167,7 @@ describe("isKeyringSupported", () => { describe("CliCredentialManager", () => { beforeEach(() => { + new MockConfigurationProvider(); vi.clearAllMocks(); vol.reset(); vi.mocked(isKeyringEnabled).mockReturnValue(false); @@ -168,19 +176,26 @@ describe("CliCredentialManager", () => { describe("storeToken", () => { it("writes files when keyring is disabled", async () => { - const { manager } = setup(); + const { manager, sink } = setup(); await manager.storeToken(TEST_URL, "my-token", configs); expect(execFile).not.toHaveBeenCalled(); expect(memfs.readFileSync(URL_FILE, "utf8")).toBe(TEST_URL); expect(memfs.readFileSync(SESSION_FILE, "utf8")).toBe("my-token"); + expect(sink.expectOne("auth.credential.store")).toMatchObject({ + properties: { + category: "file", + keyring_enabled: "false", + result: "success", + }, + }); }); it("resolves binary and invokes coder login when keyring enabled", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); stubExecFile({ stdout: "" }); - const { manager, resolver } = setup(); + const { manager, resolver, sink } = setup(); await manager.storeToken(TEST_URL, "my-secret-token", configs); @@ -191,6 +206,13 @@ describe("CliCredentialManager", () => { // Token must only appear in env, never in args expect(exec.env.CODER_SESSION_TOKEN).toBe("my-secret-token"); expect(exec.args).not.toContain("my-secret-token"); + expect(sink.expectOne("auth.credential.store")).toMatchObject({ + properties: { + category: "keyring", + keyring_enabled: "true", + result: "success", + }, + }); }); it("falls back to files when CLI version too old", async () => { @@ -208,11 +230,17 @@ describe("CliCredentialManager", () => { it("throws when CLI exec fails", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); stubExecFile({ error: "login failed" }); - const { manager } = setup(); + const { manager, sink } = setup(); await expect( manager.storeToken(TEST_URL, "token", configs), - ).rejects.toThrow("login failed"); + ).rejects.toThrow("Credential CLI operation failed"); + expect(sink.expectOne("auth.credential.store")).toMatchObject({ + properties: { + failure_category: "cli", + result: "error", + }, + }); }); it("throws when binary resolver fails and keyring enabled", async () => { @@ -261,13 +289,19 @@ describe("CliCredentialManager", () => { it("rejects with AbortError when signal is pre-aborted", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); stubExecFileAbortable(); - const { manager } = setup(); + const { manager, sink } = setup(); await expect( manager.storeToken(TEST_URL, "token", configs, { signal: AbortSignal.abort(), }), ).rejects.toThrow("The operation was aborted"); + expect(sink.expectOne("auth.credential.store")).toMatchObject({ + properties: { + failure_category: "aborted", + result: "aborted", + }, + }); }); }); @@ -369,7 +403,7 @@ describe("CliCredentialManager", () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); stubExecFile({ stdout: "" }); writeCredentialFiles(TEST_URL, "old-token"); - const { manager, resolver } = setup(); + const { manager, resolver, sink } = setup(); await manager.deleteToken(TEST_URL, configs); @@ -379,6 +413,13 @@ describe("CliCredentialManager", () => { expect(exec.args).toEqual(["logout", "--url", TEST_URL, "--yes"]); expect(memfs.existsSync(URL_FILE)).toBe(false); expect(memfs.existsSync(SESSION_FILE)).toBe(false); + expect(sink.expectOne("auth.credential.clear")).toMatchObject({ + properties: { + category: "keyring", + keyring_enabled: "true", + result: "success", + }, + }); }); it("deletes files even when keyring is disabled", async () => { @@ -395,21 +436,34 @@ describe("CliCredentialManager", () => { it("never throws on CLI error", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); stubExecFile({ error: "logout failed" }); - const { manager } = setup(); + const { manager, sink } = setup(); await expect( manager.deleteToken(TEST_URL, configs), ).resolves.not.toThrow(); + expect(sink.expectOne("auth.credential.clear")).toMatchObject({ + properties: { + failure_category: "cli", + result: "error", + }, + }); }); it("never throws when binary resolver fails", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); - const { manager } = setup(failingResolver()); + const { manager, sink } = setup(failingResolver()); await expect( manager.deleteToken(TEST_URL, configs), - ).resolves.not.toThrow(); + ).resolves.toBeUndefined(); expect(execFile).not.toHaveBeenCalled(); + expect(sink.expectOne("auth.credential.clear")).toMatchObject({ + properties: { + category: "keyring", + failure_category: "binary", + result: "error", + }, + }); }); it("forwards header command args", async () => { @@ -450,13 +504,19 @@ describe("CliCredentialManager", () => { it("throws AbortError when signal is aborted", async () => { vi.mocked(isKeyringEnabled).mockReturnValue(true); stubExecFileAbortable(); - const { manager } = setup(); + const { manager, sink } = setup(); await expect( manager.deleteToken(TEST_URL, configs, { signal: AbortSignal.abort(), }), ).rejects.toThrow("The operation was aborted"); + expect(sink.expectOne("auth.credential.clear")).toMatchObject({ + properties: { + failure_category: "aborted", + result: "aborted", + }, + }); }); }); }); diff --git a/test/unit/deployment/deploymentManager.test.ts b/test/unit/deployment/deploymentManager.test.ts index 2e90287e1..e47c75cac 100644 --- a/test/unit/deployment/deploymentManager.test.ts +++ b/test/unit/deployment/deploymentManager.test.ts @@ -6,7 +6,7 @@ import { MementoManager } from "@/core/mementoManager"; import { SecretsManager } from "@/core/secretsManager"; import { DeploymentManager } from "@/deployment/deploymentManager"; -import { createTestTelemetryService } from "../../mocks/telemetry"; +import { createTestTelemetryService, TestSink } from "../../mocks/telemetry"; import { createMockLogger, createMockServiceContainer, @@ -77,7 +77,8 @@ function createTestContext() { validationMockClient as unknown as CoderApi, ); - const telemetryService = createTestTelemetryService(); + const telemetrySink = new TestSink(); + const telemetryService = createTestTelemetryService(telemetrySink); const setDeploymentUrlSpy = vi.spyOn(telemetryService, "setDeploymentUrl"); const manager = DeploymentManager.create( @@ -101,6 +102,7 @@ function createTestContext() { contextManager, mockOAuthSessionManager, mockWorkspaceProvider, + telemetrySink, telemetryService, setDeploymentUrlSpy, manager, @@ -145,7 +147,7 @@ describe("DeploymentManager", () => { user, }); - await manager.clearDeployment(); + await manager.clearDeployment("credentials_removed"); expect(manager.getCurrentDeployment()).toBeNull(); expect(manager.isAuthenticated()).toBe(false); @@ -316,8 +318,12 @@ describe("DeploymentManager", () => { }); it("picks up deployment when not authenticated", async () => { - const { mockClient, validationMockClient, secretsManager } = - createTestContext(); + const { + mockClient, + validationMockClient, + secretsManager, + telemetrySink, + } = createTestContext(); const user = createMockUser(); validationMockClient.setAuthenticatedUserResponse(user); @@ -338,6 +344,12 @@ describe("DeploymentManager", () => { expect(mockClient.host).toBe(TEST_URL); expect(mockClient.token).toBe("synced-token"); + expect( + telemetrySink.expectOne("deployment.cross_window.detected").properties, + ).toEqual({}); + expect(telemetrySink.expectOne("deployment.recovered")).toMatchObject({ + properties: { trigger: "cross_window" }, + }); }); it("handles mTLS deployment (empty token) from other window", async () => { @@ -406,7 +418,7 @@ describe("DeploymentManager", () => { user, }); - await manager.clearDeployment(); + await manager.clearDeployment("credentials_removed"); expect(mockClient.host).toBeUndefined(); expect(mockClient.token).toBeUndefined(); @@ -423,13 +435,31 @@ describe("DeploymentManager", () => { token: "test-token", user: createMockUser(), }); - await manager.clearDeployment(); + await manager.clearDeployment("credentials_removed"); expect(setDeploymentUrlSpy).toHaveBeenLastCalledWith(""); }); }); describe("suspendSession", () => { + it("emits deployment.suspended once for an authenticated session", async () => { + const { manager, telemetrySink } = createTestContext(); + + await manager.setDeployment({ + url: TEST_URL, + safeHostname: TEST_HOSTNAME, + token: "test-token", + user: createMockUser(), + }); + + manager.suspendSession("auth_failure"); + manager.suspendSession("auth_failure"); + + const events = telemetrySink.eventsNamed("deployment.suspended"); + expect(events).toHaveLength(1); + expect(events[0].properties).toEqual({ reason: "auth_failure" }); + }); + it("clears auth state but keeps deployment for re-login", async () => { const { mockClient, @@ -447,7 +477,7 @@ describe("DeploymentManager", () => { }); expect(manager.isAuthenticated()).toBe(true); - manager.suspendSession(); + manager.suspendSession("auth_failure"); // Auth state is cleared expect(mockOAuthSessionManager.clearDeployment).toHaveBeenCalled(); @@ -469,7 +499,7 @@ describe("DeploymentManager", () => { it("recovers from suspended state when auth settings change", async () => { vi.useFakeTimers(); try { - const { mockClient, validationMockClient, manager } = + const { mockClient, validationMockClient, telemetrySink, manager } = createTestContext(); const config = new MockConfigurationProvider(); const user = createMockUser(); @@ -481,7 +511,7 @@ describe("DeploymentManager", () => { token: "", user, }); - manager.suspendSession(); + manager.suspendSession("auth_failure"); expect(manager.isAuthenticated()).toBe(false); config.set("coder.tlsCertFile", "/path/to/cert.pem"); @@ -494,6 +524,9 @@ describe("DeploymentManager", () => { expect(validationMockClient.getAuthenticatedUser).toHaveBeenCalledTimes( 1, ); + expect(telemetrySink.expectOne("deployment.recovered")).toMatchObject({ + properties: { trigger: "auth_config" }, + }); } finally { vi.useRealTimers(); } @@ -520,11 +553,11 @@ describe("DeploymentManager", () => { token: "", user, }); - manager.suspendSession(); + manager.suspendSession("auth_failure"); config.set("coder.tlsCertFile", "/path/to/cert.pem"); await vi.advanceTimersByTimeAsync(CONFIG_CHANGE_DEBOUNCE_MS); - await manager.clearDeployment(); + await manager.clearDeployment("credentials_removed"); resolveAuth(user); await vi.runAllTimersAsync(); @@ -536,8 +569,13 @@ describe("DeploymentManager", () => { }); it("recovers from suspended state when tokens update", async () => { - const { mockClient, validationMockClient, secretsManager, manager } = - createTestContext(); + const { + mockClient, + validationMockClient, + secretsManager, + telemetrySink, + manager, + } = createTestContext(); const user = createMockUser(); validationMockClient.setAuthenticatedUserResponse(user); @@ -550,7 +588,7 @@ describe("DeploymentManager", () => { }); // Suspend session (simulates session expiry) - manager.suspendSession(); + manager.suspendSession("auth_failure"); expect(manager.isAuthenticated()).toBe(false); // Simulate token update (e.g., from another window or re-login) @@ -564,6 +602,44 @@ describe("DeploymentManager", () => { // Should recover and be authenticated again expect(mockClient.token).toBe("recovered-token"); expect(manager.isAuthenticated()).toBe(true); + expect(telemetrySink.expectOne("deployment.recovered")).toMatchObject({ + properties: { trigger: "token_update" }, + }); + }); + + it("logs failed auth-config recovery", async () => { + vi.useFakeTimers(); + try { + const { validationMockClient, telemetrySink, manager } = + createTestContext(); + const config = new MockConfigurationProvider(); + const user = createMockUser(); + + await manager.setDeployment({ + url: TEST_URL, + safeHostname: TEST_HOSTNAME, + token: "test-token", + user, + }); + manager.suspendSession("auth_failure"); + validationMockClient.setAuthenticatedUserResponse( + new Error("Auth failed"), + ); + + config.set("coder.tlsCertFile", "/path/to/cert.pem"); + await vi.advanceTimersByTimeAsync(CONFIG_CHANGE_DEBOUNCE_MS); + + expect(manager.isAuthenticated()).toBe(false); + expect(telemetrySink.eventsNamed("deployment.recovered")).toHaveLength( + 0, + ); + expect( + telemetrySink.expectOne("deployment.auth_config.recovery_failed") + .properties, + ).toEqual({}); + } finally { + vi.useRealTimers(); + } }); }); }); diff --git a/test/unit/instrumentation/auth.test.ts b/test/unit/instrumentation/auth.test.ts new file mode 100644 index 000000000..de7a37be0 --- /dev/null +++ b/test/unit/instrumentation/auth.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from "vitest"; + +import { AuthTelemetry } from "@/instrumentation/auth"; + +import { createTelemetryHarness } from "../../mocks/telemetry"; + +function setup() { + const { sink, service } = createTelemetryHarness(); + return { auth: new AuthTelemetry(service), sink }; +} + +describe("AuthTelemetry", () => { + describe("traceLogin", () => { + it.each([ + { + name: "records success with the returned method", + outcome: { success: true, method: "mtls" } as const, + properties: { + source: "uri", + method: "mtls", + result: "success", + }, + }, + { + name: "records cancellation with the latest traced method", + outcome: { + success: false, + reason: "user_dismissed", + } as const, + traceMethod: "cli_token" as const, + properties: { + source: "uri", + method: "cli_token", + result: "aborted", + reason: "user_dismissed", + }, + }, + { + name: "records auth failures as errors", + outcome: { + success: false, + method: "oauth", + reason: "auth_failed", + } as const, + properties: { + source: "uri", + method: "oauth", + result: "error", + reason: "auth_failed", + }, + }, + ])("$name", async ({ outcome, traceMethod, properties }) => { + const { auth, sink } = setup(); + + await auth.traceLogin("uri", (trace) => { + if (traceMethod) { + trace.setMethod(traceMethod); + } + return Promise.resolve(outcome); + }); + + expect(sink.expectOne("auth.login")).toMatchObject({ properties }); + }); + }); + + describe("traceLogout", () => { + it("emits auth.logout success with duration", async () => { + const { auth, sink } = setup(); + + await auth.traceLogout(() => Promise.resolve({ success: true })); + + const event = sink.expectOne("auth.logout"); + expect(event.properties).toMatchObject({ result: "success" }); + expect(event.properties.reason).toBeUndefined(); + expect(event.error).toBeUndefined(); + expect(event.measurements.durationMs).toEqual(expect.any(Number)); + }); + + it("marks not_authenticated as aborted", async () => { + const { auth, sink } = setup(); + + await auth.traceLogout(() => + Promise.resolve({ success: false, reason: "not_authenticated" }), + ); + + expect(sink.expectOne("auth.logout")).toMatchObject({ + properties: { + result: "aborted", + reason: "not_authenticated", + }, + }); + }); + + it("records exceptions as errors", async () => { + const { auth, sink } = setup(); + + await expect( + auth.traceLogout(() => Promise.reject(new Error("clear failed"))), + ).rejects.toThrow("clear failed"); + + expect(sink.expectOne("auth.logout")).toMatchObject({ + properties: { + result: "error", + reason: "exception", + }, + error: { message: "clear failed" }, + }); + }); + }); +}); diff --git a/test/unit/login/loginCoordinator.test.ts b/test/unit/login/loginCoordinator.test.ts index 280b12764..03031ccb5 100644 --- a/test/unit/login/loginCoordinator.test.ts +++ b/test/unit/login/loginCoordinator.test.ts @@ -191,7 +191,12 @@ describe("LoginCoordinator", () => { safeHostname: TEST_HOSTNAME, }); - expect(result).toEqual({ success: true, user, token: "stored-token" }); + expect(result).toEqual({ + success: true, + method: "stored_token", + user, + token: "stored-token", + }); const auth = await secretsManager.getSessionAuth(TEST_HOSTNAME); expect(auth?.token).toBe("stored-token"); @@ -215,7 +220,12 @@ describe("LoginCoordinator", () => { safeHostname: TEST_HOSTNAME, }); - expect(result).toEqual({ success: true, user, token: "new-token" }); + expect(result).toEqual({ + success: true, + method: "cli_token", + user, + token: "new-token", + }); // Verify new token was persisted const auth = await secretsManager.getSessionAuth(TEST_HOSTNAME); @@ -259,10 +269,17 @@ describe("LoginCoordinator", () => { safeHostname: TEST_HOSTNAME, }); - // Both should complete with the same result const [result1, result2] = await Promise.all([login1, login2]); - expect(result1.success).toBe(true); - expect(result1).toEqual(result2); + expect(result1).toMatchObject({ + success: true, + method: "cli_token", + token: "new-token", + }); + expect(result2).toMatchObject({ + success: true, + method: "stored_token", + token: "new-token", + }); // Input box should only be shown once (guard prevents duplicate prompts) expect(vscode.window.showInputBox).toHaveBeenCalledTimes(1); @@ -284,7 +301,12 @@ describe("LoginCoordinator", () => { safeHostname: TEST_HOSTNAME, }); - expect(result).toEqual({ success: true, user, token: "" }); + expect(result).toEqual({ + success: true, + method: "mtls", + user, + token: "", + }); // Verify empty string token was persisted const auth = await secretsManager.getSessionAuth(TEST_HOSTNAME); @@ -372,7 +394,12 @@ describe("LoginCoordinator", () => { token: "provided-token", }); - expect(result).toEqual({ success: true, user, token: "provided-token" }); + expect(result).toEqual({ + success: true, + method: "provided_token", + user, + token: "provided-token", + }); }); it("falls back to stored token when provided token is invalid", async () => { @@ -396,7 +423,12 @@ describe("LoginCoordinator", () => { token: "invalid-provided-token", }); - expect(result).toEqual({ success: true, user, token: "stored-token" }); + expect(result).toEqual({ + success: true, + method: "stored_token", + user, + token: "stored-token", + }); }); it("prompts user when both provided and stored tokens are invalid", async () => { @@ -430,6 +462,7 @@ describe("LoginCoordinator", () => { expect(result).toEqual({ success: true, + method: "cli_token", user, token: "user-entered-token", }); @@ -467,6 +500,7 @@ describe("LoginCoordinator", () => { expect(result).toEqual({ success: true, + method: "cli_token", user, token: "user-entered-token", }); @@ -536,7 +570,12 @@ describe("LoginCoordinator", () => { const result = await login(); - expect(result).toEqual({ success: true, user, token: "stored-token" }); + expect(result).toEqual({ + success: true, + method: "stored_token", + user, + token: "stored-token", + }); }); }); diff --git a/test/unit/uri/uriHandler.test.ts b/test/unit/uri/uriHandler.test.ts index 7e448076d..4c87ae72f 100644 --- a/test/unit/uri/uriHandler.test.ts +++ b/test/unit/uri/uriHandler.test.ts @@ -6,6 +6,7 @@ import { SecretsManager } from "@/core/secretsManager"; import { OAuthCallback } from "@/oauth/oauthCallback"; import { CALLBACK_PATH } from "@/oauth/utils"; import { maybeAskUrl } from "@/promptUtils"; +import { NOOP_TELEMETRY_REPORTER } from "@/telemetry/reporter"; import { registerUriHandler } from "@/uri/uriHandler"; import { @@ -81,6 +82,7 @@ function createTestContext() { getContextManager: () => new MockContextManager(), getOAuthCallback: () => oauthCallback, getLogger: () => logger, + getTelemetryService: () => NOOP_TELEMETRY_REPORTER, } as unknown as ServiceContainer; vi.mocked(maybeAskUrl).mockImplementation((_m, urlParam) =>