Skip to content

Commit 4cd2ff4

Browse files
authored
fix(import): restore TablePlus password and SSH key import (#1388)
* fix(import): restore TablePlus password and SSH key import * refactor(import): preserve explicit TablePlus key paths and add test seams
1 parent 488e065 commit 4cd2ff4

10 files changed

Lines changed: 204 additions & 15 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717

1818
### Fixed
1919

20+
- Importing connections from TablePlus brings over saved passwords again. A recent release looked under the wrong keychain name, so connections imported with no passwords and no warning.
21+
- Importing an SSH connection from TablePlus no longer fills in a fake private key path such as `~/.ssh/Import a private key...` when no key was selected. Empty TLS certificate paths are skipped too.
22+
- Importing from DBeaver no longer shows an unnecessary keychain permission warning. DBeaver stores passwords in its own file, so macOS never prompts.
2023
- Raw SQL filter now suggests columns and keywords at every position in the expression, including after AND and OR, instead of only the first column. (#1346)
2124
- Plugins left incompatible after a TablePro update now update quietly in the background instead of showing a premature "could not be loaded" alert. You are only notified when no compatible version exists yet, and the message tells you what to do. (#1322)
2225
- A plugin you download and install by hand is no longer blocked by macOS Gatekeeper once its signature is verified. (#1322)

TablePro/Core/Services/Export/ForeignApp/BeekeeperStudioImporter.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ struct BeekeeperStudioImporter: ForeignAppImporter {
2626
let displayName = "Beekeeper Studio"
2727
let symbolName = "ant"
2828
let appBundleIdentifier = "io.beekeeperstudio.desktop"
29+
let readsPasswordsFromKeychain = false
2930

3031
var dataDirectoryURL: URL = FileManager.default.homeDirectoryForCurrentUser
3132
.appendingPathComponent("Library/Application Support/beekeeper-studio")

TablePro/Core/Services/Export/ForeignApp/DBeaverImporter.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ struct DBeaverImporter: ForeignAppImporter {
1616
let displayName = "DBeaver"
1717
let symbolName = "bird"
1818
let appBundleIdentifier = "org.jkiss.dbeaver.core.product"
19+
let readsPasswordsFromKeychain = false
1920

2021
/// All known DBeaver Eclipse product identifiers. Community, Enterprise,
2122
/// Ultimate, and Lite variants each register a different bundle ID, but

TablePro/Core/Services/Export/ForeignApp/DataGripImporter.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ struct DataGripImporter: ForeignAppImporter {
1515
let displayName = "DataGrip"
1616
let symbolName = "cylinder.split.1x2"
1717
let appBundleIdentifier = "com.jetbrains.datagrip"
18+
let readsPasswordsFromKeychain = true
1819

1920
/// Root holding versioned IDE config dirs (`DataGrip2024.3`, ...). Injectable for tests.
2021
var jetBrainsRoot: URL = FileManager.default.homeDirectoryForCurrentUser

TablePro/Core/Services/Export/ForeignApp/ForeignAppImporter.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ protocol ForeignAppImporter {
1818
/// app ships in multiple editions (e.g. DBeaver Community / Enterprise)
1919
/// should override `installedAppURL()` to look those up as well.
2020
var appBundleIdentifier: String { get }
21+
/// True when importing passwords reads the macOS keychain, which makes the
22+
/// system show a per-item access prompt. Importers that read passwords from
23+
/// a file (DBeaver, Beekeeper Studio) return false so no prompt is promised.
24+
var readsPasswordsFromKeychain: Bool { get }
2125
func installedAppURL() -> URL?
2226
func connectionCount() -> Int
2327
func importConnections(includePasswords: Bool) throws -> ForeignAppImportResult
@@ -103,6 +107,8 @@ enum KeychainReadResult {
103107
case cancelled
104108
}
105109

110+
typealias ForeignKeychainRead = (_ service: String, _ account: String) -> KeychainReadResult
111+
106112
enum ForeignKeychainReader {
107113
private static let logger = Logger(subsystem: "com.TablePro", category: "ForeignKeychainReader")
108114

TablePro/Core/Services/Export/ForeignApp/SequelAceImporter.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ struct SequelAceImporter: ForeignAppImporter {
1313
let displayName = "Sequel Ace"
1414
let symbolName = "cylinder.split.1x2"
1515
let appBundleIdentifier = "com.sequel-ace.sequel-ace"
16+
let readsPasswordsFromKeychain = true
1617

1718
var favoritesFileURL: URL = FileManager.default.homeDirectoryForCurrentUser
1819
.appendingPathComponent(

TablePro/Core/Services/Export/ForeignApp/TablePlusImporter.swift

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ struct TablePlusImporter: ForeignAppImporter {
1414
let displayName = "TablePlus"
1515
let symbolName = "rectangle.stack"
1616
let appBundleIdentifier = "com.tinyapp.TablePlus"
17+
let readsPasswordsFromKeychain = true
18+
19+
static let keychainService = "com.tableplus.TablePlus"
20+
21+
var readKeychain: ForeignKeychainRead = ForeignKeychainReader.readPassword
22+
var keyFileExists: (_ path: String) -> Bool = { FileManager.default.fileExists(atPath: $0) }
1723

1824
var connectionsFileURL: URL = FileManager.default.homeDirectoryForCurrentUser
1925
.appendingPathComponent("Library/Application Support/com.tinyapp.TablePlus/Data/Connections.plist")
@@ -182,16 +188,15 @@ struct TablePlusImporter: ForeignAppImporter {
182188
let port = (entry["ServerPort"] as? String).flatMap(Int.init)
183189
let username = entry["ServerUser"] as? String ?? ""
184190
let useKey = entry["isUsePrivateKey"] as? Bool ?? false
185-
let rawKeyPath = entry["ServerPrivateKeyName"] as? String ?? ""
186-
let keyPath = ForeignAppPathHelper.resolveKeyPath(rawKeyPath)
191+
let keyPath = useKey ? importedKeyPath(entry["ServerPrivateKeyName"] as? String ?? "") : ""
187192

188193
return ExportableSSHConfig(
189194
enabled: true,
190195
host: host,
191196
port: port,
192197
username: username,
193198
authMethod: useKey ? "Private Key" : "Password",
194-
privateKeyPath: useKey ? keyPath : "",
199+
privateKeyPath: keyPath,
195200
agentSocketPath: "",
196201
jumpHosts: nil,
197202
totpMode: nil,
@@ -201,6 +206,14 @@ struct TablePlusImporter: ForeignAppImporter {
201206
)
202207
}
203208

209+
private func importedKeyPath(_ rawName: String) -> String {
210+
let trimmed = rawName.trimmingCharacters(in: .whitespaces)
211+
let resolved = ForeignAppPathHelper.resolveKeyPath(trimmed)
212+
guard !resolved.isEmpty else { return "" }
213+
if trimmed.hasPrefix("/") || trimmed.hasPrefix("~/") { return resolved }
214+
return keyFileExists(PathPortability.expandHome(resolved)) ? resolved : ""
215+
}
216+
204217
private func parseSSLConfig(_ entry: [String: Any]) -> ExportableSSLConfig? {
205218
guard entry.keys.contains("tLSMode") else { return nil }
206219
let tlsMode = entry["tLSMode"] as? Int ?? 0
@@ -215,18 +228,22 @@ struct TablePlusImporter: ForeignAppImporter {
215228
}
216229

217230
let paths = entry["TlsKeyPaths"] as? [String] ?? []
231+
func certPath(_ index: Int) -> String? {
232+
guard index < paths.count, !paths[index].isEmpty else { return nil }
233+
return paths[index]
234+
}
218235
return ExportableSSLConfig(
219236
mode: mode,
220-
caCertificatePath: !paths.isEmpty ? paths[0] : nil,
221-
clientCertificatePath: paths.count > 1 ? paths[1] : nil,
222-
clientKeyPath: paths.count > 2 ? paths[2] : nil
237+
caCertificatePath: certPath(0),
238+
clientCertificatePath: certPath(1),
239+
clientKeyPath: certPath(2)
223240
)
224241
}
225242

226243
private func readCredentials(for connectionId: String, abortFlag: inout Bool) -> ExportableCredentials {
227244
func read(_ account: String) -> String? {
228245
guard !abortFlag else { return nil }
229-
switch ForeignKeychainReader.readPassword(service: "com.tinyapp.TablePlus", account: account) {
246+
switch readKeychain(Self.keychainService, account) {
230247
case .found(let value):
231248
return value
232249
case .notFound:

TablePro/Views/Connection/ImportFromApp/ImportFromAppSheet.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,13 @@ struct ImportFromAppSheet: View {
101101

102102
// MARK: - Actions
103103

104+
static func requiresKeychainConfirmation(includePasswords: Bool, importer: any ForeignAppImporter) -> Bool {
105+
includePasswords && importer.readsPasswordsFromKeychain
106+
}
107+
104108
private func beginImport(importer: any ForeignAppImporter, includePasswords: Bool) {
105-
if includePasswords, !confirmKeychainPrompts(for: importer) {
109+
if Self.requiresKeychainConfirmation(includePasswords: includePasswords, importer: importer),
110+
!confirmKeychainPrompts(for: importer) {
106111
return
107112
}
108113
startImport(importer: importer, includePasswords: includePasswords)

TableProTests/Core/Services/ForeignApp/ForeignAppImporterRegistryTests.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,20 @@ struct ForeignAppImporterRegistryTests {
8585
#expect(importer.displayName == "Beekeeper Studio")
8686
#expect(importer.appBundleIdentifier == "io.beekeeperstudio.desktop")
8787
}
88+
89+
@Test("Importers declare whether passwords are read from the keychain")
90+
func testReadsPasswordsFromKeychainFlags() {
91+
#expect(TablePlusImporter().readsPasswordsFromKeychain == true)
92+
#expect(SequelAceImporter().readsPasswordsFromKeychain == true)
93+
#expect(DataGripImporter().readsPasswordsFromKeychain == true)
94+
#expect(DBeaverImporter().readsPasswordsFromKeychain == false)
95+
#expect(BeekeeperStudioImporter().readsPasswordsFromKeychain == false)
96+
}
97+
98+
@Test("Keychain confirmation applies only to keychain-based importers when importing passwords")
99+
func testRequiresKeychainConfirmation() {
100+
#expect(ImportFromAppSheet.requiresKeychainConfirmation(includePasswords: true, importer: TablePlusImporter()))
101+
#expect(!ImportFromAppSheet.requiresKeychainConfirmation(includePasswords: true, importer: DBeaverImporter()))
102+
#expect(!ImportFromAppSheet.requiresKeychainConfirmation(includePasswords: false, importer: TablePlusImporter()))
103+
}
88104
}

TableProTests/Core/Services/ForeignApp/TablePlusImporterTests.swift

Lines changed: 145 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ struct TablePlusImporterTests {
168168
}
169169
}
170170

171-
@Test("importConnections parses SSH config")
171+
@Test("importConnections parses SSH config and keeps an explicit key path even when the file is missing")
172172
func testImportConnections_parsesSSHConfig() throws {
173173
try writeConnections([
174174
makeConnection(
@@ -179,21 +179,68 @@ struct TablePlusImporterTests {
179179
sshPort: "2222",
180180
sshUser: "deploy",
181181
usePrivateKey: true,
182-
privateKeyPath: "~/.ssh/id_rsa"
182+
privateKeyPath: "/Users/test/.ssh/id_rsa"
183183
)
184184
])
185185

186-
let result = try importer.importConnections(includePasswords: false)
187-
let conn = result.envelope.connections[0]
188-
let ssh = conn.sshConfig
186+
var imp = importer
187+
imp.keyFileExists = { _ in false }
188+
189+
let result = try imp.importConnections(includePasswords: false)
190+
let ssh = result.envelope.connections[0].sshConfig
189191

190192
#expect(ssh != nil)
191193
#expect(ssh?.enabled == true)
192194
#expect(ssh?.host == "bastion.example.com")
193-
#expect(ssh?.port == 2222)
195+
#expect(ssh?.port == 2_222)
194196
#expect(ssh?.username == "deploy")
195197
#expect(ssh?.authMethod == "Private Key")
196-
#expect(ssh?.privateKeyPath == "~/.ssh/id_rsa")
198+
#expect(ssh?.privateKeyPath == "/Users/test/.ssh/id_rsa")
199+
}
200+
201+
@Test("importConnections drops the empty-key placeholder instead of building a fake path")
202+
func testImportConnections_placeholderPrivateKey_producesNoPath() throws {
203+
try writeConnections([
204+
makeConnection(
205+
name: "SSH Placeholder",
206+
id: "ssh-placeholder",
207+
isOverSSH: true,
208+
sshHost: "bastion.example.com",
209+
sshUser: "deploy",
210+
usePrivateKey: true,
211+
privateKeyPath: "Import a private key..."
212+
)
213+
])
214+
215+
var imp = importer
216+
imp.keyFileExists = { _ in false }
217+
218+
let result = try imp.importConnections(includePasswords: false)
219+
let ssh = result.envelope.connections[0].sshConfig
220+
221+
#expect(ssh?.authMethod == "Private Key")
222+
#expect(ssh?.privateKeyPath == "")
223+
}
224+
225+
@Test("importConnections keeps a bare key name when the file exists in ~/.ssh")
226+
func testImportConnections_bareKeyName_keptWhenFileExists() throws {
227+
try writeConnections([
228+
makeConnection(
229+
name: "SSH Bare Key",
230+
id: "ssh-bare",
231+
isOverSSH: true,
232+
sshHost: "bastion.example.com",
233+
sshUser: "deploy",
234+
usePrivateKey: true,
235+
privateKeyPath: "id_rsa"
236+
)
237+
])
238+
239+
var imp = importer
240+
imp.keyFileExists = { _ in true }
241+
242+
let result = try imp.importConnections(includePasswords: false)
243+
#expect(result.envelope.connections[0].sshConfig?.privateKeyPath == "~/.ssh/id_rsa")
197244
}
198245

199246
@Test("importConnections parses SSH config with password auth")
@@ -296,6 +343,21 @@ struct TablePlusImporterTests {
296343
#expect(result.envelope.connections[0].sslConfig == nil)
297344
}
298345

346+
@Test("importConnections treats empty TablePlus TLS paths as none")
347+
func testImportConnections_emptyTLSPaths_areNil() throws {
348+
var entry = makeConnection(name: "Empty TLS", id: "tls-empty", tlsMode: 1)
349+
entry["TlsKeyPaths"] = ["", "", ""]
350+
try writeConnections([entry])
351+
352+
let result = try importer.importConnections(includePasswords: false)
353+
let ssl = result.envelope.connections[0].sslConfig
354+
355+
#expect(ssl != nil)
356+
#expect(ssl?.caCertificatePath == nil)
357+
#expect(ssl?.clientCertificatePath == nil)
358+
#expect(ssl?.clientKeyPath == nil)
359+
}
360+
299361
@Test("importConnections preserves groups")
300362
func testImportConnections_preservesGroups() throws {
301363
try writeGroups([
@@ -444,4 +506,80 @@ struct TablePlusImporterTests {
444506
#expect(result.envelope.appVersion == "TablePlus Import")
445507
#expect(result.envelope.tags == nil)
446508
}
509+
510+
// MARK: - Password Import
511+
512+
@Test("importConnections reads the database password from the correct keychain service")
513+
func testImportConnections_readsCorrectKeychainServiceAndAccount() throws {
514+
try writeConnections([makeConnection(name: "DB", id: "conn-1")])
515+
let spy = KeychainSpy()
516+
spy.responses["conn-1_database"] = .found("s3cret")
517+
518+
var imp = importer
519+
imp.readKeychain = spy.read
520+
521+
let result = try imp.importConnections(includePasswords: true)
522+
523+
#expect(spy.calls.contains { $0.service == "com.tableplus.TablePlus" && $0.account == "conn-1_database" })
524+
#expect(result.envelope.credentials?["0"]?.password == "s3cret")
525+
#expect(result.credentialsAborted == false)
526+
}
527+
528+
@Test("importConnections queries database, SSH, and key-passphrase accounts")
529+
func testImportConnections_queriesAllCredentialAccounts() throws {
530+
try writeConnections([makeConnection(name: "DB", id: "conn-1")])
531+
let spy = KeychainSpy()
532+
533+
var imp = importer
534+
imp.readKeychain = spy.read
535+
536+
_ = try imp.importConnections(includePasswords: true)
537+
538+
let accounts = Set(spy.calls.map(\.account))
539+
#expect(accounts == ["conn-1_database", "conn-1_server", "conn-1_server_key"])
540+
#expect(spy.calls.allSatisfy { $0.service == "com.tableplus.TablePlus" })
541+
}
542+
543+
@Test("importConnections leaves credentials empty and does not abort when nothing is stored")
544+
func testImportConnections_noStoredPasswords_emptyCredentialsNoAbort() throws {
545+
try writeConnections([makeConnection(name: "DB", id: "conn-1")])
546+
let spy = KeychainSpy()
547+
548+
var imp = importer
549+
imp.readKeychain = spy.read
550+
551+
let result = try imp.importConnections(includePasswords: true)
552+
553+
#expect(result.envelope.credentials == nil)
554+
#expect(result.credentialsAborted == false)
555+
}
556+
557+
@Test("importConnections aborts and stops reading after a cancelled keychain prompt")
558+
func testImportConnections_cancelledPrompt_abortsAndStops() throws {
559+
try writeConnections([
560+
makeConnection(name: "A", id: "c1"),
561+
makeConnection(name: "B", id: "c2")
562+
])
563+
let spy = KeychainSpy()
564+
spy.responses["c1_database"] = .cancelled
565+
566+
var imp = importer
567+
imp.readKeychain = spy.read
568+
569+
let result = try imp.importConnections(includePasswords: true)
570+
571+
#expect(result.credentialsAborted == true)
572+
#expect(spy.calls.count == 1)
573+
#expect(spy.calls.first?.account == "c1_database")
574+
}
575+
}
576+
577+
private final class KeychainSpy {
578+
var calls: [(service: String, account: String)] = []
579+
var responses: [String: KeychainReadResult] = [:]
580+
581+
func read(_ service: String, _ account: String) -> KeychainReadResult {
582+
calls.append((service: service, account: account))
583+
return responses[account] ?? .notFound
584+
}
447585
}

0 commit comments

Comments
 (0)