Skip to content

Commit 71b3595

Browse files
authored
feat: disable pkce flow during plugin load (aws#2153)
1 parent a6c64f2 commit 71b3595

File tree

3 files changed

+77
-14
lines changed

3 files changed

+77
-14
lines changed

server/aws-lsp-codewhisperer/src/language-server/agenticChat/errors.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ type AgenticChatErrorCode =
1111
| 'MCPServerInitTimeout' // mcp server failed to start within allowed time
1212
| 'MCPToolExecTimeout' // mcp tool call failed to complete within allowed time
1313
| 'MCPServerConnectionFailed' // mcp server failed to connect
14+
| 'MCPServerAuthFailed' // mcp server failed to complete auth flow
1415
| 'RequestAborted' // request was aborted by the user
1516
| 'RequestThrottled' // request was aborted by the user
1617

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.ts

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ import { OAuthClient } from './mcpOauthClient'
4242

4343
export const MCP_SERVER_STATUS_CHANGED = 'mcpServerStatusChanged'
4444
export const AGENT_TOOLS_CHANGED = 'agentToolsChanged'
45+
export enum AuthIntent {
46+
Interactive = 'interactive',
47+
Silent = 'silent',
48+
}
4549

4650
/**
4751
* Manages MCP servers and their tools
@@ -166,7 +170,7 @@ export class McpManager {
166170
/**
167171
* Load configurations and initialize each enabled server.
168172
*/
169-
private async discoverAllServers(): Promise<void> {
173+
private async discoverAllServers(authIntent: AuthIntent = AuthIntent.Silent): Promise<void> {
170174
// Load agent config
171175
const result = await loadAgentConfig(this.features.workspace, this.features.logging, this.agentPaths)
172176

@@ -217,7 +221,7 @@ export class McpManager {
217221
// Process servers in batches
218222
for (let i = 0; i < totalServers; i += MAX_CONCURRENT_SERVERS) {
219223
const batch = serversToInit.slice(i, i + MAX_CONCURRENT_SERVERS)
220-
const batchPromises = batch.map(([name, cfg]) => this.initOneServer(name, cfg))
224+
const batchPromises = batch.map(([name, cfg]) => this.initOneServer(name, cfg, authIntent))
221225

222226
this.features.logging.debug(
223227
`MCP: initializing batch of ${batch.length} servers (${i + 1}-${Math.min(i + MAX_CONCURRENT_SERVERS, totalServers)} of ${totalServers})`
@@ -292,7 +296,11 @@ export class McpManager {
292296
* Start a server process, connect client, and register its tools.
293297
* Errors are logged but do not stop discovery of other servers.
294298
*/
295-
private async initOneServer(serverName: string, cfg: MCPServerConfig): Promise<void> {
299+
private async initOneServer(
300+
serverName: string,
301+
cfg: MCPServerConfig,
302+
authIntent: AuthIntent = AuthIntent.Silent
303+
): Promise<void> {
296304
const DEFAULT_SERVER_INIT_TIMEOUT_MS = 60_000
297305
this.setState(serverName, McpServerStatus.INITIALIZING, 0)
298306

@@ -365,10 +373,19 @@ export class McpManager {
365373

366374
if (needsOAuth) {
367375
OAuthClient.initialize(this.features.workspace, this.features.logging)
368-
const bearer = await OAuthClient.getValidAccessToken(base)
376+
const bearer = await OAuthClient.getValidAccessToken(base, {
377+
interactive: authIntent === 'interactive',
378+
})
369379
// add authorization header if we are able to obtain a bearer token
370380
if (bearer) {
371381
headers = { ...headers, Authorization: `Bearer ${bearer}` }
382+
} else if (authIntent === 'silent') {
383+
// In silent mode we never launch a browser. If we cannot obtain a token
384+
// from cache/refresh, surface a clear auth-required error and stop here.
385+
throw new AgenticChatError(
386+
`MCP: server '${serverName}' requires OAuth. Open "Edit MCP Server" and save to sign in.`,
387+
'MCPServerAuthFailed'
388+
)
372389
}
373390
}
374391

@@ -707,7 +724,7 @@ export class McpManager {
707724
await saveAgentConfig(this.features.workspace, this.features.logging, this.agentConfig, agentPath)
708725

709726
// Add server tools to tools list after initialization
710-
await this.initOneServer(sanitizedName, newCfg)
727+
await this.initOneServer(sanitizedName, newCfg, AuthIntent.Interactive)
711728
} catch (err) {
712729
this.features.logging.error(
713730
`Failed to add MCP server '${serverName}': ${err instanceof Error ? err.message : String(err)}`
@@ -872,7 +889,7 @@ export class McpManager {
872889
this.setState(serverName, McpServerStatus.DISABLED, 0)
873890
this.emitToolsChanged(serverName)
874891
} else {
875-
await this.initOneServer(serverName, newCfg)
892+
await this.initOneServer(serverName, newCfg, AuthIntent.Interactive)
876893
}
877894
} catch (err) {
878895
this.handleError(serverName, err)
@@ -1086,7 +1103,7 @@ export class McpManager {
10861103
this.setState(serverName, McpServerStatus.DISABLED, 0)
10871104
} else {
10881105
if (!this.clients.has(serverName) && serverName !== 'Built-in') {
1089-
await this.initOneServer(serverName, this.mcpServers.get(serverName)!)
1106+
await this.initOneServer(serverName, this.mcpServers.get(serverName)!, AuthIntent.Silent)
10901107
}
10911108
}
10921109

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpOauthClient.ts

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,55 @@ export class OAuthClient {
4242

4343
/**
4444
* Return a valid Bearer token, reusing cache or refresh-token if possible,
45-
* otherwise driving one interactive PKCE flow.
45+
* otherwise (when interactive) driving one PKCE flow that may launch a browser.
4646
*/
47-
public static async getValidAccessToken(mcpBase: URL): Promise<string> {
47+
public static async getValidAccessToken(
48+
mcpBase: URL,
49+
opts: { interactive?: boolean } = { interactive: true }
50+
): Promise<string | undefined> {
51+
const interactive = opts?.interactive !== false
4852
const key = this.computeKey(mcpBase)
4953
const regPath = path.join(this.cacheDir, `${key}.registration.json`)
5054
const tokPath = path.join(this.cacheDir, `${key}.token.json`)
5155

56+
// ===== Silent branch: try cached token, then refresh, never opens a browser =====
57+
if (!interactive) {
58+
// 1) cached access token
59+
const cachedTok = await this.read<Token>(tokPath)
60+
if (cachedTok) {
61+
const expiry = cachedTok.obtained_at + cachedTok.expires_in * 1000
62+
if (Date.now() < expiry) {
63+
this.logger.info(`OAuth: using still-valid cached token (silent)`)
64+
return cachedTok.access_token
65+
}
66+
this.logger.info(`OAuth: cached token expired → try refresh (silent)`)
67+
}
68+
69+
// 2) refresh-token grant (if we have registration and refresh token)
70+
const savedReg = await this.read<Registration>(regPath)
71+
if (cachedTok?.refresh_token && savedReg) {
72+
try {
73+
const meta = await this.discoverAS(mcpBase)
74+
const refreshed = await this.refreshGrant(meta, savedReg, mcpBase, cachedTok.refresh_token)
75+
if (refreshed) {
76+
await this.write(tokPath, refreshed)
77+
this.logger.info(`OAuth: refresh grant succeeded (silent)`)
78+
return refreshed.access_token
79+
}
80+
this.logger.info(`OAuth: refresh grant did not succeed (silent)`)
81+
} catch (e) {
82+
this.logger.warn(`OAuth: silent refresh failed — ${e instanceof Error ? e.message : String(e)}`)
83+
}
84+
}
85+
86+
// 3) no token in silent mode → caller should surface auth-required UI
87+
return undefined
88+
}
89+
90+
// ===== Interactive branch: may open a browser (PKCE) =====
5291
// 1) Spin up (or reuse) loopback server + redirect URI
53-
let server: http.Server, redirectUri: string
92+
let server: http.Server | null = null
93+
let redirectUri: string
5494
const savedReg = await this.read<Registration>(regPath)
5595
if (savedReg) {
5696
const port = Number(new URL(savedReg.redirect_uri).port)
@@ -75,7 +115,9 @@ export class OAuthClient {
75115
}
76116
}
77117
} else {
78-
;({ server, redirectUri } = await this.buildCallbackServer())
118+
const created = await this.buildCallbackServer()
119+
server = created.server
120+
redirectUri = created.redirectUri
79121
this.logger.info(`OAuth: new redirect URI ${redirectUri}`)
80122
}
81123

@@ -85,7 +127,7 @@ export class OAuthClient {
85127
if (cached) {
86128
const expiry = cached.obtained_at + cached.expires_in * 1000
87129
if (Date.now() < expiry) {
88-
this.logger.info(`OAuth: using stillvalid cached token`)
130+
this.logger.info(`OAuth: using still-valid cached token`)
89131
return cached.access_token
90132
}
91133
this.logger.info(`OAuth: cached token expired → try refresh`)
@@ -98,6 +140,7 @@ export class OAuthClient {
98140
} catch (e: any) {
99141
throw new Error(`OAuth discovery failed: ${e?.message ?? String(e)}`)
100142
}
143+
101144
// 4) Register (or reuse) a dynamic client
102145
const scopes = ['openid', 'offline_access']
103146
let reg: Registration
@@ -107,7 +150,7 @@ export class OAuthClient {
107150
throw new Error(`OAuth client registration failed: ${e?.message ?? String(e)}`)
108151
}
109152

110-
// 5) Refreshtoken grant (one shot)
153+
// 5) Refresh-token grant (one shot)
111154
const attemptedRefresh = !!cached?.refresh_token
112155
if (cached?.refresh_token) {
113156
const refreshed = await this.refreshGrant(meta, reg, mcpBase, cached.refresh_token)
@@ -129,7 +172,9 @@ export class OAuthClient {
129172
throw new Error(`OAuth authorization (PKCE) failed${suffix}: ${e?.message ?? String(e)}`)
130173
}
131174
} finally {
132-
await new Promise<void>(res => server.close(() => res()))
175+
if (server) {
176+
await new Promise<void>(res => server!.close(() => res()))
177+
}
133178
}
134179
}
135180

0 commit comments

Comments
 (0)