Skip to content

Commit f0a5125

Browse files
EhabYEhabY
committed
Rely on the secrets API + Refactorings
1 parent fdc3ae3 commit f0a5125

File tree

7 files changed

+102
-103
lines changed

7 files changed

+102
-103
lines changed

‎src/commands.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,9 @@ export class Commands {
107107
await this.mementoManager.setUrl(url);
108108
await this.secretsManager.setSessionToken(label, {
109109
url,
110-
sessionToken: result.token,
110+
token: result.token,
111111
});
112112

113-
// Store on disk for CLI
114-
await this.cliManager.configure(label, url, result.token);
115-
116113
// Update contexts
117114
this.contextManager.set("coder.authenticated", true);
118115
if (result.user.roles.some((role) => role.name === "owner")) {

‎src/core/cliManager.ts‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -501,8 +501,7 @@ export class CliManager {
501501
): Promise<void> {
502502
if (url) {
503503
const urlPath = this.pathResolver.getUrlPath(label);
504-
await fs.mkdir(path.dirname(urlPath), { recursive: true });
505-
await fs.writeFile(urlPath, url);
504+
await this.atomicWriteFile(urlPath, url);
506505
}
507506
}
508507

@@ -519,30 +518,27 @@ export class CliManager {
519518
) {
520519
if (token !== null) {
521520
const tokenPath = this.pathResolver.getSessionTokenPath(label);
522-
await fs.mkdir(path.dirname(tokenPath), { recursive: true });
523-
await fs.writeFile(tokenPath, token ?? "");
521+
await this.atomicWriteFile(tokenPath, token ?? "");
524522
}
525523
}
526524

527525
/**
528-
* Read the CLI config for a deployment with the provided label.
529-
*
530-
* IF a config file does not exist, return an empty string.
531-
*
532-
* If the label is empty, read the old deployment-unaware config.
526+
* Atomically write content to a file by writing to a temporary file first,
527+
* then renaming it.
533528
*/
534-
public async readConfig(
535-
label: string,
536-
): Promise<{ url: string; token: string }> {
537-
const urlPath = this.pathResolver.getUrlPath(label);
538-
const tokenPath = this.pathResolver.getSessionTokenPath(label);
539-
const [url, token] = await Promise.allSettled([
540-
fs.readFile(urlPath, "utf8"),
541-
fs.readFile(tokenPath, "utf8"),
542-
]);
543-
return {
544-
url: url.status === "fulfilled" ? url.value.trim() : "",
545-
token: token.status === "fulfilled" ? token.value.trim() : "",
546-
};
529+
private async atomicWriteFile(
530+
filePath: string,
531+
content: string,
532+
): Promise<void> {
533+
await fs.mkdir(path.dirname(filePath), { recursive: true });
534+
const tempPath =
535+
filePath + ".temp-" + Math.random().toString(36).substring(8);
536+
try {
537+
await fs.writeFile(tempPath, content);
538+
await fs.rename(tempPath, filePath);
539+
} catch (err) {
540+
await fs.rm(tempPath, { force: true }).catch(() => {});
541+
throw err;
542+
}
547543
}
548544
}

‎src/core/secretsManager.ts‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,18 @@ export type StoredOAuthTokens = Omit<TokenResponse, "expires_in"> & {
2222

2323
export interface SessionAuth {
2424
url: string;
25-
sessionToken: string;
25+
token: string;
2626
}
2727

2828
export interface OAuthData {
2929
oauthClientRegistration?: ClientRegistrationResponse;
3030
oauthTokens?: StoredOAuthTokens;
3131
}
3232

33-
export type SessionAuthMap = Record<string, SessionAuth>;
34-
export type OAuthDataMap = Record<string, OAuthData>;
33+
// TODO not sure if I love this bulk saving of data
34+
// How do we clear out outdated info? How do we deal with race conditions across windows?
35+
type SessionAuthMap = Record<string, SessionAuth>;
36+
type OAuthDataMap = Record<string, OAuthData>;
3537

3638
interface OAuthCallbackData {
3739
state: string;
@@ -59,7 +61,7 @@ export class SecretsManager {
5961
// Initialize previous session tokens
6062
this.getSessionAuthMap().then((map) => {
6163
for (const [label, auth] of Object.entries(map)) {
62-
this.previousSessionTokens.set(label, auth.sessionToken);
64+
this.previousSessionTokens.set(label, auth.token);
6365
}
6466
});
6567
}
@@ -162,7 +164,7 @@ export class SecretsManager {
162164
): Disposable {
163165
return this.onDidChangeSessionAuthMap(async (map) => {
164166
const auth = map[label];
165-
const newToken = auth?.sessionToken ?? "";
167+
const newToken = auth?.token ?? "";
166168
const previousToken = this.previousSessionTokens.get(label) ?? "";
167169

168170
// Only fire listener if session token actually changed
@@ -199,15 +201,23 @@ export class SecretsManager {
199201
*/
200202
public async getSessionToken(label: string): Promise<string | undefined> {
201203
const map = await this.getSessionAuthMap();
202-
return map[label]?.sessionToken;
204+
return map[label]?.token;
205+
}
206+
207+
/**
208+
* Get URL for a specific deployment.
209+
*/
210+
public async getUrl(label: string): Promise<string | undefined> {
211+
const map = await this.getSessionAuthMap();
212+
return map[label]?.url;
203213
}
204214

205215
/**
206216
* Set session token for a specific deployment.
207217
*/
208218
public async setSessionToken(
209219
label: string,
210-
auth: { url: string; sessionToken: string } | undefined,
220+
auth: { url: string; token: string } | undefined,
211221
): Promise<void> {
212222
await this.updateSessionAuthMap((map) => {
213223
if (auth === undefined) {
@@ -406,7 +416,7 @@ export class SecretsManager {
406416
const sessionAuthMap: SessionAuthMap = {
407417
[label]: {
408418
url: url,
409-
sessionToken: oldToken,
419+
token: oldToken,
410420
},
411421
};
412422

‎src/extension.ts‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -153,20 +153,12 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
153153
output.debug("Registering auth listener for deployment", deploymentLabel);
154154
authChangeDisposable = secretsManager.onDidChangeDeploymentAuth(
155155
deploymentLabel,
156-
async (auth) => {
157-
const token = auth?.sessionToken ?? "";
156+
(auth) => {
157+
const token = auth?.token ?? "";
158158
client.setSessionToken(token);
159159

160160
// Update authentication context for current deployment
161161
contextManager.set("coder.authenticated", auth !== undefined);
162-
163-
output.debug("Session token changed, syncing state");
164-
165-
if (auth?.url) {
166-
const cliManager = serviceContainer.getCliManager();
167-
await cliManager.configure(deploymentLabel, auth.url, token);
168-
output.debug("Updated CLI config with new token");
169-
}
170162
},
171163
);
172164
};
@@ -187,7 +179,6 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
187179
return;
188180
}
189181

190-
const cliManager = serviceContainer.getCliManager();
191182
if (uri.path === "/open") {
192183
const owner = params.get("owner");
193184
const workspace = params.get("workspace");
@@ -234,13 +225,10 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
234225
client.setSessionToken(token);
235226
await secretsManager.setSessionToken(label, {
236227
url,
237-
sessionToken: token,
228+
token,
238229
});
239230
}
240231

241-
// Store on disk to be used by the cli.
242-
await cliManager.configure(toSafeHost(url), url, token);
243-
244232
vscode.commands.executeCommand(
245233
"coder.open",
246234
owner,
@@ -313,8 +301,13 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
313301
? params.get("token")
314302
: (params.get("token") ?? "");
315303

316-
// Store on disk to be used by the cli.
317-
await cliManager.configure(toSafeHost(url), url, token);
304+
if (token) {
305+
client.setSessionToken(token);
306+
await secretsManager.setSessionToken(label, {
307+
url,
308+
token,
309+
});
310+
}
318311

319312
vscode.commands.executeCommand(
320313
"coder.openDevContainer",

‎src/login/loginCoordinator.ts‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,8 @@ export class LoginCoordinator {
6565
options: LoginOptions,
6666
): Promise<LoginResult> {
6767
return this.executeWithGuard(options.label, () => {
68-
const {
69-
url,
70-
label,
71-
detailPrefix: reason,
72-
message,
73-
oauthSessionManager,
74-
} = options;
68+
const { url, label, detailPrefix, message, oauthSessionManager } =
69+
options;
7570

7671
// Show dialog promise
7772
const dialogPromise = this.vscodeProposed.window
@@ -81,8 +76,8 @@ export class LoginCoordinator {
8176
modal: true,
8277
useCustom: true,
8378
detail:
84-
(reason || `Authentication needed for ${label}`) +
85-
" If you've already logged in, you may close this dialog.",
79+
(detailPrefix || `Authentication needed for ${label}.`) +
80+
"\n\nIf you've already logged in, you may close this dialog.",
8681
},
8782
"Login",
8883
)
@@ -91,13 +86,22 @@ export class LoginCoordinator {
9186
// User clicked login - proceed with login flow
9287
const existingToken =
9388
await this.secretsManager.getSessionToken(label);
94-
return this.attemptLogin(
89+
const result = await this.attemptLogin(
9590
label,
9691
url,
9792
existingToken,
9893
false,
9994
oauthSessionManager,
10095
);
96+
97+
if (result.success && result.token) {
98+
await this.secretsManager.setSessionToken(label, {
99+
url,
100+
token: result.token,
101+
});
102+
}
103+
104+
return result;
101105
} else {
102106
// User cancelled
103107
return { success: false };
@@ -139,9 +143,9 @@ export class LoginCoordinator {
139143
const disposable = this.secretsManager.onDidChangeDeploymentAuth(
140144
label,
141145
(auth) => {
142-
if (auth?.sessionToken) {
146+
if (auth?.token) {
143147
disposable.dispose();
144-
resolve({ success: true });
148+
resolve({ success: true, token: auth.token });
145149
}
146150
},
147151
);

‎src/oauth/sessionManager.ts‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,7 @@ export class OAuthSessionManager implements vscode.Disposable {
140140

141141
private async clearTokenState(): Promise<void> {
142142
this.clearInMemoryTokens();
143-
await this.secretsManager.setOAuthTokens(this.label, undefined);
144-
await this.secretsManager.setOAuthClientRegistration(this.label, undefined);
143+
await this.secretsManager.clearOAuthData(this.label);
145144
}
146145

147146
private clearInMemoryTokens(): void {
@@ -274,6 +273,9 @@ export class OAuthSessionManager implements vscode.Disposable {
274273
}
275274

276275
public async setDeployment(label: string, url: string): Promise<void> {
276+
if (label === this.label && url === this.deploymentUrl) {
277+
return; // Nothing to do
278+
}
277279
this.logger.debug("Switching OAuth deployment", { label, url });
278280
this.label = label;
279281
this.deploymentUrl = url;
@@ -601,7 +603,7 @@ export class OAuthSessionManager implements vscode.Disposable {
601603
await this.secretsManager.setOAuthTokens(this.label, tokens);
602604
await this.secretsManager.setSessionToken(this.label, {
603605
url: this.deploymentUrl,
604-
sessionToken: tokenResponse.access_token,
606+
token: tokenResponse.access_token,
605607
});
606608

607609
this.logger.info("Tokens saved", {
@@ -719,19 +721,12 @@ export class OAuthSessionManager implements vscode.Disposable {
719721
await this.clearTokenState();
720722
await this.secretsManager.setSessionToken(this.label, undefined);
721723

722-
const result = await this.loginCoordinator.promptForLoginWithDialog({
724+
await this.loginCoordinator.promptForLoginWithDialog({
723725
url: this.deploymentUrl,
724726
label: this.label,
725727
detailPrefix: errorMessage,
726728
oauthSessionManager: this,
727729
});
728-
729-
if (result.token) {
730-
await this.secretsManager.setSessionToken(this.label, {
731-
url: this.deploymentUrl,
732-
sessionToken: result.token,
733-
});
734-
}
735730
}
736731

737732
/**

0 commit comments

Comments
 (0)