Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #1585 YubiKey presentation for "required" and "preferred" (#1608)
Browse files Browse the repository at this point in the history
  • Loading branch information
Brandon-T authored and iccub committed Oct 18, 2019
1 parent 08907f9 commit 6f88b1d
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 6 deletions.
2 changes: 1 addition & 1 deletion Client/WebAuthN/U2FExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ class U2FExtensions: NSObject {
getAssertionRequest.clientDataHash = clientDataHash

getAssertionRequest.options = [
YKFKeyFIDO2GetAssertionRequestOptionUP: request.userPresence,
YKFKeyFIDO2GetAssertionRequestOptionUP: request.userPresence
]

var allowList = [YKFFIDO2PublicKeyCredentialDescriptor]()
Expand Down
18 changes: 14 additions & 4 deletions Client/WebAuthN/WebAuthnAuthenticateRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
32 changes: 31 additions & 1 deletion ClientTests/U2FTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
}
Expand Down

0 comments on commit 6f88b1d

Please sign in to comment.