Skip to content

Commit 97ee0af

Browse files
authored
fix(auth): match the bearer scheme case-insensitively (#198)
Closes #188 Rebased onto `main` after #197 landed. That PR touched the same functions, so this one is now scoped down to the single thing #188 is about — see "Relationship to #197" below. ## Summary - HTTP authentication schemes are case-insensitive (RFC 9110 §11.1), but `readBearerCredential` matched the header against a literal `Bearer ` prefix. Clients that normalize the scheme to `bearer` or `BEARER` were rejected with 401 even with a valid token. - Split the header into scheme + credentials and compare the scheme with `toLowerCase()`. Nothing else about the parse changes. ## Relationship to #197 #188 asked for two things: case-insensitive scheme matching, and a constant-time compare for the secret. **#197 already landed the constant-time half** (`matchesConfiguredToken` / `constantTimeEqual`). This PR is only the scheme half, layered on top — 8 lines of production change. #197 also made a deliberate decision I have preserved rather than reworked: `readBearerCredential` returns the credential **verbatim**, so configured tokens still require a byte-for-byte match. Concretely, that means: - Repeated separator spaces are still rejected. RFC 9110's ABNF is `1*SP`, so `bearer token` is arguably legal, but #197 pins `Bearer admin-secret` → 401, and loosening it would silently reverse that call in a PR about scheme casing. Left alone; worth a separate issue if anyone wants it. - No `.trim()` was added anywhere. The credential still reaches `constantTimeEqual` exactly as sent. The only behavior that changes is which scheme spellings parse. ## Changes `readBearerCredential` now finds the first space, compares `slice(0, separator).toLowerCase()` against `"bearer"`, and returns the remainder untouched. It is the only place in `src/` that reads an inbound `Authorization` header, so both call sites (`matchesConfiguredToken` for the env tokens, `readBearerToken` for runtime-token lookup) pick the fix up. Deliberately **not** widened: - `Bearer\t<token>` still 401s — RFC 9110 allows only `1*SP` as the separator, and `\s`-based splits over-accept HTAB/NBSP, creating a parser differential with strict intermediaries. - `BearerX ...` / `Bearer-Foo ...` still 401 — the scheme is compared as a whole slice, not with `startsWith`. `toLowerCase().startsWith("bearer")` would accept these and hand back `X <token>` as the credential. ## Test plan - [x] `npm test -- src/server/api/auth.test.ts` — scheme-casing matrix across all three auth scopes (`/api`, `/v1`, `/mcp`), cookie issuance for a lowercase scheme, admin elevation on `POST /v1/actions` with `BEARER`, and the dynamic runtime-token resolver path with a lowercase scheme (asserting it receives `"oct_valid"`, not a padded string). - [x] `npm test -- src/server/connect-server.test.ts` — `/api/auth/session` is a public path that authenticates through `readLocalAuthSession` rather than the middleware, so it needs its own coverage; plus an end-to-end `bearer oct_…` runtime-token call and a `bearer local-token` → 401 on `/v1/actions` proving the scope boundary did not move with the scheme. - [x] 6-case negative matrix, chosen not to overlap #197's byte-for-byte test. The load-bearing ones are `bearer admin-Secret` (credentials stay case-sensitive even when the scheme is lowercased — this catches the likeliest wrong fix, lowercasing the whole header) and `bearer admin-secret` (pins that case-insensitivity did not loosen separator handling either). - [x] Verified the new assertions are load-bearing: 8 of them fail against `origin/main` as it stands today, post-#197. - [x] `npm run fix-check` and the full `npm test` (548 tests) pass. - [x] Differential check old-vs-new over a large header/config matrix: zero inputs that authenticated before and 401 now. Every difference is the intended widening. ## Notes Checked on both runtimes, since this file runs on Node and on Workers via `cloudflare.ts`. The change adds no imports and nothing outside plain ECMAScript. Two adjacent gaps found while working on this, left out as out of scope: - 401 responses carry no `WWW-Authenticate` header anywhere in the repo (RFC 9110 §15.5.2 requires it; RFC 6750 §3 gives the Bearer form). MCP/OAuth clients use it to decide whether to refresh. - `src/server/api/openapi.ts` emits no `securitySchemes`, though `docs/runtime-api.md` advertises `/openapi.json` for importers. Happy to file either separately.
1 parent 6f172d3 commit 97ee0af

3 files changed

Lines changed: 113 additions & 4 deletions

File tree

src/server/api/auth.test.ts

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Hono } from "hono";
2-
import { describe, expect, it } from "vitest";
2+
import { describe, expect, it, vi } from "vitest";
33
import { createLocalAuthMiddleware } from "./auth.ts";
44

55
describe("createLocalAuthMiddleware", () => {
@@ -69,6 +69,61 @@ describe("createLocalAuthMiddleware", () => {
6969
).toBe(200);
7070
});
7171

72+
it.each(["Bearer", "bearer", "BEARER", "BeArEr"])(
73+
"accepts the %s authorization scheme on every auth scope",
74+
async (scheme) => {
75+
const app = createSchemeApp();
76+
77+
expect((await app.request("/api/connections", { headers: authorize(scheme, "admin-secret") })).status).toBe(200);
78+
expect((await app.request("/v1/actions", { headers: authorize(scheme, "runtime-secret") })).status).toBe(200);
79+
expect((await app.request("/mcp/tools", { headers: authorize(scheme, "runtime-secret") })).status).toBe(200);
80+
},
81+
);
82+
83+
it.each([
84+
"Basic admin-secret",
85+
"BearerX admin-secret",
86+
"Bearer-Foo admin-secret",
87+
"Bearer",
88+
"Bearer\tadmin-secret",
89+
// A case-insensitive scheme must not loosen anything else: the credentials stay
90+
// case-sensitive and are still matched byte-for-byte after a single separator space.
91+
"bearer admin-Secret",
92+
"bearer admin-secret",
93+
])("rejects the %j authorization header", async (authorization) => {
94+
const app = createSchemeApp();
95+
96+
expect((await app.request("/api/connections", { headers: { authorization } })).status).toBe(401);
97+
});
98+
99+
it("issues the admin session cookie for a lowercase bearer scheme", async () => {
100+
const app = createSchemeApp();
101+
102+
const response = await app.request("/api/connections", {
103+
headers: authorize("bearer", "admin-secret"),
104+
});
105+
106+
expect(response.status).toBe(200);
107+
expect(response.headers.get("set-cookie")).toContain("oomol_connect_admin_session=");
108+
expect(response.headers.get("set-cookie")).not.toContain("admin-secret");
109+
});
110+
111+
it("resolves dynamic runtime tokens for a lowercase bearer scheme", async () => {
112+
const resolveRuntimeToken = vi.fn(async (token: string) =>
113+
token === "oct_valid"
114+
? { tokenId: "token-1", allowedActions: [], blockedActions: [], allowedProxies: [] }
115+
: undefined,
116+
);
117+
const app = new Hono();
118+
app.use("*", createLocalAuthMiddleware({ hasRuntimeTokens: async () => true, resolveRuntimeToken }));
119+
app.get("/v1/actions", (context) => context.json({ ok: true }));
120+
121+
const response = await app.request("/v1/actions", { headers: authorize("bearer", "oct_valid") });
122+
123+
expect(response.status).toBe(200);
124+
expect(resolveRuntimeToken).toHaveBeenCalledWith("oct_valid");
125+
});
126+
72127
it("allows configured admin tokens to elevate POST /v1/actions", async () => {
73128
const app = new Hono();
74129
app.use(
@@ -102,6 +157,15 @@ describe("createLocalAuthMiddleware", () => {
102157
})
103158
).status,
104159
).toBe(200);
160+
expect(
161+
(
162+
await app.request("/v1/actions/example.echo", {
163+
method: "POST",
164+
headers: { ...authorize("BEARER", "admin-secret"), "content-type": "application/json" },
165+
body: JSON.stringify({ input: {} }),
166+
})
167+
).status,
168+
).toBe(200);
105169
});
106170

