diff --git a/CHANGELOG.md b/CHANGELOG.md index d4f2cfa5dc..ffa5abe05a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - An already open `.sql` file, linked favorite, query link or database file raising the window without showing its connection and tab. - Picking a database in the query editor rerunning the statement at the caret, writes included, without asking. - "Connection Failed: Cancelled by user." after declining a connect prompt from Switch Connection. +- A database link with a filter replacing the filters of a tab already showing that table. - A cleared group, tag, AI rule, startup command or SSH profile, or a group moved to the top level, returning after iCloud sync. (#3309) - Exported connections taking the color of another group with the same name. (#3309) - Copy TablePro Link dropping the connect and query timeouts, and new tags after the first. (#3309) @@ -100,6 +101,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Duplicating a connection set to Blocked for external access making the copy readable over MCP. (#3309) - DuckDB names holding a quote followed by a combining mark, and ClickHouse names holding a backslash, ending their quoting early in generated SQL. (#3308) +- A database link's SQL filter shown cut to 300 characters, and connecting to a trusted database before asking about it. +- A database link through an SSH tunnel to `127.0.0.1` reusing a local database's Always Allow, whatever SSH server it named. ## [0.79.0] - 2026-10-09 diff --git a/TablePro/Core/Services/Infrastructure/ExternalConnectionGate.swift b/TablePro/Core/Services/Infrastructure/ExternalConnectionGate.swift index ef634d2c92..de7b821610 100644 --- a/TablePro/Core/Services/Infrastructure/ExternalConnectionGate.swift +++ b/TablePro/Core/Services/Infrastructure/ExternalConnectionGate.swift @@ -18,15 +18,25 @@ internal struct ExternalConnectionGate { self.prompt = prompt ?? ExternalConnectionAlertPrompt() } - internal func authorize(_ connection: DatabaseConnection, scopeName: String?) async -> Bool { + /// The trust key names neither a filter nor an SSH server, so trusting a link that carries one + /// would cover whatever SQL or server a later link picks. A tunnelled loopback host is not on + /// this Mac either. + internal func authorize( + _ connection: DatabaseConnection, + scopeName: String?, + filter: ConnectionURLFilter? = nil + ) async -> Bool { let key = ExternalConnectionTrustKey(connection: connection, scopeName: scopeName) - if key.isLoopbackHost, trustStore.isTrusted(key) { return true } + let trustable = key.isLoopbackHost && !connection.sshConfig.enabled && filter == nil + if trustable, trustStore.isTrusted(key) { return true } - switch await prompt.prompt(for: connection, offerAlwaysAllow: key.isLoopbackHost) { + switch await prompt.prompt(for: connection, filter: filter, offerAlwaysAllow: trustable) { case .connect: return true case .alwaysAllow: - trustStore.trust(key) + if trustable { + trustStore.trust(key) + } return true case .cancel: return false diff --git a/TablePro/Core/Services/Infrastructure/ExternalConnectionPrompting.swift b/TablePro/Core/Services/Infrastructure/ExternalConnectionPrompting.swift index 3ccfa0943e..fbeb1ac05c 100644 --- a/TablePro/Core/Services/Infrastructure/ExternalConnectionPrompting.swift +++ b/TablePro/Core/Services/Infrastructure/ExternalConnectionPrompting.swift @@ -14,16 +14,22 @@ internal enum ExternalConnectionDecision: Sendable { @MainActor internal protocol ExternalConnectionPrompting { - func prompt(for connection: DatabaseConnection, offerAlwaysAllow: Bool) async -> ExternalConnectionDecision + func prompt( + for connection: DatabaseConnection, + filter: ConnectionURLFilter?, + offerAlwaysAllow: Bool + ) async -> ExternalConnectionDecision } @MainActor internal struct ExternalConnectionAlertPrompt: ExternalConnectionPrompting { internal func prompt( for connection: DatabaseConnection, + filter: ConnectionURLFilter?, offerAlwaysAllow: Bool ) async -> ExternalConnectionDecision { - let response = await present(Self.makeAlert(for: connection, offerAlwaysAllow: offerAlwaysAllow)) + let alert = Self.makeAlert(for: connection, filter: filter, offerAlwaysAllow: offerAlwaysAllow) + let response = await present(alert) switch response { case .alertFirstButtonReturn: return .connect @@ -34,20 +40,45 @@ internal struct ExternalConnectionAlertPrompt: ExternalConnectionPrompting { } } - internal static func makeAlert(for connection: DatabaseConnection, offerAlwaysAllow: Bool) -> NSAlert { + internal static func makeAlert( + for connection: DatabaseConnection, + filter: ConnectionURLFilter? = nil, + offerAlwaysAllow: Bool + ) -> NSAlert { let alert = NSAlert() alert.messageText = String(localized: "Open External Database Connection?") - alert.informativeText = String( - format: String(localized: """ - An external link wants to connect to a %@ database: + let details = details(for: connection).joined(separator: "\n") + if let filter { + alert.informativeText = String( + format: String(localized: """ + An external link wants to connect to a %@ database and filter a table: - %@ + %@ - Connect only if you trust the source of this link. - """), - connection.type.rawValue, - details(for: connection).joined(separator: "\n") - ) + Connect only if you trust the source of this link and the filter below. + """), + connection.type.rawValue, + details + ) + /// `informativeText` neither scrolls nor reveals invisible characters. An overlay scroller + /// stays hidden until scrolled, so line breaks could push SQL out of the box unnoticed. + let box = AlertHelper.scrollingTextAccessory(RevealedText(filter.displayText).plainText) + box.scrollerStyle = .legacy + box.autohidesScrollers = true + alert.accessoryView = box + } else { + alert.informativeText = String( + format: String(localized: """ + An external link wants to connect to a %@ database: + + %@ + + Connect only if you trust the source of this link. + """), + connection.type.rawValue, + details + ) + } alert.alertStyle = .warning /// Connecting is the risky half of this decision, so it gives up Return. Escape stays on /// Cancel, which is the only binding that dismisses the alert from the keyboard. diff --git a/TablePro/Core/Services/Infrastructure/TabRouter.swift b/TablePro/Core/Services/Infrastructure/TabRouter.swift index e485dbc185..a2aad1bd33 100644 --- a/TablePro/Core/Services/Infrastructure/TabRouter.swift +++ b/TablePro/Core/Services/Infrastructure/TabRouter.swift @@ -13,6 +13,7 @@ internal enum TabRouterError: Error, LocalizedError { case fileNoLongerExists(URL) case userCancelled case unsupportedIntent(String) + case filterTooLong(Int, limit: Int) case connectFailedInWindow(connectionId: UUID, underlying: Error) internal var windowConnectionId: UUID? { @@ -38,6 +39,8 @@ internal enum TabRouterError: Error, LocalizedError { return String(localized: "Cancelled by user.") case .unsupportedIntent(let detail): return String(format: String(localized: "Unsupported intent: %@"), detail) + case .filterTooLong(let length, let limit): + return String(format: String(localized: "SQL is too long: %d characters (limit %d)"), length, limit) case .connectFailedInWindow(_, let underlying): return underlying.localizedDescription } @@ -228,6 +231,7 @@ internal final class TabRouter { connectionId: UUID, transientConnection: DatabaseConnection? = nil, database: String?, schema: String?, table: String, isView: Bool, objectType: TableInfo.TableType? = nil, + initialFilterState: TabFilterState? = nil, passwordOverride: String? = nil, sshPasswordOverride: String? = nil ) async throws { let connection: DatabaseConnection @@ -238,7 +242,10 @@ internal final class TabRouter { } else { throw TabRouterError.connectionNotFound(connectionId) } - if focusExistingTableTab(connectionId: connectionId, database: database, schema: schema, table: table) { + /// A filtered open gets a new tab with the filter in its payload, so the filter is in place + /// before the first load and a tab already showing the table keeps its own filters. + if initialFilterState == nil, + focusExistingTableTab(connectionId: connectionId, database: database, schema: schema, table: table) { AppActivationPolicyController.shared.activate(ignoringOtherApps: true) WindowOpener.shared.closeWelcome() return @@ -253,7 +260,9 @@ internal final class TabRouter { databaseName: database, schemaName: schema, isView: isView, - objectType: objectType + objectType: objectType, + forcesNewTab: initialFilterState != nil, + initialFilterState: initialFilterState ) DatabaseManager.shared.registerPendingSession(connection) WindowManager.shared.openTab(payload: payload) @@ -349,6 +358,13 @@ internal final class TabRouter { guard case .success(let parsed) = ConnectionURLParser.parse(url.absoluteString) else { throw TabRouterError.malformedDatabaseURL(url) } + let filter = parsed.filter + if let filter { + let length = (filter.displayText as NSString).length + guard length <= DeeplinkParser.sqlLengthLimit else { + throw TabRouterError.filterTooLong(length, limit: DeeplinkParser.sqlLengthLimit) + } + } let connections = ConnectionStorage.shared.loadConnections() let matched = connections.first { conn in @@ -369,7 +385,9 @@ internal final class TabRouter { isTransient = true } - guard await externalConnectionGate.authorize(connection, scopeName: parsed.connectionName) else { + guard await externalConnectionGate.authorize( + connection, scopeName: parsed.connectionName, filter: filter + ) else { throw TabRouterError.userCancelled } @@ -385,13 +403,11 @@ internal final class TabRouter { schema: parsed.schema, table: table, isView: parsed.isView, + initialFilterState: filter?.filterState, passwordOverride: passwordOverride, sshPasswordOverride: sshPasswordOverride ) } - if parsed.filterColumn != nil || parsed.filterCondition != nil { - try await applyFilterFromParsedURL(parsed: parsed, connectionId: connection.id) - } return } @@ -562,37 +578,4 @@ internal final class TabRouter { throw TabRouterError.userCancelled } } - - private func applyFilterFromParsedURL(parsed: ParsedConnectionURL, connectionId: UUID) async throws { - let description: String - if let condition = parsed.filterCondition, !condition.isEmpty { - description = (condition as NSString).length > 300 - ? String(condition.prefix(300)) + "…" : condition - } else { - description = [parsed.filterColumn, parsed.filterOperation, parsed.filterValue] - .compactMap { $0 }.joined(separator: " ") - } - if !description.isEmpty { - let confirmed = await AlertHelper.confirmDestructive( - title: String(localized: "Apply Filter from Link"), - message: String( - format: String(localized: "An external link wants to apply a filter:\n\n%@"), - description - ), - confirmButton: String(localized: "Apply Filter"), - cancelButton: String(localized: "Cancel"), - window: NSApp.keyWindow - ) - guard confirmed else { throw TabRouterError.userCancelled } - } - - guard let coordinator = MainContentCoordinator.allActiveCoordinators() - .first(where: { $0.connectionId == connectionId }) else { return } - coordinator.applyURLFilter( - condition: parsed.filterCondition, - column: parsed.filterColumn, - operation: parsed.filterOperation, - value: parsed.filterValue - ) - } } diff --git a/TablePro/Core/Utilities/Connection/ConnectionURLFilter.swift b/TablePro/Core/Utilities/Connection/ConnectionURLFilter.swift new file mode 100644 index 0000000000..abc4058bb8 --- /dev/null +++ b/TablePro/Core/Utilities/Connection/ConnectionURLFilter.swift @@ -0,0 +1,94 @@ +// +// ConnectionURLFilter.swift +// TablePro +// + +import Foundation + +internal enum ConnectionURLFilter: Equatable, Sendable { + case condition(String) + case column(name: String, operation: String?, value: String?) + + internal var displayText: String { + switch self { + case .condition(let sql): + return sql + case .column(let name, let operation, let value): + return [name, operation, value].compactMap { $0 }.joined(separator: " ") + } + } + + internal var filterState: TabFilterState { + TabFilterState(filters: [tableFilter], commit: .all, isVisible: true, filterLogicMode: .and) + } + + private var tableFilter: TableFilter { + switch self { + case .condition(let sql): + return TableFilter(columnName: TableFilter.rawSQLColumn, rawSQL: sql) + case .column(let name, let operation, let value): + return TableFilter( + columnName: name, + filterOperator: Self.filterOperator(forURLOperation: operation ?? "Equal"), + value: value ?? "" + ) + } + } + + private static func filterOperator(forURLOperation operation: String) -> FilterOperator { + switch operation.lowercased() { + case "equal", "equals", "=": + return .equal + case "not equal", "notequal", "!=": + return .notEqual + case "contains", "like": + return .contains + case "not contains", "notcontains", "not like": + return .notContains + case "starts with", "startswith": + return .startsWith + case "ends with", "endswith": + return .endsWith + case "greater than", "greaterthan", ">": + return .greaterThan + case "greater or equal", "greaterorequal", ">=": + return .greaterOrEqual + case "less than", "lessthan", "<": + return .lessThan + case "less or equal", "lessorequal", "<=": + return .lessOrEqual + case "is null", "isnull": + return .isNull + case "is not null", "isnotnull": + return .isNotNull + case "is empty", "isempty": + return .isEmpty + case "is not empty", "isnotempty": + return .isNotEmpty + case "in": + return .inList + case "not in", "notin": + return .notInList + case "between": + return .between + case "regex": + return .regex + default: + return .contains + } + } +} + +internal extension ParsedConnectionURL { + /// The filter is applied to the table the link opens, so a link without a table applies none. + var filter: ConnectionURLFilter? { + guard tableName != nil else { return nil } + if let filterCondition, !filterCondition.isEmpty { + return .condition(filterCondition) + } + if let filterColumn, !filterColumn.isEmpty { + return .column(name: filterColumn, operation: filterOperation, value: filterValue) + } + return nil + } +} diff --git a/TablePro/Core/Utilities/UI/AlertHelper.swift b/TablePro/Core/Utilities/UI/AlertHelper.swift index 8e82365aec..9ded43adbd 100644 --- a/TablePro/Core/Utilities/UI/AlertHelper.swift +++ b/TablePro/Core/Utilities/UI/AlertHelper.swift @@ -37,6 +37,33 @@ final class AlertHelper { return cancel } + /// A text view laid inside a scroll view by hand has to be told it may grow and that its text + /// container tracks its width. Left at its default zero-sized container it lays out no text at + /// all, so the accessory reads as an empty box. + static func scrollingTextAccessory(_ text: String) -> NSScrollView { + let scroll = NSScrollView(frame: NSRect(x: 0, y: 0, width: 380, height: 140)) + scroll.hasVerticalScroller = true + scroll.borderType = .bezelBorder + + let textView = NSTextView(frame: NSRect(origin: .zero, size: scroll.contentSize)) + textView.isEditable = false + textView.drawsBackground = false + textView.font = .monospacedSystemFont(ofSize: NSFont.smallSystemFontSize, weight: .regular) + textView.autoresizingMask = [.width] + textView.isVerticallyResizable = true + textView.isHorizontallyResizable = false + textView.maxSize = NSSize(width: CGFloat.greatestFiniteMagnitude, height: CGFloat.greatestFiniteMagnitude) + textView.textContainer?.widthTracksTextView = true + textView.textContainer?.containerSize = NSSize( + width: scroll.contentSize.width, + height: CGFloat.greatestFiniteMagnitude + ) + textView.string = text + + scroll.documentView = textView + return scroll + } + /// The window a sheet belongs on. A sheet the user is meant to read against their work must /// land on a document window, so a floating panel is never a candidate: the Quick Switcher /// closes the moment it loses focus, taking the sheet with it. An explicit window is honoured diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index c638910064..a0874c08ca 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -23572,6 +23572,16 @@ } } }, + "An external link wants to connect to a %@ database and filter a table:\n\n%@\n\nConnect only if you trust the source of this link and the filter below." : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "An external link wants to connect to a %1$@ database and filter a table:\n\n%2$@\n\nConnect only if you trust the source of this link and the filter below." + } + } + } + }, "An external link wants to connect to a %@ database:\n\n%@\n\nConnect only if you trust the source of this link." : { "localizations" : { "en" : { diff --git a/TablePro/Views/Components/TransferResultAlert.swift b/TablePro/Views/Components/TransferResultAlert.swift index 10f4bd45c3..a55489b57b 100644 --- a/TablePro/Views/Components/TransferResultAlert.swift +++ b/TablePro/Views/Components/TransferResultAlert.swift @@ -299,33 +299,6 @@ internal enum TransferResultAlert { ) return "\(counts)\n\(seconds)" } - - /// A text view laid inside a scroll view by hand has to be told it may grow and that its text - /// container tracks its width. Left at its default zero-sized container it lays out no text at - /// all, so the accessory reads as an empty box. - fileprivate static func scrollingText(_ text: String) -> NSScrollView { - let scroll = NSScrollView(frame: NSRect(x: 0, y: 0, width: 380, height: 140)) - scroll.hasVerticalScroller = true - scroll.borderType = .bezelBorder - - let textView = NSTextView(frame: NSRect(origin: .zero, size: scroll.contentSize)) - textView.isEditable = false - textView.drawsBackground = false - textView.font = .monospacedSystemFont(ofSize: NSFont.smallSystemFontSize, weight: .regular) - textView.autoresizingMask = [.width] - textView.isVerticallyResizable = true - textView.isHorizontallyResizable = false - textView.maxSize = NSSize(width: CGFloat.greatestFiniteMagnitude, height: CGFloat.greatestFiniteMagnitude) - textView.textContainer?.widthTracksTextView = true - textView.textContainer?.containerSize = NSSize( - width: scroll.contentSize.width, - height: CGFloat.greatestFiniteMagnitude - ) - textView.string = text - - scroll.documentView = textView - return scroll - } } /// The failure report is what a user has to act on, so it is selectable text with a copy button @@ -369,7 +342,7 @@ internal final class TransferReportView: NSView { height: Self.textHeight + Self.spacing + button.frame.height )) - let scroll = TransferResultAlert.scrollingText(RevealedText(Self.shownText(shown)).plainText) + let scroll = AlertHelper.scrollingTextAccessory(RevealedText(Self.shownText(shown)).plainText) scroll.frame = NSRect( x: 0, y: button.frame.height + Self.spacing, diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+URLFilter.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+URLFilter.swift deleted file mode 100644 index 27e6569547..0000000000 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+URLFilter.swift +++ /dev/null @@ -1,78 +0,0 @@ -// -// MainContentCoordinator+URLFilter.swift -// TablePro -// - -import Foundation - -extension MainContentCoordinator { - func applyURLFilter(condition: String?, column: String?, operation: String?, value: String?) { - if let condition, !condition.isEmpty { - let filter = TableFilter( - id: UUID(), - columnName: TableFilter.rawSQLColumn, - filterOperator: .equal, - value: "", - isEnabled: true, - rawSQL: condition - ) - applySingleFilter(filter) - return - } - - guard let column, !column.isEmpty else { return } - - let filterOp = mapTablePlusOperation(operation ?? "Equal") - let filter = TableFilter( - id: UUID(), - columnName: column, - filterOperator: filterOp, - value: value ?? "", - isEnabled: true - ) - applySingleFilter(filter) - } - - private func mapTablePlusOperation(_ operation: String) -> FilterOperator { - switch operation.lowercased() { - case "equal", "equals", "=": - return .equal - case "not equal", "notequal", "!=": - return .notEqual - case "contains", "like": - return .contains - case "not contains", "notcontains", "not like": - return .notContains - case "starts with", "startswith": - return .startsWith - case "ends with", "endswith": - return .endsWith - case "greater than", "greaterthan", ">": - return .greaterThan - case "greater or equal", "greaterorequal", ">=": - return .greaterOrEqual - case "less than", "lessthan", "<": - return .lessThan - case "less or equal", "lessorequal", "<=": - return .lessOrEqual - case "is null", "isnull": - return .isNull - case "is not null", "isnotnull": - return .isNotNull - case "is empty", "isempty": - return .isEmpty - case "is not empty", "isnotempty": - return .isNotEmpty - case "in": - return .inList - case "not in", "notin": - return .notInList - case "between": - return .between - case "regex": - return .regex - default: - return .contains - } - } -} diff --git a/TableProTests/Core/Services/Infrastructure/ExternalConnectionFilterDisclosureTests.swift b/TableProTests/Core/Services/Infrastructure/ExternalConnectionFilterDisclosureTests.swift new file mode 100644 index 0000000000..9a96408478 --- /dev/null +++ b/TableProTests/Core/Services/Infrastructure/ExternalConnectionFilterDisclosureTests.swift @@ -0,0 +1,75 @@ +import AppKit +@testable import TablePro +import Testing + +@MainActor +struct ExternalConnectionFilterDisclosureTests { + private func connection() -> DatabaseConnection { + DatabaseConnection( + name: "Local", host: "127.0.0.1", port: 5_432, + database: "shop", username: "postgres", type: .postgresql + ) + } + + private func shownFilter(in alert: NSAlert) -> String? { + let scroll = alert.accessoryView as? NSScrollView + return (scroll?.documentView as? NSTextView)?.string + } + + @Test("The whole condition is shown, however long") + func showsWholeCondition() { + let condition = "id > 0" + String(repeating: " ", count: 400) + "OR (SELECT pg_sleep(30)) IS NULL" + let alert = ExternalConnectionAlertPrompt.makeAlert( + for: connection(), filter: .condition(condition), offerAlwaysAllow: false + ) + #expect(shownFilter(in: alert) == condition) + } + + @Test("Invisible characters in the condition are revealed") + func revealsInvisibleCharacters() throws { + let alert = ExternalConnectionAlertPrompt.makeAlert( + for: connection(), filter: .condition("id = 1\u{202E}--"), offerAlwaysAllow: false + ) + let shown = try #require(shownFilter(in: alert)) + #expect(!shown.contains("\u{202E}")) + #expect(shown.contains("")) + } + + @Test("A column filter is shown as column, operation and value") + func showsColumnFilter() { + let alert = ExternalConnectionAlertPrompt.makeAlert( + for: connection(), + filter: .column(name: "status", operation: "=", value: "active"), + offerAlwaysAllow: false + ) + #expect(shownFilter(in: alert) == "status = active") + } + + @Test("The target is still named next to the filter") + func namesTargetWithFilter() { + let alert = ExternalConnectionAlertPrompt.makeAlert( + for: connection(), filter: .condition("id > 0"), offerAlwaysAllow: false + ) + #expect(alert.informativeText.contains("127.0.0.1:5432")) + #expect(alert.informativeText.contains("shop")) + #expect(alert.buttons.count == 2) + } + + @Test("The filter box shows a scroller whenever the filter overflows it") + func overflowShowsScroller() throws { + let padded = "id > 0" + String(repeating: "\n", count: 40) + "OR (SELECT pg_sleep(30)) IS NULL" + let alert = ExternalConnectionAlertPrompt.makeAlert( + for: connection(), filter: .condition(padded), offerAlwaysAllow: false + ) + let box = try #require(alert.accessoryView as? NSScrollView) + #expect(box.scrollerStyle == .legacy) + #expect(box.autohidesScrollers) + #expect(shownFilter(in: alert) == padded) + } + + @Test("A link without a filter shows no filter box") + func noFilterNoAccessory() { + let alert = ExternalConnectionAlertPrompt.makeAlert(for: connection(), offerAlwaysAllow: true) + #expect(alert.accessoryView == nil) + } +} diff --git a/TableProTests/Core/Services/Infrastructure/ExternalConnectionGateTests.swift b/TableProTests/Core/Services/Infrastructure/ExternalConnectionGateTests.swift index 752fd95211..42d11fc4c8 100644 --- a/TableProTests/Core/Services/Infrastructure/ExternalConnectionGateTests.swift +++ b/TableProTests/Core/Services/Infrastructure/ExternalConnectionGateTests.swift @@ -8,14 +8,20 @@ private final class SpyPrompt: ExternalConnectionPrompting { private(set) var callCount = 0 private(set) var offeredAlwaysAllow: Bool? + private(set) var shownFilter: ConnectionURLFilter? init(decision: ExternalConnectionDecision) { self.decision = decision } - func prompt(for connection: DatabaseConnection, offerAlwaysAllow: Bool) async -> ExternalConnectionDecision { + func prompt( + for connection: DatabaseConnection, + filter: ConnectionURLFilter?, + offerAlwaysAllow: Bool + ) async -> ExternalConnectionDecision { callCount += 1 offeredAlwaysAllow = offerAlwaysAllow + shownFilter = filter return decision } } @@ -36,6 +42,14 @@ struct ExternalConnectionGateTests { ) } + private func tunnelledConnection() -> DatabaseConnection { + var connection = ddevConnection() + connection.sshConfig.enabled = true + connection.sshConfig.host = "bastion.example.com" + connection.sshConfig.username = "ops" + return connection + } + private func remoteConnection() -> DatabaseConnection { DatabaseConnection( name: "Prod", host: "db.example.com", port: 3_306, @@ -119,4 +133,81 @@ struct ExternalConnectionGateTests { _ = await gate.authorize(ddevConnection(port: 49_154), scopeName: "ddev-blog") #expect(prompt.callCount == 2) } + + @Test("A trusted target still asks when the link carries a filter, and shows it") + func filterOnTrustedTargetPrompts() async throws { + let store = try makeStore() + store.trust(ExternalConnectionTrustKey(connection: ddevConnection(), scopeName: "ddev-shop")) + let prompt = SpyPrompt(decision: .connect) + let gate = ExternalConnectionGate(trustStore: store, prompt: prompt) + let filter = ConnectionURLFilter.condition("1=1) OR (SELECT pg_sleep(30)") + + #expect(await gate.authorize(ddevConnection(), scopeName: "ddev-shop", filter: filter)) + #expect(prompt.callCount == 1) + #expect(prompt.shownFilter == filter) + #expect(prompt.offeredAlwaysAllow == false) + } + + @Test("A column filter also asks on a trusted target") + func columnFilterOnTrustedTargetPrompts() async throws { + let store = try makeStore() + store.trust(ExternalConnectionTrustKey(connection: ddevConnection(), scopeName: "ddev-shop")) + let prompt = SpyPrompt(decision: .connect) + let gate = ExternalConnectionGate(trustStore: store, prompt: prompt) + let filter = ConnectionURLFilter.column(name: "status", operation: "=", value: "active") + + _ = await gate.authorize(ddevConnection(), scopeName: "ddev-shop", filter: filter) + #expect(prompt.callCount == 1) + #expect(prompt.shownFilter == filter) + } + + @Test("Answering Always Allow to a link with a filter trusts nothing") + func filterNeverPersistsTrust() async throws { + let store = try makeStore() + let prompt = SpyPrompt(decision: .alwaysAllow) + let gate = ExternalConnectionGate(trustStore: store, prompt: prompt) + + let authorized = await gate.authorize( + ddevConnection(), scopeName: "ddev-shop", filter: .condition("id > 0") + ) + + #expect(authorized) + #expect(store.entries().isEmpty) + _ = await gate.authorize(ddevConnection(), scopeName: "ddev-shop") + #expect(prompt.callCount == 2) + } + + @Test("Declining a link with a filter refuses it") + func filterCancelRefuses() async throws { + let store = try makeStore() + store.trust(ExternalConnectionTrustKey(connection: ddevConnection(), scopeName: "ddev-shop")) + let gate = ExternalConnectionGate(trustStore: store, prompt: SpyPrompt(decision: .cancel)) + + let authorized = await gate.authorize( + ddevConnection(), scopeName: "ddev-shop", filter: .condition("id > 0") + ) + + #expect(authorized == false) + } + + @Test("A tunnelled link asks even when the same local target is trusted") + func tunnelIgnoresTrust() async throws { + let store = try makeStore() + store.trust(ExternalConnectionTrustKey(connection: ddevConnection(), scopeName: "ddev-shop")) + let prompt = SpyPrompt(decision: .connect) + let gate = ExternalConnectionGate(trustStore: store, prompt: prompt) + + #expect(await gate.authorize(tunnelledConnection(), scopeName: "ddev-shop")) + #expect(prompt.callCount == 1) + #expect(prompt.offeredAlwaysAllow == false) + } + + @Test("Answering Always Allow to a tunnelled link trusts nothing") + func tunnelNeverPersistsTrust() async throws { + let store = try makeStore() + let gate = ExternalConnectionGate(trustStore: store, prompt: SpyPrompt(decision: .alwaysAllow)) + + #expect(await gate.authorize(tunnelledConnection(), scopeName: "ddev-shop")) + #expect(store.entries().isEmpty) + } } diff --git a/TableProTests/Core/Utilities/ConnectionURLFilterTests.swift b/TableProTests/Core/Utilities/ConnectionURLFilterTests.swift new file mode 100644 index 0000000000..d9e857373a --- /dev/null +++ b/TableProTests/Core/Utilities/ConnectionURLFilterTests.swift @@ -0,0 +1,95 @@ +import Foundation +@testable import TablePro +import Testing + +struct ConnectionURLFilterTests { + private func filter(_ url: String) -> ConnectionURLFilter? { + guard case .success(let parsed) = ConnectionURLParser.parse(url) else { + Issue.record("Expected \(url) to parse") + return nil + } + return parsed.filter + } + + @Test("Each raw SQL alias is read as a condition", arguments: ["condition", "raw", "query"]) + func rawAliases(_ name: String) { + let url = "mysql://root@127.0.0.1/shop?table=orders&\(name)=total+%3E+1000" + #expect(filter(url) == .condition("total > 1000")) + } + + @Test("A condition overrides the column filter") + func conditionWins() { + let url = "postgresql://u@localhost/db?table=t&column=status&value=active&condition=id%3D1" + #expect(filter(url) == .condition("id=1")) + } + + @Test("Column, operation and value make a column filter") + func columnFilter() { + let url = "postgresql://u@localhost/db?table=t&column=status&operation=contains&value=pending+review" + let parsed = filter(url) + #expect(parsed == .column(name: "status", operation: "contains", value: "pending review")) + #expect(parsed?.displayText == "status contains pending review") + } + + @Test("A filter on a link that opens no table is not applied, so there is none") + func noTableNoFilter() { + #expect(filter("postgresql://u@localhost/db?condition=1%3D1") == nil) + #expect(filter("postgresql://u@localhost/db?column=status&value=a") == nil) + } + + @Test("A view link carries its filter too") + func viewFilter() { + #expect(filter("mysql://root@localhost/shop?view=v&raw=a%3D1") == .condition("a=1")) + } + + @Test("An SSH link carries its filter") + func sshFilter() { + let url = "mysql+ssh://ops@bastion.example.com/root@127.0.0.1/shop?table=orders&raw=id%3D1" + #expect(filter(url) == .condition("id=1")) + } + + @Test("A link with a table and no filter has none") + func tableOnly() { + #expect(filter("mysql://root@localhost/shop?table=orders") == nil) + } + + @Test("The condition is shown whole") + func conditionShownWhole() { + let condition = String(repeating: "a", count: 400) + " OR 1=1" + #expect(ConnectionURLFilter.condition(condition).displayText == condition) + } + + @Test("A condition becomes one applied raw SQL filter carrying the same text") + func conditionFilterState() throws { + let state = ConnectionURLFilter.condition("total > 1000").filterState + let filter = try #require(state.filters.first) + #expect(state.filters.count == 1) + #expect(state.commit == .all) + #expect(state.isVisible) + #expect(filter.isRawSQL) + #expect(filter.rawSQL == "total > 1000") + } + + @Test("A column filter keeps its column and value and maps the operation") + func columnFilterState() throws { + let cases: [(operation: String, expected: FilterOperator)] = [ + ("contains", .contains), (">=", .greaterOrEqual), ("Is Null", .isNull), ("unknown", .contains) + ] + for (operation, expected) in cases { + let state = ConnectionURLFilter.column(name: "status", operation: operation, value: "a").filterState + let filter = try #require(state.filters.first) + #expect(filter.columnName == "status", "\(operation)") + #expect(filter.value == "a", "\(operation)") + #expect(filter.filterOperator == expected, "\(operation)") + #expect(state.commit == .all, "\(operation)") + } + } + + @Test("A column filter without an operation compares for equality") + func columnFilterDefaultsToEqual() throws { + let state = ConnectionURLFilter.column(name: "status", operation: nil, value: nil).filterState + let filter = try #require(state.filters.first) + #expect(filter.filterOperator == .equal) + #expect(filter.value.isEmpty) + } +} diff --git a/TableProUITests/ExternalLinkFilterUITests.swift b/TableProUITests/ExternalLinkFilterUITests.swift new file mode 100644 index 0000000000..b9bee5292f --- /dev/null +++ b/TableProUITests/ExternalLinkFilterUITests.swift @@ -0,0 +1,33 @@ +import XCTest + +final class ExternalLinkFilterUITests: UITestCase { + func testALinkWithAFilterShowsTheWholeFilterAndOffersNoAlwaysAllow() throws { + guard #available(macOS 13.3, *) else { + throw XCTSkip("XCUIApplication.open(_:) needs macOS 13.3") + } + let app = try launchApp(arguments: ["-AppleLanguages", "(en)"]) + XCTAssertTrue(app.windows["welcome"].waitToExist(timeout: 10)) + + let condition = "status = 'pending' AND total > 1000" + var components = URLComponents() + components.scheme = "postgresql" + components.user = "postgres" + components.host = "127.0.0.1" + components.port = 5_432 + components.path = "/shop" + components.queryItems = [ + URLQueryItem(name: "table", value: "orders"), + URLQueryItem(name: "raw", value: condition) + ] + app.open(try XCTUnwrap(components.url)) + + XCTAssertTrue(app.buttons["Connect"].waitToExist(timeout: 10), "A link with a filter must ask before connecting") + let alert = app.dialogs.firstMatch.exists ? app.dialogs.firstMatch : app.sheets.firstMatch + let shownFilter = alert.textViews.matching(NSPredicate(format: "value == %@", condition)).firstMatch + XCTAssertTrue(shownFilter.waitToExist(timeout: 5), "The alert must show the whole filter") + XCTAssertFalse(alert.buttons["Always Allow"].exists, "A link with a filter must not offer Always Allow") + + alert.buttons["Cancel"].click() + XCTAssertTrue(waitForPredicate(timeout: 5) { !alert.exists }, "Cancel must close the alert") + } +} diff --git a/docs/connections/urls.mdx b/docs/connections/urls.mdx index 699cc95aa0..c77b4715c0 100644 --- a/docs/connections/urls.mdx +++ b/docs/connections/urls.mdx @@ -148,7 +148,7 @@ postgresql://user:pass@host/mydb?schema=reporting&table=monthly_sales ### Filtering -These apply a filter to the table the URL opened. Applying one needs its own confirmation. +These open the table in a new tab with the filter applied, so a tab already showing that table keeps its own filters. A link carrying a filter always asks first, even for a trusted database, and the alert shows the whole filter. A filter longer than 51,200 characters is refused. | Parameter | Description | Example | |-----------|-------------|---------| @@ -220,6 +220,8 @@ When another client also handles the scheme, `open -b com.TablePro "mysql://…" Every URL opened from outside the app raises an **Open External Database Connection?** alert naming the database type, host and port, user, and database. **Connect** does not answer the Return key. Escape is the only keyboard answer, and it cancels. -**Always Allow** appears only for a database on this machine (`localhost`, `127.x.x.x`, `::1`). A trusted link is matched on database type, host, database, username, and the URL's `name` parameter, never the port, so the same link on a different port still connects without asking again. Remote hosts always prompt. +**Always Allow** appears only for a database on this machine (`localhost`, `127.x.x.x`, `::1`). A trusted link is matched on database type, host, database, username, and the URL's `name` parameter, never the port, so the same link on a different port still connects without asking again. Remote hosts always prompt, and so does a link through an SSH tunnel, whose database is on the SSH server. + +A link with a [filter](#filtering) asks every time and offers no **Always Allow**. Read the filter in the box under the target before choosing **Connect**: it runs on the table as soon as the connection opens. Trusted links are listed under **Settings > General**, where **Forget** drops one and **Forget All** drops every one.