From 032aeb3c4ab1d5dccf6df52eb0e0644fc71e8015 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 7 Nov 2024 15:33:37 -0600 Subject: [PATCH 1/7] Refactored DhcService (151) --- package.json | 12 +- src/common/commands.ts | 7 +- src/controllers/ExtensionController.ts | 44 +++++- src/controllers/PanelController.ts | 16 +- src/controllers/UserLoginController.ts | 93 ++++++++++- src/dh/dhc.ts | 32 ++-- src/services/DhcService.ts | 209 +++++++++++-------------- src/services/DhcServiceFactory.ts | 39 ----- src/services/DheService.ts | 4 +- src/services/SecretService.ts | 47 ++++++ src/services/ServerManager.ts | 47 +++--- src/services/cache/CoreJsApiCache.ts | 15 ++ src/services/cache/index.ts | 1 + src/services/index.ts | 1 - src/types/commonTypes.d.ts | 13 +- src/types/serviceTypes.d.ts | 21 ++- src/util/isInstanceOf.ts | 4 +- src/util/uiUtils.ts | 16 ++ 18 files changed, 394 insertions(+), 227 deletions(-) delete mode 100644 src/services/DhcServiceFactory.ts create mode 100644 src/services/cache/CoreJsApiCache.ts diff --git a/package.json b/package.json index 9096994a..e786de3f 100644 --- a/package.json +++ b/package.json @@ -219,7 +219,11 @@ "icon": "$(refresh)" }, { - "command": "vscode-deephaven.createAuthenticatedClient", + "command": "vscode-deephaven.createCoreAuthenticatedClient", + "title": "Create Authenticated Client" + }, + { + "command": "vscode-deephaven.createDHEAuthenticatedClient", "title": "Create Authenticated Client" }, { @@ -700,7 +704,11 @@ "when": "false" }, { - "command": "vscode-deephaven.createAuthenticatedClient", + "command": "vscode-deephaven.createCoreAuthenticatedClient", + "when": "false" + }, + { + "command": "vscode-deephaven.createDHEAuthenticatedClient", "when": "false" } ], diff --git a/src/common/commands.ts b/src/common/commands.ts index bbbaedef..6a2fac06 100644 --- a/src/common/commands.ts +++ b/src/common/commands.ts @@ -11,7 +11,12 @@ function cmd(cmd: T): `${typeof EXTENSION_ID}.${T}` { export const CLEAR_SECRET_STORAGE_CMD = cmd('clearSecretStorage'); export const CONNECT_TO_SERVER_CMD = cmd('connectToServer'); export const CONNECT_TO_SERVER_OPERATE_AS_CMD = cmd('connectToServerOperateAs'); -export const CREATE_AUTHENTICATED_CLIENT_CMD = cmd('createAuthenticatedClient'); +export const CREATE_CORE_AUTHENTICATED_CLIENT_CMD = cmd( + 'createCoreAuthenticatedClient' +); +export const CREATE_DHE_AUTHENTICATED_CLIENT_CMD = cmd( + 'createDHEAuthenticatedClient' +); export const CREATE_NEW_TEXT_DOC_CMD = cmd('createNewTextDoc'); export const DISCONNECT_EDITOR_CMD = cmd('disconnectEditor'); export const DISCONNECT_FROM_SERVER_CMD = cmd('disconnectFromServer'); diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index ee590fee..59e35dbe 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -38,7 +38,6 @@ import { runSelectedLinesHoverProvider, } from '../providers'; import { - DhcServiceFactory, DheJsApiCache, DheService, DheServiceCache, @@ -47,6 +46,7 @@ import { SecretService, ServerManager, URLMap, + CoreJsApiCache, } from '../services'; import type { ConnectionState, @@ -57,7 +57,7 @@ import type { IDheService, IDheServiceFactory, IDhcService, - IDhServiceFactory, + IDhcServiceFactory, IPanelService, ISecretService, IServerManager, @@ -68,6 +68,9 @@ import type { ServerConnectionTreeView, ServerState, ServerTreeView, + CoreAuthenticatedClient, + ICoreClientFactory, + CoreUnauthenticatedClient, } from '../types'; import { ServerConnectionTreeDragAndDropController } from './ServerConnectionTreeDragAndDropController'; import { ConnectionController } from './ConnectionController'; @@ -115,15 +118,19 @@ export class ExtensionController implements Disposable { readonly _config: IConfigService; private _connectionController: ConnectionController | null = null; + private _coreClientCache: URLMap | null = null; + private _coreClientFactory: ICoreClientFactory | null = null; private _coreCredentialsCache: URLMap> | null = null; + private _coreJsApiCache: IAsyncCacheService | null = + null; private _dheClientCache: URLMap | null = null; private _dheClientFactory: IDheClientFactory | null = null; private _dheServiceCache: IAsyncCacheService | null = null; private _panelController: PanelController | null = null; private _panelService: IPanelService | null = null; private _pipServerController: PipServerController | null = null; - private _dhcServiceFactory: IDhServiceFactory | null = null; + private _dhcServiceFactory: IDhcServiceFactory | null = null; private _dheJsApiCache: IAsyncCacheService | null = null; private _dheServiceFactory: IDheServiceFactory | null = null; private _secretService: ISecretService | null = null; @@ -204,12 +211,12 @@ export class ExtensionController implements Disposable { * Initialize panel controller. */ initializePanelController = (): void => { - assertDefined(this._coreCredentialsCache, 'coreCredentialsCache'); + assertDefined(this._dheClientCache, 'dheClientCache'); assertDefined(this._panelService, 'panelService'); assertDefined(this._serverManager, 'serverManager'); this._panelController = new PanelController( - this._coreCredentialsCache, + this._dheClientCache, this._serverManager, this._panelService ); @@ -237,12 +244,18 @@ export class ExtensionController implements Disposable { * Initialize user login controller. */ initializeUserLoginController = (): void => { + assertDefined(this._coreClientCache, 'coreClientCache'); + assertDefined(this._coreClientFactory, 'coreClientFactory'); + assertDefined(this._coreJsApiCache, 'coreJsApiCache'); assertDefined(this._dheClientCache, 'dheClientCache'); assertDefined(this._dheClientFactory, 'dheClientFactory'); assertDefined(this._secretService, 'secretService'); assertDefined(this._toaster, 'toaster'); this._userLoginController = new UserLoginController( + this._coreClientCache, + this._coreClientFactory, + this._coreJsApiCache, this._dheClientCache, this._dheClientFactory, this._secretService, @@ -308,13 +321,25 @@ export class ExtensionController implements Disposable { initializeServerManager = (): void => { assertDefined(this._pythonDiagnostics, 'pythonDiagnostics'); assertDefined(this._outputChannel, 'outputChannel'); + assertDefined(this._secretService, 'secretService'); assertDefined(this._toaster, 'toaster'); this._coreCredentialsCache = new URLMap>(); + this._coreJsApiCache = new CoreJsApiCache(); + this._context.subscriptions.push(this._coreJsApiCache); + this._dheJsApiCache = new DheJsApiCache(); this._context.subscriptions.push(this._dheJsApiCache); + this._coreClientFactory = async ( + url: URL + ): Promise => { + assertDefined(this._coreJsApiCache, 'coreJsApiCache'); + const dhc = await this._coreJsApiCache.get(url); + return new dhc.CoreClient(url.toString()) as CoreUnauthenticatedClient; + }; + this._dheClientFactory = async ( url: URL ): Promise => { @@ -323,16 +348,18 @@ export class ExtensionController implements Disposable { return createDheClient(dhe, getWsUrl(url)); }; + this._coreClientCache = new URLMap(); this._dheClientCache = new URLMap(); this._panelService = new PanelService(); this._context.subscriptions.push(this._panelService); - this._dhcServiceFactory = new DhcServiceFactory( - this._coreCredentialsCache, + this._dhcServiceFactory = DhcService.factory( + this._coreClientCache, this._panelService, this._pythonDiagnostics, this._outputChannel, + this._secretService, this._toaster ); @@ -349,11 +376,12 @@ export class ExtensionController implements Disposable { this._serverManager = new ServerManager( this._config, - this._coreCredentialsCache, + this._coreClientCache, this._dhcServiceFactory, this._dheClientCache, this._dheServiceCache, this._outputChannel, + this._secretService, this._toaster ); this._context.subscriptions.push(this._serverManager); diff --git a/src/controllers/PanelController.ts b/src/controllers/PanelController.ts index daebfa50..285ea3a0 100644 --- a/src/controllers/PanelController.ts +++ b/src/controllers/PanelController.ts @@ -1,10 +1,9 @@ import * as vscode from 'vscode'; -import type { dh as DhcType } from '@deephaven/jsapi-types'; +import type { AuthenticatedClient as DheAuthenticatedClient } from '@deephaven-enterprise/auth-nodejs'; import type { ConnectionState, IPanelService, IServerManager, - Lazy, VariableDefintion, WorkerURL, } from '../types'; @@ -25,18 +24,19 @@ import { import { waitFor } from '../util/promiseUtils'; import { getEmbedWidgetUrl } from '../dh/dhc'; import { ControllerBase } from './ControllerBase'; +import { getWorkerCredentials } from '../dh/dhe'; const logger = new Logger('PanelController'); export class PanelController extends ControllerBase { constructor( - coreCredentialsCache: URLMap>, + dheClientCache: URLMap, serverManager: IServerManager, panelService: IPanelService ) { super(); - this._coreCredentialsCache = coreCredentialsCache; + this._dheClientCache = dheClientCache; this._panelService = panelService; this._serverManager = serverManager; @@ -53,9 +53,7 @@ export class PanelController extends ControllerBase { ); } - private readonly _coreCredentialsCache: URLMap< - Lazy - >; + private readonly _dheClientCache: URLMap; private readonly _panelService: IPanelService; private readonly _serverManager: IServerManager; @@ -83,8 +81,10 @@ export class PanelController extends ControllerBase { // Respond to login credentials request from DH iframe if (message === DEEPHAVEN_POST_MSG.loginOptionsRequest) { + const dheClient = this._dheClientCache.get(serverOrWorkerUrl); + const credentials = - await this._coreCredentialsCache.get(serverOrWorkerUrl)?.(); + dheClient == null ? null : await getWorkerCredentials(dheClient); if (credentials == null) { logger.error('Failed to get credentials for worker', serverOrWorkerUrl); diff --git a/src/controllers/UserLoginController.ts b/src/controllers/UserLoginController.ts index 2cd12c14..18c9a881 100644 --- a/src/controllers/UserLoginController.ts +++ b/src/controllers/UserLoginController.ts @@ -8,20 +8,30 @@ import { type AuthenticatedClient as DheAuthenticatedClient, type Username, } from '@deephaven-enterprise/auth-nodejs'; +import { type dh as DhcType } from '@deephaven/jsapi-types'; import type { URLMap } from '../services'; import { ControllerBase } from './ControllerBase'; import { - CREATE_AUTHENTICATED_CLIENT_CMD, + CREATE_CORE_AUTHENTICATED_CLIENT_CMD, + CREATE_DHE_AUTHENTICATED_CLIENT_CMD, GENERATE_DHE_KEY_PAIR_CMD, } from '../common'; -import { Logger, runUserLoginWorkflow } from '../util'; +import { Logger, promptForPsk, runUserLoginWorkflow } from '../util'; import type { + CoreAuthenticatedClient, + IAsyncCacheService, + ICoreClientFactory, IDheClientFactory, ISecretService, IToastService, ServerState, } from '../types'; import { hasInteractivePermission } from '../dh/dhe'; +import { + AUTH_HANDLER_TYPE_ANONYMOUS, + AUTH_HANDLER_TYPE_PSK, + loginClient, +} from '../dh/dhc'; const logger = new Logger('UserLoginController'); @@ -30,6 +40,9 @@ const logger = new Logger('UserLoginController'); */ export class UserLoginController extends ControllerBase { constructor( + coreClientCache: URLMap, + coreClientFactory: ICoreClientFactory, + coreJsApiCache: IAsyncCacheService, dheClientCache: URLMap, dheClientFactory: IDheClientFactory, secretService: ISecretService, @@ -37,6 +50,9 @@ export class UserLoginController extends ControllerBase { ) { super(); + this.coreClientCache = coreClientCache; + this.coreClientFactory = coreClientFactory; + this.coreJsApiCache = coreJsApiCache; this.dheClientCache = dheClientCache; this.dheClientFactory = dheClientFactory; this.secretService = secretService; @@ -48,11 +64,19 @@ export class UserLoginController extends ControllerBase { ); this.registerCommand( - CREATE_AUTHENTICATED_CLIENT_CMD, - this.onCreateAuthenticatedClient + CREATE_CORE_AUTHENTICATED_CLIENT_CMD, + this.onCreateCoreAuthenticatedClient + ); + + this.registerCommand( + CREATE_DHE_AUTHENTICATED_CLIENT_CMD, + this.onCreateDHEAuthenticatedClient ); } + private readonly coreClientCache: URLMap; + private readonly coreClientFactory: ICoreClientFactory; + private readonly coreJsApiCache: IAsyncCacheService; private readonly dheClientCache: URLMap; private readonly dheClientFactory: IDheClientFactory; private readonly secretService: ISecretService; @@ -162,12 +186,69 @@ export class UserLoginController extends ControllerBase { }; /** - * Create an authenticated client. + * Create a core authenticated client. + */ + onCreateCoreAuthenticatedClient = async (serverUrl: URL): Promise => { + const client = await this.coreClientFactory(serverUrl); + + const authConfig = new Set( + (await client.getAuthConfigValues()).map(([, value]) => value) + ); + + let credentials: DhcType.LoginCredentials | null = null; + + if (authConfig.has(AUTH_HANDLER_TYPE_ANONYMOUS)) { + const dh = await this.coreJsApiCache.get(serverUrl); + credentials = { + type: dh.CoreClient.LOGIN_TYPE_ANONYMOUS, + }; + } else if (authConfig.has(AUTH_HANDLER_TYPE_PSK)) { + const token = + (await this.secretService.getPsk(serverUrl)) ?? + (await promptForPsk('Enter your Pre-Shared Key')); + + if (token == null) { + this.toast.info('Login cancelled.'); + return; + } + + credentials = { + type: AUTH_HANDLER_TYPE_PSK, + token, + }; + + this.secretService.storePsk(serverUrl, token); + } else { + throw new Error('No supported authentication methods found.'); + } + + try { + this.coreClientCache.set( + serverUrl, + await loginClient(client, credentials) + ); + } catch (err) { + logger.error( + 'An error occurred while connecting to Deephaven server:', + err + ); + this.coreClientCache.delete(serverUrl); + + this.toast.error('Login failed.'); + + if (credentials.type === AUTH_HANDLER_TYPE_PSK) { + await this.secretService.deletePsk(serverUrl); + } + } + }; + + /** + * Create a DHE authenticated client. * @param serverUrl The server URL to create a client for. * @param operateAsAnotherUser Whether to operate as another user. * @returns A promise that resolves when the client has been created or failed. */ - onCreateAuthenticatedClient = async ( + onCreateDHEAuthenticatedClient = async ( serverUrl: URL, operateAsAnotherUser: boolean ): Promise => { diff --git a/src/dh/dhc.ts b/src/dh/dhc.ts index 3b15130b..0dc40ffc 100644 --- a/src/dh/dhc.ts +++ b/src/dh/dhc.ts @@ -1,5 +1,9 @@ import type { dh as DhType } from '@deephaven/jsapi-types'; import { NoConsoleTypesError } from './errorUtils'; +import type { + CoreAuthenticatedClient, + CoreUnauthenticatedClient, +} from '../types'; export const AUTH_HANDLER_TYPE_ANONYMOUS = 'io.deephaven.auth.AnonymousAuthenticationHandler'; @@ -54,22 +58,12 @@ export function getEmbedWidgetUrl({ /** * Initialize a connection and session to a DHC server. - * @param client The client to use for the connection. - * @param credentials The credentials to use for the connection. + * @param client The authenticated client to use for the connection. * @returns A promise that resolves to the connection and session. */ export async function initDhcSession( - client: DhType.CoreClient, - credentials: DhType.LoginCredentials + client: CoreAuthenticatedClient ): Promise> { - try { - await client.login(credentials); - } catch (err) { - // eslint-disable-next-line no-console - console.error(err); - throw err; - } - const cn = await client.getAsIdeConnection(); const [type] = await cn.getConsoleTypes(); @@ -82,3 +76,17 @@ export async function initDhcSession( return { cn, session }; } + +/** + * Login a given unauthenticated client with the given credentials. + * @param client The client to login. + * @param credentials The credentials to use for the login. + * @returns The authenticated client. + */ +export async function loginClient( + client: CoreUnauthenticatedClient, + credentials: DhType.LoginCredentials +): Promise { + await client.login(credentials); + return client as unknown as CoreAuthenticatedClient; +} diff --git a/src/services/DhcService.ts b/src/services/DhcService.ts index 3ded112c..c2c6cd0c 100644 --- a/src/services/DhcService.ts +++ b/src/services/DhcService.ts @@ -1,25 +1,17 @@ import * as vscode from 'vscode'; import type { dh as DhcType } from '@deephaven/jsapi-types'; -import { initDhcApi, isAggregateError } from '@deephaven/require-jsapi'; -import { - formatTimestamp, - getCombinedSelectedLinesText, - getTempDir, - Logger, - urlToDirectoryName, -} from '../util'; -import { - AUTH_HANDLER_TYPE_ANONYMOUS, - AUTH_HANDLER_TYPE_PSK, - initDhcSession, - type ConnectionAndSession, -} from '../dh/dhc'; +import { isAggregateError } from '@deephaven/require-jsapi'; +import { formatTimestamp, getCombinedSelectedLinesText, Logger } from '../util'; +import { initDhcSession, type ConnectionAndSession } from '../dh/dhc'; import type { ConsoleType, + CoreAuthenticatedClient, IDhcService, + IDhcServiceFactory, IPanelService, + ISecretService, IToastService, - Lazy, + Psk, UniqueID, VariableChanges, VariableDefintion, @@ -27,6 +19,7 @@ import type { } from '../types'; import type { URLMap } from './URLMap'; import { + CREATE_CORE_AUTHENTICATED_CLIENT_CMD, OPEN_VARIABLE_PANELS_CMD, REFRESH_VARIABLE_PANELS_CMD, VARIABLE_UNICODE_ICONS, @@ -37,22 +30,50 @@ import { hasErrorCode } from '../util/typeUtils'; const logger = new Logger('DhcService'); export class DhcService implements IDhcService { - constructor( + static factory = ( + coreClientCache: URLMap, + panelService: IPanelService, + diagnosticsCollection: vscode.DiagnosticCollection, + outputChannel: vscode.OutputChannel, + secretService: ISecretService, + toaster: IToastService + ): IDhcServiceFactory => { + return { + create: (serverUrl: URL, tagId?: UniqueID): IDhcService => { + return new DhcService( + serverUrl, + coreClientCache, + panelService, + diagnosticsCollection, + outputChannel, + secretService, + toaster, + tagId + ); + }, + }; + }; + + private constructor( serverUrl: URL, - coreCredentialsCache: URLMap>, + coreClientCache: URLMap, panelService: IPanelService, diagnosticsCollection: vscode.DiagnosticCollection, outputChannel: vscode.OutputChannel, + secretService: ISecretService, toaster: IToastService, tagId?: UniqueID ) { - this.coreCredentialsCache = coreCredentialsCache; + this.coreClientCache = coreClientCache; this.serverUrl = serverUrl; this.panelService = panelService; this.diagnosticsCollection = diagnosticsCollection; this.outputChannel = outputChannel; + this.secretService = secretService; this.toaster = toaster; this.tagId = tagId; + + this.coreClientCache.onDidChange(this.onDidCoreClientCacheInvalidate); } private readonly _onDidDisconnect = new vscode.EventEmitter(); @@ -62,43 +83,45 @@ export class DhcService implements IDhcService { public readonly tagId?: UniqueID; private readonly subscriptions: (() => void)[] = []; - private readonly coreCredentialsCache: URLMap>; + private readonly coreClientCache: URLMap; private readonly outputChannel: vscode.OutputChannel; + private readonly secretService: ISecretService; private readonly toaster: IToastService; private readonly panelService: IPanelService; private readonly diagnosticsCollection: vscode.DiagnosticCollection; - private cachedCreateClient: Promise | null = null; - private cachedCreateSession: Promise< + private clientPromise: Promise | null = null; + private initSessionPromise: Promise< ConnectionAndSession > | null = null; - private cachedInitApi: Promise | null = null; - private dh: typeof DhcType | null = null; private cn: DhcType.IdeConnection | null = null; - private client: DhcType.CoreClient | null = null; private session: DhcType.IdeSession | null = null; get isInitialized(): boolean { - return this.cachedInitApi != null; + return this.initSessionPromise != null; } get isConnected(): boolean { - return this.dh != null && this.cn != null && this.session != null; + return this.cn != null && this.session != null; } + private onDidCoreClientCacheInvalidate = (url: URL): void => { + if (url.toString() === this.serverUrl.toString()) { + // Reset the client promise so that the next call to `getClient` can + // reinitialize it if necessary. + this.clientPromise = null; + } + }; + private clearCaches(): void { - this.cachedCreateClient = null; - this.cachedCreateSession = null; - this.cachedInitApi = null; - this.client = null; + this.clientPromise = null; + this.initSessionPromise = null; if (this.cn != null) { this.cn.close(); this._onDidDisconnect.fire(this.serverUrl); } this.cn = null; - - this.dh = null; this.session = null; this.subscriptions.forEach(dispose => dispose()); @@ -119,62 +142,27 @@ export class DhcService implements IDhcService { return defaultErrorMessage; } - async getPsk(): Promise { - const credentials = await this.coreCredentialsCache.get(this.serverUrl)?.(); - - if (credentials?.type !== AUTH_HANDLER_TYPE_PSK) { - return null; - } - - return credentials.token ?? null; + async getPsk(): Promise { + return this.secretService.getPsk(this.serverUrl); } - async initApi(): Promise { - return initDhcApi( - this.serverUrl, - getTempDir({ subDirectory: urlToDirectoryName(this.serverUrl) }) - ); - } - - async initDh(): Promise { - try { - if (this.cachedInitApi == null) { - this.outputChannel.appendLine( - `Initializing Deephaven API...: ${this.serverUrl}` - ); - this.cachedInitApi = this.initApi(); - } - this.dh = await this.cachedInitApi; + async initSession(): Promise { + const client = await this.getClient(); - this.outputChannel.appendLine( - `Initialized Deephaven API: ${this.serverUrl}` - ); - } catch (err) { - this.clearCaches(); - logger.error(err); - this.outputChannel.appendLine( - `Failed to initialize Deephaven API${err == null ? '.' : `: ${err}`}` - ); - const toastMessage = this.getToastErrorMessage( - err, - 'Failed to initialize Deephaven API' - ); - this.toaster.error(toastMessage); + if (client == null) { + const msg = 'Failed to create Deephaven client'; + logger.error(msg); + this.outputChannel.appendLine(msg); + this.toaster.error(msg); return false; } - if (this.cachedCreateClient == null) { - this.outputChannel.appendLine('Creating client...'); - this.cachedCreateClient = this.createClient(this.dh); - } - this.client = await this.cachedCreateClient; - - if (this.cachedCreateSession == null) { + if (this.initSessionPromise == null) { this.outputChannel.appendLine('Creating session...'); - this.cachedCreateSession = this.createSession(this.dh, this.client); + this.initSessionPromise = initDhcSession(client); try { - const { cn, session } = await this.cachedCreateSession; + const { cn, session } = await this.initSessionPromise; cn.subscribeToFieldUpdates(changes => { this.panelService.updateVariables( @@ -219,7 +207,7 @@ export class DhcService implements IDhcService { } try { - const { cn, session } = await this.cachedCreateSession; + const { cn, session } = await this.initSessionPromise; this.cn = cn; this.session = session; } catch (err) { @@ -243,51 +231,34 @@ export class DhcService implements IDhcService { } } - async createClient(dh: typeof DhcType): Promise { - try { - return new dh.CoreClient(this.serverUrl.toString()); - } catch (err) { - logger.error(err); - throw err; + /** + * Initialize client and login. + * @returns Client or null if initialization failed. + */ + private _initClient = async (): Promise => { + if (!this.coreClientCache.has(this.serverUrl)) { + await vscode.commands.executeCommand( + CREATE_CORE_AUTHENTICATED_CLIENT_CMD, + this.serverUrl + ); } - } - async createSession( - dh: typeof DhcType, - client: DhcType.CoreClient - ): Promise> { - if (!this.coreCredentialsCache.has(this.serverUrl)) { - const authConfig = new Set( - (await client.getAuthConfigValues()).map(([, value]) => value) - ); + const maybeClient = await this.coreClientCache.get(this.serverUrl); - if (authConfig.has(AUTH_HANDLER_TYPE_ANONYMOUS)) { - this.coreCredentialsCache.set(this.serverUrl, async () => ({ - type: dh.CoreClient.LOGIN_TYPE_ANONYMOUS, - })); - } else if (authConfig.has(AUTH_HANDLER_TYPE_PSK)) { - this.coreCredentialsCache.set(this.serverUrl, async () => ({ - type: AUTH_HANDLER_TYPE_PSK, - // TODO: Login flow UI should be a separate concern - // deephaven/vscode-deephaven/issues/151 - token: await vscode.window.showInputBox({ - ignoreFocusOut: true, - placeHolder: 'Pre-Shared Key', - prompt: 'Enter your Deephaven pre-shared key', - password: true, - }), - })); - } + return maybeClient ?? null; + }; + + async getClient(): Promise { + if (this.clientPromise == null) { + this.clientPromise = this._initClient(); } - if (this.coreCredentialsCache.has(this.serverUrl)) { - const credentials = await this.coreCredentialsCache.get( - this.serverUrl - )!(); - return initDhcSession(client, credentials); + const client = await this.clientPromise; + if (client == null) { + this.clientPromise = null; } - throw new Error('No supported authentication methods found.'); + return client; } async dispose(): Promise { @@ -320,7 +291,7 @@ export class DhcService implements IDhcService { this.diagnosticsCollection.set(editor.document.uri, []); if (this.session == null) { - await this.initDh(); + await this.initSession(); } if (this.cn == null || this.session == null) { diff --git a/src/services/DhcServiceFactory.ts b/src/services/DhcServiceFactory.ts deleted file mode 100644 index 4c4f0a9a..00000000 --- a/src/services/DhcServiceFactory.ts +++ /dev/null @@ -1,39 +0,0 @@ -import * as vscode from 'vscode'; -import type { dh as DhcType } from '@deephaven/jsapi-types'; -import { DhcService } from './DhcService'; -import type { - IDhcService, - IDhServiceFactory, - IPanelService, - IToastService, - Lazy, - UniqueID, -} from '../types'; -import type { URLMap } from './URLMap'; - -/** - * Factory for creating DhcService instances. - */ -export class DhcServiceFactory implements IDhServiceFactory { - constructor( - private coreCredentialsCache: URLMap>, - private panelService: IPanelService, - private diagnosticsCollection: vscode.DiagnosticCollection, - private outputChannel: vscode.OutputChannel, - private toaster: IToastService - ) {} - - create = (serverUrl: URL, tagId?: UniqueID): IDhcService => { - const dhService = new DhcService( - serverUrl, - this.coreCredentialsCache, - this.panelService, - this.diagnosticsCollection, - this.outputChannel, - this.toaster, - tagId - ); - - return dhService; - }; -} diff --git a/src/services/DheService.ts b/src/services/DheService.ts index 19ff9cf6..2fcbe5e0 100644 --- a/src/services/DheService.ts +++ b/src/services/DheService.ts @@ -24,7 +24,7 @@ import { getWorkerCredentials, getWorkerInfoFromQuery, } from '../dh/dhe'; -import { CREATE_AUTHENTICATED_CLIENT_CMD } from '../common'; +import { CREATE_DHE_AUTHENTICATED_CLIENT_CMD } from '../common'; const logger = new Logger('DheService'); @@ -116,7 +116,7 @@ export class DheService implements IDheService { ): Promise => { if (!this._dheClientCache.has(this.serverUrl)) { await vscode.commands.executeCommand( - CREATE_AUTHENTICATED_CLIENT_CMD, + CREATE_DHE_AUTHENTICATED_CLIENT_CMD, this.serverUrl, operateAsAnotherUser ); diff --git a/src/services/SecretService.ts b/src/services/SecretService.ts index bb227441..f6235487 100644 --- a/src/services/SecretService.ts +++ b/src/services/SecretService.ts @@ -2,11 +2,13 @@ import type { SecretStorage } from 'vscode'; import type { Username } from '@deephaven-enterprise/auth-nodejs'; import type { ISecretService, + Psk, UserKeyPairs, UserLoginPreferences, } from '../types'; const OPERATE_AS_USER_KEY = 'operateAsUser' as const; +const SERVER_PSK_KEY = 'serverPsk' as const; const SERVER_KEYS_KEY = 'serverKeys' as const; /** @@ -58,9 +60,54 @@ export class SecretService implements ISecretService { */ clearStorage = async (): Promise => { await this._secrets.delete(OPERATE_AS_USER_KEY); + await this._secrets.delete(SERVER_PSK_KEY); await this._secrets.delete(SERVER_KEYS_KEY); }; + /** + * Delete PSK for given server url. + * @param serverUrl The server URL to delete the PSK for. + * @returns A promise that resolves when the PSK has been deleted. If key + * doesn't exist, the promise will still resolve, but it will be a no-op. + */ + deletePsk = async (serverUrl: URL): Promise => { + const existingServerPsks = + await this._getJson>(SERVER_PSK_KEY); + + if (existingServerPsks == null) { + return; + } + + delete existingServerPsks[serverUrl.toString()]; + + await this._storeJson(SERVER_PSK_KEY, existingServerPsks); + }; + + /** + * Get Psk for given server url. + * @param serverUrl The server URL to get the PSK for. + * @returns The PSK or null if not found. + */ + getPsk = async (serverUrl: URL): Promise => { + const psks = await this._getJson>(SERVER_PSK_KEY); + return psks?.[serverUrl.toString()]; + }; + + /** + * Store Psk for given server url. + * @param serverUrl The server URL to store the PSK for. + * @param psk The PSK to store. + */ + storePsk = async (serverUrl: URL, psk: Psk): Promise => { + const existingServerPsks = + await this._getJson>(SERVER_PSK_KEY); + + await this._storeJson(SERVER_PSK_KEY, { + ...existingServerPsks, + [serverUrl.toString()]: psk, + }); + }; + /** * Get user login preferences for a given server. * @param serverUrl The server URL to get the map for. diff --git a/src/services/ServerManager.ts b/src/services/ServerManager.ts index 389eaa1f..70ce0e49 100644 --- a/src/services/ServerManager.ts +++ b/src/services/ServerManager.ts @@ -1,7 +1,6 @@ import * as vscode from 'vscode'; import { randomUUID } from 'node:crypto'; import type { AuthenticatedClient as DheAuthenticatedClient } from '@deephaven-enterprise/auth-nodejs'; -import type { dh as DhcType } from '@deephaven/jsapi-types'; import { isDhcServerRunning, isDheServerRunning, @@ -10,7 +9,7 @@ import { UnsupportedConsoleTypeError } from '../common'; import type { ConsoleType, IConfigService, - IDhServiceFactory, + IDhcServiceFactory, IServerManager, ConnectionState, ServerState, @@ -18,9 +17,11 @@ import type { IDheService, IAsyncCacheService, WorkerURL, - Lazy, UniqueID, IToastService, + CoreAuthenticatedClient, + ISecretService, + Psk, } from '../types'; import { getInitialServerStates, @@ -32,27 +33,28 @@ import { import { URLMap } from './URLMap'; import { URIMap } from './URIMap'; import { DhcService } from './DhcService'; -import { AUTH_HANDLER_TYPE_PSK } from '../dh/dhc'; const logger = new Logger('ServerManager'); export class ServerManager implements IServerManager { constructor( configService: IConfigService, - coreCredentialsCache: URLMap>, - dhcServiceFactory: IDhServiceFactory, + coreClientCache: URLMap, + dhcServiceFactory: IDhcServiceFactory, dheClientCache: URLMap, dheServiceCache: IAsyncCacheService, outputChannel: vscode.OutputChannel, + secretService: ISecretService, toaster: IToastService ) { this._configService = configService; this._connectionMap = new URLMap(); - this._coreCredentialsCache = coreCredentialsCache; + this._coreClientCache = coreClientCache; this._dhcServiceFactory = dhcServiceFactory; this._dheClientCache = dheClientCache; this._dheServiceCache = dheServiceCache; this._outputChannel = outputChannel; + this._secretService = secretService; this._serverMap = new URLMap(); this._toaster = toaster; this._uriConnectionsMap = new URIMap(); @@ -65,13 +67,12 @@ export class ServerManager implements IServerManager { private readonly _configService: IConfigService; private readonly _connectionMap: URLMap; - private readonly _coreCredentialsCache: URLMap< - Lazy - >; - private readonly _dhcServiceFactory: IDhServiceFactory; + private readonly _coreClientCache: URLMap; + private readonly _dhcServiceFactory: IDhcServiceFactory; private readonly _dheClientCache: URLMap; private readonly _dheServiceCache: IAsyncCacheService; private readonly _outputChannel: vscode.OutputChannel; + private readonly _secretService: ISecretService; private readonly _toaster: IToastService; private readonly _uriConnectionsMap: URIMap; private readonly _workerURLToServerURLMap: URLMap; @@ -167,14 +168,7 @@ export class ServerManager implements IServerManager { if (serverState.isManaged) { this.updateConnectionCount(serverUrl, 1); - this._coreCredentialsCache.set(serverUrl, async () => ({ - type: AUTH_HANDLER_TYPE_PSK, - token: serverState.psk, - })); - } - // Non-managed DHC server - else if (serverState.type === 'DHC') { - this.updateConnectionCount(serverUrl, 1); + this._secretService.storePsk(serverUrl, serverState.psk); } // DHE server else if (serverState.type === 'DHE') { @@ -232,13 +226,21 @@ export class ServerManager implements IServerManager { const connection = this._dhcServiceFactory.create(serverUrl, tagId); + // Initialize client + prompt for login if necessary + if (!(await connection.getClient())) { + return null; + } + this._connectionMap.set(serverUrl, connection); this._onDidUpdate.fire(); - if (!(await connection.initDh())) { + if (!(await connection.initSession())) { this._connectionMap.delete(serverUrl); + return null; } + this.updateConnectionCount(serverUrl, 1); + this._onDidConnect.fire(serverUrl); this._onDidUpdate.fire(); @@ -352,6 +354,9 @@ export class ServerManager implements IServerManager { await connection.dispose(); } + this._coreClientCache.get(serverOrWorkerUrl)?.disconnect(); + this._coreClientCache.delete(serverOrWorkerUrl); + this._onDidDisconnect.fire(serverOrWorkerUrl); this._onDidUpdate.fire(); }; @@ -518,7 +523,7 @@ export class ServerManager implements IServerManager { this._serverMap.set(server.url, { ...server, isManaged: true, - psk: randomUUID(), + psk: randomUUID() as Psk, }); } diff --git a/src/services/cache/CoreJsApiCache.ts b/src/services/cache/CoreJsApiCache.ts new file mode 100644 index 00000000..5adb54f4 --- /dev/null +++ b/src/services/cache/CoreJsApiCache.ts @@ -0,0 +1,15 @@ +import { initDhcApi } from '@deephaven/require-jsapi'; +import type { dh as DhcType } from '@deephaven/jsapi-types'; +import { getTempDir, urlToDirectoryName } from '../../util'; +import { ByURLAsyncCache } from './ByURLAsyncCache'; + +/** + * Cache Core jsapi instances by URL. + */ +export class CoreJsApiCache extends ByURLAsyncCache { + constructor() { + super(async url => + initDhcApi(url, getTempDir({ subDirectory: urlToDirectoryName(url) })) + ); + } +} diff --git a/src/services/cache/index.ts b/src/services/cache/index.ts index 9c1c1c90..67c83a1a 100644 --- a/src/services/cache/index.ts +++ b/src/services/cache/index.ts @@ -1,3 +1,4 @@ export * from './ByURLAsyncCache'; +export * from './CoreJsApiCache'; export * from './DheJsApiCache'; export * from './DheServiceCache'; diff --git a/src/services/index.ts b/src/services/index.ts index 56ff8aba..26053866 100644 --- a/src/services/index.ts +++ b/src/services/index.ts @@ -1,7 +1,6 @@ export * from './cache'; export * from './ConfigService'; export * from './DhcService'; -export * from './DhcServiceFactory'; export * from './DheService'; export * from './PanelService'; export * from './PollingService'; diff --git a/src/types/commonTypes.d.ts b/src/types/commonTypes.d.ts index 29cd57d4..1b7c15cc 100644 --- a/src/types/commonTypes.d.ts +++ b/src/types/commonTypes.d.ts @@ -36,6 +36,15 @@ export interface CoreConnectionConfig { url: URL; } +export type CoreAuthenticatedClient = Brand< + 'CoreAuthenticatedClient', + DhcType.CoreClient +>; +export type CoreUnauthenticatedClient = Brand< + 'CoreUnauthenticatedClient', + DhcType.CoreClient +>; + export type EnterpriseConnectionConfigStored = | Brand<'EnterpriseConnectionConfigStored'> | { url: string; label?: string; experimentalWorkerConfig?: WorkerConfig }; @@ -51,6 +60,8 @@ export type LoginWorkflowResult = | PasswordCredentials | Omit; +export type Psk = Brand<'Psk', string>; + export type UserKeyPairs = Record; export type UserLoginPreferences = { lastLogin?: Username; @@ -105,7 +116,7 @@ export interface UnmanagedServerState { export interface ManagedServerState { isManaged: true; - psk: string; + psk: Psk; } export type ServerState = { diff --git a/src/types/serviceTypes.d.ts b/src/types/serviceTypes.d.ts index fb32c10a..b1114f13 100644 --- a/src/types/serviceTypes.d.ts +++ b/src/types/serviceTypes.d.ts @@ -17,6 +17,9 @@ import type { UniqueID, UserKeyPairs, UserLoginPreferences, + CoreUnauthenticatedClient, + Psk, + CoreAuthenticatedClient, } from '../types/commonTypes'; import type { AuthenticatedClient as DheAuthenticatedClient, @@ -47,8 +50,8 @@ export interface IDhcService extends Disposable, ConnectionState { readonly isConnected: boolean; readonly onDidDisconnect: vscode.Event; - initDh: () => Promise; - + initSession(): Promise; + getClient(): Promise; getConsoleTypes: () => Promise>; supportsConsoleType: (consoleType: ConsoleType) => Promise; @@ -88,10 +91,14 @@ export interface IFactory { create: (...args: TArgs) => T; } +export type ICoreClientFactory = ( + serverUrl: URL +) => Promise; + /** * Factory for creating IDhService instances. */ -export type IDhServiceFactory = IFactory< +export type IDhcServiceFactory = IFactory< IDhcService, [serverUrl: URL, tagId?: UniqueID] >; @@ -121,10 +128,16 @@ export interface IPanelService extends Disposable { * Secret service interface. */ export interface ISecretService { + clearStorage(): Promise; + // PSKs + deletePsk(serverUrl: URL): Promise; + getPsk(serverUrl: URL): Promise; + storePsk(serverUrl: URL, psk: Psk): Promise; + // DHE Server keys deleteUserServerKeys(serverUrl: URL, userName: Username): Promise; getServerKeys(serverUrl: URL): Promise; storeServerKeys(serverUrl: URL, serverKeys: UserKeyPairs): Promise; - clearStorage(): Promise; + // Login preferences getUserLoginPreferences(serverUrl: URL): Promise; storeUserLoginPreferences( serverUrl: URL, diff --git a/src/util/isInstanceOf.ts b/src/util/isInstanceOf.ts index 1ded95c6..efa0afac 100644 --- a/src/util/isInstanceOf.ts +++ b/src/util/isInstanceOf.ts @@ -8,9 +8,7 @@ */ export function isInstanceOf( value: unknown, - // Possible we'll need to include non-abstract constructor type, but this seems - // to work for both abstract and non-abstract constructors. - type: abstract new (...args: any[]) => T + type: Function & { prototype: T } ): value is T { return value instanceof type; } diff --git a/src/util/uiUtils.ts b/src/util/uiUtils.ts index 87ce1531..8af680d5 100644 --- a/src/util/uiUtils.ts +++ b/src/util/uiUtils.ts @@ -14,6 +14,7 @@ import type { ConnectionPickOption, ConnectionState, UserLoginPreferences, + Psk, } from '../types'; import { sortByStringProp } from './dataUtils'; import { assertDefined } from './assertUtil'; @@ -411,6 +412,21 @@ export function promptForPassword(title: string): Promise { }) as Promise; } +/** + * Prompt the user for a pre-shared key. + * @param title Title of the prompt + * @returns The pre-shared key or undefined if cancelled by the user. + */ +export function promptForPsk(title: string): Promise { + return vscode.window.showInputBox({ + ignoreFocusOut: true, + placeHolder: 'Pre-Shared Key', + prompt: 'Enter your Deephaven pre-shared key', + password: true, + title, + }) as Promise; +} + /** * Prompt the user for an `Operate As` username. * @param title Title of the prompt From ff803c3b6f85ba1c443173c22e75b61c2ad9ba1d Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 7 Nov 2024 15:39:22 -0600 Subject: [PATCH 2/7] Include psk in browser links (151) --- src/controllers/ExtensionController.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index 59e35dbe..02f494f5 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -697,9 +697,16 @@ export class ExtensionController implements Disposable { }; onOpenInBrowser = async (serverState: ServerState): Promise => { + const psk = await this._secretService?.getPsk(serverState.url); + const serverUrl = new URL(serverState.url); + + if (psk != null) { + serverUrl.searchParams.set('psk', psk); + } + vscode.commands.executeCommand( 'vscode.open', - vscode.Uri.parse(serverState.url.toString()) + vscode.Uri.parse(serverUrl.toString()) ); }; From 882892c81b4321ec19cf365c7bef9f2a62210d29 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 7 Nov 2024 16:33:57 -0600 Subject: [PATCH 3/7] Fixed worker logins (151) --- src/controllers/ExtensionController.ts | 22 +++++++++++----------- src/controllers/PanelController.ts | 10 +++++++--- src/controllers/UserLoginController.ts | 18 ++++++++++++++++-- src/services/DheService.ts | 15 --------------- src/services/ServerManager.ts | 5 +++-- 5 files changed, 37 insertions(+), 33 deletions(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index 02f494f5..ad7320b3 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -62,7 +62,6 @@ import type { ISecretService, IServerManager, IToastService, - Lazy, ServerConnectionPanelNode, ServerConnectionPanelTreeView, ServerConnectionTreeView, @@ -120,8 +119,6 @@ export class ExtensionController implements Disposable { private _connectionController: ConnectionController | null = null; private _coreClientCache: URLMap | null = null; private _coreClientFactory: ICoreClientFactory | null = null; - private _coreCredentialsCache: URLMap> | null = - null; private _coreJsApiCache: IAsyncCacheService | null = null; private _dheClientCache: URLMap | null = null; @@ -136,6 +133,7 @@ export class ExtensionController implements Disposable { private _secretService: ISecretService | null = null; private _serverManager: IServerManager | null = null; private _userLoginController: UserLoginController | null = null; + private _workerURLToServerURLMap: URLMap | null = null; // Tree providers private _serverTreeProvider: ServerTreeProvider | null = null; @@ -214,11 +212,13 @@ export class ExtensionController implements Disposable { assertDefined(this._dheClientCache, 'dheClientCache'); assertDefined(this._panelService, 'panelService'); assertDefined(this._serverManager, 'serverManager'); + assertDefined(this._workerURLToServerURLMap, 'workerURLToServerURLMap'); this._panelController = new PanelController( this._dheClientCache, this._serverManager, - this._panelService + this._panelService, + this._workerURLToServerURLMap ); this._context.subscriptions.push(this._panelController); @@ -251,6 +251,7 @@ export class ExtensionController implements Disposable { assertDefined(this._dheClientFactory, 'dheClientFactory'); assertDefined(this._secretService, 'secretService'); assertDefined(this._toaster, 'toaster'); + assertDefined(this._workerURLToServerURLMap, 'workerURLToServerURLMap'); this._userLoginController = new UserLoginController( this._coreClientCache, @@ -259,7 +260,8 @@ export class ExtensionController implements Disposable { this._dheClientCache, this._dheClientFactory, this._secretService, - this._toaster + this._toaster, + this._workerURLToServerURLMap ); this._context.subscriptions.push(this._userLoginController); @@ -324,8 +326,6 @@ export class ExtensionController implements Disposable { assertDefined(this._secretService, 'secretService'); assertDefined(this._toaster, 'toaster'); - this._coreCredentialsCache = new URLMap>(); - this._coreJsApiCache = new CoreJsApiCache(); this._context.subscriptions.push(this._coreJsApiCache); @@ -365,7 +365,6 @@ export class ExtensionController implements Disposable { this._dheServiceFactory = DheService.factory( this._config, - this._coreCredentialsCache, this._dheClientCache, this._dheJsApiCache, this._toaster @@ -374,6 +373,8 @@ export class ExtensionController implements Disposable { this._dheServiceCache = new DheServiceCache(this._dheServiceFactory); this._context.subscriptions.push(this._dheServiceCache); + this._workerURLToServerURLMap = new URLMap(); + this._serverManager = new ServerManager( this._config, this._coreClientCache, @@ -382,7 +383,8 @@ export class ExtensionController implements Disposable { this._dheServiceCache, this._outputChannel, this._secretService, - this._toaster + this._toaster, + this._workerURLToServerURLMap ); this._context.subscriptions.push(this._serverManager); @@ -667,8 +669,6 @@ export class ExtensionController implements Disposable { // DHC ServerState if (serverOrConnectionState.type === 'DHC') { - this._coreCredentialsCache?.delete(serverOrConnectionState.url); - await this._serverManager?.disconnectFromServer( serverOrConnectionState.url ); diff --git a/src/controllers/PanelController.ts b/src/controllers/PanelController.ts index 285ea3a0..82d594a4 100644 --- a/src/controllers/PanelController.ts +++ b/src/controllers/PanelController.ts @@ -32,13 +32,15 @@ export class PanelController extends ControllerBase { constructor( dheClientCache: URLMap, serverManager: IServerManager, - panelService: IPanelService + panelService: IPanelService, + workerURLToServerURLMap: URLMap ) { super(); this._dheClientCache = dheClientCache; this._panelService = panelService; this._serverManager = serverManager; + this._workerURLToServerURLMap = workerURLToServerURLMap; this.registerCommand(OPEN_VARIABLE_PANELS_CMD, this._onOpenPanels); this.registerCommand( @@ -56,6 +58,7 @@ export class PanelController extends ControllerBase { private readonly _dheClientCache: URLMap; private readonly _panelService: IPanelService; private readonly _serverManager: IServerManager; + private readonly _workerURLToServerURLMap: URLMap; /** * Handle `postMessage` messages from the panel. @@ -81,8 +84,9 @@ export class PanelController extends ControllerBase { // Respond to login credentials request from DH iframe if (message === DEEPHAVEN_POST_MSG.loginOptionsRequest) { - const dheClient = this._dheClientCache.get(serverOrWorkerUrl); - + const dheServerUrl = this._workerURLToServerURLMap.get(serverOrWorkerUrl); + const dheClient = + dheServerUrl == null ? null : this._dheClientCache.get(dheServerUrl); const credentials = dheClient == null ? null : await getWorkerCredentials(dheClient); diff --git a/src/controllers/UserLoginController.ts b/src/controllers/UserLoginController.ts index 18c9a881..4c7c8163 100644 --- a/src/controllers/UserLoginController.ts +++ b/src/controllers/UserLoginController.ts @@ -26,9 +26,10 @@ import type { IToastService, ServerState, } from '../types'; -import { hasInteractivePermission } from '../dh/dhe'; +import { getWorkerCredentials, hasInteractivePermission } from '../dh/dhe'; import { AUTH_HANDLER_TYPE_ANONYMOUS, + AUTH_HANDLER_TYPE_DHE, AUTH_HANDLER_TYPE_PSK, loginClient, } from '../dh/dhc'; @@ -46,7 +47,8 @@ export class UserLoginController extends ControllerBase { dheClientCache: URLMap, dheClientFactory: IDheClientFactory, secretService: ISecretService, - toastService: IToastService + toastService: IToastService, + workerURLToServerURLMap: URLMap ) { super(); @@ -57,6 +59,7 @@ export class UserLoginController extends ControllerBase { this.dheClientFactory = dheClientFactory; this.secretService = secretService; this.toast = toastService; + this.workerURLToServerURLMap = workerURLToServerURLMap; this.registerCommand( GENERATE_DHE_KEY_PAIR_CMD, @@ -81,6 +84,7 @@ export class UserLoginController extends ControllerBase { private readonly dheClientFactory: IDheClientFactory; private readonly secretService: ISecretService; private readonly toast: IToastService; + private readonly workerURLToServerURLMap: URLMap; /** * Login with a given key pair and remove the public key from the server. @@ -218,6 +222,16 @@ export class UserLoginController extends ControllerBase { }; this.secretService.storePsk(serverUrl, token); + } else if (authConfig.has(AUTH_HANDLER_TYPE_DHE)) { + const dheServerUrl = this.workerURLToServerURLMap.get(serverUrl); + const dheClient = + dheServerUrl == null ? null : this.dheClientCache.get(dheServerUrl); + + if (dheClient == null) { + throw new Error(`No DHE server client found for worker '${serverUrl}'`); + } + + credentials = await getWorkerCredentials(dheClient); } else { throw new Error('No supported authentication methods found.'); } diff --git a/src/services/DheService.ts b/src/services/DheService.ts index 2fcbe5e0..50dafde2 100644 --- a/src/services/DheService.ts +++ b/src/services/DheService.ts @@ -1,5 +1,4 @@ import * as vscode from 'vscode'; -import type { dh as DhcType } from '@deephaven/jsapi-types'; import type { AuthenticatedClient as DheAuthenticatedClient } from '@deephaven-enterprise/auth-nodejs'; import type { EnterpriseDhType as DheType } from '@deephaven-enterprise/jsapi-types'; import { @@ -10,7 +9,6 @@ import { type IDheService, type IDheServiceFactory, type IToastService, - type Lazy, type QuerySerial, type UniqueID, type WorkerConfig, @@ -21,7 +19,6 @@ import { Logger } from '../util'; import { createInteractiveConsoleQuery, deleteQueries, - getWorkerCredentials, getWorkerInfoFromQuery, } from '../dh/dhe'; import { CREATE_DHE_AUTHENTICATED_CLIENT_CMD } from '../common'; @@ -35,7 +32,6 @@ export class DheService implements IDheService { /** * Creates a factory function that can be used to create DheService instances. * @param configService Configuration service. - * @param coreCredentialsCache Core credentials cache. * @param dheClientCache DHE client cache. * @param dheJsApiCache DHE JS API cache. * @param toaster Toast service for notifications. @@ -43,7 +39,6 @@ export class DheService implements IDheService { */ static factory = ( configService: IConfigService, - coreCredentialsCache: URLMap>, dheClientCache: URLMap, dheJsApiCache: IAsyncCacheService, toaster: IToastService @@ -53,7 +48,6 @@ export class DheService implements IDheService { new DheService( serverUrl, configService, - coreCredentialsCache, dheClientCache, dheJsApiCache, toaster @@ -68,14 +62,12 @@ export class DheService implements IDheService { private constructor( serverUrl: URL, configService: IConfigService, - coreCredentialsCache: URLMap>, dheClientCache: URLMap, dheJsApiCache: IAsyncCacheService, toaster: IToastService ) { this.serverUrl = serverUrl; this._config = configService; - this._coreCredentialsCache = coreCredentialsCache; this._dheClientCache = dheClientCache; this._dheJsApiCache = dheJsApiCache; this._querySerialSet = new Set(); @@ -88,9 +80,6 @@ export class DheService implements IDheService { private _clientPromise: Promise | null = null; private _isConnected: boolean = false; private readonly _config: IConfigService; - private readonly _coreCredentialsCache: URLMap< - Lazy - >; private readonly _dheClientCache: URLMap; private readonly _dheJsApiCache: IAsyncCacheService; private readonly _querySerialSet: Set; @@ -241,10 +230,6 @@ export class DheService implements IDheService { if (workerInfo == null) { throw new Error('Failed to create worker.'); } - const workerUrl = new URL(workerInfo.grpcUrl); - this._coreCredentialsCache.set(workerUrl, () => - getWorkerCredentials(dheClient) - ); this._workerInfoMap.set(workerInfo.grpcUrl, workerInfo); diff --git a/src/services/ServerManager.ts b/src/services/ServerManager.ts index 70ce0e49..6d450a3b 100644 --- a/src/services/ServerManager.ts +++ b/src/services/ServerManager.ts @@ -45,7 +45,8 @@ export class ServerManager implements IServerManager { dheServiceCache: IAsyncCacheService, outputChannel: vscode.OutputChannel, secretService: ISecretService, - toaster: IToastService + toaster: IToastService, + workerURLToServerURLMap: URLMap ) { this._configService = configService; this._connectionMap = new URLMap(); @@ -58,7 +59,7 @@ export class ServerManager implements IServerManager { this._serverMap = new URLMap(); this._toaster = toaster; this._uriConnectionsMap = new URIMap(); - this._workerURLToServerURLMap = new URLMap(); + this._workerURLToServerURLMap = workerURLToServerURLMap; this.canStartServer = false; From f8317c2ba1d8dc400cdab8f503e5abd677f0c333 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 7 Nov 2024 17:01:27 -0600 Subject: [PATCH 4/7] Moved common logic into server manager (151) --- src/controllers/ExtensionController.ts | 18 +++++----------- src/controllers/PanelController.ts | 20 +++-------------- src/controllers/UserLoginController.ts | 22 ++++++++----------- src/services/ServerManager.ts | 30 +++++++++++++++++++++++--- src/types/serviceTypes.d.ts | 5 ++++- 5 files changed, 48 insertions(+), 47 deletions(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index ad7320b3..9bc1e5d8 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -133,7 +133,6 @@ export class ExtensionController implements Disposable { private _secretService: ISecretService | null = null; private _serverManager: IServerManager | null = null; private _userLoginController: UserLoginController | null = null; - private _workerURLToServerURLMap: URLMap | null = null; // Tree providers private _serverTreeProvider: ServerTreeProvider | null = null; @@ -209,16 +208,12 @@ export class ExtensionController implements Disposable { * Initialize panel controller. */ initializePanelController = (): void => { - assertDefined(this._dheClientCache, 'dheClientCache'); assertDefined(this._panelService, 'panelService'); assertDefined(this._serverManager, 'serverManager'); - assertDefined(this._workerURLToServerURLMap, 'workerURLToServerURLMap'); this._panelController = new PanelController( - this._dheClientCache, this._serverManager, - this._panelService, - this._workerURLToServerURLMap + this._panelService ); this._context.subscriptions.push(this._panelController); @@ -250,8 +245,8 @@ export class ExtensionController implements Disposable { assertDefined(this._dheClientCache, 'dheClientCache'); assertDefined(this._dheClientFactory, 'dheClientFactory'); assertDefined(this._secretService, 'secretService'); + assertDefined(this._serverManager, 'serverManager'); assertDefined(this._toaster, 'toaster'); - assertDefined(this._workerURLToServerURLMap, 'workerURLToServerURLMap'); this._userLoginController = new UserLoginController( this._coreClientCache, @@ -260,8 +255,8 @@ export class ExtensionController implements Disposable { this._dheClientCache, this._dheClientFactory, this._secretService, - this._toaster, - this._workerURLToServerURLMap + this._serverManager, + this._toaster ); this._context.subscriptions.push(this._userLoginController); @@ -373,8 +368,6 @@ export class ExtensionController implements Disposable { this._dheServiceCache = new DheServiceCache(this._dheServiceFactory); this._context.subscriptions.push(this._dheServiceCache); - this._workerURLToServerURLMap = new URLMap(); - this._serverManager = new ServerManager( this._config, this._coreClientCache, @@ -383,8 +376,7 @@ export class ExtensionController implements Disposable { this._dheServiceCache, this._outputChannel, this._secretService, - this._toaster, - this._workerURLToServerURLMap + this._toaster ); this._context.subscriptions.push(this._serverManager); diff --git a/src/controllers/PanelController.ts b/src/controllers/PanelController.ts index 82d594a4..6c912805 100644 --- a/src/controllers/PanelController.ts +++ b/src/controllers/PanelController.ts @@ -1,5 +1,4 @@ import * as vscode from 'vscode'; -import type { AuthenticatedClient as DheAuthenticatedClient } from '@deephaven-enterprise/auth-nodejs'; import type { ConnectionState, IPanelService, @@ -15,7 +14,7 @@ import { getPanelHtml, Logger, } from '../util'; -import { DhcService, type URLMap } from '../services'; +import { DhcService } from '../services'; import { DEEPHAVEN_POST_MSG, OPEN_VARIABLE_PANELS_CMD, @@ -24,23 +23,15 @@ import { import { waitFor } from '../util/promiseUtils'; import { getEmbedWidgetUrl } from '../dh/dhc'; import { ControllerBase } from './ControllerBase'; -import { getWorkerCredentials } from '../dh/dhe'; const logger = new Logger('PanelController'); export class PanelController extends ControllerBase { - constructor( - dheClientCache: URLMap, - serverManager: IServerManager, - panelService: IPanelService, - workerURLToServerURLMap: URLMap - ) { + constructor(serverManager: IServerManager, panelService: IPanelService) { super(); - this._dheClientCache = dheClientCache; this._panelService = panelService; this._serverManager = serverManager; - this._workerURLToServerURLMap = workerURLToServerURLMap; this.registerCommand(OPEN_VARIABLE_PANELS_CMD, this._onOpenPanels); this.registerCommand( @@ -55,10 +46,8 @@ export class PanelController extends ControllerBase { ); } - private readonly _dheClientCache: URLMap; private readonly _panelService: IPanelService; private readonly _serverManager: IServerManager; - private readonly _workerURLToServerURLMap: URLMap; /** * Handle `postMessage` messages from the panel. @@ -84,11 +73,8 @@ export class PanelController extends ControllerBase { // Respond to login credentials request from DH iframe if (message === DEEPHAVEN_POST_MSG.loginOptionsRequest) { - const dheServerUrl = this._workerURLToServerURLMap.get(serverOrWorkerUrl); - const dheClient = - dheServerUrl == null ? null : this._dheClientCache.get(dheServerUrl); const credentials = - dheClient == null ? null : await getWorkerCredentials(dheClient); + await this._serverManager.getWorkerCredentials(serverOrWorkerUrl); if (credentials == null) { logger.error('Failed to get credentials for worker', serverOrWorkerUrl); diff --git a/src/controllers/UserLoginController.ts b/src/controllers/UserLoginController.ts index 4c7c8163..02ade1d8 100644 --- a/src/controllers/UserLoginController.ts +++ b/src/controllers/UserLoginController.ts @@ -23,10 +23,11 @@ import type { ICoreClientFactory, IDheClientFactory, ISecretService, + IServerManager, IToastService, ServerState, } from '../types'; -import { getWorkerCredentials, hasInteractivePermission } from '../dh/dhe'; +import { hasInteractivePermission } from '../dh/dhe'; import { AUTH_HANDLER_TYPE_ANONYMOUS, AUTH_HANDLER_TYPE_DHE, @@ -47,8 +48,8 @@ export class UserLoginController extends ControllerBase { dheClientCache: URLMap, dheClientFactory: IDheClientFactory, secretService: ISecretService, - toastService: IToastService, - workerURLToServerURLMap: URLMap + serverManager: IServerManager, + toastService: IToastService ) { super(); @@ -58,8 +59,8 @@ export class UserLoginController extends ControllerBase { this.dheClientCache = dheClientCache; this.dheClientFactory = dheClientFactory; this.secretService = secretService; + this.serverManager = serverManager; this.toast = toastService; - this.workerURLToServerURLMap = workerURLToServerURLMap; this.registerCommand( GENERATE_DHE_KEY_PAIR_CMD, @@ -83,8 +84,8 @@ export class UserLoginController extends ControllerBase { private readonly dheClientCache: URLMap; private readonly dheClientFactory: IDheClientFactory; private readonly secretService: ISecretService; + private readonly serverManager: IServerManager; private readonly toast: IToastService; - private readonly workerURLToServerURLMap: URLMap; /** * Login with a given key pair and remove the public key from the server. @@ -223,15 +224,10 @@ export class UserLoginController extends ControllerBase { this.secretService.storePsk(serverUrl, token); } else if (authConfig.has(AUTH_HANDLER_TYPE_DHE)) { - const dheServerUrl = this.workerURLToServerURLMap.get(serverUrl); - const dheClient = - dheServerUrl == null ? null : this.dheClientCache.get(dheServerUrl); - - if (dheClient == null) { - throw new Error(`No DHE server client found for worker '${serverUrl}'`); + credentials = await this.serverManager.getWorkerCredentials(serverUrl); + if (credentials == null) { + throw new Error(`Failed to get credentials for worker '${serverUrl}'`); } - - credentials = await getWorkerCredentials(dheClient); } else { throw new Error('No supported authentication methods found.'); } diff --git a/src/services/ServerManager.ts b/src/services/ServerManager.ts index 6d450a3b..cac841df 100644 --- a/src/services/ServerManager.ts +++ b/src/services/ServerManager.ts @@ -1,5 +1,6 @@ import * as vscode from 'vscode'; import { randomUUID } from 'node:crypto'; +import type { dh as DhcType } from '@deephaven/jsapi-types'; import type { AuthenticatedClient as DheAuthenticatedClient } from '@deephaven-enterprise/auth-nodejs'; import { isDhcServerRunning, @@ -33,6 +34,7 @@ import { import { URLMap } from './URLMap'; import { URIMap } from './URIMap'; import { DhcService } from './DhcService'; +import { getWorkerCredentials } from '../dh/dhe'; const logger = new Logger('ServerManager'); @@ -45,8 +47,7 @@ export class ServerManager implements IServerManager { dheServiceCache: IAsyncCacheService, outputChannel: vscode.OutputChannel, secretService: ISecretService, - toaster: IToastService, - workerURLToServerURLMap: URLMap + toaster: IToastService ) { this._configService = configService; this._connectionMap = new URLMap(); @@ -59,7 +60,7 @@ export class ServerManager implements IServerManager { this._serverMap = new URLMap(); this._toaster = toaster; this._uriConnectionsMap = new URIMap(); - this._workerURLToServerURLMap = workerURLToServerURLMap; + this._workerURLToServerURLMap = new URLMap(); this.canStartServer = false; @@ -459,6 +460,29 @@ export class ServerManager implements IServerManager { return this._uriConnectionsMap.get(uri) ?? null; }; + /** + * Get worker credentials for the given worker URL. + * @param serverOrWorkerUrl The worker URL to get credentials for. + * @returns The worker credentials, or `null` if no credentials are available. + */ + getWorkerCredentials = async ( + serverOrWorkerUrl: URL | WorkerURL + ): Promise => { + const dheServerUrl = this._workerURLToServerURLMap.get(serverOrWorkerUrl); + + if (dheServerUrl == null) { + return null; + } + + const dheClient = await this._dheClientCache.get(dheServerUrl); + + if (dheClient == null) { + return null; + } + + return getWorkerCredentials(dheClient); + }; + /** Get worker info associated with the given server URL. */ getWorkerInfo = async ( workerUrl: WorkerURL diff --git a/src/types/serviceTypes.d.ts b/src/types/serviceTypes.d.ts index b1114f13..a0c4263a 100644 --- a/src/types/serviceTypes.d.ts +++ b/src/types/serviceTypes.d.ts @@ -1,5 +1,5 @@ import * as vscode from 'vscode'; - +import type { dh as DhcType } from '@deephaven/jsapi-types'; import type { ConsoleType, CoreConnectionConfig, @@ -169,6 +169,9 @@ export interface IServerManager extends Disposable { getEditorConnection: ( editor: vscode.TextEditor ) => Promise; + getWorkerCredentials: ( + serverOrWorkerUrl: URL | WorkerURL + ) => Promise; getWorkerInfo: (workerUrl: WorkerURL) => Promise; setEditorConnection: ( editor: vscode.TextEditor, From 5ad6a6ddd1e10bca7fc20c7c448139713d1c6993 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 7 Nov 2024 17:07:12 -0600 Subject: [PATCH 5/7] Comments (151) --- src/services/DhcService.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/services/DhcService.ts b/src/services/DhcService.ts index c2c6cd0c..3ea133ca 100644 --- a/src/services/DhcService.ts +++ b/src/services/DhcService.ts @@ -30,6 +30,16 @@ import { hasErrorCode } from '../util/typeUtils'; const logger = new Logger('DhcService'); export class DhcService implements IDhcService { + /** + * Creates a factory function that can be used to create DhcService instances. + * @param coreClientCache Core client cache. + * @param panelService Panel service. + * @param diagnosticsCollection Diagnostics collection. + * @param outputChannel Output channel. + * @param secretService Secret service. + * @param toaster Toast service for notifications. + * @returns A factory function that can be used to create DhcService instances. + */ static factory = ( coreClientCache: URLMap, panelService: IPanelService, @@ -54,6 +64,10 @@ export class DhcService implements IDhcService { }; }; + /** + * Private constructor since the static `factory` method is the intended + * mechanism for instantiating. + */ private constructor( serverUrl: URL, coreClientCache: URLMap, From ce62ad4a69365512f35dad86ccd6e29bf12a19df Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 7 Nov 2024 17:59:14 -0600 Subject: [PATCH 6/7] Fixing tests (151) --- src/util/treeViewUtils.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/treeViewUtils.spec.ts b/src/util/treeViewUtils.spec.ts index 0abb39db..36768725 100644 --- a/src/util/treeViewUtils.spec.ts +++ b/src/util/treeViewUtils.spec.ts @@ -15,6 +15,7 @@ import { import type { ConsoleType, IDhcService, + Psk, ServerState, VariableDefintion, VariableType, @@ -159,7 +160,7 @@ describe('getServerTreeItem', () => { const actual = getServerTreeItem({ ...dhcServerState, ...(isManaged - ? { isManaged: true, psk: 'mock.psk' } + ? { isManaged: true, psk: 'mock.psk' as Psk } : { isManaged: false }), type, connectionCount: isConnected ? 1 : 0, From 7b23d292b5373f61e2a13c0c8b637e451b21ff0a Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 12 Nov 2024 15:39:00 -0600 Subject: [PATCH 7/7] Updated cmd titles (151) --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index e786de3f..8c06ee2a 100644 --- a/package.json +++ b/package.json @@ -220,11 +220,11 @@ }, { "command": "vscode-deephaven.createCoreAuthenticatedClient", - "title": "Create Authenticated Client" + "title": "Create Core Authenticated Client" }, { "command": "vscode-deephaven.createDHEAuthenticatedClient", - "title": "Create Authenticated Client" + "title": "Create DHE Authenticated Client" }, { "command": "vscode-deephaven.startServer",