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

biometric-ed25519 update to support BitWarden password manager #1331

Merged
merged 3 commits into from
Apr 9, 2024

Conversation

hcho112
Copy link
Contributor

@hcho112 hcho112 commented Apr 6, 2024

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • Commit messages follow the conventional commits spec
  • If this is a code change: I have written unit tests.
  • If this changes code in a published package: I have run pnpm changeset to create a changeset JSON document appropriate for this change.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

This PR contains fixes for #1330 issue. The main reason why passkey did not work with BitWarden was because they do not follow standard format for webauthn credentials in which they used data format of Uint8Array instead of ArrayBuffer. However they can be easily convert-able so in this PR I have implemented logic to do the conversion.

Test Plan

On https://github.com/near/fast-auth-signer/ repo, run it locally with commend yarn && NETWORK_ID=testnet yarn run start --https (Bitwarden only works with https)
On new Chrome browser profile, install Bitwarden chrome extension package.
manually navigate to localhost:3000/create-account and try to create an account.
(At this point, you should be able to replicate the issues)

Apply the above change directly to source code. (There is issues with setting up yarn link/npm link between these two projects)

I have confirm that above change has resolved the issue and was able to conduct create key and get keys without error.

@hcho112 hcho112 requested a review from andy-haynes April 6, 2024 01:02
Copy link

changeset-bot bot commented Apr 6, 2024

🦋 Changeset detected

Latest commit: 30e3b34

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@near-js/biometric-ed25519 Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@hcho112 hcho112 requested a review from vikinatora April 6, 2024 01:10
@hcho112 hcho112 force-pushed the support-bitwarden-webauthn branch from bd9a970 to bc2e982 Compare April 7, 2024 20:53
Copy link
Collaborator

@vikinatora vikinatora left a comment

Choose a reason for hiding this comment

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

The code looks good, I've left only a few minor styling and wording remarks

packages/biometric-ed25519/src/utils.ts Outdated Show resolved Hide resolved
packages/biometric-ed25519/src/utils.ts Outdated Show resolved Hide resolved
packages/biometric-ed25519/src/utils.ts Outdated Show resolved Hide resolved
packages/biometric-ed25519/src/index.ts Outdated Show resolved Hide resolved
packages/biometric-ed25519/src/index.ts Outdated Show resolved Hide resolved
@hcho112
Copy link
Contributor Author

hcho112 commented Apr 8, 2024

@vikinatora thank you for the review, updated PR as per request

@hcho112 hcho112 requested a review from vikinatora April 8, 2024 21:58
@vikinatora vikinatora merged commit 14256aa into master Apr 9, 2024
1 check passed
@vikinatora vikinatora deleted the support-bitwarden-webauthn branch April 9, 2024 06:02
@github-actions github-actions bot mentioned this pull request Apr 9, 2024
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