From d23c39315b6f8989ee61594cff83a830b785f05e Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sat, 10 Oct 2026 16:54:19 +0300 Subject: [PATCH 1/2] fix(storage): secrets can be removed on edit, SSH secrets go with the tunnel, a failed save restores them all (#1311) - An edit form has Remove the saved password; a blank field still keeps it. Test Connection honours it. - Saving a connection without a tunnel removes its SSH password, key and passphrase from the keyring instead of leaving them there. - A failed secret write restores the SSH secrets too (they are written key by key), not only the password and connection string. --- lib/core/storage/local_db.dart | 21 +++ .../connections/connection_edit_secrets.dart | 13 +- .../remove_saved_password_option.dart | 38 +++++ .../mongodb/mongodb_connection_form.dart | 11 ++ lib/features/mysql/mysql_connection_form.dart | 11 ++ .../postgresql_connection_form.dart | 11 ++ lib/features/redis/redis_connection_form.dart | 15 +- test/core/storage/local_db_secrets_test.dart | 156 ++++++++++++++++++ 8 files changed, 272 insertions(+), 4 deletions(-) create mode 100644 lib/features/connections/remove_saved_password_option.dart diff --git a/lib/core/storage/local_db.dart b/lib/core/storage/local_db.dart index bbfa0215..02d23a14 100644 --- a/lib/core/storage/local_db.dart +++ b/lib/core/storage/local_db.dart @@ -803,6 +803,8 @@ class LocalDb { final previousRow = ConnectionRow.fromMap(previousMaps.first); final previousSecrets = await ConnectionSecretsStore.readForConnection(row.id!); + final previousSsh = + await ConnectionSecretsStore.readSshSecretsForConnection(row.id!); await db.transaction((txn) async { final count = await txn.update( @@ -830,6 +832,10 @@ class LocalDb { passphrase: row.sshSecrets!.passphrase, jumpPassword: row.sshSecrets!.jumpPassword, ); + } else if (row.sshTunnelConfig?.enabled != true) { + // The tunnel was turned off: its password and key do not stay in the + // keyring (#1311). + await ConnectionSecretsStore.writeSshSecretsForConnection(row.id!); } } catch (e) { await db.update( @@ -844,6 +850,14 @@ class LocalDb { password: previousSecrets.password, connectionString: previousSecrets.connectionString, ); + // The SSH secrets may be half written too (four keys, one by one). + await ConnectionSecretsStore.writeSshSecretsForConnection( + row.id!, + password: previousSsh.password, + privateKey: previousSsh.privateKey, + passphrase: previousSsh.passphrase, + jumpPassword: previousSsh.jumpPassword, + ); } catch (_) { // Best-effort restore of previous secrets; surface the original error. } @@ -1040,8 +1054,13 @@ class ConnectionRow { this.sortOrder = 0, required this.createdAt, this.sshSecrets, + this.removeSavedPassword = false, }); + /// Set by an edit form (never stored): the saved password is to be removed, + /// not kept because the password field was left blank (#1311). + final bool removeSavedPassword; + final int? id; final String type; final String name; @@ -1194,6 +1213,7 @@ class ConnectionRow { int? sortOrder, String? createdAt, SshTunnelSecrets? sshSecrets, + bool? removeSavedPassword, bool clearPassword = false, bool clearConnectionString = false, bool clearSshSecrets = false, @@ -1221,6 +1241,7 @@ class ConnectionRow { sortOrder: sortOrder ?? this.sortOrder, createdAt: createdAt ?? this.createdAt, sshSecrets: clearSshSecrets ? null : (sshSecrets ?? this.sshSecrets), + removeSavedPassword: removeSavedPassword ?? this.removeSavedPassword, ); } diff --git a/lib/features/connections/connection_edit_secrets.dart b/lib/features/connections/connection_edit_secrets.dart index 36752419..acc6c4ec 100644 --- a/lib/features/connections/connection_edit_secrets.dart +++ b/lib/features/connections/connection_edit_secrets.dart @@ -48,6 +48,7 @@ Future< required String? password, required String? connectionString, required SshTunnelSecrets? sshSecrets, + bool useSavedPassword = true, }) async { final id = connectionId; if (id == null || id <= 0) { @@ -58,8 +59,10 @@ Future< ); } final prev = await ConnectionSecretsStore.readForConnection(id); - final effectivePassword = - (password == null || password.isEmpty) ? prev.password : password; + // "Remove the saved password" is ticked: test without it (#1311). + final effectivePassword = (password == null || password.isEmpty) + ? (useSavedPassword ? prev.password : null) + : password; var uri = connectionString; if (uri != null && uri.trim().isNotEmpty) { uri = injectUriPasswordIfMissing(uri, effectivePassword); @@ -91,7 +94,11 @@ Future mergeSecretsForConnectionUpdate( final passwordEmpty = edited.password == null || edited.password!.trim().isEmpty; - final password = passwordEmpty ? prev.password : edited.password; + // A blank field keeps the saved password, unless the user asked to remove + // it (#1311). A password typed together with the request wins: it replaces. + final String? password = edited.removeSavedPassword && passwordEmpty + ? null + : (passwordEmpty ? prev.password : edited.password); var connectionString = edited.connectionString; if (connectionString == null || connectionString.trim().isEmpty) { diff --git a/lib/features/connections/remove_saved_password_option.dart b/lib/features/connections/remove_saved_password_option.dart new file mode 100644 index 00000000..c72e4491 --- /dev/null +++ b/lib/features/connections/remove_saved_password_option.dart @@ -0,0 +1,38 @@ +import 'package:flutter/material.dart' as material; +import 'package:querya_desktop/shared/widgets/widgets.dart'; + +/// "Remove the saved password" in an edit form (#1311). +/// +/// A blank password field means *keep the saved one*, and the form never shows +/// it, so without this a saved password could not be taken away (a database +/// that moved to trust authentication, a password that must not stay on disk). +class RemoveSavedPasswordOption extends material.StatelessWidget { + const RemoveSavedPasswordOption({ + super.key, + required this.value, + required this.onChanged, + }); + + final bool value; + final material.ValueChanged onChanged; + + @override + material.Widget build(material.BuildContext context) { + return material.Padding( + padding: const material.EdgeInsets.only(top: 6), + child: material.Row( + children: [ + material.Checkbox( + key: const material.ValueKey('remove_saved_password'), + value: value, + onChanged: (v) => onChanged(v ?? false), + ), + const Gap(8), + const material.Flexible( + child: Text('Remove the saved password').small(), + ), + ], + ), + ); + } +} diff --git a/lib/features/mongodb/mongodb_connection_form.dart b/lib/features/mongodb/mongodb_connection_form.dart index 53a4abd0..a52d2eda 100644 --- a/lib/features/mongodb/mongodb_connection_form.dart +++ b/lib/features/mongodb/mongodb_connection_form.dart @@ -8,6 +8,7 @@ import 'package:querya_desktop/core/database/mongodb_connection.dart'; import 'package:querya_desktop/core/layout/window_layout.dart'; import 'package:querya_desktop/core/security/ssh_tunnel_config.dart'; import 'package:querya_desktop/core/storage/local_db.dart'; +import 'package:querya_desktop/features/connections/remove_saved_password_option.dart'; import 'package:querya_desktop/features/connections/connection_creation_flow.dart'; import 'package:querya_desktop/features/connections/ssh_tunnel_section.dart'; import 'package:querya_desktop/features/connections/ssl_certificate_support.dart'; @@ -98,6 +99,7 @@ class _MongoConnectionFormContentState bool _useConnectionString = false; bool _useSSL = false; + bool _removeSavedPassword = false; bool _showPassword = false; bool _isTesting = false; String? _testResult; @@ -293,6 +295,7 @@ class _MongoConnectionFormContentState password: data.password, connectionString: data.connectionString, sshSecrets: _sshConfig.enabled ? _sshSecrets : null, + useSavedPassword: !_removeSavedPassword, ); final connection = MongoConnection( id: 0, @@ -354,6 +357,7 @@ class _MongoConnectionFormContentState ); row = row.withSshTunnelConfig(_sshConfig.enabled ? _sshConfig : null); row = row.withEnvironment(_environment); + row = row.copyWith(removeSavedPassword: _removeSavedPassword); material.Navigator.of(context).pop(row); } @@ -513,10 +517,17 @@ class _MongoConnectionFormContentState placeholder: const Text('Username'), ), const Gap(12), + if (_isEditing) + RemoveSavedPasswordOption( + value: _removeSavedPassword, + onChanged: (v) => + setState(() => _removeSavedPassword = v), + ), material.Stack( children: [ TextField( controller: _passwordController, + enabled: !_removeSavedPassword, placeholder: Text( _isEditing ? 'Leave blank to keep existing' diff --git a/lib/features/mysql/mysql_connection_form.dart b/lib/features/mysql/mysql_connection_form.dart index e24ea94e..5dc26c67 100644 --- a/lib/features/mysql/mysql_connection_form.dart +++ b/lib/features/mysql/mysql_connection_form.dart @@ -8,6 +8,7 @@ import 'package:querya_desktop/core/database/mysql_connection.dart'; import 'package:querya_desktop/core/layout/window_layout.dart'; import 'package:querya_desktop/core/security/ssh_tunnel_config.dart'; import 'package:querya_desktop/core/storage/local_db.dart'; +import 'package:querya_desktop/features/connections/remove_saved_password_option.dart'; import 'package:querya_desktop/features/connections/connection_creation_flow.dart'; import 'package:querya_desktop/features/connections/ssh_tunnel_section.dart'; import 'package:querya_desktop/features/connections/ssl_certificate_support.dart'; @@ -63,6 +64,7 @@ class _MysqlConnectionFormContentState ConnectionEnvironment? _environment; final SshTunnelSecrets _sshSecrets = SshTunnelSecrets(); + bool _removeSavedPassword = false; bool _useSSL = true; bool _showPassword = false; bool _isTesting = false; @@ -220,6 +222,7 @@ class _MysqlConnectionFormContentState _passwordController.text.isEmpty ? null : _passwordController.text, connectionString: uri.isEmpty ? null : uri, sshSecrets: _sshConfig.enabled ? _sshSecrets : null, + useSavedPassword: !_removeSavedPassword, ); final conn = MysqlConnection( id: 0, @@ -287,6 +290,7 @@ class _MysqlConnectionFormContentState ); row = row.withSshTunnelConfig(_sshConfig.enabled ? _sshConfig : null); row = row.withEnvironment(_environment); + row = row.copyWith(removeSavedPassword: _removeSavedPassword); material.Navigator.of(context).pop(row); } @@ -463,11 +467,18 @@ class _MysqlConnectionFormContentState ), const Gap(16), const Text('Password').small().semiBold(), + if (_isEditing) + RemoveSavedPasswordOption( + value: _removeSavedPassword, + onChanged: (v) => + setState(() => _removeSavedPassword = v), + ), const Gap(8), material.Stack( children: [ TextField( controller: _passwordController, + enabled: !_removeSavedPassword, placeholder: Text( _isEditing ? 'Leave blank to keep existing' diff --git a/lib/features/postgresql/postgresql_connection_form.dart b/lib/features/postgresql/postgresql_connection_form.dart index 8350a59c..5ca12353 100644 --- a/lib/features/postgresql/postgresql_connection_form.dart +++ b/lib/features/postgresql/postgresql_connection_form.dart @@ -9,6 +9,7 @@ import 'package:querya_desktop/core/database/postgres_connection.dart'; import 'package:querya_desktop/core/layout/window_layout.dart'; import 'package:querya_desktop/core/security/ssh_tunnel_config.dart'; import 'package:querya_desktop/core/storage/local_db.dart'; +import 'package:querya_desktop/features/connections/remove_saved_password_option.dart'; import 'package:querya_desktop/features/connections/connection_creation_flow.dart'; import 'package:querya_desktop/features/connections/ssh_tunnel_section.dart'; import 'package:querya_desktop/shared/widgets/form_validity_notifier.dart'; @@ -63,6 +64,7 @@ class _PostgresConnectionFormContentState final SshTunnelSecrets _sshSecrets = SshTunnelSecrets(); bool _useSSL = false; + bool _removeSavedPassword = false; bool _showPassword = false; bool _isTesting = false; String? _testResult; @@ -281,6 +283,7 @@ class _PostgresConnectionFormContentState _passwordController.text.isEmpty ? null : _passwordController.text, connectionString: hasUri ? uri : null, sshSecrets: _sshConfig.enabled ? _sshSecrets : null, + useSavedPassword: !_removeSavedPassword, ); final conn = PostgresConnection( id: 0, @@ -376,6 +379,7 @@ class _PostgresConnectionFormContentState ); row = row.withSshTunnelConfig(_sshConfig.enabled ? _sshConfig : null); row = row.withEnvironment(_environment); + row = row.copyWith(removeSavedPassword: _removeSavedPassword); material.Navigator.of(context).pop(row); } @@ -603,11 +607,18 @@ class _PostgresConnectionFormContentState material.CrossAxisAlignment.stretch, children: [ const Text('Password').small().semiBold(), + if (_isEditing) + RemoveSavedPasswordOption( + value: _removeSavedPassword, + onChanged: (v) => setState( + () => _removeSavedPassword = v), + ), const Gap(8), material.Stack( children: [ TextField( controller: _passwordController, + enabled: !_removeSavedPassword, placeholder: Text( _isEditing ? 'Leave blank to keep existing' diff --git a/lib/features/redis/redis_connection_form.dart b/lib/features/redis/redis_connection_form.dart index c34ac378..e77af6b8 100644 --- a/lib/features/redis/redis_connection_form.dart +++ b/lib/features/redis/redis_connection_form.dart @@ -8,6 +8,7 @@ import 'package:querya_desktop/core/database/redis_connection.dart'; import 'package:querya_desktop/core/layout/window_layout.dart'; import 'package:querya_desktop/core/security/ssh_tunnel_config.dart'; import 'package:querya_desktop/core/storage/local_db.dart'; +import 'package:querya_desktop/features/connections/remove_saved_password_option.dart'; import 'package:querya_desktop/features/connections/connection_creation_flow.dart'; import 'package:querya_desktop/features/connections/ssh_tunnel_section.dart'; import 'package:querya_desktop/features/connections/ssl_certificate_support.dart'; @@ -61,6 +62,7 @@ class _RedisConnectionFormContentState ConnectionEnvironment? _environment; final SshTunnelSecrets _sshSecrets = SshTunnelSecrets(); + bool _removeSavedPassword = false; bool _useSSL = false; bool _showPassword = false; bool _isTesting = false; @@ -204,6 +206,7 @@ class _RedisConnectionFormContentState password: draft.password, connectionString: draft.connectionString, sshSecrets: draft.sshSecrets, + useSavedPassword: !_removeSavedPassword, ); final conn = RedisConnection.fromConnectionRow(draft.copyWith( password: secrets.password, @@ -259,7 +262,10 @@ class _RedisConnectionFormContentState void _save() { if (!_formValidNotifier.value) return; - material.Navigator.of(context).pop(_draftRow(id: widget.initial?.id)); + material.Navigator.of(context).pop( + _draftRow(id: widget.initial?.id) + .copyWith(removeSavedPassword: _removeSavedPassword), + ); } @override @@ -405,11 +411,18 @@ class _RedisConnectionFormContentState ), const Gap(16), const Text('Password (optional)').small().semiBold(), + if (_isEditing) + RemoveSavedPasswordOption( + value: _removeSavedPassword, + onChanged: (v) => + setState(() => _removeSavedPassword = v), + ), const Gap(8), material.Stack( children: [ TextField( controller: _passwordController, + enabled: !_removeSavedPassword, placeholder: Text( _isEditing ? 'Leave blank to keep existing' diff --git a/test/core/storage/local_db_secrets_test.dart b/test/core/storage/local_db_secrets_test.dart index a520deba..053cba92 100644 --- a/test/core/storage/local_db_secrets_test.dart +++ b/test/core/storage/local_db_secrets_test.dart @@ -451,5 +451,161 @@ void main() { expect(loaded.name, 'Before'); expect(loaded.password, 'keep-me'); }); + + test('Remove the saved password removes it; a typed one replaces it', + () async { + const row = ConnectionRow( + type: 'postgresql', + name: 'PG', + host: 'localhost', + port: 5432, + username: 'admin', + password: 'old-secret', + createdAt: '2026-01-01T00:00:00Z', + ); + final id = await LocalDb.instance.addConnection(row); + ConnectionRow edit({String? password, bool remove = false}) => + ConnectionRow( + id: id, + type: 'postgresql', + name: 'PG', + host: 'localhost', + port: 5432, + username: 'admin', + password: password, + createdAt: '2026-01-01T00:00:00Z', + removeSavedPassword: remove, + ); + + // Blank keeps it (as before) ... + var merged = await mergeSecretsForConnectionUpdate(edit()); + expect(merged.password, 'old-secret'); + + // ... unless removal was asked for. + merged = await mergeSecretsForConnectionUpdate(edit(remove: true)); + expect(merged.password, isNull); + await LocalDb.instance.updateConnection(merged); + expect((await ConnectionSecretsStore.readForConnection(id)).password, + isNull); + + // A password typed with the request replaces: nothing is lost by it. + merged = await mergeSecretsForConnectionUpdate( + edit(password: 'new-secret', remove: true)); + expect(merged.password, 'new-secret'); + }); + + test('turning the SSH tunnel off removes its secrets from the store', + () async { + final withTunnel = ConnectionRow( + type: 'postgresql', + name: 'PG via bastion', + host: 'db.internal', + port: 5432, + createdAt: '2026-01-01T00:00:00Z', + sshSecrets: SshTunnelSecrets( + password: 'ssh-pass', + privateKey: 'ssh-key', + passphrase: 'phrase', + jumpPassword: 'jump', + ), + ).withSshTunnelConfig(const SshTunnelConfig( + enabled: true, + host: 'bastion.example', + port: 22, + username: 'deploy', + )); + final id = await LocalDb.instance.addConnection(withTunnel); + expect( + (await ConnectionSecretsStore.readSshSecretsForConnection(id)) + .privateKey, + 'ssh-key'); + + // The form saves without a tunnel and without SSH secrets. + final off = ConnectionRow( + id: id, + type: 'postgresql', + name: 'PG via bastion', + host: 'db.internal', + port: 5432, + createdAt: '2026-01-01T00:00:00Z', + ).withSshTunnelConfig(null); + await LocalDb.instance.updateConnection( + await mergeSecretsForConnectionUpdate(off)); + + final left = await ConnectionSecretsStore.readSshSecretsForConnection(id); + expect(left.password, isNull); + expect(left.privateKey, isNull); + expect(left.passphrase, isNull); + expect(left.jumpPassword, isNull); + }); + + test('a failed update restores the SSH secrets as well', () async { + const tunnel = SshTunnelConfig( + enabled: true, + host: 'bastion.example', + port: 22, + username: 'deploy', + ); + final id = await LocalDb.instance.addConnection(ConnectionRow( + type: 'postgresql', + name: 'PG', + host: 'db.internal', + port: 5432, + password: 'db-pass', + createdAt: '2026-01-01T00:00:00Z', + sshSecrets: + SshTunnelSecrets(password: 'ssh-old', privateKey: 'key-old'), + ).withSshTunnelConfig(tunnel)); + + // Writes of the update: password, connection string, then the four SSH + // keys: the fourth write (the private key) fails. + final real = ConnectionSecretsStore.backend; + ConnectionSecretsStore.backend = _FailsOnWrite(real, failOn: 4); + addTearDown(() => ConnectionSecretsStore.backend = real); + + await expectLater( + LocalDb.instance.updateConnection(ConnectionRow( + id: id, + type: 'postgresql', + name: 'PG', + host: 'db.internal', + port: 5432, + password: 'db-pass', + createdAt: '2026-01-01T00:00:00Z', + sshSecrets: + SshTunnelSecrets(password: 'ssh-new', privateKey: 'key-new'), + ).withSshTunnelConfig(tunnel)), + throwsA(isA()), + ); + ConnectionSecretsStore.backend = real; + + final ssh = await ConnectionSecretsStore.readSshSecretsForConnection(id); + expect(ssh.password, 'ssh-old'); + expect(ssh.privateKey, 'key-old'); + expect((await ConnectionSecretsStore.readForConnection(id)).password, + 'db-pass'); + }); }); } + +/// A backend whose [failOn]th write (1-based, counted from the first write +/// through it) throws; every other call goes to [inner]. +class _FailsOnWrite implements SecretsStorageBackend { + _FailsOnWrite(this.inner, {required this.failOn}); + + final SecretsStorageBackend inner; + final int failOn; + int _writes = 0; + + @override + Future read(String key) => inner.read(key); + + @override + Future write(String key, String? value) async { + if (++_writes == failOn) throw StateError('keyring write failed'); + await inner.write(key, value); + } + + @override + Future delete(String key) => inner.delete(key); +} From acc22330926e652e5154017999d8027175ea18ba Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sat, 10 Oct 2026 16:59:24 +0300 Subject: [PATCH 2/2] fix(connections): the remove-password option is not a const widget (#1311) --- lib/features/connections/remove_saved_password_option.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/features/connections/remove_saved_password_option.dart b/lib/features/connections/remove_saved_password_option.dart index c72e4491..96ad3d77 100644 --- a/lib/features/connections/remove_saved_password_option.dart +++ b/lib/features/connections/remove_saved_password_option.dart @@ -28,8 +28,8 @@ class RemoveSavedPasswordOption extends material.StatelessWidget { onChanged: (v) => onChanged(v ?? false), ), const Gap(8), - const material.Flexible( - child: Text('Remove the saved password').small(), + material.Flexible( + child: const Text('Remove the saved password').small(), ), ], ),