Skip to content

Commit

Permalink
Make parameter names in core APIs more consistent (#52)
Browse files Browse the repository at this point in the history
This replaces the `name` parameter during both registration (required)
and authentication (optional) with `username`. This better reflects what
the parameter actually represents, and helps disambiguate it from a
display name.

In doing so, I've adjusted the calls to the backend during registration
to omit this data completely, since only the `upgrade` parameter is used
in practice. This keeps data filtering simpler.

Prior to a 1.0 tag, this retains support for the older `name` parameter
at runtime only. TypeScript users will get an immediate type error but
nothing will break if they ignore it; plain JS users will get no such
warning and things will continue to work. I'll open follow-up tasks to
remove it completely.

Fixes #48
  • Loading branch information
Firehed authored Oct 16, 2024
1 parent cc8b03a commit 991a226
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 33 deletions.
35 changes: 7 additions & 28 deletions src/SDK.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,15 @@ type WebAuthnError =
export type AuthResponse = Result<{ token: string }, WebAuthnError>
export type RegisterResponse = Result<{ token: string }, WebAuthnError>

type UserIdOrHandle =
type UserIdOrUsername =
| { id: string }
| { handle: string }
type OptionalUserIdOrHandle = UserIdOrHandle | undefined
| { username: string }
type OptionalUserIdOrUsername = UserIdOrUsername | undefined

export type UserAuthenticationInfo = UserIdOrHandle
export type UserAuthenticationInfo = UserIdOrUsername
export type UserRegistrationInfo = {
name: string
username: string
displayName?: string
id?: string
handle?: string
}

class SDK {
Expand Down Expand Up @@ -160,9 +158,8 @@ class SDK {
*/

private async doRegister(user: UserRegistrationInfo, upgrade: boolean): Promise<RegisterResponse> {
const remoteUserData = this.filterRegistrationData(user)
// User info is client-only during this stage of registration
const res = await this.api('/attestation/options', {
user: remoteUserData,
upgrade,
}) as Result<CredentialCreationOptionsJSON, WebAuthnError>
if (!res.ok) {
Expand All @@ -182,14 +179,13 @@ class SDK {
const json = registrationResponseToJSON(credential)
return await this.api('/attestation/process', {
credential: json as unknown as JsonEncodable,
user: remoteUserData,
}) as RegisterResponse
} catch (error) {
return error instanceof Error ? this.convertCredentialsError(error) : this.genericError(error)
}
}

private async doAuth(user: UserIdOrHandle|undefined): Promise<AuthResponse> {
private async doAuth(user: UserIdOrUsername|undefined): Promise<AuthResponse> {
// Get the remotely-built WebAuthn options
const res = await this.api('/assertion/options', { user }) as Result<CredentialRequestOptionsJSON, WebAuthnError>
if (!res.ok) {
Expand Down Expand Up @@ -306,23 +302,6 @@ class SDK {
this.abortSignals = [ac]
return ac.signal
}

/**
* Privacy enhancement: removes data from network request not needed by
* backend to complete registration
*/
private filterRegistrationData(user: UserRegistrationInfo): UserIdOrHandle|undefined {
// If user info provided, send only the id or handle. Do NOT send name or
// displayName.
if (user.id || user.handle) {
return {
id: user.id,
// @ts-ignore figure this type hack out later
handle: user.handle,
}
}
}

}

const formatError = <T>(error: WebAuthnError, obj: Error): Result<T, WebAuthnError> => ({
Expand Down
19 changes: 14 additions & 5 deletions src/fromJSON.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,22 @@ export const parseRequestOptions = (json: CredentialRequestOptionsJSON): Credent
return getOptions
}

export const parseCreateOptions = (user: UserRegistrationInfo, json: CredentialCreationOptionsJSON): CredentialCreationOptions => {
// Locally merge in user.name and displayName - they are never sent out (see
// filterRegistrationData) and thus are not part of the server response.
type CombinedRegistrationFormat =
| UserRegistrationInfo
| { name: string, displayName?: string } // Legacy registration: name instead of username

export const parseCreateOptions = (user: CombinedRegistrationFormat, json: CredentialCreationOptionsJSON): CredentialCreationOptions => {
// Combine the server response (w/ user.id) and the client data into the
// webAuthn structure, which requires `id`, `name`, and `displayName`.
// What WebAuthn calls `name` we call `username` to enhance usage clarity.
//
// Pre-1.0, continue to support `name` as well.
// @ts-ignore It's incorrectly inferring username|name
const name = user.username ?? user.name
json.publicKey.user = {
...json.publicKey.user,
name: user.name,
displayName: user.displayName ?? user.name,
name,
displayName: user.displayName ?? name,
}

let createOptions: CredentialCreationOptions = {}
Expand Down

0 comments on commit 991a226

Please sign in to comment.