Skip to content
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

Switch sha256 to be synchronous in @turnkey/webauthn-stamper #179

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

r-n-o
Copy link
Contributor

@r-n-o r-n-o commented Nov 28, 2023

Summary & Motivation

Bug discovered by @SimonVillage in tkhq/demo-passkey-wallet#3: on ios/safari, cancelling the passkey prompt results in subsequent prompts automatically failing. The reason why: ios is stricter than other browsers when deciding whether a passkey prompt can be triggered or not. This blog post has more information on this, but TL;DR: you need to have a maximum of one async promise between a user gesture (tap, click) and the code triggering a passkey prompt.

In our SDK we use crypto.subtle.digest to compute the SHA256 of the message to sign:

const hashBuffer = await crypto.subtle.digest("SHA-256", messageBuffer);

This means we have an async promise between a button click and the call to navigator.credentials.get, causing iOS to fail. The fix? Use a synchonous version of sha256. @noble/hashes is a great one

How I Tested These Changes

Used the federated passkey example on my localhost, and reached the demo through an ngrok tunnel. I was able to repro the issue to start with, and observe it fixed after I replaced sha256 with the synchronous variant.

Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link
Contributor

@oliviathet oliviathet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woah really interesting edge case (kinda seems like it could more common than an edge case). either way, this looks great! 🔥 🔥

@r-n-o r-n-o merged commit 1146884 into main Nov 28, 2023
5 checks passed
@r-n-o r-n-o deleted the rno/switch-to-synchronous-sha256 branch November 28, 2023 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants