Skip to content

Commit 8e5ad5d

Browse files
EhabYEhabY
committed
Rely on the secrets API + Refactorings
1 parent 125dd85 commit 8e5ad5d

File tree

8 files changed

+105
-106
lines changed

8 files changed

+105
-106
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
@@ -721,8 +721,7 @@ export class CliManager {
721721
): Promise<void> {
722722
if (url) {
723723
const urlPath = this.pathResolver.getUrlPath(label);
724-
await fs.mkdir(path.dirname(urlPath), { recursive: true });
725-
await fs.writeFile(urlPath, url);
724+
await this.atomicWriteFile(urlPath, url);
726725
}
727726
}
728727

@@ -739,30 +738,27 @@ export class CliManager {
739738
) {
740739
if (token !== null) {
741740
const tokenPath = this.pathResolver.getSessionTokenPath(label);
742-
await fs.mkdir(path.dirname(tokenPath), { recursive: true });
743-
await fs.writeFile(tokenPath, token ?? "");
741+
await this.atomicWriteFile(tokenPath, token ?? "");
744742
}
745743
}
746744

747745
/**
748-
* Read the CLI config for a deployment with the provided label.
749-
*
750-
* IF a config file does not exist, return an empty string.
751-
*
752-
* If the label is empty, read the old deployment-unaware config.
746+
* Atomically write content to a file by writing to a temporary file first,
747+
* then renaming it.
753748
*/
754-
public async readConfig(
755-
label: string,
756-
): Promise<{ url: string; token: string }> {
757-
const urlPath = this.pathResolver.getUrlPath(label);
758-
const tokenPath = this.pathResolver.getSessionTokenPath(label);
759-
const [url, token] = await Promise.allSettled([
760-
fs.readFile(urlPath, "utf8"),
761-
fs.readFile(tokenPath, "utf8"),
762-
]);
763-
return {
764-
url: url.status === "fulfilled" ? url.value.trim() : "",
765-
token: token.status === "fulfilled" ? token.value.trim() : "",
766-
};
749+
private async atomicWriteFile(
750+
filePath: string,
751+
content: string,
752+
): Promise<void> {
753+
await fs.mkdir(path.dirname(filePath), { recursive: true });
754+
const tempPath =
755+
filePath + ".temp-" + Math.random().toString(36).substring(8);
756+
try {
757+
await fs.writeFile(tempPath, content);
758+
await fs.rename(tempPath, filePath);
759+
} catch (err) {
760+
await fs.rm(tempPath, { force: true }).catch(() => {});
761+
throw err;
762+
}
767763
}
768764
}

‎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)