fix: bug fixes found during mfa testing

Author: smorrisj

client/src/connection/ssh/auth.ts

@@ -27,29 +27,46 @@ export interface AuthPresenter {
    * @param prompts an array of prompts to present to the user
    * @returns array of answers to the prompts
    */
-  presentMultiplePrompts: (prompts: Prompt[]) => Promise<string[]>;
+  presentMultiplePrompts: (
+    username: string,
+    prompts: Prompt[],
+  ) => Promise<string[]>;
 }
 
 class AuthPresenterImpl implements AuthPresenter {
   presentPasswordPrompt = async (username: string): Promise<string> => {
-    return this.presentSecurePrompt(
+    return this.presentPrompt(
       l10n.t("Enter the password for user: {username}", { username }),
       l10n.t("Password Required"),
+      true,
     );
   };
 
   presentPassphrasePrompt = async (): Promise<string> => {
-    return this.presentSecurePrompt(
+    return this.presentPrompt(
       l10n.t("Enter the passphrase for the private key"),
       l10n.t("Passphrase Required"),
+      true,
     );
   };
 
-  presentMultiplePrompts = async (prompts: Prompt[]): Promise<string[]> => {
+  presentMultiplePrompts = async (
+    username: string,
+    prompts: Prompt[],
+  ): Promise<string[]> => {
     const answers: string[] = [];
     for (const prompt of prompts) {
-      const answer = await this.presentSecurePrompt(prompt.prompt);
-      answers.push(answer);
+      const answer = await this.presentPrompt(
+        undefined,
+        l10n.t("User {username} {prompt}", {
+          username,
+          prompt: prompt.prompt,
+        }),
+        !prompt.echo,
+      );
+      if (answer) {
+        answers.push(answer);
+      }
     }
     return answers;
   };
@@ -58,17 +75,19 @@ class AuthPresenterImpl implements AuthPresenter {
    * Present a secure prompt to the user.
    * @param prompt  the prompt to display to the user
    * @param title  optional title for the prompt
+   * @param isSecureInput  whether the input should be hidden
    * @returns the user's response to the prompt
    */
-  private presentSecurePrompt = async (
+  private presentPrompt = async (
     prompt: string,
     title?: string,
+    isSecureInput?: boolean,
   ): Promise<string> => {
     return window.showInputBox({
       ignoreFocusOut: true,
       prompt: prompt,
       title: title,
-      password: true,
+      password: isSecureInput,
     });
   };
 }
@@ -119,18 +138,20 @@ export class AuthHandler {
     cb({
       type: "keyboard-interactive",
       username: username,
-      prompt: (_name, _instructions, _instructionsLang, prompts, promptCb) => {
+      prompt: (_name, _instructions, _instructionsLang, prompts, finish) => {
         // often, the server will only send a single prompt for the password.
         // however, PAM can send multiple prompts, so we need to handle that case
         this._authPresenter
-          .presentMultiplePrompts(prompts)
-          .then((answers) => promptCb(answers));
+          .presentMultiplePrompts(username, prompts)
+          .then((answers) => {
+            finish(answers);
+          });
       },
     });
   };
 
   /**
-   * Authenticate to the server using the ssh-agent. See the extension README for more information on how to set up the ssh-agent.
+   * Authenticate to the server using the ssh-agent. See the extension Docs for more information on how to set up the ssh-agent.
    * @param cb ssh2 NextHandler callback instance. This is used to pass the authentication information to the ssh server.
    * @param username the user name to use for the connection
    */

client/src/connection/ssh/const.ts

@@ -2,14 +2,10 @@
 // SPDX-License-Identifier: Apache-2.0
 
 const SECOND = 1000;
+const MINUTE = 60 * SECOND;
+const HOUR = 60 * MINUTE;
 export const KEEPALIVE_INTERVAL = 60 * SECOND; //How often (in milliseconds) to send SSH-level keepalive packets to the server. Set to 0 to disable.
-// 720 * 60 seconds = 43200 seconds = 12 hours
-export const KEEPALIVE_UNANSWERED_THRESHOLD = 720; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection.
+export const KEEPALIVE_UNANSWERED_THRESHOLD = 12 * HOUR; //How many consecutive, unanswered SSH-level keepalive packets that can be sent to the server before disconnection.
 export const WORK_DIR_START_TAG = "<WorkDirectory>";
 export const WORK_DIR_END_TAG = "</WorkDirectory>";
-export const SAS_LAUNCH_TIMEOUT = 60000;
-export const SUPPORTED_AUTH_METHODS = [
-  "publickey",
-  "password",
-  "keyboard-interactive",
-];
+export const CONNECT_READY_TIMEOUT = 5 * MINUTE; //allow extra time due to possible prompting

client/src/connection/ssh/index.ts

@@ -18,10 +18,9 @@ import { Session } from "../session";
 import { extractOutputHtmlFileName } from "../util";
 import { AuthHandler } from "./auth";
 import {
+  CONNECT_READY_TIMEOUT,
   KEEPALIVE_INTERVAL,
   KEEPALIVE_UNANSWERED_THRESHOLD,
-  SAS_LAUNCH_TIMEOUT,
-  SUPPORTED_AUTH_METHODS,
   WORK_DIR_END_TAG,
   WORK_DIR_START_TAG,
 } from "./const";
@@ -51,7 +50,6 @@ export class SSHSession extends Session {
   private _reject: ((reason?) => void) | undefined;
   private _html5FileName = "";
   private _sessionReady: boolean;
-  private _authMethods: AuthenticationType[]; //auth methods that this session can support
   private _authHandler: AuthHandler;
   private _workDirectory: string;
   private _workDirectoryParser: LineParser;
@@ -60,7 +58,6 @@ export class SSHSession extends Session {
     super();
     this._config = c;
     this._conn = client;
-    this._authMethods = ["publickey", "password", "keyboard-interactive"];
     this._sessionReady = false;
     this._authHandler = new AuthHandler();
     this._workDirectoryParser = new LineParser(
@@ -92,7 +89,7 @@ export class SSHSession extends Session {
         host: this._config.host,
         port: this._config.port,
         username: this._config.username,
-        readyTimeout: SAS_LAUNCH_TIMEOUT,
+        readyTimeout: CONNECT_READY_TIMEOUT,
         keepaliveInterval: KEEPALIVE_INTERVAL,
         keepaliveCountMax: KEEPALIVE_UNANSWERED_THRESHOLD,
 
@@ -107,6 +104,7 @@ export class SSHSession extends Session {
       }
 
       this._conn
+        .on("close", this.onConnectionClose)
         .on("ready", () => {
           this._conn.shell(this.onShell);
         })
@@ -136,6 +134,12 @@ export class SSHSession extends Session {
     this._stream.close();
   };
 
+  private onConnectionClose = () => {
+    if (!this._sessionReady) {
+      this._reject?.(new Error(l10n.t("Could not connect to the SAS server.")));
+    }
+  };
+
   private onConnectionError = (err: Error) => {
     this.clearAuthState();
     this._reject?.(err);
@@ -206,6 +210,7 @@ export class SSHSession extends Session {
 
     return foundWorkDirectory || "";
   };
+
   private resolveSystemVars = (): void => {
     const code = `%let workDir = %sysfunc(pathname(work));
     %put ${WORK_DIR_START_TAG};
@@ -216,6 +221,7 @@ export class SSHSession extends Session {
     `;
     this._stream.write(code);
   };
+
   private onStreamData = (data: Buffer): void => {
     const output = data.toString().trimEnd();
 
@@ -293,68 +299,55 @@ export class SSHSession extends Session {
    * Resets the SSH auth state.
    */
   private clearAuthState = (): void => {
-    this._authMethods = undefined;
     this._sessionReady = false;
   };
 
   private handleSSHAuthentication: AuthHandlerMiddleware = (
     authsLeft: AuthenticationType[],
     _partialSuccess: boolean,
-    cb: NextAuthHandler,
+    nextAuth: NextAuthHandler,
   ) => {
     if (!authsLeft) {
-      cb("none"); //sending none will prompt the server to send supported auth methods
+      nextAuth("none"); //sending none will prompt the server to send supported auth methods
       return;
     }
 
-    if (!this._authMethods) {
-      this._authMethods = authsLeft;
-    }
-
-    if (this._authMethods.length === 0) {
-      //if we're out of auth methods to try, then reject with an error
+    if (authsLeft.length === 0) {
       this._reject?.(
         new Error(l10n.t("Could not authenticate to the SSH server.")),
       );
-
       this.clearAuthState();
-      //returning false will stop the auth process
-      return false;
+      return false; //returning false will stop the authentication process
     }
 
-    //otherwise, fetch the next auth method to try
-    const authMethod = this._authMethods.shift();
-
-    //make sure the auth method is supported by the server
-    if (SUPPORTED_AUTH_METHODS.includes(authMethod)) {
-      switch (authMethod) {
-        case "publickey": {
-          //user set a keyfile path in profile config
-          if (this._config.privateKeyFilePath) {
-            this._authHandler.privateKeyAuth(
-              cb,
-              this._config.privateKeyFilePath,
-              this._config.username,
-            );
-          } else if (process.env.SSH_AUTH_SOCK) {
-            this._authHandler.sshAgentAuth(cb, this._config.username);
-          }
-          break;
-        }
-        case "password": {
-          this._authHandler.passwordAuth(cb, this._config.username);
-          break;
+    const authMethod = authsLeft.shift();
+    switch (authMethod) {
+      case "publickey": {
+        //user set a keyfile path in profile config
+        if (this._config.privateKeyFilePath) {
+          this._authHandler.privateKeyAuth(
+            nextAuth,
+            this._config.privateKeyFilePath,
+            this._config.username,
+          );
+        } else if (process.env.SSH_AUTH_SOCK) {
+          this._authHandler.sshAgentAuth(nextAuth, this._config.username);
         }
-        case "keyboard-interactive": {
-          this._authHandler.keyboardInteractiveAuth(cb, this._config.username);
-          break;
-        }
-        default:
-          cb("none");
+        break;
       }
-    } else {
-      console.warn(`Server does not support ${authMethod} auth method.`);
-      cb("none");
+      case "password": {
+        this._authHandler.passwordAuth(nextAuth, this._config.username);
+        break;
+      }
+      case "keyboard-interactive": {
+        this._authHandler.keyboardInteractiveAuth(
+          nextAuth,
+          this._config.username,
+        );
+        break;
+      }
+      default:
+        nextAuth(authMethod);
     }
   };
 }