teamhanko / passkeys

FIDO2-certified passkey server and SDKs for adding passkey support to any auth system
https://hanko.io/passkey-api
GNU Affero General Public License v3.0
115 stars 8 forks source link

JS passkeys sdk typing issue for `user.displayName` property #32

Closed riccardoperra closed 4 months ago

riccardoperra commented 8 months ago

I'm trying to integrate Hanko passkeys via the passkey sdk https://www.npmjs.com/package/@teamhanko/passkeys-sdk, everything works smoothly but i'm having a type issue integrating it via https://github.com/github/webauthn-json

Consider that most of types used by that library are imported from the typescript browser types definitions "lib.dom.ts" .

I have a wip implementation of your api in a Fastify route where I have an updated schema which doesn't throw any ts error:

https://github.com/riccardoperra/codeimage/blob/5d5d525e0a0b60d618a3082edc71ff94a33e4655/apps/api/src/routes/v1/passkey/startRegistration.ts#L8-L80


Screenshot 2024-01-05 alle 17 06 18

The error is related to the response of the registration.initialize api, which returns as a displayName property string | undefined.

This seems incorrect to me since even if it's not passed in the backend, the "username" property will be used a a fallback.

This is an example of the backend implementation:

  const passkeysApi = tenant({
    tenantId: fastify.config.HANKO_PASSKEYS_TENANT_ID,
    apiKey: fastify.config.HANKO_PASSKEYS_API_KEY,
    baseUrl: fastify.config.HANKO_PASSKEYS_LOGIN_BASE_URL,
  });

  fastify.post('/registration', {schema}, async request => {
    const {appUser} = request;
    fastify.log.info(
      `Init passkey registration for user with id ${appUser.id}`,
    );
    return fastify.passkeysApi.registration.initialize({
      userId: appUser.id,
      username: appUser.email,
    });
  });

Here's an example of response which always contain the displayName property:

Screenshot 2024-01-05 alle 17 17 39

Is this something intentional or is it correct that the latter is potentially undefined? I took a look at the code, I don't know Go but I didn't find the portion of code where displayName is evaluated with the name property 🤔


There are also some differences between lib.dom.ts and some of your "string" types. Typescript uses a union of defined strings const which cause some error (I think only if you have ts strict mode enabled).

I'll list just some fo them:

PubKeyCredParams

it seems required instead of being (array | null)

https://github.com/teamhanko/passkeys/blob/8ee34912888207e018423dfc21d1e6cbaf02b414/spec/passkey-server.yaml#L526-L531

https://github.com/teamhanko/passkeys/blob/8ee34912888207e018423dfc21d1e6cbaf02b414/spec/passkey-server.yaml#L763-L780

browser type from lib.dom.d.ts


interface PublicKeyCredentialCreationOptions {
    attestation?: AttestationConveyancePreference;
    authenticatorSelection?: AuthenticatorSelectionCriteria;
    challenge: BufferSource;
    excludeCredentials?: PublicKeyCredentialDescriptor[];
    extensions?: AuthenticationExtensionsClientInputs;
    pubKeyCredParams: PublicKeyCredentialParameters[]; // -> this is PublicKeyCredentialParameters[] | undefined in your schema
    rp: PublicKeyCredentialRpEntity;
    timeout?: number;
    user: PublicKeyCredentialUserEntity;
}

type PublicKeyCredentialType = "public-key"; // this is string in your schema

AuthenticatorTransport

https://github.com/teamhanko/passkeys/blob/8ee34912888207e018423dfc21d1e6cbaf02b414/spec/passkey-server.yaml#L685-L700

// this is string in your schema
type AuthenticatorTransport = "ble" | "hybrid" | "internal" | "nfc" | "usb"; 

residentKey (ResidentKeyRequirement) / userVerification (UserVerificationRequirement)

// this is string in your schema
type ResidentKeyRequirement = "discouraged" | "preferred" | "required";
type UserVerificationRequirement = "discouraged" | "preferred" | "required";

authenticatorAttachment

// this is string in your schema
type AuthenticatorAttachment = "cross-platform" | "platform";

AttestationConveyancePreference

// this is string in your schema
type AttestationConveyancePreference = "direct" | "enterprise" | "indirect" | "none";
merlindru commented 5 months ago

Wow, thank you so much for this comprehensive writeup!!

Apologies we haven't gotten around to this yet. Taking a look at this today.