Skip to content

Commit ea24593

Browse files
authored
fix(plugin-auth): the auth catch-all yields paths better-auth does not own (#4088) (#4092)
`registerAuthRoutes` mounts `rawApp.all('${basePath}/*')` over the whole auth namespace, and that handler was TERMINAL: it returned better-auth's response unconditionally, including the 404 better-auth produces for a path it does not implement. Any other plugin's route under that prefix was reachable only if it happened to register first, so a load-bearing surface depended on `kernel.use()` order — plugin-hono-server mounts `/auth/me/permissions` and `/auth/me/localization` from its own `kernel:ready` hook, objectui's whole permission layer reads the former and core's auth gate allow-lists the latter, and all of it silently 404s if AuthPlugin is registered first. Same class as #2567 and #4018: an invariant held by ordering luck rather than enforced. A 404 from better-auth now means "not my path" and the catch-all yields, in either order. Narrow by design: only 404 falls through (401/403 are real answers), precedence still favours the namespace owner, and when nothing downstream answers better-auth's own 404 is returned verbatim so the unclaimed- path wire shape is unchanged. Two mechanism details, both found by hitting them: `c.finalized` cannot discriminate (reaching the end of the chain runs Hono's notFound, which sets a response and flips the flag), so the check is on status; and the response must be assigned to `c.res` rather than returned, because the ADR-0069 D5 IP gate puts a `use()` middleware in the chain and Hono's compose only assigns a returned Response while `finalized` is false. New `auth-catchall-fallthrough.test.ts` drives a real Hono app with better-auth stubbed by a path table, registering the catch-all FIRST and the specific route SECOND — the order that used to fail. Verified to bite: reverting the fix fails the two `/auth/me/*` cases. Adds `hono` as a plugin-auth devDependency so the test exercises the real Hono chain instead of a stand-in. Local: plugin-auth 579, client 200, plugin-hono-server 132, http-conformance 46, rest auth-gate 4, and the qa/dogfood OIDC authorization-code flow 3 — the last being the proof that better-auth's own endpoints still work through the patched catch-all.
1 parent 7ac1995 commit ea24593

5 files changed

Lines changed: 228 additions & 13 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
fix(plugin-auth): the auth catch-all yields paths better-auth does not own (#4088)
6+
7+
`registerAuthRoutes` mounts `rawApp.all('${basePath}/*')` over the whole auth
8+
namespace (`/api/v1/auth` by default), and that handler was **terminal**: it
9+
returned better-auth's response unconditionally, including the 404 better-auth
10+
produces for a path it does not implement. Any other plugin's route under that
11+
prefix was therefore reachable only if it happened to register **first** — Hono
12+
runs handlers matching a path in registration order and the first to return a
13+
Response wins.
14+
15+
That put a load-bearing surface at the mercy of `kernel.use()` order.
16+
`@objectstack/plugin-hono-server` mounts `/auth/me/permissions` and
17+
`/auth/me/localization` from its own `kernel:ready` hook; objectui's entire
18+
permission layer reads the former and `core`'s auth gate allow-lists the latter
19+
as an endpoint a gated user must still reach. Register `AuthPlugin` before
20+
`HonoServerPlugin` and all of it silently 404s.
21+
22+
A 404 from better-auth now means "this path is not mine" and the catch-all yields
23+
to whatever else matched, in either registration order. Deliberately narrow:
24+
25+
- **Only 404 falls through.** 401/403 are real better-auth answers, not
26+
disclaimers of ownership.
27+
- **Precedence still favours the namespace owner.** better-auth wins every path
28+
it implements; only its leftovers are up for grabs.
29+
- **The unclaimed-path wire shape is unchanged.** When nothing downstream
30+
answers, better-auth's own 404 is returned verbatim rather than Hono's
31+
`404 Not Found`.
32+
33+
No configuration changes and no new routes. The only behavioural difference for
34+
an existing deployment is that a route another plugin mounts under
35+
`/api/v1/auth/*` now answers regardless of plugin order — previously it answered
36+
only in the lucky order.

packages/plugins/plugin-auth/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
},
3434
"devDependencies": {
3535
"@types/node": "^26.1.1",
36+
"hono": "^4.12.32",
3637
"typescript": "^6.0.3",
3738
"vitest": "^4.1.10"
3839
},
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #4088 — the auth catch-all must not swallow other plugins' routes.
5+
*
6+
* `registerAuthRoutes` mounts `rawApp.all('${basePath}/*')` over the whole auth
7+
* namespace. It used to be TERMINAL — better-auth's response was returned
8+
* unconditionally, including the 404 better-auth produces for a path it does
9+
* not implement. Any other plugin's route under that prefix was therefore
10+
* reachable only if it happened to register FIRST (Hono runs handlers matching
11+
* a path in registration order; first to return a Response wins).
12+
*
13+
* That made a load-bearing surface depend on `kernel.use()` order:
14+
* `plugin-hono-server` mounts `/auth/me/permissions` + `/auth/me/localization`
15+
* from its own `kernel:ready` hook, objectui's whole permission layer reads the
16+
* former and `core`'s auth gate allow-lists the latter — and all of it silently
17+
* 404s if AuthPlugin is registered first.
18+
*
19+
* These tests register the catch-all FIRST and the specific route SECOND, i.e.
20+
* exactly the order that used to fail, and drive a real Hono app so the
21+
* assertions are about the shipped handler rather than a stand-in.
22+
*/
23+
24+
import { describe, it, expect, vi, beforeEach } from 'vitest';
25+
import { Hono } from 'hono';
26+
import { AuthPlugin } from './auth-plugin';
27+
import type { PluginContext } from '@objectstack/core';
28+
29+
const BASE = '/api/v1/auth';
30+
31+
/** better-auth's own 404 for a path it does not implement. */
32+
const BETTER_AUTH_404 = { message: 'Not Found', code: 'NOT_FOUND' };
33+
34+
/**
35+
* Mount the plugin's real route registration on a real Hono app, with
36+
* better-auth stubbed by a path → Response table so we control exactly which
37+
* paths the namespace owner claims.
38+
*/
39+
async function mountCatchAll(owned: Record<string, () => Response>) {
40+
const app = new Hono();
41+
const ctx: PluginContext = {
42+
registerService: vi.fn(),
43+
getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)),
44+
getServices: vi.fn(() => new Map()),
45+
hook: vi.fn(),
46+
trigger: vi.fn(),
47+
logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() },
48+
getKernel: vi.fn(),
49+
} as any;
50+
51+
const plugin = new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long!!' });
52+
await plugin.init(ctx);
53+
54+
const handleRequest = vi.fn(async (req: Request) => {
55+
const path = new URL(req.url).pathname;
56+
const make = owned[path];
57+
if (make) return make();
58+
return new Response(JSON.stringify(BETTER_AUTH_404), {
59+
status: 404,
60+
headers: { 'Content-Type': 'application/json' },
61+
});
62+
});
63+
(plugin as any).authManager = { handleRequest };
64+
65+
const httpServer: any = { getRawApp: () => app, getPort: () => 0 };
66+
(plugin as any).registerAuthRoutes(httpServer, ctx);
67+
68+
return { app, handleRequest };
69+
}
70+
71+
describe('auth catch-all yields paths better-auth does not own (#4088)', () => {
72+
let mounted: Awaited<ReturnType<typeof mountCatchAll>>;
73+
74+
beforeEach(async () => {
75+
mounted = await mountCatchAll({
76+
// A path better-auth really implements, plus one that answers 401.
77+
[`${BASE}/get-session`]: () => new Response(JSON.stringify({ user: null }), { status: 200 }),
78+
[`${BASE}/protected`]: () => new Response(JSON.stringify({ error: 'nope' }), { status: 401 }),
79+
});
80+
});
81+
82+
it('lets a LATER-registered route answer — the order that used to 404', async () => {
83+
// The real collision: plugin-hono-server mounts this from its own
84+
// kernel:ready hook, which may run after AuthPlugin's.
85+
mounted.app.get(`${BASE}/me/permissions`, (c) => c.json({ authenticated: true, from: 'hono-plugin' }));
86+
87+
const res = await mounted.app.request(`http://localhost${BASE}/me/permissions`);
88+
89+
expect(res.status).toBe(200);
90+
expect(await res.json()).toEqual({ authenticated: true, from: 'hono-plugin' });
91+
});
92+
93+
it('does the same for /auth/me/localization', async () => {
94+
mounted.app.get(`${BASE}/me/localization`, (c) => c.json({ authenticated: true, currency: 'USD' }));
95+
96+
const res = await mounted.app.request(`http://localhost${BASE}/me/localization`);
97+
98+
expect(res.status).toBe(200);
99+
expect(await res.json()).toEqual({ authenticated: true, currency: 'USD' });
100+
});
101+
102+
it('keeps better-auth winning every path it DOES own', async () => {
103+
// Precedence must still favour the namespace owner: a later route must
104+
// not be able to hijack a real better-auth endpoint.
105+
mounted.app.get(`${BASE}/get-session`, (c) => c.json({ hijacked: true }));
106+
107+
const res = await mounted.app.request(`http://localhost${BASE}/get-session`);
108+
109+
expect(res.status).toBe(200);
110+
expect(await res.json()).toEqual({ user: null });
111+
});
112+
113+
it('does NOT fall through on 401 — that is a real answer, not a disclaimer', async () => {
114+
mounted.app.get(`${BASE}/protected`, (c) => c.json({ leaked: true }));
115+
116+
const res = await mounted.app.request(`http://localhost${BASE}/protected`);
117+
118+
expect(res.status).toBe(401);
119+
expect(await res.json()).toEqual({ error: 'nope' });
120+
});
121+
122+
it('returns better-auth’s own 404 verbatim when nothing else matches', async () => {
123+
// The wire shape for a genuinely unclaimed auth path must not change:
124+
// no route is registered for this path, so the fall-through finds
125+
// nothing and better-auth's 404 stands.
126+
const res = await mounted.app.request(`http://localhost${BASE}/no-such-endpoint`);
127+
128+
expect(res.status).toBe(404);
129+
expect(await res.json()).toEqual(BETTER_AUTH_404);
130+
});
131+
132+
it('still forwards to better-auth exactly once per request', async () => {
133+
mounted.app.get(`${BASE}/me/permissions`, (c) => c.json({ ok: true }));
134+
mounted.handleRequest.mockClear();
135+
136+
await mounted.app.request(`http://localhost${BASE}/me/permissions`);
137+
138+
expect(mounted.handleRequest).toHaveBeenCalledTimes(1);
139+
});
140+
});

packages/plugins/plugin-auth/src/auth-plugin.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1814,11 +1814,56 @@ export class AuthPlugin implements Plugin {
18141814
// Register wildcard route to forward all auth requests to better-auth.
18151815
// better-auth is configured with basePath matching our route prefix, so we
18161816
// forward the original request directly — no path rewriting needed.
1817-
rawApp.all(`${basePath}/*`, async (c: any) => {
1817+
//
1818+
// [#4088] This catch-all owns the whole auth namespace, and it used to be
1819+
// TERMINAL: it returned better-auth's response unconditionally, including
1820+
// the 404 better-auth produces for a path it does not implement. Any OTHER
1821+
// plugin's route under `${basePath}/*` was therefore reachable only if it
1822+
// happened to register FIRST — Hono runs handlers matching a path in
1823+
// registration order and the first to return a Response wins. That made a
1824+
// load-bearing surface depend on `kernel.use()` order:
1825+
// `plugin-hono-server` mounts `/auth/me/permissions` and
1826+
// `/auth/me/localization` from its own `kernel:ready` hook, the console's
1827+
// entire permission layer reads the former, and `core`'s auth gate
1828+
// allow-lists the latter — all of it silently 404s if AuthPlugin is used
1829+
// before HonoServerPlugin. Same class as #2567 and #4018: an invariant held
1830+
// by ordering luck rather than enforced.
1831+
//
1832+
// So a 404 now means "better-auth does not own this path" and we yield to
1833+
// whatever else matched, in either registration order. Deliberately narrow:
1834+
// 401/403 are real better-auth answers, not disclaimers of ownership, and
1835+
// when nothing downstream answers we return better-auth's own 404 verbatim
1836+
// so the wire shape for a genuinely unclaimed auth path is unchanged.
1837+
// Precedence still favours the namespace owner — better-auth wins every
1838+
// path it implements, and only its leftovers are up for grabs.
1839+
rawApp.all(`${basePath}/*`, async (c: any, next: any) => {
18181840
try {
18191841
// Forward the original request to better-auth handler
18201842
const response = await this.authManager!.handleRequest(c.req.raw);
18211843

1844+
if (response.status === 404) {
1845+
await next();
1846+
// A non-404 downstream means something else answered — hand that back.
1847+
// NOT `c.finalized`: reaching the end of the chain with nothing
1848+
// matched runs Hono's notFound handler, which sets a response and
1849+
// flips `finalized` to true, so it cannot tell "someone answered"
1850+
// from "nobody did". Status can. The one thing this trades away is a
1851+
// downstream route's own 404 BODY (better-auth's 404 is returned in
1852+
// its place); the status is identical either way, and no route under
1853+
// this prefix answers 404 today — `/auth/me/*` answer 200 with an
1854+
// `authenticated: false` payload for an anonymous caller.
1855+
if (c.res && c.res.status !== 404) return;
1856+
// Assign, don't `return`: the IP-gate `rawApp.use()` above puts a
1857+
// middleware in this chain, and Hono's compose only assigns a
1858+
// handler's returned Response while `c.finalized` is false. The
1859+
// notFound above already flipped it, so a `return response` here is
1860+
// silently dropped and the caller gets Hono's "404 Not Found" text
1861+
// instead of better-auth's JSON. Setting `c.res` is not subject to
1862+
// that condition.
1863+
c.res = response;
1864+
return;
1865+
}
1866+
18221867
// better-auth catches internal errors and returns error Responses
18231868
// without throwing, so the catch block below would never trigger.
18241869
// We proactively log server errors here for observability.

pnpm-lock.yaml

Lines changed: 5 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)