Add SSH TOTP authentication support #350

Merged
ZacharyZcR merged 3 commits from feature-ssh-totp into dev-1.8.0 2025-10-08 01:01:48 +00:00
ZacharyZcR commented 2025-10-05 07:16:46 +00:00 (Migrated from github.com)

Summary

  • Implements keyboard-interactive authentication for SSH TOTP
  • Adds reusable TOTP dialog component
  • Supports Terminal and File Manager
  • Disables Server Stats for TOTP-enabled servers (automatic polling incompatible with manual verification)

Test Plan

  • Terminal connects with TOTP
  • File Manager connects with TOTP
  • Server Stats shows proper error for TOTP servers
  • i18n support (EN/ZH)
## Summary - Implements keyboard-interactive authentication for SSH TOTP - Adds reusable TOTP dialog component - Supports Terminal and File Manager - Disables Server Stats for TOTP-enabled servers (automatic polling incompatible with manual verification) ## Test Plan - [x] Terminal connects with TOTP - [x] File Manager connects with TOTP - [x] Server Stats shows proper error for TOTP servers - [x] i18n support (EN/ZH)
gemini-code-assist[bot] commented 2025-10-05 07:17:07 +00:00 (Migrated from github.com)

Summary of Changes

Hello @ZacharyZcR, I'm Gemini Code Assist1 ! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the security of SSH connections by integrating Time-based One-Time Password (TOTP) authentication. It provides a seamless user experience for entering TOTP codes during SSH login for both the Terminal and File Manager, leveraging keyboard-interactive authentication. The changes also ensure that the Server Stats feature correctly identifies and handles servers configured with TOTP, preventing connection issues and informing users about the incompatibility with automatic data polling.

Highlights

  • SSH TOTP Authentication: Implemented Time-based One-Time Password (TOTP) authentication support for SSH connections in both the Terminal and File Manager features.
  • Reusable TOTP Dialog Component: Introduced a new, reusable UI component (TOTPDialog) to handle user input for TOTP verification codes.
  • Server Stats Handling: Server Stats functionality is now gracefully disabled for servers requiring TOTP authentication, as automatic polling is incompatible with manual verification. An appropriate error message is displayed to the user.
  • Internationalization (i18n): Added new translation keys for English and Chinese to support the new TOTP-related UI elements and messages.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩︎

## Summary of Changes Hello @ZacharyZcR, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the security of SSH connections by integrating Time-based One-Time Password (TOTP) authentication. It provides a seamless user experience for entering TOTP codes during SSH login for both the Terminal and File Manager, leveraging keyboard-interactive authentication. The changes also ensure that the Server Stats feature correctly identifies and handles servers configured with TOTP, preventing connection issues and informing users about the incompatibility with automatic data polling. ### Highlights * **SSH TOTP Authentication**: Implemented Time-based One-Time Password (TOTP) authentication support for SSH connections in both the Terminal and File Manager features. * **Reusable TOTP Dialog Component**: Introduced a new, reusable UI component (`TOTPDialog`) to handle user input for TOTP verification codes. * **Server Stats Handling**: Server Stats functionality is now gracefully disabled for servers requiring TOTP authentication, as automatic polling is incompatible with manual verification. An appropriate error message is displayed to the user. * **Internationalization (i18n)**: Added new translation keys for English and Chinese to support the new TOTP-related UI elements and messages. <details> <summary><b>Using Gemini Code Assist</b></summary> <br> The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips. <b>Invoking Gemini</b> You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page. Feature | Command | Description --- | --- | --- Code Review | `/gemini review` | Performs a code review for the current pull request in its current state. Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state. Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. Help | `/gemini help` | Displays a list of available commands. <b>Customization</b> To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). <b>Limitations & Feedback</b> Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG). <b>You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the [Gemini Code Assist IDE Extension](https://cloud.google.com/products/gemini/code-assist).</b> </details> [^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).
gemini-code-assist[bot] (Migrated from github.com) reviewed 2025-10-05 07:19:12 +00:00
gemini-code-assist[bot] (Migrated from github.com) left a comment

Code Review

