Skip to content

Commit 5637fe7

Browse files
authored
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 e87ea3d commit 5637fe7

17 files changed

Lines changed: 751 additions & 192 deletions

src/api/authInterceptor.ts

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import type * as vscode from "vscode";
99
import type { ServiceContainer } from "../core/container";
1010
import type { SecretsManager } from "../core/secretsManager";
1111
import type { Logger } from "../logging/logger";
12-
import type { RequestConfigWithMeta } from "../logging/types";
1312
import type { OAuthSessionManager } from "../oauth/sessionManager";
1413

1514
import type { CoderApi } from "./coderApi";
@@ -58,13 +57,6 @@ export class AuthInterceptor implements vscode.Disposable {
5857
throw error;
5958
}
6059

61-
if (error.config) {
62-
const config = error.config as { _retryAttempted?: boolean };
63-
if (config._retryAttempted) {
64-
throw error;
65-
}
66-
}
67-
6860
if (error.response?.status !== 401) {
6961
throw error;
7062
}
@@ -82,6 +74,26 @@ export class AuthInterceptor implements vscode.Disposable {
8274
error: AxiosError,
8375
hostname: string,
8476
): Promise<unknown> {
77+
const config = error.config;
78+
79+
// Checked before _retryAttempted so an OAuth-retry 401 caused by a
80+
// fresh settings change still gets one silent attempt.
81+
if (
82+
config &&
83+
!config._authConfigRetryAttempted &&
84+
this.client.hasAuthConfigChangedSince(config.authConfigVersion)
85+
) {
86+
config._authConfigRetryAttempted = true;
87+
this.logger.debug(
88+
"Authentication settings changed during request, retrying once",
89+
);
90+
return this.client.getAxiosInstance().request(config);
91+
}
92+
93+
if (config?._retryAttempted) {
94+
throw error;
95+
}
96+
8597
this.logger.debug("Received 401 response, attempting recovery");
8698
return this.authTelemetry.traceAuthRecovery(async (recorder) => {
8799
recorder.logReceived();
@@ -166,12 +178,9 @@ export class AuthInterceptor implements vscode.Disposable {
166178
throw error;
167179
}
168180

169-
const config = error.config as RequestConfigWithMeta & {
170-
_retryAttempted?: boolean;
171-
};
172-
config._retryAttempted = true;
173-
config.headers[coderSessionTokenHeader] = token;
174-
return this.client.getAxiosInstance().request(config);
181+
error.config._retryAttempted = true;
182+
error.config.headers[coderSessionTokenHeader] = token;
183+
return this.client.getAxiosInstance().request(error.config);
175184
}
176185

177186
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: 68 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import {
88
import { Api } from "coder/site/src/api/api";
99
import * as vscode from "vscode";
1010

11-
import { watchConfigurationChanges } from "../configWatcher";
11+
import {
12+
CONFIG_CHANGE_DEBOUNCE_MS,
13+
watchConfigurationChanges,
14+
} from "../configWatcher";
1215
import { ClientCertificateError } from "../error/clientCertificateError";
1316
import { toError } from "../error/errorUtils";
1417
import { ServerCertificateError } from "../error/serverCertificateError";
@@ -26,6 +29,7 @@ import {
2629
type RequestConfigWithMeta,
2730
} from "../logging/types";
2831
import { sizeOf } from "../logging/utils";
32+
import { AuthConfigTracker } from "../settings/authConfig";
2933
import { getHeaderCommand } from "../settings/headers";
3034
import {
3135
NOOP_TELEMETRY_REPORTER,
@@ -98,6 +102,7 @@ export class CoderApi extends Api implements vscode.Disposable {
98102
private readonly output: Logger,
99103
private readonly telemetry: TelemetryReporter,
100104
private readonly httpRequestsTelemetry: HttpRequestsTelemetry,
105+
private readonly authConfigTracker: AuthConfigTracker,
101106
) {
102107
super();
103108
this.configWatcher = this.watchConfigChanges();
@@ -117,17 +122,27 @@ export class CoderApi extends Api implements vscode.Disposable {
117122
telemetry: TelemetryReporter = NOOP_TELEMETRY_REPORTER,
118123
): CoderApi {
119124
const httpRequestsTelemetry = new HttpRequestsTelemetry(telemetry);
120-
const client = new CoderApi(output, telemetry, httpRequestsTelemetry);
125+
const authConfigTracker = new AuthConfigTracker();
126+
const client = new CoderApi(
127+
output,
128+
telemetry,
129+
httpRequestsTelemetry,
130+
authConfigTracker,
131+
);
121132
client.setCredentials(baseUrl, token);
122133

123-
setupInterceptors(client, output, httpRequestsTelemetry);
134+
setupInterceptors(client, output, httpRequestsTelemetry, authConfigTracker);
124135
return client;
125136
}
126137

127138
getHost(): string | undefined {
128139
return this.getAxiosInstance().defaults.baseURL;
129140
}
130141

142+
hasAuthConfigChangedSince(version: number | undefined): boolean {
143+
return this.authConfigTracker.hasChangedSince(version);
144+
}
145+
131146
/**
132147
* Set both host and token together. Useful for login/logout/switch to
133148
* avoid triggering multiple reconnection events.
@@ -172,6 +187,7 @@ export class CoderApi extends Api implements vscode.Disposable {
172187
*/
173188
dispose(): void {
174189
this.configWatcher.dispose();
190+
this.authConfigTracker.dispose();
175191
this.httpRequestsTelemetry.dispose();
176192
for (const socket of this.reconnectingSockets) {
177193
socket.close();
@@ -189,20 +205,24 @@ export class CoderApi extends Api implements vscode.Disposable {
189205
setting,
190206
getValue: () => vscode.workspace.getConfiguration().get(setting),
191207
}));
192-
return watchConfigurationChanges(settings, () => {
193-
const socketsToReconnect = [...this.reconnectingSockets].filter(
194-
(socket) => socket.state === ConnectionState.DISCONNECTED,
195-
);
196-
if (socketsToReconnect.length) {
197-
this.output.debug(
198-
`Configuration changed, ${socketsToReconnect.length}/${this.reconnectingSockets.size} socket(s) in DISCONNECTED state`,
208+
return watchConfigurationChanges(
209+
settings,
210+
() => {
211+
const socketsToReconnect = [...this.reconnectingSockets].filter(
212+
(socket) => socket.state === ConnectionState.DISCONNECTED,
199213
);
200-
for (const socket of socketsToReconnect) {
201-
this.output.debug(`Reconnecting WebSocket: ${socket.url}`);
202-
socket.reconnect();
214+
if (socketsToReconnect.length) {
215+
this.output.debug(
216+
`Configuration changed, ${socketsToReconnect.length}/${this.reconnectingSockets.size} socket(s) in DISCONNECTED state`,
217+
);
218+
for (const socket of socketsToReconnect) {
219+
this.output.debug(`Reconnecting WebSocket: ${socket.url}`);
220+
socket.reconnect();
221+
}
203222
}
204-
}
205-
});
223+
},
224+
{ debounceMs: CONFIG_CHANGE_DEBOUNCE_MS },
225+
);
206226
}
207227

208228
watchInboxNotifications = async (
@@ -495,6 +515,7 @@ function setupInterceptors(
495515
client: CoderApi,
496516
output: Logger,
497517
httpRequestsTelemetry: HttpRequestsTelemetry,
518+
authConfigTracker: AuthConfigTracker,
498519
): void {
499520
addRequestInterceptors(
500521
client.getAxiosInstance(),
@@ -503,20 +524,43 @@ function setupInterceptors(
503524
);
504525

505526
client.getAxiosInstance().interceptors.request.use(async (config) => {
527+
// Snapshot the version up front so it matches the config we're about
528+
// to read, not whatever it bumps to during the awaits below.
529+
config.authConfigVersion = authConfigTracker.version;
530+
531+
// Drop headers from the prior header-command run so stale keys can't
532+
// leak through if the command output changed between attempts.
533+
for (const key of config.headerCommandKeys ?? []) {
534+
config.headers.delete(key);
535+
}
536+
506537
const baseUrl = client.getAxiosInstance().defaults.baseURL;
507538
const headers = await getHeaders(
508539
baseUrl,
509540
getHeaderCommand(vscode.workspace.getConfiguration()),
510541
output,
511542
);
512-
// Add headers from the header command.
543+
const retrying =
544+
config._retryAttempted === true ||
545+
config._authConfigRetryAttempted === true;
513546
for (const [key, value] of Object.entries(headers)) {
547+
// On retry, don't let stale command output overwrite the session
548+
// token retryRequest just wrote.
549+
if (
550+
retrying &&
551+
key.toLowerCase() === coderSessionTokenHeader.toLowerCase()
552+
) {
553+
continue;
554+
}
514555
config.headers[key] = value;
515556
}
557+
// Don't track the session token: cleanup must never touch it.
558+
config.headerCommandKeys = Object.keys(headers).filter(
559+
(k) => k.toLowerCase() !== coderSessionTokenHeader.toLowerCase(),
560+
);
516561

517-
// Configure proxy and TLS.
518-
// Note that by default VS Code overrides the agent. To prevent this, set
519-
// `http.proxySupport` to `on` or `off`.
562+
// VS Code overrides the agent by default; set `http.proxySupport` to
563+
// `on` or `off` to keep ours.
520564
const agent = await createHttpAgent(vscode.workspace.getConfiguration());
521565
config.httpsAgent = agent;
522566
config.httpAgent = agent;
@@ -626,13 +670,10 @@ async function tryRefreshClientCertificate(
626670
}
627671

628672
// _certRetried is per-request (Axios creates fresh config per request).
629-
const config = err.config as RequestConfigWithMeta & {
630-
_certRetried?: boolean;
631-
};
632-
if (config._certRetried) {
673+
if (err.config._certRetried) {
633674
throw certError;
634675
}
635-
config._certRetried = true;
676+
err.config._certRetried = true;
636677

637678
output.info(
638679
`Client certificate error (alert ${certError.alertCode}), attempting refresh...`,
@@ -644,12 +685,12 @@ async function tryRefreshClientCertificate(
644685

645686
// Create new agent with refreshed certificates.
646687
const agent = await createHttpAgent(vscode.workspace.getConfiguration());
647-
config.httpsAgent = agent;
648-
config.httpAgent = agent;
688+
err.config.httpsAgent = agent;
689+
err.config.httpAgent = agent;
649690

650691
// Retry the request.
651692
output.info("Retrying request with refreshed certificates...");
652-
return axiosInstance.request(config);
693+
return axiosInstance.request(err.config);
653694
}
654695

655696
function wrapRequestTransform(

src/configWatcher.ts

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +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-
* The callback receives a map of changed settings to their new values, so
12-
* consumers can act on the new value without re-reading the configuration.
13-
* 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.
1424
*/
1525
export function watchConfigurationChanges(
1626
settings: WatchedSetting[],
1727
onChange: (changes: ReadonlyMap<string, unknown>) => void,
28+
options: WatchConfigurationChangesOptions = {},
1829
): vscode.Disposable {
1930
const appliedValues = new Map(settings.map((s) => [s.setting, s.getValue()]));
2031

21-
return vscode.workspace.onDidChangeConfiguration((e) => {
32+
const detectAndFire = () => {
2233
const changes = new Map<string, unknown>();
23-
2434
for (const { setting, getValue } of settings) {
25-
if (!e.affectsConfiguration(setting)) {
26-
continue;
27-
}
28-
2935
const newValue = getValue();
30-
3136
if (!configValuesEqual(newValue, appliedValues.get(setting))) {
3237
changes.set(setting, newValue);
3338
appliedValues.set(setting, newValue);
3439
}
3540
}
36-
3741
if (changes.size > 0) {
3842
onChange(changes);
3943
}
44+
};
45+
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;
50+
}
51+
if (!options.debounceMs) {
52+
detectAndFire();
53+
return;
54+
}
55+
clearTimeout(idleTimer);
56+
idleTimer = setTimeout(() => {
57+
idleTimer = undefined;
58+
detectAndFire();
59+
}, options.debounceMs);
4060
});
61+
62+
return {
63+
dispose: () => {
64+
clearTimeout(idleTimer);
65+
listener.dispose();
66+
},
67+
};
4168
}
4269

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

0 commit comments

Comments
 (0)