fix(connections): verify before saving, keep stored password, + tests - #91
Merged
Conversation
Two related fixes for the connect flow: - Connect now VERIFIES the connection first (via test_connection with the form's own credentials) and only saves + opens a session once it actually connects. Previously it saved first, so a failed connect left a broken saved connection behind. - save_connection no longer overwrites a stored secret with an empty value. Editing a connection re-opens the form with the password blanked (secrets are never read back into the UI), so an empty field means 'keep the stored one'. Writing the empty string wiped the real keychain password and broke the next connect — a silent data-loss bug. Now we only (over)write when a non-empty value is supplied.
Add coverage for the connection bugs hit this session: - Extract parsePostgresUri/buildPostgresUri to their own module and unit test them (the real managed-Postgres URL, hyphen/underscore passwords, percent-decoding, ssl modes, parse↔build round-trip). - Rust integration tests (real Postgres container): connect succeeds with the correct password, FAILS with a wrong one, 'prefer' falls back to plaintext on a no-TLS server, and a parameterised query runs end-to-end (guards the unnamed-statement/pooler fix).
oesukam
changed the title
…ilure - test_connection falls back to the stored keychain password (DB + SSH) when the form field is empty and the input has an id — so re-testing an edited connection (password blanked for security) uses the saved secret instead of failing with a spurious auth error. - onSaveAndConnect: on a post-save connect failure for a BRAND-NEW connection, roll back the save so a dead entry isn't left in the rail; surface the real 'Connect failed' message (not a mislabeled 'Save failed'). - SSH tests: add wrong-password-rejected and tunnel-to-dead-target tests, and pre-trust the container host key (mirrors the UI's TOFU prompt→trust flow) so container-based tunnel tests are deterministic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related fixes surfaced while debugging repeated 'Authentication failed' reports.
1. Connect verifies before saving
Per request: Connect should confirm the connection works, then save — not save first.
onSaveAndConnectnow callstest_connection(with the form's own credentials) up front and onlysaves + opens a session on success. A failed connect no longer leaves a broken saved connection behind.2. Saving no longer wipes the stored password (data-loss bug)
save_connectiondidif let Some(pw) = input.password— but the form blanks the password when editing (secrets are never read back into the UI), soinput.passwordisSome(""). That overwrote the keychain with an empty password, breaking the next connect. Now it only writes a secret when a non-empty value is supplied (same for SSH password/passphrase).Verified
cargo check/clippyclean; frontend typecheck + lint (0 errors).