Skip to content

Commit 823708a

Browse files
NiKrauseclaude
andauthored
fix: stop deriving the WebAuthn user handle from the typed name (#46)
`user.id` was `TextEncoder().encode(userId)`. An authenticator keeps one discoverable credential per (rp.id, user.id) and replaces the previous one when both match — silently, no prompt, nothing to undo. So two people registering as "anna" on a shared device destroyed each other's passkey, and with it the DID and every entry signed under it. One person re-entering their usual name after clearing storage did the same to themselves. WebAuthn L2 §5.4.3 is explicit on both counts: the handle MUST NOT carry personally identifying information such as a username or e-mail address, and SHOULD be 64 random bytes. It is now exactly that, and `userId` keeps its rightful place as `user.name`, the label the picker shows. Nothing needs migrating. No path in this package resolves a credential by handle — recovery goes through discoverable credentials, or an explicit credential ID where those are off — so credentials registered under the old scheme keep working. The derived did:key comes from the credential's public key and is unaffected. The mock authenticator now models the replacement rule, which it did not before: it files credentials by handle and drops what was in the slot. That is what makes the regression test meaningful rather than decorative. Verified: with the old line restored, three of the four new tests fail — including the one that asserts what the library sends, which holds regardless of how faithfully the mock behaves. All 67 node tests pass with the fix, prettier and eslint are clean. Closes #45 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 3cf2f78 commit 823708a

6 files changed

Lines changed: 232 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,37 @@
44

55
### Fixed
66

7+
- The WebAuthn user handle is 64 random bytes instead of the UTF-8 of whatever
8+
the caller passes as `userId`. An authenticator keeps one discoverable
9+
credential per `(rp.id, user.id)` and **replaces** the previous one when both
10+
match — silently, with no prompt and nothing to undo. Deriving the handle from
11+
a typed name therefore meant two people registering as "anna" on a shared
12+
device destroyed each other's passkey, and with it the DID and every entry
13+
signed under it. The same happened to one person re-entering their usual name
14+
after clearing storage. WebAuthn L2 §5.4.3 also forbids putting personally
15+
identifying information in the handle, which a name or e-mail plainly is, and
16+
the handle is stored in the authenticator indefinitely.
17+
18+
`userId` keeps its rightful place as `user.name`, the label the credential
19+
picker shows. It is a label only: it identifies nothing, two credentials may
20+
carry the same one, and nothing in this package looks a credential up by it.
21+
Closes #45.
22+
23+
Nothing needs migrating. Recovery goes through discoverable credentials
24+
(`readLargeBlobMetadata` calls `get()` with no `allowCredentials`), or through
25+
an explicit credential ID where those are switched off — no path resolves a
26+
credential by handle, so credentials registered under the old scheme keep
27+
working untouched. The derived `did:key` comes from the credential's public
28+
key and does not change.
29+
30+
Consumers should expect a behavioural difference: re-registering under a name
31+
that was used before now **adds** a passkey instead of replacing one. That is
32+
the point — a replaced passkey is data loss, a second entry is a choice — but
33+
it means the picker can show several, so `user.name` and `displayName` should
34+
be distinguishing enough to pick from. The new random handle is returned as
35+
`credential.userHandle` (base64url); the authenticator keeps its own copy, so
36+
storing it is optional.
37+
738
- Actually write the secret key into `largeBlob`. The keystore record carried
839
`secretKey: sk, // Will be moved to largeBlob` — and nothing ever moved it.
940
That was the only occurrence of the field in `src/`, and

playwright.node.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export default defineConfig({
2121
'webauthn-extension-support.test.js',
2222
'webauthn-varsig-challenge.test.js',
2323
'webauthn-attestation-parsing.test.js',
24+
'webauthn-user-handle.test.js',
2425
'webauthn-two-peer-replication.test.js',
2526
],
2627
fullyParallel: false,

src/webauthn/provider.js

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,11 @@ export class WebAuthnDIDProvider {
8282
* Create a WebAuthn credential for OrbitDB identity
8383
* This triggers biometric authentication (Face ID, Touch ID, Windows Hello, etc.)
8484
* @param {Object} options - Credential options
85-
* @param {string} options.userId - User ID
85+
* @param {string} options.userId - Account label shown in the credential
86+
* picker (`user.name`). A label only: it does not identify the credential,
87+
* two credentials may carry the same one, and it is never used to look one
88+
* up. The handle the authenticator files the credential under is generated
89+
* here and returned as `userHandle`.
8690
* @param {string} options.displayName - Display name
8791
* @param {string} options.domain - Domain/RP ID
8892
* @param {boolean} options.encryptKeystore - Enable keystore encryption
@@ -117,7 +121,19 @@ export class WebAuthnDIDProvider {
117121

118122
// Generate challenge for credential creation
119123
const challenge = crypto.getRandomValues(new Uint8Array(32));
120-
const userIdBytes = new TextEncoder().encode(userId);
124+
125+
// The user handle is an opaque key, not a label. An authenticator stores
126+
// one discoverable credential per (rp.id, user.id) pair and *replaces* the
127+
// previous one when both match — silently, with no prompt and nothing to
128+
// undo. Deriving the handle from a typed name therefore meant two people
129+
// registering as "anna" on a shared device destroyed each other's passkey,
130+
// and with it the DID and everything written under it (#45).
131+
//
132+
// 64 random bytes, as WebAuthn L2 §5.4.3 recommends. The same section
133+
// forbids putting personally identifying information here, which a typed
134+
// name or e-mail address plainly is. The name keeps its rightful place in
135+
// `user.name` below, where the credential picker shows it.
136+
const userHandle = crypto.getRandomValues(new Uint8Array(64));
121137

122138
webauthnLog('Calling navigator.credentials.create() for user: %s', userId);
123139

@@ -130,7 +146,7 @@ export class WebAuthnDIDProvider {
130146
id: domain,
131147
},
132148
user: {
133-
id: userIdBytes,
149+
id: userHandle,
134150
name: userId,
135151
displayName,
136152
},
@@ -247,6 +263,13 @@ export class WebAuthnDIDProvider {
247263
publicKey,
248264
userId,
249265
displayName,
266+
// Nothing in this package looks a credential up by handle — recovery
267+
// goes through discoverable credentials, or through an explicit
268+
// credential ID when those are switched off. It is surfaced anyway
269+
// because it is the only copy the caller will ever see, and a flow
270+
// that one day passes `allowCredentials` needs it. Decode with
271+
// `WebAuthnDIDProvider.base64urlToArrayBuffer()`.
272+
userHandle: WebAuthnDIDProvider.arrayBufferToBase64url(userHandle),
250273
attestationObject: new Uint8Array(
251274
credential.response.attestationObject
252275
),

tests/helpers/mock-authenticator.js

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,23 @@ export async function createMockAuthenticator({
7575
await crypto.subtle.exportKey('spki', keypair.publicKey)
7676
);
7777

78-
const credentialId = crypto.getRandomValues(
78+
// One resident credential per (rpId, user.id), keyed by the handle — the rule
79+
// that makes a name-derived handle destructive (#45). A registration whose
80+
// handle is already present replaces what was there; a new handle adds to it.
81+
const residentByHandle = new Map();
82+
let activeCredentialId = crypto.getRandomValues(
7983
new Uint8Array(credentialIdLength)
8084
);
8185
const rpIdHash = await sha256(new TextEncoder().encode(rpId));
8286

83-
const state = { signCount: 0, assertions: 0, creations: 0, prfEvals: 0 };
87+
const state = {
88+
signCount: 0,
89+
assertions: 0,
90+
creations: 0,
91+
prfEvals: 0,
92+
/** Every `user` dictionary passed to create(), in order. */
93+
createdUsers: [],
94+
};
8495

8596
// Per-credential secret the PRF output is derived from.
8697
const prfKey = prfSecret ?? crypto.getRandomValues(new Uint8Array(32));
@@ -107,7 +118,10 @@ export async function createMockAuthenticator({
107118
)
108119
);
109120

110-
const buildAuthData = ({ includeAttestedCredential }) => {
121+
const buildAuthData = ({
122+
includeAttestedCredential,
123+
credentialId = activeCredentialId,
124+
}) => {
111125
const flagByte = includeAttestedCredential
112126
? 0x01 | 0x04 | 0x40 // UP | UV | AT
113127
: 0x01 | 0x04; // UP | UV
@@ -152,8 +166,23 @@ export async function createMockAuthenticator({
152166
state.creations += 1;
153167
state.signCount += 1;
154168

169+
const user = options?.publicKey?.user ?? {};
170+
state.createdUsers.push(user);
171+
172+
const handleKey = bytesToBase64url(new Uint8Array(user.id ?? []));
173+
const credentialId = crypto.getRandomValues(
174+
new Uint8Array(credentialIdLength)
175+
);
176+
// set(), not a guard: a repeat handle silently drops the credential
177+
// that was stored under it, exactly as an authenticator does.
178+
residentByHandle.set(handleKey, credentialId);
179+
activeCredentialId = credentialId;
180+
155181
const challenge = options?.publicKey?.challenge ?? new Uint8Array(32);
156-
const authData = buildAuthData({ includeAttestedCredential: true });
182+
const authData = buildAuthData({
183+
includeAttestedCredential: true,
184+
credentialId,
185+
});
157186
const attestationObject = new Uint8Array(
158187
encode(
159188
new Map([
@@ -222,10 +251,10 @@ export async function createMockAuthenticator({
222251
);
223252

224253
return {
225-
id: bytesToBase64url(credentialId),
226-
rawId: credentialId.buffer.slice(
227-
credentialId.byteOffset,
228-
credentialId.byteOffset + credentialId.byteLength
254+
id: bytesToBase64url(activeCredentialId),
255+
rawId: activeCredentialId.buffer.slice(
256+
activeCredentialId.byteOffset,
257+
activeCredentialId.byteOffset + activeCredentialId.byteLength
229258
),
230259
type: 'public-key',
231260
response: {
@@ -246,7 +275,16 @@ export async function createMockAuthenticator({
246275
return {
247276
navigator: navigatorShim,
248277
publicKey: { algorithm: -7, x, y, keyType: 2, curve: 1 },
249-
credentialId,
278+
/** The credential an assertion would use: the most recently created one. */
279+
get credentialId() {
280+
return activeCredentialId;
281+
},
282+
/** What the authenticator still holds, one entry per distinct handle. */
283+
residentCredentials: () =>
284+
[...residentByHandle.entries()].map(([handle, id]) => ({
285+
handle,
286+
credentialId: bytesToBase64url(id),
287+
})),
250288
state,
251289
};
252290
}

tests/webauthn-user-handle.test.js

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/**
2+
* The WebAuthn user handle must not be derived from anything a person types.
3+
*
4+
* An authenticator keeps one discoverable credential per (rp.id, user.id) and
5+
* replaces the previous one when both match — no prompt, no undo. While the
6+
* handle was `TextEncoder().encode(userId)`, two people registering under the
7+
* same name on one device destroyed each other's passkey, and with it the DID
8+
* and every entry signed under it (#45).
9+
*
10+
* The first test is the load-bearing one: it asserts what the library *sends*,
11+
* so it holds regardless of how faithfully the mock models an authenticator.
12+
* The rest describe the consequence.
13+
*/
14+
import { test, expect } from '@playwright/test';
15+
import {
16+
createMockAuthenticator,
17+
installMockAuthenticator,
18+
} from './helpers/mock-authenticator.js';
19+
import { silenceWebAuthnDebugLogging } from './helpers/two-peer.js';
20+
21+
const NAME = 'anna';
22+
23+
test.describe('WebAuthn user handle', () => {
24+
let restoreAuthenticator;
25+
let restoreLogging;
26+
let authenticator;
27+
let WebAuthnDIDProvider;
28+
29+
test.beforeEach(async () => {
30+
restoreLogging = silenceWebAuthnDebugLogging();
31+
authenticator = await createMockAuthenticator();
32+
restoreAuthenticator = installMockAuthenticator(authenticator);
33+
({ WebAuthnDIDProvider } = await import('../src/webauthn/provider.js'));
34+
});
35+
36+
test.afterEach(() => {
37+
restoreAuthenticator?.();
38+
restoreLogging?.();
39+
});
40+
41+
test('is 64 random bytes, and carries nothing the user typed', async () => {
42+
await WebAuthnDIDProvider.createCredential({
43+
userId: NAME,
44+
displayName: 'Anna',
45+
});
46+
await WebAuthnDIDProvider.createCredential({
47+
userId: NAME,
48+
displayName: 'Anna',
49+
});
50+
51+
const [first, second] = authenticator.state.createdUsers;
52+
const firstHandle = new Uint8Array(first.id);
53+
const secondHandle = new Uint8Array(second.id);
54+
55+
expect(firstHandle.length).toBe(64);
56+
expect(secondHandle.length).toBe(64);
57+
58+
// Same input, different handle — the property the whole fix rests on.
59+
expect(Buffer.from(firstHandle).equals(Buffer.from(secondHandle))).toBe(
60+
false
61+
);
62+
63+
// And nothing recognisable in it: not the typed bytes, not the name as text.
64+
const typed = new TextEncoder().encode(NAME);
65+
expect(
66+
Buffer.from(firstHandle.subarray(0, typed.length)).equals(
67+
Buffer.from(typed)
68+
)
69+
).toBe(false);
70+
expect(new TextDecoder().decode(firstHandle)).not.toContain(NAME);
71+
});
72+
73+
test('keeps the typed value as the label the picker shows', async () => {
74+
await WebAuthnDIDProvider.createCredential({
75+
userId: NAME,
76+
displayName: 'Anna at the front desk',
77+
});
78+
79+
const [user] = authenticator.state.createdUsers;
80+
expect(user.name).toBe(NAME);
81+
expect(user.displayName).toBe('Anna at the front desk');
82+
});
83+
84+
test('two people with the same name no longer overwrite each other', async () => {
85+
const first = await WebAuthnDIDProvider.createCredential({
86+
userId: NAME,
87+
displayName: 'Anna',
88+
});
89+
const second = await WebAuthnDIDProvider.createCredential({
90+
userId: NAME,
91+
displayName: 'Anna',
92+
});
93+
94+
expect(second.credentialId).not.toBe(first.credentialId);
95+
96+
// Both survive. Under a name-derived handle the second registration would
97+
// have landed in the first one's slot and left a single credential here.
98+
const resident = authenticator.residentCredentials();
99+
expect(resident.length).toBe(2);
100+
expect(resident.map((entry) => entry.credentialId).sort()).toEqual(
101+
[first.credentialId, second.credentialId].sort()
102+
);
103+
});
104+
105+
test('returns the handle, base64url of 64 bytes', async () => {
106+
const credential = await WebAuthnDIDProvider.createCredential({
107+
userId: NAME,
108+
displayName: 'Anna',
109+
});
110+
111+
expect(credential.userHandle).toBeTruthy();
112+
expect(credential.userHandle).toMatch(/^[A-Za-z0-9_-]+$/);
113+
114+
const decoded = new Uint8Array(
115+
WebAuthnDIDProvider.base64urlToArrayBuffer(credential.userHandle)
116+
);
117+
expect(decoded.length).toBe(64);
118+
expect(
119+
Buffer.from(decoded).equals(
120+
Buffer.from(new Uint8Array(authenticator.state.createdUsers[0].id))
121+
)
122+
).toBe(true);
123+
});
124+
});

types/index.d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ export interface WebAuthnCredentialInfo {
9191
publicKey: WebAuthnPublicKey | Uint8Array;
9292
userId?: string;
9393
displayName?: string;
94+
/** Random 64-byte WebAuthn user handle, base64url. Not derived from userId. */
95+
userHandle?: string;
9496
attestationObject?: Uint8Array;
9597
prfInput?: Uint8Array;
9698
did?: string;
@@ -99,6 +101,7 @@ export interface WebAuthnCredentialInfo {
99101
}
100102

101103
export interface CreateCredentialOptions {
104+
/** Account label for the credential picker. Not an identifier. */
102105
userId?: string;
103106
displayName?: string;
104107
domain?: string;

0 commit comments

Comments
 (0)