Skip to content

Commit ff3919b

Browse files
committed
Improve consistency in client-side login/logout experience
1 parent bd09ae6 commit ff3919b

File tree

4 files changed

+66
-44
lines changed

4 files changed

+66
-44
lines changed

src/commands.ts

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

161161
/**
162-
* Log into the provided deployment. If the deployment URL is not specified,
162+
* Log into the provided deployment. If the deployment URL is not specified,
163163
* ask for it first with a menu showing recent URLs along with the default URL
164164
* and CODER_URL, if those are set.
165165
*/
166-
public async login(...args: string[]): Promise<void> {
167-
// Destructure would be nice but VS Code can pass undefined which errors.
168-
const inputUrl = args[0];
169-
const inputToken = args[1];
170-
const inputLabel = args[2];
171-
const isAutologin =
172-
typeof args[3] === "undefined" ? false : Boolean(args[3]);
173-
174-
const url = await this.maybeAskUrl(inputUrl);
166+
public async login(args?: {
167+
url?: string;
168+
token?: string;
169+
label?: string;
170+
autoLogin?: boolean;
171+
}): Promise<void> {
172+
const url = await this.maybeAskUrl(args?.url);
175173
if (!url) {
176174
return; // The user aborted.
177175
}
178176

179177
// It is possible that we are trying to log into an old-style host, in which
180178
// case we want to write with the provided blank label instead of generating
181179
// a host label.
182-
const label =
183-
typeof inputLabel === "undefined" ? toSafeHost(url) : inputLabel;
180+
const label = args?.label === undefined ? toSafeHost(url) : args?.label;
184181

185182
// Try to get a token from the user, if we need one, and their user.
186-
const res = await this.maybeAskToken(url, inputToken, isAutologin);
183+
const res = await this.maybeAskToken(
184+
url,
185+
args?.token,
186+
args?.autoLogin === true,
187+
);
187188
if (!res) {
188189
return; // The user aborted, or unable to auth.
189190
}
@@ -237,21 +238,23 @@ export class Commands {
237238
*/
238239
private async maybeAskToken(
239240
url: string,
240-
token: string,
241-
isAutologin: boolean,
241+
token: string | undefined,
242+
isAutoLogin: boolean,
242243
): Promise<{ user: User; token: string } | null> {
243244
const client = CoderApi.create(url, token, this.storage.output, () =>
244245
vscode.workspace.getConfiguration(),
245246
);
246-
if (!needToken(vscode.workspace.getConfiguration())) {
247-
try {
248-
const user = await client.getAuthenticatedUser();
249-
// For non-token auth, we write a blank token since the `vscodessh`
250-
// command currently always requires a token file.
251-
return { token: "", user };
252-
} catch (err) {
247+
const needsToken = needToken(vscode.workspace.getConfiguration());
248+
try {
249+
const user = await client.getAuthenticatedUser();
250+
// For non-token auth, we write a blank token since the `vscodessh`
251+
// command currently always requires a token file.
252+
// For token auth, we have valid access so we can just return the user here
253+
return { token: needsToken && token ? token : "", user };
254+
} catch (err) {
255+
if (!needToken(vscode.workspace.getConfiguration())) {
253256
const message = getErrorMessage(err, "no response from the server");
254-
if (isAutologin) {
257+
if (isAutoLogin) {
255258
this.storage.output.warn(
256259
"Failed to log in to Coder server:",
257260
message,
@@ -286,6 +289,9 @@ export class Commands {
286289
value: token || (await this.storage.getSessionToken()),
287290
ignoreFocusOut: true,
288291
validateInput: async (value) => {
292+
if (!value) {
293+
return null;
294+
}
289295
client.setSessionToken(value);
290296
try {
291297
user = await client.getAuthenticatedUser();
@@ -354,7 +360,10 @@ export class Commands {
354360
// Sanity check; command should not be available if no url.
355361
throw new Error("You are not logged in");
356362
}
363+
await this.forceLogout();
364+
}
357365

366+
public async forceLogout(): Promise<void> {
358367
// Clear from the REST client. An empty url will indicate to other parts of
359368
// the code that we are logged out.
360369
this.restClient.setHost("");
@@ -373,7 +382,7 @@ export class Commands {
373382
.showInformationMessage("You've been logged out of Coder!", "Login")
374383
.then((action) => {
375384
if (action === "Login") {
376-
vscode.commands.executeCommand("coder.login");
385+
this.login();
377386
}
378387
});
379388

src/extension.ts

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

300+
ctx.subscriptions.push(
301+
storage.onDidChangeSessionToken(async () => {
302+
const token = await storage.getSessionToken();
303+
const url = storage.getUrl();
304+
if (!token) {
305+
output.info("Logging out");
306+
await commands.forceLogout();
307+
} else if (url) {
308+
output.info("Logging in");
309+
// Should login the user directly if the URL+Token are valid
310+
await commands.login({ url, token });
311+
}
312+
}),
313+
);
314+
300315
// Since the "onResolveRemoteAuthority:ssh-remote" activation event exists
301316
// in package.json we're able to perform actions before the authority is
302317
// resolved by the remote SSH extension.
@@ -409,15 +424,10 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
409424
// Handle autologin, if not already logged in.
410425
const cfg = vscode.workspace.getConfiguration();
411426
if (cfg.get("coder.autologin") === true) {
412-
const defaultUrl = cfg.get("coder.defaultUrl") || process.env.CODER_URL;
427+
const defaultUrl =
428+
cfg.get<string>("coder.defaultUrl") || process.env.CODER_URL;
413429
if (defaultUrl) {
414-
vscode.commands.executeCommand(
415-
"coder.login",
416-
defaultUrl,
417-
undefined,
418-
undefined,
419-
"true",
420-
);
430+
commands.login({ url: defaultUrl, autoLogin: true });
421431
}
422432
}
423433
}

src/remote.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -239,12 +239,7 @@ export class Remote {
239239
await this.closeRemote();
240240
} else {
241241
// Log in then try again.
242-
await vscode.commands.executeCommand(
243-
"coder.login",
244-
baseUrlRaw,
245-
undefined,
246-
parts.label,
247-
);
242+
await this.commands.login({ url: baseUrlRaw, label: parts.label });
248243
await this.setup(remoteAuthority, firstConnect);
249244
}
250245
return;
@@ -361,12 +356,7 @@ export class Remote {
361356
if (!result) {
362357
await this.closeRemote();
363358
} else {
364-
await vscode.commands.executeCommand(
365-
"coder.login",
366-
baseUrlRaw,
367-
undefined,
368-
parts.label,
369-
);
359+
await this.commands.login({ url: baseUrlRaw, label: parts.label });
370360
await this.setup(remoteAuthority, firstConnect);
371361
}
372362
return;

src/storage.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,19 @@ export class Storage {
115115
}
116116
}
117117

118+
/**
119+
* Subscribe to changes to the session token which can be used to indicate user login status.
120+
*/
121+
public onDidChangeSessionToken(
122+
listener: () => Promise<void>,
123+
): vscode.Disposable {
124+
return this.secrets.onDidChange((e) => {
125+
if (e.key === "sessionToken") {
126+
listener();
127+
}
128+
});
129+
}
130+
118131
/**
119132
* Returns the log path for the "Remote - SSH" output panel. There is no VS
120133
* Code API to get the contents of an output panel. We use this to get the

0 commit comments

Comments
 (0)