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