From 842c83b649783d3f33dc9479738df63cf740f1ad Mon Sep 17 00:00:00 2001 From: Blink WPT Bot Date: Fri, 3 Apr 2020 08:14:58 -0700 Subject: [PATCH] [webauthn] Fix resident key credentials.get() WPT (#22576) Fix the empty allowCredentials credentials.get() WPT. The test was expecting the browser to return a non-resident credential with an undefined allowCredentials list (which defaults to empty). Moreover, the test helper itself was failing because it was expecting a credential to be added to the test. Instead, move the test to its own file that sets up the test environment to support resident keys and add rk support to the helper with the isResidentKeyTest flag that avoids appending credentials to allowCredentials. Bug: 875444 Change-Id: I8baefa3a74c2a707227df430712a09935c1fbbf1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2130671 Reviewed-by: Martin Kreichgauer Commit-Queue: Nina Satragno Cr-Commit-Position: refs/heads/master@{#755344} Co-authored-by: Nina Satragno --- webauthn/getcredential-passing.https.html | 4 -- webauthn/getcredential-rk-passing.https.html | 48 ++++++++++++++++++++ webauthn/helpers.js | 29 ++++++++++-- 3 files changed, 72 insertions(+), 9 deletions(-) create mode 100644 webauthn/getcredential-rk-passing.https.html diff --git a/webauthn/getcredential-passing.https.html b/webauthn/getcredential-passing.https.html index c5237d2cda27e5..1af35aa9cb1432 100644 --- a/webauthn/getcredential-passing.https.html +++ b/webauthn/getcredential-passing.https.html @@ -34,10 +34,6 @@ .addCredential(credPromise) .runTest("passing credentials.get() with rpId (hostname)"); - // allowCredentials - new GetCredentialsTest({path: "options.publicKey.allowCredentials", value: undefined}) - .runTest("no credential specified"); - // authnr selection user verification new GetCredentialsTest({path: "options.publicKey.userVerification", value: undefined}) .addCredential(credPromise) diff --git a/webauthn/getcredential-rk-passing.https.html b/webauthn/getcredential-rk-passing.https.html new file mode 100644 index 00000000000000..8c0254fee42584 --- /dev/null +++ b/webauthn/getcredential-rk-passing.https.html @@ -0,0 +1,48 @@ + + +WebAuthn credential.get() Resident Key Passing Tests + + + + + + + + + diff --git a/webauthn/helpers.js b/webauthn/helpers.js index 27abaaf4766c4a..c355d0586257b8 100644 --- a/webauthn/helpers.js +++ b/webauthn/helpers.js @@ -35,6 +35,10 @@ var createCredentialDefaultArgs = { alg: cose_alg_ECDSA_w_SHA256, }], + authenticatorSelection: { + requireResidentKey: false, + }, + timeout: 60000, // 1 minute excludeCredentials: [] // No excludeList } @@ -420,6 +424,9 @@ class GetCredentialsTest extends TestCase { this.credentialPromiseList = []; + // set to true to pass an empty allowCredentials list to credentials.get + this.isResidentKeyTest = false; + // enable the constructor to modify the default testObject // would prefer to do this in the super class, but have to call super() before using `this.*` if (arguments.length) { @@ -464,7 +471,9 @@ class GetCredentialsTest extends TestCase { type: "public-key" }; }); - this.testObject.options.publicKey.allowCredentials = idList; + if (!this.isResidentKeyTest) { + this.testObject.options.publicKey.allowCredentials = idList; + } // return super.test(desc); }) .catch((err) => { @@ -476,6 +485,11 @@ class GetCredentialsTest extends TestCase { validatePublicKeyCredential(ret); validateAuthenticatorAssertionResponse(ret.response); } + + setIsResidentKeyTest(isResidentKeyTest) { + this.isResidentKeyTest = isResidentKeyTest; + return this; + } } /** @@ -535,12 +549,17 @@ function validateAuthenticatorAssertionResponse(assert) { // TODO: parseAuthenticatorData() and make sure flags are correct } -function standardSetup(cb) { +function standardSetup(cb, options = {}) { // Setup an automated testing environment if available. - window.test_driver.add_virtual_authenticator({ + let authenticatorArgs = { protocol: "ctap1/u2f", - transport: "usb" - }).then(authenticator => { + transport: "usb", + hasResidentKey: false, + hasUserVerification: false, + isUserVerified: false, + }; + extendObject(authenticatorArgs, options); + window.test_driver.add_virtual_authenticator(authenticatorArgs).then(authenticator => { cb(); // XXX add a subtest to clean up the virtual authenticator since // testharness does not support waiting for promises on cleanup.