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

CredentialsContainer.create() & .get() should be able to return PublicKeyCredential, and the PKC interface should be more well-defined #60641

Open
scarletcs opened this issue Nov 29, 2024 · 0 comments

Comments

@scarletcs
Copy link

scarletcs commented Nov 29, 2024

⚙ Compilation target

ESNext or ES2024

(I am building for current up-to-date browsers.)

⚙ Library

DOM

Missing / Incorrect Definition

CredentialsContainer.create() and CredentialsContainer.get() currently always return a Promise<Credential | null> regardless of config. This is incomplete and less helpful than it could be.

In actual implementation, if a publicKey config is specified (and one type of credential config must be specified for both methods), both methods will return a PublicKeyCredential (or null). Currently we're forced to downcast to the correct type, e.g.:

const credential = await CredentialsContainer.create({ publicKey: { ... } });
if (credential) {
  const pkc = credential as PublicKeyCredential;
  // do things with pkc
}

Additionally, the PublicKeyCredential type definition is incomplete.

  • Instances are missing the pkc.toJSON() helper method.
    • Browser support still varies, so leaving it out might be deliberate. Firefox and Chrome support it; Safari does not, Opera is developing it, and all device WebViews do not support it.
  • The pkc.response field currently always has type AuthenticatorResponse. This is incomplete. Per MDN:
    • if the PublicKeyCredential was obtained via create(), the response field will be of type AuthenticatorAttestationResponse (attestation)
    • if it was obtained via get(), the response field will be of type AuthenticatorAssertionResponse (assertion).

Both AuthenticatorAttestationResponse and AuthenticatorAssertionResponse are already defined in lib.dom.d.ts.

Recommendation

PublicKeyCredential should instead be defined with a generic property that accepts the response type, e.g.:

interface PublicKeyCredential<Response extends AuthenticatorResponse = AuthenticatorResponse> {
  response: Response,
  // (other fields omitted)
}

Then create() and get() should be overloaded methods approximately like the following. All the types referenced below already exist in lib.dom.d.ts.

interface CredentialsContainer {
  create(options?: CredentialCreationOptions & { publicKey: PublicKeyCredentialCreationOptions }): Promise<PublicKeyCredential<AuthenticatorAttestationResponse> | null>;
  create(options?: CredentialCreationOptions): Promise<Credential | null>;

  get(options?: CredentialRequestOptions & { publicKey: PublicKeyCredentialRequestOptions }): Promise<PublicKeyCredential<AuthenticatorAssertionResponse> | null>;
  get(options?: CredentialRequestOptions): Promise<Credential | null>;
}

Sample Code

const toUint8Array = (str: string) => Uint8Array.from(str, c => c.charCodeAt(0));

const credential = await navigator.credentials.create({
  publicKey: {
    challenge: toUint8Array("random challenge from server"),
    rp: {
      name: "WebAuthN",
      id: "webauth.io", // Run this code on the webauthn.io domain, or change this to another domain.
    },
    user: {
      id: toUint8Array("username"),
      name: "Name",
      displayName: "Display name",
    },
    pubKeyCredParams: [
      { alg: -7, type: "public-key" },
    ],
    attestation: "direct",
  },
});

if (credential) {
  console.debug(credential.toJSON());
  //                       ~~~~~~
  // Property 'toJSON' does not exist on type 'Credential'.ts(2339)

  console.debug(credential.response);
  //                       ~~~~~~~~
  // Property 'response' does not exist on type 'Credential'.ts(2339)

  console.debug((credential as PublicKeyCredential).response.getAuthenticatorData());
  //                                                         ~~~~~~~~~~~~~~~~~~~~
  // Property 'getAuthenticatorData' does not exist on type 'AuthenticatorResponse'.ts(2339)
  
  // Currently forced to downcast twice like this essentially, which TypeScript permits:
  const pkc = credential as (PublicKeyCredential & { response: AuthenticatorAttestationResponse });
  console.debug(pkc.response.getAuthenticatorData());
}

Documentation Link

@scarletcs scarletcs changed the title CredentialsContainer.create() and CredentialsContainer.get() should be able to return PublicKeyCredential, and the PKC interface should be more well-defined CredentialsContainer.create() & .get() should be able to return PublicKeyCredential, and the PKC interface should be more well-defined Nov 29, 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

No branches or pull requests

1 participant