mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-19 11:24:57 -05:00
fix(core): make consumeOne and incrementOne optional on custom adapters
1.7.0 made both methods required on CustomAdapter and removed the factory fallback, so a custom adapter written for 1.6 type-checks against nothing and throws at request time on the first OTP, magic link, password reset, or two-factor attempt. Restore the fallbacks. consumeOne without a native implementation runs findOne then an id-guarded deleteMany and wins only when exactly one row was deleted. incrementOne runs a bounded compare-and-swap loop: findOne, then updateMany guarded on the id and the counter values just read, retried when a concurrent writer changed the row first. Both are single guarded statements per attempt, so they stay atomic across processes without transactions.
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@better-auth/core": patch
|
||||
---
|
||||
|
||||
Make `consumeOne` and `incrementOne` optional on custom adapters again. When an adapter omits them, the factory falls back to `findOne` plus an id-guarded `deleteMany`, or a compare-and-swap `updateMany` loop, instead of throwing at request time.
|
||||
@@ -707,11 +707,11 @@ Two OAuth-provider behaviors read the incoming request origin. Behind a custom s
|
||||
|
||||
The atomic-state work introduces required methods. If you use only the built-in adapters and storage, you can skip this.
|
||||
|
||||
### Database adapters must implement `incrementOne` and `consumeOne`
|
||||
### Database adapters can implement `incrementOne` and `consumeOne`
|
||||
|
||||
`incrementOne` updates one row's counter atomically and returns the row, or null when the guard did not match. `consumeOne` reads and deletes a row in one step for single-use credentials. Both are now required, and the old fallback is gone.
|
||||
`incrementOne` updates one row's counter atomically and returns the row, or null when the guard did not match. `consumeOne` reads and deletes a row in one step for single-use credentials. Both are optional. When a custom adapter omits them, Better Auth falls back to `findOne` plus an id-guarded `deleteMany` or a compare-and-swap `updateMany`, which stay atomic across processes because each is a single guarded statement.
|
||||
|
||||
**What to do:** implement both `incrementOne` and `consumeOne` in any custom adapter. A missing `consumeOne` throws at runtime. All built-in adapters already do.
|
||||
**What to do:** nothing is required. Implement both natively in a custom adapter to save a round trip on hot paths. All built-in adapters already do.
|
||||
|
||||
### Secondary storage must implement `increment` and `getAndDelete`
|
||||
|
||||
|
||||
@@ -271,8 +271,8 @@ consumeOne: async ({ model, where }) => {
|
||||
};
|
||||
```
|
||||
|
||||
<Callout type="warn">
|
||||
`consumeOne` is required. Better Auth does not provide a `findMany` plus `deleteMany` fallback because that shape cannot guarantee single-use semantics across adapters without a native atomic delete-and-return operation.
|
||||
<Callout type="info">
|
||||
`consumeOne` is optional. When it is missing, Better Auth falls back to `findOne` followed by a `deleteMany` guarded on the row id, and treats the caller as the winner only when exactly one row was deleted. Implement it natively to save a round trip.
|
||||
</Callout>
|
||||
|
||||
### `incrementOne` method
|
||||
@@ -302,8 +302,8 @@ incrementOne: async ({ model, where, increment, set }) => {
|
||||
};
|
||||
```
|
||||
|
||||
<Callout type="warn">
|
||||
`incrementOne` is required. Better Auth uses it as a compare-and-swap primitive: the `where` clause is part of the guard, so concurrent requests cannot all update a stale snapshot.
|
||||
<Callout type="info">
|
||||
`incrementOne` is optional. When it is missing, Better Auth falls back to a compare-and-swap loop: `findOne`, then `updateMany` guarded on the row id and the counter values it just read, retried when a concurrent writer got there first. Implement it natively for one round trip under contention.
|
||||
</Callout>
|
||||
|
||||
### `findOne` method
|
||||
|
||||
@@ -209,10 +209,55 @@ describe("createAdapterFactory atomic primitives", () => {
|
||||
).rejects.toThrow(/resolved to an empty update/);
|
||||
});
|
||||
|
||||
it("throws a clear error when consumeOne is missing at runtime", async () => {
|
||||
it("falls back to findOne plus id-guarded deleteMany when consumeOne is missing", async () => {
|
||||
const deleteCalls: CleanedWhere[][] = [];
|
||||
const adapter = createTestAdapter({
|
||||
adapter: createCustomAdapter({
|
||||
consumeOne: undefined as unknown as CustomAdapter["consumeOne"],
|
||||
consumeOne: undefined,
|
||||
findOne: async <T>() =>
|
||||
({ id: "verification-id", identifier_text: "stored-token" }) as T,
|
||||
deleteMany: async ({ where }) => {
|
||||
deleteCalls.push(where);
|
||||
return 1;
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
const result = await adapter.consumeOne<{ id: string; identifier: string }>(
|
||||
{
|
||||
model: "verification",
|
||||
where: [{ field: "identifier", value: "token" }],
|
||||
},
|
||||
);
|
||||
|
||||
expect(result).toEqual({
|
||||
id: "verification-id",
|
||||
identifier: "stored-token:output",
|
||||
});
|
||||
expect(deleteCalls[0]).toEqual([
|
||||
{
|
||||
field: "identifier_text",
|
||||
value: "token:consumeOne",
|
||||
operator: "eq",
|
||||
connector: "AND",
|
||||
mode: "sensitive",
|
||||
},
|
||||
{
|
||||
field: "id",
|
||||
value: "verification-id",
|
||||
operator: "eq",
|
||||
connector: "AND",
|
||||
mode: "sensitive",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("returns null from the consumeOne fallback when another caller deleted the row first", async () => {
|
||||
const adapter = createTestAdapter({
|
||||
adapter: createCustomAdapter({
|
||||
consumeOne: undefined,
|
||||
findOne: async <T>() => ({ id: "verification-id" }) as T,
|
||||
deleteMany: async () => 0,
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -221,13 +266,100 @@ describe("createAdapterFactory atomic primitives", () => {
|
||||
model: "verification",
|
||||
where: [{ field: "identifier", value: "token" }],
|
||||
}),
|
||||
).rejects.toThrow(/must implement consumeOne/);
|
||||
).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it("throws a clear error when incrementOne is missing at runtime", async () => {
|
||||
it("falls back to a compare-and-swap updateMany when incrementOne is missing", async () => {
|
||||
const updateCalls: {
|
||||
where: CleanedWhere[];
|
||||
update: Record<string, any>;
|
||||
}[] = [];
|
||||
let attemptCount = 2;
|
||||
const adapter = createTestAdapter({
|
||||
adapter: createCustomAdapter({
|
||||
incrementOne: undefined as unknown as CustomAdapter["incrementOne"],
|
||||
incrementOne: undefined,
|
||||
findOne: async <T>() =>
|
||||
({ id: "verification-id", attempt_count: attemptCount }) as T,
|
||||
updateMany: async ({ where, update }) => {
|
||||
updateCalls.push({ where, update });
|
||||
attemptCount = update.attempt_count;
|
||||
return 1;
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
const result = await adapter.incrementOne<{ id: string; attempts: number }>(
|
||||
{
|
||||
model: "verification",
|
||||
where: [{ field: "identifier", value: "token" }],
|
||||
increment: { attempts: 1 },
|
||||
},
|
||||
);
|
||||
|
||||
expect(result).toEqual({ id: "verification-id", attempts: 3 });
|
||||
expect(updateCalls).toHaveLength(1);
|
||||
expect(updateCalls[0]!.update).toEqual({ attempt_count: 3 });
|
||||
expect(updateCalls[0]!.where).toEqual([
|
||||
{
|
||||
field: "identifier_text",
|
||||
value: "token:incrementOne",
|
||||
operator: "eq",
|
||||
connector: "AND",
|
||||
mode: "sensitive",
|
||||
},
|
||||
{
|
||||
field: "id",
|
||||
value: "verification-id",
|
||||
operator: "eq",
|
||||
connector: "AND",
|
||||
mode: "sensitive",
|
||||
},
|
||||
{
|
||||
field: "attempt_count",
|
||||
value: 2,
|
||||
operator: "eq",
|
||||
connector: "AND",
|
||||
mode: "sensitive",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("retries the incrementOne fallback after losing a compare-and-swap race", async () => {
|
||||
let stored = 2;
|
||||
let updateCalls = 0;
|
||||
const adapter = createTestAdapter({
|
||||
adapter: createCustomAdapter({
|
||||
incrementOne: undefined,
|
||||
findOne: async <T>() =>
|
||||
({ id: "verification-id", attempt_count: stored }) as T,
|
||||
updateMany: async ({ update }) => {
|
||||
updateCalls++;
|
||||
if (updateCalls === 1) {
|
||||
// A concurrent writer bumped the counter between read and write.
|
||||
stored = 3;
|
||||
return 0;
|
||||
}
|
||||
stored = update.attempt_count;
|
||||
return 1;
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
const result = await adapter.incrementOne<{ attempts: number }>({
|
||||
model: "verification",
|
||||
where: [{ field: "identifier", value: "token" }],
|
||||
increment: { attempts: 1 },
|
||||
});
|
||||
|
||||
expect(updateCalls).toBe(2);
|
||||
expect(result?.attempts).toBe(4);
|
||||
});
|
||||
|
||||
it("returns null from the incrementOne fallback when the guard matches no row", async () => {
|
||||
const adapter = createTestAdapter({
|
||||
adapter: createCustomAdapter({
|
||||
incrementOne: undefined,
|
||||
findOne: async () => null,
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -237,7 +369,7 @@ describe("createAdapterFactory atomic primitives", () => {
|
||||
where: [{ field: "identifier", value: "token" }],
|
||||
increment: { attempts: 1 },
|
||||
}),
|
||||
).rejects.toThrow(/must implement incrementOne/);
|
||||
).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it("throws a clear error when updateMany does not return a finite count", async () => {
|
||||
|
||||
@@ -1366,18 +1366,47 @@ export const createAdapterFactory =
|
||||
{ model, where },
|
||||
);
|
||||
|
||||
if (typeof adapterInstance.consumeOne !== "function") {
|
||||
throw new BetterAuthError(
|
||||
`Adapter "${config.adapterId}" must implement consumeOne for atomic single-use credential consumption.`,
|
||||
);
|
||||
}
|
||||
const nativeConsumeOne = adapterInstance.consumeOne;
|
||||
const res = await withSpan(
|
||||
`db consumeOne ${model}`,
|
||||
{
|
||||
[ATTR_DB_OPERATION_NAME]: "consumeOne",
|
||||
[ATTR_DB_COLLECTION_NAME]: model,
|
||||
},
|
||||
() => adapterInstance.consumeOne<T>({ model, where }),
|
||||
async () => {
|
||||
if (typeof nativeConsumeOne === "function") {
|
||||
return nativeConsumeOne<T>({ model, where });
|
||||
}
|
||||
// Fallback for adapters without a native consumeOne: read the
|
||||
// candidate, then delete it guarded on its id. A single-row
|
||||
// DELETE is atomic on every supported store, so exactly one of
|
||||
// several concurrent callers sees a deleted count of 1.
|
||||
const idField = getFieldName({ model: unsafeModel, field: "id" });
|
||||
const target = await adapterInstance.findOne<Record<string, any>>({
|
||||
model,
|
||||
where,
|
||||
});
|
||||
if (!target) return null;
|
||||
const deleted = await adapterInstance.deleteMany({
|
||||
model,
|
||||
where: [
|
||||
...where,
|
||||
{
|
||||
field: idField,
|
||||
value: target[idField],
|
||||
operator: "eq",
|
||||
connector: "AND",
|
||||
mode: "sensitive",
|
||||
},
|
||||
],
|
||||
});
|
||||
if (typeof deleted !== "number" || !Number.isFinite(deleted)) {
|
||||
throw new BetterAuthError(
|
||||
`Adapter "${config.adapterId}" returned a non-numeric value from deleteMany during the consumeOne fallback. Return the number of deleted rows, or implement a native consumeOne.`,
|
||||
);
|
||||
}
|
||||
return deleted > 0 ? (target as T) : null;
|
||||
},
|
||||
);
|
||||
|
||||
debugLog(
|
||||
@@ -1440,11 +1469,6 @@ export const createAdapterFactory =
|
||||
{ model, where, increment: unsafeIncrement, set: unsafeSet },
|
||||
);
|
||||
|
||||
if (typeof adapterInstance.incrementOne !== "function") {
|
||||
throw new BetterAuthError(
|
||||
`Adapter "${config.adapterId}" must implement incrementOne for atomic guarded counter updates.`,
|
||||
);
|
||||
}
|
||||
const mappedKeys = config.mapKeysTransformInput ?? {};
|
||||
const increment: Record<string, number> = {};
|
||||
for (const [field, delta] of Object.entries(unsafeIncrement)) {
|
||||
@@ -1466,19 +1490,69 @@ export const createAdapterFactory =
|
||||
"incrementOne resolved to an empty update: every increment/set field was unknown to the schema or transformed away.",
|
||||
);
|
||||
}
|
||||
const nativeIncrementOne = adapterInstance.incrementOne;
|
||||
const res = await withSpan(
|
||||
`db incrementOne ${model}`,
|
||||
{
|
||||
[ATTR_DB_OPERATION_NAME]: "incrementOne",
|
||||
[ATTR_DB_COLLECTION_NAME]: model,
|
||||
},
|
||||
() =>
|
||||
adapterInstance.incrementOne<T>({
|
||||
model,
|
||||
where,
|
||||
increment,
|
||||
set,
|
||||
}),
|
||||
async () => {
|
||||
if (typeof nativeIncrementOne === "function") {
|
||||
return nativeIncrementOne<T>({ model, where, increment, set });
|
||||
}
|
||||
// Fallback for adapters without a native incrementOne: a
|
||||
// compare-and-swap loop. Read the row, then update it guarded on
|
||||
// its id and the counter values just read. A lost race returns
|
||||
// an affected count of 0 while the selector still matches, so
|
||||
// retry; a selector that no longer matches returns null.
|
||||
const idField = getFieldName({ model: unsafeModel, field: "id" });
|
||||
// Bounded retries: adapters under heavy contention should implement
|
||||
// incrementOne natively instead of relying on this loop.
|
||||
for (let attempt = 0; attempt < 5; attempt++) {
|
||||
const current = await adapterInstance.findOne<
|
||||
Record<string, any>
|
||||
>({ model, where });
|
||||
if (!current) return null;
|
||||
const idWhere: CleanedWhere = {
|
||||
field: idField,
|
||||
value: current[idField],
|
||||
operator: "eq",
|
||||
connector: "AND",
|
||||
mode: "sensitive",
|
||||
};
|
||||
const guard: CleanedWhere[] = [...where, idWhere];
|
||||
const update: Record<string, unknown> = { ...(set ?? {}) };
|
||||
for (const [field, delta] of Object.entries(increment)) {
|
||||
const previous = current[field];
|
||||
update[field] =
|
||||
(typeof previous === "number" ? previous : 0) + delta;
|
||||
if (typeof previous === "number") {
|
||||
guard.push({
|
||||
field,
|
||||
value: previous,
|
||||
operator: "eq",
|
||||
connector: "AND",
|
||||
mode: "sensitive",
|
||||
});
|
||||
}
|
||||
}
|
||||
const updated = await adapterInstance.updateMany({
|
||||
model,
|
||||
where: guard,
|
||||
update,
|
||||
});
|
||||
if (typeof updated !== "number" || !Number.isFinite(updated)) {
|
||||
throw new BetterAuthError(
|
||||
`Adapter "${config.adapterId}" returned a non-numeric value from updateMany during the incrementOne fallback. Return the number of updated rows, or implement a native incrementOne.`,
|
||||
);
|
||||
}
|
||||
if (updated > 0) {
|
||||
return adapterInstance.findOne<T>({ model, where: [idWhere] });
|
||||
}
|
||||
}
|
||||
return null;
|
||||
},
|
||||
);
|
||||
|
||||
debugLog(
|
||||
|
||||
@@ -653,8 +653,12 @@ export interface CustomAdapter {
|
||||
* `findOneAndDelete`, `OUTPUT deleted.*`) gives one round trip and the
|
||||
* strongest race-safety guarantee. Implementations must delete at most
|
||||
* one matching row.
|
||||
*
|
||||
* Optional. When omitted, the factory falls back to `findOne` followed by a
|
||||
* `deleteMany` guarded on the row id, and treats the caller as the winner
|
||||
* only when exactly one row was deleted.
|
||||
*/
|
||||
consumeOne: <T>(data: {
|
||||
consumeOne?: <T>(data: {
|
||||
model: string;
|
||||
where: CleanedWhere[];
|
||||
}) => Promise<T | null>;
|
||||
@@ -668,8 +672,12 @@ export interface CustomAdapter {
|
||||
* Implementing this natively (e.g. `UPDATE ... SET n = n + $delta WHERE ...
|
||||
* RETURNING *`) gives one round trip and the strongest race-safety
|
||||
* guarantee.
|
||||
*
|
||||
* Optional. When omitted, the factory falls back to a compare-and-swap
|
||||
* loop: `findOne`, then `updateMany` guarded on the row id and the current
|
||||
* counter values, retried when a concurrent writer changed the row first.
|
||||
*/
|
||||
incrementOne: <T>(data: {
|
||||
incrementOne?: <T>(data: {
|
||||
model: string;
|
||||
where: CleanedWhere[];
|
||||
increment: Record<string, number>;
|
||||
|
||||
Reference in New Issue
Block a user