Skip to content

Commit 1ed1988

Browse files
os-zhuangclaude
andcommitted
fix(plugin-auth): isolate optional better-auth plugin init failures from core auth
Follow-up to #3085. In the 15.1.0 incident, @better-auth/oauth-provider threw "Cannot set properties of undefined (setting 'modelName')" during plugin construction, and because the better-auth instance is built lazily per request, that single optional federation feature turned EVERY auth endpoint (sign-up / sign-in / get-session) into a 500 — a blast radius wildly out of proportion to the feature. AuthManager.buildPluginList now constructs plugins in two explicit tiers: - OPTIONAL (organization, admin, phoneNumber, magicLink, genericOAuth, jwt+oauthProvider as one atomic unit, sso, scim, deviceAuthorization): built through addOptionalPlugin(), which on failure skips the feature with one loud actionable console.error, records it in getDegradedAuthFeatures(), and leaves core email/password + session auth up. The feature's endpoints 404 until the operator fixes the underlying error and restarts. - CORE (bearer, twoFactor, haveIBeenPwned, customSession): still fail hard. Skipping these would silently weaken auth — 2FA-enrolled accounts signing in on password alone, opted-in breached-password rejection vanishing, the ADR-0069 authGate (password expiry, enforced MFA) and positions[] disappearing from sessions. Better a hard 500 than fail-open security. The OIDC discovery mount in auth-plugin.ts consults the degraded set and skips advertising /.well-known documents for an IdP whose endpoints did not come up (previously the "Failed to register OIDC discovery routes" path), logging why instead of sending external clients into 404s. Regression tests run the REAL better-auth pipeline (no betterAuth mock, a minimal in-memory IDataEngine behind the genuine ObjectQL adapter) with the exact incident TypeError injected into oauthProvider: sign-up and get-session return 200, the degraded feature is reported, the console.error fires; a throwing CORE plugin (bearer) still rejects; the degraded set resets on a healthy rebuild. Full plugin-auth suite: 457 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 01ba3b3 commit 1ed1988

4 files changed

