Helper to find the RP ID given users' devices and current origin #2729
Conversation
|
@LXIF @sea-snake please review |
| "Not possible. Devices must be registered for at least one of the following domains: ic0.app, internetcomputer.org, icp0.io" | ||
| ); | ||
| } catch (error) { | ||
| console.error(error); |
There was a problem hiding this comment.
Instead of wrapping the whole findWebAuthnRpId function in a try/catch just to console log any errors, maybe the caller of findWebAuthnRpId can console log the error instead to avoid the need for the try/catch?
Since re-throwing an error from a catch block is considered an anti pattern as far as I'm aware.
There was a problem hiding this comment.
Yes, I initially added it wanting to return something different. But then I decided against it and the try/catch was left. I'll remove it.
lmuntaner
force-pushed
the
lm-determine-web-auth-rp-id
branch
from
9c75fce to
9cb4f27
Compare
Thanks! Did you wanted to add a comment only or approve the PR? |
My bad, friday brained. Approved now. |
* Helper to determine the webauthn RP ID * Improve tests * Rename * Improve comments * Apply suggestions from code review Co-authored-by: sea-snake <[email protected]> * Remove try/catch --------- Co-authored-by: sea-snake <[email protected]>
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Motivation
We want to support user registering in multiple domains as well as logging in them.
In this PR, I introduce a helper to find the RP ID given a set of devices and current origin.
Changes
findWebAuthnRpId.Tests
🟡 Some screens were changed