Repository navigation
feat: add list and delete endpoints, store passkey name and createdAt - #12
Conversation
Deploying passkeys with
|
| Latest commit: |
97a09cc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e1c6e038.passkeys-3nt.pages.dev |
| Branch Preview URL: | https://feat-passkey-management.passkeys-3nt.pages.dev |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
This pull request introduces passkey management functionality by adding list, delete, and remove endpoints to both the core Passkeys class and the routing handler. It also enhances credential metadata storage with creation timestamps and friendly display names, accompanied by unit tests covering multi-passkey listing and deletion.
🔍 General Feedback
- Documentation in README.md should be updated to reflect the new
/passkeys/listand/passkeys/deleteroutes, along with updatedPasskeysOptionsJSDoc indicating that thekvinterface now requires adelete()method. - The CI workflow in
.github/workflows/test.ymlcurrently runsnpm test, which executestestkitontest/test.js. The multi-passkey tests intest/test_passkeys_multi.jsare currently only wired tonpm run test:runand do not execute in CI; running them alongside the test suite will prevent regressions.
📊 Token Usage & Cost Efficiency
| Metric | Value |
|---|---|
| Input Tokens (uncached) | 5,554 |
| Input Tokens (cached) | 27,636 (⚡ 83.3% cached) |
| Thinking / Reasoning Tokens | 29,219 |
| Output Tokens | 994 |
| Total Session Tokens | 63,403 |
| Cost (uncached input) | $0.0042 |
| Cost (cached input) | $0.0021 |
| Cost (thinking / reasoning) | $0.1096 |
| Cost (response output) | $0.0037 |
| Estimated Total Cost | $0.1195 |
Gemini 3.8 Flash: introductory rate $0.75/$3.75 per 1M applied; reverts to $1.5/$7.5 after 2026-12-31.
Gemini 3.8 Flash: standard tier; batch and flex are half again, priority is higher.
Cache reads are priced here, but context-cache STORAGE, where the model charges for it separately per token-hour, is not included, so the figure can run slightly low.
| async check(c) { | ||
| let sess = await getSession(this.c2(c)) | ||
| if (!sess || !sess.userId) { | ||
| let userId = sess.userId || c.data?.user?.id || c.data?.userID |
There was a problem hiding this comment.
🟡 In src/auth.js (setupUserSession), the user identifier is stored as c.data.userId with a lowercase 'd'. The fallback chain checks c.data?.userID and c.data?.user?.id, but omits c.data?.userId. Including c.data?.userId avoids potential authentication lookup misses when middleware sets userId directly on c.data.
| let userId = sess.userId || c.data?.user?.id || c.data?.userID | |
| let userId = sess.userId || c.data?.userId || c.data?.user?.id || c.data?.userID |
|
|
||
| async list(c) { | ||
| let sess = await getSession(this.c2(c)) | ||
| let userId = sess.userId || c.data?.user?.id || c.data?.userID |
There was a problem hiding this comment.
🟡 To remain consistent with src/auth.js and check(), consider adding c.data?.userId to the fallback chain here as well.
| let userId = sess.userId || c.data?.user?.id || c.data?.userID | |
| let userId = sess.userId || c.data?.userId || c.data?.user?.id || c.data?.userID |
|
|
||
| async delete(c) { | ||
| let sess = await getSession(this.c2(c)) | ||
| let userId = sess.userId || c.data?.user?.id || c.data?.userID |
There was a problem hiding this comment.
🟡 To remain consistent with src/auth.js and check(), consider adding c.data?.userId to the fallback chain here as well.
| let userId = sess.userId || c.data?.user?.id || c.data?.userID | |
| let userId = sess.userId || c.data?.userId || c.data?.user?.id || c.data?.userID |
| } | ||
| if (!passkeyId && c.request?.url) { | ||
| try { | ||
| const url = new URL(c.request.url) |
There was a problem hiding this comment.
🟡 If c.request.url is a relative path (which often occurs in test mocks or Node.js HTTP contexts), new URL(c.request.url) will throw an invalid URL error. Because it is caught by the try...catch block, extracting id from the query string fails silently. Providing a base URL fallback ensures that both relative and absolute URLs parse successfully.
| const url = new URL(c.request.url) | |
| const url = new URL(c.request.url, 'http://localhost') |
| return Response.json({ error: { message: 'Passkey not found for user' } }, { status: 404 }) | ||
| } | ||
|
|
||
| await this.opts.kv.delete(`passkeys-${passkeyId}`) |
There was a problem hiding this comment.
🟡 The PasskeysOptions JSDoc previously specified that kv only requires put() and get() methods. If a consumer provides a custom KV implementation without a delete method, this will throw an unhandled TypeError. Checking whether delete is a function before calling it makes custom KV stores safer and backwards-compatible.
| await this.opts.kv.delete(`passkeys-${passkeyId}`) | |
| if (typeof this.opts.kv.delete === 'function') { | |
| await this.opts.kv.delete(`passkeys-${passkeyId}`) | |
| } |
Adds
list()anddelete()endpoints to thePasskeysclass and stores passkeynameandcreatedAtduringcreate(). This centralizes passkey management logic in the library so consumer applications (such as flaregun-starter and thingster) do not need to duplicate KV passkey management code.