Skip to content

Commit

Permalink
[webauthn] Fix resident key credentials.get() WPT (#22576)
Browse files Browse the repository at this point in the history
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 <martinkr@google.com>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755344}

Co-authored-by: Nina Satragno <nsatragno@chromium.org>
  • Loading branch information
chromium-wpt-export-bot and nsatragno committed Apr 3, 2020
1 parent 1422c81 commit 842c83b
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 9 deletions.
4 changes: 0 additions & 4 deletions webauthn/getcredential-passing.https.html
Expand Up @@ -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)
Expand Down
48 changes: 48 additions & 0 deletions webauthn/getcredential-rk-passing.https.html
@@ -0,0 +1,48 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>WebAuthn credential.get() Resident Key Passing Tests</title>
<meta name="timeout" content="long">
<link rel="help" href="hhttps://w3c.github.io/webauthn/#resident-credential">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src=helpers.js></script>
<body></body>
<script>
standardSetup(function() {
"use strict";

// create a resident key credential
var credPromise = createCredential({
options: {
publicKey: {
authenticatorSelection: {
requireResidentKey: true,
}
}
}
});

// empty allowCredential should find the requireResidentKey: true credential
new GetCredentialsTest({path: "options.publicKey.allowCredentials", value: []})
.addCredential(credPromise)
.setIsResidentKeyTest(true)
.runTest("empty allowCredentials");

// undefined allowCredential should be equivalent to empty
new GetCredentialsTest({path: "options.publicKey.allowCredentials", value: undefined})
.addCredential(credPromise)
.setIsResidentKeyTest(true)
.runTest("undefined allowCredentials");
}, {
// browsers may not allow resident key credential creation without uv
protocol: "ctap2",
hasResidentKey: true,
hasUserVerification: true,
isUserVerified: true,
});

/* JSHINT */
/* globals standardSetup, GetCredentialsTest, createCredential */
</script>
29 changes: 24 additions & 5 deletions webauthn/helpers.js
Expand Up @@ -35,6 +35,10 @@ var createCredentialDefaultArgs = {
alg: cose_alg_ECDSA_w_SHA256,
}],

authenticatorSelection: {
requireResidentKey: false,
},

timeout: 60000, // 1 minute
excludeCredentials: [] // No excludeList
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) => {
Expand All @@ -476,6 +485,11 @@ class GetCredentialsTest extends TestCase {
validatePublicKeyCredential(ret);
validateAuthenticatorAssertionResponse(ret.response);
}

setIsResidentKeyTest(isResidentKeyTest) {
this.isResidentKeyTest = isResidentKeyTest;
return this;
}
}

/**
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 842c83b

Please sign in to comment.