Handle Authentication Errors (#14)

* Added special case handling for authentcation.

When the authentication sends an error, there is no second message, which causes the authentication promise to never fail or succeed.

This implementation is a bit "hacky" it adds the "errorMessageIsFinal" property to every `ConnectionHandler`.

* Update Sources/NIOPostgres/Connection/PostgresConnectionHandler.swift

Co-Authored-By: tbartelmess <[email protected]>

* Added "testInvalidPassword" to all tests

* Added warning

Author: tbartelmess

Sources/NIOPostgres/Connection/PostgresConnection+Authenticate.swift

@@ -24,7 +24,11 @@ private final class PostgresAuthenticationRequest: PostgresRequestHandler {
     let database: String?
     let password: String?
     var state: State
-    
+
+    var errorMessageIsFinal: Bool {
+        return true
+    }
+
     init(username: String, database: String?, password: String?) {
         self.state = .ready
         self.username = username

Sources/NIOPostgres/Connection/PostgresConnectionHandler.swift

@@ -11,6 +11,16 @@ extension PostgresConnection {
 public protocol PostgresRequestHandler {
     func respond(to message: PostgresMessage) throws -> [PostgresMessage]?
     func start() throws -> [PostgresMessage]
+
+    #warning("TODO: Workaround for Authentication see #14")
+    var errorMessageIsFinal: Bool { get }
+}
+
+
+extension PostgresRequestHandler {
+    var errorMessageIsFinal: Bool {
+        return false
+    }
 }
 
 // MARK: Private
@@ -48,7 +58,12 @@ final class PostgresConnectionHandler: ChannelDuplexHandler {
         switch message.identifier {
         case .error:
             let error = try PostgresMessage.Error(message: message)
-            request.error = PostgresError.server(error)
+            let postgresError = PostgresError.server(error)
+            if request.delegate.errorMessageIsFinal {
+                request.promise.fail(postgresError)
+                self.queue.removeFirst()
+            }
+            request.error = postgresError
         case .notice:
             let notice = try PostgresMessage.Error(message: message)
             print("[NIOPostgres] [NOTICE] \(notice)")

Tests/NIOPostgresTests/NIOPostgresTests.swift

@@ -284,4 +284,16 @@ final class NIOPostgresTests: XCTestCase {
             }
         }
     }
+
+    func testInvalidPassword() throws {
+        let auth = PostgresConnection.testUnauthenticated(on: eventLoop).flatMap({ (connection) in
+            connection.authenticate(username: "invalid", database: "invalid", password: "bad").map { connection }
+        })
+        do {
+            let _ = try auth.wait()
+            XCTFail("The authentication should fail")
+        } catch let error as PostgresError {
+           XCTAssertEqual(error.code, .invalid_password)
+        }
+    }
 }

Tests/NIOPostgresTests/Utilities.swift

@@ -2,22 +2,28 @@ import NIO
 import NIOPostgres
 
 extension PostgresConnection {
-    static func test(on eventLoop: EventLoop) -> EventLoopFuture<PostgresConnection> {
+    static func address() throws -> SocketAddress {
+        #if os(Linux)
+        return try .newAddressResolving(host: "psql", port: 5432)
+        #else
+        return try .init(ipAddress: "127.0.0.1", port: 5432)
+        #endif
+    }
+
+    static func testUnauthenticated(on eventLoop: EventLoop) -> EventLoopFuture<PostgresConnection> {
         do {
-            let address: SocketAddress
-            #if os(Linux)
-            address = try .newAddressResolving(host: "psql", port: 5432)
-            #else
-            address = try .init(ipAddress: "127.0.0.1", port: 5432)
-            #endif
-            return connect(to: address, on: eventLoop).flatMap { conn in
-                return conn.authenticate(username: "vapor_username", database: "vapor_database", password: "vapor_password")
-                    .map { conn }
-            }
+            return connect(to: try address(), on: eventLoop)
         } catch {
             return eventLoop.makeFailedFuture(error)
         }
     }
+
+    static func test(on eventLoop: EventLoop) -> EventLoopFuture<PostgresConnection> {
+        return testUnauthenticated(on: eventLoop).flatMap { conn in
+            return conn.authenticate(username: "vapor_username", database: "vapor_database", password: "vapor_password")
+                .map { conn }
+        }
+    }
 }
 
 

Tests/NIOPostgresTests/XCTestManifests.swift

@@ -17,6 +17,7 @@ extension NIOPostgresTests {
         ("testSelectPerformance", testSelectPerformance),
         ("testRangeSelectPerformance", testRangeSelectPerformance),
         ("testRangeSelectDecodePerformance", testRangeSelectDecodePerformance),
+        ("testInvalidPassword", testInvalidPassword),
     ]
 }