github-starred/better-auth
2
0
Fork 0
mirror of https://github.com/better-auth/better-auth.git synced 2026-10-19 11:24:57 -05:00
Code Issues 2.5k Packages Projects Releases 112 Wiki Activity

Merge branch 'main' into ping-maxwell/c-ping-maxwell/fix-error-link-apostrophe-f89a

Browse Source
This commit is contained in:
Maxwell
2026-05-20 02:49:34 +10:00
committed by GitHub
parent 76f42c794d f77060af3a
commit 7ef95543cd
10 changed files with 217 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/consume-verification-rejects-expired.md
+6
View File
@@ -0,0 +1,6 @@
---
"better-auth": patch
"@better-auth/oauth-provider": patch
---
Expired magic-link tokens and OAuth authorization codes are now reliably rejected. Magic-link verify redirects to `?error=INVALID_TOKEN` for expired tokens (was `?error=EXPIRED_TOKEN`). The OIDC, MCP, and `@better-auth/oauth-provider` `/token` endpoints return `error_description: "invalid code"` for expired codes (was `"code expired"`). The OAuth `error` value stays `invalid_grant`.
packages/better-auth/src/db/internal-adapter.test.ts
+124
View File
@@ -1546,6 +1546,23 @@ describe("internal adapter test", async () => {
expect(result).toBeNull();
});
it("returns null when the row exists but has already expired", async () => {
const adapter = await makeAdapter();
await adapter.createVerificationValue({
identifier: "consume:expired",
value: "user-expired",
expiresAt: new Date(Date.now() - 1_000),
});
const result = await adapter.consumeVerificationValue("consume:expired");
expect(result).toBeNull();
// The expired row must still be invalidated so a later replay cannot
// consume it after a cleanup pass.
const replay = await adapter.findVerificationValue("consume:expired");
expect(replay).toBeNull();
});
it("aborts the consume when a delete.before hook returns false", async () => {
const veto = vi.fn().mockReturnValue(false);
const adapter = await makeAdapter({
@@ -1678,6 +1695,113 @@ describe("internal adapter test", async () => {
expect(store.has("verification:consume:secondary")).toBe(false);
});
it("returns null when the secondary storage row has already expired", async () => {
const store = new Map<string, string>();
const adapter = await makeAdapter({
verification: { storeInDatabase: false },
secondaryStorage: {
set(key, value) {
store.set(key, value);
},
get(key) {
return store.get(key) ?? null;
},
delete(key) {
store.delete(key);
},
},
});
// Bypass `createVerificationValue`'s TTL gate by writing directly
// with an `expiresAt` already in the past. This mirrors a row that
// was valid when written but reached the consume call after expiry.
store.set(
"verification:consume:secondary-expired",
JSON.stringify({
id: "row-id",
identifier: "consume:secondary-expired",
value: "secondary-expired-user",
expiresAt: new Date(Date.now() - 1_000),
createdAt: new Date(),
updatedAt: new Date(),
}),
);
const result = await adapter.consumeVerificationValue(
"consume:secondary-expired",
);
expect(result).toBeNull();
// The expired row is still consumed (deleted) so it cannot be
// replayed later.
expect(store.has("verification:consume:secondary-expired")).toBe(false);
});
it("rehydrates string `expiresAt` from secondary storage JSON", async () => {
const store = new Map<string, string>();
const adapter = await makeAdapter({
verification: { storeInDatabase: false },
secondaryStorage: {
set(key, value) {
store.set(key, value);
},
get(key) {
return store.get(key) ?? null;
},
delete(key) {
store.delete(key);
},
},
});
await adapter.createVerificationValue({
identifier: "consume:secondary-hydrate",
value: "hydrate-user",
expiresAt: new Date(Date.now() + 60_000),
});
const result = await adapter.consumeVerificationValue(
"consume:secondary-hydrate",
);
expect(result).not.toBeNull();
expect(result!.value).toBe("hydrate-user");
expect(result!.expiresAt).toBeInstanceOf(Date);
expect(result!.expiresAt.getTime()).toBeGreaterThan(Date.now());
});
it("returns null when secondary storage `expiresAt` cannot be parsed", async () => {
const store = new Map<string, string>();
const adapter = await makeAdapter({
verification: { storeInDatabase: false },
secondaryStorage: {
set(key, value) {
store.set(key, value);
},
get(key) {
return store.get(key) ?? null;
},
delete(key) {
store.delete(key);
},
},
});
store.set(
"verification:consume:secondary-invalid-date",
JSON.stringify({
id: "row-id",
identifier: "consume:secondary-invalid-date",
value: "bad-row",
expiresAt: "not-a-date",
createdAt: new Date(),
updatedAt: new Date(),
}),
);
const result = await adapter.consumeVerificationValue(
"consume:secondary-invalid-date",
);
expect(result).toBeNull();
});
it("serializes the secondary storage compatibility fallback within one process", async () => {
const store = new Map<string, string>();
const adapter = await makeAdapter({
packages/better-auth/src/db/internal-adapter.ts
+78 -74
View File
@@ -1230,9 +1230,9 @@ export const createInternalAdapter = (
* return value, because consuming one row invalidates the whole
* identifier and stale rows cannot be replayed.
*
* Expiry is intentionally not checked here. Callers retain their
* existing expiry handling so each call site can surface its own
* error code (for example `token_expired` vs `invalid_grant`).
* Rows past their `expiresAt` are treated as already invalid: the row
* is still deleted (so it cannot be replayed later) but `null` is
* returned. Callers do not need their own `expiresAt` gate.
*
* The secondary-storage-only path (`storeInDatabase: false`) is atomic
* only when the configured storage implements `getAndDelete`; otherwise
@@ -1259,30 +1259,34 @@ export const createInternalAdapter = (
? [storedIdentifier, identifier]
: [storedIdentifier];
// After a JSON round-trip `expiresAt` arrives as a string, so coerce
// it back to a valid `Date` to match what the DB adapter returns.
const hydrateCachedVerification = (raw: unknown): Verification | null => {
if (!raw) return null;
const candidate =
typeof raw === "string"
? safeJSONParse<Verification>(raw)
: typeof raw === "object"
? (raw as Verification)
: null;
if (!candidate) return null;
const expiresAt = new Date(candidate.expiresAt);
if (!Number.isFinite(expiresAt.getTime())) return null;
return { ...candidate, expiresAt };
};
let consumed: Verification | null = null;
if (secondaryStorage && !options.verification?.storeInDatabase) {
const parseCachedVerification = (raw: unknown) => {
if (!raw) return null;
// Secondary storage wrappers can return either a JSON string
// (the default codec) or a pre-parsed object (some Redis client
// integrations do this). Accept both shapes; only call
// safeJSONParse when we actually have a string.
if (typeof raw === "string") {
return safeJSONParse<Verification>(raw);
}
if (typeof raw === "object") {
return raw as Verification;
}
return null;
};
const consumeCacheKey = async (key: string) => {
if (secondaryStorage.getAndDelete) {
return parseCachedVerification(
return hydrateCachedVerification(
await secondaryStorage.getAndDelete(key),
);
}
return withVerificationConsumeLock(key, async () => {
const raw = await secondaryStorage.get(key);
const parsed = parseCachedVerification(raw);
const parsed = hydrateCachedVerification(raw);
if (!parsed) return null;
await secondaryStorage.delete(key);
return parsed;
@@ -1300,67 +1304,67 @@ export const createInternalAdapter = (
secondaryStorage.delete(`verification:${candidate}`),
),
);
return cached;
consumed = cached;
break;
}
return null;
}
} else {
const consumeByIdentifier = async (
id: string,
): Promise<Verification | null> =>
withVerificationConsumeLock(`verification:${id}`, () =>
runWithTransaction(adapter, async () => {
const txAdapter = await getCurrentAdapter(adapter);
const where = [{ field: "identifier", value: id }];
const rows = await txAdapter.findMany<Verification>({
model: "verification",
where,
sortBy: { field: "createdAt", direction: "desc" },
limit: 1,
});
const latest = rows[0] ?? null;
if (!latest) return null;
async function consumeByIdentifier(
id: string,
): Promise<Verification | null> {
const where = [{ field: "identifier", value: id }];
return withVerificationConsumeLock(`verification:${id}`, () =>
runWithTransaction(adapter, async () => {
const txAdapter = await getCurrentAdapter(adapter);
const rows = await txAdapter.findMany<Verification>({
model: "verification",
where,
sortBy: { field: "createdAt", direction: "desc" },
limit: 1,
});
const latest = rows[0] ?? null;
if (!latest) return null;
// FIXME(consume-identifier-atomic): add an adapter primitive that
// deletes all rows for an identifier and returns the latest row in
// one operation. Until then, consume the latest row as the race gate
// and invalidate stale rows inside the same transaction/local lock.
return consumeOneWithHooks<Verification>(
"verification",
[{ field: "id", value: latest.id }],
async () => {
const row = await txAdapter.consumeOne<Verification>({
model: "verification",
where: [{ field: "id", value: latest.id }],
});
if (!row) return null;
await txAdapter.deleteMany({
model: "verification",
where,
});
return row;
},
latest,
);
}),
);
// FIXME(consume-identifier-atomic): add an adapter primitive that
// deletes all rows for an identifier and returns the latest row in
// one operation. Until then, consume the latest row as the race gate
// and invalidate stale rows inside the same transaction/local lock.
const hookWhere = [{ field: "id", value: latest.id }];
return consumeOneWithHooks<Verification>(
"verification",
hookWhere,
async () => {
const consumed = await txAdapter.consumeOne<Verification>({
model: "verification",
where: hookWhere,
});
if (!consumed) return null;
await txAdapter.deleteMany({
model: "verification",
where,
});
return consumed;
},
latest,
);
}),
);
}
for (const stored of identifiersToTry) {
consumed = await consumeByIdentifier(stored);
if (consumed) break;
}
let consumed: Verification | null = null;
for (const stored of identifiersToTry) {
consumed = await consumeByIdentifier(stored);
if (consumed) break;
}
if (consumed && secondaryStorage) {
await Promise.all(
identifiersToTry.map((stored) =>
secondaryStorage.delete(`verification:${stored}`),
),
);
if (consumed && secondaryStorage) {
await Promise.all(
identifiersToTry.map((stored) =>
secondaryStorage.delete(`verification:${stored}`),
),
);
}
}
// Single expiry gate. A row past its `expiresAt` is treated as already
// invalid, so callers can rely on a non-null return meaning "valid".
if (!consumed || consumed.expiresAt < new Date()) return null;
return consumed;
},
updateVerificationByIdentifier: async (
packages/better-auth/src/plugins/magic-link/index.ts
-3
View File
@@ -376,9 +376,6 @@ export const magicLink = (options: MagicLinkOptions) => {
if (!tokenValue) {
redirectWithError("INVALID_TOKEN");
}
if (tokenValue.expiresAt < new Date()) {
redirectWithError("EXPIRED_TOKEN");
}
const { email, name } = JSON.parse(tokenValue.value) as {
email: string;
name?: string | undefined;
packages/better-auth/src/plugins/magic-link/magic-link.test.ts
+2 -2
View File
@@ -119,7 +119,7 @@ describe("magic link", async () => {
onError(context) {
expect(context.response.status).toBe(302);
const location = context.response.headers.get("location");
expect(location).toContain("?error=EXPIRED_TOKEN");
expect(location).toContain("?error=INVALID_TOKEN");
},
},
);
@@ -555,7 +555,7 @@ describe("magic link storeToken", async () => {
* Magic-link tokens are consumed atomically on the first verification call that
* finds the token: the row is deleted before any subsequent success checks
* (signup gates, session creation, etc.), so even a verify that ends in
* `EXPIRED_TOKEN`, `new_user_signup_disabled`, or `failed_to_create_session`
* `INVALID_TOKEN`, `new_user_signup_disabled`, or `failed_to_create_session`
* still burns the token. `allowedAttempts` is retained on the options type for
* backward compatibility but does not multiply redemptions; a token mints at
* most one session regardless of the value.
packages/better-auth/src/plugins/mcp/index.ts
-6
View File
@@ -551,12 +551,6 @@ export const mcp = (options: MCPOptions) => {
error: "invalid_grant",
});
}
if (verificationValue.expiresAt < new Date()) {
throw new APIError("UNAUTHORIZED", {
error_description: "code expired",
error: "invalid_grant",
});
}
if (!client_id) {
throw new APIError("UNAUTHORIZED", {
packages/better-auth/src/plugins/oidc-provider/index.ts
-6
View File
@@ -884,12 +884,6 @@ export const oidcProvider = (options: OIDCOptions) => {
error: "invalid_grant",
});
}
if (verificationValue.expiresAt < new Date()) {
throw new APIError("UNAUTHORIZED", {
error_description: "code expired",
error: "invalid_grant",
});
}
if (!client_id) {
throw new APIError("UNAUTHORIZED", {
error_description: "client_id is required",
packages/core/src/types/context.ts
+5 -3
View File
@@ -220,9 +220,11 @@ export interface InternalAdapter<
* Atomically consume a single-use verification row by `identifier` and
* return it. Only the first concurrent caller receives the latest row;
* subsequent callers receive `null`. Consuming one row invalidates the
* whole identifier so stale rows cannot be replayed. Callers MUST gate any
* state change (issue session, mint token, change password) on a non-null
* result.
* whole identifier so stale rows cannot be replayed. Rows past their
* `expiresAt` are treated as already invalid: the row is deleted but
* `null` is returned, so callers do not need to gate on `expiresAt`
* themselves. Callers MUST gate any state change (issue session, mint
* token, change password) on a non-null result.
*
* Replaces the racy `findVerificationValue` + `deleteVerificationByIdentifier`
* pair at single-use credential consumption sites.
packages/oauth-provider/src/token.ts
+1 -8
View File
@@ -646,14 +646,7 @@ async function checkVerificationValue(
if (!verification) {
throw new APIError("UNAUTHORIZED", {
error_description: "Invalid code",
error: "invalid_grant",
});
}
if (!verification.expiresAt || verification.expiresAt < new Date()) {
throw new APIError("UNAUTHORIZED", {
error_description: "code expired",
error_description: "invalid code",
error: "invalid_grant",
});
}
test/unit/magic-link-secondary-storage.test.ts
+1 -1
View File
@@ -217,7 +217,7 @@ describe("magic link with secondary storage (string return)", async () => {
onError(context) {
expect(context.response.status).toBe(302);
const location = context.response.headers.get("location");
expect(location).toContain("?error=EXPIRED_TOKEN");
expect(location).toContain("?error=INVALID_TOKEN");
},
},
);