From 33be9fa6f8f31f8a07ed9f0953d6925a84b0c5a1 Mon Sep 17 00:00:00 2001 From: kuldeepsinh19 Date: Mon, 2 Nov 2026 09:28:57 +0530 Subject: [PATCH] fix(better-auth): support compound where in update and delete The adapter turned a where with two or more conditions into `{ AND: [...] }` and passed it to ORM `update()`/`delete()`, which need a unique field at the top level. better-auth updates with compound wheres (e.g. @better-auth/sso row locks, device authorization claims on 1.6.x), so those calls failed with "At least one unique field or field set must be set at where", and compound deletes were silently skipped because `delete()` swallowed every error. `update()`/`delete()` now lift AND-ed equality conditions to the top level and keep the rest as filters. When a single-field unique key is present the row is updated/deleted directly; a missing row returns `null` from `update()` (better-auth's contract) and is ignored by `delete()`, while other errors propagate. Without a unique key, `update()` finds a matching row and updates it by id with the original conditions as a guard, and `delete()` uses `deleteMany()`. Fixes #2694 --- .../auth-adapters/better-auth/package.json | 1 + .../auth-adapters/better-auth/src/adapter.ts | 115 ++++++--- .../better-auth/test/adapter-db.test.ts | 238 ++++++++++++++++++ pnpm-lock.yaml | 3 + 4 files changed, 319 insertions(+), 38 deletions(-) create mode 100644 packages/auth-adapters/better-auth/test/adapter-db.test.ts diff --git a/packages/auth-adapters/better-auth/package.json b/packages/auth-adapters/better-auth/package.json index 8179a10a1..2922dc8dc 100644 --- a/packages/auth-adapters/better-auth/package.json +++ b/packages/auth-adapters/better-auth/package.json @@ -70,6 +70,7 @@ "@types/tmp": "catalog:", "@zenstackhq/cli": "workspace:*", "@zenstackhq/eslint-config": "workspace:*", + "@zenstackhq/testtools": "workspace:*", "@zenstackhq/tsdown-config": "workspace:*", "@zenstackhq/typescript-config": "workspace:*", "@zenstackhq/vitest-config": "workspace:*", diff --git a/packages/auth-adapters/better-auth/src/adapter.ts b/packages/auth-adapters/better-auth/src/adapter.ts index c393f3319..357cca2ce 100644 --- a/packages/auth-adapters/better-auth/src/adapter.ts +++ b/packages/auth-adapters/better-auth/src/adapter.ts @@ -1,5 +1,6 @@ import type { BetterAuthOptions, Where } from 'better-auth'; import { BetterAuthError } from '@better-auth/core/error'; +import { ORMErrorReason, QueryUtils } from '@zenstackhq/orm'; import type { ClientContract, ModelOperations, UpdateInput } from '@zenstackhq/orm'; import type { GetModels, SchemaDef } from '@zenstackhq/orm/schema'; import { @@ -44,6 +45,14 @@ export const zenstackAdapter = (db: ClientContract w.operator === 'eq' || !w.operator; + const convertWhereCondition = (model: string, w: Where) => ({ + [getFieldName({ model, field: w.field })]: isEqualityCondition(w) + ? w.value + : { + [operatorToORMOperator(w.operator!)]: w.value, + }, + }); const convertWhereClause = (model: string, where?: Where[]): any => { if (!where || !where.length) return {}; if (where.length === 1) { @@ -51,37 +60,12 @@ export const zenstackAdapter = (db: ClientContract w.connector === 'AND' || !w.connector); const or = where.filter((w) => w.connector === 'OR'); - const andClause = and.map((w) => { - return { - [getFieldName({ model, field: w.field })]: - w.operator === 'eq' || !w.operator - ? w.value - : { - [operatorToORMOperator(w.operator)]: w.value, - }, - }; - }); - const orClause = or.map((w) => { - return { - [getFieldName({ model, field: w.field })]: - w.operator === 'eq' || !w.operator - ? w.value - : { - [operatorToORMOperator(w.operator)]: w.value, - }, - }; - }); + const andClause = and.map((w) => convertWhereCondition(model, w)); + const orClause = or.map((w) => convertWhereCondition(model, w)); return { ...(andClause.length ? { AND: andClause } : {}), @@ -89,6 +73,37 @@ export const zenstackAdapter = (db: ClientContract { + const result: Record = {}; + const and: Record[] = []; + const or: Record[] = []; + for (const w of where) { + const field = getFieldName({ model, field: w.field }); + if (w.connector === 'OR') { + or.push(convertWhereCondition(model, w)); + } else if (isEqualityCondition(w) && !(field in result)) { + result[field] = w.value; + } else { + and.push(convertWhereCondition(model, w)); + } + } + if (and.length) result['AND'] = and; + if (or.length) result['OR'] = or; + + const isUnique = QueryUtils.getUniqueFields(db.$schema, model).some( + (unique) => 'def' in unique && result[unique.name] !== undefined && result[unique.name] !== null, + ); + return { where: result, isUnique }; + }; + + // checked by `reason` rather than `instanceof ORMError`, in case the app loads another copy of the ORM + const isNotFoundError = (err: unknown) => + (err as { reason?: unknown } | null)?.reason === ORMErrorReason.NOT_FOUND; + function requireModelDb(db: ClientContract, model: string) { const modelDb = db[model as keyof typeof db]; if (!modelDb) { @@ -145,11 +160,32 @@ export const zenstackAdapter = (db: ClientContract { const modelDb = requireModelDb(db, model); - const whereClause = convertWhereClause(model, where); - return await modelDb.update({ - where: whereClause, - data: update as UpdateInput, any>, - }); + const data = update as UpdateInput, any>; + const uniqueWhere = convertUniqueWhereClause(model, where); + let updateWhere = uniqueWhere.where; + if (!uniqueWhere.isUnique) { + // No unique field to select the row by: find a matching row, then update it by its id, + // keeping the original conditions as a guard in case the row changed in between. + const filter = convertWhereClause(model, where); + const idFields = QueryUtils.requireIdFields(db.$schema, model); + const found: any = await modelDb.findFirst({ + where: filter, + select: Object.fromEntries(idFields.map((f) => [f, true])), + }); + if (!found) return null; + const idWhere = + idFields.length === 1 + ? { [idFields[0]!]: found[idFields[0]!] } + : { [idFields.join('_')]: Object.fromEntries(idFields.map((f) => [f, found[f]])) }; + updateWhere = { ...idWhere, AND: [filter] }; + } + try { + return await modelDb.update({ where: updateWhere as any, data }); + } catch (err) { + // better-auth expects `null` when no row matches + if (isNotFoundError(err)) return null; + throw err; + } }, async updateMany({ model, where, update }) { @@ -164,13 +200,16 @@ export const zenstackAdapter = (db: ClientContract { const modelDb = requireModelDb(db, model); - const whereClause = convertWhereClause(model, where); + const uniqueWhere = convertUniqueWhereClause(model, where); + if (!uniqueWhere.isUnique) { + await modelDb.deleteMany({ where: convertWhereClause(model, where) }); + return; + } try { - await modelDb.delete({ - where: whereClause, - }); - } catch { + await modelDb.delete({ where: uniqueWhere.where as any }); + } catch (err) { // If the record doesn't exist, we don't want to throw an error + if (!isNotFoundError(err)) throw err; } }, diff --git a/packages/auth-adapters/better-auth/test/adapter-db.test.ts b/packages/auth-adapters/better-auth/test/adapter-db.test.ts new file mode 100644 index 000000000..a6ff14265 --- /dev/null +++ b/packages/auth-adapters/better-auth/test/adapter-db.test.ts @@ -0,0 +1,238 @@ +import type { BetterAuthOptions } from '@better-auth/core'; +import type { BetterAuthDBSchema } from 'better-auth/db'; +import { createTestClient } from '@zenstackhq/testtools'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { zenstackAdapter } from '../src/adapter'; + +const zmodel = ` +model SsoProvider { + id String @id + providerId String @unique + issuer String +} + +model DeviceCode { + id String @id + status String + userId String? +} + +model ApiToken { + id String @id + token String @unique + userId String +} +`; + +// Plugin models, so better-auth's adapter factory knows the fields and transforms `where` like it does at runtime. +const pluginSchema = { + ssoProvider: { + modelName: 'ssoProvider', + fields: { + providerId: { type: 'string', required: true, unique: true }, + issuer: { type: 'string', required: true }, + }, + }, + deviceCode: { + modelName: 'deviceCode', + fields: { + status: { type: 'string', required: true }, + userId: { type: 'string', required: false }, + }, + }, + apiToken: { + modelName: 'apiToken', + fields: { + token: { type: 'string', required: true, unique: true }, + userId: { type: 'string', required: true }, + }, + }, +} satisfies BetterAuthDBSchema; + +const authOptions = { + plugins: [{ id: 'test-models', schema: pluginSchema }], +} as unknown as BetterAuthOptions; + +describe('ZenStack Better Auth adapter with a database', () => { + let db: any; + let adapter: ReturnType>; + + beforeEach(async () => { + db = await createTestClient(zmodel); + const provider = db.$schema.provider.type === 'postgresql' ? 'postgresql' : 'sqlite'; + adapter = zenstackAdapter(db, { provider })(authOptions); + }); + + afterEach(async () => { + await db?.$disconnect(); + }); + + it('updates with a unique field plus extra conditions, like @better-auth/sso row locks (#2694)', async () => { + await db.ssoProvider.create({ data: { id: 'sso-1', providerId: 'acme', issuer: 'https://idp.acme.test' } }); + + const row = await adapter.update<{ id: string; providerId: string }>({ + model: 'ssoProvider', + where: [ + { field: 'id', value: 'sso-1' }, + { field: 'providerId', value: 'acme' }, + ], + update: { providerId: 'acme' }, + }); + + expect(row).toMatchObject({ id: 'sso-1', providerId: 'acme' }); + }); + + it('updates with a compound where inside a transaction (#2694)', async () => { + await db.ssoProvider.create({ data: { id: 'sso-1', providerId: 'acme', issuer: 'https://idp.acme.test' } }); + + const row = await adapter.transaction((trx) => + trx.update<{ id: string; issuer: string }>({ + model: 'ssoProvider', + where: [ + { field: 'id', value: 'sso-1' }, + { field: 'providerId', value: 'acme' }, + ], + update: { issuer: 'https://idp2.acme.test' }, + }), + ); + + expect(row).toMatchObject({ id: 'sso-1', issuer: 'https://idp2.acme.test' }); + }); + + it('updates a single row when the where has no unique field (#2694)', async () => { + await db.deviceCode.create({ data: { id: 'device-1', status: 'pending', userId: null } }); + await db.deviceCode.create({ data: { id: 'device-2', status: 'pending', userId: null } }); + + const row = await adapter.update<{ id: string }>({ + model: 'deviceCode', + where: [{ field: 'status', value: 'pending' }], + update: { status: 'approved' }, + }); + + await expect(db.deviceCode.count({ where: { status: 'approved' } })).resolves.toBe(1); + await expect(db.deviceCode.findUnique({ where: { id: row!.id } })).resolves.toMatchObject({ + status: 'approved', + }); + }); + + it('returns null when a compound update matches nothing (#2694)', async () => { + await db.ssoProvider.create({ data: { id: 'sso-1', providerId: 'acme', issuer: 'https://idp.acme.test' } }); + + const row = await adapter.update({ + model: 'ssoProvider', + where: [ + { field: 'id', value: 'sso-1' }, + { field: 'providerId', value: 'other' }, + ], + update: { providerId: 'other' }, + }); + + expect(row).toBeNull(); + await expect(db.ssoProvider.findUnique({ where: { id: 'sso-1' } })).resolves.toMatchObject({ + providerId: 'acme', + }); + }); + + it('claims a row only once with a compare-and-swap update, like device authorization (#2694)', async () => { + await db.deviceCode.create({ data: { id: 'device-1', status: 'pending', userId: null } }); + const claim = (userId: string) => + adapter.update<{ id: string; userId: string | null }>({ + model: 'deviceCode', + where: [ + { field: 'id', value: 'device-1' }, + { field: 'status', value: 'pending' }, + { field: 'userId', value: null }, + ], + update: { userId }, + }); + + await expect(claim('user-1')).resolves.toMatchObject({ id: 'device-1', userId: 'user-1' }); + await expect(claim('user-2')).resolves.toBeNull(); + await expect(db.deviceCode.findUnique({ where: { id: 'device-1' } })).resolves.toMatchObject({ + userId: 'user-1', + }); + }); + + it('updates without a unique field and returns the updated row (#2694)', async () => { + await db.deviceCode.create({ data: { id: 'device-1', status: 'pending', userId: null } }); + + const row = await adapter.update({ + model: 'deviceCode', + where: [{ field: 'status', value: 'pending' }], + update: { status: 'approved' }, + }); + + // the update changes the filtered field, so the row must not be read back with the original filter + expect(row).toMatchObject({ id: 'device-1', status: 'approved' }); + }); + + it('returns null when updating a missing row by id (#2694)', async () => { + const row = await adapter.update({ + model: 'deviceCode', + where: [{ field: 'id', value: 'missing' }], + update: { status: 'approved' }, + }); + + expect(row).toBeNull(); + }); + + it('updates rows matched with OR conditions (#2694)', async () => { + await db.deviceCode.create({ data: { id: 'device-1', status: 'pending', userId: null } }); + + const row = await adapter.update({ + model: 'deviceCode', + where: [ + { field: 'status', value: 'denied', connector: 'OR' }, + { field: 'status', value: 'pending', connector: 'OR' }, + ], + update: { status: 'approved' }, + }); + + expect(row).toMatchObject({ id: 'device-1', status: 'approved' }); + }); + + it('deletes with a unique field plus extra conditions (#2694)', async () => { + await db.apiToken.create({ data: { id: 'token-1', token: 'secret', userId: 'user-1' } }); + + await adapter.delete({ + model: 'apiToken', + where: [ + { field: 'id', value: 'token-1' }, + { field: 'userId', value: 'user-1' }, + ], + }); + + await expect(db.apiToken.findUnique({ where: { id: 'token-1' } })).resolves.toBeNull(); + }); + + it('keeps the row when a compound delete matches nothing (#2694)', async () => { + await db.apiToken.create({ data: { id: 'token-1', token: 'secret', userId: 'user-1' } }); + + await expect( + adapter.delete({ + model: 'apiToken', + where: [ + { field: 'id', value: 'token-1' }, + { field: 'userId', value: 'user-2' }, + ], + }), + ).resolves.toBeUndefined(); + + await expect(db.apiToken.findUnique({ where: { id: 'token-1' } })).resolves.not.toBeNull(); + }); + + it('deletes without a unique field (#2694)', async () => { + await db.apiToken.create({ data: { id: 'token-1', token: 'secret', userId: 'user-1' } }); + await db.apiToken.create({ data: { id: 'token-2', token: 'other', userId: 'user-2' } }); + + await adapter.delete({ model: 'apiToken', where: [{ field: 'userId', value: 'user-1' }] }); + + await expect(db.apiToken.findMany({ orderBy: { id: 'asc' } })).resolves.toMatchObject([{ id: 'token-2' }]); + }); + + it('does not throw when deleting a missing row by id', async () => { + await expect( + adapter.delete({ model: 'apiToken', where: [{ field: 'id', value: 'missing' }] }), + ).resolves.toBeUndefined(); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8f9f69741..b83bc40ed 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -193,6 +193,9 @@ importers: '@zenstackhq/eslint-config': specifier: workspace:* version: link:../../config/eslint-config + '@zenstackhq/testtools': + specifier: workspace:* + version: link:../../testtools '@zenstackhq/tsdown-config': specifier: workspace:* version: link:../../config/tsdown-config