Skip to content

Commit 20bc214

Browse files
committed
Improve consistency in client-side login/logout experience
1 parent 52df12c commit 20bc214

File tree

4 files changed

+71
-49
lines changed

4 files changed

+71
-49
lines changed

src/commands.ts

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -166,31 +166,32 @@ export class Commands {
166166
}
167167

168168
/**
169-
* Log into the provided deployment. If the deployment URL is not specified,
169+
* Log into the provided deployment. If the deployment URL is not specified,
170170
* ask for it first with a menu showing recent URLs along with the default URL
171171
* and CODER_URL, if those are set.
172172
*/
173-
public async login(...args: string[]): Promise<void> {
174-
// Destructure would be nice but VS Code can pass undefined which errors.
175-
const inputUrl = args[0];
176-
const inputToken = args[1];
177-
const inputLabel = args[2];
178-
const isAutologin =
179-
typeof args[3] === "undefined" ? false : Boolean(args[3]);
180-
181-
const url = await this.maybeAskUrl(inputUrl);
173+
public async login(args?: {
174+
url?: string;
175+
token?: string;
176+
label?: string;
177+
autoLogin?: boolean;
178+
}): Promise<void> {
179+
const url = await this.maybeAskUrl(args?.url);
182180
if (!url) {
183181
return; // The user aborted.
184182
}
185183

186184
// It is possible that we are trying to log into an old-style host, in which
187185
// case we want to write with the provided blank label instead of generating
188186
// a host label.
189-
const label =
190-
typeof inputLabel === "undefined" ? toSafeHost(url) : inputLabel;
187+
const label = args?.label === undefined ? toSafeHost(url) : args?.label;
191188

192189
// Try to get a token from the user, if we need one, and their user.
193-
const res = await this.maybeAskToken(url, inputToken, isAutologin);
190+
const res = await this.maybeAskToken(
191+
url,
192+
args?.token,
193+
args?.autoLogin === true,
194+
);
194195
if (!res) {
195196
return; // The user aborted, or unable to auth.
196197
}
@@ -244,21 +245,23 @@ export class Commands {
244245
*/
245246
private async maybeAskToken(
246247
url: string,
247-
token: string,
248-
isAutologin: boolean,
248+
token: string | undefined,
249+
isAutoLogin: boolean,
249250
): Promise<{ user: User; token: string } | null> {
250251
const client = CoderApi.create(url, token, this.logger, () =>
251252
vscode.workspace.getConfiguration(),
252253
);
253-
if (!needToken(vscode.workspace.getConfiguration())) {
254-
try {
255-
const user = await client.getAuthenticatedUser();
256-
// For non-token auth, we write a blank token since the `vscodessh`
257-
// command currently always requires a token file.
258-
return { token: "", user };
259-
} catch (err) {
254+
const needsToken = needToken(vscode.workspace.getConfiguration());
255+
try {
256+
const user = await client.getAuthenticatedUser();
257+
// For non-token auth, we write a blank token since the `vscodessh`
258+
// command currently always requires a token file.
259+
// For token auth, we have valid access so we can just return the user here
260+
return { token: needsToken && token ? token : "", user };
261+
} catch (err) {
262+
if (!needToken(vscode.workspace.getConfiguration())) {
260263
const message = getErrorMessage(err, "no response from the server");
261-
if (isAutologin) {
264+
if (isAutoLogin) {
262265
this.logger.warn("Failed to log in to Coder server:", message);
263266
} else {
264267
this.vscodeProposed.window.showErrorMessage(
@@ -290,6 +293,9 @@ export class Commands {
290293
value: token || (await this.secretsManager.getSessionToken()),
291294
ignoreFocusOut: true,
292295
validateInput: async (value) => {
296+
if (!value) {
297+
return null;
298+
}
293299
client.setSessionToken(value);
294300
try {
295301
user = await client.getAuthenticatedUser();
@@ -358,7 +364,10 @@ export class Commands {
358364
// Sanity check; command should not be available if no url.
359365
throw new Error("You are not logged in");
360366
}
367+
await this.forceLogout();
368+
}
361369

370+
public async forceLogout(): Promise<void> {
362371
// Clear from the REST client. An empty url will indicate to other parts of
363372
// the code that we are logged out.
364373
this.restClient.setHost("");
@@ -377,7 +386,7 @@ export class Commands {
377386
.showInformationMessage("You've been logged out of Coder!", "Login")
378387
.then((action) => {
379388
if (action === "Login") {
380-
vscode.commands.executeCommand("coder.login");
389+
this.login();
381390
}
382391
});
383392

src/core/secretsManager.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import type { SecretStorage } from "vscode";
1+
import type { SecretStorage, Disposable } from "vscode";
2+
3+
const SESSION_TOKEN_KEY = "sessionToken";
24

35
export class SecretsManager {
46
constructor(private readonly secrets: SecretStorage) {}
@@ -8,9 +10,9 @@ export class SecretsManager {
810
*/
911
public async setSessionToken(sessionToken?: string): Promise<void> {
1012
if (!sessionToken) {
11-
await this.secrets.delete("sessionToken");
13+
await this.secrets.delete(SESSION_TOKEN_KEY);
1214
} else {
13-
await this.secrets.store("sessionToken", sessionToken);
15+
await this.secrets.store(SESSION_TOKEN_KEY, sessionToken);
1416
}
1517
}
1618

@@ -19,11 +21,22 @@ export class SecretsManager {
1921
*/
2022
public async getSessionToken(): Promise<string | undefined> {
2123
try {
22-
return await this.secrets.get("sessionToken");
23-
} catch (ex) {
24+
return await this.secrets.get(SESSION_TOKEN_KEY);
25+
} catch {
2426
// The VS Code session store has become corrupt before, and
2527
// will fail to get the session token...
2628
return undefined;
2729
}
2830
}
31+
32+
/**
33+
* Subscribe to changes to the session token which can be used to indicate user login status.
34+
*/
35+
public onDidChangeSessionToken(listener: () => Promise<void>): Disposable {
36+
return this.secrets.onDidChange((e) => {
37+
if (e.key === SESSION_TOKEN_KEY) {
38+
listener();
39+
}
40+
});
41+
}
2942
}

src/extension.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,21 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
309309
commands.viewLogs.bind(commands),
310310
);
311311

312+
ctx.subscriptions.push(
313+
secretsManager.onDidChangeSessionToken(async () => {
314+
const token = await secretsManager.getSessionToken();
315+
const url = mementoManager.getUrl();
316+
if (!token) {
317+
output.info("Logging out");
318+
await commands.forceLogout();
319+
} else if (url) {
320+
output.info("Logging in");
321+
// Should login the user directly if the URL+Token are valid
322+
await commands.login({ url, token });
323+
}
324+
}),
325+
);
326+
312327
// Since the "onResolveRemoteAuthority:ssh-remote" activation event exists
313328
// in package.json we're able to perform actions before the authority is
314329
// resolved by the remote SSH extension.
@@ -423,15 +438,10 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
423438
// Handle autologin, if not already logged in.
424439
const cfg = vscode.workspace.getConfiguration();
425440
if (cfg.get("coder.autologin") === true) {
426-
const defaultUrl = cfg.get("coder.defaultUrl") || process.env.CODER_URL;
441+
const defaultUrl =
442+
cfg.get<string>("coder.defaultUrl") || process.env.CODER_URL;
427443
if (defaultUrl) {
428-
vscode.commands.executeCommand(
429-
"coder.login",
430-
defaultUrl,
431-
undefined,
432-
undefined,
433-
"true",
434-
);
444+
commands.login({ url: defaultUrl, autoLogin: true });
435445
}
436446
}
437447
}

src/remote.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,7 @@ export class Remote {
241241
await this.closeRemote();
242242
} else {
243243
// Log in then try again.
244-
await vscode.commands.executeCommand(
245-
"coder.login",
246-
baseUrlRaw,
247-
undefined,
248-
parts.label,
249-
);
244+
await this.commands.login({ url: baseUrlRaw, label: parts.label });
250245
await this.setup(remoteAuthority, firstConnect);
251246
}
252247
return;
@@ -366,12 +361,7 @@ export class Remote {
366361
if (!result) {
367362
await this.closeRemote();
368363
} else {
369-
await vscode.commands.executeCommand(
370-
"coder.login",
371-
baseUrlRaw,
372-
undefined,
373-
parts.label,
374-
);
364+
await this.commands.login({ url: baseUrlRaw, label: parts.label });
375365
await this.setup(remoteAuthority, firstConnect);
376366
}
377367
return;

0 commit comments

Comments
 (0)