From 8bf0fe5ec7e08acaa3027e49d9bcfb5e68aa80d2 Mon Sep 17 00:00:00 2001 From: bifrost0x Date: Wed, 2 Sep 2026 19:45:22 +0200 Subject: [PATCH 1/2] fix: address current project feedback --- .env.example | 1 + app/ldap_service.py | 57 +++++++++--- app/security_features.py | 1 + app/session_insights.py | 29 ++++-- config.py | 55 +++++++----- docker-compose.ldap.yml | 2 + docs/ldap-authentication.md | 19 +++- docs/wiki/LDAP-and-Active-Directory.md | 9 ++ static/css/session-workspace.css | 2 +- static/css/sftp-file-manager.css | 9 ++ static/css/webssh-2.css | 18 +++- static/js/command-set-manager.js | 3 + static/js/command-workspace.js | 2 +- static/js/i18n.js | 6 ++ templates/admin.html | 4 +- templates/change_password.html | 4 +- templates/index.html | 14 +-- templates/login.html | 4 +- templates/register.html | 4 +- templates/security.html | 4 +- tests/e2e/file-workspace.spec.js | 19 ++++ .../e2e/primary-workspace-navigation.spec.js | 33 ++++++- tests/e2e/quick-connect-redesign.spec.js | 1 + tests/e2e/session-workspace.spec.js | 3 + tests/test_key_management_ui.py | 2 +- tests/test_ldap_service.py | 89 +++++++++++++++++++ tests/test_production_config.py | 65 ++++++++++++++ tests/test_profile_launcher_ui.py | 8 +- tests/test_security_features.py | 1 + tests/test_session_insights.py | 29 +++++- tests/test_webssh2_shell.py | 8 +- 31 files changed, 427 insertions(+), 78 deletions(-) diff --git a/.env.example b/.env.example index 30cdf3d1..623f73d1 100644 --- a/.env.example +++ b/.env.example @@ -136,6 +136,7 @@ LDAP_ENABLED=false LDAP_AUTO_PROVISION=false LDAP_PROVIDER_ID=default LDAP_URL= +LDAP_BACKUP_URL= LDAP_BASE_DN= LDAP_BIND_DN= LDAP_BIND_PASSWORD_FILE=/run/webssh-auth/ldap_bind_password diff --git a/app/ldap_service.py b/app/ldap_service.py index d1852994..ad1f9ed9 100644 --- a/app/ldap_service.py +++ b/app/ldap_service.py @@ -10,7 +10,7 @@ import os import ssl import stat -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path from urllib.parse import urlsplit @@ -39,6 +39,7 @@ class LDAPSettings: unique_id_attribute: str connect_timeout: int operation_timeout: int + backup_url: str = '' @classmethod def from_config(cls): @@ -55,8 +56,15 @@ def from_config(cls): unique_id_attribute=config.LDAP_UNIQUE_ID_ATTRIBUTE, connect_timeout=config.LDAP_CONNECT_TIMEOUT, operation_timeout=config.LDAP_OPERATION_TIMEOUT, + backup_url=getattr(config, 'LDAP_BACKUP_URL', ''), ) + def endpoints(self): + return tuple(filter(None, (self.url, self.backup_url))) + + def for_url(self, url): + return replace(self, url=url, backup_url='') + @dataclass(frozen=True) class LDAPIdentity: @@ -239,6 +247,26 @@ class LDAPDirectory: def __init__(self, settings=None, *, backend=None): self.settings = settings or LDAPSettings.from_config() self.backend = backend or BonsaiBackend() + self.active_url = None + + def _endpoint_settings(self): + urls = list(self.settings.endpoints()) + if self.active_url in urls: + urls.remove(self.active_url) + urls.insert(0, self.active_url) + return tuple(self.settings.for_url(url) for url in urls) + + def _with_failover(self, operation): + last_error = None + for endpoint_settings in self._endpoint_settings(): + try: + result = operation(endpoint_settings) + except LDAPUnavailable as exc: + last_error = exc + continue + self.active_url = endpoint_settings.url + return result + raise LDAPUnavailable('LDAP servers are unavailable') from last_error def lookup(self, username): normalized_username = str(username or '').strip() @@ -249,10 +277,12 @@ def lookup(self, username): ) bind_password = _read_secret(self.settings.bind_password_file) try: - entries = self.backend.search_user( - self.settings, - bind_password, - filter_expression, + entries = self._with_failover( + lambda endpoint_settings: self.backend.search_user( + endpoint_settings, + bind_password, + filter_expression, + ) ) finally: bind_password = None @@ -278,15 +308,22 @@ def lookup(self, username): def verify_password(self, distinguished_name, password): if not password: return False - return bool(self.backend.verify_password( - self.settings, - distinguished_name, - password, + return bool(self._with_failover( + lambda endpoint_settings: self.backend.verify_password( + endpoint_settings, + distinguished_name, + password, + ) )) def probe(self): bind_password = _read_secret(self.settings.bind_password_file) try: - return bool(self.backend.probe(self.settings, bind_password)) + return bool(self._with_failover( + lambda endpoint_settings: self.backend.probe( + endpoint_settings, + bind_password, + ) + )) finally: bind_password = None diff --git a/app/security_features.py b/app/security_features.py index 733eea55..717a52d9 100644 --- a/app/security_features.py +++ b/app/security_features.py @@ -48,6 +48,7 @@ 'ldap': ( 'LDAP_ENABLED', 'LDAP_URL', + 'LDAP_BACKUP_URL', 'LDAP_BASE_DN', 'LDAP_BIND_DN', 'LDAP_BIND_PASSWORD_FILE', diff --git a/app/session_insights.py b/app/session_insights.py index 1f153652..afe9fe37 100644 --- a/app/session_insights.py +++ b/app/session_insights.py @@ -56,6 +56,25 @@ """ +LINUX_NETWORK_AWK_PROGRAM = r"""NR > 2 { + separator = index($0, ":") + if (separator == 0) next + interface = substr($0, 1, separator - 1) + gsub(/^[[:space:]]+|[[:space:]]+$/, "", interface) + if (interface == "lo") next + counters = substr($0, separator + 1) + sub(/^[[:space:]]+/, "", counters) + count = split(counters, fields, /[[:space:]]+/) + if (count >= 16) { + received += fields[1] + transmitted += fields[9] + } + } END { + print "network_received_bytes=" received + 0 + print "network_transmitted_bytes=" transmitted + 0 + }""" + + LINUX_DIAGNOSTICS_COMMAND = LINUX_STATS_COMMAND + r""" if [ -r /proc/meminfo ] && command -v awk >/dev/null 2>&1; then awk ' @@ -76,15 +95,7 @@ }' /proc/stat 2>/dev/null fi if [ -r /proc/net/dev ] && command -v awk >/dev/null 2>&1; then - awk -F '[: ]+' 'NR > 2 { - if ($2 != "lo") { - received += $3 - transmitted += $11 - } - } END { - print "network_received_bytes=" received + 0 - print "network_transmitted_bytes=" transmitted + 0 - }' /proc/net/dev 2>/dev/null + awk '""" + LINUX_NETWORK_AWK_PROGRAM + r"""' /proc/net/dev 2>/dev/null fi process_probe=$(ps -p $$ -o pid= 2>&1) diff --git a/config.py b/config.py index e03efa51..7a241b77 100644 --- a/config.py +++ b/config.py @@ -91,6 +91,7 @@ ) LDAP_PROVIDER_ID = os.environ.get('LDAP_PROVIDER_ID', 'default').strip() LDAP_URL = os.environ.get('LDAP_URL', '').strip() +LDAP_BACKUP_URL = os.environ.get('LDAP_BACKUP_URL', '').strip() LDAP_BASE_DN = os.environ.get('LDAP_BASE_DN', '').strip() LDAP_BIND_DN = os.environ.get('LDAP_BIND_DN', '').strip() LDAP_BIND_PASSWORD_FILE = os.environ.get( @@ -707,28 +708,38 @@ def _canonical_smb_target(raw_value): 'LDAP_ENABLED is true' ) - ldap_url_error = ( - 'SECURITY ERROR: LDAP_URL must be an exact ldap:// or ' - 'ldaps:// server URL without credentials, path, query, or ' - 'fragment and with a valid port' - ) - try: - parsed_ldap_url = urlsplit(LDAP_URL) - ldap_hostname = parsed_ldap_url.hostname - ldap_port = parsed_ldap_url.port - except ValueError as exc: - raise RuntimeError(ldap_url_error) from exc - if ( - parsed_ldap_url.scheme not in {'ldap', 'ldaps'} - or not ldap_hostname - or (ldap_port is not None and ldap_port < 1) - or parsed_ldap_url.username is not None - or parsed_ldap_url.password is not None - or parsed_ldap_url.path - or parsed_ldap_url.query - or parsed_ldap_url.fragment - ): - raise RuntimeError(ldap_url_error) + def _validate_ldap_url(setting_name, value): + ldap_url_error = ( + f'SECURITY ERROR: {setting_name} must be an exact ldap:// or ' + 'ldaps:// server URL without credentials, path, query, or ' + 'fragment and with a valid port' + ) + try: + parsed_ldap_url = urlsplit(value) + ldap_hostname = parsed_ldap_url.hostname + ldap_port = parsed_ldap_url.port + except ValueError as exc: + raise RuntimeError(ldap_url_error) from exc + if ( + parsed_ldap_url.scheme not in {'ldap', 'ldaps'} + or not ldap_hostname + or (ldap_port is not None and ldap_port < 1) + or parsed_ldap_url.username is not None + or parsed_ldap_url.password is not None + or parsed_ldap_url.path + or parsed_ldap_url.query + or parsed_ldap_url.fragment + ): + raise RuntimeError(ldap_url_error) + + _validate_ldap_url('LDAP_URL', LDAP_URL) + if LDAP_BACKUP_URL: + _validate_ldap_url('LDAP_BACKUP_URL', LDAP_BACKUP_URL) + if LDAP_BACKUP_URL == LDAP_URL: + raise RuntimeError( + 'SECURITY ERROR: LDAP_BACKUP_URL must identify a ' + 'different server than LDAP_URL' + ) if LDAP_USER_FILTER.count('{username}') != 1: raise RuntimeError( 'SECURITY ERROR: LDAP_USER_FILTER must contain exactly one ' diff --git a/docker-compose.ldap.yml b/docker-compose.ldap.yml index 0ddcc8f9..1af3a166 100644 --- a/docker-compose.ldap.yml +++ b/docker-compose.ldap.yml @@ -7,6 +7,8 @@ services: LDAP_AUTO_PROVISION: "false" LDAP_PROVIDER_ID: default LDAP_URL: "" + # Optional failover endpoint for the same directory and CA trust. + LDAP_BACKUP_URL: "" LDAP_BASE_DN: "" LDAP_BIND_DN: "" LDAP_USER_FILTER: "" diff --git a/docs/ldap-authentication.md b/docs/ldap-authentication.md index 813cce36..6ae0766b 100644 --- a/docs/ldap-authentication.md +++ b/docs/ldap-authentication.md @@ -41,7 +41,8 @@ the provided `docker-compose.ldap.yml` overlay with the same WebSSH image; no Collect these values before activation: -1. An LDAP server DNS name whose TLS certificate matches that name. +1. A primary LDAP server DNS name whose TLS certificate matches that name and, + optionally, a backup server with its own certificate-matching DNS name. 2. Whether to use `ldap://host:389` with StartTLS or `ldaps://host:636`. 3. The user search base DN. 4. A read-only bind account DN and its password. It needs only enough access to @@ -67,6 +68,7 @@ LDAP_ENABLED: "true" LDAP_AUTO_PROVISION: "false" LDAP_PROVIDER_ID: corp-ad LDAP_URL: ldaps://dc01.ad.example.com:636 +LDAP_BACKUP_URL: ldaps://dc02.ad.example.com:636 LDAP_BASE_DN: OU=People,DC=ad,DC=example,DC=com LDAP_BIND_DN: CN=svc-webssh,OU=Service Accounts,DC=ad,DC=example,DC=com LDAP_USER_FILTER: "(&(objectCategory=person)(objectClass=user)(sAMAccountName={username})(!(userAccountControl:1.2.840.113556.1.4.803:=2)))" @@ -77,6 +79,14 @@ The final filter clause excludes disabled AD accounts. If the organization has a dedicated WebSSH access group, add a directory-approved `memberOf` clause. Nested group semantics vary and must be validated by the AD administrator. +`LDAP_BACKUP_URL` is optional and must point to the same logical directory as +`LDAP_URL`. WebSSH tries it only when the primary endpoint is unavailable. Each +URL is connected to directly, so the CA bundle must validate the DNS name in +that endpoint's certificate; a round-robin alias is not required. A completed +bind that rejects a user's password is authoritative and is not retried against +the other endpoint. Keep the provider ID, base DN, bind account, filter, and +stable-ID attribute identical across both endpoints. + ### Generic OpenLDAP example ```yaml @@ -84,6 +94,7 @@ LDAP_ENABLED: "true" LDAP_AUTO_PROVISION: "false" LDAP_PROVIDER_ID: primary-openldap LDAP_URL: ldap://ldap.example.com:389 +# LDAP_BACKUP_URL: ldap://ldap-backup.example.com:389 LDAP_BASE_DN: ou=people,dc=example,dc=com LDAP_BIND_DN: cn=svc-webssh,ou=services,dc=example,dc=com LDAP_USER_FILTER: "(&(objectClass=inetOrgPerson)(uid={username})(!(pwdAccountLockedTime=*)))" @@ -204,8 +215,10 @@ remain in the separate secret volume and are intentionally not included. administrator. WebSSH never chooses one result from an ambiguous search. - **AD user is found but cannot bind:** check disabled/locked/expired state, logon restrictions, and whether the DC accepts the supplied username flow. -- **LDAP outage signs users out:** this is intentional fail-closed behavior. - Restore directory/TLS service; do not enable a password fallback. +- **LDAP outage signs users out:** WebSSH tries the optional backup endpoint + after a transport failure. If neither endpoint is available, the intentional + fail-closed behavior applies. Restore directory/TLS service; do not enable a + password fallback. - **Provider ID changed:** restore the original `LDAP_PROVIDER_ID`. It is part of every stable mapping and should remain constant for that directory. diff --git a/docs/wiki/LDAP-and-Active-Directory.md b/docs/wiki/LDAP-and-Active-Directory.md index 8eca7565..b7e9c233 100644 --- a/docs/wiki/LDAP-and-Active-Directory.md +++ b/docs/wiki/LDAP-and-Active-Directory.md @@ -61,6 +61,7 @@ services: LDAP_ENABLED: "true" LDAP_PROVIDER_ID: corp-ad LDAP_URL: ldaps://dc01.ad.example.com:636 + LDAP_BACKUP_URL: ldaps://dc02.ad.example.com:636 LDAP_BASE_DN: OU=People,DC=ad,DC=example,DC=com LDAP_BIND_DN: CN=svc-webssh,OU=Service Accounts,DC=ad,DC=example,DC=com LDAP_USER_FILTER: "(&(objectCategory=person)(objectClass=user)(sAMAccountName={username})(!(userAccountControl:1.2.840.113556.1.4.803:=2)))" @@ -71,6 +72,12 @@ The final filter clause excludes disabled AD accounts. If access is restricted to a group, use a directory-approved `memberOf` rule. Nested group semantics vary and must be validated by the AD administrator. +`LDAP_BACKUP_URL` is optional and must identify another server for the same +logical directory. It is tried only after the primary endpoint is unavailable. +WebSSH connects to each URL directly, so every endpoint DNS name must match its +own TLS certificate and be trusted by `LDAP_CA_FILE`; a round-robin alias is not +needed. Invalid user credentials are not retried against the backup endpoint. + ### OpenLDAP example ```yaml @@ -80,6 +87,7 @@ services: LDAP_ENABLED: "true" LDAP_PROVIDER_ID: primary-openldap LDAP_URL: ldap://ldap.example.com:389 + # LDAP_BACKUP_URL: ldap://ldap-backup.example.com:389 LDAP_BASE_DN: ou=people,dc=example,dc=com LDAP_BIND_DN: cn=svc-webssh,ou=services,dc=example,dc=com LDAP_USER_FILTER: "(&(objectClass=inetOrgPerson)(uid={username})(!(pwdAccountLockedTime=*)))" @@ -96,6 +104,7 @@ attribute is not available. | `LDAP_ENABLED` | `false` | Enable the subsystem | | `LDAP_PROVIDER_ID` | `default` | Stable 1-64 character local provider identifier | | `LDAP_URL` | empty | Exact `ldap://` or `ldaps://` server URL | +| `LDAP_BACKUP_URL` | empty | Optional second URL for the same directory, tried after transport failure | | `LDAP_BASE_DN` | empty | User subtree base | | `LDAP_BIND_DN` | empty | Least-privilege search account DN | | `LDAP_BIND_PASSWORD_FILE` | `/run/webssh-auth/ldap_bind_password` | Absolute private file path | diff --git a/static/css/session-workspace.css b/static/css/session-workspace.css index 9c7c178a..d26da5a4 100644 --- a/static/css/session-workspace.css +++ b/static/css/session-workspace.css @@ -158,7 +158,7 @@ .fm-embedded-mode .fm-toolbar-right { display: grid; - grid-template-columns: repeat(5, minmax(0, 1fr)); + grid-template-columns: repeat(6, minmax(0, 1fr)); gap: 4px; } diff --git a/static/css/sftp-file-manager.css b/static/css/sftp-file-manager.css index 425e4908..b3a8af69 100644 --- a/static/css/sftp-file-manager.css +++ b/static/css/sftp-file-manager.css @@ -2604,6 +2604,11 @@ background: var(--bg-primary); } +.fm-source-search:focus-within { + outline: 2px solid var(--ws2-focus, var(--accent-primary)); + outline-offset: 2px; +} + .fm-source-search .material-icons { color: var(--text-muted); font-size: 19px; @@ -2618,6 +2623,10 @@ color: var(--text-primary); } +.fm-source-search input:focus-visible { + outline: 0 !important; +} + .fm-source-groups { display: flex; min-height: 0; diff --git a/static/css/webssh-2.css b/static/css/webssh-2.css index 71cb4913..5cabb2f0 100644 --- a/static/css/webssh-2.css +++ b/static/css/webssh-2.css @@ -1694,6 +1694,14 @@ body:has(.auth-container) { gap: 10px; } +.primary-workspace-surface .management-panel-actions > .btn { + display: inline-flex; + height: 38px; + align-items: center; + justify-content: center; + gap: 6px; +} + .primary-workspace-surface .profile-management-list { gap: 0; margin-top: 14px; @@ -1976,7 +1984,15 @@ body:has(.auth-container) { } .primary-workspace-surface #commandLibraryPanel { - gap: 14px; + gap: 0; +} + +.primary-workspace-surface #commandLibraryPanel > :is( + .os-filter-toolbar, + .commands-table-wrapper, + .command-help +) { + margin-top: 14px; } .primary-workspace-surface .command-toolbar { diff --git a/static/js/command-set-manager.js b/static/js/command-set-manager.js index be442fe4..eba24ead 100644 --- a/static/js/command-set-manager.js +++ b/static/js/command-set-manager.js @@ -211,6 +211,9 @@ window.CommandSetManager = { this.returnToConnection = false; this.showManagementList(); this.openModal(); + setTimeout(() => { + document.getElementById('commandSetManagementSearch')?.focus(); + }, 0); }, openBuilder(id = null, returnToConnection = false) { diff --git a/static/js/command-workspace.js b/static/js/command-workspace.js index 6c6efb53..7d8b7a21 100644 --- a/static/js/command-workspace.js +++ b/static/js/command-workspace.js @@ -60,7 +60,7 @@ window.CommandWorkspace = { if (focusContent) { const target = next === 'library' ? document.getElementById('commandSearchInput') - : document.getElementById('newCommandSetBtn'); + : document.getElementById('commandSetManagementSearch'); setTimeout(() => target?.focus(), 0); } }, diff --git a/static/js/i18n.js b/static/js/i18n.js index 25a66a01..7b868d1b 100644 --- a/static/js/i18n.js +++ b/static/js/i18n.js @@ -38,6 +38,7 @@ const translations = { 'connection.recentConnections': 'Recent Connections', 'connection.recentConnectionsHint': 'Stored only in this browser for your account.', 'connection.noRecentConnections': 'Your recent connections will appear here.', + 'time.justNow': 'Just now', 'connection.details': 'Connection Details', 'connection.detailsHint': 'Enter the destination and choose how to authenticate.', 'connection.savedContext': 'Saved connection', @@ -1289,6 +1290,7 @@ const translations = { 'connection.recentConnections': 'Kết nối gần đây', 'connection.recentConnectionsHint': 'Chỉ được lưu trong trình duyệt này cho tài khoản của bạn.', 'connection.noRecentConnections': 'Các kết nối gần đây của bạn sẽ xuất hiện tại đây.', + 'time.justNow': 'Vừa xong', 'connection.details': 'Chi tiết kết nối', 'connection.detailsHint': 'Nhập đích đến và chọn cách xác thực.', 'connection.savedContext': 'Kết nối đã lưu', @@ -2539,6 +2541,7 @@ const translations = { 'connection.recentConnections': 'Letzte Verbindungen', 'connection.recentConnectionsHint': 'Wird nur in diesem Browser für deinen Account gespeichert.', 'connection.noRecentConnections': 'Deine letzten Verbindungen erscheinen hier.', + 'time.justNow': 'Gerade eben', 'connection.details': 'Verbindungsdetails', 'connection.detailsHint': 'Gib das Ziel ein und wähle die Authentifizierung.', 'connection.savedContext': 'Gespeicherte Verbindung', @@ -3788,6 +3791,7 @@ const translations = { 'connection.recentConnections': 'Connexions récentes', 'connection.recentConnectionsHint': 'Stockées uniquement dans ce navigateur pour votre compte.', 'connection.noRecentConnections': 'Vos connexions récentes apparaîtront ici.', + 'time.justNow': 'À l’instant', 'connection.details': 'Détails de connexion', 'connection.detailsHint': 'Saisissez la destination et choisissez le mode d’authentification.', 'connection.savedContext': 'Connexion enregistrée', @@ -5037,6 +5041,7 @@ const translations = { 'connection.recentConnections': 'Conexiones recientes', 'connection.recentConnectionsHint': 'Se guardan solo en este navegador para tu cuenta.', 'connection.noRecentConnections': 'Tus conexiones recientes aparecerán aquí.', + 'time.justNow': 'Ahora mismo', 'connection.details': 'Detalles de conexión', 'connection.detailsHint': 'Introduce el destino y elige cómo autenticarte.', 'connection.savedContext': 'Conexión guardada', @@ -6286,6 +6291,7 @@ const translations = { 'connection.recentConnections': '最近连接', 'connection.recentConnectionsHint': '仅在此浏览器中为你的账户保存。', 'connection.noRecentConnections': '你的最近连接将显示在这里。', + 'time.justNow': '刚刚', 'connection.details': '连接详情', 'connection.detailsHint': '输入目标并选择身份验证方式。', 'connection.savedContext': '已保存的连接', diff --git a/templates/admin.html b/templates/admin.html index c115735a..c25c1681 100644 --- a/templates/admin.html +++ b/templates/admin.html @@ -14,7 +14,7 @@ - +
@@ -448,7 +448,7 @@

Final destru {% if not embedded %}
- + diff --git a/templates/change_password.html b/templates/change_password.html index 4ada2877..eb58d719 100644 --- a/templates/change_password.html +++ b/templates/change_password.html @@ -8,7 +8,7 @@ - +
@@ -119,7 +119,7 @@

Change Password

- + diff --git a/templates/index.html b/templates/index.html index a8b881d2..59c95662 100644 --- a/templates/index.html +++ b/templates/index.html @@ -18,9 +18,9 @@ - - - + + + @@ -959,7 +959,7 @@

Command Library

@@ -1418,7 +1418,7 @@

File Preview

window.WEBSSH_TRANSFER_LIMITS = Object.freeze({{ transfer_limits | tojson }}); window.WEBSSH_SSH_INPUT_LIMITS = Object.freeze({{ ssh_input_limits | tojson }}); - + @@ -1434,7 +1434,7 @@

File Preview

- + @@ -1447,7 +1447,7 @@

File Preview

- + diff --git a/templates/login.html b/templates/login.html index 59ff4772..d7d0c643 100644 --- a/templates/login.html +++ b/templates/login.html @@ -11,7 +11,7 @@ - + @@ -320,7 +320,7 @@

Continue with GitHub

- + diff --git a/templates/register.html b/templates/register.html index 9c1a7a21..cef22563 100644 --- a/templates/register.html +++ b/templates/register.html @@ -8,7 +8,7 @@ - + @@ -136,7 +136,7 @@

Create a local WebSSH account

- + diff --git a/templates/security.html b/templates/security.html index 081758ac..fa0b446e 100644 --- a/templates/security.html +++ b/templates/security.html @@ -14,7 +14,7 @@ - +
@@ -412,7 +412,7 @@

Co
- + diff --git a/tests/e2e/file-workspace.spec.js b/tests/e2e/file-workspace.spec.js index 523df2f2..f2c62cda 100644 --- a/tests/e2e/file-workspace.spec.js +++ b/tests/e2e/file-workspace.spec.js @@ -111,6 +111,25 @@ test('source-first workspace preserves panes and exposes only functional SFTP ac await expect(page.locator('#fmNewSmbSource')).toBeDisabled(); await expect(page.locator('#fmNewSmbSource')).toContainText('Disabled by administrator'); await expect(page.locator('#fmSourceSearch')).toBeFocused(); + const sourceSearchFocus = await page.locator('.fm-source-search').evaluate(label => { + const input = label.querySelector('input'); + const labelStyle = getComputedStyle(label); + const inputStyle = getComputedStyle(input); + const labelBounds = label.getBoundingClientRect(); + const inputBounds = input.getBoundingClientRect(); + return { + labelOutline: labelStyle.outlineStyle, + inputOutline: inputStyle.outlineStyle, + labelLeft: Math.round(labelBounds.left), + labelRight: Math.round(labelBounds.right), + inputLeft: Math.round(inputBounds.left), + inputRight: Math.round(inputBounds.right), + }; + }); + expect(sourceSearchFocus.labelOutline).toBe('solid'); + expect(sourceSearchFocus.inputOutline).toBe('none'); + expect(sourceSearchFocus.labelLeft).toBeLessThan(sourceSearchFocus.inputLeft); + expect(sourceSearchFocus.labelRight).toBeGreaterThanOrEqual(sourceSearchFocus.inputRight); await page.keyboard.press('Shift+Tab'); await expect(page.locator('#fmSourceLauncherClose')).toBeFocused(); await page.keyboard.press('Shift+Tab'); diff --git a/tests/e2e/primary-workspace-navigation.spec.js b/tests/e2e/primary-workspace-navigation.spec.js index 0bc48fb6..c0832dda 100644 --- a/tests/e2e/primary-workspace-navigation.spec.js +++ b/tests/e2e/primary-workspace-navigation.spec.js @@ -35,10 +35,15 @@ async function managementPanelGeometry(page, panelId, contentSelector) { const actions = panel.querySelector('.management-panel-actions').getBoundingClientRect(); const content = panel.querySelector(contentSelector).getBoundingClientRect(); return { + headingTop: Math.round(heading.top), headingBottom: Math.round(heading.bottom), toolbarTop: Math.round(toolbar.top), toolbarBottom: Math.round(toolbar.bottom), + searchTop: Math.round(search.top), + searchHeight: Math.round(search.height), searchCenter: Math.round((search.top + search.bottom) / 2), + actionsTop: Math.round(actions.top), + actionsHeight: Math.round(actions.height), actionsCenter: Math.round((actions.top + actions.bottom) / 2), contentTop: Math.round(content.top), }; @@ -110,11 +115,13 @@ test('Hosts, File Manager, and Commands share the main surface while Workspaces await expect(page.locator('#commandSetsPanel .management-panel-heading p')).toHaveText( 'Build reusable command sequences and assign one to any saved connection.', ); - expectManagementPanelHierarchy(await managementPanelGeometry( + await expect(page.locator('#commandSetManagementSearch')).toBeFocused(); + const commandSetsGeometry = await managementPanelGeometry( page, '#commandSetsPanel', '#commandSetManagementList', - )); + ); + expectManagementPanelHierarchy(commandSetsGeometry); await page.locator('#commandLibraryTab').click(); await expect(page.locator('#commandLibraryPanel')).toBeVisible(); await expect(page.locator('#commandSearchInput')).toBeFocused(); @@ -122,11 +129,29 @@ test('Hosts, File Manager, and Commands share the main surface while Workspaces await expect(page.locator('#commandLibraryPanel .management-panel-heading p')).toHaveText( 'Browse, search, and manage commands available to your sessions.', ); - expectManagementPanelHierarchy(await managementPanelGeometry( + const commandLibraryGeometry = await managementPanelGeometry( page, '#commandLibraryPanel', '.os-filter-toolbar', - )); + ); + expectManagementPanelHierarchy(commandLibraryGeometry); + expect({ + headingTop: commandLibraryGeometry.headingTop, + toolbarTop: commandLibraryGeometry.toolbarTop, + searchTop: commandLibraryGeometry.searchTop, + searchHeight: commandLibraryGeometry.searchHeight, + actionsTop: commandLibraryGeometry.actionsTop, + actionsHeight: commandLibraryGeometry.actionsHeight, + }).toEqual({ + headingTop: commandSetsGeometry.headingTop, + toolbarTop: commandSetsGeometry.toolbarTop, + searchTop: commandSetsGeometry.searchTop, + searchHeight: commandSetsGeometry.searchHeight, + actionsTop: commandSetsGeometry.actionsTop, + actionsHeight: commandSetsGeometry.actionsHeight, + }); + await page.locator('#commandSetsTab').click(); + await expect(page.locator('#commandSetManagementSearch')).toBeFocused(); await page.locator('#fileTransferBtn').click(); await expect(page.locator('#fileTransferBtn')).toHaveAttribute('aria-current', 'page'); diff --git a/tests/e2e/quick-connect-redesign.spec.js b/tests/e2e/quick-connect-redesign.spec.js index f04cc25c..b10b8365 100644 --- a/tests/e2e/quick-connect-redesign.spec.js +++ b/tests/e2e/quick-connect-redesign.spec.js @@ -73,6 +73,7 @@ test('presents a focused two-column quick connect without a saved-profile picker await expect(page.locator('#connectionProfileContext')).toHaveClass(/hidden/); await expect(page.locator('.recent-connection-item').first()).toHaveRole('button'); await expect(page.locator('.recent-connection-item')).toHaveCount(7); + await expect(page.locator('.recent-conn-time').first()).toHaveText('Just now'); await expect(page.locator('#recentConnectionsCard')).toHaveAttribute('open', ''); const historyViewport = await page.locator('#recentConnectionsList').evaluate(list => { diff --git a/tests/e2e/session-workspace.spec.js b/tests/e2e/session-workspace.spec.js index 94d4d49e..c8c1987d 100644 --- a/tests/e2e/session-workspace.spec.js +++ b/tests/e2e/session-workspace.spec.js @@ -355,6 +355,8 @@ test('single-session workspace keeps terminal primary with on-demand Files, Diag return { maxRowHeight: Math.max(...fileRows.map(row => bounds(row).height)), toolbarOverflow: toolbar.scrollWidth - toolbar.clientWidth, + toolbarRows: new Set(Array.from(toolbar.querySelectorAll('button')) + .map(button => Math.round(bounds(button).top))).size, toolbarButtonsInside: Array.from(toolbar.querySelectorAll('button')).every(button => { const buttonBounds = bounds(button); return buttonBounds.left >= toolbarBounds.left @@ -367,6 +369,7 @@ test('single-session workspace keeps terminal primary with on-demand Files, Diag }); expect(embeddedFileLayout.maxRowHeight).toBeLessThanOrEqual(46); expect(embeddedFileLayout.toolbarOverflow).toBeLessThanOrEqual(1); + expect(embeddedFileLayout.toolbarRows).toBe(1); expect(embeddedFileLayout.toolbarButtonsInside).toBe(true); expect(embeddedFileLayout.xtermPadding).toBe('0px'); expect(embeddedFileLayout.viewportBackground).toBe(embeddedFileLayout.terminalBackground); diff --git a/tests/test_key_management_ui.py b/tests/test_key_management_ui.py index d68b0b24..c8c0437d 100644 --- a/tests/test_key_management_ui.py +++ b/tests/test_key_management_ui.py @@ -102,7 +102,7 @@ def test_key_replacement_event_updates_ui_and_asset_version(): assert "socket.on('key_replaced'" in APP assert 'ProfileManager.upsertKeySummary(data.key)' in APP assert "filename='js/profile-manager.js') }}?v=19" in TEMPLATE - assert "filename='js/i18n.js') }}?v=44" in TEMPLATE + assert "filename='js/i18n.js') }}?v=45" in TEMPLATE assert "filename='js/app.js') }}?v=28" in TEMPLATE assert "filename='css/style.css') }}?v=24" in TEMPLATE diff --git a/tests/test_ldap_service.py b/tests/test_ldap_service.py index 77447bc4..0f76dafe 100644 --- a/tests/test_ldap_service.py +++ b/tests/test_ldap_service.py @@ -30,6 +30,37 @@ def verify_password(self, settings, distinguished_name, password): return not self.reject_password +class _FailoverBackend: + def __init__(self, entries, *, unavailable=(), reject_password=False): + self.entries = list(entries) + self.unavailable = set(unavailable) + self.reject_password = reject_password + self.search_urls = [] + self.bind_urls = [] + self.probe_urls = [] + + def _check_available(self, settings): + from app.ldap_service import LDAPUnavailable + + if settings.url in self.unavailable: + raise LDAPUnavailable('test endpoint unavailable') + + def search_user(self, settings, _bind_password, _filter_expression): + self.search_urls.append(settings.url) + self._check_available(settings) + return self.entries + + def verify_password(self, settings, _distinguished_name, _password): + self.bind_urls.append(settings.url) + self._check_available(settings) + return not self.reject_password + + def probe(self, settings, _bind_password): + self.probe_urls.append(settings.url) + self._check_available(settings) + return True + + def _settings(tmp_path): from app.ldap_service import LDAPSettings @@ -157,6 +188,64 @@ def test_directory_authentication_maps_bind_rejection_to_false(tmp_path): assert backend.bind_calls[0][2] == "user-secret" +def test_directory_fails_over_and_keeps_the_working_endpoint_for_user_bind( + tmp_path, +): + from dataclasses import replace + + from app.ldap_service import LDAPDirectory, LDAPIdentity + + primary = 'ldaps://dc01.ad.example.com:636' + backup = 'ldaps://dc02.ad.example.com:636' + settings = replace(_settings(tmp_path), url=primary, backup_url=backup) + identity = LDAPIdentity( + provider='primary', + subject='stable-id', + distinguished_name='uid=alice,dc=example,dc=com', + ) + backend = _FailoverBackend([identity], unavailable={primary}) + directory = LDAPDirectory(settings, backend=backend) + + assert directory.lookup('alice') == identity + backend.unavailable.clear() + assert directory.verify_password(identity.distinguished_name, 'user-secret') + + assert backend.search_urls == [primary, backup] + assert backend.bind_urls == [backup] + assert directory.active_url == backup + + +def test_directory_does_not_retry_an_authoritative_password_rejection(tmp_path): + from dataclasses import replace + + from app.ldap_service import LDAPDirectory + + primary = 'ldaps://dc01.ad.example.com:636' + backup = 'ldaps://dc02.ad.example.com:636' + settings = replace(_settings(tmp_path), url=primary, backup_url=backup) + backend = _FailoverBackend([], reject_password=True) + directory = LDAPDirectory(settings, backend=backend) + + assert directory.verify_password('uid=alice,dc=example,dc=com', 'wrong') is False + assert backend.bind_urls == [primary] + + +def test_directory_readiness_uses_the_backup_after_transport_failure(tmp_path): + from dataclasses import replace + + from app.ldap_service import LDAPDirectory + + primary = 'ldap://dc01.ad.example.com:389' + backup = 'ldap://dc02.ad.example.com:389' + settings = replace(_settings(tmp_path), url=primary, backup_url=backup) + backend = _FailoverBackend([], unavailable={primary}) + directory = LDAPDirectory(settings, backend=backend) + + assert directory.probe() is True + assert backend.probe_urls == [primary, backup] + assert directory.active_url == backup + + def test_bind_secret_is_bounded_and_never_taken_from_environment( tmp_path, monkeypatch, diff --git a/tests/test_production_config.py b/tests/test_production_config.py index c7c03f5a..0e48ca5e 100644 --- a/tests/test_production_config.py +++ b/tests/test_production_config.py @@ -18,6 +18,7 @@ 'DEPLOYMENT_PROFILE', 'LDAP_BASE_DN', 'LDAP_AUTO_PROVISION', + 'LDAP_BACKUP_URL', 'LDAP_BIND_DN', 'LDAP_BIND_PASSWORD_FILE', 'LDAP_CA_FILE', @@ -305,6 +306,69 @@ def test_enabled_ldap_accepts_starttls_with_bounded_timeouts(): assert result.stdout.splitlines()[-1] == '4 6' +def test_enabled_ldap_accepts_a_distinct_tls_verified_backup_endpoint(): + result = _load_config( + _production_env( + LDAP_ENABLED='true', + LDAP_URL='ldaps://dc01.ad.example.com:636', + LDAP_BACKUP_URL='ldaps://dc02.ad.example.com:636', + LDAP_BASE_DN='ou=people,dc=ad,dc=example,dc=com', + LDAP_BIND_DN='cn=webssh,ou=services,dc=ad,dc=example,dc=com', + LDAP_BIND_PASSWORD_FILE='/run/webssh-auth/ldap_bind_password', + LDAP_CA_FILE='/run/webssh-auth/ldap_ca.pem', + LDAP_USER_FILTER='(&(objectClass=person)(uid={username}))', + LDAP_UNIQUE_ID_ATTRIBUTE='objectGUID', + ), + 'import config; print(config.LDAP_BACKUP_URL)', + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.splitlines()[-1] == 'ldaps://dc02.ad.example.com:636' + + +@pytest.mark.parametrize( + 'backup_url', + ( + 'http://dc02.ad.example.com', + 'ldaps://user:secret@dc02.ad.example.com:636', + 'ldap://dc02.ad.example.com:bad', + ), +) +def test_enabled_ldap_rejects_an_unsafe_backup_endpoint(backup_url): + result = _load_config(_production_env( + LDAP_ENABLED='true', + LDAP_URL='ldaps://dc01.ad.example.com:636', + LDAP_BACKUP_URL=backup_url, + LDAP_BASE_DN='ou=people,dc=ad,dc=example,dc=com', + LDAP_BIND_DN='cn=webssh,ou=services,dc=ad,dc=example,dc=com', + LDAP_BIND_PASSWORD_FILE='/run/webssh-auth/ldap_bind_password', + LDAP_CA_FILE='/run/webssh-auth/ldap_ca.pem', + LDAP_USER_FILTER='(&(objectClass=person)(uid={username}))', + LDAP_UNIQUE_ID_ATTRIBUTE='objectGUID', + )) + + assert result.returncode != 0 + assert 'LDAP_BACKUP_URL' in result.stdout + result.stderr + + +def test_enabled_ldap_rejects_a_duplicate_backup_endpoint(): + url = 'ldaps://dc01.ad.example.com:636' + result = _load_config(_production_env( + LDAP_ENABLED='true', + LDAP_URL=url, + LDAP_BACKUP_URL=url, + LDAP_BASE_DN='ou=people,dc=ad,dc=example,dc=com', + LDAP_BIND_DN='cn=webssh,ou=services,dc=ad,dc=example,dc=com', + LDAP_BIND_PASSWORD_FILE='/run/webssh-auth/ldap_bind_password', + LDAP_CA_FILE='/run/webssh-auth/ldap_ca.pem', + LDAP_USER_FILTER='(&(objectClass=person)(uid={username}))', + LDAP_UNIQUE_ID_ATTRIBUTE='objectGUID', + )) + + assert result.returncode != 0 + assert 'LDAP_BACKUP_URL' in result.stdout + result.stderr + + @pytest.mark.parametrize( ('setting_name', 'setting_value'), ( @@ -518,6 +582,7 @@ def test_ldap_compose_overlay_supplies_complete_secret_infrastructure(): 'LDAP_AUTO_PROVISION', 'LDAP_PROVIDER_ID', 'LDAP_URL', + 'LDAP_BACKUP_URL', 'LDAP_BASE_DN', 'LDAP_BIND_DN', 'LDAP_USER_FILTER', diff --git a/tests/test_profile_launcher_ui.py b/tests/test_profile_launcher_ui.py index 60a29d9c..9ece87c8 100644 --- a/tests/test_profile_launcher_ui.py +++ b/tests/test_profile_launcher_ui.py @@ -33,9 +33,9 @@ def test_merged_profile_frontend_assets_have_distinct_cache_versions(): template = read('templates/index.html') expected_versions = { "filename='css/style.css'": '?v=24', - "filename='css/sftp-file-manager.css'": '?v=21', - "filename='js/i18n.js'": '?v=44', - "filename='js/command-workspace.js'": '?v=3', + "filename='css/sftp-file-manager.css'": '?v=22', + "filename='js/i18n.js'": '?v=45', + "filename='js/command-workspace.js'": '?v=4', "filename='js/command-palette-utils.js'": '?v=1', "filename='js/profile-launcher-utils.js'": '?v=5', "filename='js/connection-launcher.js'": '?v=1', @@ -48,7 +48,7 @@ def test_merged_profile_frontend_assets_have_distinct_cache_versions(): "filename='js/sftp-file-manager.js'": '?v=35', "filename='js/jump-host-manager.js'": '?v=4', "filename='js/command-library.js'": '?v=5', - "filename='js/command-set-manager.js'": '?v=6', + "filename='js/command-set-manager.js'": '?v=7', "filename='js/session-command-launcher.js'": '?v=5', "filename='js/connection-history.js'": '?v=2', "filename='js/app.js'": '?v=28', diff --git a/tests/test_security_features.py b/tests/test_security_features.py index ab4cb7c5..9a7fba2f 100644 --- a/tests/test_security_features.py +++ b/tests/test_security_features.py @@ -154,6 +154,7 @@ def test_provider_status_exposes_configuration_names_and_documentation_only( assert ldap['configuration_keys'] == ( 'LDAP_ENABLED', 'LDAP_URL', + 'LDAP_BACKUP_URL', 'LDAP_BASE_DN', 'LDAP_BIND_DN', 'LDAP_BIND_PASSWORD_FILE', diff --git a/tests/test_session_insights.py b/tests/test_session_insights.py index cefb6237..3209c068 100644 --- a/tests/test_session_insights.py +++ b/tests/test_session_insights.py @@ -1,6 +1,9 @@ -import pytest +import shutil +import subprocess from threading import Event, Lock, Thread +import pytest + from app import session_insights @@ -242,6 +245,30 @@ def test_remote_diagnostics_use_a_fixed_safe_environment_without_elevation(): assert 'command -v uname' in session_insights.LINUX_STATS_COMMAND +@pytest.mark.skipif(shutil.which('awk') is None, reason='awk is not installed') +def test_network_collector_reads_bytes_for_short_and_long_interface_names(): + proc_net_dev = """\ +Inter-| Receive | Transmit + face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed + lo: 99999999 9999 0 0 0 0 0 0 88888888 8888 0 0 0 0 0 0 + eth0: 10485760 1000 0 0 0 0 0 0 12345 100 0 0 0 0 0 0 +enp0s31f6: 20971520 2000 0 0 0 0 0 0 54321 200 0 0 0 0 0 0 +""" + + result = subprocess.run( + ['awk', session_insights.LINUX_NETWORK_AWK_PROGRAM], + input=proc_net_dev, + text=True, + capture_output=True, + check=True, + ) + + assert result.stdout.splitlines() == [ + 'network_received_bytes=31457280', + 'network_transmitted_bytes=66666', + ] + + class FakeChannel: def __init__(self, payload=VALID_PAYLOAD.encode(), *, status=0, recv_error=None): diff --git a/tests/test_webssh2_shell.py b/tests/test_webssh2_shell.py index 34f03bd4..8f4275d1 100644 --- a/tests/test_webssh2_shell.py +++ b/tests/test_webssh2_shell.py @@ -137,16 +137,16 @@ def test_every_user_facing_page_uses_current_shared_asset_versions(app, client): response = client.get(path) assert response.status_code == 200 assert b'css/style.css?v=24' in response.data - assert b'css/webssh-2.css?v=31' in response.data - assert b'js/i18n.js?v=44' in response.data + assert b'css/webssh-2.css?v=32' in response.data + assert b'js/i18n.js?v=45' in response.data client.post("/logout") for path in ("/login", "/register"): response = client.get(path) assert response.status_code == 200 assert b'css/style.css?v=24' in response.data - assert b'css/webssh-2.css?v=31' in response.data - assert b'js/i18n.js?v=44' in response.data + assert b'css/webssh-2.css?v=32' in response.data + assert b'js/i18n.js?v=45' in response.data def test_compact_workspace_controls_keep_accessible_names_and_close_command_input( From bef2b533f215280f537d9b7f5cfa86380c6ae40d Mon Sep 17 00:00:00 2001 From: bifrost0x Date: Wed, 2 Sep 2026 19:51:27 +0200 Subject: [PATCH 2/2] fix: normalize LDAP failover endpoints --- config.py | 22 +++++++++++++++++++--- tests/test_production_config.py | 16 ++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/config.py b/config.py index 7a241b77..b03b9659 100644 --- a/config.py +++ b/config.py @@ -731,11 +731,27 @@ def _validate_ldap_url(setting_name, value): or parsed_ldap_url.fragment ): raise RuntimeError(ldap_url_error) + return parsed_ldap_url - _validate_ldap_url('LDAP_URL', LDAP_URL) + primary_ldap_url = _validate_ldap_url('LDAP_URL', LDAP_URL) if LDAP_BACKUP_URL: - _validate_ldap_url('LDAP_BACKUP_URL', LDAP_BACKUP_URL) - if LDAP_BACKUP_URL == LDAP_URL: + backup_ldap_url = _validate_ldap_url( + 'LDAP_BACKUP_URL', + LDAP_BACKUP_URL, + ) + + def _ldap_endpoint_identity(parsed_url): + default_port = 636 if parsed_url.scheme == 'ldaps' else 389 + return ( + parsed_url.scheme, + parsed_url.hostname.casefold().rstrip('.'), + parsed_url.port or default_port, + ) + + if ( + _ldap_endpoint_identity(backup_ldap_url) + == _ldap_endpoint_identity(primary_ldap_url) + ): raise RuntimeError( 'SECURITY ERROR: LDAP_BACKUP_URL must identify a ' 'different server than LDAP_URL' diff --git a/tests/test_production_config.py b/tests/test_production_config.py index 0e48ca5e..d665bd2d 100644 --- a/tests/test_production_config.py +++ b/tests/test_production_config.py @@ -351,12 +351,20 @@ def test_enabled_ldap_rejects_an_unsafe_backup_endpoint(backup_url): assert 'LDAP_BACKUP_URL' in result.stdout + result.stderr -def test_enabled_ldap_rejects_a_duplicate_backup_endpoint(): - url = 'ldaps://dc01.ad.example.com:636' +@pytest.mark.parametrize( + 'backup_url', + ( + 'ldaps://dc01.ad.example.com:636', + 'ldaps://DC01.AD.EXAMPLE.COM:636', + 'ldaps://dc01.ad.example.com', + 'ldaps://dc01.ad.example.com.:636', + ), +) +def test_enabled_ldap_rejects_a_duplicate_backup_endpoint(backup_url): result = _load_config(_production_env( LDAP_ENABLED='true', - LDAP_URL=url, - LDAP_BACKUP_URL=url, + LDAP_URL='ldaps://dc01.ad.example.com:636', + LDAP_BACKUP_URL=backup_url, LDAP_BASE_DN='ou=people,dc=ad,dc=example,dc=com', LDAP_BIND_DN='cn=webssh,ou=services,dc=ad,dc=example,dc=com', LDAP_BIND_PASSWORD_FILE='/run/webssh-auth/ldap_bind_password',