Add browser client support #490
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive browser client support to the NetBird dashboard, introducing remote access functionality for SSH and RDP connections. The primary purpose is to enable users to access remote peers directly from the browser without requiring native NetBird clients.
Key changes:
- Introduces WebAssembly-based NetBird client for browser environments
- Implements SSH terminal functionality using xterm.js for in-browser SSH sessions
- Adds RDP support with IronRDP WASM integration for remote desktop access
- Creates temporary peer and policy management for ephemeral browser connections
Reviewed Changes
Copilot reviewed 65 out of 67 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/wireguard.ts | Implements Curve25519 cryptography for WireGuard key generation in browser |
| src/modules/remote-access/useNetBirdClient.ts | Core NetBird WASM client integration with connection management |
| src/modules/remote-access/ssh/Terminal.tsx | SSH terminal component using xterm.js for browser-based SSH sessions |
| src/modules/remote-access/rdp/useRemoteDesktop.ts | RDP connection management with certificate handling |
| src/modules/peers/PeersTable.tsx | Enhanced peer table with remote access buttons and browser peer filtering |
| src/components/PeerGroupSelector.tsx | Extended selector to support peer resources in access control policies |
| src/app/(remote-access)/peer/ssh/page.tsx | Full-screen SSH session page with connection lifecycle management |
| src/app/(remote-access)/peer/rdp/page.tsx | Full-screen RDP session page with credential and certificate modals |
Comments suppressed due to low confidence (1)
src/modules/remote-access/rdp/ironrdp-input-handler.ts:1
- Browser detection using user agent strings is fragile and unreliable. Consider using feature detection instead, such as checking for specific clipboard API capabilities.
/**
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| export const isNativeSSHSupported = (version: string) => { | ||
| if (version == "development") return true; | ||
| const versionNumber = parseVersionString(version); | ||
| return versionNumber >= 999; | ||
| }; |
There was a problem hiding this comment.
The version number 999 appears to be a magic number. Consider defining this as a named constant like NATIVE_SSH_MIN_VERSION = 999 to make the code more maintainable and self-documenting.
|
|
||
| const WASM_CONFIG = { | ||
| SCRIPT_PATH: "/wasm_exec.js", | ||
| WASM_PATH: "https://pkgs.netbird.io/wasm/client", |
There was a problem hiding this comment.
The hardcoded URL should be moved to configuration or environment variables to support different deployment environments (development, staging, production).
| WASM_PATH: "https://pkgs.netbird.io/wasm/client", | |
| WASM_PATH: config.NETBIRD_WASM_PATH || "https://pkgs.netbird.io/wasm/client", |
| destroy(): void; | ||
| } | ||
|
|
||
| const IRON_RDP_PKG = "/ironrdp-pkg/ironrdp_web.js"; |
There was a problem hiding this comment.
The hardcoded path should be configurable to support different deployment scenarios and CDN configurations.
| const IRON_RDP_PKG = "/ironrdp-pkg/ironrdp_web.js"; | |
| // Allow the WASM package path to be configured via window.IRON_RDP_PKG_PATH | |
| const IRON_RDP_PKG = | |
| typeof window !== "undefined" && typeof window.IRON_RDP_PKG_PATH === "string" | |
| ? window.IRON_RDP_PKG_PATH | |
| : "/ironrdp-pkg/ironrdp_web.js"; |
| } | ||
|
|
||
| // Don't send clipboard combinations here - let the clipboard events handle them (only for Chromium browsers) | ||
| const isChromium = /Chrome/.test(navigator.userAgent); |
There was a problem hiding this comment.
This browser detection pattern is repeated in the code. Consider extracting this into a utility function or using feature detection for clipboard capabilities instead.
| policy?.name?.startsWith("Temporary") && | ||
| policy?.name?.endsWith("client") && | ||
| policy?.description?.startsWith("Temporary") && | ||
| policy?.description?.endsWith("client") |
There was a problem hiding this comment.
The temporary policy detection logic is duplicated and uses magic strings. Consider extracting this into a utility function like isTempolicyPorary(policy) with defined constants for the prefixes and suffixes.
| (condition: boolean) => | ||
| peers?.filter((peer) => | ||
| condition | ||
| ? peer.kernel_version === "wasm" |
There was a problem hiding this comment.
The magic string 'wasm' should be defined as a constant like BROWSER_CLIENT_KERNEL_VERSION = 'wasm' to improve maintainability and prevent typos.
No description provided.