Skip to content

Commit

Permalink
[chromedriver] Flatten webauthn command responses
Browse files Browse the repository at this point in the history
Update WebAuthn command responses to match the spec:

* AddVirtualAuthenticator should return `authenticatorId` instead of an
  object containing `authenticatorId`.
* GetCredentials should return an array instead of an object containing
  an array of credentials.

Bug: 922572
Change-Id: Id7fc36eca79ae7624c662d8674be0f62a7df72e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1841239
Commit-Queue: John Chen <[email protected]>
Auto-Submit: Nina Satragno <[email protected]>
Reviewed-by: John Chen <[email protected]>
Cr-Commit-Position: refs/heads/master@{#703340}
  • Loading branch information
nsatragno authored and Commit Bot committed Oct 7, 2019
1 parent c46e04f commit 0b1df3d
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 17 deletions.
24 changes: 12 additions & 12 deletions test/run_py_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2468,19 +2468,19 @@ def testRemoveVirtualAuthenticator(self):
self._driver.RemoveVirtualAuthenticator, 'id')

# Create an authenticator and try removing it.
response = self._driver.AddVirtualAuthenticator(
authenticatorId = self._driver.AddVirtualAuthenticator(
protocol = 'ctap2',
transport = 'usb',
hasResidentKey = False,
hasUserVerification = False,
)
self._driver.RemoveVirtualAuthenticator(response['authenticatorId'])
self._driver.RemoveVirtualAuthenticator(authenticatorId)

# Trying to remove the same authenticator should fail.
self.assertRaisesRegexp(
chromedriver.InvalidArgument,
'Could not find a Virtual Authenticator matching the ID',
self._driver.RemoveVirtualAuthenticator, response['authenticatorId'])
self._driver.RemoveVirtualAuthenticator, authenticatorId)

def testAddCredential(self):

Expand All @@ -2500,7 +2500,7 @@ def testAddCredential(self):
transport = 'usb',
hasResidentKey = False,
hasUserVerification = False,
)['authenticatorId']
)

# Register a credential and try authenticating with it.
self._driver.AddCredential(
Expand All @@ -2525,7 +2525,7 @@ def testAddCredentialBase64Errors(self):
transport = 'usb',
hasResidentKey = False,
hasUserVerification = False,
)['authenticatorId']
)

# Try adding a credentialId that is encoded in vanilla base64.
self.assertRaisesRegexp(
Expand Down Expand Up @@ -2560,15 +2560,15 @@ def testGetCredentials(self):
hasResidentKey = True,
hasUserVerification = True,
isUserVerified = True,
)['authenticatorId']
)

# Register a credential via the webauthn API.
result = self._driver.ExecuteAsyncScript(script)
self.assertEquals('OK', result['status'])
credentialId = result['credential']['id']

# GetCredentials should return the credential that was just created.
credentials = self._driver.GetCredentials(authenticatorId)['credentials']
credentials = self._driver.GetCredentials(authenticatorId)
self.assertEquals(1, len(credentials))
self.assertEquals(credentialId, credentials[0]['credentialId'])
self.assertEquals(True, credentials[0]['isResidentCredential'])
Expand All @@ -2588,7 +2588,7 @@ def testRemoveCredential(self):
authenticatorId = self._driver.AddVirtualAuthenticator(
protocol = 'ctap2',
transport = 'usb',
)['authenticatorId']
)

# Register two credentials.
result = self._driver.ExecuteAsyncScript(script)
Expand All @@ -2600,12 +2600,12 @@ def testRemoveCredential(self):
credential2Id = result['credential']['id']

# GetCredentials should return both credentials.
credentials = self._driver.GetCredentials(authenticatorId)['credentials']
credentials = self._driver.GetCredentials(authenticatorId)
self.assertEquals(2, len(credentials))

# Removing the first credential should leave only the first one.
self._driver.RemoveCredential(authenticatorId, credential1Id)
credentials = self._driver.GetCredentials(authenticatorId)['credentials']
credentials = self._driver.GetCredentials(authenticatorId)
self.assertEquals(1, len(credentials))
self.assertEquals(credential2Id, credentials[0]['credentialId'])

Expand All @@ -2619,7 +2619,7 @@ def testRemoveAllCredentials(self):
authenticatorId = self._driver.AddVirtualAuthenticator(
protocol = 'ctap2',
transport = 'usb',
)['authenticatorId']
)

# Register a credential via the webauthn API.
result = self._driver.ExecuteAsyncScript(register_credential_script)
Expand Down Expand Up @@ -2665,7 +2665,7 @@ def testSetUserVerified(self):
transport = 'usb',
hasResidentKey = True,
hasUserVerification = True,
)['authenticatorId']
)

# Configure the virtual authenticator to fail user verification.
self._driver.SetUserVerified(authenticatorId, False)
Expand Down
28 changes: 23 additions & 5 deletions webauthn_commands.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
namespace {

static constexpr char kBase64UrlError[] = " must be a base64url encoded string";
static constexpr char kDevToolsDidNotReturnExpectedValue[] =
"DevTools did not return the expected value";

// Creates a base::DictionaryValue by cloning the parameters specified by
// |mapping| from |params|.
Expand Down Expand Up @@ -119,8 +121,19 @@ Status ExecuteAddVirtualAuthenticator(WebView* web_view,
if (protocol && *protocol == "ctap1/u2f")
*protocol = "u2f";

return web_view->SendCommandAndGetResult("WebAuthn.addVirtualAuthenticator",
std::move(mapped_params), value);
std::unique_ptr<base::Value> result;
Status status = web_view->SendCommandAndGetResult(
"WebAuthn.addVirtualAuthenticator", std::move(mapped_params), &result);
if (status.IsError())
return status;

base::Optional<base::Value> authenticator_id =
result->ExtractKey("authenticatorId");
if (!authenticator_id)
return Status(kUnknownError, kDevToolsDidNotReturnExpectedValue);

*value = std::make_unique<base::Value>(std::move(*authenticator_id));
return status;
}

Status ExecuteRemoveVirtualAuthenticator(WebView* web_view,
Expand Down Expand Up @@ -158,17 +171,22 @@ Status ExecuteAddCredential(WebView* web_view,
Status ExecuteGetCredentials(WebView* web_view,
const base::Value& params,
std::unique_ptr<base::Value>* value) {
std::unique_ptr<base::Value> result;
Status status = web_view->SendCommandAndGetResult(
"WebAuthn.getCredentials",
MapParams({{"authenticatorId", "authenticatorId"}}, params), value);
MapParams({{"authenticatorId", "authenticatorId"}}, params), &result);
if (status.IsError())
return status;

for (base::Value& credential : (*value)->FindKey("credentials")->GetList()) {
base::Optional<base::Value> credentials = result->ExtractKey("credentials");
if (!credentials)
return Status(kUnknownError, kDevToolsDidNotReturnExpectedValue);

for (base::Value& credential : credentials->GetList()) {
ConvertBase64ToBase64Url(&credential,
{"credentialId", "privateKey", "userHandle"});
}

*value = std::make_unique<base::Value>(std::move(*credentials));
return status;
}

Expand Down

0 comments on commit 0b1df3d

Please sign in to comment.