From d81c247609ad7844108744c997d60f4f2a95419d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 01:39:44 +0000 Subject: [PATCH 1/2] fix(plugin-auth): register the auth service-composition bindings independently of registerRoutes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AuthPlugin` bound the outbound mail transport, the SMS transport, the deployment email locale, the brand name and the SMS locale inside the same `kernel:ready` hook that mounts `/api/v1/auth/*`, and that hook was gated on `registerRoutes`. `registerRoutes` answers a transport-mounting question. The bindings are service composition and are true of an embedding regardless of who serves the routes, so every routes-less embedding came up with no mail transport, no locale on either channel and no brand binding — silently, because the `logger.info` lines that would have reported the wiring sat inside the same skipped block. Split the hook: the composition block moves verbatim into its own unconditional `ctx.hook('kernel:ready', …)`, registered before the route hook so the ordering a routing host had is preserved. Route registration itself stays under `if (this.options.registerRoutes)`. This is the shape the sibling hooks in this file already use and already name ("Registered independently of `registerRoutes` so an embedding that serves no auth routes still gets the diagnosis") — the file applied the distinction to the diagnosis hook and not to the wiring the diagnosis exists to report on. The `#14319` describe block now runs against both values of `registerRoutes` rather than only the default, and pins both acceptance criteria: the composition completes either way, and a `registerRoutes: false` kernel still mounts no auth routes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- .../plugin-auth/src/auth-plugin.test.ts | 204 ++++++--- .../plugins/plugin-auth/src/auth-plugin.ts | 421 ++++++++++-------- 2 files changed, 372 insertions(+), 253 deletions(-) diff --git a/packages/plugins/plugin-auth/src/auth-plugin.test.ts b/packages/plugins/plugin-auth/src/auth-plugin.test.ts index be4648c6d6..f746d6818f 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.test.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.test.ts @@ -541,6 +541,9 @@ describe('AuthPlugin', () => { let hookCapture: ReturnType; let setEmailLocaleSpy: ReturnType; let setSmsLocaleSpy: ReturnType; + let setEmailServiceSpy: ReturnType; + let rawApp: { all: ReturnType; get: ReturnType; post: ReturnType; use: ReturnType }; + let httpServer: { getRawApp: ReturnType; use: ReturnType }; type Resolved = { value: unknown; source: string }; @@ -576,10 +579,26 @@ describe('AuthPlugin', () => { * `i18n` is passed as `null` to mean "no such service" — `getService` * THROWS for an unregistered service, which is the shape the plugin * probes for, not a falsy return. + * + * `registerRoutes` is threaded through on purpose (#14724). It answers a + * TRANSPORT-MOUNTING question — "does this plugin mount its own + * `/api/v1/auth/*` routes" — and must not decide any of the SERVICE + * COMPOSITION this block is about. A routes-less embedding (every cloud + * tenant environment kernel is one) has the same mail transport, brand + * and locale needs as a routing one; before #14724 all of it hung off the + * routing flag, so a `registerRoutes: false` kernel did not even READ the + * `localization` namespace. */ - const boot = async (opts: { settings?: unknown; i18nDefault?: string | null }) => { + const boot = async (opts: { + settings?: unknown; + i18nDefault?: string | null; + registerRoutes?: boolean; + email?: unknown; + }) => { hookCapture = createHookCapture(); mockContext.hook = hookCapture.hookFn; + rawApp = { all: vi.fn(), get: vi.fn(), post: vi.fn(), use: vi.fn() }; + httpServer = { getRawApp: vi.fn(() => rawApp), use: vi.fn() }; // `: any` on the RETURN, not a cast on the assignment: `getService` is // generic (`(name: string) => T`), so an inferred union return is a // TS2322 — the exact debt `check:test-typecheck` ledgers for the older @@ -587,6 +606,11 @@ describe('AuthPlugin', () => { // states its shape instead of adding to it. mockContext.getService = vi.fn((name: string): any => { if (name === 'manifest') return { register: vi.fn() }; + if (name === 'http-server') return httpServer; + if (name === 'email') { + if (opts.email === undefined) throw new Error('Service not found: email'); + return opts.email; + } if (name === 'settings') { if (opts.settings === undefined) throw new Error('Service not found: settings'); return opts.settings; @@ -599,9 +623,11 @@ describe('AuthPlugin', () => { }); setEmailLocaleSpy = vi.spyOn(AuthManager.prototype, 'setDefaultEmailLocale'); setSmsLocaleSpy = vi.spyOn(AuthManager.prototype, 'setDefaultSmsLocale'); + setEmailServiceSpy = vi.spyOn(AuthManager.prototype, 'setEmailService'); authPlugin = new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long', baseUrl: 'http://localhost:3000', + ...(opts.registerRoutes === undefined ? {} : { registerRoutes: opts.registerRoutes }), }); await authPlugin.init(mockContext); await authPlugin.start(mockContext); @@ -611,72 +637,138 @@ describe('AuthPlugin', () => { afterEach(() => { setEmailLocaleSpy?.mockRestore(); setSmsLocaleSpy?.mockRestore(); - }); + setEmailServiceSpy?.mockRestore(); + }); + + // #14724 — run the whole block against BOTH values of the routing flag. + // Covering only the default is how the split defect survived: a test that + // exercises one branch of a flag cannot see the other. + describe.each([ + ['registerRoutes: true (default)', true], + ['registerRoutes: false (routes-less embedding)', false], + ] as const)('%s', (_label, registerRoutes) => { + it('a zh-CN workspace binds zh-CN on the EMAIL channel, not just on SMS', async () => { + const settings = makeSettings({ value: 'zh-CN', source: 'tenant' }); + await boot({ settings, i18nDefault: 'en', registerRoutes }); + + expect(settings.get).toHaveBeenCalledWith('localization', 'locale', {}); + // The regression, stated as the two channels agreeing. Before #14319 the + // SMS assertion passed and the email one read 'en'. + expect(setSmsLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); + expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); + }); - it('a zh-CN workspace binds zh-CN on the EMAIL channel, not just on SMS', async () => { - const settings = makeSettings({ value: 'zh-CN', source: 'tenant' }); - await boot({ settings, i18nDefault: 'en' }); + it.each(['ja-JP', 'es-ES'])('and the same for a %s workspace', async (locale) => { + await boot({ + settings: makeSettings({ value: locale, source: 'global' }), + i18nDefault: 'en', + registerRoutes, + }); + expect(setEmailLocaleSpy).toHaveBeenLastCalledWith(locale); + }); - expect(settings.get).toHaveBeenCalledWith('localization', 'locale', {}); - // The regression, stated as the two channels agreeing. Before #14319 the - // SMS assertion passed and the email one read 'en'. - expect(setSmsLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); - expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); - }); + it('a workspace that never chose a language keeps the app build-time default', async () => { + // `get` answers the manifest default ('en-US') for an untouched + // workspace, so taking `value` unconditionally would demote every + // deployment that declared `i18n.defaultLocale` — the #8195 behaviour + // this change must not regress. + await boot({ + settings: makeSettings({ value: 'en-US', source: 'default' }), + i18nDefault: 'zh-CN', + registerRoutes, + }); + expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); + }); - it.each(['ja-JP', 'es-ES'])('and the same for a %s workspace', async (locale) => { - await boot({ settings: makeSettings({ value: locale, source: 'global' }), i18nDefault: 'en' }); - expect(setEmailLocaleSpy).toHaveBeenLastCalledWith(locale); - }); + it('names NO locale when neither producer speaks — the documented en-US fallback', async () => { + // The issue's second acceptance criterion: absent language ⇒ English. + // Spelled as an ABSENT locale rather than 'en-US', because that is what + // `EmailService`'s ladder contract ("no locale means the DOCUMENTED + // default") is written against. + await boot({ + settings: makeSettings({ value: 'en-US', source: 'default' }), + registerRoutes, + }); + expect(setEmailLocaleSpy).toHaveBeenLastCalledWith(undefined); + }); - it('a workspace that never chose a language keeps the app build-time default', async () => { - // `get` answers the manifest default ('en-US') for an untouched - // workspace, so taking `value` unconditionally would demote every - // deployment that declared `i18n.defaultLocale` — the #8195 behaviour - // this change must not regress. - await boot({ - settings: makeSettings({ value: 'en-US', source: 'default' }), - i18nDefault: 'zh-CN', + it('binds the build-time default when there is no settings service at all', async () => { + await boot({ i18nDefault: 'ja-JP', registerRoutes }); + expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('ja-JP'); }); - expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); - }); - it('names NO locale when neither producer speaks — the documented en-US fallback', async () => { - // The issue's second acceptance criterion: absent language ⇒ English. - // Spelled as an ABSENT locale rather than 'en-US', because that is what - // `EmailService`'s ladder contract ("no locale means the DOCUMENTED - // default") is written against. - await boot({ settings: makeSettings({ value: 'en-US', source: 'default' }) }); - expect(setEmailLocaleSpy).toHaveBeenLastCalledWith(undefined); - }); + it('re-binds live when the workspace switches language', async () => { + const settings = makeSettings({ value: 'en-US', source: 'default' }); + await boot({ settings, i18nDefault: 'en', registerRoutes }); + expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('en'); - it('binds the build-time default when there is no settings service at all', async () => { - await boot({ i18nDefault: 'ja-JP' }); - expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('ja-JP'); - }); + expect(settings.subscribe).toHaveBeenCalledWith('localization', expect.any(Function)); + settings.set({ value: 'zh-CN', source: 'tenant' }); + for (const handler of settings.handlers) handler(); + // The subscribe handler is fire-and-forget (`void`); flush its promise. + await new Promise((resolve) => setImmediate(resolve)); - it('re-binds live when the workspace switches language', async () => { - const settings = makeSettings({ value: 'en-US', source: 'default' }); - await boot({ settings, i18nDefault: 'en' }); - expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('en'); + expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); + expect(setSmsLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); + }); - expect(settings.subscribe).toHaveBeenCalledWith('localization', expect.any(Function)); - settings.set({ value: 'zh-CN', source: 'tenant' }); - for (const handler of settings.handlers) handler(); - // The subscribe handler is fire-and-forget (`void`); flush its promise. - await new Promise((resolve) => setImmediate(resolve)); + it('leaves the build-time default standing when the settings read fails', async () => { + const settings = makeSettings(new Error('boom')); + await expect( + boot({ settings, i18nDefault: 'zh-CN', registerRoutes }), + ).resolves.toBeUndefined(); + expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); + expect(mockContext.logger.warn).toHaveBeenCalledWith( + expect.stringContaining('failed to apply localization.locale'), + ); + }); - expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); - expect(setSmsLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); - }); + // ── #14724 acceptance criterion 1 ────────────────────────────────── + // The service composition is complete at the end of `kernel:ready` + // whichever way the routing flag is set. Measured before the split with + // `registerRoutes: false`: settingsRead [], emailLocaleCalls [], + // smsLocaleCalls [], emailServiceCalls [] — the namespace was not even + // read. + it('ends kernel:ready with the transport wired and BOTH locales bound', async () => { + const settings = makeSettings({ value: 'zh-CN', source: 'tenant' }); + const email = { send: vi.fn() }; + await boot({ settings, email, i18nDefault: 'en', registerRoutes }); + + expect(mockContext.getService).toHaveBeenCalledWith('email'); + expect(setEmailServiceSpy).toHaveBeenCalledWith(email); + expect(settings.get).toHaveBeenCalledWith('localization', 'locale', {}); + expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); + expect(setSmsLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); + }); - it('leaves the build-time default standing when the settings read fails', async () => { - const settings = makeSettings(new Error('boom')); - await expect(boot({ settings, i18nDefault: 'zh-CN' })).resolves.toBeUndefined(); - expect(setEmailLocaleSpy).toHaveBeenLastCalledWith('zh-CN'); - expect(mockContext.logger.warn).toHaveBeenCalledWith( - expect.stringContaining('failed to apply localization.locale'), - ); + // ── #14724 acceptance criterion 2 — the regression guard ─────────── + // Route registration itself stays gated. This is the half that must + // NOT move: `registerRoutes: false` mounts no auth routes, and it is + // pinned here rather than left to prose. + it(`${registerRoutes ? 'mounts' : 'mounts NO'} auth routes`, async () => { + await boot({ + settings: makeSettings({ value: 'zh-CN', source: 'tenant' }), + email: { send: vi.fn() }, + i18nDefault: 'en', + registerRoutes, + }); + + const httpServerLookups = (mockContext.getService as ReturnType).mock.calls + .filter((args: unknown[]) => args[0] === 'http-server'); + + if (registerRoutes) { + expect(httpServerLookups.length).toBeGreaterThan(0); + expect(httpServer.getRawApp).toHaveBeenCalled(); + expect(rawApp.all).toHaveBeenCalledWith('/api/v1/auth/*', expect.any(Function)); + } else { + // The `http-server` service is present in this harness, so a zero + // lookup count is a statement about the gate and not about the + // service being absent. + expect(httpServerLookups.length).toBe(0); + expect(httpServer.getRawApp).not.toHaveBeenCalled(); + expect(rawApp.all).not.toHaveBeenCalled(); + } + }); }); }); diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index fcd095b0ca..6e96f028e1 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -719,220 +719,247 @@ export class AuthPlugin implements Plugin { // the package that defines them; auth-plugin no longer piggy-backs on // its kernel:ready hook for this. - // Defer HTTP route registration to kernel:ready hook. - // This ensures all plugins (including HonoServerPlugin) have completed - // their init and start phases before we attempt to look up the - // http-server service — making AuthPlugin resilient to plugin - // loading order. - if (this.options.registerRoutes) { - ctx.hook('kernel:ready', async () => { - // Inject the email service if available so better-auth callbacks - // (sendResetPassword / sendVerificationEmail / sendInvitationEmail - // / sendMagicLink) can actually deliver mail. Resolved here on - // kernel:ready so EmailServicePlugin has had a chance to register. - if (this.authManager) { - await this.ensureAuthSettingsBound(ctx); - - let emailSvc: IEmailService | undefined; - try { emailSvc = ctx.getService('email'); } catch { emailSvc = undefined; } - if (emailSvc) { - this.authManager.setEmailService(emailSvc); - ctx.logger.info('Auth: email service wired (transactional mail enabled)'); + // Bind the auth manager's live SERVICE COMPOSITION on `kernel:ready`: the + // outbound mail transport, the SMS transport, the deployment email locale, + // the brand name and the SMS locale. + // + // Registered independently of `registerRoutes`, the shape the sibling + // hooks below already use and already name ("Registered independently of + // `registerRoutes` so an embedding that serves no auth routes still gets + // the diagnosis"). `registerRoutes` answers "should this plugin mount its + // own `/api/v1/auth/*` routes on the kernel's `http-server`" — a + // TRANSPORT-MOUNTING question. Everything bound here is SERVICE + // COMPOSITION, true of an embedding regardless of who serves the routes. + // + // Gating the two together left every routes-less embedding with no mail + // transport, no locale on either channel and no brand binding — and + // silently, because the `logger.info` lines that would have reported the + // wiring sat inside the same skipped block. The population is not an edge + // case: every cloud tenant environment kernel constructs this plugin with + // `registerRoutes: false` by design, since the per-env kernel has no + // `http-server` and the host worker proxies the auth routes to it. The + // consequence was that the `localization.locale` precedence rung below + // could not reach a tenant environment at all — not degraded, absent. + // + // Registered BEFORE the route hook so that, when routes ARE mounted, the + // composition still completes ahead of them: the ordering the single + // combined hook used to give for free. + ctx.hook('kernel:ready', async () => { + // Inject the email service if available so better-auth callbacks + // (sendResetPassword / sendVerificationEmail / sendInvitationEmail + // / sendMagicLink) can actually deliver mail. Resolved here on + // kernel:ready so EmailServicePlugin has had a chance to register. + if (this.authManager) { + await this.ensureAuthSettingsBound(ctx); + + let emailSvc: IEmailService | undefined; + try { emailSvc = ctx.getService('email'); } catch { emailSvc = undefined; } + if (emailSvc) { + this.authManager.setEmailService(emailSvc); + ctx.logger.info('Auth: email service wired (transactional mail enabled)'); + } else { + // No email service. The verification / password-reset callbacks now + // THROW when invoked without a transport (so an explicit resend + // reports a real error rather than faking success). If verification + // is REQUIRED, that means every signup would be stuck — surface the + // misconfiguration loudly at boot instead of one failure per signup. + const requiresEmail = !!this.authManager.getPublicConfig?.()?.emailPassword?.requireEmailVerification; + if (requiresEmail) { + ctx.logger.error( + 'Auth: email verification is REQUIRED but NO email service is registered — ' + + 'verification & password-reset emails will FAIL and new users will be locked ' + + 'out at sign-in. Register an email service (e.g. EmailServicePlugin + OS_EMAIL_*) ' + + 'or disable verification (OS_AUTH_REQUIRE_EMAIL_VERIFICATION=false).', + ); } else { - // No email service. The verification / password-reset callbacks now - // THROW when invoked without a transport (so an explicit resend - // reports a real error rather than faking success). If verification - // is REQUIRED, that means every signup would be stuck — surface the - // misconfiguration loudly at boot instead of one failure per signup. - const requiresEmail = !!this.authManager.getPublicConfig?.()?.emailPassword?.requireEmailVerification; - if (requiresEmail) { - ctx.logger.error( - 'Auth: email verification is REQUIRED but NO email service is registered — ' - + 'verification & password-reset emails will FAIL and new users will be locked ' - + 'out at sign-in. Register an email service (e.g. EmailServicePlugin + OS_EMAIL_*) ' - + 'or disable verification (OS_AUTH_REQUIRE_EMAIL_VERIFICATION=false).', - ); - } else { - ctx.logger.info('Auth: no email service registered — transactional mail disabled'); - } - } - - // #2780 — inject the SMS service so the phoneNumber plugin's OTP - // callbacks (send-otp / password-reset OTP) and the import - // SMS-invite path can deliver. Same lazy-resolution contract as - // the email service: absent ⇒ OTP endpoints keep failing loudly - // (NOT_SUPPORTED) while phone+password sign-in still works. - let smsSvc: ISmsService | undefined; - try { smsSvc = ctx.getService('sms'); } catch { smsSvc = undefined; } - if (smsSvc) { - this.authManager.setSmsService(smsSvc); - if (this.authManager.isPhoneNumberEnabled()) { - ctx.logger.info( - this.authManager.isPhoneOtpDeliverable() - ? 'Auth: sms service wired (phone-number OTP sign-in / reset enabled)' - : 'Auth: sms service present but NOT configured with a real provider — phone-number OTP stays disabled in production', - ); - } - } else if (this.authManager.isPhoneNumberEnabled()) { - ctx.logger.info('Auth: no sms service registered — phone-number OTP disabled (password sign-in only)'); + ctx.logger.info('Auth: no email service registered — transactional mail disabled'); } + } - // #8195 / #14319 — the locale named on every auth EMAIL, so the - // localized `sys_email_template` rows are reachable through the - // platform's own send path instead of sitting dormant. - // - // This binds the DEPLOYMENT rung, resolved here at the plugin - // layer. Since the 2026-09-02 ruling (#14319) it is the SECOND rung: - // a request-triggered send whose recipient IS the requester takes - // that caller's own `Accept-Language` first, resolved at send time in - // `AuthManager` (`authEmailLocaleFromRequest`, which carries the - // ruling text). What is bound here answers when the request named no - // locale this platform ships a row for, or when there is no request - // at all — invitations, scheduled and admin-initiated mail. - // - // ⚠️ The superseded 2026-08-13 ruling made this rung the WHOLE answer - // and rejected `Accept-Language` outright. Do not restore that - // reading here; the single place the history is recorded is - // `AuthManager.setDefaultEmailLocale`. - // - // #14319 measured that "the deployment default" has TWO producers, - // and that auth email was reading the weaker one: - // - // - `II18nService.getDefaultLocale()` is the app artifact's - // BUILD-TIME `i18n.defaultLocale` (`AppPlugin` pushes it through - // `setDefaultLocale`; the CLI passes `config.i18n.defaultLocale`), - // and is the bare `en` when the app declares nothing. - // - `localization.locale` (ADR-0053) is the workspace's RUNTIME - // language — Setup, Localization, tenant-scoped — whose four - // options are exactly `AUTH_EMAIL_TEMPLATE_LOCALES`. It is - // already the authority for auth SMS (#2815) and for audit - // activity summaries (framework#3039). - // - // A workspace that declared Chinese therefore received Chinese SMS - // and ENGLISH mail. The workspace setting now outranks the - // build-time default, and only when it is EXPLICIT - // (`source !== 'default'`) — the same precedence the `appName` - // binding below uses, because `get` answers the manifest default - // (`en-US`) for an untouched workspace and taking it unconditionally - // would demote every deployment that declared `i18n.defaultLocale`. - // - // Both i18n hops are probed rather than assumed: `getService` THROWS - // for an unregistered service (i18n is not a required dependency of - // auth) and `getDefaultLocale` is OPTIONAL on the contract. Nothing - // resolvable ⇒ nothing is named, which is precisely the pre-#8195 - // behaviour — `EmailService` resolves its documented `en-US` default. - let i18nDefaultEmailLocale: string | undefined; - try { - const i18n = ctx.getService('i18n'); - const locale = - typeof i18n?.getDefaultLocale === 'function' ? i18n.getDefaultLocale() : undefined; - i18nDefaultEmailLocale = typeof locale === 'string' ? locale : undefined; - } catch { - // i18n service is optional — leave the build-time default unset. - } - const applyEmailLocale = (workspaceLocale: string | undefined): void => { - this.authManager?.setDefaultEmailLocale(workspaceLocale ?? i18nDefaultEmailLocale); - }; - // Baseline = the build-time default. Superseded below when a settings - // service answers with an explicit workspace language, and left - // standing when there is no settings service or its read fails. - applyEmailLocale(undefined); - if (typeof i18nDefaultEmailLocale === 'string' && i18nDefaultEmailLocale.trim()) { + // #2780 — inject the SMS service so the phoneNumber plugin's OTP + // callbacks (send-otp / password-reset OTP) and the import + // SMS-invite path can deliver. Same lazy-resolution contract as + // the email service: absent ⇒ OTP endpoints keep failing loudly + // (NOT_SUPPORTED) while phone+password sign-in still works. + let smsSvc: ISmsService | undefined; + try { smsSvc = ctx.getService('sms'); } catch { smsSvc = undefined; } + if (smsSvc) { + this.authManager.setSmsService(smsSvc); + if (this.authManager.isPhoneNumberEnabled()) { ctx.logger.info( - `Auth: bound auth email locale to i18n default=${i18nDefaultEmailLocale}`, + this.authManager.isPhoneOtpDeliverable() + ? 'Auth: sms service wired (phone-number OTP sign-in / reset enabled)' + : 'Auth: sms service present but NOT configured with a real provider — phone-number OTP stays disabled in production', ); } + } else if (this.authManager.isPhoneNumberEnabled()) { + ctx.logger.info('Auth: no sms service registered — phone-number OTP disabled (password sign-in only)'); + } - // Bind the email brand name (`{{appName}}`) to the live - // `branding.workspace_name` setting so the admin UI can rename the - // product without a redeploy. Only an *explicitly set* value - // overrides the configured `appName` — when the operator hasn't - // customised it (resolver returns the manifest default), we clear - // the override so the deployment's `appName` (e.g. `OS_APP_NAME`) - // keeps precedence. Mirrors EmailServicePlugin's settings binding. - try { - const settings = ctx.getService('settings'); - if (settings && typeof settings.get === 'function') { - const applyBrand = async () => { - try { - const resolved = await settings.get('branding', 'workspace_name', {}); - const explicit = resolved && resolved.source !== 'default' - ? resolved.value - : undefined; - this.authManager?.setAppName( - typeof explicit === 'string' ? explicit : undefined, - ); - } catch (err: any) { - ctx.logger.warn( - 'Auth: failed to apply branding.workspace_name: ' + (err?.message ?? err), - ); - } - }; - await applyBrand(); - if (typeof settings.subscribe === 'function') { - settings.subscribe('branding', () => { - void applyBrand(); - }); - ctx.logger.info('Auth: bound appName to settings namespace=branding'); + // #8195 / #14319 — the locale named on every auth EMAIL, so the + // localized `sys_email_template` rows are reachable through the + // platform's own send path instead of sitting dormant. + // + // This binds the DEPLOYMENT rung, resolved here at the plugin + // layer. Since the 2026-09-02 ruling (#14319) it is the SECOND rung: + // a request-triggered send whose recipient IS the requester takes + // that caller's own `Accept-Language` first, resolved at send time in + // `AuthManager` (`authEmailLocaleFromRequest`, which carries the + // ruling text). What is bound here answers when the request named no + // locale this platform ships a row for, or when there is no request + // at all — invitations, scheduled and admin-initiated mail. + // + // ⚠️ The superseded 2026-08-13 ruling made this rung the WHOLE answer + // and rejected `Accept-Language` outright. Do not restore that + // reading here; the single place the history is recorded is + // `AuthManager.setDefaultEmailLocale`. + // + // #14319 measured that "the deployment default" has TWO producers, + // and that auth email was reading the weaker one: + // + // - `II18nService.getDefaultLocale()` is the app artifact's + // BUILD-TIME `i18n.defaultLocale` (`AppPlugin` pushes it through + // `setDefaultLocale`; the CLI passes `config.i18n.defaultLocale`), + // and is the bare `en` when the app declares nothing. + // - `localization.locale` (ADR-0053) is the workspace's RUNTIME + // language — Setup, Localization, tenant-scoped — whose four + // options are exactly `AUTH_EMAIL_TEMPLATE_LOCALES`. It is + // already the authority for auth SMS (#2815) and for audit + // activity summaries (framework#3039). + // + // A workspace that declared Chinese therefore received Chinese SMS + // and ENGLISH mail. The workspace setting now outranks the + // build-time default, and only when it is EXPLICIT + // (`source !== 'default'`) — the same precedence the `appName` + // binding below uses, because `get` answers the manifest default + // (`en-US`) for an untouched workspace and taking it unconditionally + // would demote every deployment that declared `i18n.defaultLocale`. + // + // Both i18n hops are probed rather than assumed: `getService` THROWS + // for an unregistered service (i18n is not a required dependency of + // auth) and `getDefaultLocale` is OPTIONAL on the contract. Nothing + // resolvable ⇒ nothing is named, which is precisely the pre-#8195 + // behaviour — `EmailService` resolves its documented `en-US` default. + let i18nDefaultEmailLocale: string | undefined; + try { + const i18n = ctx.getService('i18n'); + const locale = + typeof i18n?.getDefaultLocale === 'function' ? i18n.getDefaultLocale() : undefined; + i18nDefaultEmailLocale = typeof locale === 'string' ? locale : undefined; + } catch { + // i18n service is optional — leave the build-time default unset. + } + const applyEmailLocale = (workspaceLocale: string | undefined): void => { + this.authManager?.setDefaultEmailLocale(workspaceLocale ?? i18nDefaultEmailLocale); + }; + // Baseline = the build-time default. Superseded below when a settings + // service answers with an explicit workspace language, and left + // standing when there is no settings service or its read fails. + applyEmailLocale(undefined); + if (typeof i18nDefaultEmailLocale === 'string' && i18nDefaultEmailLocale.trim()) { + ctx.logger.info( + `Auth: bound auth email locale to i18n default=${i18nDefaultEmailLocale}`, + ); + } + + // Bind the email brand name (`{{appName}}`) to the live + // `branding.workspace_name` setting so the admin UI can rename the + // product without a redeploy. Only an *explicitly set* value + // overrides the configured `appName` — when the operator hasn't + // customised it (resolver returns the manifest default), we clear + // the override so the deployment's `appName` (e.g. `OS_APP_NAME`) + // keeps precedence. Mirrors EmailServicePlugin's settings binding. + try { + const settings = ctx.getService('settings'); + if (settings && typeof settings.get === 'function') { + const applyBrand = async () => { + try { + const resolved = await settings.get('branding', 'workspace_name', {}); + const explicit = resolved && resolved.source !== 'default' + ? resolved.value + : undefined; + this.authManager?.setAppName( + typeof explicit === 'string' ? explicit : undefined, + ); + } catch (err: any) { + ctx.logger.warn( + 'Auth: failed to apply branding.workspace_name: ' + (err?.message ?? err), + ); } + }; + await applyBrand(); + if (typeof settings.subscribe === 'function') { + settings.subscribe('branding', () => { + void applyBrand(); + }); + ctx.logger.info('Auth: bound appName to settings namespace=branding'); + } - // #2815 — bind the auth SMS locale to the deployment default - // (`localization.locale`) so OTP/invitation texts render in the - // workspace language. #14319 binds the auth EMAIL locale from - // the very same read, so the two auth channels can no longer - // disagree about what language this workspace speaks. - // Live-rebinds on settings changes. - const applyLocalizationLocale = async () => { - try { - const resolved = await settings.get('localization', 'locale', {}); - const value = resolved?.value; - this.authManager?.setDefaultSmsLocale( - typeof value === 'string' ? value : undefined, - ); - // Email takes the workspace language only when it was - // EXPLICITLY set; a manifest default leaves the app's - // build-time `i18n.defaultLocale` standing (block above). - const explicit = - resolved && resolved.source !== 'default' && typeof value === 'string' - ? value - : undefined; - applyEmailLocale(explicit); - if (explicit) { - ctx.logger.info( - `Auth: bound auth email locale to localization.locale=${explicit}`, - ); - } - } catch (err: any) { - ctx.logger.warn( - 'Auth: failed to apply localization.locale: ' + (err?.message ?? err), + // #2815 — bind the auth SMS locale to the deployment default + // (`localization.locale`) so OTP/invitation texts render in the + // workspace language. #14319 binds the auth EMAIL locale from + // the very same read, so the two auth channels can no longer + // disagree about what language this workspace speaks. + // Live-rebinds on settings changes. + const applyLocalizationLocale = async () => { + try { + const resolved = await settings.get('localization', 'locale', {}); + const value = resolved?.value; + this.authManager?.setDefaultSmsLocale( + typeof value === 'string' ? value : undefined, + ); + // Email takes the workspace language only when it was + // EXPLICITLY set; a manifest default leaves the app's + // build-time `i18n.defaultLocale` standing (block above). + const explicit = + resolved && resolved.source !== 'default' && typeof value === 'string' + ? value + : undefined; + applyEmailLocale(explicit); + if (explicit) { + ctx.logger.info( + `Auth: bound auth email locale to localization.locale=${explicit}`, ); } - }; - await applyLocalizationLocale(); - if (typeof settings.subscribe === 'function') { - settings.subscribe('localization', () => { - void applyLocalizationLocale(); - }); + } catch (err: any) { + ctx.logger.warn( + 'Auth: failed to apply localization.locale: ' + (err?.message ?? err), + ); } + }; + await applyLocalizationLocale(); + if (typeof settings.subscribe === 'function') { + settings.subscribe('localization', () => { + void applyLocalizationLocale(); + }); } - } catch { - // settings service is optional — keep the configured appName. } - - // #2815 — seed the built-in bilingual auth SMS templates into - // sys_notification_template (insert-if-missing; tenant edits are - // never overwritten). Only meaningful when phone sign-in is on; - // the table may not exist yet on a fresh env (messaging provisions - // it at kernel:ready), so failures log-and-continue. - if (this.authManager.isPhoneNumberEnabled()) { - const engine = this.authManager.getDataEngine(); - if (engine) { - const { seedPhoneSmsTemplates } = await import('./phone-sms-texts.js'); - await seedPhoneSmsTemplates(engine, ctx.logger); - } + } catch { + // settings service is optional — keep the configured appName. + } + + // #2815 — seed the built-in bilingual auth SMS templates into + // sys_notification_template (insert-if-missing; tenant edits are + // never overwritten). Only meaningful when phone sign-in is on; + // the table may not exist yet on a fresh env (messaging provisions + // it at kernel:ready), so failures log-and-continue. + if (this.authManager.isPhoneNumberEnabled()) { + const engine = this.authManager.getDataEngine(); + if (engine) { + const { seedPhoneSmsTemplates } = await import('./phone-sms-texts.js'); + await seedPhoneSmsTemplates(engine, ctx.logger); } } + } + }); + // Defer HTTP route registration to kernel:ready hook. + // This ensures all plugins (including HonoServerPlugin) have completed + // their init and start phases before we attempt to look up the + // http-server service — making AuthPlugin resilient to plugin + // loading order. + if (this.options.registerRoutes) { + ctx.hook('kernel:ready', async () => { let httpServer: IHttpServer | null = null; try { httpServer = ctx.getService('http-server'); From c503acd447c15ae7fa8af2cc1c5ba756d6843259 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 02:13:17 +0000 Subject: [PATCH 2/2] chore(auth): add the changeset and re-anchor the system-context census row The census row for `plugin-auth`'s session-resolution middleware anchors a LINE NUMBER in `auth-plugin.ts`; the hook split moved that read site from :1353 to :1380 without changing a character of it. Re-anchored with the gate's own `--fix`, which is the repair it prescribes for pure line rot. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- .../auth-bindings-off-registerroutes.md | 42 +++++++++++++++++++ content/docs/permissions/system-context.mdx | 2 +- 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 .changeset/auth-bindings-off-registerroutes.md diff --git a/.changeset/auth-bindings-off-registerroutes.md b/.changeset/auth-bindings-off-registerroutes.md new file mode 100644 index 0000000000..5841cf6f09 --- /dev/null +++ b/.changeset/auth-bindings-off-registerroutes.md @@ -0,0 +1,42 @@ +--- +"@objectstack/plugin-auth": patch +--- + +fix(plugin-auth): the mail transport, brand and locale bindings no longer depend on `registerRoutes` + +`AuthPlugin` bound five things to the live kernel on `kernel:ready` — the +outbound mail transport (`setEmailService`), the SMS transport +(`setSmsService`), the deployment email locale (`setDefaultEmailLocale`), the +brand name (`setAppName`) and the SMS locale (`setDefaultSmsLocale`) — from +inside the same hook that mounts `/api/v1/auth/*`, and that hook was gated on +`registerRoutes`. + +`registerRoutes` answers a transport-mounting question: should this plugin put +its own routes on the kernel's `http-server`. The bindings are service +composition, and they are true of an embedding regardless of who serves the +routes. So an embedding that serves auth routes itself — the whole point of +`registerRoutes: false` — came up with no mail transport, no locale on either +channel and no brand binding. Silently: the `logger.info` lines that report the +wiring were inside the same skipped block, and the `localization` settings +namespace was not even read. One visible consequence was that the workspace +language could not reach auth mail on such a host at all, and +`/api/v1/auth/config` answered `requireEmailVerification: false` because +`resolveRequireEmailVerification()` saw no transport. + +The composition block now registers as its own unconditional +`ctx.hook('kernel:ready', …)` — the shape the sibling diagnosis and dev-seed +hooks in this plugin already use — placed before the route hook so a routing +host keeps the ordering the single combined hook gave it. + +Route registration itself stays gated: a `registerRoutes: false` kernel still +mounts no auth routes. + +**Behaviour change for `registerRoutes: false` embeddings.** They now resolve +the `email`, `sms`, `i18n` and `settings` services at `kernel:ready`, apply +`branding.workspace_name` and `localization.locale` (subscribing to both), seed +the built-in auth SMS templates when phone sign-in is enabled, and emit the +four wiring `info` lines. Hosts that had compensated by wiring these by hand +should expect the plugin's own binding to run as well; both paths are +idempotent setters, and an explicit workspace setting keeps outranking a +manifest default exactly as it does on a routing host. Nothing changes for a +host that leaves `registerRoutes` at its default. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index b26e78e827..26928a4a0a 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -97,7 +97,7 @@ that silently does not happen. | 8 | `explain()` may target a principal other than the caller | plugin-security | Get: no `manage_users` / delegated-admin check | `security-plugin.ts:3857` | | 9 | Anonymous-deny treats the caller as authenticated | core | Get: passes the 401 seam with no `userId` | `anonymous-deny.ts:154` | | 10 | Permission-set projection middleware skipped | plugin-security | Lose: projection of permission-set-derived columns | `permission-set-projection.ts:1015` | -| 11 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `auth-plugin.ts:1353` | +| 11 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `auth-plugin.ts:1380` | | 12 | Per-request performance timings disclosed | observability | Get: timing headers a normal caller cannot pull | `perf-timing.ts:474` | | 13 | Permission-set **overlay discard** skips the tenant-admin assertion | plugin-security | Get: an overlay can be discarded with no authenticated tenant administrator | `permission-set-overlay-discard.ts:142` | | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` |