Skip to content

Commit e658e20

Browse files
committed
fix: recover mTLS auth after settings races (#976)
A session suspended by an mTLS or coder.headerCommand failure stayed suspended even after the user fixed the setting. Two races caused it: a request could 401 from a mid-flight settings change with no way to recover, and nothing re-tried once the fix landed. - Suspended sessions auto-recover once auth settings become valid again, with no logout/login round-trip. - A 401 caused by a settings change mid-flight is retried silently under the new settings instead of escalating to an interactive prompt. - Retried requests no longer carry stale headers from a previous header-command run. - Logout, deployment switch, or dispose during an in-flight auth verify is no longer overwritten by the verify finishing. - Cross-window login keeps listening if the first observed token from another window is invalid, so a follow-up valid write still resolves the dialog. - Config-change side-effects (reload prompt, recovery, reconnects) fire once edits settle instead of on the first event of a burst. - Concurrent logout no longer leaves stale deployment data in storage. Closes #973
1 parent bb48b25 commit e658e20

17 files changed

Lines changed: 760 additions & 197 deletions

src/api/authInterceptor.ts

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import type * as vscode from "vscode";
77

88
import type { SecretsManager } from "../core/secretsManager";
99
import type { Logger } from "../logging/logger";
10-
import type { RequestConfigWithMeta } from "../logging/types";
1110
import type { OAuthSessionManager } from "../oauth/sessionManager";
1211

1312
import type { CoderApi } from "./coderApi";
@@ -51,13 +50,6 @@ export class AuthInterceptor implements vscode.Disposable {
5150
throw error;
5251
}
5352

54-
if (error.config) {
55-
const config = error.config as { _retryAttempted?: boolean };
56-
if (config._retryAttempted) {
57-
throw error;
58-
}
59-
}
60-
6153
if (error.response?.status !== 401) {
6254
throw error;
6355
}
@@ -75,6 +67,26 @@ export class AuthInterceptor implements vscode.Disposable {
7567
error: AxiosError,
7668
hostname: string,
7769
): Promise<unknown> {
70+
const config = error.config;
71+
72+
// Checked before _retryAttempted so an OAuth-retry 401 caused by a
73+
// fresh settings change still gets one silent attempt.
74+
if (
75+
config &&
76+
!config._authConfigRetryAttempted &&
77+
this.client.hasAuthConfigChangedSince(config.authConfigVersion)
78+
) {
79+
config._authConfigRetryAttempted = true;
80+
this.logger.debug(
81+
"Authentication settings changed during request, retrying once",
82+
);
83+
return this.client.getAxiosInstance().request(config);
84+
}
85+
86+
if (config?._retryAttempted) {
87+
throw error;
88+
}
89+
7890
this.logger.debug("Received 401 response, attempting recovery");
7991

8092
if (await this.oauthSessionManager.isLoggedInWithOAuth(hostname)) {
@@ -142,12 +154,9 @@ export class AuthInterceptor implements vscode.Disposable {
142154
throw error;
143155
}
144156

145-
const config = error.config as RequestConfigWithMeta & {
146-
_retryAttempted?: boolean;
147-
};
148-
config._retryAttempted = true;
149-
config.headers[coderSessionTokenHeader] = token;
150-
return this.client.getAxiosInstance().request(config);
157+
error.config._retryAttempted = true;
158+
error.config.headers[coderSessionTokenHeader] = token;
159+
return this.client.getAxiosInstance().request(error.config);
151160
}
152161

153162
public dispose(): void {

src/api/axios.d.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import "axios";
2+
3+
declare module "axios" {
4+
interface InternalAxiosRequestConfig {
5+
/** Set once the OAuth-refresh or interactive re-auth path has run. */
6+
_retryAttempted?: boolean;
7+
/**
8+
* Set once the silent auth-config-change retry has run. Separate
9+
* from `_retryAttempted` so a follow-up 401 can still escalate to
10+
* OAuth or interactive re-auth.
11+
*/
12+
_authConfigRetryAttempted?: boolean;
13+
/** Set once a client-certificate error has been retried with refreshed certs. */
14+
_certRetried?: boolean;
15+
/**
16+
* Auth-config version snapshotted when the request was stamped.
17+
* Compared on a 401 to detect that auth settings changed mid-flight.
18+
*/
19+
authConfigVersion?: number;
20+
/**
21+
* Headers the previous header-command run added to this request.
22+
* Cleared at the start of the next pass so stale keys don't leak
23+
* through when the command output changes between retries.
24+
*/
25+
headerCommandKeys?: string[];
26+
}
27+
}

src/api/coderApi.ts

Lines changed: 70 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ import {
1717
import * as vscode from "vscode";
1818
import { type ClientOptions } from "ws";
1919

20-
import { watchConfigurationChanges } from "../configWatcher";
20+
import {
21+
CONFIG_CHANGE_DEBOUNCE_MS,
22+
watchConfigurationChanges,
23+
} from "../configWatcher";
2124
import { ClientCertificateError } from "../error/clientCertificateError";
2225
import { toError } from "../error/errorUtils";
2326
import { ServerCertificateError } from "../error/serverCertificateError";
@@ -35,6 +38,7 @@ import {
3538
HttpClientLogLevel,
3639
} from "../logging/types";
3740
import { sizeOf } from "../logging/utils";
41+
import { AuthConfigTracker } from "../settings/authConfig";
3842
import { getHeaderCommand } from "../settings/headers";
3943
import { HttpStatusCode, WebSocketCloseCode } from "../websocket/codes";
4044
import {
@@ -86,7 +90,10 @@ export class CoderApi extends Api implements vscode.Disposable {
8690
>();
8791
private readonly configWatcher: vscode.Disposable;
8892

89-
private constructor(private readonly output: Logger) {
93+
private constructor(
94+
private readonly output: Logger,
95+
private readonly authConfigTracker: AuthConfigTracker,
96+
) {
9097
super();
9198
this.configWatcher = this.watchConfigChanges();
9299
}
@@ -100,17 +107,22 @@ export class CoderApi extends Api implements vscode.Disposable {
100107
token: string | undefined,
101108
output: Logger,
102109
): CoderApi {
103-
const client = new CoderApi(output);
110+
const authConfigTracker = new AuthConfigTracker();
111+
const client = new CoderApi(output, authConfigTracker);
104112
client.setCredentials(baseUrl, token);
105113

106-
setupInterceptors(client, output);
114+
setupInterceptors(client, output, authConfigTracker);
107115
return client;
108116
}
109117

110118
getHost(): string | undefined {
111119
return this.getAxiosInstance().defaults.baseURL;
112120
}
113121

122+
hasAuthConfigChangedSince(version: number | undefined): boolean {
123+
return this.authConfigTracker.hasChangedSince(version);
124+
}
125+
114126
/**
115127
* Set both host and token together. Useful for login/logout/switch to
116128
* avoid triggering multiple reconnection events.
@@ -155,6 +167,7 @@ export class CoderApi extends Api implements vscode.Disposable {
155167
*/
156168
dispose(): void {
157169
this.configWatcher.dispose();
170+
this.authConfigTracker.dispose();
158171
for (const socket of this.reconnectingSockets) {
159172
socket.close();
160173
}
@@ -171,20 +184,24 @@ export class CoderApi extends Api implements vscode.Disposable {
171184
setting,
172185
getValue: () => vscode.workspace.getConfiguration().get(setting),
173186
}));
174-
return watchConfigurationChanges(settings, () => {
175-
const socketsToReconnect = [...this.reconnectingSockets].filter(
176-
(socket) => socket.state === ConnectionState.DISCONNECTED,
177-
);
178-
if (socketsToReconnect.length) {
179-
this.output.debug(
180-
`Configuration changed, ${socketsToReconnect.length}/${this.reconnectingSockets.size} socket(s) in DISCONNECTED state`,
187+
return watchConfigurationChanges(
188+
settings,
189+
() => {
190+
const socketsToReconnect = [...this.reconnectingSockets].filter(
191+
(socket) => socket.state === ConnectionState.DISCONNECTED,
181192
);
182-
for (const socket of socketsToReconnect) {
183-
this.output.debug(`Reconnecting WebSocket: ${socket.url}`);
184-
socket.reconnect();
193+
if (socketsToReconnect.length) {
194+
this.output.debug(
195+
`Configuration changed, ${socketsToReconnect.length}/${this.reconnectingSockets.size} socket(s) in DISCONNECTED state`,
196+
);
197+
for (const socket of socketsToReconnect) {
198+
this.output.debug(`Reconnecting WebSocket: ${socket.url}`);
199+
socket.reconnect();
200+
}
185201
}
186-
}
187-
});
202+
},
203+
{ debounceMs: CONFIG_CHANGE_DEBOUNCE_MS },
204+
);
188205
}
189206

190207
watchInboxNotifications = async (
@@ -473,24 +490,51 @@ export class CoderApi extends Api implements vscode.Disposable {
473490
/**
474491
* Set up logging and request interceptors for the CoderApi instance.
475492
*/
476-
function setupInterceptors(client: CoderApi, output: Logger): void {
493+
function setupInterceptors(
494+
client: CoderApi,
495+
output: Logger,
496+
authConfigTracker: AuthConfigTracker,
497+
): void {
477498
addLoggingInterceptors(client.getAxiosInstance(), output);
478499

479500
client.getAxiosInstance().interceptors.request.use(async (config) => {
501+
// Snapshot the version up front so it matches the config we're about
502+
// to read, not whatever it bumps to during the awaits below.
503+
config.authConfigVersion = authConfigTracker.version;
504+
505+
// Drop headers from the prior header-command run so stale keys can't
506+
// leak through if the command output changed between attempts.
507+
for (const key of config.headerCommandKeys ?? []) {
508+
config.headers.delete(key);
509+
}
510+
480511
const baseUrl = client.getAxiosInstance().defaults.baseURL;
481512
const headers = await getHeaders(
482513
baseUrl,
483514
getHeaderCommand(vscode.workspace.getConfiguration()),
484515
output,
485516
);
486-
// Add headers from the header command.
517+
const retrying =
518+
config._retryAttempted === true ||
519+
config._authConfigRetryAttempted === true;
487520
for (const [key, value] of Object.entries(headers)) {
521+
// On retry, don't let stale command output overwrite the session
522+
// token retryRequest just wrote.
523+
if (
524+
retrying &&
525+
key.toLowerCase() === coderSessionTokenHeader.toLowerCase()
526+
) {
527+
continue;
528+
}
488529
config.headers[key] = value;
489530
}
531+
// Don't track the session token: cleanup must never touch it.
532+
config.headerCommandKeys = Object.keys(headers).filter(
533+
(k) => k.toLowerCase() !== coderSessionTokenHeader.toLowerCase(),
534+
);
490535

491-
// Configure proxy and TLS.
492-
// Note that by default VS Code overrides the agent. To prevent this, set
493-
// `http.proxySupport` to `on` or `off`.
536+
// VS Code overrides the agent by default; set `http.proxySupport` to
537+
// `on` or `off` to keep ours.
494538
const agent = await createHttpAgent(vscode.workspace.getConfiguration());
495539
config.httpsAgent = agent;
496540
config.httpAgent = agent;
@@ -594,13 +638,10 @@ async function tryRefreshClientCertificate(
594638
}
595639

596640
// _certRetried is per-request (Axios creates fresh config per request).
597-
const config = err.config as RequestConfigWithMeta & {
598-
_certRetried?: boolean;
599-
};
600-
if (config._certRetried) {
641+
if (err.config._certRetried) {
601642
throw certError;
602643
}
603-
config._certRetried = true;
644+
err.config._certRetried = true;
604645

605646
output.info(
606647
`Client certificate error (alert ${certError.alertCode}), attempting refresh...`,
@@ -612,12 +653,12 @@ async function tryRefreshClientCertificate(
612653

613654
// Create new agent with refreshed certificates.
614655
const agent = await createHttpAgent(vscode.workspace.getConfiguration());
615-
config.httpsAgent = agent;
616-
config.httpAgent = agent;
656+
err.config.httpsAgent = agent;
657+
err.config.httpAgent = agent;
617658

618659
// Retry the request.
619660
output.info("Retrying request with refreshed certificates...");
620-
return axiosInstance.request(config);
661+
return axiosInstance.request(err.config);
621662
}
622663

623664
function wrapRequestTransform(

src/configWatcher.ts

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,70 @@
11
import { isDeepStrictEqual } from "node:util";
22
import * as vscode from "vscode";
33

4+
/** Idle window for config-change reactions. Coalesces rapid edits into one fire. */
5+
export const CONFIG_CHANGE_DEBOUNCE_MS = 200;
6+
47
export interface WatchedSetting {
58
setting: string;
69
getValue: () => unknown;
710
}
811

12+
export interface WatchConfigurationChangesOptions {
13+
/**
14+
* Idle window in ms. Each new event resets the timer; the callback
15+
* fires once settings have been quiet for this long. Unset means fire
16+
* synchronously on every event.
17+
*/
18+
debounceMs?: number;
19+
}
20+
921
/**
10-
* Watch for configuration changes and invoke a callback when values change.
11-
* Only fires when actual values change, not just when settings are touched.
22+
* Watch for configuration changes and fire when watched values change.
23+
* With `debounceMs`, defers until settings have been quiet for that long.
1224
*/
1325
export function watchConfigurationChanges(
1426
settings: WatchedSetting[],
15-
onChange: (changedSettings: string[]) => void,
27+
onChange: (changes: ReadonlyMap<string, unknown>) => void,
28+
options: WatchConfigurationChangesOptions = {},
1629
): vscode.Disposable {
1730
const appliedValues = new Map(settings.map((s) => [s.setting, s.getValue()]));
1831

19-
return vscode.workspace.onDidChangeConfiguration((e) => {
20-
const changedSettings: string[] = [];
21-
32+
const detectAndFire = () => {
33+
const changes = new Map<string, unknown>();
2234
for (const { setting, getValue } of settings) {
23-
if (!e.affectsConfiguration(setting)) {
24-
continue;
25-
}
26-
2735
const newValue = getValue();
28-
2936
if (!configValuesEqual(newValue, appliedValues.get(setting))) {
30-
changedSettings.push(setting);
37+
changes.set(setting, newValue);
3138
appliedValues.set(setting, newValue);
3239
}
3340
}
41+
if (changes.size > 0) {
42+
onChange(changes);
43+
}
44+
};
3445

35-
if (changedSettings.length > 0) {
36-
onChange(changedSettings);
46+
let idleTimer: ReturnType<typeof setTimeout> | undefined;
47+
const listener = vscode.workspace.onDidChangeConfiguration((e) => {
48+
if (!settings.some((s) => e.affectsConfiguration(s.setting))) {
49+
return;
3750
}
51+
if (!options.debounceMs) {
52+
detectAndFire();
53+
return;
54+
}
55+
clearTimeout(idleTimer);
56+
idleTimer = setTimeout(() => {
57+
idleTimer = undefined;
58+
detectAndFire();
59+
}, options.debounceMs);
3860
});
61+
62+
return {
63+
dispose: () => {
64+
clearTimeout(idleTimer);
65+
listener.dispose();
66+
},
67+
};
3968
}
4069

4170
function configValuesEqual(a: unknown, b: unknown): boolean {

0 commit comments

Comments
 (0)