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

Refactor public key credential for tests #42

Conversation

Jubeki
Copy link
Contributor

@Jubeki Jubeki commented Apr 6, 2023

Just asking, can the publicKeyCredential() also be used in submitPasskeyBasedRegisterAttempt() they look similar, but I am not sure how they differ exactly.

@claudiodekker
Copy link
Owner

claudiodekker commented Apr 7, 2023

So, in theory, they're both WebAuthn Public Key Credentials. I've just used the "Public Key Credential" and "Passkey" terms to differentiate the use-cases.

First, the textbook differences:

  • Discoverable Credentials (Resident Keys): These credentials are stored on the authenticator itself, along with their associated metadata. The private key and a user handle containing a unique identifier (user.id) are stored in persistent memory on the authenticator during the registration process. During the authentication process, the authenticator returns the user handle, allowing the relying party (RP) to look up the associated user without requiring the user to enter a username. This enables passwordless, first-factor authentication and high-assurance multi-factor authentication with a single login step.
  • Non-Discoverable Credentials: These credentials are not stored on the authenticator. The user provides a username or another identifier during authentication, and the server sends a list of allowed credential descriptors (allowCredentials list) to the client. The authenticator then uses this list to find the appropriate private key to sign the authentication challenge. In this scenario, the authenticator doesn't need to store a per-site private key for every relying party, and the server's list of credential descriptors is used to locate the corresponding key.

As you might've gathered already, based on the above, Passkeys are Discoverable Credentials. It's mostly just a marketing-friendly rebrand to indicate they fully replace usernames/passwords.

However, there's also some limits, for example, when you have a Roaming Authenticator such as a Yubikey, and use it as a Passkey, you unfortunately risk running into a storage limit, as Yubikeys can only hold 25 "Resident Keys" (read: Discoveable Credentials) due to it being a hardware-based USB-stick-like device. This limit does not exist for non-discoverable credentials, and likely also doesn't exist on Platform Authenticators since they're stored on-device and could in theory be offloaded by the OS.

As such, I've split these two textbook differences up into two different terms in the code, and named them accordingly.

  • Passkeys: Discoverable Credentials [Limited to only allow Platform authenticators / exclude Yubikeys and other "Roaming" authenticators]
  • Public Key Credentials: Non-Discoverable credentials, used exclusively for Multi Factor Authentication, not limited in terms of authenticator type (since in this case, no Resident Key metadata is stored on the client).

While I agree the distinction isn't super clear, especially since it's re-using existing terms, I haven't had the time yet to rename "PublicKeyCredentials" yet, but here's some thoughts I had:

  • WebAuthnMfaCredential: This name highlights that these credentials are specific to WebAuthn/FIDO2 and are used for multi-factor authentication.
  • NonResidentCredential: This name emphasizes that these credentials are not stored on the authenticator (i.e., they are non-discoverable) and differentiates them from discoverable Passkey credentials.

The same applies to the MultiFactorCredential model: I use that same table to store Passkeys, so it being called MultiFactorCredential also isn't accurate. Perhaps AuthenticatorCredential would be a better model name.

In either case, I don't want to rename these now, as they're honestly just details that are easier to adjust once everything else is implemented (as to prevent merge conflicts on PR's / WIP stuff). It doesn't affect how the code is structured or ran, but at least you have an idea now with regards to what they represent!

In summary: there's to some extent a definite difference between the two. Feel free to use these new terms in the tests already, though.

Hope this explains!

@Jubeki
Copy link
Contributor Author

Jubeki commented Apr 7, 2023

Okay. I understood the difference!

I agree that it is a good idea to keep them different even if they mostly do similar things.

Renaming the MultiFactorCredential Ideas:

  • AuthenticationCredential (maybe)
  • KeyCredential (also saved TOTP)

@claudiodekker
Copy link
Owner

claudiodekker commented Apr 11, 2023

@Jubeki This one is ready for review as well, right? Or was it still work in progress?

@claudiodekker claudiodekker added the optimization Improvements that don't change functionality label Apr 18, 2023
@claudiodekker claudiodekker merged commit 7001df4 into claudiodekker:master Apr 18, 2023
github-actions bot pushed a commit that referenced this pull request Apr 18, 2023
@Jubeki Jubeki deleted the refactor-public-key-credential-for-tests branch April 19, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization Improvements that don't change functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants