diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bd731d9f..41e51636b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Installing or updating a plugin right after updating TablePro now refetches the current plugin list first, so it no longer fails against a stale cached list (the error a restart used to clear). (#1380) - Pressing Esc to close the Raw SQL filter suggestions, or to clear a search field, no longer also exits fullscreen. (#1403) +- Connecting an OAuth-capable MCP client like Claude Code with an invalid or expired token now shows a clear error instead of a confusing "Invalid OAuth error response". (#1409) ## [0.44.0] - 2026-05-23 diff --git a/TablePro/Core/MCP/Transport/MCPHttpConnectionContext.swift b/TablePro/Core/MCP/Transport/MCPHttpConnectionContext.swift index bae0ba1e0..ae655ae0e 100644 --- a/TablePro/Core/MCP/Transport/MCPHttpConnectionContext.swift +++ b/TablePro/Core/MCP/Transport/MCPHttpConnectionContext.swift @@ -147,12 +147,13 @@ actor HttpConnectionContext { await send(payload) } - func writePlainJsonResponse(status: HttpStatus, body: Data) async { + func writePlainJsonResponse(status: HttpStatus, body: Data, extraHeaders: [(String, String)] = []) async { if cancelled { return } var headers: [(String, String)] = [ ("Content-Type", "application/json"), ("Connection", "close") ] + headers.append(contentsOf: extraHeaders) headers.append(contentsOf: self.corsHeaders()) let head = HttpResponseHead(status: status, headers: HttpHeaders(headers)) let payload = HttpResponseEncoder.encode(head, body: body) @@ -165,6 +166,25 @@ actor HttpConnectionContext { await writePlainJsonResponse(status: status, body: payload) } + func writePlainJsonError( + status: HttpStatus, + error: String, + errorDescription: String, + extraHeaders: [(String, String)] = [] + ) async { + struct ErrorBody: Encodable { + let error: String + let errorDescription: String + enum CodingKeys: String, CodingKey { + case error + case errorDescription = "error_description" + } + } + let body = ErrorBody(error: error, errorDescription: errorDescription) + let payload = (try? JSONEncoder().encode(body)) ?? Data() + await writePlainJsonResponse(status: status, body: payload, extraHeaders: extraHeaders) + } + func writeOptions204() async { if cancelled { return } var headers: [(String, String)] = [("Connection", "close")] diff --git a/TablePro/Core/MCP/Transport/MCPHttpRequestRouter.swift b/TablePro/Core/MCP/Transport/MCPHttpRequestRouter.swift index df2aa5abf..5b6f09be4 100644 --- a/TablePro/Core/MCP/Transport/MCPHttpRequestRouter.swift +++ b/TablePro/Core/MCP/Transport/MCPHttpRequestRouter.swift @@ -42,15 +42,11 @@ struct MCPHttpRequestRouter: Sendable { case .delete: await handleDeleteMcp(head: head, context: context, clientAddress: clientAddress) default: - await respondTopLevel( - context: context, - error: MCPProtocolError( - code: JsonRpcErrorCode.methodNotFound, - message: "Method not allowed", - httpStatus: .methodNotAllowed - ), - requestId: nil - ) + if pathMatchesMcp(head.path) { + await respondHttpMethodNotAllowed(context: context) + } else { + await respondHttpNotFound(context: context) + } } } @@ -177,15 +173,7 @@ struct MCPHttpRequestRouter: Sendable { clientAddress: MCPClientAddress ) async { guard pathMatchesMcp(head.path) else { - await respondTopLevel( - context: context, - error: MCPProtocolError( - code: JsonRpcErrorCode.methodNotFound, - message: "Method not found", - httpStatus: .notFound - ), - requestId: nil - ) + await respondHttpNotFound(context: context) return } @@ -242,15 +230,7 @@ struct MCPHttpRequestRouter: Sendable { now: Date ) async { guard pathMatchesMcp(head.path) else { - await respondTopLevel( - context: context, - error: MCPProtocolError( - code: JsonRpcErrorCode.methodNotFound, - message: "Method not found", - httpStatus: .notFound - ), - requestId: nil - ) + await respondHttpNotFound(context: context) return } @@ -344,15 +324,7 @@ struct MCPHttpRequestRouter: Sendable { clientAddress: MCPClientAddress ) async { guard pathMatchesMcp(head.path) else { - await respondTopLevel( - context: context, - error: MCPProtocolError( - code: JsonRpcErrorCode.methodNotFound, - message: "Method not found", - httpStatus: .notFound - ), - requestId: nil - ) + await respondHttpNotFound(context: context) return } @@ -447,6 +419,25 @@ struct MCPHttpRequestRouter: Sendable { await context.cancel() } + private func respondHttpNotFound(context: HttpConnectionContext) async { + await context.writePlainJsonError( + status: .notFound, + error: "not_found", + errorDescription: "TablePro's MCP server does not provide this endpoint." + ) + await context.cancel() + } + + private func respondHttpMethodNotAllowed(context: HttpConnectionContext) async { + await context.writePlainJsonError( + status: .methodNotAllowed, + error: "method_not_allowed", + errorDescription: "This HTTP method is not supported.", + extraHeaders: [("Allow", "GET, POST, DELETE, OPTIONS")] + ) + await context.cancel() + } + private func pathMatchesMcp(_ path: String) -> Bool { let trimmed = stripQueryString(path) return trimmed == "/mcp" || trimmed == "/mcp/" diff --git a/TableProTests/Core/MCP/Transport/MCPHttpServerTransportTests.swift b/TableProTests/Core/MCP/Transport/MCPHttpServerTransportTests.swift index a2c495b80..967e28f4a 100644 --- a/TableProTests/Core/MCP/Transport/MCPHttpServerTransportTests.swift +++ b/TableProTests/Core/MCP/Transport/MCPHttpServerTransportTests.swift @@ -113,6 +113,42 @@ struct MCPHttpServerTransportTests { return (envelope.id, envelope.error.code, envelope.error.message) } + private func makeRequest(port: UInt16, path: String, method: String, body: Data? = nil) -> URLRequest { + guard let url = URL(string: "http://127.0.0.1:\(port)\(path)") else { + fatalError("Failed to construct test URL") + } + var request = URLRequest(url: url) + request.httpMethod = method + request.setValue("Bearer test", forHTTPHeaderField: "Authorization") + if let body { + request.httpBody = body + request.setValue("application/json", forHTTPHeaderField: "Content-Type") + } + return request + } + + private func decodeJsonObject(_ data: Data) throws -> [String: Any] { + guard let object = try JSONSerialization.jsonObject(with: data) as? [String: Any] else { + throw TestError.malformedJsonBody + } + return object + } + + private func expectPlainError( + data: Data, + response: URLResponse?, + status: Int, + error: String, + label: String + ) throws { + let http = try #require(response as? HTTPURLResponse, "\(label): expected an HTTP response") + #expect(http.statusCode == status, "\(label): expected status \(status)") + let object = try decodeJsonObject(data) + #expect(object["jsonrpc"] == nil, "\(label): must not be a JSON-RPC envelope") + #expect((object["error"] as? String) == error, "\(label): error must be the string \"\(error)\"") + #expect((object["error_description"] as? String)?.isEmpty == false, "\(label): needs an error_description") + } + private func runEchoLoop( transport: MCPHttpServerTransport, consumer: StubExchangeConsumer, @@ -336,8 +372,8 @@ struct MCPHttpServerTransportTests { #expect(http.statusCode == 413) } - @Test("Method not found at unknown path returns 404 with JSON-RPC error envelope") - func unknownPathReturns404() async throws { + @Test("Unknown HTTP paths return a plain 404, not a JSON-RPC envelope, so OAuth clients get a clear error") + func unknownPathsReturnPlainNotFound() async throws { let auth = StubAlwaysAllowAuthenticator() let (transport, _, port) = try await startedTransport(authenticator: auth) defer { Task { await transport.stop() } } @@ -346,19 +382,52 @@ struct MCPHttpServerTransportTests { await runEchoLoop(transport: transport, consumer: consumer) defer { Task { await consumer.stop() } } - guard let url = URL(string: "http://127.0.0.1:\(port)/foo") else { - Issue.record("Failed to construct URL") - return + let cases: [(path: String, method: String, body: Data?)] = [ + ("/foo", "GET", nil), + ("/foo", "POST", Data("{}".utf8)), + ("/foo", "DELETE", nil), + ("/foo", "PUT", Data("{}".utf8)), + ("/.well-known/oauth-protected-resource", "GET", nil), + ("/.well-known/oauth-authorization-server", "GET", nil), + ("/register", "POST", Data("{}".utf8)) + ] + + for testCase in cases { + let request = makeRequest(port: port, path: testCase.path, method: testCase.method, body: testCase.body) + let (data, response) = try await URLSession.shared.data(for: request) + try expectPlainError( + data: data, + response: response, + status: 404, + error: "not_found", + label: "\(testCase.method) \(testCase.path)" + ) } - var request = URLRequest(url: url) - request.httpMethod = "GET" - request.setValue("Bearer test", forHTTPHeaderField: "Authorization") + } + + @Test("Unsupported HTTP method returns a plain 405, not a JSON-RPC envelope") + func unsupportedMethodReturnsPlain405() async throws { + let auth = StubAlwaysAllowAuthenticator() + let (transport, _, port) = try await startedTransport(authenticator: auth) + defer { Task { await transport.stop() } } + + let consumer = StubExchangeConsumer() + await runEchoLoop(transport: transport, consumer: consumer) + defer { Task { await consumer.stop() } } + + let request = makeRequest(port: port, path: "/mcp", method: "PUT", body: Data("{}".utf8)) let (data, response) = try await URLSession.shared.data(for: request) + try expectPlainError( + data: data, + response: response, + status: 405, + error: "method_not_allowed", + label: "PUT /mcp" + ) let http = try #require(response as? HTTPURLResponse) - - #expect(http.statusCode == 404) - let parsed = try parseJsonRpcError(data) - #expect(parsed.code == JsonRpcErrorCode.methodNotFound) + let allow = http.value(forHTTPHeaderField: "Allow") + #expect(allow?.contains("POST") == true) + #expect(allow?.contains("GET") == true) } @Test("OPTIONS request returns 204 with CORS headers reflecting allowed origin") @@ -589,4 +658,5 @@ struct MCPHttpServerTransportTests { private enum TestError: Error { case serverDidNotStart case expectedErrorEnvelope + case malformedJsonBody }