From d5563281c4d6b35eeee6c3818706f90766d88a8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Thu, 28 May 2026 12:56:42 +0700 Subject: [PATCH 1/3] fix(mcp): wire requireAuthentication into auth path and gate restart race (#1093) --- CHANGELOG.md | 1 + .../MCP/Auth/MCPCompositeAuthenticator.swift | 31 +++++ TablePro/Core/MCP/Auth/MCPPrincipal.swift | 11 ++ TablePro/Core/MCP/MCPAuditLogger.swift | 10 ++ TablePro/Core/MCP/MCPServerManager.swift | 33 ++++- .../Transport/MCPHttpServerTransport.swift | 63 ++++++++-- .../Core/Storage/AppSettingsManager.swift | 32 +++-- .../Views/Settings/Sections/MCPSection.swift | 20 +++- .../Sections/MCPTokenCreateSheet.swift | 7 ++ .../Auth/MCPCompositeAuthenticatorTests.swift | 113 ++++++++++++++++++ docs/features/mcp.mdx | 2 + 11 files changed, 301 insertions(+), 22 deletions(-) create mode 100644 TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift create mode 100644 TableProTests/Core/MCP/Auth/MCPCompositeAuthenticatorTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 09b2f6533..26c9fec86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - iOS: running a query that returns a very large result no longer crashes the app. The query editor keeps the first rows it loads, stops before memory runs low, and tells you to add LIMIT to fetch more. - iOS: Safe Mode "Confirm Writes" now prompts before saving a row edit or inserting a row, matching the query editor. Previously grid edits and inserts saved with no confirmation. - Redshift: schema switching now works, along with the contains, starts with, and ends with filters and table search. All previously failed with a SQL syntax error. (#1439) +- MCP server: the first authenticated request no longer hangs after turning on Require Authentication. Turning the setting on now creates a default token if you have none, shows it once for you to copy, and waits for the server to be ready before the next request can run. The Token Name field also focuses on first click in the Generate Token sheet. (#1093) ## [0.45.0] - 2026-05-26 diff --git a/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift b/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift new file mode 100644 index 000000000..44ccd3f6b --- /dev/null +++ b/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift @@ -0,0 +1,31 @@ +import Foundation +import os + +public actor MCPCompositeAuthenticator: MCPAuthenticator { + private static let logger = Logger(subsystem: "com.TablePro", category: "MCP.Auth") + + private let bearer: MCPBearerTokenAuthenticator + private let requireAuthentication: Bool + + public init( + bearer: MCPBearerTokenAuthenticator, + requireAuthentication: Bool + ) { + self.bearer = bearer + self.requireAuthentication = requireAuthentication + } + + public func authenticate( + authorizationHeader: String?, + clientAddress: MCPClientAddress + ) async -> MCPAuthDecision { + if !requireAuthentication, case .loopback = clientAddress { + MCPAuditLogger.logAuthAllowedAnonymous(ip: "127.0.0.1") + return .allow(MCPPrincipal.anonymousLoopback) + } + return await bearer.authenticate( + authorizationHeader: authorizationHeader, + clientAddress: clientAddress + ) + } +} diff --git a/TablePro/Core/MCP/Auth/MCPPrincipal.swift b/TablePro/Core/MCP/Auth/MCPPrincipal.swift index d4de96724..2e443d761 100644 --- a/TablePro/Core/MCP/Auth/MCPPrincipal.swift +++ b/TablePro/Core/MCP/Auth/MCPPrincipal.swift @@ -48,4 +48,15 @@ public struct MCPPrincipal: Sendable, Equatable, Hashable { hasher.combine(tokenFingerprint) hasher.combine(tokenId) } + + public static let anonymousLoopback = MCPPrincipal( + tokenFingerprint: "anonymous-loopback", + tokenId: nil, + scopes: [.toolsRead, .toolsWrite, .resourcesRead, .admin], + metadata: MCPPrincipalMetadata( + label: "Anonymous (loopback)", + issuedAt: .distantPast, + expiresAt: nil + ) + ) } diff --git a/TablePro/Core/MCP/MCPAuditLogger.swift b/TablePro/Core/MCP/MCPAuditLogger.swift index f92f3e5a2..e1413d10a 100644 --- a/TablePro/Core/MCP/MCPAuditLogger.swift +++ b/TablePro/Core/MCP/MCPAuditLogger.swift @@ -22,6 +22,16 @@ enum MCPAuditLogger { ) } + static func logAuthAllowedAnonymous(ip: String) { + serverAuth.info("Auth allowed anonymously (loopback, requireAuthentication=false): ip=\(ip, privacy: .public)") + record( + category: .auth, + action: "auth.anonymousLoopback", + outcome: .success, + details: "ip=\(ip)" + ) + } + static func logAuthFailure(reason: String, ip: String) { serverAuth.warning("Auth failure: reason=\(reason, privacy: .public) ip=\(ip, privacy: .public)") record( diff --git a/TablePro/Core/MCP/MCPServerManager.swift b/TablePro/Core/MCP/MCPServerManager.swift index 3a6b90516..ba6fa61d2 100644 --- a/TablePro/Core/MCP/MCPServerManager.swift +++ b/TablePro/Core/MCP/MCPServerManager.swift @@ -42,6 +42,7 @@ final class MCPServerManager { private var internalBridgeToken: String? private var serverGeneration: Int = 0 private var revocationObserverId: UUID? + private var lifecycleTask: Task? var isRunning: Bool { if case .running = state { return true } else { return false } @@ -97,10 +98,14 @@ final class MCPServerManager { let newRateLimiter = MCPRateLimiter() rateLimiter = newRateLimiter - let authenticator = MCPBearerTokenAuthenticator( + let bearerAuthenticator = MCPBearerTokenAuthenticator( tokenStore: newTokenStore, rateLimiter: newRateLimiter ) + let authenticator = MCPCompositeAuthenticator( + bearer: bearerAuthenticator, + requireAuthentication: settings.requireAuthentication + ) let newTransport = MCPHttpServerTransport( configuration: configuration, @@ -174,6 +179,32 @@ final class MCPServerManager { await start(port: port) } + func scheduleStart(port: UInt16) { + enqueueLifecycle { [weak self] in + await self?.start(port: port) + } + } + + func scheduleStop() { + enqueueLifecycle { [weak self] in + await self?.stop() + } + } + + func scheduleRestart(port: UInt16) { + enqueueLifecycle { [weak self] in + await self?.restart(port: port) + } + } + + private func enqueueLifecycle(_ work: @escaping @MainActor () async -> Void) { + let previousTask = lifecycleTask + lifecycleTask = Task { @MainActor in + await previousTask?.value + await work() + } + } + func lazyStart() async { if case .running = state { return } if case .starting = state { return } diff --git a/TablePro/Core/MCP/Transport/MCPHttpServerTransport.swift b/TablePro/Core/MCP/Transport/MCPHttpServerTransport.swift index fa09cefa2..64f7bba16 100644 --- a/TablePro/Core/MCP/Transport/MCPHttpServerTransport.swift +++ b/TablePro/Core/MCP/Transport/MCPHttpServerTransport.swift @@ -24,6 +24,9 @@ public actor MCPHttpServerTransport { private var sseWriters: [UUID: MCPSseWriter] = [:] private var sseConnectionsBySession: [MCPSessionId: UUID] = [:] private var sessionEventsTask: Task? + private var readyContinuation: CheckedContinuation? + private var readyTimeoutTask: Task? + private static let readyTimeout: Duration = .seconds(5) nonisolated public let exchanges: AsyncStream nonisolated private let exchangesContinuation: AsyncStream.Continuation @@ -71,33 +74,64 @@ public actor MCPHttpServerTransport { emitState(.starting) let parameters: NWParameters = makeParameters() - + let newListener: NWListener do { - let newListener = try NWListener(using: parameters) - listener = newListener + newListener = try NWListener(using: parameters) + } catch { + emitState(.failed(reason: error.localizedDescription)) + throw MCPHttpServerError.bindFailed(reason: error.localizedDescription) + } + listener = newListener - newListener.stateUpdateHandler = { [weak self] state in - guard let self else { return } - Task { await self.handleListenerState(state) } - } + newListener.stateUpdateHandler = { [weak self] state in + guard let self else { return } + Task { await self.handleListenerState(state) } + } - newListener.newConnectionHandler = { [weak self] connection in - guard let self else { return } - Task { await self.handleNewConnection(connection) } - } + newListener.newConnectionHandler = { [weak self] connection in + guard let self else { return } + Task { await self.handleNewConnection(connection) } + } - newListener.start(queue: .global(qos: .userInitiated)) - startSessionEventListener() + do { + try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in + readyContinuation = continuation + readyTimeoutTask = Task { [weak self] in + try? await Task.sleep(for: Self.readyTimeout) + await self?.handleReadyTimeout() + } + newListener.start(queue: .global(qos: .userInitiated)) + startSessionEventListener() + } } catch { + readyTimeoutTask?.cancel() + readyTimeoutTask = nil emitState(.failed(reason: error.localizedDescription)) + newListener.cancel() listener = nil throw MCPHttpServerError.bindFailed(reason: error.localizedDescription) } } + private func resumeReady(with result: Result) { + guard let continuation = readyContinuation else { return } + readyContinuation = nil + readyTimeoutTask?.cancel() + readyTimeoutTask = nil + continuation.resume(with: result) + } + + private func handleReadyTimeout() { + guard readyContinuation != nil else { return } + Self.logger.error("MCP HTTP listener did not reach .ready within timeout") + resumeReady(with: .failure(MCPHttpServerError.bindFailed(reason: "listener startup timed out"))) + } + public func stop() async { Self.logger.info("Stopping MCP HTTP server") + resumeReady(with: .failure(MCPHttpServerError.bindFailed(reason: "stop() called before listener ready"))) + sessionEventsTask?.cancel() sessionEventsTask = nil @@ -181,15 +215,18 @@ public actor MCPHttpServerTransport { let port = listener?.port?.rawValue ?? configuration.port Self.logger.info("MCP HTTP server listening on port \(port, privacy: .public)") emitState(.running(port: port)) + resumeReady(with: .success(())) case .failed(let error): Self.logger.error("MCP HTTP listener failed: \(error.localizedDescription, privacy: .public)") emitState(.failed(reason: error.localizedDescription)) listener?.cancel() listener = nil + resumeReady(with: .failure(error)) case .cancelled: Self.logger.debug("MCP HTTP listener cancelled") + resumeReady(with: .failure(MCPHttpServerError.bindFailed(reason: "listener cancelled before ready"))) default: break diff --git a/TablePro/Core/Storage/AppSettingsManager.swift b/TablePro/Core/Storage/AppSettingsManager.swift index e809ef815..e10d774be 100644 --- a/TablePro/Core/Storage/AppSettingsManager.swift +++ b/TablePro/Core/Storage/AppSettingsManager.swift @@ -146,18 +146,36 @@ final class AppSettingsManager { let remoteChanged = mcp.allowRemoteConnections != oldValue.allowRemoteConnections let authChanged = mcp.requireAuthentication != oldValue.requireAuthentication if enabledChanged || portChanged || remoteChanged || authChanged { - let settings = mcp - Task { [mcpServerManager] in - if settings.enabled { - await mcpServerManager.restart(port: UInt16(clamping: settings.port)) - } else { - await mcpServerManager.stop() - } + if mcp.enabled { + mcpServerManager.scheduleRestart(port: UInt16(clamping: mcp.port)) + } else { + mcpServerManager.scheduleStop() } } } } + @MainActor + func setRequireAuthentication(_ value: Bool) async -> (token: MCPAuthToken, plaintext: String)? { + guard value, !mcp.requireAuthentication else { + mcp.requireAuthentication = value + return nil + } + + let tokenStore = MCPTokenStore() + await tokenStore.loadFromDisk() + let existing = await tokenStore.list().filter { $0.name != MCPTokenStore.stdioBridgeTokenName } + guard existing.isEmpty else { + mcp.requireAuthentication = value + return nil + } + + let defaultName = String(localized: "Default token") + let result = await tokenStore.generate(name: defaultName, permissions: .fullAccess) + mcp.requireAuthentication = value + return result + } + @ObservationIgnored private let storage: AppSettingsStorage @ObservationIgnored private let themeEngine: ThemeEngine @ObservationIgnored private let syncTracker: SyncChangeTracker diff --git a/TablePro/Views/Settings/Sections/MCPSection.swift b/TablePro/Views/Settings/Sections/MCPSection.swift index b9f1a1944..f54e71c36 100644 --- a/TablePro/Views/Settings/Sections/MCPSection.swift +++ b/TablePro/Views/Settings/Sections/MCPSection.swift @@ -4,6 +4,7 @@ import SwiftUI struct MCPSection: View { @Binding var settings: MCPSettings @State private var manager = MCPServerManager.shared + @State private var settingsManager = AppSettingsManager.shared @State private var tokenList: [MCPAuthToken] = [] @State private var showSetupSheet = false @State private var showCreateSheet = false @@ -11,6 +12,23 @@ struct MCPSection: View { @State private var revealedToken: MCPAuthToken? @State private var revealedPlaintext: String = "" + private var requireAuthBinding: Binding { + Binding( + get: { settings.requireAuthentication }, + set: { newValue in + Task { @MainActor in + let bootstrap = await settingsManager.setRequireAuthentication(newValue) + if let bootstrap { + revealedToken = bootstrap.token + revealedPlaintext = bootstrap.plaintext + showRevealSheet = true + } + await refreshTokens() + } + } + ) + } + var body: some View { Section(String(localized: "Integrations")) { Toggle(String(localized: "Enable MCP Server"), isOn: $settings.enabled) @@ -72,7 +90,7 @@ struct MCPSection: View { private var authenticationSection: some View { Section(String(localized: "Authentication")) { - Toggle(String(localized: "Require authentication"), isOn: $settings.requireAuthentication) + Toggle(String(localized: "Require authentication"), isOn: requireAuthBinding) if settings.requireAuthentication { MCPTokenListView( diff --git a/TablePro/Views/Settings/Sections/MCPTokenCreateSheet.swift b/TablePro/Views/Settings/Sections/MCPTokenCreateSheet.swift index 113f93658..11df697bf 100644 --- a/TablePro/Views/Settings/Sections/MCPTokenCreateSheet.swift +++ b/TablePro/Views/Settings/Sections/MCPTokenCreateSheet.swift @@ -1,6 +1,10 @@ import SwiftUI struct MCPTokenCreateSheet: View { + private enum Field: Hashable { + case name + } + @Environment(\.dismiss) private var dismiss let onGenerate: (String, TokenPermissions, Set?, Date?) -> Void @@ -11,6 +15,7 @@ struct MCPTokenCreateSheet: View { @State private var expirationOption: ExpirationOption = .never @State private var customExpirationDate = Calendar.current.date(byAdding: .day, value: 30, to: .now) ?? .now @State private var connections: [DatabaseConnection] = [] + @FocusState private var focused: Field? var body: some View { VStack(spacing: 0) { @@ -29,6 +34,7 @@ struct MCPTokenCreateSheet: View { .padding() } .frame(minWidth: 480, minHeight: 520) + .defaultFocus($focused, .name) .task { connections = ConnectionStorage.shared.loadConnections() } @@ -37,6 +43,7 @@ struct MCPTokenCreateSheet: View { private var nameSection: some View { Section(String(localized: "Token Name")) { TextField(String(localized: "e.g., Claude Code on VPS"), text: $tokenName) + .focused($focused, equals: .name) } } diff --git a/TableProTests/Core/MCP/Auth/MCPCompositeAuthenticatorTests.swift b/TableProTests/Core/MCP/Auth/MCPCompositeAuthenticatorTests.swift new file mode 100644 index 000000000..0fb08b9cd --- /dev/null +++ b/TableProTests/Core/MCP/Auth/MCPCompositeAuthenticatorTests.swift @@ -0,0 +1,113 @@ +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +@Suite("MCP Composite Authenticator") +struct MCPCompositeAuthenticatorTests { + private func makeValidated(label: String = "test") -> MCPValidatedToken { + MCPValidatedToken( + tokenId: UUID(), + label: label, + scopes: [.toolsRead, .toolsWrite], + issuedAt: Date(timeIntervalSince1970: 1_000_000), + expiresAt: nil + ) + } + + private func makeBearer(_ store: FakeMCPTokenStore) -> MCPBearerTokenAuthenticator { + MCPBearerTokenAuthenticator(tokenStore: store, rateLimiter: MCPRateLimiter()) + } + + @Test("Loopback + auth disabled + no header allows anonymous") + func loopbackAuthDisabledNoHeader() async { + let store = FakeMCPTokenStore() + let composite = MCPCompositeAuthenticator(bearer: makeBearer(store), requireAuthentication: false) + let decision = await composite.authenticate(authorizationHeader: nil, clientAddress: .loopback) + guard case .allow(let principal) = decision else { + Issue.record("Expected allow, got \(decision)") + return + } + #expect(principal == MCPPrincipal.anonymousLoopback) + #expect(principal.scopes.contains(.toolsWrite)) + #expect(principal.scopes.contains(.admin)) + } + + @Test("Loopback + auth disabled + invalid bearer still allows anonymous") + func loopbackAuthDisabledIgnoresBearer() async { + let store = FakeMCPTokenStore() + let composite = MCPCompositeAuthenticator(bearer: makeBearer(store), requireAuthentication: false) + let decision = await composite.authenticate( + authorizationHeader: "Bearer tp_invalidtoken", + clientAddress: .loopback + ) + guard case .allow(let principal) = decision else { + Issue.record("Expected allow, got \(decision)") + return + } + #expect(principal == MCPPrincipal.anonymousLoopback) + } + + @Test("Loopback + auth required + valid bearer allows token principal") + func loopbackAuthRequiredValidToken() async { + let store = FakeMCPTokenStore() + let plaintext = "tp_realtoken" + let validated = makeValidated(label: "Token A") + await store.register(plaintext, validated: validated) + let composite = MCPCompositeAuthenticator(bearer: makeBearer(store), requireAuthentication: true) + let decision = await composite.authenticate( + authorizationHeader: "Bearer \(plaintext)", + clientAddress: .loopback + ) + guard case .allow(let principal) = decision else { + Issue.record("Expected allow, got \(decision)") + return + } + #expect(principal.metadata.label == "Token A") + #expect(principal != MCPPrincipal.anonymousLoopback) + } + + @Test("Loopback + auth required + missing header denies") + func loopbackAuthRequiredMissingHeader() async { + let store = FakeMCPTokenStore() + let composite = MCPCompositeAuthenticator(bearer: makeBearer(store), requireAuthentication: true) + let decision = await composite.authenticate(authorizationHeader: nil, clientAddress: .loopback) + guard case .deny(let reason) = decision else { + Issue.record("Expected deny, got \(decision)") + return + } + #expect(reason.httpStatus == 401) + } + + @Test("Remote + auth disabled still requires bearer") + func remoteAuthDisabledStillRequiresBearer() async { + let store = FakeMCPTokenStore() + let composite = MCPCompositeAuthenticator(bearer: makeBearer(store), requireAuthentication: false) + let decision = await composite.authenticate( + authorizationHeader: nil, + clientAddress: .remote("192.168.1.5") + ) + guard case .deny(let reason) = decision else { + Issue.record("Expected deny on remote even with auth disabled, got \(decision)") + return + } + #expect(reason.httpStatus == 401) + } + + @Test("Remote + auth required + valid bearer allows token principal") + func remoteAuthRequiredValidToken() async { + let store = FakeMCPTokenStore() + let plaintext = "tp_remote_token" + await store.register(plaintext, validated: makeValidated(label: "Remote Token")) + let composite = MCPCompositeAuthenticator(bearer: makeBearer(store), requireAuthentication: true) + let decision = await composite.authenticate( + authorizationHeader: "Bearer \(plaintext)", + clientAddress: .remote("192.168.1.5") + ) + guard case .allow(let principal) = decision else { + Issue.record("Expected allow, got \(decision)") + return + } + #expect(principal.metadata.label == "Remote Token") + } +} diff --git a/docs/features/mcp.mdx b/docs/features/mcp.mdx index 903c0de3c..3cf807505 100644 --- a/docs/features/mcp.mdx +++ b/docs/features/mcp.mdx @@ -72,6 +72,8 @@ For manual configuration, see [MCP Clients](/external-api/mcp-clients). The HTTP Tokens are managed under **Settings > Integrations > Authentication**. The pairing flow, scopes, allowlists, expiry, and revocation are documented in [Tokens](/external-api/tokens). +When **Require authentication** is off and the server is bound to localhost only, requests from your own machine are accepted without a bearer token. Remote connections always require a token. Turning **Require authentication** on for the first time creates a default token, writes it to disk, and shows it once so you can copy it; existing tokens are left untouched. + The activity log under **Settings > Integrations > Activity Log** shows every authentication, tool call, and resource read with the token id, category, action, connection, and outcome. Entries are kept for 90 days. ## Remote access From a9dea236da26d151946c0b08c7c0fd1b7e5fe8ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Thu, 28 May 2026 13:05:46 +0700 Subject: [PATCH 2/3] refactor(mcp): address review feedback on the auth-required toggle --- TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift | 3 --- TablePro/Core/Storage/AppSettingsManager.swift | 6 ++++-- TablePro/Views/Settings/Sections/MCPSection.swift | 4 ++++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift b/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift index 44ccd3f6b..4fadd5ed1 100644 --- a/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift +++ b/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift @@ -1,9 +1,6 @@ import Foundation -import os public actor MCPCompositeAuthenticator: MCPAuthenticator { - private static let logger = Logger(subsystem: "com.TablePro", category: "MCP.Auth") - private let bearer: MCPBearerTokenAuthenticator private let requireAuthentication: Bool diff --git a/TablePro/Core/Storage/AppSettingsManager.swift b/TablePro/Core/Storage/AppSettingsManager.swift index e10d774be..582710c1a 100644 --- a/TablePro/Core/Storage/AppSettingsManager.swift +++ b/TablePro/Core/Storage/AppSettingsManager.swift @@ -162,8 +162,10 @@ final class AppSettingsManager { return nil } - let tokenStore = MCPTokenStore() - await tokenStore.loadFromDisk() + let tokenStore = mcpServerManager.tokenStore ?? MCPTokenStore() + if mcpServerManager.tokenStore == nil { + await tokenStore.loadFromDisk() + } let existing = await tokenStore.list().filter { $0.name != MCPTokenStore.stdioBridgeTokenName } guard existing.isEmpty else { mcp.requireAuthentication = value diff --git a/TablePro/Views/Settings/Sections/MCPSection.swift b/TablePro/Views/Settings/Sections/MCPSection.swift index f54e71c36..d558a95e9 100644 --- a/TablePro/Views/Settings/Sections/MCPSection.swift +++ b/TablePro/Views/Settings/Sections/MCPSection.swift @@ -11,12 +11,16 @@ struct MCPSection: View { @State private var showRevealSheet = false @State private var revealedToken: MCPAuthToken? @State private var revealedPlaintext: String = "" + @State private var isAuthBootstrapping = false private var requireAuthBinding: Binding { Binding( get: { settings.requireAuthentication }, set: { newValue in + guard !isAuthBootstrapping else { return } + isAuthBootstrapping = true Task { @MainActor in + defer { isAuthBootstrapping = false } let bootstrap = await settingsManager.setRequireAuthentication(newValue) if let bootstrap { revealedToken = bootstrap.token From 4c5a0d1574dccf7c41ca01c01fdf79320db385c7 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 28 May 2026 18:30:53 +0700 Subject: [PATCH 3/3] refactor(mcp): scope anonymous loopback principal to the composite authenticator (#1093) --- .../MCP/Auth/MCPCompositeAuthenticator.swift | 13 +++++++- TablePro/Core/MCP/Auth/MCPPrincipal.swift | 11 ------- .../Views/Settings/Sections/MCPSection.swift | 30 ++++++++++--------- .../Auth/MCPCompositeAuthenticatorTests.swift | 8 +++-- 4 files changed, 33 insertions(+), 29 deletions(-) diff --git a/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift b/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift index 4fadd5ed1..b21c27376 100644 --- a/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift +++ b/TablePro/Core/MCP/Auth/MCPCompositeAuthenticator.swift @@ -4,6 +4,17 @@ public actor MCPCompositeAuthenticator: MCPAuthenticator { private let bearer: MCPBearerTokenAuthenticator private let requireAuthentication: Bool + private static let anonymousLoopbackPrincipal = MCPPrincipal( + tokenFingerprint: "anonymous-loopback", + tokenId: nil, + scopes: [.toolsRead, .toolsWrite, .resourcesRead, .admin], + metadata: MCPPrincipalMetadata( + label: "Anonymous (loopback)", + issuedAt: .distantPast, + expiresAt: nil + ) + ) + public init( bearer: MCPBearerTokenAuthenticator, requireAuthentication: Bool @@ -18,7 +29,7 @@ public actor MCPCompositeAuthenticator: MCPAuthenticator { ) async -> MCPAuthDecision { if !requireAuthentication, case .loopback = clientAddress { MCPAuditLogger.logAuthAllowedAnonymous(ip: "127.0.0.1") - return .allow(MCPPrincipal.anonymousLoopback) + return .allow(Self.anonymousLoopbackPrincipal) } return await bearer.authenticate( authorizationHeader: authorizationHeader, diff --git a/TablePro/Core/MCP/Auth/MCPPrincipal.swift b/TablePro/Core/MCP/Auth/MCPPrincipal.swift index 2e443d761..d4de96724 100644 --- a/TablePro/Core/MCP/Auth/MCPPrincipal.swift +++ b/TablePro/Core/MCP/Auth/MCPPrincipal.swift @@ -48,15 +48,4 @@ public struct MCPPrincipal: Sendable, Equatable, Hashable { hasher.combine(tokenFingerprint) hasher.combine(tokenId) } - - public static let anonymousLoopback = MCPPrincipal( - tokenFingerprint: "anonymous-loopback", - tokenId: nil, - scopes: [.toolsRead, .toolsWrite, .resourcesRead, .admin], - metadata: MCPPrincipalMetadata( - label: "Anonymous (loopback)", - issuedAt: .distantPast, - expiresAt: nil - ) - ) } diff --git a/TablePro/Views/Settings/Sections/MCPSection.swift b/TablePro/Views/Settings/Sections/MCPSection.swift index d558a95e9..7ad36fb60 100644 --- a/TablePro/Views/Settings/Sections/MCPSection.swift +++ b/TablePro/Views/Settings/Sections/MCPSection.swift @@ -16,23 +16,25 @@ struct MCPSection: View { private var requireAuthBinding: Binding { Binding( get: { settings.requireAuthentication }, - set: { newValue in - guard !isAuthBootstrapping else { return } - isAuthBootstrapping = true - Task { @MainActor in - defer { isAuthBootstrapping = false } - let bootstrap = await settingsManager.setRequireAuthentication(newValue) - if let bootstrap { - revealedToken = bootstrap.token - revealedPlaintext = bootstrap.plaintext - showRevealSheet = true - } - await refreshTokens() - } - } + set: { applyRequireAuthentication($0) } ) } + private func applyRequireAuthentication(_ newValue: Bool) { + guard !isAuthBootstrapping else { return } + isAuthBootstrapping = true + Task { @MainActor in + defer { isAuthBootstrapping = false } + let bootstrap = await settingsManager.setRequireAuthentication(newValue) + if let bootstrap { + revealedToken = bootstrap.token + revealedPlaintext = bootstrap.plaintext + showRevealSheet = true + } + await refreshTokens() + } + } + var body: some View { Section(String(localized: "Integrations")) { Toggle(String(localized: "Enable MCP Server"), isOn: $settings.enabled) diff --git a/TableProTests/Core/MCP/Auth/MCPCompositeAuthenticatorTests.swift b/TableProTests/Core/MCP/Auth/MCPCompositeAuthenticatorTests.swift index 0fb08b9cd..0a223ff7b 100644 --- a/TableProTests/Core/MCP/Auth/MCPCompositeAuthenticatorTests.swift +++ b/TableProTests/Core/MCP/Auth/MCPCompositeAuthenticatorTests.swift @@ -28,7 +28,8 @@ struct MCPCompositeAuthenticatorTests { Issue.record("Expected allow, got \(decision)") return } - #expect(principal == MCPPrincipal.anonymousLoopback) + #expect(principal.tokenId == nil) + #expect(principal.tokenFingerprint == "anonymous-loopback") #expect(principal.scopes.contains(.toolsWrite)) #expect(principal.scopes.contains(.admin)) } @@ -45,7 +46,8 @@ struct MCPCompositeAuthenticatorTests { Issue.record("Expected allow, got \(decision)") return } - #expect(principal == MCPPrincipal.anonymousLoopback) + #expect(principal.tokenId == nil) + #expect(principal.tokenFingerprint == "anonymous-loopback") } @Test("Loopback + auth required + valid bearer allows token principal") @@ -64,7 +66,7 @@ struct MCPCompositeAuthenticatorTests { return } #expect(principal.metadata.label == "Token A") - #expect(principal != MCPPrincipal.anonymousLoopback) + #expect(principal.tokenId != nil) } @Test("Loopback + auth required + missing header denies")