Skip to content

Commit 24ebec5

Browse files
committed
Refactor OAuth flow: separate authorization from session management
The changes extract the OAuth authorization flow into a dedicated OAuthAuthorizer class: - OAuthAuthorizer handles login: client registration, PKCE, browser flow, token exchange - OAuthSessionManager focuses on token lifecycle: refresh, revocation, interceptor - LoginCoordinator now owns OAuthAuthorizer and stores OAuth tokens in session auth - Removes oauthSessionManager from Commands and UriHandler constructors This separation clarifies responsibilities and simplifies the auth flow.
1 parent cc36b81 commit 24ebec5

File tree

16 files changed

+1315
-1502
lines changed

16 files changed

+1315
-1502
lines changed

src/commands.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import { type DeploymentManager } from "./deployment/deploymentManager";
1919
import { CertificateError } from "./error";
2020
import { type Logger } from "./logging/logger";
2121
import { type LoginCoordinator } from "./login/loginCoordinator";
22-
import { type OAuthSessionManager } from "./oauth/sessionManager";
2322
import { maybeAskAgent, maybeAskUrl } from "./promptUtils";
2423
import { escapeCommandArg, toRemoteAuthority, toSafeHost } from "./util";
2524
import {
@@ -52,7 +51,6 @@ export class Commands {
5251
public constructor(
5352
serviceContainer: ServiceContainer,
5453
private readonly extensionClient: CoderApi,
55-
private readonly oauthSessionManager: OAuthSessionManager,
5654
private readonly deploymentManager: DeploymentManager,
5755
) {
5856
this.vscodeProposed = serviceContainer.getVsCodeProposed();
@@ -107,7 +105,6 @@ export class Commands {
107105
safeHostname,
108106
url,
109107
autoLogin: args?.autoLogin,
110-
oauthSessionManager: this.oauthSessionManager,
111108
});
112109

113110
if (!result.success) {

src/core/container.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export class ServiceContainer implements vscode.Disposable {
4848
this.mementoManager,
4949
this.vscodeProposed,
5050
this.logger,
51+
context.extension.id,
5152
);
5253
}
5354

@@ -89,5 +90,6 @@ export class ServiceContainer implements vscode.Disposable {
8990
dispose(): void {
9091
this.contextManager.dispose();
9192
this.logger.dispose();
93+
this.loginCoordinator.dispose();
9294
}
9395
}

src/extension.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
7373
const oauthSessionManager = OAuthSessionManager.create(
7474
deployment,
7575
serviceContainer,
76-
ctx.extension.id,
7776
);
7877
ctx.subscriptions.push(oauthSessionManager);
7978

@@ -152,19 +151,13 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
152151

153152
// Register globally available commands. Many of these have visibility
154153
// controlled by contexts, see `when` in the package.json.
155-
const commands = new Commands(
156-
serviceContainer,
157-
client,
158-
oauthSessionManager,
159-
deploymentManager,
160-
);
154+
const commands = new Commands(serviceContainer, client, deploymentManager);
161155

162156
ctx.subscriptions.push(
163157
registerUriHandler(
164158
serviceContainer,
165159
deploymentManager,
166160
commands,
167-
oauthSessionManager,
168161
vscodeProposed,
169162
),
170163
vscode.commands.registerCommand(

src/login/loginCoordinator.ts

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,40 +5,48 @@ import * as vscode from "vscode";
55
import { CoderApi } from "../api/coderApi";
66
import { needToken } from "../api/utils";
77
import { CertificateError } from "../error";
8-
import { type OAuthSessionManager } from "../oauth/sessionManager";
8+
import { OAuthAuthorizer } from "../oauth/authorizer";
9+
import { buildOAuthTokenData } from "../oauth/utils";
910
import { maybeAskAuthMethod, maybeAskUrl } from "../promptUtils";
1011

1112
import type { User } from "coder/site/src/api/typesGenerated";
1213

1314
import type { MementoManager } from "../core/mementoManager";
14-
import type { SecretsManager } from "../core/secretsManager";
15+
import type { OAuthTokenData, SecretsManager } from "../core/secretsManager";
1516
import type { Deployment } from "../deployment/types";
1617
import type { Logger } from "../logging/logger";
1718

1819
type LoginResult =
1920
| { success: false }
20-
| { success: true; user: User; token: string };
21+
| { success: true; user: User; token: string; oauth?: OAuthTokenData };
2122

2223
interface LoginOptions {
2324
safeHostname: string;
2425
url: string | undefined;
25-
oauthSessionManager: OAuthSessionManager;
2626
autoLogin?: boolean;
2727
token?: string;
2828
}
2929

3030
/**
3131
* Coordinates login prompts across windows and prevents duplicate dialogs.
3232
*/
33-
export class LoginCoordinator {
33+
export class LoginCoordinator implements vscode.Disposable {
3434
private loginQueue: Promise<unknown> = Promise.resolve();
35+
private readonly oauthAuthorizer: OAuthAuthorizer;
3536

3637
constructor(
3738
private readonly secretsManager: SecretsManager,
3839
private readonly mementoManager: MementoManager,
3940
private readonly vscodeProposed: typeof vscode,
4041
private readonly logger: Logger,
41-
) {}
42+
extensionId: string,
43+
) {
44+
this.oauthAuthorizer = new OAuthAuthorizer(
45+
secretsManager,
46+
logger,
47+
extensionId,
48+
);
49+
}
4250

4351
/**
4452
* Direct login - for user-initiated login via commands.
@@ -47,12 +55,11 @@ export class LoginCoordinator {
4755
public async ensureLoggedIn(
4856
options: LoginOptions & { url: string },
4957
): Promise<LoginResult> {
50-
const { safeHostname, url, oauthSessionManager } = options;
58+
const { safeHostname, url } = options;
5159
return this.executeWithGuard(async () => {
5260
const result = await this.attemptLogin(
5361
{ safeHostname, url },
5462
options.autoLogin ?? false,
55-
oauthSessionManager,
5663
options.token,
5764
);
5865

@@ -68,8 +75,7 @@ export class LoginCoordinator {
6875
public async ensureLoggedInWithDialog(
6976
options: LoginOptions & { message?: string; detailPrefix?: string },
7077
): Promise<LoginResult> {
71-
const { safeHostname, url, detailPrefix, message, oauthSessionManager } =
72-
options;
78+
const { safeHostname, url, detailPrefix, message } = options;
7379
return this.executeWithGuard(async () => {
7480
// Show dialog promise
7581
const dialogPromise = this.vscodeProposed.window
@@ -101,7 +107,6 @@ export class LoginCoordinator {
101107
const result = await this.attemptLogin(
102108
{ url: newUrl, safeHostname },
103109
false,
104-
oauthSessionManager,
105110
options.token,
106111
);
107112

@@ -137,6 +142,7 @@ export class LoginCoordinator {
137142
await this.secretsManager.setSessionAuth(safeHostname, {
138143
url,
139144
token: result.token,
145+
oauth: result.oauth, // undefined for non-OAuth logins
140146
});
141147
await this.mementoManager.addToUrlHistory(url);
142148
}
@@ -195,7 +201,6 @@ export class LoginCoordinator {
195201
private async attemptLogin(
196202
deployment: Deployment,
197203
isAutoLogin: boolean,
198-
oauthSessionManager: OAuthSessionManager,
199204
providedToken?: string,
200205
): Promise<LoginResult> {
201206
const client = CoderApi.create(deployment.url, "", this.logger);
@@ -232,15 +237,9 @@ export class LoginCoordinator {
232237
const authMethod = await maybeAskAuthMethod(client);
233238
switch (authMethod) {
234239
case "oauth":
235-
return this.loginWithOAuth(oauthSessionManager, deployment);
236-
case "legacy": {
237-
const result = await this.loginWithToken(client);
238-
if (result.success) {
239-
// Clear OAuth state since user explicitly chose token auth
240-
await oauthSessionManager.clearOAuthState();
241-
}
242-
return result;
243-
}
240+
return this.loginWithOAuth(deployment);
241+
case "legacy":
242+
return this.loginWithToken(client);
244243
case undefined:
245244
return { success: false }; // User aborted
246245
}
@@ -359,27 +358,29 @@ export class LoginCoordinator {
359358
/**
360359
* OAuth authentication flow.
361360
*/
362-
private async loginWithOAuth(
363-
oauthSessionManager: OAuthSessionManager,
364-
deployment: Deployment,
365-
): Promise<LoginResult> {
361+
private async loginWithOAuth(deployment: Deployment): Promise<LoginResult> {
366362
try {
367363
this.logger.info("Starting OAuth authentication");
368364

369-
const { token, user } = await vscode.window.withProgress(
365+
const { tokenResponse, user } = await vscode.window.withProgress(
370366
{
371367
location: vscode.ProgressLocation.Notification,
372368
title: "Authenticating",
373369
cancellable: true,
374370
},
375-
async (progress, token) =>
376-
await oauthSessionManager.login(deployment, progress, token),
371+
async (progress, cancellationToken) =>
372+
await this.oauthAuthorizer.login(
373+
deployment,
374+
progress,
375+
cancellationToken,
376+
),
377377
);
378378

379379
return {
380380
success: true,
381-
token,
381+
token: tokenResponse.access_token,
382382
user,
383+
oauth: buildOAuthTokenData(tokenResponse),
383384
};
384385
} catch (error) {
385386
const title = "OAuth authentication failed";
@@ -394,4 +395,8 @@ export class LoginCoordinator {
394395
return { success: false };
395396
}
396397
}
398+
399+
public dispose(): void {
400+
this.oauthAuthorizer.dispose();
401+
}
397402
}

0 commit comments

Comments
 (0)