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

Add Sign in with passkey Button #14577

Merged
merged 15 commits into from
Sep 25, 2024
Merged

Conversation

yunochi
Copy link
Contributor

@yunochi yunochi commented Sep 19, 2024

What

  • Instead of entering an ID and authenticating with a passkey, create a dedicated 'Login with Passkey' button.
  • idから入力してpasskey認証をする代わりに、「Passkeyでログインボタン」を作成します

#14574

Why

Password-less login accounts have the inconvenience of having to first enter an ID and then click the login button to authenticate the passkey.
In addition, multi-account users have the inconvenience of having to enter an ID and then select a passkey from the list.
Since Misskey already uses residentKey, it is possible to skip the ID entry process and log in directly by selecting a passkey.

パスワードレスログイン アカウントは、id を最初に入力してからログインボタンを押してパスキー認証をする不便があります。 さらに、複数のアカウントを持つユーザーは、id入力後にパスキーリストから選択する必要があるという不便さもあります。
MisskeyはすでにresidentKeyを使用しているため、idを入力するプロセスをスキップしてパスキーを選択してすぐにログインすることが可能です。

Additional info (optional)

  • WHY Not use conditional mediation
    • At the point when a passkey is requested, it is difficult to know whether the user has enabled 'Login without password'.
    • Passkeyを要求する時点では、そのユーザーが「パスワードなしでログイン」を有効にしたかどうかを知ることは困難です。

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added packages/frontend Client side specific issue/PR packages/backend Server side specific issue/PR packages/misskey-js labels Sep 19, 2024
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 13.39031% with 304 lines in your changes missing coverage. Please review.

Project coverage is 41.26%. Comparing base (fde94f6) to head (af2d939).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
...kend/src/server/api/SigninWithPasskeyApiService.ts 17.34% 143 Missing ⚠️
packages/frontend/src/components/MkSignin.vue 0.00% 86 Missing ⚠️
packages/backend/src/core/WebAuthnService.ts 16.25% 67 Missing ⚠️
...ackages/backend/src/server/api/ApiServerService.ts 22.22% 7 Missing ⚠️
...ackages/frontend/src/components/MkSigninDialog.vue 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14577      +/-   ##
===========================================
+ Coverage    39.58%   41.26%   +1.68%     
===========================================
  Files         1542     1547       +5     
  Lines       192801   198879    +6078     
  Branches      2522     2507      -15     
===========================================
+ Hits         76312    82067    +5755     
- Misses      115924   116247     +323     
  Partials       565      565              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Sep 19, 2024

このPRによるapi.jsonの差分
差分はありません。
Get diff files from Workflow Page

@yunochi yunochi marked this pull request as ready for review September 19, 2024 09:10
@syuilo syuilo requested a review from acid-chicken September 20, 2024 12:17
@yunochi yunochi force-pushed the sign-in-with-passkey branch from 2451597 to 5209a5c Compare September 20, 2024 12:27
yunochi and others added 12 commits September 24, 2024 09:20
Use specific rate limiting key: 'signin-with-passkey'  for passkey sign-in API to avoid collisions with signin rate-limit.
- Increased the rate limit for Passkey sign-in attempts to accommodate the two API calls needed per sign-in.
- Improved error messages and handling in both the `WebAuthnService` and the `SigninWithPasskeyApiService`, providing more context and better usability.
- Updated error messages to provide more specific and helpful details to the user.

These changes aim to enhance the Passkey sign-in experience by providing more robust error handling, improving security by limiting API calls, and delivering a more user-friendly interface.
- Separate the flow of 1FA and 2FA.
- Remove duplicate passkey buttons
@yunochi yunochi force-pushed the sign-in-with-passkey branch from 5209a5c to 61d077a Compare September 24, 2024 00:51
@syuilo
Copy link
Member

syuilo commented Sep 25, 2024

マージするか

locales/ja-JP.yml Outdated Show resolved Hide resolved
@syuilo
Copy link
Member

syuilo commented Sep 25, 2024

可能なら何らかのテストが欲しいところではあるわね

@syuilo syuilo added this to the v2024.9.0 milestone Sep 25, 2024
- update index.d.ts
- update ko-KR.yml, en-US.yml
- Fix: Reflect Changed i18n key on MkSignin
@Squarecat-meow
Copy link
Contributor

サジェストのlocales/ja-JP.ymlは反映しました!またフロントも反映したバージョンでコミットしました。
また、テストを作成しなかった理由はPasskeyのシミュレーションをどういうふうに作ればわからなかったので一応おいておきました。アイディアがありましたら共有していただけると幸いです。

@syuilo syuilo merged commit d8dd168 into misskey-dev:develop Sep 25, 2024
20 checks passed
@syuilo
Copy link
Member

syuilo commented Sep 25, 2024

👍🏻 👍🏻

HotoRas pushed a commit to HotoRas/misskey-neko that referenced this pull request Sep 27, 2024
* Sign in with passkey (PoC)

* 💄 Added "Login with Passkey" Button

* refactor: Improve error response when WebAuthn challenge fails

* signinResponse should be placed under the SigninWithPasskeyResponse object.

* Frontend fix

* Fix: Rate limiting key for passkey signin

Use specific rate limiting key: 'signin-with-passkey'  for passkey sign-in API to avoid collisions with signin rate-limit.

* Refactor: enhance Passkey sign-in flow and error handling

- Increased the rate limit for Passkey sign-in attempts to accommodate the two API calls needed per sign-in.
- Improved error messages and handling in both the `WebAuthnService` and the `SigninWithPasskeyApiService`, providing more context and better usability.
- Updated error messages to provide more specific and helpful details to the user.

These changes aim to enhance the Passkey sign-in experience by providing more robust error handling, improving security by limiting API calls, and delivering a more user-friendly interface.

* Refactor: Streamline 2FA flow and remove redundant Passkey button.

- Separate the flow of 1FA and 2FA.
- Remove duplicate passkey buttons

* Fix: Add error messages to MkSignin

* chore: Hide passkey button if the entered user does not use passkey login

* Update CHANGELOG.md

* Refactor: Rename functions and Add comments

* Update locales/ja-JP.yml

Co-authored-by: syuilo <[email protected]>

* Fix: Update translation

- update index.d.ts
- update ko-KR.yml, en-US.yml
- Fix: Reflect Changed i18n key on MkSignin

---------

Co-authored-by: Squarecat-meow <[email protected]>
Co-authored-by: syuilo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR packages/misskey-js
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants