diff --git a/.changeset/plugin-auth-optional-plugin-isolation.md b/.changeset/plugin-auth-optional-plugin-isolation.md new file mode 100644 index 0000000000..d475628936 --- /dev/null +++ b/.changeset/plugin-auth-optional-plugin-isolation.md @@ -0,0 +1,11 @@ +--- +"@objectstack/plugin-auth": patch +--- + +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. + +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. + +`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). + +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. diff --git a/packages/plugins/plugin-auth/src/auth-manager.optional-plugin-isolation.test.ts b/packages/plugins/plugin-auth/src/auth-manager.optional-plugin-isolation.test.ts new file mode 100644 index 0000000000..5e1dc39840 --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-manager.optional-plugin-isolation.test.ts @@ -0,0 +1,219 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +// +// Regression tests for optional-plugin isolation in AuthManager.buildPluginList. +// +// The 15.1.0 incident: `@better-auth/oauth-provider` (resolved to 1.6.23 by +// downstream installs while the workspace tested 1.7.0-rc.1) threw +// `Cannot set properties of undefined (setting 'modelName')` during plugin +// construction. Because the better-auth instance is built lazily per request, +// that single optional plugin took down EVERY auth endpoint with a 500. +// +// Unlike auth-manager.test.ts (which mocks `better-auth` itself), this file +// runs the REAL better-auth against its in-memory adapter and only wraps two +// plugin factories in controllable failure switches — so "sign-up returns +// 200" is asserted against the genuine request pipeline. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { AuthManager } from './auth-manager'; + +const failures = vi.hoisted(() => ({ oauthProvider: false, bearer: false })); + +vi.mock('@better-auth/oauth-provider', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + oauthProvider: (...args: any[]) => { + if (failures.oauthProvider) { + // Byte-for-byte the 15.1.0 incident error (1.6/1.7 family mix). + throw new TypeError("Cannot set properties of undefined (setting 'modelName')"); + } + return actual.oauthProvider(...args); + }, + }; +}); + +vi.mock('better-auth/plugins/bearer', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + bearer: (...args: any[]) => { + if (failures.bearer) throw new TypeError('bearer exploded (simulated)'); + return actual.bearer(...args); + }, + }; +}); + +/** + * Minimal in-memory IDataEngine covering exactly the surface the ObjectQL + * adapter drives (insert / findOne / find / count / update / delete with the + * eq/$ne/$in/$gt/$gte/$lt/$lte/$regex filter shapes convertWhere produces), + * so the REAL sign-up pipeline can persist users/sessions/accounts. + */ +const createMemoryEngine = () => { + const tables = new Map(); + const rows = (name: string) => { + if (!tables.has(name)) tables.set(name, []); + return tables.get(name)!; + }; + const eq = (a: any, b: any) => + a instanceof Date || b instanceof Date + ? new Date(a as any).getTime() === new Date(b as any).getTime() + : a === b; + const matches = (row: any, where: Record = {}) => + Object.entries(where).every(([k, v]) => { + const actual = row[k]; + if (v && typeof v === 'object' && !Array.isArray(v) && !(v instanceof Date)) { + if ('$ne' in v) return !eq(actual, v.$ne); + if ('$in' in v) return (v.$in as any[]).some((x) => eq(actual, x)); + if ('$gt' in v) return actual > v.$gt; + if ('$gte' in v) return actual >= v.$gte; + if ('$lt' in v) return actual < v.$lt; + if ('$lte' in v) return actual <= v.$lte; + if ('$regex' in v) return new RegExp(String(v.$regex)).test(String(actual ?? '')); + } + return eq(actual, v); + }); + let seq = 0; + return { + async insert(name: string, data: any) { + const row = { id: data.id ?? `row_${++seq}`, ...data }; + rows(name).push(row); + return { ...row }; + }, + async findOne(name: string, q: any = {}) { + return rows(name).find((r) => matches(r, q.where)) ?? null; + }, + async find(name: string, q: any = {}) { + let out = rows(name).filter((r) => matches(r, q.where)); + const order = q.orderBy?.[0]; + if (order) { + out = [...out].sort( + (a, b) => (a[order.field] > b[order.field] ? 1 : -1) * (order.order === 'desc' ? -1 : 1), + ); + } + if (q.offset) out = out.slice(q.offset); + if (q.limit) out = out.slice(0, q.limit); + return out.map((r) => ({ ...r })); + }, + async count(name: string, q: any = {}) { + return rows(name).filter((r) => matches(r, q.where)).length; + }, + async update(name: string, patch: any) { + const row = rows(name).find((r) => r.id === patch.id); + if (!row) return null; + Object.assign(row, patch); + return { ...row }; + }, + async delete(name: string, q: any = {}) { + const table = rows(name); + const keep = table.filter((r) => !matches(r, q.where)); + tables.set(name, keep); + return table.length - keep.length; + }, + }; +}; + +describe('AuthManager – optional better-auth plugin isolation', () => { + let errorSpy: ReturnType; + + const makeManager = () => + new AuthManager({ + secret: 'test-secret-at-least-32-chars-long!!', + baseUrl: 'http://localhost:3000', + dataEngine: createMemoryEngine() as any, + plugins: { oidcProvider: true }, + }); + + const signUp = (manager: AuthManager, email: string) => + manager.handleRequest( + new Request('http://localhost:3000/api/v1/auth/sign-up/email', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + email, + password: 'S3cure!Passw0rd-isolation', + name: 'Isolation Test', + }), + }), + ); + + beforeEach(() => { + failures.oauthProvider = false; + failures.bearer = false; + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + errorSpy.mockRestore(); + }); + + it('healthy control: oidcProvider constructs for real and sign-up returns 200', async () => { + const manager = makeManager(); + const response = await signUp(manager, 'healthy@example.com'); + + expect(response.status).toBe(200); + expect(manager.getDegradedAuthFeatures()).toEqual([]); + }); + + it('15.1.0 regression: a throwing oauthProvider is skipped and sign-up still returns 200', async () => { + failures.oauthProvider = true; + const manager = makeManager(); + + const response = await signUp(manager, 'degraded@example.com'); + expect(response.status).toBe(200); + const body: any = await response.json(); + expect(body?.user?.email).toBe('degraded@example.com'); + + // The failure is recorded (jwt + oauthProvider land atomically, so the + // whole oidcProvider unit is what degrades)… + expect(manager.getDegradedAuthFeatures()).toEqual([ + expect.objectContaining({ + feature: 'oidcProvider', + error: expect.stringContaining('modelName'), + }), + ]); + // …and loudly: one actionable console.error naming the feature. + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining('Optional auth feature "oidcProvider" failed to initialize'), + expect.any(TypeError), + ); + }); + + it('get-session also survives a degraded optional plugin', async () => { + failures.oauthProvider = true; + const manager = makeManager(); + + const response = await manager.handleRequest( + new Request('http://localhost:3000/api/v1/auth/get-session'), + ); + expect(response.status).toBe(200); + }); + + it('core plugin (bearer) still fails hard — no fail-open for security-bearing plugins', async () => { + failures.bearer = true; + const manager = makeManager(); + + await expect(signUp(manager, 'core@example.com')).rejects.toThrow( + 'bearer exploded (simulated)', + ); + // Core failures are NOT recorded as degraded features — the instance + // never came up at all. + expect(manager.getDegradedAuthFeatures()).toEqual([]); + }); + + it('degraded state resets when the instance is rebuilt with the fault gone', async () => { + failures.oauthProvider = true; + const manager = makeManager(); + await signUp(manager, 'rebuild-a@example.com'); + expect(manager.getDegradedAuthFeatures()).toHaveLength(1); + + // Simulate the operator fixing the underlying issue + a config-driven + // rebuild (applyConfigPatch resets the lazy instance). + failures.oauthProvider = false; + manager.applyConfigPatch({}); + const response = await signUp(manager, 'rebuild-b@example.com'); + + expect(response.status).toBe(200); + expect(manager.getDegradedAuthFeatures()).toEqual([]); + }); +}); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index ca6426b4bb..5fcfb75c55 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -560,6 +560,10 @@ export class AuthManager { // the node that issues a flag (other nodes catch up within the TTL). private _mustChangeCache: { value: boolean; at: number } = { value: false, at: 0 }; private _mustChangeRefreshing = false; + // Optional auth features whose better-auth plugin failed to initialize and + // was skipped so core auth stays up (feature key → error message). Rebuilt + // by every buildPluginList() run; see addOptionalPlugin(). + private degradedFeatures = new Map(); /** * Result of the dev-only admin seed (set by `AuthPlugin.maybeSeedDevAdmin` @@ -1303,14 +1307,77 @@ export class AuthManager { } } + /** + * Construct one OPTIONAL better-auth plugin, isolating failures. + * + * The 15.1.0 incident: `@better-auth/oauth-provider` threw during + * construction (`Cannot set properties of undefined (setting 'modelName')`) + * and, because the better-auth instance is (re)built lazily per request, + * EVERY auth endpoint — sign-up, sign-in, get-session — returned 500. One + * optional federation feature must never take down core email/password + + * session auth, so a throwing optional plugin is skipped with a loud + * console.error and recorded in `degradedFeatures` (surfaced via + * getDegradedAuthFeatures(); the OIDC discovery mount checks it). + * + * `build` may return one plugin or an array that must land atomically + * (e.g. jwt + oauthProvider): on failure NOTHING from the unit is pushed. + */ + private async addOptionalPlugin( + plugins: any[], + feature: string, + build: () => Promise, + ): Promise { + try { + const built = await build(); + for (const p of Array.isArray(built) ? built : [built]) plugins.push(p); + } catch (err: any) { + const message = err?.message ?? String(err); + this.degradedFeatures.set(feature, message); + console.error( + `[AuthManager] Optional auth feature "${feature}" failed to initialize and is DISABLED ` + + `for this process — its endpoints will 404. Core email/password + session auth is unaffected. ` + + `Fix the underlying error (often a better-auth version mismatch) and restart. Cause: ${message}`, + err, + ); + } + } + + /** + * Optional auth features skipped by the last plugin-list build because + * their better-auth plugin threw during initialization. Empty when the + * instance is healthy (or not built yet). Keys match the + * `AuthPluginConfig` flag names (`oidcProvider`, `sso`, `scim`, …). + */ + getDegradedAuthFeatures(): Array<{ feature: string; error: string }> { + return Array.from(this.degradedFeatures, ([feature, error]) => ({ feature, error })); + } + /** * Build the list of better-auth plugins based on AuthPluginConfig flags. * * Each plugin that introduces its own database tables is configured with * a `schema` option containing the appropriate snake_case field mappings, * so that `createAdapterFactory` transforms them automatically. + * + * Failure semantics — two tiers, chosen per plugin below: + * + * CORE (construction failure fails the whole instance — better a hard + * 500 than silently weakened auth): + * • bearer — token auth for every non-cookie API client. + * • twoFactor — skipping it would let accounts WITH 2FA enrolled + * sign in on password alone (fail-open). + * • haveIBeenPwned— operator explicitly opted into breached-password + * rejection; dropping it silently relaxes policy. + * • customSession — carries positions[]/isPlatformAdmin and the + * ADR-0069 authGate (password expiry, enforced MFA); + * without it authorization and policy gates vanish. + * + * OPTIONAL (skipped on failure via addOptionalPlugin — feature 404s, + * core auth stays up): organization, admin, phoneNumber, magicLink, + * genericOAuth, jwt+oauthProvider, sso, scim, deviceAuthorization. */ private async buildPluginList(): Promise { + this.degradedFeatures.clear(); const pluginConfig: Partial = this.config.plugins ?? {}; const plugins: any[] = []; @@ -1384,6 +1451,7 @@ export class AuthManager { plugins.push(bearer()); if (enabled.organization) { + await this.addOptionalPlugin(plugins, 'organization', async () => { const { organization } = await import('better-auth/plugins/organization'); // Build a `roles` map that registers each app-supplied org role // (e.g. CRM's sales_rep, sales_manager) as a valid Better-Auth role @@ -1419,7 +1487,7 @@ export class AuthManager { customOrgRoles = undefined; } } - plugins.push(organization({ + return organization({ schema: buildOrganizationPluginSchema(), // Enable the team sub-feature so the framework's `sys_team` / // `sys_team_member` tables (already declared in platform-objects) @@ -1604,7 +1672,8 @@ export class AuthManager { console.error(`[AuthManager] sendInvitationEmail failed (swallowed): ${err?.message ?? err}`); } }, - })); + }); + }); } if (enabled.twoFactor) { @@ -1628,18 +1697,21 @@ export class AuthManager { } if (enabled.admin) { - const { admin } = await import('better-auth/plugins/admin'); - // Platform admin: ban/unban, set-password, impersonate, set-role. - // Schema mapping ensures the plugin's added user/session columns - // match ObjectStack's snake_case conventions (ban_reason, - // ban_expires, impersonated_by). `role` and `banned` are already - // snake_case-compatible. - plugins.push(admin({ - schema: buildAdminPluginSchema(), - })); + await this.addOptionalPlugin(plugins, 'admin', async () => { + const { admin } = await import('better-auth/plugins/admin'); + // Platform admin: ban/unban, set-password, impersonate, set-role. + // Schema mapping ensures the plugin's added user/session columns + // match ObjectStack's snake_case conventions (ban_reason, + // ban_expires, impersonated_by). `role` and `banned` are already + // snake_case-compatible. + return admin({ + schema: buildAdminPluginSchema(), + }); + }); } if (enabled.phoneNumber) { + await this.addOptionalPlugin(plugins, 'phoneNumber', async () => { const { phoneNumber } = await import('better-auth/plugins/phone-number'); // #2766 V1.5 wired phone+password sign-in; #2780 opens the OTP surface // (`/phone-number/send-otp` + `/verify`, `/request-password-reset` + @@ -1651,7 +1723,7 @@ export class AuthManager { // by the admin create-user/import routes with a placeholder email // (see placeholder-email.ts), never by OTP self-signup. const otpCfg = this.config.phoneOtp ?? {}; - plugins.push(phoneNumber({ + return phoneNumber({ schema: buildPhoneNumberPluginSchema(), // Wrong-code attempts before the stored OTP is invalidated. Explicit // (even though 3 is the better-auth default) — #2780 names it a @@ -1677,13 +1749,15 @@ export class AuthManager { sendPasswordResetOTP: async ({ phoneNumber: phone, code }) => { await this.deliverPhoneOtp(phone, code); }, - })); + }); + }); } if (enabled.magicLink) { + await this.addOptionalPlugin(plugins, 'magicLink', async () => { const { magicLink } = await import('better-auth/plugins/magic-link'); // magic-link reuses the `verification` table — no extra schema mapping needed. - plugins.push(magicLink({ + return magicLink({ sendMagicLink: async ({ email: recipientEmail, url, token }) => { // #2766 V1.5 — placeholder addresses are never real recipients. if (isPlaceholderEmail(recipientEmail)) { @@ -1718,14 +1792,17 @@ export class AuthManager { throw err; } }, - })); + }); + }); } // OIDC / Generic OAuth2 providers (enterprise SSO via genericOAuth plugin) if (this.config.oidcProviders?.length) { + const oidcProviders = this.config.oidcProviders; + await this.addOptionalPlugin(plugins, 'genericOAuth', async () => { const { genericOAuth } = await import('better-auth/plugins/generic-oauth'); - plugins.push(genericOAuth({ - config: this.config.oidcProviders.map(p => ({ + return genericOAuth({ + config: oidcProviders.map(p => ({ providerId: p.providerId, ...(p.discoveryUrl ? { discoveryUrl: p.discoveryUrl } : {}), ...(p.issuer ? { issuer: p.issuer } : {}), @@ -1737,7 +1814,8 @@ export class AuthManager { ...(p.scopes ? { scopes: p.scopes } : {}), ...(p.pkce != null ? { pkce: p.pkce } : {}), })), - })); + }); + }); } // OAuth/OIDC Provider — turn this server into an OpenID Connect Identity @@ -1751,16 +1829,20 @@ export class AuthManager { // models — see `buildOauthProviderPluginSchema()` for the snake_case // mappings to ObjectStack's `sys_oauth_*` tables. if (enabled.oidcProvider) { + // jwt + oauthProvider are one atomic unit: on failure neither lands. + // This is the plugin that broke every fresh 15.1.0 project (modelName + // TypeError from a 1.6/1.7 version mix) — see addOptionalPlugin. + await this.addOptionalPlugin(plugins, 'oidcProvider', async () => { // The new @better-auth/oauth-provider package requires the `jwt` // plugin (used to sign id_tokens / JWT access tokens). Register it // automatically — it is otherwise an internal implementation detail // and forcing every consumer to opt in would be poor DX. const { jwt } = await import('better-auth/plugins'); - plugins.push(jwt({ schema: buildJwtPluginSchema() })); + const jwtPlugin = jwt({ schema: buildJwtPluginSchema() }); const { oauthProvider } = await import('@better-auth/oauth-provider'); const dcr = resolveDcrEnabled(pluginConfig); - plugins.push(oauthProvider({ + return [jwtPlugin, oauthProvider({ // Console SPA renders both pages (replaces the legacy Account SPA at // /_account). Override `uiBasePath` in AuthConfig if Console is // mounted elsewhere. @@ -1786,7 +1868,8 @@ export class AuthManager { // an anonymous registration mints no authority by itself. allowDynamicClientRegistration: dcr, allowUnauthenticatedClientRegistration: dcr, - })); + })]; + }); } // External SSO (OIDC / SAML) relying-party — lets this environment federate @@ -1797,6 +1880,7 @@ export class AuthManager { // // Toggle with `OS_SSO_ENABLED` (mirrors `OS_OIDC_PROVIDER_ENABLED`). if (enabled.sso) { + await this.addOptionalPlugin(plugins, 'sso', async () => { const { sso } = await import('@better-auth/sso'); // NOTE: unlike `oauthProvider`, @better-auth/sso hardcodes its `ssoProvider` // model and accepts NO `schema` option (verified against 1.6.20 — no @@ -1816,10 +1900,11 @@ export class AuthManager { // external IdP's email domain be DNS-verified before it may complete a // login — preventing an org admin from registering a provider for a domain // they don't control. Off by default to preserve the register→login flow. - plugins.push(sso({ + return sso({ organizationProvisioning: { defaultRole: 'member' }, ...(enabled.ssoDomainVerification ? { domainVerification: { enabled: true } } : {}), - })); + }); + }); } // External SCIM 2.0 Service Provider (@better-auth/scim, MIT) — lets an @@ -1835,8 +1920,10 @@ export class AuthManager { // storeSCIMToken: 'hashed' — never persist the bearer in cleartext; the // plaintext is returned exactly once from generate-token (for the IdP admin). if (enabled.scim) { - const { scim } = await import('@better-auth/scim'); - plugins.push(scim({ storeSCIMToken: 'hashed' })); + await this.addOptionalPlugin(plugins, 'scim', async () => { + const { scim } = await import('@better-auth/scim'); + return scim({ storeSCIMToken: 'hashed' }); + }); } // Device Authorization Grant (RFC 8628) — for CLI / TV-style devices. @@ -1848,11 +1935,13 @@ export class AuthManager { // the `user_code` query parameter that better-auth appends to // `verification_uri_complete`. if (enabled.deviceAuthorization) { - const { deviceAuthorization } = await import('better-auth/plugins/device-authorization'); - plugins.push(deviceAuthorization({ - verificationUri: this.getConsolePageUrl('/auth/device'), - schema: buildDeviceAuthorizationPluginSchema(), - })); + await this.addOptionalPlugin(plugins, 'deviceAuthorization', async () => { + const { deviceAuthorization } = await import('better-auth/plugins/device-authorization'); + return deviceAuthorization({ + verificationUri: this.getConsolePageUrl('/auth/device'), + schema: buildDeviceAuthorizationPluginSchema(), + }); + }); } // customSession() — augments the session payload with the canonical diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 2fea38d34c..bc65c22e9e 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -1751,6 +1751,25 @@ export class AuthPlugin implements Plugin { */ private async registerOidcDiscoveryRoutes(rawApp: any, ctx: PluginContext): Promise { const auth = await this.authManager!.getAuthInstance(); + + // The oauthProvider plugin may have been skipped by the optional-plugin + // isolation in AuthManager.buildPluginList (a failing optional plugin no + // longer fails the whole instance — the 15.1.0 lesson). Advertising + // .well-known discovery documents for an IdP whose endpoints are not + // mounted would send every external client into 404s, so skip the mounts + // and say why. Core auth is already up at this point. + const degradedOidc = this.authManager! + .getDegradedAuthFeatures() + .find((d) => d.feature === 'oidcProvider'); + if (degradedOidc) { + ctx.logger.error( + 'OIDC provider is configured but its better-auth plugin failed to initialize; ' + + 'skipping /.well-known discovery mounts. External SSO/MCP clients cannot use this ' + + `deployment as an IdP until the underlying error is fixed: ${degradedOidc.error}`, + ); + return; + } + const { oauthProviderAuthServerMetadata, oauthProviderOpenIdConfigMetadata } = await import( '@better-auth/oauth-provider' );