Merge pull request #62 from nodes-vapor/feature/filter-headers

cweinberger · web-flow · commit 663a84f03240 · 2020-02-26T16:49:56.000+01:00
Add tests for the keyFilter feature &amp; apply filter everywhere
diff --git a/README.md b/README.md
@@ -115,6 +115,20 @@ enum BreadcrumbType {
 }
 ```
 
+#### Filter out fields from reports
+Usually you will receive information such as headers, query params or post body fields in the reports from Bugsnag. To ensure that you do not track sensitive information, you can configure Bugsnag with a list of fields that should be filtered out:
+
+```swift
+BugsnagConfig(
+    apiKey: "apiKey",
+    releaseStage: "test",
+    keyFilters: ["password", "email", "authorization", "lastname"]
+)
+```
+In this case Bugsnag Reports won't contain header fields, query params or post body json fields with the keys/names **password**, **email**, **authorization**, **lastname**.
+
+⚠️ Note: in JSON bodies, this only works for the first level of fields and not for nested children.
+
 ## 🏆 Credits
 
 This package is developed and maintained by the Vapor team at [Nodes](https://www.nodesagency.com).
diff --git a/Sources/Bugsnag/Bugsnag.swift b/Sources/Bugsnag/Bugsnag.swift
@@ -33,7 +33,7 @@ struct BugsnagEvent: Encodable {
         breadcrumbs: [BugsnagBreadcrumb],
         error: Error,
         httpRequest: HTTPRequest? = nil,
-        keyFilters: [String],
+        keyFilters: Set<String>,
         metadata: [String: CustomDebugStringConvertible],
         payloadVersion: String,
         severity: Severity,
@@ -109,16 +109,17 @@ struct BugsnagRequest: Encodable {
     let referer: String
     let url: String
 
-    init(httpRequest: HTTPRequest, keyFilters: [String]) {
+    init(httpRequest: HTTPRequest, keyFilters: Set<String>) {
         self.body = BugsnagRequest.filter(httpRequest.body, using: keyFilters)
         self.clientIp = httpRequest.remotePeer.hostname
-        self.headers = Dictionary(httpRequest.headers.map { $0 }) { first, second in second }
+        let filteredHeaders = BugsnagRequest.filter(httpRequest.headers, using: keyFilters)
+        self.headers = Dictionary(filteredHeaders.map { $0 }) { first, second in second }
         self.httpMethod = httpRequest.method.string
         self.referer = httpRequest.remotePeer.description
-        self.url = httpRequest.urlString
+        self.url = BugsnagRequest.filter(httpRequest.urlString, using: keyFilters)
     }
 
-    static private func filter(_ body: HTTPBody, using filters: [String]) -> String? {
+    static private func filter(_ body: HTTPBody, using filters: Set<String>) -> String? {
         guard
             let data = body.data,
             let unwrap = try? JSONSerialization.jsonObject(with: data) as? [String: Any],
@@ -131,6 +132,23 @@ struct BugsnagRequest: Encodable {
         let json = try? JSONSerialization.data(withJSONObject: filtered, options: [.prettyPrinted])
         return json.flatMap { String(data: $0, encoding: .utf8) }
     }
+
+    static private func filter(_ headers: HTTPHeaders, using filters: Set<String>) -> HTTPHeaders {
+        var mutableHeaders = headers
+        filters.forEach { mutableHeaders.remove(name: $0) }
+        return mutableHeaders
+    }
+
+    /**
+     @discussion Currently returns the original (unfiltered) url if anything goes wrong.
+     */
+    static private func filter(_ urlString: String, using filters: Set<String>) -> String {
+        guard var urlComponents = URLComponents(string: urlString) else {
+            return urlString
+        }
+        urlComponents.queryItems?.removeAll(where: { filters.contains($0.name) })
+        return urlComponents.string ?? urlString
+    }
 }
 
 struct BugsnagThread: Encodable {
diff --git a/Sources/Bugsnag/BugsnagConfig.swift b/Sources/Bugsnag/BugsnagConfig.swift
@@ -3,7 +3,7 @@ public struct BugsnagConfig {
     let releaseStage: String
     /// A version identifier, (eg. a git hash)
     let version: String?
-    let keyFilters: [String]
+    let keyFilters: Set<String>
     let shouldReport: Bool
     let debug: Bool
 
@@ -18,7 +18,7 @@ public struct BugsnagConfig {
         self.apiKey = apiKey
         self.releaseStage = releaseStage
         self.version = version
-        self.keyFilters = keyFilters
+        self.keyFilters = Set(keyFilters)
         self.shouldReport = shouldReport
         self.debug = debug
     }
diff --git a/Tests/BugsnagTests/BugsnagTests.swift b/Tests/BugsnagTests/BugsnagTests.swift
@@ -55,4 +55,172 @@ final class BugsnagTests: XCTestCase {
         let request = Request(using: application)
         try reporter.report(NotFound(), on: request).wait()
     }
+
+    func testKeyFiltersWorkInRequestBody() throws {
+        var capturedSendReportParameters: (
+            host: String,
+            headers: HTTPHeaders,
+            body: Data,
+            container: Container
+        )?
+
+        let reporter = BugsnagReporter(
+            config: .init(apiKey: "apiKey", releaseStage: "test", keyFilters: ["password", "email"]),
+            sendReport: { host, headers, data, container in
+                capturedSendReportParameters = (host, headers, data, container)
+                return container.future(Response(http: HTTPResponse(status: .ok), using: container))
+            })
+        let application = try Application.test()
+        let request = Request(using: application)
+        request.http.method = .POST
+        request.http.body = TestBody.default.httpBody
+
+        _ = try! reporter.report(NotFound(), on: request).wait()
+
+        guard let params = capturedSendReportParameters else {
+            XCTFail()
+            return
+        }
+
+        let responseBody = try JSONDecoder().decode(BugsnagResponseBody<TestBody>.self, from: params.body)
+
+        guard let body = responseBody.events.first?.request?.body else {
+            XCTFail("Unable to parse request body")
+            return
+        }
+        XCTAssertNil(body.password, "test that password is removed")
+        XCTAssertNil(body.email, "test that email is removed")
+        XCTAssertEqual(body.hash, TestBody.default.hash, "test that hash is not altered")
+    }
+
+    func testKeyFiltersWorkInHeaderFields() throws {
+        var capturedSendReportParameters: (
+            host: String,
+            headers: HTTPHeaders,
+            body: Data,
+            container: Container
+        )?
+
+        let reporter = BugsnagReporter(
+            config: .init(apiKey: "apiKey", releaseStage: "test", keyFilters: ["password", "email"]),
+            sendReport: { host, headers, data, container in
+                capturedSendReportParameters = (host, headers, data, container)
+                return container.future(Response(http: HTTPResponse(status: .ok), using: container))
+            })
+        let application = try Application.test()
+        let request = Request(using: application)
+        request.http.method = .POST
+        request.http.body = TestBody.default.httpBody
+        var headers = request.http.headers
+        headers.add(name: HTTPHeaderName("password"), value: TestBody.default.password!)
+        headers.add(name: HTTPHeaderName("email"), value: TestBody.default.email!)
+        headers.add(name: HTTPHeaderName("hash"), value: TestBody.default.hash!)
+        request.http.headers = headers
+
+        _ = try! reporter.report(NotFound(), on: request).wait()
+
+        guard let params = capturedSendReportParameters else {
+            XCTFail()
+            return
+        }
+
+        let responseBody = try JSONDecoder().decode(BugsnagResponseBody<TestBody>.self, from: params.body)
+
+        guard let responseHeaders = responseBody.events.first?.request?.headers else {
+            XCTFail("Unable to parse response headers")
+            return
+        }
+
+        XCTAssertNil(responseHeaders["password"], "test that password is removed")
+        XCTAssertNil(responseHeaders["email"], "test that email is removed")
+        XCTAssertEqual(responseHeaders["hash"], TestBody.default.hash!, "test that hash is not altered")
+    }
+
+    func testKeyFiltersWorkInURLQueryParams() throws {
+        var capturedSendReportParameters: (
+            host: String,
+            headers: HTTPHeaders,
+            body: Data,
+            container: Container
+        )?
+
+        let reporter = BugsnagReporter(
+            config: .init(apiKey: "apiKey", releaseStage: "test", keyFilters: ["password", "email"]),
+            sendReport: { host, headers, data, container in
+                capturedSendReportParameters = (host, headers, data, container)
+                return container.future(Response(http: HTTPResponse(status: .ok), using: container))
+            })
+        let application = try Application.test()
+        let request = Request(using: application)
+        request.http.url = URL(string: "http://foo.bar.com/?password=\(TestBody.default.password!)&email=\(TestBody.default.email!)&hash=\(TestBody.default.hash!)")!
+        request.http.method = .POST
+        request.http.body = TestBody.default.httpBody
+        var headers = request.http.headers
+        headers.add(name: HTTPHeaderName("password"), value: TestBody.default.password!)
+        headers.add(name: HTTPHeaderName("email"), value: TestBody.default.email!)
+        headers.add(name: HTTPHeaderName("hash"), value: TestBody.default.hash!)
+        request.http.headers = headers
+
+        _ = try! reporter.report(NotFound(), on: request).wait()
+
+        guard let params = capturedSendReportParameters else {
+            XCTFail()
+            return
+        }
+
+        let responseBody = try JSONDecoder().decode(BugsnagResponseBody<TestBody>.self, from: params.body)
+
+        guard let responseURLString = responseBody.events.first?.request?.url else {
+            XCTFail("Unable to parse response url")
+            return
+        }
+
+        let urlComponents = URLComponents(string: responseURLString)
+        let passwordItem = urlComponents?.queryItems?.filter { $0.name == "password" }.last
+        let emailItem = urlComponents?.queryItems?.filter { $0.name == "email" }.last
+        let hashItem = urlComponents?.queryItems?.filter { $0.name == "hash" }.last
+        
+        XCTAssertNil(passwordItem, "test that password is removed")
+        XCTAssertNil(emailItem, "test that email is removed")
+        XCTAssertEqual(hashItem?.value, TestBody.default.hash!, "test that hash is not altered")
+    }
+}
+
+struct TestBody: Codable {
+    var password: String?
+    var email: String?
+    var hash: String?
+
+    static var `default`: TestBody {
+        return .init(password: "TopSecret", email: "foo@bar.com", hash: "myAwesomeHash")
+    }
+
+    var httpBody: HTTPBody {
+        return try! HTTPBody(data: JSONEncoder().encode(self))
+    }
+}
+
+struct BugsnagResponseBody<T: Codable>: Codable {
+    struct Event: Codable {
+        struct Request: Codable {
+            let body: T?
+            let headers: [String: String]?
+            let url: String?
+
+            // custom decoding needed as the format is JSON string (not JSON object)
+            init(from decoder: Decoder) throws {
+                let container = try decoder.container(keyedBy: CodingKeys.self)
+                let bodyString = try container.decode(String.self, forKey: .body)
+                guard let data = bodyString.data(using: .utf8) else {
+                    throw Abort(.internalServerError)
+                }
+                body = try JSONDecoder().decode(T.self, from: data)
+                headers = try container.decode(Dictionary.self, forKey: .headers)
+                url = try container.decode(String.self, forKey: .url)
+            }
+        }
+        let request: Request?
+    }
+    let apiKey: String
+    let events: [Event]
 }
diff --git a/Tests/BugsnagTests/XCTestManifests.swift b/Tests/BugsnagTests/XCTestManifests.swift
@@ -6,6 +6,9 @@ extension BugsnagTests {
     //   `swift test --generate-linuxmain`
     // to regenerate.
     static let __allTests__BugsnagTests = [
+        ("testKeyFiltersWorkInHeaderFields", testKeyFiltersWorkInHeaderFields),
+        ("testKeyFiltersWorkInRequestBody", testKeyFiltersWorkInRequestBody),
+        ("testKeyFiltersWorkInURLQueryParams", testKeyFiltersWorkInURLQueryParams),
         ("testReportingCanBeDisabled", testReportingCanBeDisabled),
         ("testSendReport", testSendReport),
     ]