107171
it("matches configured tokens byte-for-byte after the bearer scheme", async () => {
@@ -130,3 +194,16 @@ describe("createLocalAuthMiddleware", () => {
130194
);
131195
});
132196
});
197+
198+
function createSchemeApp(): Hono {
199+
const app = new Hono();
200+
app.use("*", createLocalAuthMiddleware({ adminToken: "admin-secret", runtimeToken: "runtime-secret" }));
201+
app.get("/api/connections", (context) => context.json({ ok: true }));
202+
app.get("/v1/actions", (context) => context.json({ ok: true }));
203+
app.get("/mcp/tools", (context) => context.json({ ok: true }));
204+
return app;
205+
}
206+
207+
function authorize(scheme: string, token: string): Record<string, string> {
208+
return { authorization: `${scheme} ${token}` };
209+
}

src/server/api/auth.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { deleteCookie, getCookie, setCookie } from "hono/cookie";
66
import { isConsoleShellRequest } from "./console-paths.ts";
77
import { jsonError } from "./http-utils.ts";
88

9+
const bearerScheme = "bearer";
910
const authCookieName = "oomol_connect_admin_session";
1011
const authCookieVersion = "v1";
1112
const authCookieMaxAgeSeconds = 2_592_000;
@@ -266,9 +267,18 @@ function readBearerToken(context: Context): string | undefined {
266267
return normalizeToken(readBearerCredential(context));
267268
}
268269

269-
/** Bearer credential exactly as sent, so configured tokens still require a byte-for-byte match. */
270+
/**
271+
* Bearer credential exactly as sent, so configured tokens still require a byte-for-byte match.
272+
*
273+
* Authentication schemes are case-insensitive (RFC 9110), so `bearer` and `BEARER` are accepted;
274+
* only the credentials stay case-sensitive.
275+
*/
270276
function readBearerCredential(context: Context): string {
271277
const authorization = context.req.header("authorization") ?? "";
272-
const prefix = "Bearer ";
273-
return authorization.startsWith(prefix) ? authorization.slice(prefix.length) : "";
278+
const separator = authorization.indexOf(" ");
279+
if (separator < 0 || authorization.slice(0, separator).toLowerCase() !== bearerScheme) {
280+
return "";
281+
}
282+
283+
return authorization.slice(separator + 1);
274284
}

src/server/connect-server.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,6 +1102,17 @@ describe("ConnectServer", () => {
11021102
authenticated: true,
11031103
});
11041104

1105+
const lowercaseBearer = await app.request("/api/auth/session", {
1106+
headers: { authorization: "bearer local-token" },
1107+
});
1108+
expect(lowercaseBearer.status).toBe(200);
1109+
expect(lowercaseBearer.headers.get("set-cookie")).toContain("oomol_connect_admin_session=");
1110+
expect(lowercaseBearer.headers.get("set-cookie")).not.toContain("local-token");
1111+
await expect(lowercaseBearer.json()).resolves.toEqual({
1112+
adminAuthConfigured: true,
1113+
authenticated: true,
1114+
});
1115+
11051116
const authorized = await app.request("/api/providers", {
11061117
headers: { authorization: "Bearer local-token" },
11071118
});
@@ -1183,6 +1194,17 @@ describe("ConnectServer", () => {
11831194
headers: { authorization: `Bearer ${createdBody.token}` },
11841195
});
11851196
expect(runtimeTokenCall.status).toBe(200);
1197+
1198+
// A case-insensitive scheme widens the scheme only, never the auth scope.
1199+
const lowercaseRuntimeTokenCall = await app.request("/v1/actions", {
1200+
headers: { authorization: `bearer ${createdBody.token}` },
1201+
});
1202+
expect(lowercaseRuntimeTokenCall.status).toBe(200);
1203+
1204+
const lowercaseAdminTokenRuntimeCall = await app.request("/v1/actions", {
1205+
headers: { authorization: "bearer local-token" },
1206+
});
1207+
expect(lowercaseAdminTokenRuntimeCall.status).toBe(401);
11861208
});
11871209

11881210
it("manages runtime tokens and gates runtime API calls after one is created", async () => {

0 commit comments

Comments
 (0)