From 31720c198764aa82d227b96f4f5c798883f94c9c Mon Sep 17 00:00:00 2001 From: hcagricakir Date: Thu, 9 Jul 2026 12:33:46 +0300 Subject: [PATCH 1/2] fix: reprompt mysql password after auth-expired reconnect --- CHANGELOG.md | 4 + .../Database/ConnectionHealthMonitor.swift | 24 ++- .../Database/DatabaseManager+Health.swift | 199 ++++++++++++++---- TablePro/Core/Database/DatabaseManager.swift | 1 - .../ReconnectCredentialRecoveryTests.swift | 68 ++++++ 5 files changed, 243 insertions(+), 53 deletions(-) create mode 100644 TableProTests/Core/Database/ReconnectCredentialRecoveryTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d1fed34b..a4fa73620 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- MySQL and MariaDB connections that prompt for a password at connect time now ask for a fresh password after an auto-reconnect hits an authentication failure such as `1045` / `SQLSTATE 28000`, instead of looping forever in Connecting with the expired session password. + ## [0.56.1] - 2026-07-09 ### Removed diff --git a/TablePro/Core/Database/ConnectionHealthMonitor.swift b/TablePro/Core/Database/ConnectionHealthMonitor.swift index ed9b2cd3b..e22e6ecce 100644 --- a/TablePro/Core/Database/ConnectionHealthMonitor.swift +++ b/TablePro/Core/Database/ConnectionHealthMonitor.swift @@ -18,6 +18,12 @@ extension ConnectionHealthMonitor { case checking case reconnecting(attempt: Int) // 1-based attempt number } + + enum ReconnectOutcome: Sendable, Equatable { + case success + case retry + case abort + } } // MARK: - ConnectionHealthMonitor @@ -40,7 +46,7 @@ actor ConnectionHealthMonitor { private let connectionId: UUID private let pingHandler: @Sendable () async -> Bool - private let reconnectHandler: @Sendable () async -> Bool + private let reconnectHandler: @Sendable () async -> ReconnectOutcome private let onStateChanged: @Sendable (UUID, HealthState) async -> Void // MARK: - State @@ -59,12 +65,12 @@ actor ConnectionHealthMonitor { /// - pingHandler: Closure that executes a lightweight query (e.g., `SELECT 1`) /// and returns `true` if the connection is alive. /// - reconnectHandler: Closure that attempts to re-establish the connection - /// and returns `true` on success. + /// and reports whether the monitor should mark success, retry later, or stop. /// - onStateChanged: Closure invoked whenever the health state transitions. init( connectionId: UUID, pingHandler: @escaping @Sendable () async -> Bool, - reconnectHandler: @escaping @Sendable () async -> Bool, + reconnectHandler: @escaping @Sendable () async -> ReconnectOutcome, onStateChanged: @escaping @Sendable (UUID, HealthState) async -> Void ) { self.connectionId = connectionId @@ -188,15 +194,17 @@ actor ConnectionHealthMonitor { return } - let success = await reconnectHandler() - - if success { + switch await reconnectHandler() { + case .success: Self.logger.info("Reconnect succeeded on attempt \(attempt) for connection \(self.connectionId)") await transitionTo(.healthy) return + case .abort: + Self.logger.info("Reconnect aborted for connection \(self.connectionId)") + return + case .retry: + Self.logger.warning("Reconnect attempt \(attempt) failed for connection \(self.connectionId)") } - - Self.logger.warning("Reconnect attempt \(attempt) failed for connection \(self.connectionId)") } Self.logger.debug("Reconnect loop cancelled after \(attempt) attempts for connection \(self.connectionId)") diff --git a/TablePro/Core/Database/DatabaseManager+Health.swift b/TablePro/Core/Database/DatabaseManager+Health.swift index ccec16033..73207981e 100644 --- a/TablePro/Core/Database/DatabaseManager+Health.swift +++ b/TablePro/Core/Database/DatabaseManager+Health.swift @@ -14,6 +14,12 @@ import TableProPluginKit // MARK: - Health Monitoring extension DatabaseManager { + internal enum ReconnectCredentialResolution: Equatable { + case fail + case retry(String) + case abort + } + /// Start health monitoring for a connection internal func startHealthMonitor(for connectionId: UUID) async { Self.logger.info("startHealthMonitor called for \(connectionId) (existing monitors: \(self.healthMonitors.count))") @@ -49,26 +55,36 @@ extension DatabaseManager { } }, reconnectHandler: { [weak self] in - guard let self else { return false } - guard let session = await self.activeSessions[connectionId] else { return false } + guard let self else { return .abort } + guard let session = await self.activeSessions[connectionId] else { return .abort } await SchemaService.shared.invalidate(connectionId: connectionId) await DatabaseTreeMetadataService.shared.handleReconnect(connectionId: connectionId) do { - let result = try await self.trackOperation(sessionId: connectionId) { + guard let result = try await self.trackOperation(sessionId: connectionId) { try await self.reconnectDriver(for: session) + } else { + self.updateSession(connectionId) { session in + session.status = .error(String(localized: "Reconnect cancelled")) + } + return .abort } - await self.updateSession(connectionId) { session in + self.updateSession(connectionId) { session in session.driver = result.driver session.effectiveConnection = result.effectiveConnection session.status = .connected if let schemaDriver = result.driver as? SchemaSwitchable { session.currentSchema = schemaDriver.currentSchema } + if let cachedPassword = result.cachedPassword, + !session.connection.usesAWSIAM + { + session.cachedPassword = cachedPassword + } } - return true + return .success } catch { Self.logger.debug("Reconnect failed: \(error.localizedDescription)") - return false + return .retry } }, onStateChanged: { [weak self] id, state in @@ -106,11 +122,12 @@ extension DatabaseManager { internal struct ReconnectResult { let driver: DatabaseDriver let effectiveConnection: DatabaseConnection + let cachedPassword: String? } /// Creates a fresh driver, connects, and applies timeout for the given session. /// For SSH-tunneled sessions, rebuilds the tunnel before connecting the driver. - internal func reconnectDriver(for session: ConnectionSession) async throws -> ReconnectResult { + internal func reconnectDriver(for session: ConnectionSession) async throws -> ReconnectResult? { session.driver?.disconnect() // Rebuild the tunnel if needed; otherwise reuse effective connection @@ -121,32 +138,14 @@ extension DatabaseManager { connectionForDriver = session.effectiveConnection ?? session.connection } - let driver = try await DatabaseDriverFactory.createDriver( - for: connectionForDriver, - passwordOverride: session.cachedPassword, - awaitPlugins: true - ) - - do { - try await driver.connect() - } catch { - driver.disconnect() - if session.connection.resolvedSSHConfig.enabled { - do { - try await SSHTunnelManager.shared.closeTunnel(connectionId: session.connection.id) - } catch { - Self.logger.warning("Failed to close SSH tunnel during reconnect: \(error.localizedDescription)") - } - } - if session.connection.isCloudflareEnabled { - do { - try await CloudflareTunnelManager.shared.closeTunnel(connectionId: session.connection.id) - } catch { - Self.logger.warning("Failed to close Cloudflare tunnel during reconnect: \(error.localizedDescription)") - } - } - throw error + guard let connectResult = try await connectReconnectDriver( + for: session, + effectiveConnection: connectionForDriver, + passwordOverride: session.cachedPassword + ) else { + return nil } + let driver = connectResult.driver await applyTimeoutAndStartupCommands( on: driver, @@ -159,7 +158,11 @@ extension DatabaseManager { savedDatabase: databaseSwitchRequiresReconnect(session.connection) ? nil : session.currentDatabase ) - return ReconnectResult(driver: driver, effectiveConnection: connectionForDriver) + return ReconnectResult( + driver: driver, + effectiveConnection: connectionForDriver, + cachedPassword: connectResult.cachedPassword + ) } func applyTimeoutAndStartupCommands( @@ -250,9 +253,9 @@ extension DatabaseManager { { let isApiOnly = pluginManager.connectionMode(for: session.connection.type) == .apiOnly guard let prompted = await PasswordPromptHelper.prompt( - connectionName: session.connection.name, - isAPIToken: isApiOnly, - window: NSApp.keyWindow + session.connection.name, + isApiOnly, + NSApp.keyWindow ) else { updateSession(sessionId) { $0.status = .disconnected } return @@ -260,12 +263,15 @@ extension DatabaseManager { passwordOverride = prompted } - let driver = try await DatabaseDriverFactory.createDriver( - for: effectiveConnection, - passwordOverride: passwordOverride, - awaitPlugins: true - ) - try await driver.connect() + guard let connectResult = try await connectReconnectDriver( + for: session, + effectiveConnection: effectiveConnection, + passwordOverride: passwordOverride + ) else { + updateSession(sessionId) { $0.status = .disconnected } + return + } + let driver = connectResult.driver await applyTimeoutAndStartupCommands( on: driver, @@ -285,8 +291,10 @@ extension DatabaseManager { if let schemaDriver = driver as? SchemaSwitchable { session.currentSchema = schemaDriver.currentSchema } - if let passwordOverride, !session.connection.usesAWSIAM { - session.cachedPassword = passwordOverride + if let cachedPassword = connectResult.cachedPassword, + !session.connection.usesAWSIAM + { + session.cachedPassword = cachedPassword } } @@ -311,4 +319,107 @@ extension DatabaseManager { } } } + + internal func connectReconnectDriver( + for session: ConnectionSession, + effectiveConnection: DatabaseConnection, + passwordOverride initialPasswordOverride: String? + ) async throws -> (driver: DatabaseDriver, cachedPassword: String?)? { + var passwordOverride = initialPasswordOverride + + while true { + let driver = try await DatabaseDriverFactory.createDriver( + for: effectiveConnection, + passwordOverride: passwordOverride, + awaitPlugins: true + ) + + do { + try await driver.connect() + return (driver, passwordOverride) + } catch { + driver.disconnect() + + switch await reconnectCredentialResolution( + for: session, + error: error, + currentPassword: passwordOverride + ) { + case .retry(let newPassword): + passwordOverride = newPassword + case .abort: + return nil + case .fail: + await closeReconnectTunnels(for: session.connection) + throw error + } + } + } + } + + internal func reconnectCredentialResolution( + for session: ConnectionSession, + error: Error, + currentPassword: String?, + prompt: @escaping @MainActor (_ connectionName: String, _ isAPIToken: Bool, _ window: NSWindow?) async -> String? = PasswordPromptHelper.prompt + ) async -> ReconnectCredentialResolution { + guard session.connection.promptForPassword, + !pluginManager.hidesPassword(for: session.connection), + isAuthenticationFailure(error) + else { + return .fail + } + + let isApiOnly = pluginManager.connectionMode(for: session.connection.type) == .apiOnly + guard let prompted = await prompt( + session.connection.name, + isApiOnly, + NSApp.keyWindow + ) else { + return .abort + } + + if prompted == currentPassword { + return .fail + } + + return .retry(prompted) + } + + internal func isAuthenticationFailure(_ error: Error) -> Bool { + if let pluginError = error as? any PluginDriverError { + if pluginError.pluginSqlState == "28000" { + return true + } + if let code = pluginError.pluginErrorCode, code == 1045 { + return true + } + let message = pluginError.pluginErrorMessage.lowercased() + return message.contains("access denied") + || message.contains("authentication failed") + || message.contains("invalid credentials") + } + + let message = error.localizedDescription.lowercased() + return message.contains("access denied") + || message.contains("authentication failed") + || message.contains("invalid credentials") + } + + private func closeReconnectTunnels(for connection: DatabaseConnection) async { + if connection.resolvedSSHConfig.enabled { + do { + try await SSHTunnelManager.shared.closeTunnel(connectionId: connection.id) + } catch { + Self.logger.warning("Failed to close SSH tunnel during reconnect: \(error.localizedDescription)") + } + } + if connection.isCloudflareEnabled { + do { + try await CloudflareTunnelManager.shared.closeTunnel(connectionId: connection.id) + } catch { + Self.logger.warning("Failed to close Cloudflare tunnel during reconnect: \(error.localizedDescription)") + } + } + } } diff --git a/TablePro/Core/Database/DatabaseManager.swift b/TablePro/Core/Database/DatabaseManager.swift index 1af729e0f..09c6cba59 100644 --- a/TablePro/Core/Database/DatabaseManager.swift +++ b/TablePro/Core/Database/DatabaseManager.swift @@ -5,7 +5,6 @@ // Created by Ngo Quoc Dat on 16/12/25. // -import AppKit import Foundation import Observation import os diff --git a/TableProTests/Core/Database/ReconnectCredentialRecoveryTests.swift b/TableProTests/Core/Database/ReconnectCredentialRecoveryTests.swift new file mode 100644 index 000000000..92e4a001a --- /dev/null +++ b/TableProTests/Core/Database/ReconnectCredentialRecoveryTests.swift @@ -0,0 +1,68 @@ +import Testing +@testable import TablePro +import TableProPluginKit + +@Suite("Reconnect credential recovery", .serialized) +@MainActor +struct ReconnectCredentialRecoveryTests { + @Test("Authentication failures are detected from SQLSTATE") + func detectsAuthFailureFromSQLState() { + let error = FakePluginAuthError( + pluginErrorMessage: "Access denied", + pluginErrorCode: nil, + pluginSqlState: "28000" + ) + + #expect(DatabaseManager.shared.isAuthenticationFailure(error)) + } + + @Test("Prompt-for-password connections retry with fresh password") + func retriesWithFreshPassword() async { + let manager = DatabaseManager.shared + var connection = TestFixtures.makeConnection(name: "Prod") + connection.promptForPassword = true + let session = ConnectionSession(connection: connection) + let error = FakePluginAuthError( + pluginErrorMessage: "Access denied", + pluginErrorCode: 1045, + pluginSqlState: "28000" + ) + + let resolution = await manager.reconnectCredentialResolution( + for: session, + error: error, + currentPassword: "expired-secret", + prompt: { _, _, _ in "fresh-secret" } + ) + + #expect(resolution == .retry("fresh-secret")) + } + + @Test("Cancelling the prompt aborts reconnect recovery") + func abortsWhenPromptCancelled() async { + let manager = DatabaseManager.shared + var connection = TestFixtures.makeConnection(name: "Prod") + connection.promptForPassword = true + let session = ConnectionSession(connection: connection) + let error = FakePluginAuthError( + pluginErrorMessage: "Access denied", + pluginErrorCode: 1045, + pluginSqlState: "28000" + ) + + let resolution = await manager.reconnectCredentialResolution( + for: session, + error: error, + currentPassword: "expired-secret", + prompt: { _, _, _ in nil } + ) + + #expect(resolution == .abort) + } +} + +private struct FakePluginAuthError: PluginDriverError { + let pluginErrorMessage: String + let pluginErrorCode: Int? + let pluginSqlState: String? +} From c8b159571c85fa8846c65c7c777821e45afce148 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 10 Jul 2026 08:38:35 +0700 Subject: [PATCH 2/2] fix(connections): fix build errors and tunnel leak in reconnect password recovery --- .../Database/DatabaseManager+Health.swift | 50 ++++++++++++------- .../ReconnectCredentialRecoveryTests.swift | 49 ++++++++++++++++++ 2 files changed, 81 insertions(+), 18 deletions(-) diff --git a/TablePro/Core/Database/DatabaseManager+Health.swift b/TablePro/Core/Database/DatabaseManager+Health.swift index 73207981e..3b557cf14 100644 --- a/TablePro/Core/Database/DatabaseManager+Health.swift +++ b/TablePro/Core/Database/DatabaseManager+Health.swift @@ -60,15 +60,15 @@ extension DatabaseManager { await SchemaService.shared.invalidate(connectionId: connectionId) await DatabaseTreeMetadataService.shared.handleReconnect(connectionId: connectionId) do { - guard let result = try await self.trackOperation(sessionId: connectionId) { + guard let result = try await self.trackOperation(sessionId: connectionId, operation: { try await self.reconnectDriver(for: session) - } else { - self.updateSession(connectionId) { session in - session.status = .error(String(localized: "Reconnect cancelled")) + }) else { + await self.updateSession(connectionId) { session in + session.status = .disconnected } return .abort } - self.updateSession(connectionId) { session in + await self.updateSession(connectionId) { session in session.driver = result.driver session.effectiveConnection = result.effectiveConnection session.status = .connected @@ -84,6 +84,16 @@ extension DatabaseManager { return .success } catch { Self.logger.debug("Reconnect failed: \(error.localizedDescription)") + // Auth failures are not transient. Retrying with the same expired + // credential just re-prompts on every attempt, so stop the loop. + if await self.isAuthenticationFailure(error) { + await self.updateSession(connectionId) { session in + session.status = .error( + String(format: String(localized: "Reconnect failed: %@"), error.localizedDescription) + ) + } + return .abort + } return .retry } }, @@ -253,9 +263,9 @@ extension DatabaseManager { { let isApiOnly = pluginManager.connectionMode(for: session.connection.type) == .apiOnly guard let prompted = await PasswordPromptHelper.prompt( - session.connection.name, - isApiOnly, - NSApp.keyWindow + connectionName: session.connection.name, + isAPIToken: isApiOnly, + window: NSApp.keyWindow ) else { updateSession(sessionId) { $0.status = .disconnected } return @@ -348,6 +358,7 @@ extension DatabaseManager { case .retry(let newPassword): passwordOverride = newPassword case .abort: + await closeReconnectTunnels(for: session.connection) return nil case .fail: await closeReconnectTunnels(for: session.connection) @@ -386,24 +397,27 @@ extension DatabaseManager { return .retry(prompted) } + private static let invalidAuthorizationSQLState = "28000" + private static let mysqlAccessDeniedErrorCode = 1_045 + internal func isAuthenticationFailure(_ error: Error) -> Bool { if let pluginError = error as? any PluginDriverError { - if pluginError.pluginSqlState == "28000" { + if pluginError.pluginSqlState == Self.invalidAuthorizationSQLState { return true } - if let code = pluginError.pluginErrorCode, code == 1045 { + if pluginError.pluginErrorCode == Self.mysqlAccessDeniedErrorCode { return true } - let message = pluginError.pluginErrorMessage.lowercased() - return message.contains("access denied") - || message.contains("authentication failed") - || message.contains("invalid credentials") + return messageIndicatesAuthenticationFailure(pluginError.pluginErrorMessage) } + return messageIndicatesAuthenticationFailure(error.localizedDescription) + } - let message = error.localizedDescription.lowercased() - return message.contains("access denied") - || message.contains("authentication failed") - || message.contains("invalid credentials") + private func messageIndicatesAuthenticationFailure(_ message: String) -> Bool { + let lowered = message.lowercased() + return lowered.contains("access denied") + || lowered.contains("authentication failed") + || lowered.contains("invalid credentials") } private func closeReconnectTunnels(for connection: DatabaseConnection) async { diff --git a/TableProTests/Core/Database/ReconnectCredentialRecoveryTests.swift b/TableProTests/Core/Database/ReconnectCredentialRecoveryTests.swift index 92e4a001a..5a984ac20 100644 --- a/TableProTests/Core/Database/ReconnectCredentialRecoveryTests.swift +++ b/TableProTests/Core/Database/ReconnectCredentialRecoveryTests.swift @@ -59,6 +59,55 @@ struct ReconnectCredentialRecoveryTests { #expect(resolution == .abort) } + + @Test("Non-authentication errors fail without prompting") + func nonAuthErrorFailsWithoutPrompting() async { + let manager = DatabaseManager.shared + var connection = TestFixtures.makeConnection(name: "Prod") + connection.promptForPassword = true + let session = ConnectionSession(connection: connection) + let error = FakePluginAuthError( + pluginErrorMessage: "Lost connection to server", + pluginErrorCode: 2013, + pluginSqlState: "HY000" + ) + + var didPrompt = false + let resolution = await manager.reconnectCredentialResolution( + for: session, + error: error, + currentPassword: "cached-secret", + prompt: { _, _, _ in + didPrompt = true + return "fresh-secret" + } + ) + + #expect(resolution == .fail) + #expect(didPrompt == false) + } + + @Test("Re-entering the same password fails instead of looping") + func samePasswordFails() async { + let manager = DatabaseManager.shared + var connection = TestFixtures.makeConnection(name: "Prod") + connection.promptForPassword = true + let session = ConnectionSession(connection: connection) + let error = FakePluginAuthError( + pluginErrorMessage: "Access denied", + pluginErrorCode: 1045, + pluginSqlState: "28000" + ) + + let resolution = await manager.reconnectCredentialResolution( + for: session, + error: error, + currentPassword: "expired-secret", + prompt: { _, _, _ in "expired-secret" } + ) + + #expect(resolution == .fail) + } } private struct FakePluginAuthError: PluginDriverError {