feat(fastify): add account link routes (wallet, email) #90
feat(fastify): add account link routes (wallet, email) #90gaboesquivel merged 6 commits intomainfrom
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
- Add verification.type column (magic_link | link_email), web3_nonces table - Add GET /auth/web3/nonce for SIWE nonce - Add POST /account/link/wallet/verify (Bearer, chain+message+signature) - Add POST /account/link/email/request and verify with LinkEmailEmail template - Add web3-verify lib for EIP-155 and Solana signature verification - Update magiclink to set/filter by type - Update authentication.mdx docs Co-authored-by: Cursor <cursoragent@cursor.com>
…and core - Fix magiclink request duplicate type property (TS1117) - Regenerate OpenAPI from routes (never edit directly) - Regenerate @repo/core from OpenAPI Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/docu/content/docs/architecture/authentication.mdx (1)
58-58:⚠️ Potential issue | 🟡 MinorMermaid diagram still references the old nonce endpoint path.
Line 58 shows
GET /auth/web3/:chain/nonce?address=...but line 112 documents the new endpoint asGET /auth/web3/noncewithchainas a query parameter. Update the diagram to match.Proposed fix
- Client->>FastifyAPI: GET /auth/web3/:chain/nonce?address=... + Client->>FastifyAPI: GET /auth/web3/nonce?chain=...&address=...
🤖 Fix all issues with AI agents
In `@apps/fastify/src/lib/web3-verify.ts`:
- Around line 72-78: normalizeAddress currently returns the EIP‑55 checksummed
address via getAddress(address) while nonces are stored lowercase, causing
inconsistent address casing across tables; change normalizeAddress (in
web3-verify.ts) to return getAddress(address).toLowerCase() so it canonicalizes
to lowercase; this will align with nonce storage in nonce.ts and allow verify.ts
to use normalizedAddress directly (removing the separate lookupAddr lowercasing)
and keep walletIdentities rows consistent.
In `@apps/fastify/src/routes/account/link/email/request.ts`:
- Around line 80-87: The insert of the verification record via
db.insert(verification).values(...) can leave an orphan if
emailProvider.emails.send throws; capture the inserted record id (generate the
randomUUID() into a variable instead of inlining), then wrap the
emailProvider.emails.send call in a try/catch and on error delete the
verification row (use the same id or identifier/tokenHash to remove the record)
or rethrow after cleanup; update the request flow around
db.insert(verification).values, emailProvider.emails.send, token/tokenHash and
expiresAt to ensure delete runs on send failure.
In `@apps/fastify/src/routes/account/link/email/verify.test.ts`:
- Around line 74-87: The test uses a made-up string ('expired-token-xxxx') which
triggers INVALID_TOKEN, not the expired path; to fix, create a real verification
record with an expiresAt set in the past (use your test DB helper or the same
model used by the route), retrieve its token and then call the POST
/account/link/email/verify with Authorization from getSessionToken() and payload
{ token: <expiredToken> }; assert response.statusCode === 401 and JSON body.code
=== 'EXPIRED_TOKEN' (alternatively, if you prefer to keep the current negative
test, rename the test to assert INVALID_TOKEN and require status 401 only).
Ensure this change targets the test in verify.test.ts and mirrors the route
behavior implemented in verify.ts.
In `@apps/fastify/src/routes/account/link/email/verify.ts`:
- Around line 94-109: The three DB mutations (deleting verification via
db.delete(verification) using verificationRecord.id, updating the users table
via db.update(users).set(...) with userId, and updating sessions via
db.update(sessions).set(...) using request.session.session.id after generating
refreshJti/hashToken and sessionExpiresAt) must be executed inside a single
Drizzle transaction so they either all succeed or all roll back; refactor to run
deletion of the verification row, the users update (email, emailVerified,
updatedAt), and the sessions update (token, expiresAt) within
db.transaction/transactional API and ensure errors cause a rollback so the
consumed token isn’t lost on partial failure.
In `@apps/fastify/src/routes/account/link/wallet/verify.ts`:
- Around line 131-158: Wrap the logic that checks for an existing wallet and
inserts into walletIdentities (the block using existing,
db.insert(walletIdentities), and db.delete(web3Nonce) that references nonceRow
and request.session.user.id) inside a database transaction so the
check-and-insert is atomic; inside that transaction attempt the insert and, on
catching a unique-constraint/database error for wallet_chain_address_unique,
return the same 409 WALLET_ALREADY_LINKED response instead of letting it bubble
as a 500; ensure the web3Nonce deletion
(db.delete(web3Nonce).where(eq(web3Nonce.id, nonceRow.id))) still runs exactly
once (inside the transaction or in a finally cleanup) so the nonce is removed
whether the insert succeeds or fails.
🧹 Nitpick comments (11)
apps/fastify/package.json (1)
39-39: Hardcoded local DB credentials in script — acceptable for dev but worth noting.The
postgres:postgres@127.0.0.1:54322credentials are standard Supabase CLI local defaults, so the static analysis flag (CKV_SECRET_4) is a false positive in practice. However, consider referencing an env var or.envfile instead to keep the pattern consistent with other scripts and avoid tripping secret scanners in CI.apps/fastify/src/routes/auth/web3/nonce.ts (3)
49-61: Redundant manual validation — TypeBox schema already enforces these constraints.The
querystringschema (Lines 36-38) defineschainasType.Union([Type.Literal('eip155'), Type.Literal('solana')])andaddressas a requiredType.String(). Fastify's TypeBox validation will reject invalid/missing values before the handler executes, so both theisValidChaincheck (Line 49) and the!chain || !addresscheck (Line 56) are dead code.♻️ Remove redundant checks
async (request, reply) => { const { chain, address } = request.query - if (!isValidChain(chain)) { - return reply.code(400).send({ - code: 'INVALID_CHAIN', - message: 'Chain must be eip155 or solana', - }) - } - - if (!chain || !address?.trim()) { - return reply.code(400).send({ - code: 'MISSING_PARAMS', - message: 'chain and address query parameters are required', - }) - } - const db = await getDb()
78-87: Delete + insert not wrapped in a transaction.If the
inserton Line 81 fails after thedeleteon Line 78 succeeds, the user's nonce is lost with no record created. Wrapping both in a transaction ensures atomicity.♻️ Use a transaction
- await db - .delete(web3Nonce) - .where(and(eq(web3Nonce.chain, chain), eq(web3Nonce.address, normalizedAddr))) - await db.insert(web3Nonce).values({ - id: randomUUID(), - chain, - address: normalizedAddr, - nonce, - expiresAt, + await db.transaction(async tx => { + await tx + .delete(web3Nonce) + .where(and(eq(web3Nonce.chain, chain), eq(web3Nonce.address, normalizedAddr))) + await tx.insert(web3Nonce).values({ + id: randomUUID(), + chain, + address: normalizedAddr, + nonce, + expiresAt, + }) })
12-18: ImportisValidChainfromvalidate-address.tsto eliminate duplication.
isValidChainis already exported fromapps/fastify/src/routes/auth/web3/validate-address.ts. Remove the local function andVALID_CHAINS/ValidChaindefinitions (lines 13–18) and import it instead. Note:VALID_CHAINSis not exported; it's only a local constant invalidate-address.tsused to define the exportedWeb3Chaintype.♻️ Proposed fix
+import { isValidChain, type Web3Chain } from './validate-address.js' -const VALID_CHAINS = ['eip155', 'solana'] as const -type ValidChain = (typeof VALID_CHAINS)[number] - -function isValidChain(chain: string): chain is ValidChain { - return VALID_CHAINS.includes(chain as ValidChain) -}Replace
ValidChainreferences with the importedWeb3Chaintype where needed.apps/fastify/src/lib/web3-verify.ts (1)
6-9:Web3Chainand chain constants duplicated across multiple files.
Web3Chain,VALID_CHAINS, andisValidChainare defined invalidate-address.ts,nonce.ts,verify.ts, and this file. Consider a single shared module (e.g.,validate-address.tsor a newweb3-chains.ts) to be the canonical source.apps/fastify/src/routes/account/link/wallet/verify.ts (1)
55-61: Redundant chain validation — already enforced by TypeBox schema.Same issue as in
nonce.ts: theVerifySchema(Line 19) constrainschaintoType.Union([Type.Literal('eip155'), Type.Literal('solana')]), so Fastify rejects invalid values before the handler runs. This check is dead code.apps/fastify/src/routes/account/link/wallet/verify.test.ts (1)
10-192: Good integration coverage for EIP-155 — but Solana chain is untested.The test suite thoroughly covers the EIP-155 happy path, unauthorized access, invalid signature, and wallet conflict scenarios. However, there are no tests exercising the Solana wallet linking flow. Since
verify.tssupports both chains with different verification logic (nacl vs. viem), a Solana test would help prevent regressions in that code path.Would you like me to open an issue to track adding Solana wallet linking tests?
apps/fastify/src/routes/account/account.spec.ts (1)
20-22: Inconsistent import style: missing.jsextension on test imports.Lines 2–4 use the
.jsextension (standard for TypeScript ESM), but the test-suite side-effect imports on lines 20–22 omit it. While Vitest typically resolves these fine, it's inconsistent within the same file.Suggested fix
-import './link/wallet/verify.test' -import './link/email/request.test' -import './link/email/verify.test' +import './link/wallet/verify.test.js' +import './link/email/request.test.js' +import './link/email/verify.test.js'apps/fastify/src/routes/account/link/email/request.test.ts (1)
4-22:getSessionTokenhelper is duplicated and less robust than the version inverify.test.ts.This helper is nearly identical to the one in
verify.test.ts(lines 4-20), but that version accepts anfastify.fakeEmail?.clear()before requesting the magic link. Without theclear()call here,extractToken()could pick up a stale email in tests that send emails before calling this helper (e.g., the "EMAIL_ALREADY_IN_USE" test at line 84).Consider extracting a shared helper, or at minimum add a
clear()call to match the safer pattern.Quick fix — align with verify.test.ts
async function getSessionToken(): Promise<string> { + fastify.fakeEmail?.clear() await fastify.inject({ method: 'POST', url: '/auth/magiclink/request',apps/fastify/openapi/openapi.json (1)
943-1015: New/auth/web3/nonceendpoint overlaps with existing chain-specific nonce endpoints.The new unified
GET /auth/web3/nonce?chain=eip155|solana&address=...(line 943) coexists with the pre-existingGET /auth/web3/eip155/nonce(line 1016) andGET /auth/web3/solana/nonce(line 1201). This creates two ways to fetch a nonce for the same chains, which may confuse API consumers and complicate deprecation.Consider deprecating the chain-specific nonce endpoints or documenting the intended migration path.
apps/fastify/src/routes/account/link/email/request.ts (1)
74-78: Nit:typeof email === 'string'is redundant.
Type.String({ format: 'email' })by TypeBox before the handler runs, so this check always evaluates totrue.Simplify
const storePlain = env.NODE_ENV !== 'production' && env.ALLOW_TEST === true && - typeof email === 'string' && email.endsWith('@test.ai')
- web3-verify: canonicalize EIP155 address to lowercase (align with nonce storage) - email/request: delete verification record on send failure to avoid orphans - email/verify.test: create real expired token record, assert EXPIRED_TOKEN - email/verify: wrap delete/update/update in single transaction - wallet/verify: wrap check-insert in transaction, handle 23505 as 409 Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests
Chores