From d6d338fa092f2ba0dba0b98019faf4690125c60d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 21 Oct 2026 04:28:24 +0700 Subject: [PATCH 1/2] feat(connections): show a saved password and test a password source --- .../ConnectionCredentialResolver.swift | 2 +- .../Connection/PasswordSourceResolver.swift | 13 +- .../Models/Connection/PasswordSource.swift | 5 + .../TablePro/Resources/Localizable.xcstrings | 15 ++ .../Connection/PasswordPromptToggle.swift | 47 ++++-- .../Views/Connection/PasswordSourceKind.swift | 78 +++++++++ .../Views/Connection/PasswordSourceTest.swift | 127 +++++++++++++++ .../Views/Connection/SecretReveal.swift | 74 +++++++++ .../Components/CredentialProfilePicker.swift | 14 +- .../ConnectionFormCoordinator.swift | 21 ++- .../Panes/GeneralPaneView.swift | 7 +- .../ViewModels/AuthPaneViewModel.swift | 29 +++- .../CredentialProfileEditorSheet.swift | 150 +++++++++--------- .../PasswordSourceResolverTests.swift | 30 ++++ .../PasswordSourceTestModelTests.swift | 140 ++++++++++++++++ .../Views/Connection/SecretRevealTests.swift | 45 ++++++ .../AuthPanePasswordSourceTests.swift | 112 +++++++++++++ 17 files changed, 792 insertions(+), 117 deletions(-) create mode 100644 apple/TablePro/Views/Connection/PasswordSourceKind.swift create mode 100644 apple/TablePro/Views/Connection/PasswordSourceTest.swift create mode 100644 apple/TablePro/Views/Connection/SecretReveal.swift create mode 100644 apple/TableProTests/Views/Connection/PasswordSourceTestModelTests.swift create mode 100644 apple/TableProTests/Views/Connection/SecretRevealTests.swift create mode 100644 apple/TableProTests/Views/ConnectionForm/ViewModels/AuthPanePasswordSourceTests.swift diff --git a/apple/TablePro/Core/Database/ConnectionCredentialResolver.swift b/apple/TablePro/Core/Database/ConnectionCredentialResolver.swift index de277efbec..a33fd6e3bc 100644 --- a/apple/TablePro/Core/Database/ConnectionCredentialResolver.swift +++ b/apple/TablePro/Core/Database/ConnectionCredentialResolver.swift @@ -104,7 +104,7 @@ enum ConnectionCredentialResolver { /// The profiles file carries its own tag for exactly this: a source can name a shell /// command, so the file has to be the one TablePro last wrote before one runs. guard CredentialProfileStorage.shared.storeIsTrusted else { - throw PasswordSourceResolver.ResolutionError.storeNotTrusted + throw PasswordSourceResolver.ResolutionError.profileStoreNotTrusted } return try await PasswordSourceResolver.resolve(source, for: ConnectionScriptContext(connection: connection)) } diff --git a/apple/TablePro/Core/Utilities/Connection/PasswordSourceResolver.swift b/apple/TablePro/Core/Utilities/Connection/PasswordSourceResolver.swift index 3be50e012f..341ec2e906 100644 --- a/apple/TablePro/Core/Utilities/Connection/PasswordSourceResolver.swift +++ b/apple/TablePro/Core/Utilities/Connection/PasswordSourceResolver.swift @@ -15,7 +15,7 @@ enum PasswordSourceResolver { private static let commandTimeoutSeconds: UInt64 = 30 private static let maxOutputBytes = 1_048_576 - enum ResolutionError: LocalizedError { + enum ResolutionError: LocalizedError, Equatable { case fileNotFound(path: String) case fileUnreadable(path: String) case environmentVariableNotSet(name: String) @@ -26,6 +26,7 @@ enum PasswordSourceResolver { case invalidSecretJson case jsonKeyNotFound(key: String) case storeNotTrusted + case profileStoreNotTrusted var errorDescription: String? { switch self { @@ -64,10 +65,20 @@ enum PasswordSourceResolver { password source was not run. Open the connection and save it again to \ confirm the change. """) + case .profileStoreNotTrusted: + return String(localized: """ + Your credential profiles file was changed outside TablePro, so this profile's \ + password source was not run. + """) } } } + // A test reports what the source returns right now, so it neither reads nor fills the reuse cache. + static func test(_ source: PasswordSource, for context: ConnectionScriptContext) async throws -> String { + try await resolve(source.withoutReuse, for: context) + } + static func resolve( _ source: PasswordSource, for context: ConnectionScriptContext? = nil, diff --git a/apple/TablePro/Models/Connection/PasswordSource.swift b/apple/TablePro/Models/Connection/PasswordSource.swift index 0c593b9599..8a22bbb928 100644 --- a/apple/TablePro/Models/Connection/PasswordSource.swift +++ b/apple/TablePro/Models/Connection/PasswordSource.swift @@ -91,6 +91,11 @@ enum PasswordSource: Codable, Hashable, Sendable { value.flatMap { reuseSecondsRange.contains($0) ? $0 : nil } } + var withoutReuse: PasswordSource { + guard case .command(let shell, _) = self else { return self } + return .command(shell: shell) + } + /// Decodes a password source from a connection container, treating a present-but-malformed /// entry as absent so one bad connection cannot fail loading of the whole store. static func resilientlyDecoded( diff --git a/apple/TablePro/Resources/Localizable.xcstrings b/apple/TablePro/Resources/Localizable.xcstrings index 383fdb7a32..145e5ffe3b 100644 --- a/apple/TablePro/Resources/Localizable.xcstrings +++ b/apple/TablePro/Resources/Localizable.xcstrings @@ -16692,6 +16692,9 @@ } } } + }, + "A test here has no connection to take the host, port and database from." : { + }, "A text index cannot be unique." : { "extractionState" : "manual", @@ -156880,6 +156883,9 @@ } } } + }, + "show a saved password" : { + }, "Secret Access Key" : { "extractionState" : "manual", @@ -160199,6 +160205,9 @@ } } } + }, + "Set in connections.json." : { + }, "Set Cells to Value" : { "localizations" : { @@ -179014,6 +179023,9 @@ } } } + }, + "Test Source" : { + }, "Test the current connection settings" : { "localizations" : { @@ -211677,6 +211689,9 @@ } } } + }, + "Your credential profiles file was changed outside TablePro, so this profile's password source was not run." : { + }, "Your database schema and query data will be sent to the AI provider for analysis. Allow for this connection?" : { "localizations" : { diff --git a/apple/TablePro/Views/Connection/PasswordPromptToggle.swift b/apple/TablePro/Views/Connection/PasswordPromptToggle.swift index 0f9e77b667..e93cdc358a 100644 --- a/apple/TablePro/Views/Connection/PasswordPromptToggle.swift +++ b/apple/TablePro/Views/Connection/PasswordPromptToggle.swift @@ -2,40 +2,53 @@ // PasswordPromptToggle.swift // TablePro // -// Toggle + conditional SecureField for the "ask for password on every connection" option. -// import SwiftUI import TableProPluginKit struct PasswordPromptToggle: View { - let type: DatabaseType - @Binding var promptForPassword: Bool - @Binding var password: String - @Binding var additionalFieldValues: [String: String] + @ObservedObject var coordinator: ConnectionFormCoordinator + + private var auth: AuthPaneViewModel { coordinator.auth } private var isApiOnly: Bool { - PluginManager.shared.connectionMode(for: type) == .apiOnly + PluginManager.shared.connectionMode(for: coordinator.network.type) == .apiOnly + } + + private var passwordTitle: String { + isApiOnly ? String(localized: "API Token") : String(localized: "Password") } var body: some View { - if !promptForPassword { - SecureField( - isApiOnly ? String(localized: "API Token") : String(localized: "Password"), - text: $password - ) + if !auth.promptForPassword { + if let source = auth.passwordSource { + PasswordSourceSummary(title: passwordTitle, source: source) + PasswordSourceTestView( + source: source, + context: { coordinator.passwordSourceContext() }, + refusal: { + coordinator.storage.storeIsTrusted ? nil : PasswordSourceResolver.ResolutionError.storeNotTrusted + } + ) + } else { + RevealablePasswordField( + title: passwordTitle, + text: $coordinator.auth.password, + requiresAuthentication: auth.storedPasswordState != .absent + ) + } } Toggle( isApiOnly ? String(localized: "Prompt for API token") : String(localized: "Prompt for password"), - isOn: $promptForPassword + isOn: $coordinator.auth.promptForPassword ) - .onChange(of: promptForPassword) { newValue in + .onChange(of: auth.promptForPassword) { newValue in if newValue { - password = "" - if additionalFieldValues["usePgpass"] == "true" { - additionalFieldValues["usePgpass"] = "" + coordinator.auth.password = "" + if coordinator.auth.additionalFieldValues["usePgpass"] == "true" { + coordinator.auth.additionalFieldValues["usePgpass"] = "" } } } diff --git a/apple/TablePro/Views/Connection/PasswordSourceKind.swift b/apple/TablePro/Views/Connection/PasswordSourceKind.swift new file mode 100644 index 0000000000..16306b775e --- /dev/null +++ b/apple/TablePro/Views/Connection/PasswordSourceKind.swift @@ -0,0 +1,78 @@ +// +// PasswordSourceKind.swift +// TablePro +// + +import Foundation + +enum PasswordSourceKind: String, CaseIterable, Identifiable { + case file, env, command, onePassword, vault, awsSecretsManager + + var id: String { rawValue } + + var displayName: String { + switch self { + case .file: String(localized: "A file") + case .env: String(localized: "An environment variable") + case .command: String(localized: "The output of a command") + case .onePassword: "1Password" + case .vault: "HashiCorp Vault" + case .awsSecretsManager: "AWS Secrets Manager" + } + } + + var valueLabel: String { + switch self { + case .file: String(localized: "Path") + case .env: String(localized: "Variable") + case .command: String(localized: "Command") + case .onePassword: String(localized: "Reference") + case .vault: String(localized: "Path") + case .awsSecretsManager: String(localized: "Secret ID") + } + } + + var fieldLabel: String? { + switch self { + case .vault: String(localized: "Field") + case .awsSecretsManager: String(localized: "JSON Key") + case .file, .env, .command, .onePassword: nil + } + } + + var placeholder: String { + switch self { + case .file: "~/.secrets/db-password" + case .env: "DB_PASSWORD" + case .command: "security find-generic-password -w -s db" + case .onePassword: "op://vault/item/password" + case .vault: "secret/data/db" + case .awsSecretsManager: "prod/db/credentials" + } + } + + func source(value: String, field: String, reuseSeconds: Int?) -> PasswordSource { + let trimmedValue = value.trimmingCharacters(in: .whitespaces) + let trimmedField = field.trimmingCharacters(in: .whitespaces) + switch self { + case .file: return .file(path: trimmedValue) + case .env: return .env(variable: trimmedValue) + case .command: return .command(shell: value, reuseSeconds: reuseSeconds) + case .onePassword: return .onePassword(reference: trimmedValue) + case .vault: return .vault(path: trimmedValue, field: trimmedField) + case .awsSecretsManager: + return .awsSecretsManager(secretId: trimmedValue, jsonKey: trimmedField.isEmpty ? nil : trimmedField) + } + } + + static func decompose(_ source: PasswordSource) -> (kind: PasswordSourceKind, value: String, field: String) { + switch source { + case .file(let path): (.file, path, "") + case .env(let variable): (.env, variable, "") + case .command(let shell, _): (.command, shell, "") + case .onePassword(let reference): (.onePassword, reference, "") + case .vault(let path, let field): (.vault, path, field) + case .awsSecretsManager(let secretId, let jsonKey): (.awsSecretsManager, secretId, jsonKey ?? "") + } + } +} diff --git a/apple/TablePro/Views/Connection/PasswordSourceTest.swift b/apple/TablePro/Views/Connection/PasswordSourceTest.swift new file mode 100644 index 0000000000..a7e13271ef --- /dev/null +++ b/apple/TablePro/Views/Connection/PasswordSourceTest.swift @@ -0,0 +1,127 @@ +// +// PasswordSourceTest.swift +// TablePro +// + +import Combine +import SwiftUI + +@MainActor +final class PasswordSourceTestModel: ObservableObject { + enum Outcome: Equatable { + case returned(String) + case failed(String) + } + + @Published private(set) var isRunning = false + @Published private var latest: (source: PasswordSource, environment: [String: String], outcome: Outcome)? + + private let resolve: @Sendable (PasswordSource, ConnectionScriptContext) async throws -> String + + init( + resolve: @escaping @Sendable (PasswordSource, ConnectionScriptContext) async throws -> String = { source, context in + try await PasswordSourceResolver.test(source, for: context) + } + ) { + self.resolve = resolve + } + + // A result describes the source and connection details it ran with; after an edit to either it is about something else. + func outcome(for source: PasswordSource, context: @autoclosure () -> ConnectionScriptContext) -> Outcome? { + guard let latest, latest.source == source, latest.environment == context().environment else { return nil } + return latest.outcome + } + + func run(_ source: PasswordSource, context: ConnectionScriptContext, refusal: (any Error)?) async { + guard !isRunning else { return } + if let refusal { + latest = (source, context.environment, .failed(refusal.localizedDescription)) + return + } + isRunning = true + defer { isRunning = false } + do { + let value = try await resolve(source, context) + latest = (source, context.environment, .returned(value)) + } catch { + latest = (source, context.environment, .failed(error.localizedDescription)) + } + } +} + +struct PasswordSourceTestView: View { + let source: PasswordSource + let context: () -> ConnectionScriptContext + let refusal: () -> (any Error)? + + @StateObject private var model = PasswordSourceTestModel() + @StateObject private var reveal = SecretReveal() + + var body: some View { + HStack { + Button(String(localized: "Test Source")) { + let context = context() + let refusal = refusal() + Task { await model.run(source, context: context, refusal: refusal) } + } + .disabled(model.isRunning) + .accessibilityIdentifier("password-source-test") + if model.isRunning { + ProgressView() + .controlSize(.small) + } + } + switch model.outcome(for: source, context: context()) { + case .returned(let value): + LabeledContent(String(localized: "Result")) { + if reveal.isRevealed { + Text(value) + .font(.body.monospaced()) + .textSelection(.enabled) + .lineLimit(4) + } else { + Text(Self.masked(value)) + .accessibilityLabel(String(localized: "Hidden")) + } + } + SecretRevealToggle(reveal: reveal, requiresAuthentication: true) + case .failed(let message): + Label(message, systemImage: "exclamationmark.triangle.fill") + .font(.caption) + .foregroundStyle(.red) + .textSelection(.enabled) + case nil: + EmptyView() + } + } + + static func masked(_ value: String) -> String { + String(repeating: "\u{2022}", count: min(value.count, 24)) + } +} + +struct PasswordSourceSummary: View { + let title: String + let source: PasswordSource + + private var parts: (kind: PasswordSourceKind, value: String, field: String) { + PasswordSourceKind.decompose(source) + } + + var body: some View { + LabeledContent(title, value: parts.kind.displayName) + LabeledContent(parts.kind.valueLabel) { + Text(parts.value) + .font(.body.monospaced()) + .textSelection(.enabled) + .lineLimit(3) + .truncationMode(.middle) + } + if let fieldLabel = parts.kind.fieldLabel, !parts.field.isEmpty { + LabeledContent(fieldLabel, value: parts.field) + } + Text("Set in connections.json.") + .font(.caption) + .foregroundStyle(.secondary) + } +} diff --git a/apple/TablePro/Views/Connection/SecretReveal.swift b/apple/TablePro/Views/Connection/SecretReveal.swift new file mode 100644 index 0000000000..ea1f9dd7b0 --- /dev/null +++ b/apple/TablePro/Views/Connection/SecretReveal.swift @@ -0,0 +1,74 @@ +// +// SecretReveal.swift +// TablePro +// + +import Combine +import SwiftUI + +@MainActor +final class SecretReveal: ObservableObject { + @Published private(set) var isRevealed = false + + private let authenticator: any OperationAuthenticating + private var hasAuthenticated = false + private var isAuthenticating = false + + // LAContext shows this as "TablePro is trying to .", so it stays a lowercase phrase with no period. + static var authenticationReason: String { + String(localized: "show a saved password") + } + + init(authenticator: any OperationAuthenticating = BiometricOperationAuthenticating()) { + self.authenticator = authenticator + } + + func setRevealed(_ reveal: Bool, requiresAuthentication: Bool) async { + guard reveal else { + isRevealed = false + return + } + guard !isRevealed, !isAuthenticating else { return } + if requiresAuthentication, !hasAuthenticated { + isAuthenticating = true + let granted = await authenticator.authenticate(reason: Self.authenticationReason) + isAuthenticating = false + guard granted else { return } + hasAuthenticated = true + } + isRevealed = true + } +} + +struct SecretRevealToggle: View { + @ObservedObject var reveal: SecretReveal + let requiresAuthentication: Bool + + var body: some View { + Toggle(String(localized: "Show password"), isOn: Binding( + get: { reveal.isRevealed }, + set: { newValue in + Task { await reveal.setRevealed(newValue, requiresAuthentication: requiresAuthentication) } + } + )) + } +} + +// NSSecureTextField has no reveal of its own, so revealing swaps in a plain field, as the system's password fields do. +struct RevealablePasswordField: View { + let title: String + @Binding var text: String + let requiresAuthentication: Bool + + @StateObject private var reveal = SecretReveal() + + var body: some View { + if reveal.isRevealed { + TextField(title, text: $text) + .autocorrectionDisabled() + } else { + SecureField(title, text: $text) + } + SecretRevealToggle(reveal: reveal, requiresAuthentication: requiresAuthentication) + } +} diff --git a/apple/TablePro/Views/ConnectionForm/Components/CredentialProfilePicker.swift b/apple/TablePro/Views/ConnectionForm/Components/CredentialProfilePicker.swift index 85f26fa138..0c3842e74a 100644 --- a/apple/TablePro/Views/ConnectionForm/Components/CredentialProfilePicker.swift +++ b/apple/TablePro/Views/ConnectionForm/Components/CredentialProfilePicker.swift @@ -42,6 +42,16 @@ struct CredentialProfilePicker: View { } else if let profile = auth.selectedCredentialProfile { LabeledContent(String(localized: "Username"), value: displayUsername(profile)) LabeledContent(String(localized: "Password"), value: profile.passwordMode.displayName) + if case .source(let source) = profile.passwordMode, let coordinator = auth.coordinator?.value { + PasswordSourceTestView( + source: source, + context: { coordinator.passwordSourceContext() }, + refusal: { + CredentialProfileStorage.shared.storeIsTrusted + ? nil : PasswordSourceResolver.ResolutionError.profileStoreNotTrusted + } + ) + } } HStack(spacing: 12) { @@ -66,7 +76,9 @@ struct CredentialProfilePicker: View { } .sheet(isPresented: $auth.isSavingCredentialsAsProfile) { CredentialProfileEditorSheet( - existingProfile: auth.profileFromCurrentCredentials(), + existingProfile: auth.profileFromCurrentCredentials( + sourceStoreIsTrusted: auth.coordinator?.value?.storage.storeIsTrusted ?? false + ), initialPassword: auth.password, initialDatabaseType: auth.coordinator?.value?.network.type, onSave: { saved in diff --git a/apple/TablePro/Views/ConnectionForm/ConnectionFormCoordinator.swift b/apple/TablePro/Views/ConnectionForm/ConnectionFormCoordinator.swift index dd9c2d1f27..72b775adf6 100644 --- a/apple/TablePro/Views/ConnectionForm/ConnectionFormCoordinator.swift +++ b/apple/TablePro/Views/ConnectionForm/ConnectionFormCoordinator.swift @@ -607,19 +607,26 @@ final class ConnectionFormCoordinator: ObservableObject { } } + // What a password source sees for the values in the form, saved or not. + func passwordSourceContext() -> ConnectionScriptContext { + ConnectionScriptContext(connection: buildEdits().applied(to: DatabaseConnection(id: connectionId ?? UUID(), name: ""))) + } + + // The edits leave out the prompt flag and the source, which a save carries over. A test without them signs in differently from Connect. + func connectionForTest() -> DatabaseConnection { + var connection = buildEdits().applied(to: DatabaseConnection(id: UUID(), name: "")) + connection.promptForPassword = auth.effectivePromptForPassword + connection.passwordSource = auth.passwordSourceInUse + return connection + } + func test() { guard testTask == nil else { return } isTesting = true testResult = nil let window = NSApp.keyWindow - var testConn = buildEdits().applied(to: DatabaseConnection(id: UUID(), name: "")) - /// `credentialMode` rides along from the edits, so the resolver finds the linked profile - /// under its own id rather than the throwaway connection's. The connection's own password - /// source is only resurrected when nothing else supplies a password. - testConn.passwordSource = auth.password.isEmpty && !auth.usesCredentialProfile - ? originalConnection?.passwordSource - : nil + let testConn = connectionForTest() temporaryTestIds.insert(testConn.id) let password = auth.password diff --git a/apple/TablePro/Views/ConnectionForm/Panes/GeneralPaneView.swift b/apple/TablePro/Views/ConnectionForm/Panes/GeneralPaneView.swift index 2f1957f2c4..3a758502d7 100644 --- a/apple/TablePro/Views/ConnectionForm/Panes/GeneralPaneView.swift +++ b/apple/TablePro/Views/ConnectionForm/Panes/GeneralPaneView.swift @@ -249,12 +249,7 @@ struct GeneralPaneView: View { authFieldRow(field) } if !coordinator.auth.hidesPassword { - PasswordPromptToggle( - type: type, - promptForPassword: $coordinator.auth.promptForPassword, - password: $coordinator.auth.password, - additionalFieldValues: $coordinator.auth.additionalFieldValues - ) + PasswordPromptToggle(coordinator: coordinator) } ForEach(authFields.rest, id: \.id) { field in authFieldRow(field) diff --git a/apple/TablePro/Views/ConnectionForm/ViewModels/AuthPaneViewModel.swift b/apple/TablePro/Views/ConnectionForm/ViewModels/AuthPaneViewModel.swift index 57bac571d8..c222b40473 100644 --- a/apple/TablePro/Views/ConnectionForm/ViewModels/AuthPaneViewModel.swift +++ b/apple/TablePro/Views/ConnectionForm/ViewModels/AuthPaneViewModel.swift @@ -42,6 +42,9 @@ final class AuthPaneViewModel: ObservableObject { /// it only if there was something to clear and the read succeeded. @Published private(set) var storedPasswordState: ConnectionStorage.StoredSecretState = .absent + // Lives only in connections.json: the form shows and tests it, never edits it. + @Published private(set) var passwordSource: PasswordSource? + @Published var coordinator: WeakCoordinatorRef? var authFields: [ConnectionField] { @@ -90,10 +93,15 @@ final class AuthPaneViewModel: ObservableObject { var usesCredentialProfile: Bool { credentialMode.profileId != nil } + // A prompt arrives as an override and a linked profile is resolved first, so either one leaves the source unused. + var passwordSourceInUse: PasswordSource? { + usesCredentialProfile || effectivePromptForPassword ? nil : passwordSource + } + /// Enough to make a profile out of. A password-only engine such as Redis has no username, so /// requiring one hid the promotion exactly where it was most useful. var hasPromotableCredentials: Bool { - !resolvedUsername.isEmpty || !password.isEmpty || effectivePromptForPassword + !resolvedUsername.isEmpty || !password.isEmpty || effectivePromptForPassword || passwordSource != nil } var selectedCredentialProfile: CredentialProfile? { @@ -127,21 +135,31 @@ final class AuthPaneViewModel: ObservableObject { if let refreshed = coordinator?.value?.storage.loadConnection(id: coordinator?.value?.connectionId ?? UUID()) { username = refreshed.username promptForPassword = refreshed.promptForPassword + passwordSource = refreshed.passwordSource storedPasswordState = coordinator?.value?.storage.passwordState(for: refreshed.id) ?? .absent password = coordinator?.value?.storage.loadPassword(for: refreshed.id) ?? "" } } - /// The credentials currently typed into the form, as an unsaved profile for the editor to open - /// with. Promoting what is already filled in beats making the user retype it. - func profileFromCurrentCredentials() -> CredentialProfile { + // Promoting what is already filled in beats making the user retype it. + func profileFromCurrentCredentials(sourceStoreIsTrusted: Bool) -> CredentialProfile { CredentialProfile( name: "", username: resolvedUsername, - passwordMode: effectivePromptForPassword ? .prompt : .stored + passwordMode: promotedPasswordMode(sourceStoreIsTrusted: sourceStoreIsTrusted) ) } + // Same order as a connect resolves them. A source leaves only a file TablePro last wrote, as in `unlinkConnections`. + private func promotedPasswordMode(sourceStoreIsTrusted: Bool) -> CredentialPasswordMode { + if effectivePromptForPassword { return .prompt } + if let passwordSource { + return sourceStoreIsTrusted ? .source(passwordSource) : .prompt + } + if usePgpass { return .pgpass } + return .stored + } + var usePgpass: Bool { additionalFieldValues["usePgpass"] == "true" } @@ -186,6 +204,7 @@ final class AuthPaneViewModel: ObservableObject { func load(from connection: DatabaseConnection, storage: ConnectionStorage) { username = connection.username promptForPassword = connection.promptForPassword + passwordSource = connection.passwordSource credentialMode = connection.credentialMode loadCredentialProfiles() diff --git a/apple/TablePro/Views/Settings/Profiles/CredentialProfileEditorSheet.swift b/apple/TablePro/Views/Settings/Profiles/CredentialProfileEditorSheet.swift index 93652e0386..89f31cc8ee 100644 --- a/apple/TablePro/Views/Settings/Profiles/CredentialProfileEditorSheet.swift +++ b/apple/TablePro/Views/Settings/Profiles/CredentialProfileEditorSheet.swift @@ -23,7 +23,7 @@ struct CredentialProfileEditorSheet: View { @State private var username: String = "" @State private var passwordKind: PasswordKind = .stored @State private var password: String = "" - @State private var revealsPassword = false + @State private var passwordWasPrefilled = false @State private var sourceKind: PasswordSourceKind = .file @State private var sourceValue: String = "" @State private var sourceField: String = "" @@ -121,14 +121,11 @@ struct CredentialProfileEditorSheet: View { switch passwordKind { case .stored: - /// Revealing swaps the control rather than unmasking one: `NSSecureTextField` has - /// no reveal of its own, and this is how the system's own password fields do it. - if revealsPassword { - TextField(String(localized: "Password"), text: $password) - } else { - SecureField(String(localized: "Password"), text: $password) - } - Toggle(String(localized: "Show password"), isOn: $revealsPassword) + RevealablePasswordField( + title: String(localized: "Password"), + text: $password, + requiresAuthentication: passwordWasPrefilled + ) case .prompt: Text("Every connection using this profile asks for the password once each time TablePro runs.") .font(.caption) @@ -144,11 +141,8 @@ struct CredentialProfileEditorSheet: View { } } TextField(sourceKind.valueLabel, text: $sourceValue, prompt: Text(sourceKind.placeholder)) - if sourceKind == .vault { - TextField(String(localized: "Field"), text: $sourceField, prompt: Text("password")) - } - if sourceKind == .awsSecretsManager { - TextField(String(localized: "JSON Key"), text: $sourceField, prompt: Text("password")) + if let fieldLabel = sourceKind.fieldLabel { + TextField(fieldLabel, text: $sourceField, prompt: Text("password")) } if sourceKind == .command { Picker(String(localized: "Reuse Result For"), selection: $commandReuseSeconds) { @@ -169,10 +163,71 @@ struct CredentialProfileEditorSheet: View { Text("It stays on this Mac and never syncs, because it can name a command to run.") .font(.caption) .foregroundStyle(.secondary) + if let testedSource { + PasswordSourceTestView( + source: testedSource, + context: { testContext }, + refusal: { + Self.testRefusal( + for: testedSource, + storedSource: storedSource, + storeIsTrusted: storage.storeIsTrusted + ) + } + ) + if sourceKind == .command, mentionsConnectionDetails { + Text("A test here has no connection to take the host, port and database from.") + .font(.caption) + .foregroundStyle(.secondary) + } + } } } } + // MARK: - Test Source + + private var testedSource: PasswordSource? { + guard !sourceValue.trimmingCharacters(in: .whitespaces).isEmpty else { return nil } + return sourceKind.source(value: sourceValue, field: sourceField, reuseSeconds: nil) + } + + private var storedSource: PasswordSource? { + guard isStored, case .source(let source) = existingProfile?.passwordMode else { return nil } + return source + } + + private var testContext: ConnectionScriptContext { + ConnectionScriptContext( + connectionId: existingProfile?.id ?? UUID(), + connectionName: "", + host: "", + port: 0, + user: username.trimmingCharacters(in: .whitespaces), + database: "" + ) + } + + private var mentionsConnectionDetails: Bool { + sourceValue.contains("TABLEPRO_") || testContext.substitutingPlaceholders(in: sourceValue) != sourceValue + } + + // A source typed here is the user's own; one read back unchanged from a profiles file something else wrote is not. + static func testRefusal( + for source: PasswordSource, + storedSource: PasswordSource?, + storeIsTrusted: Bool + ) -> PasswordSourceResolver.ResolutionError? { + guard !storeIsTrusted, let storedSource, source == normalizedLikeTheFields(storedSource) else { return nil } + return .profileStoreNotTrusted + } + + // The fields trim what they hold, so the stored value is compared the same way or padding alone would pass for an edit. + private static func normalizedLikeTheFields(_ source: PasswordSource) -> PasswordSource { + let (kind, value, field) = PasswordSourceKind.decompose(source) + return kind.source(value: value, field: field, reuseSeconds: nil) + } + // MARK: - Secure fields @ViewBuilder @@ -289,13 +344,14 @@ struct CredentialProfileEditorSheet: View { } else if isStored { secretsWereReadable = false } + passwordWasPrefilled = !password.isEmpty case .prompt: passwordKind = .prompt case .pgpass: passwordKind = .pgpass case .source(let source): passwordKind = .source - (sourceKind, sourceValue, sourceField) = Self.decompose(source) + (sourceKind, sourceValue, sourceField) = PasswordSourceKind.decompose(source) if case .command(_, let reuseSeconds) = source { commandReuseSeconds = reuseSeconds } @@ -380,17 +436,6 @@ struct CredentialProfileEditorSheet: View { onDelete?() dismiss() } - - private static func decompose(_ source: PasswordSource) -> (PasswordSourceKind, String, String) { - switch source { - case .file(let path): (.file, path, "") - case .env(let variable): (.env, variable, "") - case .command(let shell, _): (.command, shell, "") - case .onePassword(let reference): (.onePassword, reference, "") - case .vault(let path, let field): (.vault, path, field) - case .awsSecretsManager(let secretId, let jsonKey): (.awsSecretsManager, secretId, jsonKey ?? "") - } - } } // MARK: - Kinds @@ -410,57 +455,4 @@ extension CredentialProfileEditorSheet { } } } - - enum PasswordSourceKind: String, CaseIterable, Identifiable { - case file, env, command, onePassword, vault, awsSecretsManager - - var id: String { rawValue } - - var displayName: String { - switch self { - case .file: String(localized: "A file") - case .env: String(localized: "An environment variable") - case .command: String(localized: "The output of a command") - case .onePassword: "1Password" - case .vault: "HashiCorp Vault" - case .awsSecretsManager: "AWS Secrets Manager" - } - } - - var valueLabel: String { - switch self { - case .file: String(localized: "Path") - case .env: String(localized: "Variable") - case .command: String(localized: "Command") - case .onePassword: String(localized: "Reference") - case .vault: String(localized: "Path") - case .awsSecretsManager: String(localized: "Secret ID") - } - } - - var placeholder: String { - switch self { - case .file: "~/.secrets/db-password" - case .env: "DB_PASSWORD" - case .command: "security find-generic-password -w -s db" - case .onePassword: "op://vault/item/password" - case .vault: "secret/data/db" - case .awsSecretsManager: "prod/db/credentials" - } - } - - func source(value: String, field: String, reuseSeconds: Int?) -> PasswordSource { - let trimmedValue = value.trimmingCharacters(in: .whitespaces) - let trimmedField = field.trimmingCharacters(in: .whitespaces) - switch self { - case .file: return .file(path: trimmedValue) - case .env: return .env(variable: trimmedValue) - case .command: return .command(shell: value, reuseSeconds: reuseSeconds) - case .onePassword: return .onePassword(reference: trimmedValue) - case .vault: return .vault(path: trimmedValue, field: trimmedField) - case .awsSecretsManager: - return .awsSecretsManager(secretId: trimmedValue, jsonKey: trimmedField.isEmpty ? nil : trimmedField) - } - } - } } diff --git a/apple/TableProTests/Core/Utilities/PasswordSourceResolverTests.swift b/apple/TableProTests/Core/Utilities/PasswordSourceResolverTests.swift index 6869db9388..9cade99f64 100644 --- a/apple/TableProTests/Core/Utilities/PasswordSourceResolverTests.swift +++ b/apple/TableProTests/Core/Utilities/PasswordSourceResolverTests.swift @@ -244,6 +244,36 @@ struct PasswordSourceResolverTests { PasswordCommandCache.shared.forget(connectionId: connection.id) } + @Test("A test runs the command now, whatever output a connect is reusing") + func testIgnoresTheReuseCache() async throws { + let counter = FileManager.default.temporaryDirectory.appendingPathComponent("tablepro-runs-\(UUID().uuidString)") + defer { try? FileManager.default.removeItem(at: counter) } + let cache = PasswordCommandCache() + let context = ConnectionScriptContext( + connectionId: UUID(), connectionName: "c", host: "h", port: 1, user: "u", database: "d" + ) + let source = PasswordSource.command( + shell: "echo run >> '\(counter.path)'; wc -l < '\(counter.path)' | tr -d ' '", + reuseSeconds: 600 + ) + + #expect(try await PasswordSourceResolver.resolve(source, for: context, cache: cache) == "1") + #expect(try await PasswordSourceResolver.test(source, for: context) == "2") + #expect(try await PasswordSourceResolver.test(source, for: context) == "3") + #expect(try await PasswordSourceResolver.resolve(source, for: context, cache: cache) == "1") + } + + @Test("A test fills in the connection's details like a connect does") + func testFillsConnectionDetails() async throws { + let context = ConnectionScriptContext( + connectionId: UUID(), connectionName: "c", host: "db.example.com", port: 5432, user: "u", database: "d" + ) + + let output = try await PasswordSourceResolver.test(.command(shell: "echo {host}:$TABLEPRO_PORT"), for: context) + + #expect(output == "db.example.com:5432") + } + @Test("Without a lifetime the command runs on every connect") func commandWithoutLifetimeRunsEachTime() async throws { let counter = FileManager.default.temporaryDirectory.appendingPathComponent("tablepro-runs-\(UUID().uuidString)") diff --git a/apple/TableProTests/Views/Connection/PasswordSourceTestModelTests.swift b/apple/TableProTests/Views/Connection/PasswordSourceTestModelTests.swift new file mode 100644 index 0000000000..a99dbec0aa --- /dev/null +++ b/apple/TableProTests/Views/Connection/PasswordSourceTestModelTests.swift @@ -0,0 +1,140 @@ +import Foundation +@testable import TablePro +import Testing + +@MainActor +struct PasswordSourceTestModelTests { + private let context = ConnectionScriptContext( + connectionId: UUID(), connectionName: "c", host: "h", port: 1, user: "u", database: "d" + ) + + private final class RunCounter: @unchecked Sendable { + private let lock = NSLock() + private var count = 0 + + var value: Int { lock.withLock { count } } + + func increment() { + lock.withLock { count += 1 } + } + } + + @Test("A refused test reports why and never runs the source") + func refusalSkipsTheRun() async { + let runs = RunCounter() + let model = PasswordSourceTestModel { _, _ in + runs.increment() + return "secret" + } + let source = PasswordSource.command(shell: "echo secret") + + await model.run(source, context: context, refusal: PasswordSourceResolver.ResolutionError.storeNotTrusted) + + #expect(runs.value == 0) + #expect(model.outcome(for: source, context: context) == .failed( + PasswordSourceResolver.ResolutionError.storeNotTrusted.localizedDescription + )) + } + + @Test("A command that prints a password returns it") + func commandOutputIsReturned() async { + let model = PasswordSourceTestModel() + let source = PasswordSource.command(shell: "echo hunter2") + + await model.run(source, context: context, refusal: nil) + + #expect(model.outcome(for: source, context: context) == .returned("hunter2")) + #expect(!model.isRunning) + } + + @Test("A command that fails shows its exit code and stderr") + func failingCommandShowsStderr() async { + let model = PasswordSourceTestModel() + let source = PasswordSource.command(shell: "echo 'op: not signed in' >&2; exit 3") + + await model.run(source, context: context, refusal: nil) + + guard case .failed(let message) = model.outcome(for: source, context: context) else { + Issue.record("Expected a failure") + return + } + #expect(message.contains("op: not signed in")) + #expect(message.contains("3")) + } + + @Test("A result is shown only for the source that produced it") + func outcomeBelongsToItsSource() async { + let model = PasswordSourceTestModel { _, _ in "secret" } + let tested = PasswordSource.env(variable: "A") + + await model.run(tested, context: context, refusal: nil) + + #expect(model.outcome(for: tested, context: context) == .returned("secret")) + #expect(model.outcome(for: .env(variable: "B"), context: context) == nil) + } + + @Test("Editing the connection's details hides a result that used the old ones") + func outcomeBelongsToItsConnectionDetails() async { + let model = PasswordSourceTestModel { _, context in context.host } + let source = PasswordSource.command(shell: "echo {host}") + let moved = ConnectionScriptContext( + connectionId: context.connectionId, connectionName: "c", host: "other", port: 1, user: "u", database: "d" + ) + + await model.run(source, context: context, refusal: nil) + + #expect(model.outcome(for: source, context: context) == .returned("h")) + #expect(model.outcome(for: source, context: moved) == nil) + } + + @Test("A new connection's details match from one render to the next") + func outcomeIgnoresTheThrowawayId() async { + let model = PasswordSourceTestModel { _, _ in "secret" } + let source = PasswordSource.env(variable: "A") + let rerendered = ConnectionScriptContext( + connectionId: UUID(), connectionName: "c", host: "h", port: 1, user: "u", database: "d" + ) + + await model.run(source, context: context, refusal: nil) + + #expect(model.outcome(for: source, context: rerendered) == .returned("secret")) + } + + @Test("The profile editor runs a source the user typed, even from an untrusted profiles file") + func editorRunsTypedSource() { + let stored = PasswordSource.command(shell: "echo stored") + + #expect(CredentialProfileEditorSheet.testRefusal( + for: .command(shell: "echo typed"), storedSource: stored, storeIsTrusted: false + ) == nil) + #expect(CredentialProfileEditorSheet.testRefusal( + for: .command(shell: "echo new"), storedSource: nil, storeIsTrusted: false + ) == nil) + } + + @Test("The profile editor refuses a source read back from an untrusted profiles file") + func editorRefusesUntrustedStoredSource() { + let stored = PasswordSource.command(shell: "echo stored") + + #expect(CredentialProfileEditorSheet.testRefusal( + for: stored, storedSource: stored, storeIsTrusted: false + ) == .profileStoreNotTrusted) + #expect(CredentialProfileEditorSheet.testRefusal( + for: stored, storedSource: stored, storeIsTrusted: true + ) == nil) + } + + @Test("Whitespace the fields trim away does not count as an edit") + func editorRefusesAPaddedStoredSource() { + #expect(CredentialProfileEditorSheet.testRefusal( + for: .file(path: "/tmp/secret"), + storedSource: .file(path: " /tmp/secret "), + storeIsTrusted: false + ) == .profileStoreNotTrusted) + #expect(CredentialProfileEditorSheet.testRefusal( + for: .command(shell: "echo stored"), + storedSource: .command(shell: "echo stored", reuseSeconds: 300), + storeIsTrusted: false + ) == .profileStoreNotTrusted) + } +} diff --git a/apple/TableProTests/Views/Connection/SecretRevealTests.swift b/apple/TableProTests/Views/Connection/SecretRevealTests.swift new file mode 100644 index 0000000000..0de4fbb5cf --- /dev/null +++ b/apple/TableProTests/Views/Connection/SecretRevealTests.swift @@ -0,0 +1,45 @@ +import Foundation +@testable import TablePro +import Testing + +@MainActor +struct SecretRevealTests { + @Test("A saved password stays hidden when authentication is refused") + func refusedAuthenticationKeepsItHidden() async { + let authenticator = StubAuthenticating(answer: false) + let reveal = SecretReveal(authenticator: authenticator) + + await reveal.setRevealed(true, requiresAuthentication: true) + + #expect(!reveal.isRevealed) + #expect(authenticator.callCount == 1) + #expect(authenticator.lastReason == SecretReveal.authenticationReason) + } + + @Test("A saved password shows after authenticating, and asks only once") + func authenticatesOnce() async { + let authenticator = StubAuthenticating(answer: true) + let reveal = SecretReveal(authenticator: authenticator) + + await reveal.setRevealed(true, requiresAuthentication: true) + #expect(reveal.isRevealed) + + await reveal.setRevealed(false, requiresAuthentication: true) + #expect(!reveal.isRevealed) + + await reveal.setRevealed(true, requiresAuthentication: true) + #expect(reveal.isRevealed) + #expect(authenticator.callCount == 1) + } + + @Test("A password typed in this window shows without authenticating") + func typedPasswordNeedsNoAuthentication() async { + let authenticator = StubAuthenticating(answer: false) + let reveal = SecretReveal(authenticator: authenticator) + + await reveal.setRevealed(true, requiresAuthentication: false) + + #expect(reveal.isRevealed) + #expect(authenticator.callCount == 0) + } +} diff --git a/apple/TableProTests/Views/ConnectionForm/ViewModels/AuthPanePasswordSourceTests.swift b/apple/TableProTests/Views/ConnectionForm/ViewModels/AuthPanePasswordSourceTests.swift new file mode 100644 index 0000000000..b6ece9b16d --- /dev/null +++ b/apple/TableProTests/Views/ConnectionForm/ViewModels/AuthPanePasswordSourceTests.swift @@ -0,0 +1,112 @@ +import Foundation +@testable import TablePro +import Testing + +@MainActor +struct AuthPanePasswordSourceTests { + private let suite: EphemeralDefaults + + init() throws { + suite = try EphemeralDefaults() + } + + private func makeStorage() -> ConnectionStorage { + let fileURL = FileManager.default.temporaryDirectory + .appendingPathComponent("tablepro-tests") + .appendingPathComponent(UUID().uuidString) + .appendingPathComponent("connections.json") + try? FileManager.default.createDirectory( + at: fileURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + return ConnectionStorage(fileURL: fileURL, userDefaults: suite.defaults, keychain: InMemoryKeychain()) + } + + private func loadedModel(source: PasswordSource?, configure: (inout DatabaseConnection) -> Void = { _ in }) -> AuthPaneViewModel { + var connection = TestFixtures.makeConnection() + connection.username = "app" + connection.passwordSource = source + configure(&connection) + let model = AuthPaneViewModel() + model.load(from: connection, storage: makeStorage()) + return model + } + + @Test("The form knows where a connection's password comes from") + func loadsThePasswordSource() { + let source = PasswordSource.command(shell: "op read op://vault/db/password") + let model = loadedModel(source: source) + + #expect(model.passwordSource == source) + #expect(model.passwordSourceInUse == source) + } + + @Test("A prompt or a linked profile overrides the connection's own source") + func promptAndProfileOverrideTheSource() { + let model = loadedModel(source: .env(variable: "DB_PASSWORD")) + + model.promptForPassword = true + #expect(model.passwordSourceInUse == nil) + + model.promptForPassword = false + model.credentialMode = .profile(id: UUID()) + #expect(model.passwordSourceInUse == nil) + } + + @Test("Saving a source-backed connection as a profile keeps the source") + func promotionKeepsATrustedSource() { + let source = PasswordSource.file(path: "~/.secrets/db") + let model = loadedModel(source: source) + + #expect(model.profileFromCurrentCredentials(sourceStoreIsTrusted: true).passwordMode == .source(source)) + } + + @Test("A source from a connections file something else changed becomes a prompt, never a profile source") + func promotionDropsAnUntrustedSource() { + let model = loadedModel(source: .command(shell: "echo owned")) + + #expect(model.profileFromCurrentCredentials(sourceStoreIsTrusted: false).passwordMode == .prompt) + } + + @Test("Saving a ~/.pgpass connection as a profile keeps ~/.pgpass") + func promotionKeepsPgpass() { + let model = loadedModel(source: nil) + model.additionalFieldValues["usePgpass"] = "true" + + #expect(model.profileFromCurrentCredentials(sourceStoreIsTrusted: true).passwordMode == .pgpass) + } + + @Test("Test Connection asks for the password when Prompt for password is on") + func testConnectionCarriesThePromptFlag() { + let coordinator = ConnectionFormCoordinator(connectionId: nil) + coordinator.auth.promptForPassword = true + + let connection = coordinator.connectionForTest() + + #expect(connection.promptForPassword) + #expect(ConnectionCredentialResolver.promptsForPassword(connection)) + } + + @Test("Test Connection runs the connection's source, not a Keychain password left beside it") + func testConnectionUsesTheSource() { + let source = PasswordSource.env(variable: "DB_PASSWORD") + var connection = TestFixtures.makeConnection() + connection.passwordSource = source + let storage = makeStorage() + #expect(storage.savePassword("stale", for: connection.id)) + let coordinator = ConnectionFormCoordinator(connectionId: nil) + coordinator.auth.load(from: connection, storage: storage) + + #expect(coordinator.auth.password == "stale") + #expect(coordinator.connectionForTest().passwordSource == source) + } + + @Test("A connection with only a source can still be saved as a profile") + func sourceAloneIsPromotable() { + let model = loadedModel(source: .env(variable: "REDIS_PASSWORD")) { connection in + connection.username = "" + } + + #expect(model.hasPromotableCredentials) + } +} From 89666add22a88830f1317b407fe2148350965bc4 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 21 Oct 2026 04:29:06 +0700 Subject: [PATCH 2/2] docs(changelog): add the #3723 fragment --- apple/changelog.d/3723.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 apple/changelog.d/3723.added.md diff --git a/apple/changelog.d/3723.added.md b/apple/changelog.d/3723.added.md new file mode 100644 index 0000000000..958796f140 --- /dev/null +++ b/apple/changelog.d/3723.added.md @@ -0,0 +1 @@ +Show a saved connection password after Touch ID, and test a password source without connecting.