From 518946e39380e47b438ed761ad437097da2dfa60 Mon Sep 17 00:00:00 2001 From: gjsjohnmurray Date: Tue, 8 Oct 2024 11:38:27 +0100 Subject: [PATCH 1/3] Test connection with retrieved password, and sign out if this fails --- src/authenticationProvider.ts | 76 +++++++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 17 deletions(-) diff --git a/src/authenticationProvider.ts b/src/authenticationProvider.ts index 75e4c55..e2d0281 100644 --- a/src/authenticationProvider.ts +++ b/src/authenticationProvider.ts @@ -14,6 +14,8 @@ import { } from "vscode"; import { ServerManagerAuthenticationSession } from "./authenticationSession"; import { globalState } from "./extension"; +import { getServerSpec } from "./api/getServerSpec"; +import { makeRESTRequest } from "./makeRESTRequest"; export const AUTHENTICATION_PROVIDER = "intersystems-server-credentials"; const AUTHENTICATION_PROVIDER_LABEL = "InterSystems Server Credentials"; @@ -35,6 +37,7 @@ export class ServerManagerAuthenticationProvider implements AuthenticationProvid private readonly _secretStorage; private _sessions: ServerManagerAuthenticationSession[] = []; + private _checkedSessions: ServerManagerAuthenticationSession[] = []; private _serverManagerExtension = extensions.getExtension("intersystems-community.servermanager"); @@ -52,7 +55,7 @@ export class ServerManagerAuthenticationProvider implements AuthenticationProvid this._initializedDisposable?.dispose(); } - // This function is called first when `vscode.authentication.getSessions` is called. + // This function is called first when `vscode.authentication.getSession` is called. public async getSessions(scopes: string[] = []): Promise { await this._ensureInitialized(); let sessions = this._sessions; @@ -61,7 +64,11 @@ export class ServerManagerAuthenticationProvider implements AuthenticationProvid for (let index = 0; index < scopes.length; index++) { sessions = sessions.filter((session) => session.scopes[index] === scopes[index].toLowerCase()); } - return sessions; + + if (sessions.length === 1) { + sessions = sessions.filter(async (session) => await this._isStillValid(session)); + } + return sessions || []; } // This function is called after `this.getSessions` is called, and only when: @@ -104,9 +111,17 @@ export class ServerManagerAuthenticationProvider implements AuthenticationProvid // Return existing session if found const sessionId = ServerManagerAuthenticationProvider.sessionId(serverName, userName); - const existingSession = this._sessions.find((s) => s.id === sessionId); + let existingSession = this._sessions.find((s) => s.id === sessionId); if (existingSession) { - return existingSession; + if (this._checkedSessions.find((s) => s.id === sessionId)) { + return existingSession; + } + + // Check if the session is still valid + if (await this._isStillValid(existingSession)) { + this._checkedSessions.push(existingSession); + return existingSession; + } } let password: string | undefined = ""; @@ -190,25 +205,52 @@ export class ServerManagerAuthenticationProvider implements AuthenticationProvid return session; } + private async _isStillValid(session: ServerManagerAuthenticationSession): Promise { + if (this._checkedSessions.find((s) => s.id === session.id)) { + return true; + } + const serverSpec = await getServerSpec(session.serverName); + if (serverSpec) { + serverSpec.username = session.userName; + serverSpec.password = session.accessToken; + const response = await makeRESTRequest("HEAD", serverSpec); + if (response?.status !== 200) { + this._removeSession(session.id, true); + return false; + } + } + this._checkedSessions.push(session); + return true; + } + // This function is called when the end user signs out of the account. public async removeSession(sessionId: string): Promise { + this._removeSession(sessionId); + } + + private async _removeSession(sessionId: string, alwaysDeletePassword = false): Promise { const index = this._sessions.findIndex((item) => item.id === sessionId); const session = this._sessions[index]; - let deletePassword = false; const credentialKey = ServerManagerAuthenticationProvider.credentialKey(sessionId); - if (await this.secretStorage.get(credentialKey)) { - const passwordOption = workspace.getConfiguration("intersystemsServerManager.credentialsProvider") - .get("deletePasswordOnSignout", "ask"); - deletePassword = (passwordOption === "always"); - if (passwordOption === "ask") { - const choice = await window.showWarningMessage( - `Do you want to keep the password or delete it?`, - { detail: `The ${AUTHENTICATION_PROVIDER_LABEL} account you signed out (${session.account.label}) is currently storing its password securely on your workstation.`, modal: true }, - { title: "Keep", isCloseAffordance: true }, - { title: "Delete", isCloseAffordance: false }, - ); - deletePassword = (choice?.title === "Delete"); + let deletePassword = false; + const hasStoredPassword = await this.secretStorage.get(credentialKey) !== undefined; + if (alwaysDeletePassword) { + deletePassword = hasStoredPassword; + } else { + if (hasStoredPassword) { + const passwordOption = workspace.getConfiguration("intersystemsServerManager.credentialsProvider") + .get("deletePasswordOnSignout", "ask"); + deletePassword = (passwordOption === "always"); + if (passwordOption === "ask") { + const choice = await window.showWarningMessage( + `Do you want to keep the password or delete it?`, + { detail: `The ${AUTHENTICATION_PROVIDER_LABEL} account you signed out (${session.account.label}) is currently storing its password securely on your workstation.`, modal: true }, + { title: "Keep", isCloseAffordance: true }, + { title: "Delete", isCloseAffordance: false }, + ); + deletePassword = (choice?.title === "Delete"); + } } } if (deletePassword) { From 92604b30f69fc544d8d0b00e63f5aa47b4eac0bb Mon Sep 17 00:00:00 2001 From: gjsjohnmurray Date: Tue, 8 Oct 2024 11:55:43 +0100 Subject: [PATCH 2/3] Remove obsolete code missed by #190 --- src/api/getServerSpec.ts | 14 -------------- src/extension.ts | 18 +++++++++--------- src/makeRESTRequest.ts | 2 +- src/ui/serverManagerView.ts | 4 +--- 4 files changed, 11 insertions(+), 27 deletions(-) diff --git a/src/api/getServerSpec.ts b/src/api/getServerSpec.ts index 6c5474d..e532075 100644 --- a/src/api/getServerSpec.ts +++ b/src/api/getServerSpec.ts @@ -1,31 +1,17 @@ import * as vscode from "vscode"; import { IServerSpec } from "@intersystems-community/intersystems-servermanager"; -interface ICredentialSet { - username: string; - password: string; -} - -export let credentialCache = new Map(); - /** * Get a server specification. * * @param name The name. * @param scope The settings scope to use for the lookup. - * @param flushCredentialCache Flush the session's cache of credentials obtained from keystore and/or user prompting. - * @param noCredentials Set username and password as undefined; do not fetch credentials from anywhere. * @returns Server specification or undefined. */ export async function getServerSpec( name: string, scope?: vscode.ConfigurationScope, - flushCredentialCache: boolean = false, - noCredentials: boolean = false, ): Promise { - if (flushCredentialCache) { - credentialCache[name] = undefined; - } // To avoid breaking existing users, continue to return a default server definition even after we dropped that feature let server: IServerSpec | undefined = vscode.workspace.getConfiguration("intersystems.servers", scope).get(name) || legacyEmbeddedServer(name); diff --git a/src/extension.ts b/src/extension.ts index f8db93d..45aa478 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -254,7 +254,7 @@ export function activate(context: vscode.ExtensionContext) { if (pathParts && pathParts.length === 4) { const serverName = pathParts[1]; const namespace = pathParts[3]; - const serverSpec = await getServerSpec(serverName, undefined, undefined, true); + const serverSpec = await getServerSpec(serverName); if (serverSpec) { const ISFS_ID = "intersystems-community.vscode-objectscript"; const isfsExtension = vscode.extensions.getExtension(ISFS_ID); @@ -379,18 +379,18 @@ export function activate(context: vscode.ExtensionContext) { /** * Get specification for the named server. * - * If the `"intersystemsServerManager.authentication.provider"` setting is "intersystems-server-credentials": - * - the returned object will not contain `password`. To get this: + * The returned object will not contain `password`. To get that: * ``` - * const session = await vscode.authentication.getSession('intersystems-server-credentials', [serverSpec.name, serverSpec.username]); + * const session: vscode.AuthenticationSession = await vscode.authentication.getSession('intersystems-server-credentials', [serverSpec.name, serverSpec.username]); * ``` * The `accessToken` property of the returned [`AuthenticationSession`](https://code.visualstudio.com/api/references/vscode-api#AuthenticationSession) is the password. - * - `flushCredentialsCache` param will be ignored; - * - `noCredentials` property of `options` param has no effect; + * + * The `flushCredentialsCache` param is obsolete and has no effect; + * The `noCredentials` property of `options` param is obsolete and has no effect; * * @param name Name of the server, used as the key into the 'intersystems.servers' settings object * @param scope Settings scope to look in. - * @param flushCredentialCache If passed as true, flush extension's credential cache. + * @param flushCredentialCache Obsolete, has no effect. * @param options * @returns { IServerSpec } Server specification object. */ @@ -398,9 +398,9 @@ export function activate(context: vscode.ExtensionContext) { name: string, scope?: vscode.ConfigurationScope, flushCredentialCache: boolean = false, - options?: { hideFromRecents?: boolean, noCredentials?: boolean }, + options?: { hideFromRecents?: boolean, /* Obsolete */ noCredentials?: boolean }, ): Promise { - const spec = await getServerSpec(name, scope, flushCredentialCache, options?.noCredentials); + const spec = await getServerSpec(name, scope); if (spec && !options?.hideFromRecents) { await view.addToRecents(name); } diff --git a/src/makeRESTRequest.ts b/src/makeRESTRequest.ts index 1508a24..4d338bd 100644 --- a/src/makeRESTRequest.ts +++ b/src/makeRESTRequest.ts @@ -176,7 +176,7 @@ export async function makeRESTRequest( */ export async function logout(serverName: string) { - const server = await getServerSpec(serverName, undefined, false, true); + const server = await getServerSpec(serverName, undefined); if (!server) { return; diff --git a/src/ui/serverManagerView.ts b/src/ui/serverManagerView.ts index a74e040..e1ff84a 100644 --- a/src/ui/serverManagerView.ts +++ b/src/ui/serverManagerView.ts @@ -1,6 +1,6 @@ import * as vscode from "vscode"; import { getServerNames } from "../api/getServerNames"; -import { credentialCache, getServerSpec } from "../api/getServerSpec"; +import { getServerSpec } from "../api/getServerSpec"; import { getServerSummary } from "../api/getServerSummary"; import { IServerName } from "@intersystems-community/intersystems-servermanager"; import { makeRESTRequest } from "../makeRESTRequest"; @@ -386,7 +386,6 @@ async function serverFeatures(element: ServerTreeItem, params?: any): Promise { From 797655fed9082f21f116ed3c288626dd02d8008f Mon Sep 17 00:00:00 2001 From: gjsjohnmurray Date: Tue, 8 Oct 2024 17:16:46 +0100 Subject: [PATCH 3/3] Prompt for password if the stored one didn't work --- src/authenticationProvider.ts | 8 +++++--- src/makeRESTRequest.ts | 2 +- src/ui/serverManagerView.ts | 17 +++++++++++------ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/authenticationProvider.ts b/src/authenticationProvider.ts index e2d0281..f203791 100644 --- a/src/authenticationProvider.ts +++ b/src/authenticationProvider.ts @@ -66,7 +66,9 @@ export class ServerManagerAuthenticationProvider implements AuthenticationProvid } if (sessions.length === 1) { - sessions = sessions.filter(async (session) => await this._isStillValid(session)); + if (!(await this._isStillValid(sessions[0]))) { + sessions = []; + } } return sessions || []; } @@ -214,8 +216,8 @@ export class ServerManagerAuthenticationProvider implements AuthenticationProvid serverSpec.username = session.userName; serverSpec.password = session.accessToken; const response = await makeRESTRequest("HEAD", serverSpec); - if (response?.status !== 200) { - this._removeSession(session.id, true); + if (response?.status === 401) { + await this._removeSession(session.id, true); return false; } } diff --git a/src/makeRESTRequest.ts b/src/makeRESTRequest.ts index 4d338bd..1658212 100644 --- a/src/makeRESTRequest.ts +++ b/src/makeRESTRequest.ts @@ -165,7 +165,7 @@ export async function makeRESTRequest( return respdata; } catch (error) { console.log(error); - return undefined; + return error.response; } } diff --git a/src/ui/serverManagerView.ts b/src/ui/serverManagerView.ts index e1ff84a..f1730ab 100644 --- a/src/ui/serverManagerView.ts +++ b/src/ui/serverManagerView.ts @@ -383,8 +383,13 @@ async function serverFeatures(element: ServerTreeItem, params?: any): Promise { + response.data.result.content.namespaces.map((namespace: string) => { children.push(new NamespaceTreeItem({ parent: element, label: name, id: name }, namespace, name, serverApiVersion)); }); } @@ -555,7 +560,7 @@ async function namespaceProjects(element: ProjectsTreeItem, params?: any): Promi { apiVersion: 1, namespace: params.ns, path: "/action/query" }, { query: "SELECT Name, Description FROM %Studio.Project", parameters: [] } ); - if (response !== undefined) { + if (response?.status === 200) { if (response.data.result.content === undefined) { let message; if (response.data.status?.errors[0]?.code === 5540) { @@ -641,7 +646,7 @@ async function namespaceWebApps(element: ProjectsTreeItem, params?: any): Promis serverSpec, { apiVersion: 1, namespace: "%SYS", path: `/cspapps/${params.ns}` } ); - if (response !== undefined) { + if (response?.status === 200) { if (response.data.result.content === undefined) { vscode.window.showErrorMessage(response.data.status.summary); return undefined;