feat!: migrate to better-auth 1.6#323
Conversation
|
@ramonclaudio is attempting to deploy a commit to the Convex Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdd support for where.mode ("sensitive" | "insensitive") with case-insensitive filtering and null normalization; bump package and better-auth peer/dev deps to >=1.6.2 <1.7.0; expose VERSION on plugins; pass Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant Client
participant CrossDomainPlugin
participant OneTimeTokenPlugin
participant CookieUtil
end
Client->>CrossDomainPlugin: POST /verifyOneTimeToken (outer flags: asResponse=true,...)
CrossDomainPlugin->>OneTimeTokenPlugin: verifyOneTimeToken(..., asResponse=false, returnHeaders=false, returnStatus=false)
OneTimeTokenPlugin-->>CrossDomainPlugin: session (raw object)
CrossDomainPlugin->>CookieUtil: setSessionCookie(session)
CrossDomainPlugin-->>Client: 200 with session cookie
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
106-107: Align dev and peerbetter-authranges to avoid compatibility drift.
peerDependencies.better-authis pinned to>=1.6.0 <1.7.0, butdevDependenciesuses^1.6.0forbetter-auth,@better-auth/core, and@better-auth/test-utils. The caret range can expand beyond 1.6.x and diverge from the peer constraint, creating a mismatch between tested and published compatibility windows.♻️ Proposed alignment
- "@better-auth/core": "^1.6.0", - "@better-auth/test-utils": "^1.6.0", + "@better-auth/core": ">=1.6.0 <1.7.0", + "@better-auth/test-utils": ">=1.6.0 <1.7.0", ... - "better-auth": "^1.6.0", + "better-auth": ">=1.6.0 <1.7.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 106 - 107, Dev and peer ranges for better-auth currently diverge; update the devDependency entries "better-auth", "@better-auth/core", and "@better-auth/test-utils" to use the exact same range as peerDependencies (>=1.6.0 <1.7.0) so tests run against the same compatibility window you publish; locate those package names in package.json and replace their "^1.6.0" caret ranges with ">=1.6.0 <1.7.0".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Around line 106-107: Dev and peer ranges for better-auth currently diverge;
update the devDependency entries "better-auth", "@better-auth/core", and
"@better-auth/test-utils" to use the exact same range as peerDependencies
(>=1.6.0 <1.7.0) so tests run against the same compatibility window you publish;
locate those package names in package.json and replace their "^1.6.0" caret
ranges with ">=1.6.0 <1.7.0".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b3cbc19-4e4f-4f4e-b44e-523c9edb8446
⛔ Files ignored due to path filters (3)
examples/next/package-lock.jsonis excluded by!**/package-lock.jsonexamples/react/package-lock.jsonis excluded by!**/package-lock.jsonexamples/tanstack/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
CHANGELOG.mdpackage.jsonsrc/auth-options.tssrc/client/adapter-utils.tssrc/client/create-api.tssrc/component/adapterTest.tssrc/plugins/convex/client.tssrc/plugins/convex/index.test.tssrc/plugins/convex/index.tssrc/plugins/cross-domain/client.tssrc/plugins/cross-domain/index.tssrc/version.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/cross-domain/index.test.ts`:
- Around line 121-133: The test's inputCtx currently pre-sets flags (asResponse,
returnHeaders, returnStatus) to false so a server-side override could be masked;
update the test to flip or omit those flags so it actually verifies the endpoint
forces overrides. Specifically, in the inputCtx object used in the test
(properties asResponse, returnHeaders, returnStatus) set them to true (or remove
them entirely) instead of false, and make the same change for the other inputCtx
instance later in the file to ensure the test fails if flag-forwarding/override
logic breaks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1259a0a6-7ead-4085-ac34-80d7d81d51e2
📒 Files selected for processing (3)
CHANGELOG.mdsrc/client/adapter-utils.tssrc/plugins/cross-domain/index.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- src/client/adapter-utils.ts
4c84ef9 to
0dcaf2e
Compare
- better-auth peer ^1.6.1 → ^1.6.5 (bun resolves to 1.6.6) - patches/@Convex-Dev%2Fbetter-auth@0.11.4.patch refreshed from get-convex/better-auth#323 HEAD (a7d5b33): - peer >=1.6.0 → >=1.6.5 <1.7.0 - cross-domain parseSetCookieHeader delegated to better-auth/cookies (local copy pre-dated better-auth's 1.6.0 lookahead fix, #8301) - asResponse: false at 7 internal plugin endpoint sites - twoFactor.verified schema (1.6.2 compat) - Where.mode case-folding in adapter validators - version field on all 4 plugins - picks up @better-auth/oauth-provider GHSA-xr8f-h2gw-9xh6 via peer Verified: bun install clean, tsc --noEmit clean. Lint errors in sidebar.tsx/profile.tsx are pre-existing and unrelated.
|
Thanks for your work on this @ramonclaudio! Digging in soon |
|
Thanks @erquhart! Ping me if anything needs to be reworked. |
Bumps the `better-auth` peer to `>=1.6.0 <1.7.0` and dev deps to `^1.6.0`. Bumps package version to `0.12.0`. Users on `^0.11.x` must also bump `better-auth` to `^1.6.0` in their own `package.json`. Inherited semantic changes from upstream (freshAge from `createdAt`, oidc-provider deprecation, account cookie cache miss, non-blocking scrypt) are documented in `CHANGELOG.md`.
v1.6.0 changed `to-auth-endpoints.ts:108-109` so `shouldReturnResponse`
defaults to true whenever `ctx.request instanceof Request`. The convex
and cross-domain plugins call `jwt.endpoints.getToken`,
`jwt.endpoints.getJwks`, `oidcProvider.endpoints.getOpenIdConfig`, and
`oneTimeToken.endpoints.verifyOneTimeToken` from inside hook handlers
where `ctx.request` is a real `Request`. Without `asResponse: false`
those calls now return `Response` objects instead of the expected
payload. The destructured `{ token }` becomes undefined, JWT cookies
get set to the literal string `"undefined"`, and the existing
`try/catch` at `convex/index.ts:347-351` swallows the failure. Tests
stay green; production auth silently breaks.
Pass `asResponse: false` at all 7 sites (6 in `convex/index.ts`, 1 in
`cross-domain/index.ts`). Add a hook-level regression test that mocks
`jwt.endpoints.getToken` with v1.6.0 wrap-on-Request semantics and
asserts the resulting `set-cookie` header contains a real token, not
`"undefined"`.
…oken Mirrors the convex jwt cookie regression test. Mocks better-auth/plugins/one-time-token to return a Response unless asResponse === false (matching v1.6.0 wrap-on-Request semantics) and asserts the cross-domain endpoint forwards asResponse: false to the inner call. Without the fix, setSessionCookie would crash on response.session.token being undefined and the OTT verification would fail loudly mid-flow.
Convex indexes are byte-compared so checkUniqueFields cannot enforce case-insensitive uniqueness. Better Auth doesn't currently mark unique fields insensitive, but if it ever does, uniqueness must be enforced via a separate normalized field. One-line guard against a silent future regression.
Adds 4 verified inherited changes downstream Convex users will observe: - requestPasswordReset now runs originCheck on redirectTo (#8392). Reset URLs not in trustedOrigins return 403. - checkPassword throws INVALID_PASSWORD instead of CREDENTIAL_ACCOUNT_NOT_FOUND for missing-credential cases (#8902, in 1.6.1). UI catching the old code in deleteUser, changePassword, and 2FA flows must update. - genericOAuth with verification.storeIdentifier: hashed no longer double-hashes state (#8980). - auth.$Infer and auth.$ERROR_CODES no longer collapse to any when generic options bleed through (#8981). Strict TS users may see new compile errors. Also adds a Migration section pointing at npx auth upgrade and notes the ~46% smaller better-auth install. Wires the new cross-domain regression test entry into the bullet list.
Pre-set inputCtx flags to true so the spread inside the handler carries true forward unless production explicitly overrides. Verified by temporarily dropping the override in cross-domain/index.ts:188 and watching the test fail with 'expected true to be false'. Catches CodeRabbit's gap: with the original false values, a refactor that drops asResponse: false would still produce asResponse: false via the spread and the assertion would pass spuriously.
better-auth 1.6.2 (#8711) adds a verified boolean to the twoFactor table. Without this field in the component schema, enableTwoFactor writes fail with ArgumentValidationError. Also bumps peer and dev deps to >=1.6.2, aligns dev dep ranges with the peer range, documents the twoFactor migration in the changelog, and adds returnHeaders/returnStatus assertions to the convex plugin regression test for parity with the cross-domain test.
Our local parseSetCookieHeader used a naive header.split(", ") which
mis-split Set-Cookie values containing RFC-1123 Expires dates
("Expires=Wed, 21 Oct 2015 07:28:00 GMT") into four garbage entries.
This is exactly the bug better-auth fixed in 1.6.0 (#8301) by adding a
lookahead-aware splitSetCookieHeader in better-auth/cookies.
Delete the local implementation, re-export parseSetCookieHeader from
better-auth/cookies so downstream callers (and the existing test
import) keep working. The CookieAttributes type moves to the
better-auth-shipped shape (value, max-age, expires, domain, path,
secure, httponly, samesite) which aligns with what we already consume
in getSetCookie and crossDomainClient.onSuccess.
Add two regression tests for Set-Cookie headers containing
Expires=... GMT both alone and joined with a sibling cookie.
Prior entries in this CHANGELOG are 3-19 lines. 0.11.0 (the 1.5 migration, direct analog to this one) is 4 lines. The 0.12.0 section had grown to 98 lines with the version-grouped inherited subsections. Collapse the inherited section: keep the GHSA + CSRF + baseURL fixes as a single trailing bullet in the main list, drop the per-version subsections, point readers to better-auth's release notes for the full delta, and fold the critical migration items (schema regen for twoFactor.verified and checkPassword error code change) into the Migration prose. Result: 23 lines, in range with 0.10.0's historical maximum.
98a11ea to
b8380ba
Compare
|
Blocked on better-auth/better-auth#9340 |
ea42a16 to
97c15c5
Compare
|
@erquhart we made a specific release for this https://github.com/better-auth/better-auth/releases/tag/v1.6.9 |
1.6.7 and 1.6.8 crash the Convex V8 isolate on every /api/auth/* request via a bare @opentelemetry/api import from @better-auth/core. 1.6.9 (better-auth/better-auth#9340) routes the import through a package self-reference so browser/edge export conditions resolve to a noop under Convex's esbuild. - bump peer better-auth to >=1.6.9 <1.7.0 - bump dev deps better-auth, @better-auth/core, @better-auth/test-utils to ~1.6.9 - refresh root and examples/{next,react,tanstack} lockfiles - update 0.12 migration guide and framework install snippets
|
@gustavovalverde y'all are amazing, thank you! |
Bump peer to
>=1.6.7 <1.7.0and fix five runtime breaks that ship with better-auth 1.6.Where.mode(v1.6.0). TheWheretype gainedmode?: "sensitive" | "insensitive"andCleanedWhere = Required<Where>forces it onto every where clause the adapter receives. BothadapterWhereValidatorand per-tablewhereValidatorreject unknown fields, so every adapter call throwsArgumentValidationErrorthe moment a user bumps the peer.shouldReturnResponse(v1.6.0, commit8304f65).to-auth-endpoints.tsnow defaults to returning aResponsewhen the context carries aRequest(const shouldReturnResponse = context?.asResponse ?? hasRequest). In 1.5.x this was justcontext?.asResponse. The convex and cross-domain plugins call internal endpoints from hook handlers wherectx.requestis real. WithoutasResponse: falsethe destructured{ token }isundefined, JWT cookies get the literal string"undefined", and cross-domainsetSessionCookiecrashes onresponse.session.token. Test suite stays green because test contexts lack a realRequest. Production auth silently breaks.twoFactor.verified(v1.6.2, PR #8711). Newverifiedboolean column on thetwoFactortable.enableTwoFactorwrites{ verified: false }, Convex validator rejects unknown fields, 2FA enrollment crashes.parseSetCookieHeader(pre-existing, caught in the 1.6.5 audit).src/plugins/cross-domain/client.tsshipped a local copy that split on", "and shatteredExpires=Wed, 21 Oct 2015 07:28:00 GMTinto four garbage cookies. better-auth fixed its own splitter in 1.6.0 (#8301) with a lookahead heuristic. Delegate tobetter-auth/cookies, drop the 34-line local copy. Every cross-domain user with a session cookie carryingExpireshits this (effectively everyone oncesession.expiresInkicks in)../instrumentation(v1.6.6, #9111).packages/core/src/instrumentation/api.tswires a dynamicimport("@opentelemetry/api")into everywithSpancall. Convex's V8 isolate rejects bare specifiers synchronously fromimport()instead of rejecting the returned promise, so the inline.catch()never runs. Every/api/auth/*request 500s with[Better Auth]: Error: Relative import path "@opentelemetry/api" not prefixed with / or ./ or ../. 1.6.5 was the last safe release, 1.6.7 the first with the fix. Not patched in this component: this PR raises the peer floor to 1.6.7 where #9281 routes./instrumentationto a noop onbrowser/edgeexport conditions, which Convex's esbuild picks up viaplatform: "browser".Verified against a live Convex deployment running
tanstack-convex-starter. Self e2e (4 Playwright), upstreamcaseInsensitiveTestSuite(12 tests), and three new hook-level regression tests (asResponseconvex,asResponsecross-domain, Set-CookieExpires). Build / typecheck / vitest (183 passed) / lint all clean.Breaking:
^0.11.xusers must bumpbetter-authto^1.6.7in their ownpackage.json.chore(deps)!: peer>=1.6.7 <1.7.0, dev deps^1.6.7, version0.12.0. Pulls in GHSA-xr8f-h2gw-9xh6 (authz bypass in@better-auth/oauth-provider, 1.6.5), 1.6.2 OAuth state CSRF fix (#8949), 1.6.3 baseURL hardening for directauth.api.*calls (#9113, #9131), 1.6.5$sessionSignalfix for/change-passwordand/revoke-other-sessions(#9087), 1.6.6 SSRF hardening in@better-auth/oauth-provider(#9226), and 1.6.7./instrumentationnoop for Convex (#9281)feat(adapter): validators acceptmode: "sensitive" | "insensitive".findIndexexcludes insensitive clauses from the indexable set (Convex indexes are byte-compared).paginateunique/in fast-paths skip them.filterByWherecase-folds foreq/ne/in/not_in/contains/starts_with/ends_withand normalizesundefinedasnullfor null comparisonsfix(plugins): passasResponse: false, returnHeaders: false, returnStatus: falseat all 7 internal.endpoints.*call sites. Regression tests mock the inner plugin to return aResponseunlessasResponse === false. Pre-set outer flags totrueso dropping the override fails the assertionfix(cross-domain): delete localparseSetCookieHeader, re-export frombetter-auth/cookies. Two regression tests pin behavior onExpires=Wed, 21 Oct 2015 07:28:00 GMTalone and joined with a sibling cookiefix(schema): addverified: v.optional(v.union(v.null(), v.boolean()))to twoFactor table for 1.6.2 compatfeat(plugins): exposeversiononconvex,convexClient,crossDomain,crossDomainClientchore:__skipDeprecationWarning: trueon both internaloidcProviderinstantiationstest: wire upstreamcaseInsensitiveTestSuite(12 tests) into base adapter profiledocs(changelog):0.12.0entry with migration notes. Migration folds in thecheckPassword→INVALID_PASSWORDswitch (1.6.1 #8902) and thetwoFactorschema regen. Full inherited behavior delta lives in better-auth's own release notes, linked outCloses #330, closes #324.
Supersedes #328, #326.
Rebased onto
mainat v0.11.5.See also #329 (React token cache invalidation on session rotation). Orthogonal, no dependency.
Summary by CodeRabbit
Release Notes
Breaking Changes
better-authversion increased to 1.6.7 or higher. Update your dependency constraints accordingly.New Features
Schema Updates
verifiedfield to track verification status.Bug Fixes
parseSetCookieHeadernow handles Set-Cookie headers containingExpiresdates with commas.