This pull request introduces support for SSH TOTP (Time-based One-Time Password) authentication for both the Terminal and File Manager features. It adds a reusable TOTPDialog component for entering verification codes and modifies the backend SSH connection logic to handle the keyboard-interactive authentication flow required for TOTP. The changes are well-structured, but there are opportunities for improvement in error handling, type safety, and code duplication. I've identified a potential memory leak in how pending TOTP sessions are managed and a bug in the server-stats module's handling of keyboard-interactive prompts. Additionally, some refactoring could make the code more maintainable.

## Code Review This pull request introduces support for SSH TOTP (Time-based One-Time Password) authentication for both the Terminal and File Manager features. It adds a reusable `TOTPDialog` component for entering verification codes and modifies the backend SSH connection logic to handle the keyboard-interactive authentication flow required for TOTP. The changes are well-structured, but there are opportunities for improvement in error handling, type safety, and code duplication. I've identified a potential memory leak in how pending TOTP sessions are managed and a bug in the server-stats module's handling of keyboard-interactive prompts. Additionally, some refactoring could make the code more maintainable.
@@ -94,7 +94,16 @@ interface SSHSession {
timeout?: NodeJS.Timeout;
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:11 +00:00

medium

For better type safety and maintainability, the config property should be typed more specifically than any. The ConnectConfig type from the ssh2 library would be appropriate here.

  config: import("ssh2").ConnectConfig;
![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) For better type safety and maintainability, the `config` property should be typed more specifically than `any`. The `ConnectConfig` type from the `ssh2` library would be appropriate here. ```suggestion config: import("ssh2").ConnectConfig; ```
@@ -97,1 +103,4 @@
}
const sshSessions: Record<string, SSHSession> = {};
const pendingTOTPSessions: Record<string, PendingTOTPSession> = {};
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:11 +00:00

high

The pendingTOTPSessions object stores sessions awaiting TOTP verification. If a client initiates a connection but never completes the TOTP step, the corresponding entry in this object is never removed, leading to a memory leak. Consider implementing a periodic cleanup mechanism to remove stale entries from pendingTOTPSessions.

![high](https://www.gstatic.com/codereviewagent/high-priority.svg) The `pendingTOTPSessions` object stores sessions awaiting TOTP verification. If a client initiates a connection but never completes the TOTP step, the corresponding entry in this object is never removed, leading to a memory leak. Consider implementing a periodic cleanup mechanism to remove stale entries from `pendingTOTPSessions`.
@@ -370,0 +482,4 @@
fileLogger.success("TOTP verification successful", {
operation: "file_totp_verify",
sessionId,
userId,
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:11 +00:00

medium

The timeout value 120000 is a magic number. It's better to define it as a named constant at the top of the file for better readability and easier maintenance (e.g., const PENDING_TOTP_SESSION_TIMEOUT_MS = 120 * 1000;).

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The timeout value `120000` is a magic number. It's better to define it as a named constant at the top of the file for better readability and easier maintenance (e.g., `const PENDING_TOTP_SESSION_TIMEOUT_MS = 120 * 1000;`).
@@ -370,0 +485,4 @@
userId,
});
res.json({ status: "success", message: "TOTP verified, SSH connection established" });
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:11 +00:00

medium

Swallowing errors in an empty catch block can hide potential issues and make debugging difficult. It's better to at least log the error, even if it's just a warning.

    } catch (e) {
      fileLogger.warn("Failed to end client on TOTP timeout", { error: e instanceof Error ? e.message : String(e), sessionId });
    }
![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) Swallowing errors in an empty `catch` block can hide potential issues and make debugging difficult. It's better to at least log the error, even if it's just a warning. ```typescript } catch (e) { fileLogger.warn("Failed to end client on TOTP timeout", { error: e instanceof Error ? e.message : String(e), sessionId }); } ```
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:11 +00:00

medium

