From b4e90b1a76bcb6e9b07adbee522d75a7fa4a5b1d Mon Sep 17 00:00:00 2001 From: TylerLeonhardt <2644648+TylerLeonhardt@users.noreply.github.com> Date: Tue, 8 Sep 2026 19:57:00 -0700 Subject: [PATCH] agentHost: refresh expiring GitHub credentials (#334418) Propagate authentication session lifetimes through AHP and use the Copilot SDK token provider for renewable Entra-backed GitHub sessions. Centralize static and provider-backed credential behavior behind one owner and reuse shared expiry helpers throughout authentication replay. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extensions/github-authentication/package.json | 3 +- .../github-authentication/src/github.ts | 52 +++++-- .../src/test/github.test.ts | 85 +++++++++- .../github-authentication/tsconfig.json | 3 +- src/vs/base/common/date.ts | 25 +++ src/vs/base/test/common/date.test.ts | 21 ++- .../browser/agentHostProtocolClient.ts | 19 ++- src/vs/platform/agentHost/common/agent.ts | 4 +- .../common/state/protocol/.ahp-version | 2 +- .../common/state/protocol/common/commands.ts | 20 ++- .../state/protocol/common/notifications.ts | 10 +- .../node/agentHostAuthenticationService.ts | 27 +++- .../agentHost/node/copilot/copilotAgent.ts | 84 +++++----- .../node/copilot/copilotAgentSession.ts | 40 +++-- .../node/copilot/copilotGitHubCredentials.ts | 145 ++++++++++++++++++ .../node/copilot/copilotSessionLauncher.ts | 12 +- .../agentHostProtocolClient.test.ts | 4 +- .../agentHost/test/node/agentService.test.ts | 4 +- .../agentHost/test/node/copilotAgent.test.ts | 80 +++++++++- .../test/node/copilotAgentSession.test.ts | 14 +- .../node/copilotGitHubCredentials.test.ts | 93 +++++++++++ .../test/node/copilotSessionLauncher.test.ts | 19 +-- .../platform/agentHost/test/node/mockAgent.ts | 6 +- .../common/extensionsApiProposals.ts | 3 + .../agentSessions/agentHost/agentHostAuth.ts | 107 +++++++------ .../agentSessions/agentHostAuth.test.ts | 43 ++++-- .../authentication/common/authentication.ts | 1 + ...vscode.proposed.authSessionExpiration.d.ts | 18 +++ 28 files changed, 744 insertions(+), 200 deletions(-) create mode 100644 src/vs/platform/agentHost/node/copilot/copilotGitHubCredentials.ts create mode 100644 src/vs/platform/agentHost/test/node/copilotGitHubCredentials.test.ts create mode 100644 src/vscode-dts/vscode.proposed.authSessionExpiration.d.ts diff --git a/extensions/github-authentication/package.json b/extensions/github-authentication/package.json index eb0e7f2b6184..a835d1c03c26 100644 --- a/extensions/github-authentication/package.json +++ b/extensions/github-authentication/package.json @@ -20,7 +20,8 @@ "enabledApiProposals": [ "authIssuers", "authProviderSpecific", - "authSessionAccountIcon" + "authSessionAccountIcon", + "authSessionExpiration" ], "activationEvents": [], "capabilities": { diff --git a/extensions/github-authentication/src/github.ts b/extensions/github-authentication/src/github.ts index 5d8ff26b966c..5250aeb743b6 100644 --- a/extensions/github-authentication/src/github.ts +++ b/extensions/github-authentication/src/github.ts @@ -42,6 +42,8 @@ interface ITransientSession { readonly expiresAt: number; } +const TOKEN_RENEWAL_WINDOW_MS = 60 * 60 * 1000; + export enum AuthProviderType { github = 'github', githubEnterprise = 'github-enterprise' @@ -267,7 +269,10 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid /** Every session held only by this process, whether or not its token is still any good. */ private get transientSessions(): vscode.AuthenticationSession[] { - return [...this._transientSessions.values()].map(held => held.session); + return [...this._transientSessions.values()].map(held => ({ + ...held.session, + expiresIn: held.expiresAt > Date.now() ? Math.ceil((held.expiresAt - Date.now()) / 1000) : undefined + })); } get onDidChangeSessions() { @@ -397,8 +402,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid return undefined; } - const session = this.sessionFor(renewed.account, renewed.token, [...renewed.scopes]); - this._transientSessions.set(session.id, { session, expiresAt: Date.now() + renewed.expiresIn * 1000 }); + const session = this.storeTransientSession(this.sessionFor(renewed.account, renewed.token, [...renewed.scopes]), renewed.expiresIn); this.afterSessionLoad(session); return session; } @@ -422,7 +426,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid for (const session of wanted) { // No transient entry means a persisted session, and nothing persisted carries an expiry. const held = this._transientSessions.get(session.id); - if (!held || held.expiresAt > now) { + if (!held || held.expiresAt > now + TOKEN_RENEWAL_WINDOW_MS) { usable.push(session); } else { stale.push(session); @@ -437,8 +441,26 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid } const renewed = await Promise.all(stale.map(session => this.renew(session))); - this.evict(stale.filter((_, index) => !renewed[index]), 'their token ran out and could not be renewed'); - return [...usable, ...renewed.filter((session?: T): session is T => Boolean(session))]; + const expired: vscode.AuthenticationSession[] = []; + const retained: vscode.AuthenticationSession[] = []; + for (let index = 0; index < stale.length; index++) { + if (renewed[index]) { + continue; + } + + const held = this._transientSessions.get(stale[index].id); + const remainingLifetime = held ? held.expiresAt - Date.now() : 0; + if (held && remainingLifetime > 0) { + retained.push({ + ...held.session, + expiresIn: Math.ceil(remainingLifetime / 1000) + }); + } else { + expired.push(stale[index]); + } + } + this.evict(expired, 'their token ran out and could not be renewed'); + return [...usable, ...retained, ...renewed.filter((session?: T): session is T => Boolean(session))]; } /** @@ -531,8 +553,7 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid // The same session with a new token, so it keeps its id and is reported as changed rather // than as one session going away and another arriving. - const next: vscode.AuthenticationSession = { ...session, accessToken: renewed.token }; - this._transientSessions.set(next.id, { session: next, expiresAt: Date.now() + renewed.expiresIn * 1000 }); + const next = this.storeTransientSession({ ...session, accessToken: renewed.token }, renewed.expiresIn); this._logger.info(`Renewed session ${session.id}.`); this._sessionChangeEmitter.fire({ added: [], removed: [], changed: [next] }); return next; @@ -794,20 +815,19 @@ export class GitHubAuthenticationProvider implements vscode.AuthenticationProvid const exchanged = await this._githubServer.loginWithMicrosoft(scopes, { microsoftAccount: await this.rememberedMicrosoftAccount(gitHubAccountLabel) }); - const session = this.sessionFor(exchanged.account, exchanged.token, scopes); + const session = this.storeTransientSession(this.sessionFor(exchanged.account, exchanged.token, scopes), exchanged.expiresIn); this.afterSessionLoad(session); - this._transientSessions.set(session.id, { - session, - // The exchange reports what GitHub said the token is good for; when that runs out is - // this side's question, since it is the side that has to hold the session until then. - expiresAt: Date.now() + exchanged.expiresIn * 1000 - }); - this._sessionChangeEmitter.fire({ added: [session], removed: [], changed: [] }); return session; } + private storeTransientSession(session: vscode.AuthenticationSession, expiresIn: number): vscode.AuthenticationSession { + const result = { ...session, expiresIn }; + this._transientSessions.set(result.id, { session: result, expiresAt: Date.now() + expiresIn * 1000 }); + return result; + } + /** * A session for a GitHub identity that has already been resolved, so nothing here needs a * lookup. diff --git a/extensions/github-authentication/src/test/github.test.ts b/extensions/github-authentication/src/test/github.test.ts index b8b2a07c1ff8..54c110098f38 100644 --- a/extensions/github-authentication/src/test/github.test.ts +++ b/extensions/github-authentication/src/test/github.test.ts @@ -8,7 +8,8 @@ import * as vscode from 'vscode'; import { AccountLinks } from '../common/accountLinks'; import { IGitHubUserInfo } from '../common/gitHubAccount'; import { Log } from '../common/logger'; -import { EntraTokenExchangeError, EntraTokenExchangeFailure, IEntraRenewal, IEntraRenewedToken } from '../entraTokenExchange'; +import { EntraTokenExchangeError, EntraTokenExchangeFailure, IEntraExchangedToken, IEntraLoginOptions, IEntraRenewal, IEntraRenewedToken } from '../entraTokenExchange'; +import { GitHubSignInProvider } from '../flows'; import { AuthProviderType, GitHubAuthenticationProvider } from '../github'; import { TestMemento } from './testMemento'; @@ -128,6 +129,7 @@ suite('GitHub Microsoft-brokered sessions', () => { _microsoftGeneration: number; _microsoft: { getAccounts(): Promise }; _githubServer: { + loginWithMicrosoft(scopes: readonly string[], options?: IEntraLoginOptions): Promise; renewWithMicrosoft(renewal: IEntraRenewal): Promise; sendAdditionalTelemetryInfo(session: vscode.AuthenticationSession): Promise; }; @@ -140,6 +142,8 @@ suite('GitHub Microsoft-brokered sessions', () => { readonly accountLinks: AccountLinks; /** Every renewal the provider put on the wire, in order. */ readonly renewals: IEntraRenewal[]; + /** Every interactive Microsoft exchange the provider put on the wire, in order. */ + readonly logins: Array<{ readonly scopes: readonly string[]; readonly options: IEntraLoginOptions | undefined }>; /** What the provider told VS Code changed, as `verb account` for each session. */ readonly announced: string[]; /** The sessions still held in memory, as `account until` for each. */ @@ -163,8 +167,10 @@ suite('GitHub Microsoft-brokered sessions', () => { /** Sessions already held in memory, and how long each has left in milliseconds. */ transient?: readonly (readonly [vscode.AuthenticationSession, number])[]; microsoftAccounts?: (call: number) => vscode.AuthenticationSessionAccountInformation[]; + login?: (call: number, scopes: readonly string[], options: IEntraLoginOptions | undefined) => Promise; renew?: (call: number, renewal: IEntraRenewal) => Promise; } = {}): IHarness { + const logins: Array<{ scopes: readonly string[]; options: IEntraLoginOptions | undefined }> = []; const renewals: IEntraRenewal[] = []; const announced: string[] = []; const accountLinks = new AccountLinks(new TestMemento(), STORAGE_KEY, logger); @@ -186,6 +192,12 @@ suite('GitHub Microsoft-brokered sessions', () => { getAccounts: async () => overrides.microsoftAccounts?.(microsoftReads++) ?? [MICROSOFT_ACCOUNT] }, _githubServer: { + loginWithMicrosoft: async (scopes, options) => { + logins.push({ scopes, options }); + return overrides.login + ? await overrides.login(logins.length - 1, scopes, options) + : { token: `gho_login_${logins.length}`, expiresIn: 7200, account: GITHUB_ACCOUNT }; + }, renewWithMicrosoft: async renewal => { renewals.push(renewal); return overrides.renew @@ -210,6 +222,7 @@ suite('GitHub Microsoft-brokered sessions', () => { provider: provider as GitHubAuthenticationProvider, state: provider as IProviderState, accountLinks, + logins, renewals, announced, heldSessions: () => [...transientSessions.values()] @@ -218,6 +231,24 @@ suite('GitHub Microsoft-brokered sessions', () => { }; } + test('publishes the lifetime reported by an interactive Microsoft exchange', async () => { + const harness = createHarness(); + + const session = await harness.provider.createSession(SCOPES, { provider: GitHubSignInProvider.Microsoft }); + + assert.deepStrictEqual({ + token: session.accessToken, + expiresIn: session.expiresIn, + logins: harness.logins, + announced: harness.announced, + }, { + token: 'gho_login_1', + expiresIn: 7200, + logins: [{ scopes: SCOPES, options: { microsoftAccount: undefined } }], + announced: ['added mona_contoso'], + }); + }); + async function withLink(harness: IHarness): Promise { await harness.accountLinks.link(MICROSOFT_ACCOUNT.label, { id: GITHUB_ACCOUNT.id, label: GITHUB_ACCOUNT.accountName }); return harness; @@ -235,16 +266,29 @@ suite('GitHub Microsoft-brokered sessions', () => { scopes: sessions.map(session => session.scopes), renewals: harness.renewals, announced: harness.announced, - held: harness.heldSessions() + held: harness.heldSessions(), + expiresIn: sessions.map(session => session.expiresIn), }, { accounts: ['mona_contoso'], scopes: [SCOPES], renewals: [{ scopes: SCOPES, gitHubAccountId: '42', microsoftAccount: MICROSOFT_ACCOUNT }], announced: ['added mona_contoso'], - held: ['mona_contoso live'] + held: ['mona_contoso live'], + expiresIn: [3600] }); }); + test('recomputes the remaining lifetime of a cached Microsoft-brokered session', async () => { + const session = sessionFor('mona_contoso', 'cached', 'gho_cached'); + const harness = createHarness({ + transient: [[session, 2 * 60 * 60 * 1000]] + }); + + const [resolved] = await harness.provider.getSessions(SCOPES); + + assert.ok(resolved.expiresIn !== undefined && resolved.expiresIn > 3600 && resolved.expiresIn <= 7200); + }); + test('keeps what the user agreed to when a restore fails for a reason that says nothing about who they are', async () => { const ambiguous = async (failure: EntraTokenExchangeFailure) => { const harness = await withLink(createHarness({ @@ -315,29 +359,54 @@ suite('GitHub Microsoft-brokered sessions', () => { }); }); - test('settles every session whose token has run out, not only when there is nothing to hand back', async () => { + test('renews every session in the renewal window, not only when there is nothing else to hand back', async () => { const harness = await withLink(createHarness({ // Another account, signed in the ordinary way, so it has no expiry and is always usable. persisted: [sessionFor('hubot', 'persisted', 'gho_persisted')], - transient: [[sessionFor('mona_contoso', 'expired', 'gho_stale'), -1000]] + transient: [[sessionFor('mona_contoso', 'expiring', 'gho_stale'), 60 * 60 * 1000]] })); const sessions = await harness.provider.getSessions(SCOPES); assert.deepStrictEqual({ accounts: sessions.map(session => session.account.label).sort(), - // An expired session left unprocessed is never handed out, never renewed and never + // A stale session left unprocessed is never handed out, never renewed and never // reported as removed, but stays a candidate on every read for the life of the window. held: harness.heldSessions(), // Renewed in place, so it keeps its id and is reported as changed rather than as one // account going away and another arriving. announced: harness.announced, - tokens: sessions.map(session => session.accessToken).sort() + tokens: sessions.map(session => session.accessToken).sort(), + expirations: sessions.map(session => [session.account.label, session.expiresIn]).sort(), }, { accounts: ['hubot', 'mona_contoso'], held: ['mona_contoso live'], announced: ['changed mona_contoso'], - tokens: ['gho_1', 'gho_persisted'] + tokens: ['gho_1', 'gho_persisted'], + expirations: [['hubot', undefined], ['mona_contoso', 3600]] + }); + }); + + test('keeps a still-valid session when proactive renewal fails', async () => { + const harness = await withLink(createHarness({ + transient: [[sessionFor('mona_contoso', 'expiring', 'gho_still_valid'), 60 * 60 * 1000]], + renew: async () => { throw new EntraTokenExchangeError(EntraTokenExchangeFailure.Network, 'offline'); } + })); + + const sessions = await harness.provider.getSessions(SCOPES); + + assert.deepStrictEqual({ + tokens: sessions.map(session => session.accessToken), + hasUsableLifetime: sessions.every(session => session.expiresIn !== undefined && session.expiresIn > 0 && session.expiresIn <= 3600), + held: harness.heldSessions(), + announced: harness.announced, + renewals: harness.renewals.length + }, { + tokens: ['gho_still_valid'], + hasUsableLifetime: true, + held: ['mona_contoso live'], + announced: [], + renewals: 1 }); }); }); diff --git a/extensions/github-authentication/tsconfig.json b/extensions/github-authentication/tsconfig.json index ff8545f48547..836b5861933f 100644 --- a/extensions/github-authentication/tsconfig.json +++ b/extensions/github-authentication/tsconfig.json @@ -20,6 +20,7 @@ "../../src/vscode-dts/vscode.d.ts", "../../src/vscode-dts/vscode.proposed.authIssuers.d.ts", "../../src/vscode-dts/vscode.proposed.authProviderSpecific.d.ts", - "../../src/vscode-dts/vscode.proposed.authSessionAccountIcon.d.ts" + "../../src/vscode-dts/vscode.proposed.authSessionAccountIcon.d.ts", + "../../src/vscode-dts/vscode.proposed.authSessionExpiration.d.ts" ] } diff --git a/src/vs/base/common/date.ts b/src/vs/base/common/date.ts index 6562ab5a7d95..aa43c9474696 100644 --- a/src/vs/base/common/date.ts +++ b/src/vs/base/common/date.ts @@ -14,6 +14,31 @@ const week = day * 7; const month = day * 30; const year = day * 365; +/** + * Returns the absolute expiration time for a relative lifetime in seconds. + */ +export function getExpirationTime(expiresIn: number | undefined, now = Date.now()): number | undefined { + return expiresIn === undefined ? undefined : now + expiresIn * 1000; +} + +/** + * Returns whether a known expiration time has passed. + */ +export function isExpired(expiresAt: number | undefined, now = Date.now()): boolean { + return expiresAt !== undefined && expiresAt <= now; +} + +/** + * Returns the positive number of whole seconds until `expiresAt`, rounded up. + */ +export function getRemainingTimeInSeconds(expiresAt: number | undefined, now = Date.now()): number | undefined { + if (expiresAt === undefined) { + return undefined; + } + const remaining = Math.ceil((expiresAt - now) / 1000); + return remaining > 0 ? remaining : undefined; +} + /** * Create a localized difference of the time between now and the specified date. * @param date The date to generate the difference from. diff --git a/src/vs/base/test/common/date.test.ts b/src/vs/base/test/common/date.test.ts index 70a725552f98..ef67204bf5d7 100644 --- a/src/vs/base/test/common/date.test.ts +++ b/src/vs/base/test/common/date.test.ts @@ -4,13 +4,32 @@ *--------------------------------------------------------------------------------------------*/ import { strictEqual } from 'assert'; -import { fromNow, fromNowByDay, getDurationString, safeIntl } from '../../common/date.js'; +import { fromNow, fromNowByDay, getDurationString, getExpirationTime, getRemainingTimeInSeconds, isExpired, safeIntl } from '../../common/date.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from './utils.js'; import { LANGUAGE_DEFAULT } from '../../common/platform.js'; suite('Date', () => { ensureNoDisposablesAreLeakedInTestSuite(); + test('getRemainingTimeInSeconds', () => { + strictEqual(getRemainingTimeInSeconds(undefined, 1000), undefined); + strictEqual(getRemainingTimeInSeconds(1000, 1000), undefined); + strictEqual(getRemainingTimeInSeconds(1001, 1000), 1); + strictEqual(getRemainingTimeInSeconds(2001, 1000), 2); + }); + + test('getExpirationTime', () => { + strictEqual(getExpirationTime(undefined, 1000), undefined); + strictEqual(getExpirationTime(0, 1000), 1000); + strictEqual(getExpirationTime(2, 1000), 3000); + }); + + test('isExpired', () => { + strictEqual(isExpired(undefined, 1000), false); + strictEqual(isExpired(1000, 1000), true); + strictEqual(isExpired(1001, 1000), false); + }); + suite('fromNow', () => { test('appendAgoLabel', () => { strictEqual(fromNow(Date.now() - 35000), '35 secs'); diff --git a/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts b/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts index 35f1b47a29df..37ebc1c65a9f 100644 --- a/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts +++ b/src/vs/platform/agentHost/browser/agentHostProtocolClient.ts @@ -37,6 +37,7 @@ import { AhpErrorCodes, JsonRpcErrorCodes } from '../common/state/protocol/error import { ChatSourceKind, ContentEncoding, ResourceRequestParams, type CompletionsParams, type CompletionsResult, type CreateTerminalParams, type ResolveSessionConfigResult, type SessionConfigCompletionsResult } from '../common/state/protocol/commands.js'; import type { InvokeChangesetOperationParams, InvokeChangesetOperationResult } from '../common/state/protocol/channels-changeset/commands.js'; import { decodeBase64, encodeBase64 } from '../../../base/common/buffer.js'; +import { getExpirationTime, getRemainingTimeInSeconds, isExpired } from '../../../base/common/date.js'; import type { FetchAutomationRunsParams, FetchAutomationRunsResult, ListAutomationTriggerDefinitionsParams, ListAutomationTriggerDefinitionsResult, RunAutomationParams, RunAutomationResult } from '../common/state/protocol/channels-automation/commands.js'; import { ILoadEstimator, LoadEstimator } from '../../../base/parts/ipc/common/ipc.net.js'; import { ITelemetryService, TelemetryLevel, TELEMETRY_CRASH_REPORTER_SETTING_ID, TELEMETRY_OLD_SETTING_ID, TELEMETRY_SETTING_ID } from '../../telemetry/common/telemetry.js'; @@ -275,7 +276,7 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect /** Pending JSON-RPC requests keyed by request id. */ private readonly _pendingRequests = new Map(); - private readonly _authentication = new Map(); + private readonly _authentication = new Map(); private _nextRequestId = 1; /** @@ -1008,7 +1009,8 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect if (initialAuthentication) { const normalizedParams = this._normalizeAuthenticationParams(initialAuthentication); initialAuthenticationKey = this._authenticationKey(normalizedParams); - this._authentication.set(initialAuthenticationKey, normalizedParams); + const expiresAt = getExpirationTime(normalizedParams.expiresIn); + this._authentication.set(initialAuthenticationKey, { params: normalizedParams, expiresAt }); } } catch (error) { throw new InitialAuthenticationError(error); @@ -1017,12 +1019,20 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect return; } } - await Promise.all([...this._authentication.entries()].map(async ([key, params]) => { + await Promise.all([...this._authentication.entries()].map(async ([key, authentication]) => { + const now = Date.now(); + if (isExpired(authentication.expiresAt, now)) { + this._authentication.delete(key); + return; + } + const expiresIn = getRemainingTimeInSeconds(authentication.expiresAt, now); + const params = authentication.params; try { await this._dispatchRequest('authenticate', { channel: ROOT_STATE_URI, ...params, scopes: params.scopes ? [...params.scopes] : undefined, + ...(expiresIn === undefined ? {} : { expiresIn }), }, this._state.kind === AgentHostClientState.Connecting ? { bypassInitializeQueue: true, bypassReconnectGate: true } : { bypassReconnectGate: true }); @@ -1411,6 +1421,7 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect */ async authenticate(params: AuthenticateParams): Promise { const normalizedParams = this._normalizeAuthenticationParams(params); + const expiresAt = getExpirationTime(params.expiresIn); await this._sendRequest('authenticate', { channel: ROOT_STATE_URI, ...normalizedParams, @@ -1418,7 +1429,7 @@ export class AgentHostProtocolClient extends Disposable implements IAgentConnect }); const key = this._authenticationKey(normalizedParams); if (params.token) { - this._authentication.set(key, normalizedParams); + this._authentication.set(key, { params: normalizedParams, expiresAt }); } else { this._authentication.delete(key); } diff --git a/src/vs/platform/agentHost/common/agent.ts b/src/vs/platform/agentHost/common/agent.ts index a9c5caf83bba..7dcbe0c11611 100644 --- a/src/vs/platform/agentHost/common/agent.ts +++ b/src/vs/platform/agentHost/common/agent.ts @@ -295,6 +295,8 @@ export interface AuthenticateParams { /** The bearer token value (RFC 6750). */ readonly token: string; + /** The access token's remaining lifetime in seconds, when known. */ + readonly expiresIn?: number; } /** Request for a previously accepted bearer token. */ @@ -1279,7 +1281,7 @@ export interface IAgent { getProtectedResources(): ProtectedResourceMetadata[]; /** An empty token revokes the credential previously forwarded for this resource. */ - authenticate(resource: string, token: string): Promise; + authenticate(resource: string, token: string, expiresIn?: number): Promise; /** Optional token consumer for provider-owned resources such as MCP servers. */ handleAuthenticationToken?(params: AuthenticateParams): Promise; diff --git a/src/vs/platform/agentHost/common/state/protocol/.ahp-version b/src/vs/platform/agentHost/common/state/protocol/.ahp-version index 6110301d110e..7dc5824dd8a3 100644 --- a/src/vs/platform/agentHost/common/state/protocol/.ahp-version +++ b/src/vs/platform/agentHost/common/state/protocol/.ahp-version @@ -1 +1 @@ -60706330 +fd0471d4 diff --git a/src/vs/platform/agentHost/common/state/protocol/common/commands.ts b/src/vs/platform/agentHost/common/state/protocol/common/commands.ts index 5ac6b4778a52..f20293b34fe5 100644 --- a/src/vs/platform/agentHost/common/state/protocol/common/commands.ts +++ b/src/vs/platform/agentHost/common/state/protocol/common/commands.ts @@ -1130,7 +1130,8 @@ export interface ResourceMkdirResult { * ```jsonc * // Client → Server * { "jsonrpc": "2.0", "id": 3, "method": "authenticate", - * "params": { "channel": "ahp-root://", "resource": "https://api.github.com", "token": "gho_xxxx" } } + * "params": { "channel": "ahp-root://", "resource": "https://api.github.com", + * "token": "gho_xxxx", "expiresIn": 3540 } } * * // Server → Client (success) * { "jsonrpc": "2.0", "id": 3, "result": {} } @@ -1150,6 +1151,23 @@ export interface AuthenticateParams extends BaseParams { resource: string; /** Bearer token obtained from the resource's authorization server */ token: string; + /** + * The access token's remaining lifetime, in seconds, when this + * `authenticate` request is sent. This corresponds to `expires_in` in an + * OAuth 2.0 token response (RFC 6749 section 5.1). + * + * If the client retained the original token response, it MUST subtract the + * elapsed time before forwarding this value. Omit this field when the + * authorization server did not supply an expiry or the expiry is otherwise + * unknown. When supplied, the value MUST be a positive integer. + * + * This field is irrelevant when `token` is empty to revoke authentication + * and SHOULD be omitted in that case. + * + * @integer + * @minimum 1 + */ + expiresIn?: number; /** * OAuth scopes the token grants, when known. Lets the server determine * whether a specific challenge — e.g. the `requiredScopes` on a live diff --git a/src/vs/platform/agentHost/common/state/protocol/common/notifications.ts b/src/vs/platform/agentHost/common/state/protocol/common/notifications.ts index caff18926682..dbf91000407e 100644 --- a/src/vs/platform/agentHost/common/state/protocol/common/notifications.ts +++ b/src/vs/platform/agentHost/common/state/protocol/common/notifications.ts @@ -17,7 +17,10 @@ import type { ProtectedResourceMetadata, URI } from './state.js'; export const enum AuthRequiredReason { /** The client has not yet authenticated for the resource */ Required = 'required', - /** A previously valid token has expired or been revoked */ + /** + * A previously valid token has expired or been revoked. The client must + * acquire or renew the credential rather than replaying the challenged token. + */ Expired = 'expired', } @@ -32,8 +35,9 @@ export const enum AuthRequiredReason { * to; the `resource` field carries the complete OAuth protected resource * metadata (per RFC 9728). * - * Clients should obtain a fresh token and push it via the `authenticate` - * command. + * Clients should obtain or renew the credential and push the resulting token + * via the `authenticate` command. When `reason` is `expired`, clients MUST NOT + * blindly replay the challenged token. * * @category Protocol Notifications * @method auth/required diff --git a/src/vs/platform/agentHost/node/agentHostAuthenticationService.ts b/src/vs/platform/agentHost/node/agentHostAuthenticationService.ts index fb3aabd25aab..2ba34954ba6e 100644 --- a/src/vs/platform/agentHost/node/agentHostAuthenticationService.ts +++ b/src/vs/platform/agentHost/node/agentHostAuthenticationService.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { getExpirationTime, getRemainingTimeInSeconds, isExpired } from '../../../base/common/date.js'; import { Emitter, Event } from '../../../base/common/event.js'; import { Disposable } from '../../../base/common/lifecycle.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; @@ -34,6 +35,7 @@ interface IStoredAuthToken { readonly resource: string; readonly scopes: readonly string[]; readonly token: string; + readonly expiresAt: number | undefined; } export class AgentHostAuthenticationService extends Disposable implements IAgentHostAuthenticationService, IAgentHostAuthenticationController { @@ -51,6 +53,7 @@ export class AgentHostAuthenticationService extends Disposable implements IAgent async authenticate(params: AuthenticateParams, providers: Iterable): Promise { this._logService.trace(`[AgentHostAuthenticationService] authenticate called: resource=${params.resource}`); + const expiresAt = getExpirationTime(params.expiresIn); const providerList = [...providers]; // Multiple providers may share the same protected resource (e.g. // both Copilot CLI and Claude consume the Copilot-scoped OAuth credential). @@ -63,7 +66,7 @@ export class AgentHostAuthenticationService extends Disposable implements IAgent p => p.getProtectedResources().some(r => r.resource === params.resource), ); const settled = await Promise.allSettled( - matching.map(p => p.authenticate(params.resource, params.token)), + matching.map(p => p.authenticate(params.resource, params.token, params.expiresIn)), ); let authenticated = false; let rejected = false; @@ -106,7 +109,7 @@ export class AgentHostAuthenticationService extends Disposable implements IAgent // while clearing its own live state. this._tokens.delete(key); } else if (authenticated) { - this._tokens.set(key, { resource: params.resource, scopes, token: params.token }); + this._tokens.set(key, { resource: params.resource, scopes, token: params.token, expiresAt }); } const token = this._tokens.get(key)?.token; if (previousToken !== token) { @@ -117,11 +120,17 @@ export class AgentHostAuthenticationService extends Disposable implements IAgent async replay(provider: IAgent): Promise { const protectedResources = new Set(provider.getProtectedResources().map(resource => resource.resource)); - for (const stored of this._tokens.values()) { - const params: AuthenticateParams = { resource: stored.resource, scopes: stored.scopes, token: stored.token }; + for (const [key, stored] of this._tokens) { + const now = Date.now(); + if (isExpired(stored.expiresAt, now)) { + this._tokens.delete(key); + continue; + } + const expiresIn = getRemainingTimeInSeconds(stored.expiresAt, now); + const params: AuthenticateParams = { resource: stored.resource, scopes: stored.scopes, token: stored.token, expiresIn }; if (protectedResources.has(stored.resource)) { try { - await provider.authenticate(stored.resource, stored.token); + await provider.authenticate(stored.resource, stored.token, expiresIn); } catch (error) { this._logService.error(error, `[AgentHostAuthenticationService] Provider '${provider.id}' rejected replayed authentication for resource=${stored.resource}`); } @@ -139,7 +148,7 @@ export class AgentHostAuthenticationService extends Disposable implements IAgent getAuthToken(request: IAgentHostAuthTokenRequest): string | undefined { const scopes = this._normalizeScopes(request.scopes); const exact = this._tokens.get(this._key(request.resource, scopes)); - if (exact) { + if (exact && !isExpired(exact.expiresAt)) { return exact.token; } if (scopes.length === 0) { @@ -149,7 +158,7 @@ export class AgentHostAuthenticationService extends Disposable implements IAgent const requested = new Set(scopes); let best: IStoredAuthToken | undefined; for (const candidate of this._tokens.values()) { - if (candidate.resource !== request.resource || candidate.scopes.length === 0) { + if (candidate.resource !== request.resource || candidate.scopes.length === 0 || isExpired(candidate.expiresAt)) { continue; } if (!this._containsAll(candidate.scopes, requested)) { @@ -165,7 +174,8 @@ export class AgentHostAuthenticationService extends Disposable implements IAgent // Compatibility for clients that resolved the right token before scopes // were forwarded through the authenticate command. - return this._tokens.get(this._key(request.resource, []))?.token; + const unscoped = this._tokens.get(this._key(request.resource, [])); + return unscoped && !isExpired(unscoped.expiresAt) ? unscoped.token : undefined; } private _containsAll(scopes: readonly string[], requested: ReadonlySet): boolean { @@ -184,4 +194,5 @@ export class AgentHostAuthenticationService extends Disposable implements IAgent private _normalizeScopes(scopes: readonly string[] | undefined): readonly string[] { return scopes ? [...new Set(scopes)].sort() : []; } + } diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index 794858eb56fa..5d66841eb254 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -83,6 +83,7 @@ import { createCopilotCliEnvironment } from './copilotCliEnvironment.js'; import { ICopilotSessionContext, projectFromCopilotContext } from './copilotGitProject.js'; import { parsedPluginsEqual, toChildCustomizations } from './copilotPluginConverters.js'; import { CopilotGitHubTelemetryForwarder } from './copilotGitHubTelemetryForwarder.js'; +import { CopilotGitHubCredentials } from './copilotGitHubCredentials.js'; import { CopilotSecondaryAssignmentContext } from './copilotSecondaryAssignmentContext.js'; import { CopilotSessionLauncher, AutoTierConfigKey, ContextSizeConfigKey, ThinkingLevelConfigKey, getCopilotContextTier, isCopilotReasoningEffort, resolveCopilotAutoTier, resolveCopilotReasoningEffort, type CopilotSessionLaunchPlan, type IActiveClientSnapshot } from './copilotSessionLauncher.js'; import { CopilotAgentStartupConfig } from './copilotAgentStartupConfig.js'; @@ -842,7 +843,7 @@ export class CopilotAgent extends Disposable implements IAgent { private _closedConnectionRecovery: { readonly clientFailureId: string; readonly promise: Promise } | undefined; private readonly _authenticationSequencer = new Sequencer(); private _updatingGitHubCredentials = false; - private _githubToken: string | undefined; + private readonly _githubCredentials = this._register(new CopilotGitHubCredentials()); private _serverToolHost: IAgentServerToolHost | undefined; setServerToolHost(host: IAgentServerToolHost): void { @@ -963,6 +964,7 @@ export class CopilotAgent extends Disposable implements IAgent { @IAgentHostWorktreeIsolation worktree: IAgentHostWorktreeIsolation, ) { super(); + this._register(this._githubCredentials.onDidRequestRefresh(() => this._handleCopilotSessionAuthRequired())); this._worktree = worktree; this._lastStartupConfig = this._readClientStartupConfig(); this._plugins = this._register(this._instantiationService.createInstance(PluginController, () => this._ensureClient())); @@ -1362,9 +1364,9 @@ export class CopilotAgent extends Disposable implements IAgent { async getNetworkDiagnosticsEndpoints(): Promise { let capiUrl = process.env['VSCODE_AGENT_HOST_CAPI_URL_OVERRIDE'] || COPILOT_CAPI_URL; - if (this._githubToken) { + if (this._githubCredentials.token) { try { - capiUrl = await this._copilotApiService.resolveApiEndpoint(this._githubToken) || capiUrl; + capiUrl = await this._copilotApiService.resolveApiEndpoint(this._githubCredentials.token) || capiUrl; } catch (error) { this._logService.debug(`[Copilot] CAPI endpoint discovery for network diagnostics failed; using ${capiUrl}: ${error instanceof Error ? error.message : String(error)}`); } @@ -1378,7 +1380,7 @@ export class CopilotAgent extends Disposable implements IAgent { } async getNetworkDiagnosticsAccount(): Promise { - return this._githubToken ? this._copilotApiService.resolveUserLogin?.(this._githubToken) : undefined; + return this._githubCredentials.token ? this._copilotApiService.resolveUserLogin?.(this._githubCredentials.token) : undefined; } async getManagedSettingsDiagnostics(): Promise { @@ -1403,7 +1405,7 @@ export class CopilotAgent extends Disposable implements IAgent { stage = 'querying native MDM and GitHub managed settings'; return getCopilotManagedSettingsDiagnostics( runtimeSdk, - this._githubToken, + this._githubCredentials.token, this._gitHubEndpointService.getEnterpriseUri() ?? 'https://github.com', AbortSignal.timeout(COPILOT_MANAGED_SETTINGS_DIAGNOSTICS_TIMEOUT_MS), COPILOT_MANAGED_SETTINGS_QUERY_TIMEOUT_MS, @@ -1718,7 +1720,7 @@ export class CopilotAgent extends Disposable implements IAgent { }; } - async authenticate(resource: string, token: string): Promise { + async authenticate(resource: string, token: string, expiresIn?: number): Promise { if (resource === this._gitHubEndpointService.getRepoResource().resource) { return true; } @@ -1733,18 +1735,18 @@ export class CopilotAgent extends Disposable implements IAgent { if (token) { this._authenticationRequired.set(undefined, undefined); } - await this._applyGitHubToken(token || undefined); + await this._applyGitHubToken(token || undefined, expiresIn); }); return true; } - private async _applyGitHubToken(token: string | undefined): Promise { - if (this._githubToken === token) { + private async _applyGitHubToken(token: string | undefined, expiresIn: number | undefined): Promise { + const { tokenChanged, modeChanged: tokenProviderModeChanged } = this._githubCredentials.update(token, expiresIn); + if (!tokenChanged && !tokenProviderModeChanged) { return; } this._logService.info(`[Copilot] Auth token ${token ? 'updated' : 'cleared'}`); this._telemetryService.setCommonProperty('copilotSku', undefined); - this._githubToken = token; this._updateRestrictedTelemetry(token); this._refreshProxy(); if (!token) { @@ -1765,21 +1767,27 @@ export class CopilotAgent extends Disposable implements IAgent { return; } const host = this._gitHubEndpointService.getEnterpriseUri() ?? 'https://github.com'; - let restartRequired = false; + let restartRequired = tokenProviderModeChanged; this._updatingGitHubCredentials = true; try { - for (const session of this._allLiveSessions()) { - try { - const result = await session.updateGitHubCredentials(host, token); - if (!result.success) { - restartRequired = true; - this._logService.warn(`[Copilot:${session.sessionId}] GitHub credential update was rejected; scheduling a safe CopilotClient restart`); - } else if (result.copilotUserResolved === false) { - this._logService.warn(`[Copilot:${session.sessionId}] GitHub credentials were updated, but Copilot user metadata could not be resolved; plan, quota, and billing metadata may be degraded. Reauthenticate to restore it.`); + if (!tokenProviderModeChanged) { + for (const session of this._allLiveSessions()) { + // Provider-backed SDK sessions receive this token through their registered callback. + if (!session.usesStaticGitHubToken) { + continue; + } + try { + const result = await session.updateGitHubCredentials(host, token); + if (!result.success) { + restartRequired = true; + this._logService.warn(`[Copilot:${session.sessionId}] GitHub credential update was rejected; scheduling a safe CopilotClient restart`); + } else if (result.copilotUserResolved === false) { + this._logService.warn(`[Copilot:${session.sessionId}] GitHub credentials were updated, but Copilot user metadata could not be resolved; plan, quota, and billing metadata may be degraded. Reauthenticate to restore it.`); + } + } catch (error) { + restartRequired = true; + this._logService.warn(`[Copilot:${session.sessionId}] Failed to update GitHub credentials; scheduling a safe CopilotClient restart: ${getErrorMessage(error)}`); } - } catch (error) { - restartRequired = true; - this._logService.warn(`[Copilot:${session.sessionId}] Failed to update GitHub credentials; scheduling a safe CopilotClient restart: ${getErrorMessage(error)}`); } } } finally { @@ -1787,7 +1795,7 @@ export class CopilotAgent extends Disposable implements IAgent { await this._applyPendingClientRestart(); } if (restartRequired) { - await this._requestClientRestart('GitHub credential update failed'); + await this._requestClientRestart(tokenProviderModeChanged ? 'GitHub credential mode changed' : 'GitHub credential update failed'); } await this._resolveCopilotSku(token); void this._scheduleModelRefresh(); @@ -1803,7 +1811,7 @@ export class CopilotAgent extends Disposable implements IAgent { private async _resolveCopilotSku(githubToken: string): Promise { try { const copilotSku = await this._copilotApiService.resolveCopilotSku?.(githubToken); - if (copilotSku && this._githubToken === githubToken) { + if (copilotSku && this._githubCredentials.token === githubToken) { this._telemetryService.setCommonProperty('copilotSku', copilotSku); } } catch (err) { @@ -1832,7 +1840,7 @@ export class CopilotAgent extends Disposable implements IAgent { private async _resolveRestrictedTelemetry(githubToken: string): Promise { try { const ctx = await this._copilotApiService.resolveRestrictedTelemetryContext(githubToken); - if (this._githubToken !== githubToken) { + if (this._githubCredentials.token !== githubToken) { return; // token changed while resolving; a newer call owns the state } this._applyRestrictedTelemetry({ @@ -1882,7 +1890,7 @@ export class CopilotAgent extends Disposable implements IAgent { } const sessionId = notification.sessionId; - const githubToken = this._githubToken; + const githubToken = this._githubCredentials.token; if (!githubToken) { await router.route(notification, undefined, additionalProperties); return; @@ -1890,7 +1898,7 @@ export class CopilotAgent extends Disposable implements IAgent { try { const context = await this._copilotApiService.resolveRestrictedTelemetryContext(githubToken); - if (this._githubToken !== githubToken) { + if (this._githubCredentials.token !== githubToken) { return; } await router.route(notification, { @@ -2014,7 +2022,7 @@ export class CopilotAgent extends Disposable implements IAgent { return; } - const tokenAtRefreshStart = this._githubToken; + const tokenAtRefreshStart = this._githubCredentials.token; if (!tokenAtRefreshStart) { this._capiModels = []; this._publishModels(); @@ -2022,7 +2030,7 @@ export class CopilotAgent extends Disposable implements IAgent { } try { const models = await this._listModels(tokenAtRefreshStart); - if (this._githubToken === tokenAtRefreshStart && this._modelCatalogGeneration === generation) { + if (this._githubCredentials.token === tokenAtRefreshStart && this._modelCatalogGeneration === generation) { this._capiModels = models; this._publishModels(); } @@ -2030,7 +2038,7 @@ export class CopilotAgent extends Disposable implements IAgent { // Token rotated mid-flight — a newer refresh owns the result — or // teardown began while the request was in flight, in which case a // retry would just resurrect the client we are tearing down. - if (this._githubToken !== tokenAtRefreshStart || this._modelCatalogGeneration !== generation || this._shutdownPromise) { + if (this._githubCredentials.token !== tokenAtRefreshStart || this._modelCatalogGeneration !== generation || this._shutdownPromise) { return; } if (/\b401\b/.test(getErrorMessage(err))) { @@ -3958,7 +3966,7 @@ export class CopilotAgent extends Disposable implements IAgent { disabledRootMcpServers: await this._disabledRootMcpServers(sessionUri, sdkSessionId, snapshot), activeClientToolSet: activeClient.toolSet, shellManager, - githubToken: this._githubToken, + githubCredentials: this._githubCredentials.forSession(), model: provisional.model, longContextWindow: this._longContextWindowFor(provisional.model?.id), freeLongContext: this._isFreeLongContext(provisional.model?.id), @@ -4480,7 +4488,7 @@ export class CopilotAgent extends Disposable implements IAgent { disabledRootMcpServers: await this._disabledRootMcpServers(session, sdkSessionId, snapshot), activeClientToolSet: activeClient.toolSet, shellManager, - githubToken: this._githubToken, + githubCredentials: this._githubCredentials.forSession(), fallback: { model, longContextWindow: this._longContextWindowFor(model?.id), freeLongContext: this._isFreeLongContext(model?.id) }, }; } else { @@ -4495,7 +4503,7 @@ export class CopilotAgent extends Disposable implements IAgent { disabledRootMcpServers: await this._disabledRootMcpServers(session, chatSdkId, snapshot), activeClientToolSet: activeClient.toolSet, shellManager, - githubToken: this._githubToken, + githubCredentials: this._githubCredentials.forSession(), model, longContextWindow: this._longContextWindowFor(model?.id), freeLongContext: this._isFreeLongContext(model?.id), @@ -4939,7 +4947,7 @@ export class CopilotAgent extends Disposable implements IAgent { disabledRootMcpServers: await this._disabledRootMcpServers(configurationResource, info.sdkSessionId, snapshot), activeClientToolSet: activeClient.toolSet, shellManager, - githubToken: this._githubToken, + githubCredentials: this._githubCredentials.forSession(), fallback: { model: info.model, longContextWindow: this._longContextWindowFor(info.model?.id), freeLongContext: this._isFreeLongContext(info.model?.id) }, }; agentSession = this._createAgentSession(launchPlan, workingDirectory, activeClient, { sessionUri: configurationResource, chatChannelUri: chat, resource: context.resource }); @@ -5099,6 +5107,7 @@ export class CopilotAgent extends Disposable implements IAgent { if (!this._shutdownPromise) { this._isShuttingDown = true; this._sessionsPendingRegistration.clearAndDisposeAll(); + this._githubCredentials.shutdown(); for (const lifetime of this._sessionLifetimes.values()) { void lifetime.close(); } @@ -5213,9 +5222,9 @@ export class CopilotAgent extends Disposable implements IAgent { } let capiUrl = env['VSCODE_AGENT_HOST_CAPI_URL_OVERRIDE'] || COPILOT_CAPI_URL; - if (this._githubToken) { + if (this._githubCredentials.token) { try { - const discovered = await this._copilotApiService.resolveApiEndpoint(this._githubToken); + const discovered = await this._copilotApiService.resolveApiEndpoint(this._githubCredentials.token); if (discovered) { capiUrl = discovered; } @@ -5316,7 +5325,6 @@ export class CopilotAgent extends Disposable implements IAgent { // MCP reconcile has no host call of its own, so read the retained host snapshot lazily. hostCustomizations: () => this._retainedHostCustomizations(sessionUri), serverToolHost: this._serverToolHost, - isLaunchTokenCurrent: () => this._githubToken === launchPlan.githubToken, onTurnEnded: () => this._onChatTurnEnded(), }, ); @@ -5496,7 +5504,7 @@ export class CopilotAgent extends Disposable implements IAgent { disabledRootMcpServers: await this._disabledRootMcpServers(sessionUri, sessionId, snapshot), activeClientToolSet: activeClient.toolSet, shellManager, - githubToken: this._githubToken, + githubCredentials: this._githubCredentials.forSession(), workspaceless: storedMetadata.workspaceless, fallback: { model: storedMetadata.model, diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index aa040dd4c92a..7e7047829c3d 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -481,8 +481,6 @@ export interface ICopilotAgentSessionOptions { * the future) and exposes SDK tool handlers that execute them in-process. */ readonly serverToolHost?: IAgentServerToolHost; - /** Returns whether the token that launched this session is still the active account token. */ - readonly isLaunchTokenCurrent?: () => boolean; /** Overrides source-launch detection for deterministic tests. */ readonly enableDevelopmentErrorInjection?: boolean; @@ -932,6 +930,7 @@ export class CopilotAgentSession extends Disposable { * non-destructive idle release to avoid disconnecting mid-turn. */ get hasActiveTurn(): boolean { return this._currentTurn.value !== undefined; } + get usesStaticGitHubToken(): boolean { return this._launchPlan.githubCredentials.usesStaticToken; } get chatUri(): URI { return this._chatChannelUri; } get currentTurnId(): string | undefined { return this._currentTurn.value?.id; } @@ -1094,7 +1093,6 @@ export class CopilotAgentSession extends Disposable { private readonly _shellInitScriptInstanceId = generateUuid().substring(0, 8); private readonly _launchPlan: CopilotSessionLaunchPlan; private _detectInterruptedTurnOnRestore: boolean; - private readonly _isLaunchTokenStillCurrent: () => boolean; /** Notifies the agent that this chat's turn ended. See {@link ICopilotAgentSessionOptions.onTurnEnded}. */ private readonly _onTurnEnded: () => void; private readonly _shellManager: ShellManager | undefined; @@ -1150,6 +1148,7 @@ export class CopilotAgentSession extends Disposable { private readonly _repoInfoTelemetry: AgentHostRepoInfoTelemetry; private _activeRepoInfoTurn: { readonly telemetryMessageId: string; + readonly githubToken: string | undefined; cancelled: boolean; begin: Promise<{ readonly context: IAgentHostRestrictedTelemetryContext; readonly baseBranch: string | undefined } | undefined>; } | undefined; @@ -1182,7 +1181,6 @@ export class CopilotAgentSession extends Disposable { this._sessionLauncher = options.sessionLauncher; this._launchPlan = options.launchPlan; this._detectInterruptedTurnOnRestore = options.launchPlan.kind === 'resume'; - this._isLaunchTokenStillCurrent = options.isLaunchTokenCurrent ?? (() => true); this._onTurnEnded = options.onTurnEnded ?? (() => { }); this._shellManager = options.shellManager; this._nonPtyShellTerminals = this._register(this._instantiationService.createInstance(NonPtyShellTerminalStreams, options.sessionUri, options.chatChannelUri)); @@ -2260,9 +2258,13 @@ export class CopilotAgentSession extends Disposable { /** Updates the GitHub credentials used by this live SDK session. */ async updateGitHubCredentials(host: string, token: string): Promise { - return this._wrapper.session.rpc.gitHubAuth.setCredentials({ + const result = await this._wrapper.session.rpc.gitHubAuth.setCredentials({ credentials: { type: 'token', host, token }, }); + if (result.success) { + this._launchPlan.githubCredentials.updateStaticToken(token); + } + return result; } private _setPromptCacheState(promptCache: ISessionPromptCacheState | undefined): void { @@ -2383,7 +2385,7 @@ export class CopilotAgentSession extends Disposable { } private async _initialGitHubMcpToken(request: McpAuthRequest): Promise { - const githubToken = this._launchPlan.githubToken; + const githubToken = this._currentGitHubToken; const requestUrl = normalizeMcpServerUrl(request.serverUrl); if (!githubToken || requestUrl === undefined) { return undefined; @@ -4705,10 +4707,10 @@ export class CopilotAgentSession extends Disposable { } } - private async _beginRepoInfoTelemetry(telemetryMessageId: string, clientType: AgentHostClientType, isCurrent: () => boolean): Promise<{ readonly context: IAgentHostRestrictedTelemetryContext; readonly baseBranch: string | undefined } | undefined> { + private async _beginRepoInfoTelemetry(telemetryMessageId: string, clientType: AgentHostClientType, githubToken: string | undefined, isCurrent: () => boolean): Promise<{ readonly context: IAgentHostRestrictedTelemetryContext; readonly baseBranch: string | undefined } | undefined> { let resolved: { readonly context: IAgentHostRestrictedTelemetryContext; readonly baseBranch: string | undefined } | undefined; try { - resolved = await this._resolveRepoInfoTelemetryContext(); + resolved = await this._resolveRepoInfoTelemetryContext(githubToken); } catch (error) { this._logService.warn(`[Copilot:${this.sessionId}] Failed to resolve repository info telemetry context: ${getErrorMessage(error)}`); return undefined; @@ -4733,10 +4735,18 @@ export class CopilotAgentSession extends Disposable { return; } this._activeRepoInfoTurn = undefined; - const isCurrent = () => !turn.cancelled && this._isLaunchTokenCurrent(); + const isCurrent = () => !turn.cancelled && turn.githubToken !== undefined && this._isGitHubTokenCurrent(turn.githubToken); void turn.begin.then(resolved => this._endRepoInfoTelemetry(turn.telemetryMessageId, resolved, isCurrent)); } + private _isGitHubTokenCurrent(token: string): boolean { + return this._launchPlan.githubCredentials.isCurrentToken(token); + } + + private get _currentGitHubToken(): string | undefined { + return this._launchPlan.githubCredentials.token; + } + private _cancelActiveRepoInfoTelemetry(): void { const turn = this._activeRepoInfoTurn; if (!turn) { @@ -4747,11 +4757,10 @@ export class CopilotAgentSession extends Disposable { void turn.begin.finally(() => this._repoInfoTelemetry.clearTurn(turn.telemetryMessageId)); } - private async _resolveRepoInfoTelemetryContext(): Promise<{ readonly context: IAgentHostRestrictedTelemetryContext; readonly baseBranch: string | undefined } | undefined> { + private async _resolveRepoInfoTelemetryContext(githubToken: string | undefined): Promise<{ readonly context: IAgentHostRestrictedTelemetryContext; readonly baseBranch: string | undefined } | undefined> { if (this._configurationService.getRootValue(platformRootSchema, AgentHostDisableRepoInfoTelemetryConfigKey) === true) { return undefined; } - const githubToken = this._launchPlan.githubToken; if (!githubToken) { return undefined; } @@ -4765,10 +4774,6 @@ export class CopilotAgentSession extends Disposable { return { context: this._toRepoInfoTelemetryContext(rawContext), baseBranch }; } - private _isLaunchTokenCurrent(): boolean { - return this._launchPlan.githubToken !== undefined && this._isLaunchTokenStillCurrent(); - } - private _toRepoInfoTelemetryContext(context: IRestrictedTelemetryContext): IAgentHostRestrictedTelemetryContext { return { restrictedTelemetryEnabled: context.restrictedTelemetryEnabled, @@ -6535,11 +6540,12 @@ export class CopilotAgentSession extends Disposable { this._cancelActiveRepoInfoTelemetry(); const turn: NonNullable = { telemetryMessageId, + githubToken: this._currentGitHubToken, cancelled: false, begin: Promise.resolve(undefined), }; - const isCurrent = () => !turn.cancelled && this._isLaunchTokenCurrent(); - turn.begin = this._beginRepoInfoTelemetry(telemetryMessageId, this._currentTurn.value?.clientType ?? AgentHostClientType.Unknown, isCurrent); + const isCurrent = () => !turn.cancelled && turn.githubToken !== undefined && this._isGitHubTokenCurrent(turn.githubToken); + turn.begin = this._beginRepoInfoTelemetry(telemetryMessageId, this._currentTurn.value?.clientType ?? AgentHostClientType.Unknown, turn.githubToken, isCurrent); this._activeRepoInfoTurn = turn; } })); diff --git a/src/vs/platform/agentHost/node/copilot/copilotGitHubCredentials.ts b/src/vs/platform/agentHost/node/copilot/copilotGitHubCredentials.ts new file mode 100644 index 000000000000..1e6ce8a83af1 --- /dev/null +++ b/src/vs/platform/agentHost/node/copilot/copilotGitHubCredentials.ts @@ -0,0 +1,145 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import type { GitHubTokenProvider } from '@github/copilot-sdk'; +import { DeferredPromise, timeout } from '../../../../base/common/async.js'; +import { getExpirationTime, getRemainingTimeInSeconds } from '../../../../base/common/date.js'; +import { Emitter, Event } from '../../../../base/common/event.js'; +import { Disposable } from '../../../../base/common/lifecycle.js'; + +const COPILOT_GITHUB_TOKEN_REFRESH_THRESHOLD_SECONDS = 60 * 60; +const COPILOT_GITHUB_TOKEN_REFRESH_TIMEOUT_MS = 30_000; + +type CopilotGitHubSdkSessionOptions = + | { readonly gitHubToken: string | undefined; readonly gitHubTokenProvider?: never } + | { readonly gitHubToken?: never; readonly gitHubTokenProvider: GitHubTokenProvider }; + +/** + * The GitHub credential configuration captured when an SDK session launches. + */ +export class CopilotGitHubSessionCredentials { + private constructor( + private _staticToken: string | undefined, + private readonly _provider: CopilotGitHubCredentials | undefined, + ) { } + + static fromToken(token: string | undefined): CopilotGitHubSessionCredentials { + return new CopilotGitHubSessionCredentials(token, undefined); + } + + static fromProvider(provider: CopilotGitHubCredentials): CopilotGitHubSessionCredentials { + return new CopilotGitHubSessionCredentials(undefined, provider); + } + + get usesStaticToken(): boolean { + return this._provider === undefined; + } + + get token(): string | undefined { + return this._provider?.token ?? this._staticToken; + } + + get sdkSessionOptions(): CopilotGitHubSdkSessionOptions { + return this._provider + ? { gitHubTokenProvider: this._provider.tokenProvider } + : { gitHubToken: this._staticToken }; + } + + isCurrentToken(token: string): boolean { + return this.token === token; + } + + updateStaticToken(token: string): void { + if (!this.usesStaticToken) { + throw new Error('Cannot update provider-backed GitHub credentials as a static token'); + } + this._staticToken = token; + } +} + +/** + * Owns the current GitHub credential mode and supplies refreshable credentials to SDK sessions. + */ +export class CopilotGitHubCredentials extends Disposable { + private _token: string | undefined; + private _expiresAt: number | undefined; + private _usesTokenProvider = false; + private _pendingRefresh: DeferredPromise | undefined; + private _isShutdown = false; + + constructor( + private readonly _now = Date.now, + private readonly _refreshTimeoutMs = COPILOT_GITHUB_TOKEN_REFRESH_TIMEOUT_MS, + ) { + super(); + } + + private readonly _onDidRequestRefresh = this._register(new Emitter()); + readonly onDidRequestRefresh: Event = this._onDidRequestRefresh.event; + + get token(): string | undefined { + return this._token; + } + + readonly tokenProvider: GitHubTokenProvider = async ({ reason }) => { + let expiresIn = getRemainingTimeInSeconds(this._expiresAt, this._now()); + if (!this._isShutdown && this._usesTokenProvider && (reason === 'refresh' || expiresIn === undefined || expiresIn <= COPILOT_GITHUB_TOKEN_REFRESH_THRESHOLD_SECONDS)) { + await this._requestRefresh(); + expiresIn = getRemainingTimeInSeconds(this._expiresAt, this._now()); + } + if (this._isShutdown || !this._usesTokenProvider || !this._token || expiresIn === undefined || expiresIn <= COPILOT_GITHUB_TOKEN_REFRESH_THRESHOLD_SECONDS) { + return { kind: 'cancelled' }; + } + return { kind: 'token', accessToken: this._token, expiresIn }; + }; + + forSession(): CopilotGitHubSessionCredentials { + return this._usesTokenProvider + ? CopilotGitHubSessionCredentials.fromProvider(this) + : CopilotGitHubSessionCredentials.fromToken(this._token); + } + + update(token: string | undefined, expiresIn: number | undefined): { readonly tokenChanged: boolean; readonly modeChanged: boolean } { + const tokenChanged = this._token !== token; + const usesTokenProvider = token !== undefined && expiresIn !== undefined; + const modeChanged = this._usesTokenProvider !== usesTokenProvider; + this._token = token; + this._expiresAt = usesTokenProvider ? getExpirationTime(expiresIn, this._now()) : undefined; + this._usesTokenProvider = usesTokenProvider; + this._completePendingRefresh(); + return { tokenChanged, modeChanged }; + } + + shutdown(): void { + this._isShutdown = true; + this.update(undefined, undefined); + } + + private async _requestRefresh(): Promise { + let pending = this._pendingRefresh; + if (!pending) { + pending = new DeferredPromise(); + this._pendingRefresh = pending; + this._onDidRequestRefresh.fire(); + } + const refreshTimeout = timeout(this._refreshTimeoutMs); + try { + await Promise.race([pending.p, refreshTimeout]); + } finally { + refreshTimeout.cancel(); + } + } + + private _completePendingRefresh(): void { + const pending = this._pendingRefresh; + this._pendingRefresh = undefined; + pending?.complete(); + } + + override dispose(): void { + this.shutdown(); + super.dispose(); + } +} diff --git a/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts b/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts index 97a48a9e125f..52e2687145e4 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts @@ -30,6 +30,7 @@ import { IAgentHostSessionOpenTelemetry } from '../agentHostSessionOpenTelemetry import { IByokLmBridgeRegistry } from '../byokLmBridgeRegistry.js'; import { IByokLmProxyService, type IByokLmProxyHandle } from './byokLmProxyService.js'; import type { ICopilotMcpServerInfo, ICopilotPluginInfo } from './copilotAgent.js'; +import { CopilotGitHubSessionCredentials } from './copilotGitHubCredentials.js'; import { toSdkHooks, toSdkInstructionDirectories, toSdkMcpServers, toSdkMcpServersFromConfigMap, toSdkSessionCustomAgents, toSdkSkillDirectories } from './copilotPluginConverters.js'; import { CopilotSessionWrapper } from './copilotSessionWrapper.js'; import { ShellManager, createShellTools, type IUnsandboxedCommandConfirmationRequest } from './copilotShellTools.js'; @@ -252,7 +253,7 @@ interface ICopilotSessionLaunchBase { */ readonly activeClientToolSet: ActiveClientToolSet; readonly shellManager: ShellManager | undefined; - readonly githubToken: string | undefined; + readonly githubCredentials: CopilotGitHubSessionCredentials; /** * Whether this is a workspace-less session. Threaded into the @@ -1024,14 +1025,7 @@ export class CopilotSessionLauncher implements ICopilotSessionLauncher { pluginDirectories: coalesce(plugins.map(p => p.pluginDir)) .filter(d => d.scheme === Schemas.file).map(d => d.fsPath), tools: promptOverrides.tools, - // Pass the GitHub token at the session level. The SDK's - // client-level `gitHubToken` authenticates the CLI process, - // but each session also needs its own token resolved into a - // GitHub identity (login, Copilot plan, endpoints) to drive - // model routing and quota — without this the session - // errors with "Session was not created with authentication - // info or custom provider" on first send. See #318693. - gitHubToken: plan.githubToken, + ...plan.githubCredentials.sdkSessionOptions, // Enable infinite sessions so the SDK provisions a workspace // directory (containing `plan.md`, `checkpoints/`, `files/`). // The workspace is required for plan mode to work — without diff --git a/src/vs/platform/agentHost/test/electron-browser/agentHostProtocolClient.test.ts b/src/vs/platform/agentHost/test/electron-browser/agentHostProtocolClient.test.ts index 19fa407d8180..14669157ffd2 100644 --- a/src/vs/platform/agentHost/test/electron-browser/agentHostProtocolClient.test.ts +++ b/src/vs/platform/agentHost/test/electron-browser/agentHostProtocolClient.test.ts @@ -2915,7 +2915,7 @@ suite('AgentHostProtocolClient', () => { jsonrpc: '2.0', id: initialAnnotationsSubscribe.id, result: { snapshot: { resource: annotationsUri.toString(), state: { annotations: [] }, fromSeq: 5 } }, }); - const authentication = client.authenticate({ resource: 'https://api.github.com', token: 'token' }); + const authentication = client.authenticate({ resource: 'https://api.github.com', token: 'token', expiresIn: 3600 }); const initialAuthenticate = await waitForRequest(transports[0], 'authenticate'); transports[0].fireMessage({ jsonrpc: '2.0', id: initialAuthenticate.id, result: {} }); await authentication; @@ -2961,6 +2961,8 @@ suite('AgentHostProtocolClient', () => { }); const restoredAuthenticate = await waitForRequestAt(reconnectTransport, 'authenticate', 0); + const restoredExpiresIn = (restoredAuthenticate.params as { expiresIn?: number }).expiresIn; + assert.ok(restoredExpiresIn !== undefined && restoredExpiresIn > 0 && restoredExpiresIn <= 3600); const managedSettings = reconnectTransport.sentMessages.find(message => hasKey(message, { method: true }) && message.method === 'setClientManagedSettingsPermissions'); assert.ok(managedSettings, 'managed settings should be restored after fresh initialization'); assert.ok( diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 89cb67b5aee9..b2ac738d993a 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -7178,10 +7178,10 @@ suite('AgentService (node dispatcher)', () => { test('routes token to provider matching the resource', async () => { registerTestAgentProvider(service, copilotAgent); - const result = await service.authenticate({ resource: 'https://api.github.com', token: 'ghp_test123' }); + const result = await service.authenticate({ resource: 'https://api.github.com', token: 'ghp_test123', expiresIn: 3600 }); assert.deepStrictEqual(result, { authenticated: true }); - assert.deepStrictEqual(copilotAgent.authenticateCalls, [{ resource: 'https://api.github.com', token: 'ghp_test123' }]); + assert.deepStrictEqual(copilotAgent.authenticateCalls, [{ resource: 'https://api.github.com', token: 'ghp_test123', expiresIn: 3600 }]); }); test('returns not authenticated for unknown resource', async () => { diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index 05a1ab1b6480..f549b2ef24f9 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -49,7 +49,7 @@ import { AgentHostClientConnectionKind, AgentHostLaunchKind, AgentHostTransportK import { ISessionDataService } from '../../common/sessionDataService.js'; import { buildDefaultChatUri, buildChatUri, buildSubagentChatUri, buildSubagentSessionUri, parseRequiredSessionUriFromChatUri, CustomizationLoadStatus, MessageKind, readSessionEhcliAdoptable, ResponsePartKind, ROOT_STATE_URI, ToolResultContentType, TurnState, customizationId, AH_META_IS_ARCHIVED_DB_KEY, AH_META_IS_READ_DB_KEY, type ClientPluginCustomization, type Customization, type PluginCustomization, type ToolCallResult, type Turn, RuleCustomization } from '../../common/state/sessionState.js'; import { ChatOriginKind, CustomizationEnablementKind, CustomizationType, SessionStatus, ToolCallContributorKind, type AgentSelection, type ModelSelection, type ProtectedResourceMetadata, type ToolDefinition } from '../../common/state/protocol/state.js'; -import { ActionType, type ChatAction, type SessionAction } from '../../common/state/sessionActions.js'; +import { ActionType, AuthRequiredReason, type ChatAction, type SessionAction } from '../../common/state/sessionActions.js'; import { AgentConfigurationService, IAgentConfigurationService } from '../../node/agentConfigurationService.js'; import { AgentHostAuthenticationService, IAgentHostAuthenticationService } from '../../node/agentHostAuthenticationService.js'; @@ -63,6 +63,7 @@ import { IAgentHostTerminalManager } from '../../node/agentHostTerminalManager.j import { IAgentHostOTelService } from '../../common/otel/agentHostOTelService.js'; import { AgentHostCompletions, IAgentHostCompletions } from '../../node/agentHostCompletions.js'; import { COPILOT_AGENT_HOST_SYSTEM_MESSAGE, CopilotAgent, getCopilotManagedSettingsDiagnostics, rebaseUnder, REFRESH_DEBOUNCE_MS, resolveCopilotOtlpMetricsEndpoint } from '../../node/copilot/copilotAgent.js'; +import { CopilotGitHubSessionCredentials } from '../../node/copilot/copilotGitHubCredentials.js'; import { GITHUB_MCP_SERVER_NAME } from '../../node/shared/githubMcpServer.js'; import { AGENT_HOST_FILE_LINK_INSTRUCTIONS } from '../../node/shared/fileLinkInstructions.js'; import { COPILOT_AGENT_HOST_LARGE_OUTPUT_TOOL_INSTRUCTION } from '../../node/copilot/prompts/toolInstructions.js'; @@ -200,11 +201,13 @@ function setDefaultSessionStub(agent: CopilotAgent, sessionId: string, stub: unk sessionUri?: URI; resourceUri?: URI; chatChannelUri?: URI; + usesStaticGitHubToken?: boolean; bindChatChannel?: (uri: URI) => void; destroySession?: () => Promise; }; typed.sessionId ??= sessionId; typed.sessionUri ??= sessionUri; + typed.usesStaticGitHubToken ??= true; // A session-backed (default) chat's host-chosen persistence scope is the // session itself; that is how the agent identifies it without rebuilding a // default-chat URI (see `CopilotAgent._findSessionChat`). @@ -767,6 +770,7 @@ interface IFakeAgentSession { interface ICredentialUpdateSession { readonly hasActiveTurn: boolean; + readonly usesStaticGitHubToken?: boolean; updateGitHubCredentials(host: string, token: string): Promise<{ readonly success: boolean; readonly copilotUserResolved?: boolean }>; dispose(): void; } @@ -777,6 +781,7 @@ class MockCopilotSession { readonly workingDirectoryCalls: string[] = []; readonly workingDirectoryErrors: Array = []; readonly workingDirectoryResults: string[] = []; + readonly gitHubCredentialUpdates: Array<{ credentials: { type: 'token'; host: string; token: string } }> = []; readonly rpc = { eventLog: { registerInterest: async () => ({ handle: 'sampling-interest' }), @@ -786,7 +791,10 @@ class MockCopilotSession { update: async () => ({ success: true }), }, gitHubAuth: { - setCredentials: async () => ({ success: true, copilotUserResolved: true }), + setCredentials: async (params: { credentials: { type: 'token'; host: string; token: string } }) => { + this.gitHubCredentialUpdates.push(params); + return { success: true, copilotUserResolved: true }; + }, }, permissions: { setMode: async ({ mode }: { mode: PermissionMode }) => ({ success: true, mode }), @@ -1196,7 +1204,7 @@ function createAgentSessionThroughAgent(agent: CopilotAgent, instantiationServic resolvedAgentName: undefined, snapshot: options?.snapshot ?? { tools: [], plugins: [], mcpServers: {} }, shellManager, - githubToken: 'token', + githubCredentials: CopilotGitHubSessionCredentials.fromToken('token'), model: undefined, }; return { session: agentInternals._createAgentSession(launchPlan, options?.workingDirectory, activeClient), activeClient, createOptions: () => createOptions }; @@ -2618,7 +2626,7 @@ suite('CopilotAgent', () => { await waitForState(agent.models, models => models.length === 0); assert.deepStrictEqual({ - githubToken: agent['_githubToken'], + githubToken: agent['_githubCredentials'].token, models: agent.models.get(), }, { githubToken: undefined, @@ -2658,7 +2666,7 @@ suite('CopilotAgent', () => { } }); - test('updates every live session after a changed auth token without restarting an unchanged proxy', async () => { + test('updates live sessions and restarts only when the credential mode changes', async () => { const client = new TestCopilotClient([], [{ id: 'gpt-4o', name: 'GPT-4o', @@ -2666,6 +2674,7 @@ suite('CopilotAgent', () => { const agent = createTestAgent(disposables, { copilotClient: client }); const first = { hasActiveTurn: false, + usesStaticGitHubToken: true, updates: [] as Array<{ host: string; token: string }>, async updateGitHubCredentials(host: string, token: string) { this.updates.push({ host, token }); @@ -2675,6 +2684,7 @@ suite('CopilotAgent', () => { } satisfies ICredentialUpdateSession & { updates: Array<{ host: string; token: string }> }; const second = { hasActiveTurn: false, + usesStaticGitHubToken: true, updates: [] as Array<{ host: string; token: string }>, async updateGitHubCredentials(host: string, token: string) { this.updates.push({ host, token }); @@ -2688,15 +2698,20 @@ suite('CopilotAgent', () => { setDefaultSessionStub(agent, 'second', second); await agent.authenticate('https://api.github.com', 'model-token-a'); await agent.authenticate('https://api.github.com', 'model-token-a'); + await agent.authenticate('https://api.github.com', 'model-token-a', 7200); assert.deepStrictEqual({ firstUpdates: first.updates, secondUpdates: second.updates, stops: client.stopCallCount, }, { - firstUpdates: [{ host: 'https://github.com', token: 'model-token-a' }], - secondUpdates: [{ host: 'https://github.com', token: 'model-token-a' }], - stops: 0, + firstUpdates: [ + { host: 'https://github.com', token: 'model-token-a' }, + ], + secondUpdates: [ + { host: 'https://github.com', token: 'model-token-a' }, + ], + stops: 1, }); } finally { await disposeAgent(agent); @@ -10194,8 +10209,57 @@ suite('CopilotAgent', () => { assert.deepStrictEqual({ configToken: capturedConfig?.gitHubToken, + hasTokenProvider: capturedConfig?.gitHubTokenProvider !== undefined, }, { configToken: 'gh-token-abc', + hasTokenProvider: false, + }); + } finally { + await disposeAgent(agent); + } + }); + + test('materialization uses refreshable GitHub credentials when the lifetime is known', async () => { + const sessionDataService = disposables.add(new TestSessionDataService()); + const client = new TestCopilotClient([]); + const mockSession = new MockCopilotSession(); + let capturedConfig: Parameters[0] | undefined; + const agent = createTestAgent(disposables, { sessionDataService, copilotClient: client }); + client.createSession = async config => { + capturedConfig = config; + return mockSession as unknown as CopilotSession; + }; + + try { + await agent.authenticate('https://api.github.com', 'initial-token', 3600); + const result = await provisionSession(agent, { + session: AgentSession.uri('copilotcli', 'refreshable-token-session'), + workingDirectories: [URI.file('/workspace')], + }); + await agent.chats.sendMessage(defaultChatUri(result.session), 'hello', undefined, undefined, undefined, undefined, exactChatContext(result.session, defaultChatUri(result.session), result.session)); + + const provider = capturedConfig?.gitHubTokenProvider; + assert.ok(provider); + const refresh = provider({ host: 'github.com', sessionId: 'refreshable-token-session', reason: 'refresh' }); + const authenticationRequired = agent.authenticationRequired.get(); + await agent.authenticate('https://api.github.com', 'refreshed-token', 7200); + await refresh; + + assert.deepStrictEqual({ + gitHubToken: capturedConfig?.gitHubToken, + hasTokenProvider: capturedConfig?.gitHubTokenProvider !== undefined, + authenticationRequired, + credentialUpdates: mockSession.gitHubCredentialUpdates, + clientStops: client.stopCallCount, + }, { + gitHubToken: undefined, + hasTokenProvider: true, + authenticationRequired: { + resource: GITHUB_COPILOT_PROTECTED_RESOURCE, + reason: AuthRequiredReason.Expired, + }, + credentialUpdates: [], + clientStops: 0, }); } finally { await disposeAgent(agent); diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index 84ec7b614163..fd824962673a 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -45,6 +45,7 @@ import { toHostSnapshotAttachmentMeta } from '../../common/meta/agentSnapshotAtt import { STREAMING_TOOL_DISPLAY_INTERVAL_MS } from '../../common/streamingToolCallDisplay.js'; import { CustomizationEnablementKind, CustomizationType, McpAuthRequiredReason, McpServerStatus, type Customization, type McpServerCustomization } from '../../common/state/protocol/channels-session/state.js'; import { CopilotAgentSession, type ICopilotWorkingDirectoryChangeTransaction } from '../../node/copilot/copilotAgentSession.js'; +import { CopilotGitHubCredentials, CopilotGitHubSessionCredentials } from '../../node/copilot/copilotGitHubCredentials.js'; import { buildNonPtyShellTerminalUri } from '../../node/copilot/copilotNonPtyShellTerminals.js'; import { ShellManager } from '../../node/copilot/copilotShellTools.js'; import { buildMcpChannel } from '../../node/shared/mcpCustomizationController.js'; @@ -807,12 +808,12 @@ async function createAgentSession(disposables: DisposableStore, options?: { /** Platform used to compute the SDK sandbox policy. Defaults to `'linux'` so sandbox tests are deterministic. */ platform?: NodeJS.Platform; githubToken?: string; + githubCredentials?: CopilotGitHubSessionCredentials; copilotApiEndpoint?: string; gitService?: IAgentHostGitService; gitHubEndpointService?: IAgentHostGitHubEndpointService; restrictedTelemetryContext?: IRestrictedTelemetryContext; restrictedTelemetryContextError?: Error; - isLaunchTokenCurrent?: () => boolean; onTurnEnded?: () => void; modelId?: string; enableDevelopmentErrorInjection?: boolean; @@ -879,7 +880,7 @@ async function createAgentSession(disposables: DisposableStore, options?: { resolvedAgentName: undefined, snapshot: options?.clientSnapshot ?? { tools: [], plugins: [], mcpServers: {} }, shellManager: options?.shellManager, - githubToken: options?.githubToken, + githubCredentials: options?.githubCredentials ?? CopilotGitHubSessionCredentials.fromToken(options?.githubToken), isEphemeral: options?.isEphemeral, hasScopedEditSurface: options?.hasScopedEditSurface, }; @@ -1104,7 +1105,6 @@ async function createAgentSession(disposables: DisposableStore, options?: { customizationDirectory: options?.customizationDirectory, serverToolHost: options?.serverToolHost, platform: options?.platform ?? 'linux', - isLaunchTokenCurrent: options?.isLaunchTokenCurrent, onTurnEnded: options?.onTurnEnded, enableDevelopmentErrorInjection: options?.enableDevelopmentErrorInjection ?? true, realpath: options?.realpath, @@ -13205,9 +13205,10 @@ Use the attached image as context. }); test('drops an in-flight capture when the launch token is no longer current', async () => { - let tokenCurrent = true; const workingDirectory = URI.file('/repo'); const telemetryService = new CapturingRestrictedTelemetryService(); + const githubCredentials = disposables.add(new CopilotGitHubCredentials()); + githubCredentials.update('github-token', 7200); const gitService: IAgentHostGitService = { ...createNoopGitService(), getRepositoryRoot: async () => workingDirectory, @@ -13222,8 +13223,7 @@ Use the attached image as context. workingDirectory, gitService, telemetryService, - githubToken: 'github-token', - isLaunchTokenCurrent: () => tokenCurrent, + githubCredentials: githubCredentials.forSession(), restrictedTelemetryContext: { restrictedTelemetryEnabled: true, trackingId: 'tracking-id', @@ -13235,7 +13235,7 @@ Use the attached image as context. }, }); mockSession.fire('assistant.turn_start', { turnId: 'root-turn' }); - tokenCurrent = false; + githubCredentials.update('replacement-token', 7200); await timeout(0); assert.deepStrictEqual(telemetryService.events.filter(event => event.eventName === 'request.repoInfo'), []); diff --git a/src/vs/platform/agentHost/test/node/copilotGitHubCredentials.test.ts b/src/vs/platform/agentHost/test/node/copilotGitHubCredentials.test.ts new file mode 100644 index 000000000000..d246dc59bd99 --- /dev/null +++ b/src/vs/platform/agentHost/test/node/copilotGitHubCredentials.test.ts @@ -0,0 +1,93 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { CopilotGitHubCredentials } from '../../node/copilot/copilotGitHubCredentials.js'; + +suite('CopilotGitHubCredentials', () => { + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + test('coalesces refreshes and refreshes short-lived tokens before returning them', async () => { + let now = 1_000_000; + const credentials = disposables.add(new CopilotGitHubCredentials(() => now)); + credentials.update('initial-token', 7200); + let refreshRequests = 0; + disposables.add(credentials.onDidRequestRefresh(() => refreshRequests++)); + + const firstRefresh = credentials.tokenProvider({ host: 'github.com', sessionId: 'session', reason: 'refresh' }); + const secondRefresh = credentials.tokenProvider({ host: 'github.com', sessionId: 'session', reason: 'refresh' }); + assert.strictEqual(refreshRequests, 1); + + now += 1000; + credentials.update('refreshed-token', 7200); + const refreshedTokens = await Promise.all([firstRefresh, secondRefresh]); + credentials.update('short-token', 3600); + const preflightRefresh = credentials.tokenProvider({ host: 'github.com', sessionId: 'session', reason: 'initial' }); + assert.strictEqual(refreshRequests, 2); + credentials.update('preflight-refreshed-token', 7200); + + assert.deepStrictEqual({ + refreshedTokens, + preflightToken: await preflightRefresh, + }, { + refreshedTokens: [ + { kind: 'token', accessToken: 'refreshed-token', expiresIn: 7200 }, + { kind: 'token', accessToken: 'refreshed-token', expiresIn: 7200 }, + ], + preflightToken: { kind: 'token', accessToken: 'preflight-refreshed-token', expiresIn: 7200 }, + }); + }); + + test('cancels a refresh that does not complete in time', async () => { + const credentials = disposables.add(new CopilotGitHubCredentials(() => 1_000_000, 0)); + credentials.update('expiring-token', 3600); + let refreshRequests = 0; + disposables.add(credentials.onDidRequestRefresh(() => refreshRequests++)); + + const token = await credentials.tokenProvider({ host: 'github.com', sessionId: 'session', reason: 'initial' }); + + assert.deepStrictEqual({ refreshRequests, token }, { + refreshRequests: 1, + token: { kind: 'cancelled' }, + }); + }); + + test('captures credential mode for each SDK session', () => { + const credentials = disposables.add(new CopilotGitHubCredentials()); + credentials.update('static-token', undefined); + const staticSession = credentials.forSession(); + const modeChange = credentials.update('provider-token', 7200); + const providerSession = credentials.forSession(); + + staticSession.updateStaticToken('updated-static-token'); + + assert.deepStrictEqual({ + modeChange, + staticSession: { + usesStaticToken: staticSession.usesStaticToken, + token: staticSession.token, + options: staticSession.sdkSessionOptions, + }, + providerSession: { + usesStaticToken: providerSession.usesStaticToken, + token: providerSession.token, + hasProvider: providerSession.sdkSessionOptions.gitHubTokenProvider !== undefined, + }, + }, { + modeChange: { tokenChanged: true, modeChanged: true }, + staticSession: { + usesStaticToken: true, + token: 'updated-static-token', + options: { gitHubToken: 'updated-static-token' }, + }, + providerSession: { + usesStaticToken: false, + token: 'provider-token', + hasProvider: true, + }, + }); + }); +}); diff --git a/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts b/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts index 57e92020cf1e..7fdc6a01416f 100644 --- a/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts @@ -33,6 +33,7 @@ import type { IAgentHostTerminalManager } from '../../node/agentHostTerminalMana import { ByokLmBridgeRegistry, IByokLmBridgeRegistry } from '../../node/byokLmBridgeRegistry.js'; import { ByokLmProxyService, IByokLmProxyService, type IByokLmProxyHandle } from '../../node/copilot/byokLmProxyService.js'; import { resolveCopilotMcpServerInfo, type ICopilotPluginInfo } from '../../node/copilot/copilotAgent.js'; +import { CopilotGitHubSessionCredentials } from '../../node/copilot/copilotGitHubCredentials.js'; import { CopilotSessionLauncher, filterClientToolNames, getCopilotAutoTier, getCopilotReasoningEffort, isCopilotReasoningEffort, resolveByokSessionConfig, normalizeToolFilterPatterns, resolveConfiguredReasoningEffortOverride, resolveCopilotAutoTier, resolveCopilotReasoningEffort, toSdkToolFilterPatterns, type CopilotSessionLaunchPlan, type ICopilotSessionRuntime } from '../../node/copilot/copilotSessionLauncher.js'; import { buildDefaultChatUri } from '../../common/state/sessionState.js'; import type { IAgentHostSessionOpenTelemetry } from '../../node/agentHostSessionOpenTelemetry.js'; @@ -511,7 +512,7 @@ suite('CopilotSessionLauncher shared session config', () => { disabledRootMcpServers: ['github', 'azure'], activeClientToolSet: new ActiveClientToolSet(), shellManager: undefined, - githubToken: undefined, + githubCredentials: CopilotGitHubSessionCredentials.fromToken(undefined), }; const createPlan: CopilotSessionLaunchPlan = { ...basePlan, @@ -678,7 +679,7 @@ suite('CopilotSessionLauncher resume fallback', () => { snapshot: { tools: [], plugins: [], mcpServers: {} }, activeClientToolSet: new ActiveClientToolSet(), shellManager: undefined, - githubToken: undefined, + githubCredentials: CopilotGitHubSessionCredentials.fromToken(undefined), kind: 'resume', fallback: { model: undefined }, }, @@ -902,7 +903,7 @@ suite('CopilotSessionLauncher GPT-5.6 customizations', () => { snapshot: { tools: [], plugins: [], mcpServers: {} }, activeClientToolSet: new ActiveClientToolSet(), shellManager: undefined, - githubToken: undefined, + githubCredentials: CopilotGitHubSessionCredentials.fromToken(undefined), model: { id: 'claude-sonnet-4.5', config: {} }, }; @@ -933,7 +934,7 @@ suite('CopilotSessionLauncher GPT-5.6 customizations', () => { snapshot: { tools: [], plugins: [], mcpServers: {} }, activeClientToolSet: new ActiveClientToolSet(), shellManager: undefined, - githubToken: undefined, + githubCredentials: CopilotGitHubSessionCredentials.fromToken(undefined), model: { id: 'claude-sonnet-4.5', config: {} }, }; @@ -963,7 +964,7 @@ suite('CopilotSessionLauncher GPT-5.6 customizations', () => { snapshot: { tools: [], plugins: [], mcpServers: {} }, activeClientToolSet: new ActiveClientToolSet(), shellManager: undefined, - githubToken: undefined, + githubCredentials: CopilotGitHubSessionCredentials.fromToken(undefined), model: { id: 'claude-sonnet-4.5', config: {} }, }; @@ -993,7 +994,7 @@ suite('CopilotSessionLauncher GPT-5.6 customizations', () => { snapshot: { tools: [], plugins: [], mcpServers: {} }, activeClientToolSet: new ActiveClientToolSet(), shellManager: undefined, - githubToken: undefined, + githubCredentials: CopilotGitHubSessionCredentials.fromToken(undefined), model: { id: 'claude-sonnet-4.5', config: {} }, }; @@ -1020,7 +1021,7 @@ suite('CopilotSessionLauncher GPT-5.6 customizations', () => { snapshot: { tools: [], plugins: [], mcpServers: {} }, activeClientToolSet: new ActiveClientToolSet(), shellManager: undefined, - githubToken: undefined, + githubCredentials: CopilotGitHubSessionCredentials.fromToken(undefined), fallback: { model: { id: 'gpt-5.6-sol', config: {} } }, }; @@ -1333,7 +1334,7 @@ suite('CopilotSessionLauncher resume config', () => { snapshot, activeClientToolSet: new ActiveClientToolSet(), shellManager: undefined, - githubToken: 'token', + githubCredentials: CopilotGitHubSessionCredentials.fromToken('token'), fallback: { model }, }; const runtime = { createClientSdkTools, createServerSdkTools: () => [] }; @@ -1565,7 +1566,7 @@ suite('CopilotSessionLauncher auto tier', () => { snapshot: { tools: [], plugins: [], mcpServers: {} }, activeClientToolSet: new ActiveClientToolSet(), shellManager: undefined, - githubToken: undefined, + githubCredentials: CopilotGitHubSessionCredentials.fromToken(undefined), }; const plan: CopilotSessionLaunchPlan = kind === 'create' ? { ...base, kind: 'create', model } diff --git a/src/vs/platform/agentHost/test/node/mockAgent.ts b/src/vs/platform/agentHost/test/node/mockAgent.ts index fe321991e33c..e7f189b5c525 100644 --- a/src/vs/platform/agentHost/test/node/mockAgent.ts +++ b/src/vs/platform/agentHost/test/node/mockAgent.ts @@ -85,7 +85,7 @@ export class MockAgent implements IAgent { readonly respondToPermissionCalls: { requestId: string; approved: boolean }[] = []; readonly changeModelCalls: { session: URI; model: ModelSelection; chat?: URI }[] = []; readonly changeAgentCalls: { session: URI; agent: AgentSelection | undefined; chat?: URI }[] = []; - readonly authenticateCalls: { resource: string; token: string }[] = []; + readonly authenticateCalls: { resource: string; token: string; expiresIn?: number }[] = []; readonly setClientCustomizationsCalls: { clientId: string; customizations: ClientPluginCustomization[] }[] = []; readonly setClientToolsCalls: { clientId: string; tools: readonly ToolDefinition[] }[] = []; readonly removeActiveClientCalls: { chat: URI; clientId: string }[] = []; @@ -401,8 +401,8 @@ export class MockAgent implements IAgent { async materializeChat(_chat: URI, _context: URI | IAgentChatContext, _providerData: string | undefined): Promise { } - async authenticate(resource: string, token: string): Promise { - this.authenticateCalls.push({ resource, token }); + async authenticate(resource: string, token: string, expiresIn?: number): Promise { + this.authenticateCalls.push({ resource, token, ...(expiresIn === undefined ? {} : { expiresIn }) }); return true; } diff --git a/src/vs/platform/extensions/common/extensionsApiProposals.ts b/src/vs/platform/extensions/common/extensionsApiProposals.ts index bfc17bad459a..77ae2c267cee 100644 --- a/src/vs/platform/extensions/common/extensionsApiProposals.ts +++ b/src/vs/platform/extensions/common/extensionsApiProposals.ts @@ -48,6 +48,9 @@ const _allApiProposals = { authSessionAudience: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.authSessionAudience.d.ts', }, + authSessionExpiration: { + proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.authSessionExpiration.d.ts', + }, authenticationChallenges: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.authenticationChallenges.d.ts', }, diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts index 81db903d83b9..837e0001e8df 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts @@ -166,8 +166,8 @@ export class AgentHostAuthTokenCache { } } -type AuthenticationTokenResolution = - | { readonly kind: 'resolved'; readonly token: string } +type AuthenticationSessionResolution = + | { readonly kind: 'resolved'; readonly session: AuthenticationSession } | { readonly kind: 'signedOut' } | { readonly kind: 'unavailable' }; @@ -217,7 +217,7 @@ export class AgentHostAuthenticationRecovery { const commandService = accessor.get(ICommandService); const logService = accessor.get(ILogService); const scopes = resource.scopes_supported ?? []; - const resolution = await resolveAuthenticationTokenForResource( + const resolution = await resolveAuthenticationSessionForResource( URI.parse(resource.resource), resource.authorization_servers ?? [], scopes, @@ -227,23 +227,23 @@ export class AgentHostAuthenticationRecovery { ); throwIfAuthenticationStale(options); if (resolution.kind !== 'resolved') { - logAuthenticationTokenResolution(logService, options.logPrefix, resource.resource, resolution); + logAuthenticationSessionResolution(logService, options.logPrefix, resource.resource, resolution); if (resolution.kind === 'signedOut') { this._resentTokens.delete(key); } return; } - const token = resolution.token; + const session = resolution.session; const previousToken = this._resentTokens.get(key); - if (previousToken !== undefined && previousToken === token) { + if (previousToken !== undefined && previousToken === session.accessToken) { options.authTokenCache?.clear(resource.resource, resource.scopes_supported); throwIfAuthenticationStale(options); - const interactiveToken = await forceAuthenticationInteractively(authenticationService, commandService, logService, resource, options); + const interactiveSession = await forceAuthenticationInteractively(authenticationService, commandService, logService, resource, options); throwIfAuthenticationStale(options); - if (interactiveToken) { - this._resentTokens.set(key, interactiveToken); - if (interactiveToken === token) { + if (interactiveSession) { + this._resentTokens.set(key, interactiveSession.accessToken); + if (interactiveSession.accessToken === session.accessToken) { logService.info(`${options.logPrefix} Interactive authentication completed without a new token for ${resource.resource}`); } } @@ -251,28 +251,28 @@ export class AgentHostAuthenticationRecovery { } options.authTokenCache?.clear(resource.resource, resource.scopes_supported); - if (await forwardAuthenticationToken(options, resource.resource, resource.scopes_supported ?? [], token)) { - this._resentTokens.set(key, token); + if (await forwardAuthenticationToken(options, resource.resource, resource.scopes_supported ?? [], session)) { + this._resentTokens.set(key, session.accessToken); logService.info(`${options.logPrefix} Authenticating for resource: ${resource.resource}`); } } } /** - * Resolves a bearer token for a protected resource by trying each + * Resolves an authentication session for a protected resource by trying each * authorization server in order. First attempts an exact scope match, * then falls back to finding the session whose scopes are the narrowest * superset of the requested scopes. */ -export async function resolveTokenForResource( +export async function resolveSessionForResource( resourceServer: URI, authorizationServers: readonly string[], scopes: readonly string[], authenticationService: IAuthenticationService, logService: ILogService, logPrefix: string, -): Promise { - const resolution = await resolveAuthenticationTokenForResource( +): Promise { + const resolution = await resolveAuthenticationSessionForResource( resourceServer, authorizationServers, scopes, @@ -280,17 +280,17 @@ export async function resolveTokenForResource( logService, logPrefix, ); - return resolution.kind === 'resolved' ? resolution.token : undefined; + return resolution.kind === 'resolved' ? resolution.session : undefined; } -async function resolveAuthenticationTokenForResource( +async function resolveAuthenticationSessionForResource( resourceServer: URI, authorizationServers: readonly string[], scopes: readonly string[], authenticationService: IAuthenticationService, logService: ILogService, logPrefix: string, -): Promise { +): Promise { let hasUnavailableProvider = false; for (const server of authorizationServers) { const serverUri = URI.parse(server); @@ -328,7 +328,7 @@ async function resolveAuthenticationTokenForResource( } const exactSession = sessions[0]; if (exactSession) { - return { kind: 'resolved', token: exactSession.accessToken }; + return { kind: 'resolved', session: exactSession }; } let allSessions: readonly AuthenticationSession[]; @@ -341,7 +341,7 @@ async function resolveAuthenticationTokenForResource( continue; } const requestedSet = new Set(scopes); - let bestToken: string | undefined; + let bestSession: AuthenticationSession | undefined; let bestExtraScopes = Infinity; for (const session of allSessions) { const sessionScopes = new Set(session.scopes); @@ -356,12 +356,12 @@ async function resolveAuthenticationTokenForResource( const extraScopes = sessionScopes.size - requestedSet.size; if (extraScopes < bestExtraScopes) { bestExtraScopes = extraScopes; - bestToken = session.accessToken; + bestSession = session; } } } - if (bestToken) { - return { kind: 'resolved', token: bestToken }; + if (bestSession) { + return { kind: 'resolved', session: bestSession }; } } return hasUnavailableProvider ? { kind: 'unavailable' } : { kind: 'signedOut' }; @@ -372,6 +372,8 @@ export interface IAgentHostAuthenticateRequest { readonly scopes?: readonly string[]; /** An empty token revokes the credential previously forwarded for this resource and scope set. */ readonly token: string; + /** The access token's remaining lifetime in seconds, when known. */ + readonly expiresIn?: number; } export interface IAgentHostAuthenticationOptions { @@ -406,12 +408,19 @@ async function forwardAuthenticationToken( options: Pick, resource: string, scopes: readonly string[] | undefined, - token: string, + session: Pick | undefined, ): Promise { throwIfAuthenticationStale(options); - const request = { resource, scopes, token }; + const token = session?.accessToken ?? ''; + const expiresIn = session?.expiresIn; + const request: IAgentHostAuthenticateRequest = { + resource, + scopes, + token, + ...(expiresIn !== undefined && Number.isInteger(expiresIn) && expiresIn > 0 ? { expiresIn } : {}), + }; if (options.authTokenCache) { - return options.authTokenCache.authenticate(resource, scopes, token, () => options.authenticate(request)); + return options.authTokenCache.authenticate(resource, scopes ?? [], token, () => options.authenticate(request)); } await options.authenticate(request); return true; @@ -457,7 +466,7 @@ export async function authenticateProtectedResourcesWithToken( options: Pick, ): Promise { for (const resource of protectedResources) { - await forwardAuthenticationToken(options, resource.resource, resource.scopes_supported, token); + await forwardAuthenticationToken(options, resource.resource, resource.scopes_supported, { accessToken: token }); } } @@ -504,24 +513,24 @@ export async function revokeAuthenticationForRemovedSessions( } reconciledResources.add(key); - const resolution = await resolveTokenForProtectedResource(authenticationService, logService, resource, options); + const resolution = await resolveSessionForProtectedResource(authenticationService, logService, resource, options); throwIfAuthenticationStale(options); if (resolution.kind === 'unavailable') { - logAuthenticationTokenResolution(logService, options.logPrefix, resource.resource, resolution); + logAuthenticationSessionResolution(logService, options.logPrefix, resource.resource, resolution); continue; } if (resolution.kind === 'resolved') { // Another account still covers this resource; forward it so the host // swaps credentials instead of losing them. Unchanged tokens are // deduped by the cache. - if (await forwardAuthenticationToken(options, resource.resource, scopes, resolution.token)) { + if (await forwardAuthenticationToken(options, resource.resource, scopes, resolution.session)) { logService.info(`${options.logPrefix} Authenticating for resource after session removal: ${resource.resource}`); } continue; } options.authTokenCache?.clear(resource.resource, scopes); - if (await forwardAuthenticationToken(options, resource.resource, scopes, '')) { + if (await forwardAuthenticationToken(options, resource.resource, scopes, undefined)) { logService.info(`${options.logPrefix} Clearing authentication for resource after session removal: ${resource.resource}`); } } @@ -576,14 +585,14 @@ async function authenticateProtectedResourceWithServices( options: IAgentHostAuthenticationOptions, ): Promise { throwIfAuthenticationStale(options); - const resolution = await resolveTokenForProtectedResource(authenticationService, logService, resource, options); + const resolution = await resolveSessionForProtectedResource(authenticationService, logService, resource, options); throwIfAuthenticationStale(options); if (resolution.kind !== 'resolved') { - logAuthenticationTokenResolution(logService, options.logPrefix, resource.resource, resolution); + logAuthenticationSessionResolution(logService, options.logPrefix, resource.resource, resolution); return false; } - const authenticated = await forwardAuthenticationToken(options, resource.resource, resource.scopes_supported ?? [], resolution.token); + const authenticated = await forwardAuthenticationToken(options, resource.resource, resource.scopes_supported ?? [], resolution.session); if (!authenticated) { logService.trace(`${options.logPrefix} Authentication state for ${resource.resource} unchanged; skipping authenticate RPC`); return false; @@ -592,13 +601,13 @@ async function authenticateProtectedResourceWithServices( return true; } -async function resolveTokenForProtectedResource( +async function resolveSessionForProtectedResource( authenticationService: IAuthenticationService, logService: ILogService, resource: ProtectedResourceMetadata, options: Pick, -): Promise { - return resolveAuthenticationTokenForResource( +): Promise { + return resolveAuthenticationSessionForResource( URI.parse(resource.resource), resource.authorization_servers ?? [], resource.scopes_supported ?? [], @@ -608,11 +617,11 @@ async function resolveTokenForProtectedResource( ); } -function logAuthenticationTokenResolution( +function logAuthenticationSessionResolution( logService: ILogService, logPrefix: string, resource: string, - resolution: Exclude, + resolution: Exclude, ): void { if (resolution.kind === 'unavailable') { logService.info(`${logPrefix} Authentication provider is not ready for resource: ${resource}; deferring authentication`); @@ -637,7 +646,7 @@ export async function resolveAuthenticationInteractively( throwIfAuthenticationStale(options); const resourceUri = URI.parse(resource.resource); const scopes = resource.scopes_supported ?? []; - const existingToken = await resolveTokenForResource( + const existingSession = await resolveSessionForResource( resourceUri, resource.authorization_servers ?? [], scopes, @@ -646,8 +655,8 @@ export async function resolveAuthenticationInteractively( options.logPrefix, ); throwIfAuthenticationStale(options); - if (existingToken) { - await forwardAuthenticationToken(options, resource.resource, scopes, existingToken); + if (existingSession) { + await forwardAuthenticationToken(options, resource.resource, scopes, existingSession); logService.info(`${options.logPrefix} Interactive authentication succeeded for ${resource.resource}`); return true; } @@ -664,7 +673,7 @@ async function forceAuthenticationInteractively( logService: ILogService, resource: ProtectedResourceMetadata, options: IAgentHostAuthenticationOptions, -): Promise { +): Promise { throwIfAuthenticationStale(options); const scopes = resource.scopes_supported ?? []; const setupResult = await commandService.executeCommand(CHAT_SETUP_ACTION_ID, undefined, { @@ -681,7 +690,7 @@ async function forceAuthenticationInteractively( if (!setupResult.success) { throw setupResult.error ?? new Error(localize('agentHost.signInFailed', "Failed to sign in to use GitHub Copilot.")); } - const token = await resolveTokenForResource( + const session = await resolveSessionForResource( URI.parse(resource.resource), resource.authorization_servers ?? [], scopes, @@ -690,16 +699,16 @@ async function forceAuthenticationInteractively( options.logPrefix, ); throwIfAuthenticationStale(options); - if (!token) { + if (!session) { logService.info(`${options.logPrefix} Interactive authentication did not provide a token for ${resource.resource}`); return undefined; } options.authTokenCache?.clear(resource.resource, scopes); - if (!await forwardAuthenticationToken(options, resource.resource, scopes, token)) { + if (!await forwardAuthenticationToken(options, resource.resource, scopes, session)) { return undefined; } logService.info(`${options.logPrefix} Interactive authentication completed for ${resource.resource}`); - return token; + return session; } export async function resolveMcpServerAuthentication( @@ -884,7 +893,7 @@ async function authenticateMcpSession( updateAccess: boolean, agentHost: { readonly authority: string; readonly label: string } | undefined, ): Promise { - await forwardAuthenticationToken(options, options.mcpServerUrl, scopes, session.accessToken); + await forwardAuthenticationToken(options, options.mcpServerUrl, scopes, session); if (updateAccess) { authenticationMcpAccessService.updateAllowedMcpServers(providerId, session.account.label, [{ id: options.mcpServerId, name: options.mcpServerName, allowed: true, url: options.mcpServerUrl, agentHost }]); authenticationMcpService.updateAccountPreference(options.mcpServerId, providerId, session.account); diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts index edbdee0d59a0..e4d2fb8c5a40 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts @@ -21,7 +21,7 @@ import { IAuthenticationMcpUsageService } from '../../../../../services/authenti import { IAuthenticationService, type AuthenticationSession, type IAuthenticationProvider } from '../../../../../services/authentication/common/authentication.js'; import { IDynamicAuthenticationProviderStorageService } from '../../../../../services/authentication/common/dynamicAuthenticationProviderStorage.js'; import { CHAT_SETUP_ACTION_ID } from '../../../browser/actions/chatActions.js'; -import { AgentHostAuthenticationRecovery, authenticateProtectedResources, resolveAuthenticationInteractively, resolveTokenForResource, AgentHostAuthTokenCache, agentHostMcpServerId, resolveMcpServerAuthentication, modelRequiresAgentAuthentication, revokeAuthenticationForRemovedSessions, type IAgentHostAuthenticationOptions } from '../../../browser/agentSessions/agentHost/agentHostAuth.js'; +import { AgentHostAuthenticationRecovery, authenticateProtectedResources, resolveAuthenticationInteractively, resolveSessionForResource, AgentHostAuthTokenCache, agentHostMcpServerId, resolveMcpServerAuthentication, modelRequiresAgentAuthentication, revokeAuthenticationForRemovedSessions, type IAgentHostAuthenticationOptions } from '../../../browser/agentSessions/agentHost/agentHostAuth.js'; import { createAgentModelByokMeta } from '../../../../../../platform/agentHost/common/agentModelByokMeta.js'; class TestCommandService extends mock() { @@ -46,7 +46,7 @@ function createAuthInstantiationService(disposables: Pick Promise; - getSessions?: (providerId: string, scopes: string[] | undefined, options: any, activate: boolean) => Promise; + getSessions?: (providerId: string, scopes: string[] | undefined, options: any, activate: boolean) => Promise; createSession?: (providerId: string, scopes: string[], options: any) => Promise<{ accessToken: string }>; createDynamicAuthenticationProvider?: (...args: Parameters) => Promise<{ readonly id: string } | undefined>; getProvider?: IAuthenticationService['getProvider']; @@ -93,7 +93,7 @@ suite('agentHostMcpServerId', () => { }); }); -suite('resolveTokenForResource', () => { +suite('resolveSessionForResource', () => { const log = new NullLogService(); const resource = URI.parse('https://api.example.com'); @@ -102,7 +102,7 @@ suite('resolveTokenForResource', () => { test('returns undefined when no authorization servers provided', async () => { const authService = createMockAuthService({}); - const token = await resolveTokenForResource(resource, [], ['read'], authService, log, 'test'); + const token = (await resolveSessionForResource(resource, [], ['read'], authService, log, 'test'))?.accessToken; assert.strictEqual(token, undefined); }); @@ -110,7 +110,7 @@ suite('resolveTokenForResource', () => { const authService = createMockAuthService({ getOrActivateProviderIdForServer: () => Promise.resolve(undefined), }); - const token = await resolveTokenForResource(resource, ['https://auth.example.com'], ['read'], authService, log, 'test'); + const token = (await resolveSessionForResource(resource, ['https://auth.example.com'], ['read'], authService, log, 'test'))?.accessToken; assert.strictEqual(token, undefined); }); @@ -124,7 +124,7 @@ suite('resolveTokenForResource', () => { return Promise.resolve([]); }, }); - const token = await resolveTokenForResource(resource, ['https://auth.example.com'], ['read'], authService, log, 'test'); + const token = (await resolveSessionForResource(resource, ['https://auth.example.com'], ['read'], authService, log, 'test'))?.accessToken; assert.strictEqual(token, 'exact-token'); }); @@ -143,7 +143,7 @@ suite('resolveTokenForResource', () => { ]); }, }); - const token = await resolveTokenForResource(resource, ['https://auth.example.com'], ['read'], authService, log, 'test'); + const token = (await resolveSessionForResource(resource, ['https://auth.example.com'], ['read'], authService, log, 'test'))?.accessToken; assert.strictEqual(token, 'narrow-token'); }); @@ -160,7 +160,7 @@ suite('resolveTokenForResource', () => { ]); }, }); - const token = await resolveTokenForResource(resource, ['https://auth.example.com'], ['read'], authService, log, 'test'); + const token = (await resolveSessionForResource(resource, ['https://auth.example.com'], ['read'], authService, log, 'test'))?.accessToken; assert.strictEqual(token, undefined); }); @@ -176,11 +176,11 @@ suite('resolveTokenForResource', () => { }, getSessions: () => Promise.resolve([{ scopes: ['read'], accessToken: 'server2-token' }]), }); - const token = await resolveTokenForResource( + const token = (await resolveSessionForResource( resource, ['https://auth1.example.com', 'https://auth2.example.com'], ['read'], authService, log, 'test', - ); + ))?.accessToken; assert.strictEqual(token, 'server2-token'); assert.strictEqual(calls.length, 2); }); @@ -1137,14 +1137,14 @@ suite('authenticateProtectedResources', () => { getOrActivateProviderIdForServer: () => Promise.resolve('provider-1'), getSessions: (_providerId, scopes) => { if (scopes) { - return Promise.resolve([{ scopes: ['read'], accessToken: 'cached-token' }]); + return Promise.resolve([{ scopes: ['read'], accessToken: 'cached-token', expiresIn: 3600 }]); } return Promise.resolve([]); }, }); const cache = new AgentHostAuthTokenCache(); - const requests: { resource: string; scopes?: readonly string[]; token: string }[] = []; + const requests: { resource: string; scopes?: readonly string[]; token: string; expiresIn?: number }[] = []; const agents = [{ protectedResources: [protectedResource] }] as unknown as readonly AgentInfo[]; const instantiationService = createAuthInstantiationService(disposables, authService); @@ -1163,6 +1163,25 @@ suite('authenticateProtectedResources', () => { }, }); + assert.deepStrictEqual(requests, [{ resource: protectedResource.resource, scopes: ['read'], token: 'cached-token', expiresIn: 3600 }]); + }); + + test('forwards a token without a malformed session expiry', async () => { + const authService = createMockAuthService({ + getOrActivateProviderIdForServer: () => Promise.resolve('provider-1'), + getSessions: (_providerId, scopes) => Promise.resolve(scopes ? [{ scopes: ['read'], accessToken: 'cached-token', expiresIn: 0 }] : []), + }); + const requests: { resource: string; scopes?: readonly string[]; token: string; expiresIn?: number }[] = []; + const agents = [{ protectedResources: [protectedResource] }] as unknown as readonly AgentInfo[]; + const instantiationService = createAuthInstantiationService(disposables, authService); + + await instantiationService.invokeFunction(authenticateProtectedResources, agents, { + logPrefix: '[AgentHost]', + authenticate: async request => { + requests.push(request); + }, + }); + assert.deepStrictEqual(requests, [{ resource: protectedResource.resource, scopes: ['read'], token: 'cached-token' }]); }); diff --git a/src/vs/workbench/services/authentication/common/authentication.ts b/src/vs/workbench/services/authentication/common/authentication.ts index bd9683498b33..04c67d5c86b3 100644 --- a/src/vs/workbench/services/authentication/common/authentication.ts +++ b/src/vs/workbench/services/authentication/common/authentication.ts @@ -34,6 +34,7 @@ export interface AuthenticationSession { account: AuthenticationSessionAccount; scopes: ReadonlyArray; idToken?: string; + expiresIn?: number; } export interface AuthenticationSessionsChangeEvent { diff --git a/src/vscode-dts/vscode.proposed.authSessionExpiration.d.ts b/src/vscode-dts/vscode.proposed.authSessionExpiration.d.ts new file mode 100644 index 000000000000..4d183777a704 --- /dev/null +++ b/src/vscode-dts/vscode.proposed.authSessionExpiration.d.ts @@ -0,0 +1,18 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +declare module 'vscode' { + + export interface AuthenticationSession { + /** + * The access token's remaining lifetime, in seconds, when the authentication provider returns the session. + * + * This corresponds to `expires_in` in an OAuth 2.0 token response. Providers returning + * cached sessions must recompute this value. This is undefined when the authentication + * provider does not know the access token's expiry. When defined, this must be a positive integer. + */ + readonly expiresIn?: number; + } +}