Skip to content

Commit 3267132

Browse files
datlechindatlechin
committed
fix: address all PR review comments
1 parent a9e56a8 commit 3267132

21 files changed

Lines changed: 171 additions & 116 deletions

‎TablePro/Core/SSH/Auth/AgentAuthenticator.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
// TablePro
44
//
55

6-
import CLibSSH2
76
import Foundation
87
import os
98

10-
struct AgentAuthenticator: SSHAuthenticator {
9+
import CLibSSH2
10+
11+
internal struct AgentAuthenticator: SSHAuthenticator {
1112
private static let logger = Logger(subsystem: "com.TablePro", category: "AgentAuthenticator")
1213

1314
/// Protects setenv/unsetenv of SSH_AUTH_SOCK across concurrent tunnel setups

‎TablePro/Core/SSH/Auth/CompositeAuthenticator.swift‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,28 @@
33
// TablePro
44
//
55

6-
import CLibSSH2
76
import Foundation
87
import os
98

9+
import CLibSSH2
10+
1011
/// Authenticator that tries multiple auth methods in sequence.
1112
/// Used for servers requiring e.g. password + keyboard-interactive (TOTP).
12-
struct CompositeAuthenticator: SSHAuthenticator {
13+
internal struct CompositeAuthenticator: SSHAuthenticator {
1314
private static let logger = Logger(subsystem: "com.TablePro", category: "CompositeAuthenticator")
1415

1516
let authenticators: [any SSHAuthenticator]
1617

1718
func authenticate(session: OpaquePointer, username: String) throws {
19+
var lastError: Error?
1820
for (index, authenticator) in authenticators.enumerated() {
19-
Self.logger.debug(
20-
"Trying authenticator \(index + 1)/\(authenticators.count): \(String(describing: type(of: authenticator)))"
21-
)
22-
23-
try authenticator.authenticate(session: session, username: username)
21+
Self.logger.debug("Trying authenticator \(index + 1)/\(authenticators.count)")
22+
do {
23+
try authenticator.authenticate(session: session, username: username)
24+
} catch {
25+
Self.logger.debug("Authenticator \(index + 1) failed: \(error)")
26+
lastError = error
27+
}
2428

2529
if libssh2_userauth_authenticated(session) != 0 {
2630
Self.logger.info("Authentication succeeded after \(index + 1) step(s)")
@@ -29,7 +33,7 @@ struct CompositeAuthenticator: SSHAuthenticator {
2933
}
3034

3135
if libssh2_userauth_authenticated(session) == 0 {
32-
throw SSHTunnelError.authenticationFailed
36+
throw lastError ?? SSHTunnelError.authenticationFailed
3337
}
3438
}
3539
}

‎TablePro/Core/SSH/Auth/KeyboardInteractiveAuthenticator.swift‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,20 @@
33
// TablePro
44
//
55

6-
import CLibSSH2
76
import Foundation
87
import os
98

9+
import CLibSSH2
10+
1011
/// Prompt type classification for keyboard-interactive authentication
11-
enum KBDINTPromptType {
12+
internal enum KBDINTPromptType {
1213
case password
1314
case totp
1415
case unknown
1516
}
1617

1718
/// Context passed through the libssh2 session abstract pointer to the C callback
18-
final class KeyboardInteractiveContext {
19+
internal final class KeyboardInteractiveContext {
1920
var password: String?
2021
var totpCode: String?
2122

@@ -82,7 +83,7 @@ private let kbdintCallback: @convention(c) (
8283
}
8384
}
8485

85-
struct KeyboardInteractiveAuthenticator: SSHAuthenticator {
86+
internal struct KeyboardInteractiveAuthenticator: SSHAuthenticator {
8687
private static let logger = Logger(
8788
subsystem: "com.TablePro",
8889
category: "KeyboardInteractiveAuthenticator"

‎TablePro/Core/SSH/Auth/PasswordAuthenticator.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
// TablePro
44
//
55

6-
import CLibSSH2
76
import Foundation
87

9-
struct PasswordAuthenticator: SSHAuthenticator {
8+
import CLibSSH2
9+
10+
internal struct PasswordAuthenticator: SSHAuthenticator {
1011
let password: String
1112

1213
func authenticate(session: OpaquePointer, username: String) throws {

‎TablePro/Core/SSH/Auth/PromptTOTPProvider.swift‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,39 +10,48 @@ import Foundation
1010
///
1111
/// This provider blocks the calling thread while the alert is displayed on the main thread.
1212
/// It is intended for interactive SSH sessions where no TOTP secret is configured.
13-
final class PromptTOTPProvider: TOTPProvider, @unchecked Sendable {
13+
internal final class PromptTOTPProvider: TOTPProvider, @unchecked Sendable {
1414
func provideCode() throws -> String {
15+
if Thread.isMainThread {
16+
return try handleResult(showAlert())
17+
}
18+
1519
let semaphore = DispatchSemaphore(value: 0)
1620
var code: String?
17-
1821
DispatchQueue.main.async {
19-
let alert = NSAlert()
20-
alert.messageText = String(localized: "Verification Code Required")
21-
alert.informativeText = String(
22-
localized: "Enter the TOTP verification code for SSH authentication."
23-
)
24-
alert.alertStyle = .informational
25-
alert.addButton(withTitle: String(localized: "Connect"))
26-
alert.addButton(withTitle: String(localized: "Cancel"))
27-
28-
let textField = NSTextField(frame: NSRect(x: 0, y: 0, width: 200, height: 24))
29-
textField.placeholderString = "000000"
30-
alert.accessoryView = textField
31-
alert.window.initialFirstResponder = textField
32-
33-
let response = alert.runModal()
34-
if response == .alertFirstButtonReturn {
35-
code = textField.stringValue
36-
}
22+
code = self.showAlert()
3723
semaphore.signal()
3824
}
39-
4025
let result = semaphore.wait(timeout: .now() + 120)
41-
4226
guard result == .success else {
4327
throw SSHTunnelError.connectionTimeout
4428
}
29+
return try handleResult(code)
30+
}
31+
32+
private func showAlert() -> String? {
33+
let alert = NSAlert()
34+
alert.messageText = String(localized: "Verification Code Required")
35+
alert.informativeText = String(
36+
localized: "Enter the TOTP verification code for SSH authentication."
37+
)
38+
alert.alertStyle = .informational
39+
alert.addButton(withTitle: String(localized: "Connect"))
40+
alert.addButton(withTitle: String(localized: "Cancel"))
41+
42+
let textField = NSTextField(frame: NSRect(x: 0, y: 0, width: 200, height: 24))
43+
textField.placeholderString = "000000"
44+
alert.accessoryView = textField
45+
alert.window.initialFirstResponder = textField
46+
47+
let response = alert.runModal()
48+
if response == .alertFirstButtonReturn {
49+
return textField.stringValue
50+
}
51+
return nil
52+
}
4553

54+
private func handleResult(_ code: String?) throws -> String {
4655
guard let totpCode = code, !totpCode.isEmpty else {
4756
throw SSHTunnelError.authenticationFailed
4857
}

‎TablePro/Core/SSH/Auth/PublicKeyAuthenticator.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
// TablePro
44
//
55

6-
import CLibSSH2
76
import Foundation
87

9-
struct PublicKeyAuthenticator: SSHAuthenticator {
8+
import CLibSSH2
9+
10+
internal struct PublicKeyAuthenticator: SSHAuthenticator {
1011
let privateKeyPath: String
1112
let passphrase: String?
1213

‎TablePro/Core/SSH/Auth/SSHAuthenticator.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
// TablePro
44
//
55

6-
import CLibSSH2
76
import Foundation
87

8+
import CLibSSH2
9+
910
/// Protocol for SSH authentication methods
10-
protocol SSHAuthenticator: Sendable {
11+
internal protocol SSHAuthenticator: Sendable {
1112
/// Authenticate the SSH session
1213
/// - Parameters:
1314
/// - session: libssh2 session pointer

‎TablePro/Core/SSH/Auth/TOTPProvider.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import Foundation
77

88
/// Protocol for providing TOTP verification codes
9-
protocol TOTPProvider: Sendable {
9+
internal protocol TOTPProvider: Sendable {
1010
/// Generate or obtain a TOTP code
1111
/// - Returns: The TOTP code string
1212
/// - Throws: SSHTunnelError if the code cannot be obtained
@@ -18,7 +18,7 @@ protocol TOTPProvider: Sendable {
1818
/// If the current code expires in less than 5 seconds, waits for the next
1919
/// period to avoid submitting a code that expires during the authentication handshake.
2020
/// The maximum wait is ~6 seconds (bounded).
21-
struct AutoTOTPProvider: TOTPProvider {
21+
internal struct AutoTOTPProvider: TOTPProvider {
2222
let generator: TOTPGenerator
2323

2424
func provideCode() throws -> String {

‎TablePro/Core/SSH/HostKeyStore.swift‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import Foundation
1212
import os
1313

1414
/// Manages SSH host key verification for known hosts
15-
final class HostKeyStore: @unchecked Sendable {
15+
internal final class HostKeyStore: @unchecked Sendable {
1616
static let shared = HostKeyStore()
1717

1818
private static let logger = Logger(subsystem: "com.TablePro", category: "HostKeyStore")
@@ -27,11 +27,14 @@ final class HostKeyStore: @unchecked Sendable {
2727
private let lock = NSLock()
2828

2929
private init() {
30-
let appSupport = FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask).first!
30+
guard let appSupport = FileManager.default.urls(
31+
for: .applicationSupportDirectory, in: .userDomainMask
32+
).first else {
33+
self.filePath = NSTemporaryDirectory() + "TablePro_known_hosts"
34+
return
35+
}
3136
let tableProDir = appSupport.appendingPathComponent("TablePro")
32-
3337
try? FileManager.default.createDirectory(at: tableProDir, withIntermediateDirectories: true)
34-
3538
self.filePath = tableProDir.appendingPathComponent("known_hosts").path
3639
}
3740

@@ -57,7 +60,7 @@ final class HostKeyStore: @unchecked Sendable {
5760
let currentFingerprint = Self.fingerprint(of: keyData)
5861
let entries = loadEntries()
5962

60-
guard let existing = entries.first(where: { $0.host == hostKey }) else {
63+
guard let existing = entries.first(where: { $0.host == hostKey && $0.keyType == keyType }) else {
6164
Self.logger.info("Unknown host key for \(hostKey)")
6265
return .unknown(fingerprint: currentFingerprint, keyType: keyType)
6366
}
@@ -85,8 +88,8 @@ final class HostKeyStore: @unchecked Sendable {
8588
let hostKey = hostIdentifier(hostname, port)
8689
var entries = loadEntries()
8790

88-
// Remove existing entry for this host if present
89-
entries.removeAll { $0.host == hostKey }
91+
// Remove existing entry for this host and key type if present
92+
entries.removeAll { $0.host == hostKey && $0.keyType == keyType }
9093

9194
entries.append((host: hostKey, keyType: keyType, keyData: key))
9295
saveEntries(entries)

‎TablePro/Core/SSH/HostKeyVerifier.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import Foundation
1111
import os
1212

1313
/// Handles host key verification with UI prompts
14-
enum HostKeyVerifier {
14+
internal enum HostKeyVerifier {
1515
private static let logger = Logger(subsystem: "com.TablePro", category: "HostKeyVerifier")
1616

1717
/// Verify the host key, prompting the user if needed.

0 commit comments

Comments
 (0)