Lines changed: 368 additions & 30 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
Contain the blast radius of a failing optional better-auth plugin: core email/password + session auth now stays up when an optional feature plugin throws during initialization.
6+
7+
Previously, one throwing optional plugin (the 15.1.0 incident: `@better-auth/oauth-provider` threw `Cannot set properties of undefined (setting 'modelName')` from a 1.6/1.7 version mix) failed the whole lazily-built better-auth instance, turning EVERY auth endpoint — sign-up, sign-in, get-session — into a 500.
8+
9+
`AuthManager.buildPluginList` now classifies plugins in two tiers. Optional feature plugins (organization, admin, phoneNumber, magicLink, genericOAuth, jwt+oauthProvider as one atomic unit, sso, scim, deviceAuthorization) are constructed through an isolation wrapper: on failure the feature is skipped with a loud actionable `console.error`, recorded in `getDegradedAuthFeatures()`, and its endpoints 404 while core auth keeps working. Security-bearing plugins (bearer, twoFactor, haveIBeenPwned, customSession with its ADR-0069 authGate) still fail hard — better a hard 500 than silently weakened auth (e.g. 2FA-enrolled accounts signing in on password alone).
10+
11+
The OIDC discovery mount (`/.well-known/{oauth-authorization-server,openid-configuration}`) checks the degraded set and skips advertising an IdP whose endpoints did not come up, with a clear error log instead of sending external clients into 404s.
Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Regression tests for optional-plugin isolation in AuthManager.buildPluginList.
4+
//
5+
// The 15.1.0 incident: `@better-auth/oauth-provider` (resolved to 1.6.23 by
6+
// downstream installs while the workspace tested 1.7.0-rc.1) threw
7+
// `Cannot set properties of undefined (setting 'modelName')` during plugin
8+
// construction. Because the better-auth instance is built lazily per request,
9+
// that single optional plugin took down EVERY auth endpoint with a 500.
10+
//
11+
// Unlike auth-manager.test.ts (which mocks `better-auth` itself), this file
12+
// runs the REAL better-auth against its in-memory adapter and only wraps two
13+
// plugin factories in controllable failure switches — so "sign-up returns
14+
// 200" is asserted against the genuine request pipeline.
15+
16+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
17+
import { AuthManager } from './auth-manager';
18+
19+
const failures = vi.hoisted(() => ({ oauthProvider: false, bearer: false }));
20+
21+
vi.mock('@better-auth/oauth-provider', async (importOriginal) => {
22+
const actual = await importOriginal<Record<string, any>>();
23+
return {
24+
...actual,
25+
oauthProvider: (...args: any[]) => {
26+
if (failures.oauthProvider) {
27+
// Byte-for-byte the 15.1.0 incident error (1.6/1.7 family mix).
28+
throw new TypeError("Cannot set properties of undefined (setting 'modelName')");
29+
}
30+
return actual.oauthProvider(...args);
31+
},
32+
};
33+
});
34+
35+
vi.mock('better-auth/plugins/bearer', async (importOriginal) => {
36+
const actual = await importOriginal<Record<string, any>>();
37+
return {
38+
...actual,
39+
bearer: (...args: any[]) => {
40+
if (failures.bearer) throw new TypeError('bearer exploded (simulated)');
41+
return actual.bearer(...args);
42+
},
43+
};
44+
});
45+
46+
/**
47+
* Minimal in-memory IDataEngine covering exactly the surface the ObjectQL
48+
* adapter drives (insert / findOne / find / count / update / delete with the
49+
* eq/$ne/$in/$gt/$gte/$lt/$lte/$regex filter shapes convertWhere produces),
50+
* so the REAL sign-up pipeline can persist users/sessions/accounts.
51+
*/
52+
const createMemoryEngine = () => {
53+
const tables = new Map<string, any[]>();
54+
const rows = (name: string) => {
55+
if (!tables.has(name)) tables.set(name, []);
56+
return tables.get(name)!;
57+
};
58+
const eq = (a: any, b: any) =>
59+
a instanceof Date || b instanceof Date
60+
? new Date(a as any).getTime() === new Date(b as any).getTime()
61+
: a === b;
62+
const matches = (row: any, where: Record<string, any> = {}) =>
63+
Object.entries(where).every(([k, v]) => {
64+
const actual = row[k];
65+
if (v && typeof v === 'object' && !Array.isArray(v) && !(v instanceof Date)) {
66+
if ('$ne' in v) return !eq(actual, v.$ne);
67+
if ('$in' in v) return (v.$in as any[]).some((x) => eq(actual, x));
68+
if ('$gt' in v) return actual > v.$gt;
69+
if ('$gte' in v) return actual >= v.$gte;
70+
if ('$lt' in v) return actual < v.$lt;
71+
if ('$lte' in v) return actual <= v.$lte;
72+
if ('$regex' in v) return new RegExp(String(v.$regex)).test(String(actual ?? ''));
73+
}
74+
return eq(actual, v);
75+
});
76+
let seq = 0;
77+
return {
78+
async insert(name: string, data: any) {
79+
const row = { id: data.id ?? `row_${++seq}`, ...data };
80+
rows(name).push(row);
81+
return { ...row };
82+
},
83+
async findOne(name: string, q: any = {}) {
84+
return rows(name).find((r) => matches(r, q.where)) ?? null;
85+
},
86+
async find(name: string, q: any = {}) {
87+
let out = rows(name).filter((r) => matches(r, q.where));
88+
const order = q.orderBy?.[0];
89+
if (order) {
90+
out = [...out].sort(
91+
(a, b) => (a[order.field] > b[order.field] ? 1 : -1) * (order.order === 'desc' ? -1 : 1),
92+
);
93+
}
94+
if (q.offset) out = out.slice(q.offset);
95+
if (q.limit) out = out.slice(0, q.limit);
96+
return out.map((r) => ({ ...r }));
97+
},
98+
async count(name: string, q: any = {}) {
99+
return rows(name).filter((r) => matches(r, q.where)).length;
100+
},
101+
async update(name: string, patch: any) {
102+
const row = rows(name).find((r) => r.id === patch.id);
103+
if (!row) return null;
104+
Object.assign(row, patch);
105+
return { ...row };
106+
},
107+
async delete(name: string, q: any = {}) {
108+
const table = rows(name);
109+
const keep = table.filter((r) => !matches(r, q.where));
110+
tables.set(name, keep);
111+
return table.length - keep.length;
112+
},
113+
};
114+
};
115+
116+
describe('AuthManager – optional better-auth plugin isolation', () => {
117+
let errorSpy: ReturnType<typeof vi.spyOn>;
118+
119+
const makeManager = () =>
120+
new AuthManager({
121+
secret: 'test-secret-at-least-32-chars-long!!',
122+
baseUrl: 'http://localhost:3000',
123+
dataEngine: createMemoryEngine() as any,
124+
plugins: { oidcProvider: true },
125+
});
126+
127+
const signUp = (manager: AuthManager, email: string) =>
128+
manager.handleRequest(
129+
new Request('http://localhost:3000/api/v1/auth/sign-up/email', {
130+
method: 'POST',
131+
headers: { 'Content-Type': 'application/json' },
132+
body: JSON.stringify({
133+
email,
134+
password: 'S3cure!Passw0rd-isolation',
135+
name: 'Isolation Test',
136+
}),
137+
}),
138+
);
139+
140+
beforeEach(() => {
141+
failures.oauthProvider = false;
142+
failures.bearer = false;
143+
errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
144+
});
145+
146+
afterEach(() => {
147+
errorSpy.mockRestore();
148+
});
149+
150+
it('healthy control: oidcProvider constructs for real and sign-up returns 200', async () => {
151+
const manager = makeManager();
152+
const response = await signUp(manager, 'healthy@example.com');
153+
154+
expect(response.status).toBe(200);
155+
expect(manager.getDegradedAuthFeatures()).toEqual([]);
156+
});
157+
158+
it('15.1.0 regression: a throwing oauthProvider is skipped and sign-up still returns 200', async () => {
159+
failures.oauthProvider = true;
160+
const manager = makeManager();
161+
162+
const response = await signUp(manager, 'degraded@example.com');
163+
expect(response.status).toBe(200);
164+
const body: any = await response.json();
165+
expect(body?.user?.email).toBe('degraded@example.com');
166+
167+
// The failure is recorded (jwt + oauthProvider land atomically, so the
168+
// whole oidcProvider unit is what degrades)…
169+
expect(manager.getDegradedAuthFeatures()).toEqual([
170+
expect.objectContaining({
171+
feature: 'oidcProvider',
172+
error: expect.stringContaining('modelName'),
173+
}),
174+
]);
175+
// …and loudly: one actionable console.error naming the feature.
176+
expect(errorSpy).toHaveBeenCalledWith(
177+
expect.stringContaining('Optional auth feature "oidcProvider" failed to initialize'),
178+
expect.any(TypeError),
179+
);
180+
});
181+
182+
it('get-session also survives a degraded optional plugin', async () => {
183+
failures.oauthProvider = true;
184+
const manager = makeManager();
185+
186+
const response = await manager.handleRequest(
187+
new Request('http://localhost:3000/api/v1/auth/get-session'),
188+
);
189+
expect(response.status).toBe(200);
190+
});
191+
192+
it('core plugin (bearer) still fails hard — no fail-open for security-bearing plugins', async () => {
193+
failures.bearer = true;
194+
const manager = makeManager();
195+
196+
await expect(signUp(manager, 'core@example.com')).rejects.toThrow(
197+
'bearer exploded (simulated)',
198+
);
199+
// Core failures are NOT recorded as degraded features — the instance
200+
// never came up at all.
201+
expect(manager.getDegradedAuthFeatures()).toEqual([]);
202+
});
203+
204+
it('degraded state resets when the instance is rebuilt with the fault gone', async () => {
205+
failures.oauthProvider = true;
206+
const manager = makeManager();
207+
await signUp(manager, 'rebuild-a@example.com');
208+
expect(manager.getDegradedAuthFeatures()).toHaveLength(1);
209+
210+
// Simulate the operator fixing the underlying issue + a config-driven
211+
// rebuild (applyConfigPatch resets the lazy instance).
212+
failures.oauthProvider = false;
213+
manager.applyConfigPatch({});
214+
const response = await signUp(manager, 'rebuild-b@example.com');
215+
216+
expect(response.status).toBe(200);
217+
expect(manager.getDegradedAuthFeatures()).toEqual([]);
218+
});
219+
});

0 commit comments

Comments
 (0)