Skip to content

Commit 85f0290

Browse files
committed
Improve login experience
1 parent 423742c commit 85f0290

File tree

7 files changed

+47
-46
lines changed

7 files changed

+47
-46
lines changed

src/commands.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ export class Commands {
8888
this.logger.info("Using deployment label", label);
8989

9090
const result = await this.loginCoordinator.promptForLogin({
91-
deployment: { url, label },
91+
label,
92+
url,
9293
autoLogin: args?.autoLogin,
9394
oauthSessionManager: this.oauthSessionManager,
9495
});

src/core/container.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export class ServiceContainer implements vscode.Disposable {
4444
this.contextManager = new ContextManager();
4545
this.loginCoordinator = new LoginCoordinator(
4646
this.secretsManager,
47+
this.mementoManager,
4748
this.vscodeProposed,
4849
this.logger,
4950
);

src/extension.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
385385
switch (state) {
386386
case AuthAction.LOGIN: {
387387
const url = mementoManager.getUrl();
388-
// Should login the user directly if the URL+Token are valid
388+
// Should login the user directly if the URL and the stored token are valid
389389
await commands.login({ url });
390390

391391
// Re-register auth listener for the new deployment
@@ -432,6 +432,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
432432
// workspaces belonging to this deployment.
433433
client.setHost(details.url);
434434
client.setSessionToken(details.token);
435+
mementoManager.setUrl(details.url);
435436
await oauthSessionManager.setDeployment({
436437
label: details.label,
437438
url: details.url,
@@ -539,11 +540,10 @@ async function migrateAuthStorage(
539540
// Get deployment URL from memento
540541
const url = mementoManager.getUrl();
541542
if (!url) {
542-
output.info("No URL configured, skipping migration");
543+
output.debug("No URL configured, skipping migration");
543544
return;
544545
}
545546

546-
// Perform migration using SecretsManager method
547547
const label = toSafeHost(url);
548548
const migrated = await secretsManager.migrateFromLegacyStorage(url, label);
549549

@@ -556,7 +556,6 @@ async function migrateAuthStorage(
556556
output.error(
557557
`Auth storage migration failed: ${error}. You may need to log in again.`,
558558
);
559-
// Don't throw - allow extension to continue
560559
}
561560
}
562561

src/login/loginCoordinator.ts

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,30 @@
11
import { getErrorMessage } from "coder/site/src/api/errors";
22
import * as vscode from "vscode";
33

4-
import { type Deployment } from "src/core/deployment";
5-
64
import { CoderApi } from "../api/coderApi";
75
import { needToken } from "../api/utils";
6+
import { type Deployment } from "../core/deployment";
7+
import { type MementoManager } from "../core/mementoManager";
88
import { type SecretsManager } from "../core/secretsManager";
99
import { CertificateError } from "../error";
1010
import { type Logger } from "../logging/logger";
11-
import { maybeAskAuthMethod } from "../promptUtils";
11+
import { maybeAskAuthMethod, maybeAskUrl } from "../promptUtils";
1212

1313
import type { User } from "coder/site/src/api/typesGenerated";
1414

1515
import type { OAuthSessionManager } from "../oauth/sessionManager";
1616

17-
export interface LoginResult {
17+
interface LoginResult {
1818
success: boolean;
1919
user?: User;
2020
token?: string;
2121
}
2222

23-
export interface LoginOptions {
24-
deployment: Deployment;
23+
interface LoginOptions {
24+
label: string;
25+
url: string | undefined;
2526
oauthSessionManager: OAuthSessionManager;
2627
autoLogin?: boolean;
27-
message?: string;
28-
detailPrefix?: string;
2928
}
3029

3130
/**
@@ -36,6 +35,7 @@ export class LoginCoordinator {
3635

3736
constructor(
3837
private readonly secretsManager: SecretsManager,
38+
private readonly mementoManager: MementoManager,
3939
private readonly vscodeProposed: typeof vscode,
4040
private readonly logger: Logger,
4141
) {}
@@ -44,12 +44,12 @@ export class LoginCoordinator {
4444
* Direct login - for user-initiated login via commands.
4545
*/
4646
public async promptForLogin(
47-
options: Omit<LoginOptions, "message" | "detailPrefix">,
47+
options: LoginOptions & { url: string },
4848
): Promise<LoginResult> {
49-
const { deployment, oauthSessionManager } = options;
50-
return this.executeWithGuard(options.deployment.label, async () => {
49+
const { label, url, oauthSessionManager } = options;
50+
return this.executeWithGuard(label, async () => {
5151
return this.attemptLogin(
52-
deployment,
52+
{ label, url },
5353
options.autoLogin ?? false,
5454
oauthSessionManager,
5555
);
@@ -60,10 +60,10 @@ export class LoginCoordinator {
6060
* Shows dialog then login - for system-initiated auth (remote, OAuth refresh).
6161
*/
6262
public async promptForLoginWithDialog(
63-
options: LoginOptions,
63+
options: LoginOptions & { message?: string; detailPrefix?: string },
6464
): Promise<LoginResult> {
65-
const { deployment, detailPrefix, message, oauthSessionManager } = options;
66-
return this.executeWithGuard(deployment.label, () => {
65+
const { label, url, detailPrefix, message, oauthSessionManager } = options;
66+
return this.executeWithGuard(label, () => {
6767
// Show dialog promise
6868
const dialogPromise = this.vscodeProposed.window
6969
.showErrorMessage(
@@ -72,24 +72,32 @@ export class LoginCoordinator {
7272
modal: true,
7373
useCustom: true,
7474
detail:
75-
(detailPrefix ||
76-
`Authentication needed for ${deployment.label}.`) +
75+
(detailPrefix || `Authentication needed for ${label}.`) +
7776
"\n\nIf you've already logged in, you may close this dialog.",
7877
},
7978
"Login",
8079
)
8180
.then(async (action) => {
8281
if (action === "Login") {
83-
// User clicked login - proceed with login flow
82+
// Proceed with the login flow, handling logging in from another window
83+
const newUrl = await maybeAskUrl(
84+
this.mementoManager,
85+
url || (await this.secretsManager.getUrl(label)), // TODO can we assume that "https://<label>" is always valid (wouldn't work if it's http)?
86+
label,
87+
);
88+
if (!newUrl) {
89+
throw new Error("URL must be provided");
90+
}
91+
8492
const result = await this.attemptLogin(
85-
deployment,
93+
{ url: newUrl, label },
8694
false,
8795
oauthSessionManager,
8896
);
8997

9098
if (result.success && result.token) {
91-
await this.secretsManager.setSessionAuth(deployment.label, {
92-
url: deployment.url,
99+
await this.secretsManager.setSessionAuth(label, {
100+
url: newUrl,
93101
token: result.token,
94102
});
95103
}
@@ -102,10 +110,7 @@ export class LoginCoordinator {
102110
});
103111

104112
// Race between user clicking login and cross-window detection
105-
return Promise.race([
106-
dialogPromise,
107-
this.waitForCrossWindowLogin(deployment.label),
108-
]);
113+
return Promise.race([dialogPromise, this.waitForCrossWindowLogin(label)]);
109114
});
110115
}
111116

src/oauth/sessionManager.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ export class OAuthSessionManager implements vscode.Disposable {
149149
}
150150

151151
this.storedTokens = tokens;
152-
this.logger.info(`Loaded stored OAuth tokens for ${tokens.deployment_url}`);
152+
this.logger.info(`Loaded stored OAuth tokens for ${this.deployment.label}`);
153153
}
154154

155155
private async clearOAuthState(): Promise<void> {
@@ -749,7 +749,8 @@ export class OAuthSessionManager implements vscode.Disposable {
749749
await this.secretsManager.clearAllAuthData(deployment.label);
750750

751751
await this.loginCoordinator.promptForLoginWithDialog({
752-
deployment,
752+
label: deployment.label,
753+
url: deployment.url,
753754
detailPrefix: errorMessage,
754755
oauthSessionManager: this,
755756
});

src/promptUtils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ async function askURL(
7373
.get<string>("coder.defaultUrl")
7474
?.trim();
7575
const quickPick = vscode.window.createQuickPick();
76+
quickPick.ignoreFocusOut = true;
7677
quickPick.value =
7778
selection ||
7879
lastUsedUrl ||

src/remote/remote.ts

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import { Inbox } from "../inbox";
3636
import { type Logger } from "../logging/logger";
3737
import { type LoginCoordinator } from "../login/loginCoordinator";
3838
import { OAuthSessionManager } from "../oauth/sessionManager";
39-
import { maybeAskUrl } from "../promptUtils";
4039
import {
4140
AuthorityPrefix,
4241
escapeCommandArg,
@@ -138,9 +137,13 @@ export class Remote {
138137
);
139138
disposables.push(remoteOAuthManager);
140139

141-
const promptForLoginAndRetry = async (message: string, url: string) => {
140+
const promptForLoginAndRetry = async (
141+
message: string,
142+
url: string | undefined,
143+
) => {
142144
const result = await this.loginCoordinator.promptForLoginWithDialog({
143-
deployment: { url, label: parts.label },
145+
label: parts.label,
146+
url,
144147
message,
145148
detailPrefix: `You must log in to access ${workspaceName}.`,
146149
oauthSessionManager: remoteOAuthManager,
@@ -160,17 +163,7 @@ export class Remote {
160163
!baseUrlRaw ||
161164
(!token && needToken(vscode.workspace.getConfiguration()))
162165
) {
163-
const mementoManager = this.serviceContainer.getMementoManager();
164-
const newUrl = await maybeAskUrl(
165-
mementoManager,
166-
baseUrlRaw, // TODO can we assume that "https://<parts.label>" is always valid?
167-
parts.label,
168-
);
169-
if (!newUrl) {
170-
throw new Error("URL must be provided");
171-
}
172-
173-
return promptForLoginAndRetry("You are not logged in...", newUrl);
166+
return promptForLoginAndRetry("You are not logged in...", baseUrlRaw);
174167
}
175168

176169
this.logger.info("Using deployment URL", baseUrlRaw);

0 commit comments

Comments
 (0)