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

Feat: Deterministic Passkey Generation ENG-487 #23

Merged
merged 15 commits into from
Sep 13, 2024

Conversation

HashMapsData2Value
Copy link
Contributor

@HashMapsData2Value HashMapsData2Value commented Sep 6, 2024

ℹ Overview

Introduces: https://github.com/algorandfoundation/deterministic-P256-kt

Summary: Adds the ability to set a mnemonic, generating passkeys deterministically based on the origin and user id.

In detail:

  • Adds DerivedSecrets repo/db, a database made specifically to store derivedSecret keys and mnemonics. Though it is not necessary to do so, and we could instead only store the former.
  • Imports the deterministic passkey library, dP256.
  • With the help of derivedSecretParent key, all passkeys are generate deterministically from the userId, the
  • Adds PassKeyMnemonicFragment, a dialogue that pops up and prompts the user to provide a mnemonic or. to generate a 24 word one.
  • Makes it so that the Notification fragment does not get "cancelled" when the user presses elsewhere on the screen.

✅ Acceptance:

  • Conventional Commits

@HashMapsData2Value HashMapsData2Value added the enhancement New feature or request label Sep 6, 2024
@HashMapsData2Value HashMapsData2Value self-assigned this Sep 6, 2024
@HashMapsData2Value HashMapsData2Value changed the title WIP: Deterministic Passkey Generation ENG-487 Feat: Deterministic Passkey Generation ENG-487 Sep 10, 2024
@HashMapsData2Value HashMapsData2Value marked this pull request as ready for review September 10, 2024 02:05
@HashMapsData2Value HashMapsData2Value marked this pull request as draft September 10, 2024 02:16
@HashMapsData2Value
Copy link
Contributor Author

HashMapsData2Value commented Sep 10, 2024

Michael and I did a test run. Unfortunately, when you wish to use passkeys to sign into Github, there's no step where you input your email and then GitHub responds with the UserId. Instead, it just presents a "sign in with passkey" button, which when you connect your phone, will prompt it for the Credential Id. However, if you've just re-installed Liquid Auth, it will be empty, mnemonic or no, there will be no passkey Credential Id to present.

A solution is proposed as follows:

  • Switch from using the UserId to using the UserHandle to deterministically generate the passkeys (and in turn, Credential Id).
  • Add a "recovery/manual add" flow where you can specify 1) the UserHandle, 2) the Origin and a passkey will be created with the appropriate credential id.

This solves the problem. However, by replacing the UserId with the UserHandle we, we lose passkey continuity for a service where a user switches their UserHandle (e.g. their GitHub username) to a new one. If they try to recover Liquid Auth they will need to type in their old UserHandle to generate the passkey that is registered.

A UI solution could be as follows. If someone changes their UserHandle, the next time they do a sign in with passkeys, if we can presume that there will be a flow of information (indexed by the Credential Id) where the (new) UserHandle will be included and made available to LiquidAuth, it can check with the UserHandle stored on the phone. If it notices that the passkey was generated with the inclusion of an old UserHandle, it can flag that passkey as being "dirty" and encourage the user to create a new passkey - this one generated with the new UserHandle.

@HashMapsData2Value HashMapsData2Value marked this pull request as ready for review September 11, 2024 22:30
Copy link
Member

@PhearZero PhearZero left a comment

Choose a reason for hiding this comment

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

Tested against the current app and the staged refactor.

Both worked well and recovery went through as expected for third party sites

@HashMapsData2Value HashMapsData2Value merged commit 56754a8 into develop Sep 13, 2024
1 check passed
PhearZero pushed a commit that referenced this pull request Sep 13, 2024
* feat: first demo of P256

* feat: deterministically generate cred id

* feat: new dP256 jar file with BC

* fix: does signing with java.security

* chore: rename method name

* fix: problems with merge

* feat: only ask about mnemonic the first time

* feat: separate the storing of derived secrets from credentials

* fix: make dialogues better, not as cancellable, etc

* chore. remove comments

* feat: changes from UserId --> UserHandle and introduces a manual recreation button

* chore: replace dP256 jar with one that has userHandle instead of userId

* fix: take lowercase of userHandle to avoid confusion

* chore: UI fixes
engineering-ci bot pushed a commit that referenced this pull request Sep 13, 2024
# [1.0.0-canary.6](v1.0.0-canary.5...v1.0.0-canary.6) (2024-09-13)

### Features

* Deterministic Passkey Generation ENG-487 ([#23](#23)) ([f271bc4](f271bc4))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants