From 14f83cd549e6b4df33efcd0bd5ec8380b80e50ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Gruszczy=C5=84ski?= Date: Wed, 15 Apr 2026 09:22:15 +0200 Subject: [PATCH] form fix --- backend/app/api/routes/routers.py | 10 +- backend/app/services/router_service.py | 53 +++++++---- backend/tests/test_routers.py | 57 +++++++++++ frontend/src/app/core/services/ui.service.ts | 47 +++++++++ .../routers/router-detail-page.component.ts | 95 ++++++++++++------- .../routers/routers-page.component.ts | 28 ++++-- frontend/src/assets/i18n/en.json | 2 + frontend/src/assets/i18n/es.json | 2 + frontend/src/assets/i18n/no.json | 2 + frontend/src/assets/i18n/pl.json | 2 + 10 files changed, 235 insertions(+), 63 deletions(-) diff --git a/backend/app/api/routes/routers.py b/backend/app/api/routes/routers.py index 5a30bbc..22d1938 100644 --- a/backend/app/api/routes/routers.py +++ b/backend/app/api/routes/routers.py @@ -28,6 +28,8 @@ def serialize_router(router: Router, global_settings) -> RouterResponse: effective_username = router_user uses_global_switchos_credentials = False has_effective_password = bool(router_password) + uses_global_ssh_key = False + has_effective_ssh_key = False if router.device_type == 'switchos': effective_username = router_user or default_swos_user @@ -35,11 +37,15 @@ def serialize_router(router: Router, global_settings) -> RouterResponse: (not router_user and default_swos_user) or (not router_password and default_swos_password) ) has_effective_password = bool(router_password or default_swos_password) + else: + uses_password_auth = bool(router_password) + uses_global_ssh_key = bool(has_global_key and not has_router_key and not uses_password_auth) + has_effective_ssh_key = bool(has_router_key or uses_global_ssh_key) payload = RouterResponse.model_validate(router, from_attributes=True).model_dump() payload['effective_username'] = effective_username - payload['uses_global_ssh_key'] = router.device_type == 'routeros' and has_global_key and not has_router_key - payload['has_effective_ssh_key'] = router.device_type == 'routeros' and (has_router_key or has_global_key) + payload['uses_global_ssh_key'] = uses_global_ssh_key + payload['has_effective_ssh_key'] = has_effective_ssh_key payload['uses_global_switchos_credentials'] = uses_global_switchos_credentials payload['has_effective_password'] = has_effective_password payload['supports_export'] = router.device_type == 'routeros' diff --git a/backend/app/services/router_service.py b/backend/app/services/router_service.py index 5bd17ee..925a957 100644 --- a/backend/app/services/router_service.py +++ b/backend/app/services/router_service.py @@ -15,6 +15,12 @@ from app.services.swos_beta_service import swos_beta_service class RouterService: + connect_timeout_seconds = 10 + auth_timeout_seconds = 10 + banner_timeout_seconds = 10 + command_timeout_seconds = 20 + sftp_timeout_seconds = 20 + def ping(self, router: Router): if getattr(router, 'disable_ping', False): return {'router_id': router.id, 'reachable': False, 'latency_ms': None, 'disabled': True} @@ -59,28 +65,41 @@ class RouterService: def _connect(self, router: Router, global_ssh_key: str | None = None): client = paramiko.SSHClient() client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) - key_source = router.ssh_key.strip() if router.ssh_key and router.ssh_key.strip() else (global_ssh_key or "") + + router_key = (router.ssh_key or '').strip() + router_password = (router.ssh_password or '').strip() + global_key = (global_ssh_key or '').strip() + + use_password_auth = bool(router_password and not router_key) + key_source = router_key or ('' if use_password_auth else global_key) + + connect_kwargs = { + 'hostname': router.host, + 'port': router.port, + 'username': router.ssh_user, + 'timeout': self.connect_timeout_seconds, + 'auth_timeout': self.auth_timeout_seconds, + 'banner_timeout': self.banner_timeout_seconds, + 'allow_agent': False, + 'look_for_keys': False, + } + if key_source: pkey = self._load_pkey(key_source) - client.connect(router.host, port=router.port, username=router.ssh_user, pkey=pkey, timeout=10) + client.connect(pkey=pkey, **connect_kwargs) else: - client.connect( - router.host, - port=router.port, - username=router.ssh_user, - password=router.ssh_password, - timeout=10, - allow_agent=False, - look_for_keys=False, - banner_timeout=10, - ) + client.connect(password=router_password or None, **connect_kwargs) + + transport = client.get_transport() + if transport is not None: + transport.set_keepalive(15) return client def export(self, router: Router, global_ssh_key: str | None = None) -> str: if router.device_type != 'routeros': raise ValueError('Export tekstowy jest dostępny tylko dla RouterOS.') client = self._connect(router, global_ssh_key) - _, stdout, _ = client.exec_command('/export') + _, stdout, _ = client.exec_command('/export', timeout=self.command_timeout_seconds) output = stdout.read().decode('utf-8', errors='ignore') client.close() return output @@ -92,9 +111,10 @@ class RouterService: return local_path client = self._connect(router, global_ssh_key) - _, stdout, _ = client.exec_command(f'/system backup save name={backup_name}') + _, stdout, _ = client.exec_command(f'/system backup save name={backup_name}', timeout=self.command_timeout_seconds) stdout.channel.recv_exit_status() sftp = client.open_sftp() + sftp.get_channel().settimeout(self.sftp_timeout_seconds) remote_file = f'{backup_name}.backup' sftp.get(remote_file, local_path) try: @@ -110,6 +130,7 @@ class RouterService: raise ValueError('Przywracanie plików jest dostępne tylko dla RouterOS.') client = self._connect(router, global_ssh_key) sftp = client.open_sftp() + sftp.get_channel().settimeout(self.sftp_timeout_seconds) target_name = Path(local_backup_path).name sftp.put(local_backup_path, target_name) sftp.close() @@ -119,9 +140,9 @@ class RouterService: tested_at = datetime.utcnow() try: client = self._connect(router, global_ssh_key) - _, stdout, _ = client.exec_command('/system resource print without-paging') + _, stdout, _ = client.exec_command('/system resource print without-paging', timeout=self.command_timeout_seconds) resource_output = stdout.read().decode('utf-8', errors='ignore') - _, stdout, _ = client.exec_command('/system identity print') + _, stdout, _ = client.exec_command('/system identity print', timeout=self.command_timeout_seconds) identity_output = stdout.read().decode('utf-8', errors='ignore') client.close() model = 'Unknown' diff --git a/backend/tests/test_routers.py b/backend/tests/test_routers.py index 567c8e6..6210253 100644 --- a/backend/tests/test_routers.py +++ b/backend/tests/test_routers.py @@ -60,3 +60,60 @@ def test_router_list_marks_global_ssh_key_usage(monkeypatch, tmp_path): payload = list_response.json() assert payload[0]["uses_global_ssh_key"] is True assert payload[0]["has_effective_ssh_key"] is True + + +def test_router_password_auth_overrides_global_ssh_key(monkeypatch, tmp_path): + monkeypatch.setenv("DATABASE_URL", f"sqlite:///{tmp_path / 'routers-password.db'}") + monkeypatch.setenv("DATA_DIR", str(tmp_path / 'data-password')) + monkeypatch.setenv("SECRET_KEY", "test-secret") + monkeypatch.setenv("DEFAULT_ADMIN_USERNAME", "admin") + monkeypatch.setenv("DEFAULT_ADMIN_PASSWORD", "admin") + + with TestClient(app) as client: + login_response = client.post("/api/auth/login", data={"username": "admin", "password": "admin"}) + token = login_response.json()["access_token"] + headers = {"Authorization": f"Bearer {token}"} + + settings_response = client.put( + "/api/settings", + json={ + "backup_retention_days": 7, + "log_retention_days": 7, + "export_cron": "", + "binary_cron": "", + "retention_cron": "", + "enable_auto_export": False, + "connection_test_interval_minutes": 0, + "global_ssh_key": "-----BEGIN OPENSSH PRIVATE KEY-----\nabc\n-----END OPENSSH PRIVATE KEY-----", + "pushover_token": None, + "pushover_userkey": None, + "notify_failures_only": True, + "smtp_host": None, + "smtp_port": 587, + "smtp_login": None, + "smtp_password": None, + "smtp_notifications_enabled": False, + "recipient_email": None, + "clear_global_ssh_key": False + }, + headers=headers, + ) + assert settings_response.status_code == 200 + + create_response = client.post( + "/api/routers", + json={ + "name": "edge02", + "host": "10.0.0.2", + "port": 22, + "ssh_user": "admin", + "ssh_password": "secret-pass", + "ssh_key": None + }, + headers=headers, + ) + assert create_response.status_code == 200 + payload = create_response.json() + assert payload["uses_global_ssh_key"] is False + assert payload["has_effective_ssh_key"] is False + assert payload["has_effective_password"] is True diff --git a/frontend/src/app/core/services/ui.service.ts b/frontend/src/app/core/services/ui.service.ts index 6582692..72c72d2 100644 --- a/frontend/src/app/core/services/ui.service.ts +++ b/frontend/src/app/core/services/ui.service.ts @@ -1,3 +1,4 @@ +import { HttpErrorResponse } from '@angular/common/http'; import { Injectable, inject } from '@angular/core'; import { TranslateService } from '@ngx-translate/core'; import { ConfirmationService, MessageService } from 'primeng/api'; @@ -45,6 +46,17 @@ export class UiService { this.messageService.clear(); } + + apiError(error: unknown, fallbackDetailKey: string) { + const detail = this.extractErrorMessage(error) || this.t(fallbackDetailKey); + this.messageService.add({ + severity: 'error', + summary: this.t('toast.error'), + detail + }); + } + + confirm(options: ConfirmOptions): Promise { return new Promise((resolve) => { let resolved = false; @@ -75,7 +87,42 @@ export class UiService { return this.t(key, params); } + + private extractErrorMessage(error: unknown): string | null { + if (!error) { + return null; + } + if (error instanceof HttpErrorResponse) { + if ((error as { name?: string }).name === 'TimeoutError') { + return this.t('toast.requestTimeout'); + } + const payload = error.error; + if (typeof payload === 'string' && payload.trim()) { + return payload.trim(); + } + if (payload && typeof payload === 'object') { + const detail = (payload as { detail?: unknown }).detail; + if (typeof detail === 'string' && detail.trim()) { + return detail.trim(); + } + } + if (typeof error.message === 'string' && error.message.trim()) { + return error.message.trim(); + } + return null; + } + const maybeTimeout = error as { name?: string; message?: string }; + if (maybeTimeout.name === 'TimeoutError') { + return this.t('toast.requestTimeout'); + } + if (typeof maybeTimeout.message === 'string' && maybeTimeout.message.trim()) { + return maybeTimeout.message.trim(); + } + return null; + } + private t(key: string, params?: Record): string { return this.translate.instant(key, params); } } + diff --git a/frontend/src/app/features/routers/router-detail-page.component.ts b/frontend/src/app/features/routers/router-detail-page.component.ts index e7ba275..277486b 100644 --- a/frontend/src/app/features/routers/router-detail-page.component.ts +++ b/frontend/src/app/features/routers/router-detail-page.component.ts @@ -1,4 +1,5 @@ import { CommonModule } from '@angular/common'; +import { finalize, timeout } from 'rxjs'; import { HttpResponse } from '@angular/common/http'; import { Component, OnInit, inject } from '@angular/core'; import { FormBuilder, ReactiveFormsModule, Validators } from '@angular/forms'; @@ -242,17 +243,25 @@ export class RouterDetailPageComponent implements OnInit { if (payload.device_type === 'switchos') { payload.ssh_key = ''; } - this.api.http.put(`${this.api.baseUrl}/routers/${this.routerId}`, payload).subscribe({ - next: (routerItem) => { - this.routerItem = routerItem; - this.connection = this.mapStoredConnection(routerItem); - this.editVisible = false; - this.ui.success('toast.routerUpdated'); - }, - complete: () => { - this.saving = false; - } - }); + this.api.http + .put(`${this.api.baseUrl}/routers/${this.routerId}`, payload) + .pipe( + timeout(15000), + finalize(() => { + this.saving = false; + }) + ) + .subscribe({ + next: (routerItem) => { + this.routerItem = routerItem; + this.connection = this.mapStoredConnection(routerItem); + this.editVisible = false; + this.ui.success('toast.routerUpdated'); + }, + error: (error) => { + this.ui.apiError(error, 'toast.routerSaveFailed'); + } + }); } saveSettings() { @@ -268,17 +277,25 @@ export class RouterDetailPageComponent implements OnInit { payload.disable_export_backups = true; payload.disable_binary_backups = true; } - this.api.http.put(`${this.api.baseUrl}/routers/${this.routerId}`, payload).subscribe({ - next: (routerItem) => { - this.routerItem = routerItem; - this.connection = this.mapStoredConnection(routerItem); - this.patchSettingsForm(routerItem); - this.ui.success('toast.routerUpdated'); - }, - complete: () => { - this.savingSettings = false; - } - }); + this.api.http + .put(`${this.api.baseUrl}/routers/${this.routerId}`, payload) + .pipe( + timeout(15000), + finalize(() => { + this.savingSettings = false; + }) + ) + .subscribe({ + next: (routerItem) => { + this.routerItem = routerItem; + this.connection = this.mapStoredConnection(routerItem); + this.patchSettingsForm(routerItem); + this.ui.success('toast.routerUpdated'); + }, + error: (error) => { + this.ui.apiError(error, 'toast.routerSaveFailed'); + } + }); } private patchSettingsForm(item: DeviceItem) { @@ -327,20 +344,28 @@ export class RouterDetailPageComponent implements OnInit { return; } this.testing = true; - this.api.http.get(`${this.api.baseUrl}/routers/${this.routerId}/test-connection`).subscribe({ - next: (result) => { - this.connection = result; - this.syncStoredConnection(result); - if (result.success) { - this.ui.success('toast.connectionSuccessful'); - } else { - this.ui.error('toast.connectionFailed'); + this.api.http + .get(`${this.api.baseUrl}/routers/${this.routerId}/test-connection`) + .pipe( + timeout(15000), + finalize(() => { + this.testing = false; + }) + ) + .subscribe({ + next: (result) => { + this.connection = result; + this.syncStoredConnection(result); + if (result.success) { + this.ui.success('toast.connectionSuccessful'); + } else { + this.ui.apiError({ message: result.error }, 'toast.connectionFailed'); + } + }, + error: (error) => { + this.ui.apiError(error, 'toast.connectionFailed'); } - }, - complete: () => { - this.testing = false; - } - }); + }); } compareToLatest(id: number) { diff --git a/frontend/src/app/features/routers/routers-page.component.ts b/frontend/src/app/features/routers/routers-page.component.ts index d929c13..fce71b5 100644 --- a/frontend/src/app/features/routers/routers-page.component.ts +++ b/frontend/src/app/features/routers/routers-page.component.ts @@ -1,4 +1,5 @@ import { CommonModule } from '@angular/common'; +import { finalize, timeout } from 'rxjs'; import { Component, OnInit, inject } from '@angular/core'; import { FormBuilder, ReactiveFormsModule, Validators } from '@angular/forms'; import { Router } from '@angular/router'; @@ -167,16 +168,23 @@ export class RoutersPageComponent implements OnInit { ? this.api.http.put(`${this.api.baseUrl}/routers/${this.editingId}`, payload) : this.api.http.post(`${this.api.baseUrl}/routers`, payload); - request$.subscribe({ - next: () => { - this.ui.success(this.editingId ? 'toast.routerUpdated' : 'toast.routerCreated'); - this.visible = false; - this.load(); - }, - complete: () => { - this.saving = false; - } - }); + request$ + .pipe( + timeout(15000), + finalize(() => { + this.saving = false; + }) + ) + .subscribe({ + next: () => { + this.ui.success(this.editingId ? 'toast.routerUpdated' : 'toast.routerCreated'); + this.visible = false; + this.load(); + }, + error: (error) => { + this.ui.apiError(error, 'toast.routerSaveFailed'); + } + }); } async remove(id: number) { diff --git a/frontend/src/assets/i18n/en.json b/frontend/src/assets/i18n/en.json index 097d09c..3e5e616 100644 --- a/frontend/src/assets/i18n/en.json +++ b/frontend/src/assets/i18n/en.json @@ -458,6 +458,8 @@ "archivePrepared": "Archive prepared.", "exportedRouters": "Export completed for {{count}} devices.", "binaryCompletedRouters": "Binary backup completed for {{count}} devices.", + "routerSaveFailed": "Could not save device.", + "requestTimeout": "Request timed out. Check connection and try again.", "routerCreated": "Router created.", "routerUpdated": "Router updated.", "routerDeleted": "Router deleted.", diff --git a/frontend/src/assets/i18n/es.json b/frontend/src/assets/i18n/es.json index 1cade1d..2b3004b 100644 --- a/frontend/src/assets/i18n/es.json +++ b/frontend/src/assets/i18n/es.json @@ -440,6 +440,8 @@ "archivePrepared": "Archivo preparado.", "exportedRouters": "Exportación completada para {{count}} routers.", "binaryCompletedRouters": "Copia binaria completada para {{count}} routers.", + "routerSaveFailed": "No se pudo guardar el dispositivo.", + "requestTimeout": "Se agotó el tiempo de espera. Comprueba la conexión e inténtalo de nuevo.", "routerCreated": "Router creado.", "routerUpdated": "Router actualizado.", "routerDeleted": "Router eliminado.", diff --git a/frontend/src/assets/i18n/no.json b/frontend/src/assets/i18n/no.json index 60a852e..a5682cc 100644 --- a/frontend/src/assets/i18n/no.json +++ b/frontend/src/assets/i18n/no.json @@ -440,6 +440,8 @@ "archivePrepared": "Arkiv klargjort.", "exportedRouters": "Export fullført for {{count}} rutere.", "binaryCompletedRouters": "Binær backup fullført for {{count}} rutere.", + "routerSaveFailed": "Kunne ikke lagre enheten.", + "requestTimeout": "Tidsavbrudd for forespørselen. Sjekk tilkoblingen og prøv igjen.", "routerCreated": "Ruter opprettet.", "routerUpdated": "Ruter oppdatert.", "routerDeleted": "Ruter slettet.", diff --git a/frontend/src/assets/i18n/pl.json b/frontend/src/assets/i18n/pl.json index 7f1d32b..7156eb1 100644 --- a/frontend/src/assets/i18n/pl.json +++ b/frontend/src/assets/i18n/pl.json @@ -458,6 +458,8 @@ "archivePrepared": "Archiwum zostało przygotowane.", "exportedRouters": "Wykonano export dla {{count}} urządzeń.", "binaryCompletedRouters": "Wykonano backup binarny dla {{count}} urządzeń.", + "routerSaveFailed": "Nie udało się zapisać urządzenia.", + "requestTimeout": "Przekroczono czas oczekiwania. Sprawdź połączenie i spróbuj ponownie.", "routerCreated": "Urządzenie zostało dodane.", "routerUpdated": "Urządzenie zostało zaktualizowane.", "routerDeleted": "Urządzenie zostało usunięte.",