New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add note about decoding U2F keyhandles. #1272
Conversation
U2F key handles need to be base64url decoded before being used as ArrayBuffers in WebAuthn calls. This isn't completely obvious so this change adds a clarification to the appid and appidExclude extensions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, LGTM!
thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nevermind)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are just comments on @baloo's comments (github is not allowing me to make comments without it being construed as a "review"). [and then after I did a [submit review], baloo's comments (and mine) disappeared? oh well... ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, good catch @agl :)
I just suggest we link to our dfn of "base64url encoding".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thx @agl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems right.
U2F key handles need to be base64url decoded before being used as
ArrayBuffers in WebAuthn calls. This isn't completely obvious so this
change adds a clarification to the appid and appidExclude extensions.
Preview | Diff