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

fix/528-simplify-use-of-credential-id #529

Merged
merged 7 commits into from
Feb 25, 2024

Conversation

MasterKale
Copy link
Owner

@MasterKale MasterKale commented Feb 23, 2024

This PR attempts to make @simplewebauthn/server simpler to use by keeping credentialID a string throughout registration and authentication. This PR contains the following breaking changes!

Fixes #528.

Registration

  1. excludeCredentials when calling generateRegistrationOptions() expects id to be a string. type is no longer necessary here either.
  2. registrationInfo.credentialID returned from verifyRegistrationResponse() is now the base64url-encoded credential ID string instead of a Uint8Array.

Authentication

  1. allowCredentials when calling generateAuthenticationOptions() expects id to be a string. type is no longer necessary here either.
  2. authenticationInfo.credentialID returned from verifyAuthenticationResponse() is now the base64url-encoded credential ID string instead of a Uint8Array.
  3. AuthenticatorDevice.credentialID (in @simplewebauthn/types) is now a base64url-encoded string instead of a Uint8Array. This will break existing use of the authenticator argument in verifyAuthenticationResponse().

Helpers

  1. isoBase64URL.isBase64url() is now called isoBase64URL.isBase64URL() (note the "URL" in the method name)

Rationale

The idea here is that, in practice, it makes a lot of sense for a given credential ID to be persisted as the base64url-encoded string. This makes it easier, for example, to confirm which credentials are making it to the WebAuthn API call, to copy-paste the ID from a WebAuthn response to look up the credential in the RP's database, to share the credential ID with others...in all of these, strings help the developer discern one credential from another.

Thinking about the developer experience, let's start with an RP dev wiring up registration for a user. First they generate WebAuthn registration options:

const opts = generateRegistrationOptions({ ... });

Then the options make it to WebAuthn via @simplewebauthn/browser, and a response is sent back to be verified:

// Assume the response succeeded verification
const verification = verifyRegistrationResponse({ ... });
// Imagine `credentialID` is of type `string` now
const { credentialID } = verification.registrationInfo;

Later, the user comes back to authenticate with a passkey. The RP dev first generates authentication options, specifying the user's registered credential:

const opts = generateAuthenticationOptions({
  // ...
  // `credentialID` is still `string`, no need for a helper to get it to `Uint8Array`
  allowCredentials: [{ id: credentialID }],
});

When verifying the authentication response, the credential ID can again be used as-is with no helpers needed:

const verification = await verifyAuthenticationResponse({
  // ...
  authenticator: {
    // ...
    // No need to use any helpers here either
    credentialID,
  },
});

Thinking about how credentialID might get persisted across various SQL and NoSQL solutions, strings are so easy to implement, query, and think about that most users of this library are probably already storing credentialID as a string, but needing to maintain code that goes to and from base64url string and bytes. I think in my gut this PR will simplify use of this library even further by removing the need to remember how and when to go between the two formats.

Yes, the spec says these values are bytes, but this is a library intended to make it simpler to implement WebAuthn. This PR feels like a great direction to take things in the spirit of that.

@MasterKale MasterKale added package:server @simplewebauthn/server package:types @simplewebauthn/typescript-types labels Feb 23, 2024
@MasterKale MasterKale added this to the v10.0.0 milestone Feb 23, 2024
@MasterKale MasterKale merged commit 6eb5ac6 into master Feb 25, 2024
2 checks passed
@MasterKale MasterKale deleted the fix/528-simplify-use-of-credential-id branch February 25, 2024 00:45
@MasterKale MasterKale removed this from the v10.0.0 milestone Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:server @simplewebauthn/server package:types @simplewebauthn/typescript-types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve ergonomics of excludeCredentials and allowCredentials
1 participant