fix: Remove ArrayBuffer checks from WebAuthnIdentity #857
Conversation
This PR removes `instanceof ArrayBuffer` checks from the `WebAuthnIdentity`. The reason is that these checks hide errors and cause problem in conjunction with the Bitwarden password manager (which sets these fields to `Uint8Array`). See also this issue: dfinity/internet-identity#2235 Simply removing the checks solved the issue entirely, as the fields that supposedly need to be `ArrayBuffer` are converted to `Uint8Array` anyway. This change was tested with Internet Identity and the Bitwarden extension in Chrome.
frederikrothenberger
left a comment
frederikrothenberger
left a comment
There was a problem hiding this comment.
Thanks, these changes work too!
| throw new Error('Could not create credentials.'); | ||
| } | ||
|
|
||
| const response = creds.response as AuthenticatorAttestationResponse; |
There was a problem hiding this comment.
Aren't you now potentially casting an undefined to an AuthenticatorAttestationResponse, given that the snippet you removed also checked for creds.response === undefined? In other words, aren't you potentially facing uncaught undefined issues in the following lines?
There was a problem hiding this comment.
None of the typing suggests that creds.response can be undefined, given the other checks we've done to make sure that _createCredential yields PublicKeyCredentialWithAttachment | null
There was a problem hiding this comment.
So this undefined test was useless?
if (creds.response === undefined || !(creds.rawId instanceof ArrayBuffer)) {
Good then, thanks for the feedback.
There was a problem hiding this comment.
It may have been. We have also updated the version of Typescript and other browser typings since this was written, so something I'm unaware of may have changed in the meantime
3 participants
Description
This PR removes
instanceof ArrayBufferchecks from theWebAuthnIdentity. The reason is that these checks hide errors and cause problem in conjunction with the BitWarden password manager (which sets these fields toUint8Array). See also this issue: dfinity/internet-identity#2235Simply removing the checks solved the issue entirely, as the fields that supposedly need to be
ArrayBufferare converted toUint8Arrayanyway.How Has This Been Tested?
This change was manually tested with Internet Identity and the BitWarden extension in Chrome.
Checklist: