From 6f88b1d48591c54cd420c6a22d8c7deaa48b87a2 Mon Sep 17 00:00:00 2001
From: Brandon-T <bthomas@brave.com>
Date: Fri, 18 Oct 2019 04:54:05 -0400
Subject: [PATCH] Fix #1585 YubiKey presentation for "required" and "preferred"
 (#1608)

---
 Client/WebAuthN/U2FExtensions.swift           |  2 +-
 .../WebAuthnAuthenticateRequest.swift         | 18 ++++++++---
 ClientTests/U2FTests.swift                    | 32 ++++++++++++++++++-
 3 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/Client/WebAuthN/U2FExtensions.swift b/Client/WebAuthN/U2FExtensions.swift
index 4dc4bdfe19e..df6ec6b560a 100644
--- a/Client/WebAuthN/U2FExtensions.swift
+++ b/Client/WebAuthN/U2FExtensions.swift
@@ -536,7 +536,7 @@ class U2FExtensions: NSObject {
             getAssertionRequest.clientDataHash = clientDataHash
             
             getAssertionRequest.options = [
-                YKFKeyFIDO2GetAssertionRequestOptionUP: request.userPresence,
+                YKFKeyFIDO2GetAssertionRequestOptionUP: request.userPresence
             ]
 
             var allowList = [YKFFIDO2PublicKeyCredentialDescriptor]()
diff --git a/Client/WebAuthN/WebAuthnAuthenticateRequest.swift b/Client/WebAuthN/WebAuthnAuthenticateRequest.swift
index dff14adb68e..fe4b5a60215 100644
--- a/Client/WebAuthN/WebAuthnAuthenticateRequest.swift
+++ b/Client/WebAuthN/WebAuthnAuthenticateRequest.swift
@@ -3,11 +3,18 @@
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 import Shared
 
+enum WebAuthnUserVerification: String, Codable {
+    case required
+    case preferred
+    case discouraged
+}
+
 struct WebAuthnAuthenticateRequest {
     var rpID: String?
     var challenge: String
     var allowCredentials: [String] = []
-    var userPresence: Bool
+    let userPresence: Bool
+    var userVerification: WebAuthnUserVerification
 
     enum RequestKeys: String, CodingKey {
         case publicKey
@@ -35,9 +42,12 @@ extension WebAuthnAuthenticateRequest: Decodable {
         rpID = try publicKeyDictionary.decodeIfPresent(String.self, forKey: .rpId)
         challenge = try publicKeyDictionary.decode(String.self, forKey: .challenge)
         
-        // userPresence is the inverse of userVerification, UP by default is true
-        let userVerifcationString = try publicKeyDictionary.decodeIfPresent(String.self, forKey: .userVerification) ?? "discouraged"
-        userPresence = userVerifcationString == "discouraged"
+        // As of the latest spec changes, this valid will always be true!
+        // https://github.com/w3c/webauthn/pull/1140/files
+        userPresence = true
+        
+        // As a result of the above, we need to ensure that the default value is preferred
+        userVerification = try publicKeyDictionary.decodeIfPresent(WebAuthnUserVerification.self, forKey: .userVerification) ?? .preferred
         
         let allowCredentialsArray = try publicKeyDictionary.decode([AllowCredentials].self, forKey: .allowCredentials)
     
diff --git a/ClientTests/U2FTests.swift b/ClientTests/U2FTests.swift
index a2c94700ba8..d67a7aa59d7 100644
--- a/ClientTests/U2FTests.swift
+++ b/ClientTests/U2FTests.swift
@@ -57,7 +57,37 @@ class U2FTests: XCTestCase {
             XCTAssertEqual(request.challenge, "mdMbxTPACurawWFHqkoltSUwDear2OZQVl/uhBNqiaM=", "request challenge is correct.")
             XCTAssertEqual(request.allowCredentials.count, 1, "request allowCredential count is correct")
             XCTAssertEqual(request.allowCredentials.first, "OvQO5490o1w89Op/9dp4w7VvKuLEk5NHcfOnc2ZECtc=", "request allowCredential is correct")
-            XCTAssertTrue(request.userPresence, "request userPresence is correct")
+            XCTAssertTrue(request.userPresence, "request userPresence is correct") //User presence must always be true now!
+        } catch {
+            XCTFail("\(error)")
+        }
+    }
+    
+    func testWebAuthnUserVerification() {
+        do {
+            var payloads = [String]()
+            let verifications = ["required", "preferred", "discouraged"]
+            let expected: [WebAuthnUserVerification] = [.required, .preferred, .discouraged, .preferred]
+            
+            // Test expected verifications
+            for verification in verifications {
+                let payload = "{\"publicKey\":{\"allowCredentials\":[{\"type\":\"public-key\",\"id\":\"OvQO5490o1w89Op/9dp4w7VvKuLEk5NHcfOnc2ZECtc=\"}],\"rpId\":\"demo.brave.com\",\"timeout\":90000,\"userVerification\":\"\(verification)\",\"challenge\":\"mdMbxTPACurawWFHqkoltSUwDear2OZQVl/uhBNqiaM=\"},\"signal\":{}}"
+                payloads.append(payload)
+            }
+            
+            // Test when userPresence is MISSING from the payload..
+        payloads.append("{\"publicKey\":{\"allowCredentials\":[{\"type\":\"public-key\",\"id\":\"OvQO5490o1w89Op/9dp4w7VvKuLEk5NHcfOnc2ZECtc=\"}],\"rpId\":\"demo.brave.com\",\"timeout\":90000,\"challenge\":\"mdMbxTPACurawWFHqkoltSUwDear2OZQVl/uhBNqiaM=\"},\"signal\":{}}")
+            
+            for i in 0..<payloads.count {
+                guard let jsonData = payloads[i].data(using: String.Encoding.utf8) else {
+                    XCTFail("Failed parsing webauthn authentication data")
+                    return
+                }
+                
+                let request =  try JSONDecoder().decode(WebAuthnAuthenticateRequest.self, from: jsonData)
+                
+                XCTAssertTrue(request.userVerification == expected[i], "request userPresence is correct")
+            }
         } catch {
             XCTFail("\(error)")
         }