The timeout value 60000 is a magic number. It's better to define it as a named constant for better readability and easier maintenance (e.g., const TOTP_VERIFICATION_TIMEOUT_MS = 60 * 1000;).

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The timeout value `60000` is a magic number. It's better to define it as a named constant for better readability and easier maintenance (e.g., `const TOTP_VERIFICATION_TIMEOUT_MS = 60 * 1000;`).
@@ -95,6 +95,40 @@ class SSHConnectionPool {
reject(err);
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:11 +00:00

high

Calling finish([]) sends an empty array of responses. If the server expects a response for each prompt (even an empty string), this will likely cause authentication to fail. To ensure compatibility, you should provide one empty string response for each prompt.

            finish(prompts.map(() => ""));
![high](https://www.gstatic.com/codereviewagent/high-priority.svg) Calling `finish([])` sends an empty array of responses. If the server expects a response for each prompt (even an empty string), this will likely cause authentication to fail. To ensure compatibility, you should provide one empty string response for each prompt. ```suggestion finish(prompts.map(() => "")); ```
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:11 +00:00

medium

This if condition is redundant because throw error; is executed regardless of whether the condition is met. The catch block can be simplified by removing the conditional check.

      throw error;
![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) This `if` condition is redundant because `throw error;` is executed regardless of whether the condition is met. The `catch` block can be simplified by removing the conditional check. ```typescript throw error; ```
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:11 +00:00

medium

This regular expression for detecting TOTP prompts is duplicated across multiple files (file-manager.ts, server-stats.ts). To improve maintainability and ensure consistency, consider extracting it into a shared constant in a common utility file.

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) This regular expression for detecting TOTP prompts is duplicated across multiple files (`file-manager.ts`, `server-stats.ts`). To improve maintainability and ensure consistency, consider extracting it into a shared constant in a common utility file.
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:11 +00:00

medium

There is code duplication in this catch block. The calls to setMetrics(null) and setShowStatsUI(false) are repeated in every conditional branch. You can refactor this to set these states once after the conditional logic, making the code cleaner and easier to maintain.

                    if (error?.code === "TOTP_REQUIRED" ||
                        (error?.response?.status === 403 && error?.response?.data?.error === "TOTP_REQUIRED")) {
                      toast.error(t("serverStats.totpUnavailable"));
                    } else {
                      setServerStatus("offline");
                    }
                    setMetrics(null);
                    setShowStatsUI(false);
![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) There is code duplication in this `catch` block. The calls to `setMetrics(null)` and `setShowStatsUI(false)` are repeated in every conditional branch. You can refactor this to set these states once after the conditional logic, making the code cleaner and easier to maintain. ``` if (error?.code === "TOTP_REQUIRED" || (error?.response?.status === 403 && error?.response?.data?.error === "TOTP_REQUIRED")) { toast.error(t("serverStats.totpUnavailable")); } else { setServerStatus("offline"); } setMetrics(null); setShowStatsUI(false); ```
@@ -0,0 +42,4 @@
if (input && input.value.trim()) {
onSubmit(input.value.trim());
}
}}
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:12 +00:00

medium

The form submission logic directly accesses the form element from the event to get the input value. A more idiomatic React approach is to use a controlled component by managing the input's value with useState. This makes the component's state more predictable and easier to manage.

Example:

const [code, setCode] = React.useState('');

// in form
<Input value={code} onChange={(e) => setCode(e.target.value)} ... />

// in onSubmit
if (code.trim()) {
  onSubmit(code.trim());
}
![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) The form submission logic directly accesses the form element from the event to get the input value. A more idiomatic React approach is to use a controlled component by managing the input's value with `useState`. This makes the component's state more predictable and easier to manage. Example: ```tsx const [code, setCode] = React.useState(''); // in form <Input value={code} onChange={(e) => setCode(e.target.value)} ... /> // in onSubmit if (code.trim()) { onSubmit(code.trim()); } ```
gemini-code-assist[bot] (Migrated from github.com) commented 2025-10-05 07:19:12 +00:00

medium

Attaching the response object using as any bypasses TypeScript's type checking. For better type safety, consider adding an optional response property to the ApiError class definition. This would make the error structure more explicit and easier to work with.

![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg) Attaching the `response` object using `as any` bypasses TypeScript's type checking. For better type safety, consider adding an optional `response` property to the `ApiError` class definition. This would make the error structure more explicit and easier to work with.
Sign in to join this conversation.