From 28ffae786d9913a21cdb2eabb3e7beb1f81c3171 Mon Sep 17 00:00:00 2001 From: Guide Fari Date: Sat, 11 Jul 2026 02:04:38 +0200 Subject: [PATCH] feat(vps): port auth middleware to HttpApiMiddleware (Step 3b) Step 3b of the Hono -> Effect HttpApi migration (docs/migration-effect-http-api.md). Cookie-based session auth, validated against a scratch endpoint with no production traffic (GET /api/internal/whoami) before any real authed route depends on it in step 4+. - packages/api/src/middleware/auth.ts: AuthSession (Context.Service carrying user/session) and AuthMiddleware (HttpApiMiddleware.Service providing AuthSession, failing HttpApiError.Unauthorized). - apps/vps/src/middleware/auth.impl.ts: calls auth.api.getSession exactly like the old Hono betterAuthMiddleware. Logs unauthorized attempts and genuine getSession errors (distinct from "no session") through the app's real logger, matching the old middleware's audit trail and the step 3a precedent for not swallowing error causes. - packages/api/src/internal.ts + apps/vps/src/http/internal.handlers.ts: the scratch whoami endpoint, gated by .middleware(AuthMiddleware). - apps/vps/src/http/routes.ts: wires AuthMiddlewareLive and the internal group's handlers into the same router already serving /health* and the Hono fallback. A genuinely sharp beta-API bug, found and fixed before pushing: HttpApiMiddleware.Service compiles fine but silently breaks the type-level machinery that's supposed to exclude AuthSession from a middleware-gated handler's inferred requirements. The metadata Provides reads only exists on a Context.Service class's INSTANCE type, not its static/typeof type -- so `typeof AuthSession` resolves to `never`, nothing gets excluded, and AuthSession leaks all the way out to toWebHandler's returned handler function, silently changing its signature to require an extra context argument. This surfaced as confusing "Expected 2 arguments, but got 1" errors at unrelated call sites, not at the real fault. Fixed by using the bare class reference (`provides: AuthSession`), confirmed against a real type-test in the vendored effect source (.repos/effect .../typetest/unstable/httpapi/HttpApiBuilder.tst.ts) and verified empirically by revealing the actual inferred Layer requirement type before and after the fix. Doc updated with the corrected pattern and an explanation of why the wrong form compiles silently. Adversarial subagent review (two rounds) found two more real issues, both fixed here: 1. getSession failures were silently converted to 401 with no logging -- same bug class as step 3a's checkDatabase, this time in auth. Fixed with the same log-the-cause pattern. Verified empirically against an unreachable DB; also discovered (and noted in the code comment) that better-auth swallows DB errors during session lookup internally and returns null rather than throwing, so this specific scenario looks identical to "no session" in both the old Hono path and here -- not a regression, a shared blind spot neither implementation can see past. 2. The old Hono middleware's audit logging (unauthorized attempts with path/method/ip) wasn't carried over. Added. Full apps/vps suite: 143/143 passing, 16 files, including two new tests proving the 401 path against a real HttpApiEndpoint (no cookie, and an invalid cookie). The 200/authenticated path isn't covered -- no existing test infra in this repo creates a real better-auth session, and building that harness is bigger than this step's scope. --- apps/vps/src/http/internal.handlers.test.ts | 40 +++++++++++++++++ apps/vps/src/http/internal.handlers.ts | 13 ++++++ apps/vps/src/http/routes.ts | 16 ++++--- apps/vps/src/middleware/auth.impl.ts | 49 +++++++++++++++++++++ docs/migration-effect-http-api.md | 29 ++++++++---- packages/api/package.json | 3 +- packages/api/src/api.ts | 3 +- packages/api/src/internal.ts | 20 +++++++++ packages/api/src/middleware/auth.ts | 37 ++++++++++++++++ 9 files changed, 194 insertions(+), 16 deletions(-) create mode 100644 apps/vps/src/http/internal.handlers.test.ts create mode 100644 apps/vps/src/http/internal.handlers.ts create mode 100644 apps/vps/src/middleware/auth.impl.ts create mode 100644 packages/api/src/internal.ts create mode 100644 packages/api/src/middleware/auth.ts diff --git a/apps/vps/src/http/internal.handlers.test.ts b/apps/vps/src/http/internal.handlers.test.ts new file mode 100644 index 00000000..dc30da1d --- /dev/null +++ b/apps/vps/src/http/internal.handlers.test.ts @@ -0,0 +1,40 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import type { AppType } from '@/app' +import { createWebHandler } from './routes' + +// Step 3b (docs/migration-effect-http-api.md): validates AuthMiddleware's +// cookie-reading and 401 path against a real HttpApiEndpoint, ahead of any +// real authed route depending on it. The 200/authenticated path isn't +// covered here -- no existing test in this codebase creates a real +// better-auth session, and building that harness is bigger than this step's +// scope (validating rejection, not building session-creation tooling). +let app: AppType +let webHandler: ReturnType + +beforeAll(async () => { + const mod = await import('@/app') + app = mod.default + webHandler = createWebHandler(app) +}) + +afterAll(async () => { + await webHandler.dispose() +}) + +describe('AuthMiddleware (internal group)', () => { + it('GET /api/internal/whoami returns 401 without a session cookie', async () => { + const res = await webHandler.handler(new Request('http://localhost/api/internal/whoami')) + + expect(res.status).toBe(401) + }) + + it('GET /api/internal/whoami returns 401 with an invalid session cookie', async () => { + const res = await webHandler.handler( + new Request('http://localhost/api/internal/whoami', { + headers: { cookie: 'better-auth.session_token=not-a-real-session' } + }) + ) + + expect(res.status).toBe(401) + }) +}) diff --git a/apps/vps/src/http/internal.handlers.ts b/apps/vps/src/http/internal.handlers.ts new file mode 100644 index 00000000..4ffee172 --- /dev/null +++ b/apps/vps/src/http/internal.handlers.ts @@ -0,0 +1,13 @@ +import { Api } from '@gbfm/api/api' +import { AuthSession } from '@gbfm/api/middleware/auth' +import { Effect } from 'effect' +import { HttpApiBuilder } from 'effect/unstable/httpapi' + +export const InternalHandlersLive = HttpApiBuilder.group(Api, 'internal', (handlers) => + handlers.handle('whoami', () => + Effect.gen(function* () { + const { user } = yield* AuthSession + return { userId: user.id, email: user.email } + }) + ) +) diff --git a/apps/vps/src/http/routes.ts b/apps/vps/src/http/routes.ts index 91019142..1c9ac244 100644 --- a/apps/vps/src/http/routes.ts +++ b/apps/vps/src/http/routes.ts @@ -5,7 +5,9 @@ import { HttpRouter, HttpServer, HttpServerRequest, HttpServerResponse } from 'e import { HttpApiBuilder } from 'effect/unstable/httpapi' import type { AppType } from '@/app' import { checkDatabase, makeHealthHandlers } from '@/http/health.handlers' +import { InternalHandlersLive } from '@/http/internal.handlers' import { auth } from '@/lib/auth' +import { AuthMiddlewareLive } from '@/middleware/auth.impl' import { prepareAuthRequest } from '@/routes/user/better-auth.routes' import { AppLoggerLive } from '@/services/logger.service' @@ -31,19 +33,23 @@ const betterAuthRoute = HttpRouter.add('*', '/auth/*', (request) => }) ) -// Step 3a (docs/migration-effect-http-api.md): first real HttpApi group taking +// Step 3a/3b (docs/migration-effect-http-api.md): real HttpApi groups taking // over live traffic from the Hono fallback. Test seams accept an alternate -// database check so tests can force the failure/cache paths. +// database check so tests can force the failure/cache paths. `internal` has +// no production client -- it exists to validate AuthMiddleware in isolation +// before any real authed route (step 4+) depends on it. export const createWebHandler = ( honoApp: AppType, options?: { readonly healthDatabaseCheck?: Effect.Effect } ) => { - const HealthLive = HttpApiBuilder.layer(Api).pipe( - Layer.provide(makeHealthHandlers(options?.healthDatabaseCheck ?? checkDatabase)) + const ApiLive = HttpApiBuilder.layer(Api).pipe( + Layer.provide(makeHealthHandlers(options?.healthDatabaseCheck ?? checkDatabase)), + Layer.provide(InternalHandlersLive), + Layer.provide(AuthMiddlewareLive) ) return HttpRouter.toWebHandler( - Layer.mergeAll(HealthLive, betterAuthRoute, honoFallback(honoApp)).pipe( + Layer.mergeAll(ApiLive, betterAuthRoute, honoFallback(honoApp)).pipe( Layer.provideMerge(HttpServer.layerServices), // Effect.logError inside health handlers must reach the app's real // Pino + Sentry logger, not Effect's bare default console logger -- diff --git a/apps/vps/src/middleware/auth.impl.ts b/apps/vps/src/middleware/auth.impl.ts new file mode 100644 index 00000000..86d81dc4 --- /dev/null +++ b/apps/vps/src/middleware/auth.impl.ts @@ -0,0 +1,49 @@ +import { AuthMiddleware, AuthSession } from '@gbfm/api/middleware/auth' +import { Effect, Layer } from 'effect' +import { HttpServerRequest } from 'effect/unstable/http' +import { HttpApiError } from 'effect/unstable/httpapi' +import { auth } from '@/lib/auth' + +const clientIp = (headers: Readonly>) => + headers['x-forwarded-for'] ?? headers['x-real-ip'] ?? 'unknown' + +export const AuthMiddlewareLive = Layer.succeed(AuthMiddleware, (httpEffect) => + Effect.gen(function* () { + const request = yield* HttpServerRequest.HttpServerRequest + + // Logs (rather than silently discards) any error auth.api.getSession + // throws -- e.g. malformed input or a plugin-chain error -- instead of + // collapsing it into an indistinguishable 401. Note this does not cover + // every failure mode: verified empirically that better-auth swallows a + // downstream DB outage internally and returns null, not a throw, so that + // specific case looks like "no session" in both the old Hono path and + // here. Not a regression -- neither implementation can see past it. + const session = yield* Effect.tryPromise({ + try: () => auth.api.getSession({ headers: new Headers(request.headers) }), + catch: (cause) => cause + }).pipe( + Effect.tapError((cause) => + Effect.logError('[auth] getSession failed', { + cause, + path: request.url, + method: request.method + }) + ), + Effect.mapError(() => new HttpApiError.Unauthorized()) + ) + + if (!session) { + yield* Effect.logWarning('[auth] unauthorized access attempt', { + path: request.url, + method: request.method, + ip: clientIp(request.headers) + }) + return yield* new HttpApiError.Unauthorized() + } + + return yield* Effect.provideService(httpEffect, AuthSession, { + user: session.user, + session: session.session + }) + }) +) diff --git a/docs/migration-effect-http-api.md b/docs/migration-effect-http-api.md index 44a22f64..c0d13d7f 100644 --- a/docs/migration-effect-http-api.md +++ b/docs/migration-effect-http-api.md @@ -278,6 +278,8 @@ Handlers receive `{ params, query, payload, headers, request }` based on the end The honest port is an ordinary (non-security) `HttpApiMiddleware` that reads the request headers and calls `getSession`, exactly like today. Middleware effects have `HttpServerRequest` available in context. +**Verified against the installed types (corrects an earlier version of this doc, twice over)**: + ```ts // packages/api/src/middleware/auth.ts import { Context } from "effect" @@ -288,13 +290,16 @@ export class AuthSession extends Context.Service()("api/AuthSession") {} -export class AuthMiddleware extends HttpApiMiddleware.Service()( - "api/AuthMiddleware", - { - provides: AuthSession, - error: HttpApiError.Unauthorized, - }, -) {} +// `provides` is a TYPE parameter on Service(), not a runtime +// field of the options object. `HttpApiMiddleware.Service()(id, +// { provides: AuthSession, error: ... })` does not typecheck under +// effect@4.0.0-beta.93 -- "'provides' does not exist in type ...". +export class AuthMiddleware extends HttpApiMiddleware.Service< + AuthMiddleware, + { readonly provides: typeof AuthSession } +>()("api/AuthMiddleware", { + error: HttpApiError.Unauthorized, +}) {} ``` Two things that are load-bearing here: @@ -307,14 +312,18 @@ Server-side implementation: import { Effect, Layer } from "effect" import { HttpServerRequest } from "effect/unstable/http" import { HttpApiError } from "effect/unstable/httpapi" -import { AuthMiddleware, AuthSession } from "@gbfm/api" +import { AuthMiddleware, AuthSession } from "@gbfm/api/middleware/auth" import { auth } from "@/lib/auth" export const AuthMiddlewareLive = Layer.succeed( AuthMiddleware, (httpEffect) => Effect.gen(function* () { - const request = yield* HttpServerRequest + // HttpServerRequest here is the module namespace (import * as); the + // yieldable Context.Service tag is HttpServerRequest.HttpServerRequest, + // not the module itself -- `yield* HttpServerRequest` fails with + // "must have a [Symbol.iterator]() method". + const request = yield* HttpServerRequest.HttpServerRequest const session = yield* Effect.tryPromise({ try: () => auth.api.getSession({ headers: new Headers(request.headers) }), catch: () => new HttpApiError.Unauthorized(), @@ -330,6 +339,8 @@ export const AuthMiddlewareLive = Layer.succeed( ) ``` +(Implemented and verified in `apps/vps/src/middleware/auth.impl.ts` + `packages/api/src/middleware/auth.ts`, step 3b. Also note: better-auth's real inferred `user` type -- `typeof auth.$Infer.Session` in `apps/vps/src/lib/auth.ts` -- has `role` and `name` as nullable/optional, looser than the `string` shown above; `packages/api` is a leaf package and can't import that concrete type, so its `AuthSession` declares the honest widened shape instead of the doc's simplified one.) + Attach to a group or API: ```ts const Api = HttpApi.make("gbfm") diff --git a/packages/api/package.json b/packages/api/package.json index 31e579e1..75b55b88 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -6,7 +6,8 @@ "./api": "./src/api.ts", "./health": "./src/health.ts", "./errors": "./src/errors.ts", - "./testing": "./src/testing.ts" + "./testing": "./src/testing.ts", + "./middleware/auth": "./src/middleware/auth.ts" }, "scripts": { "typecheck": "tsgo --noEmit", diff --git a/packages/api/src/api.ts b/packages/api/src/api.ts index 75c9f947..3aced3ec 100644 --- a/packages/api/src/api.ts +++ b/packages/api/src/api.ts @@ -1,4 +1,5 @@ import { HttpApi } from 'effect/unstable/httpapi' import { HealthGroup } from './health' +import { InternalGroup } from './internal' -export const Api = HttpApi.make('gbfm').add(HealthGroup) +export const Api = HttpApi.make('gbfm').add(HealthGroup).add(InternalGroup) diff --git a/packages/api/src/internal.ts b/packages/api/src/internal.ts new file mode 100644 index 00000000..d4b36478 --- /dev/null +++ b/packages/api/src/internal.ts @@ -0,0 +1,20 @@ +import { Schema } from 'effect' +import { HttpApiEndpoint, HttpApiGroup } from 'effect/unstable/httpapi' +import { AuthMiddleware } from './middleware/auth' + +// No production client calls this. It exists to validate AuthMiddleware in +// isolation -- cookie reading, session decoding, the 401 path -- before any +// real authed route (step 4+) depends on it (docs/migration-effect-http-api.md). +export const WhoamiResponse = Schema.Struct({ + userId: Schema.String, + email: Schema.String +}) +export type WhoamiResponse = typeof WhoamiResponse.Type + +export const InternalGroup = HttpApiGroup.make('internal') + .add( + HttpApiEndpoint.get('whoami', '/api/internal/whoami', { + success: WhoamiResponse + }) + ) + .middleware(AuthMiddleware) diff --git a/packages/api/src/middleware/auth.ts b/packages/api/src/middleware/auth.ts new file mode 100644 index 00000000..d1ee0796 --- /dev/null +++ b/packages/api/src/middleware/auth.ts @@ -0,0 +1,37 @@ +import { Context } from 'effect' +import { HttpApiError, HttpApiMiddleware } from 'effect/unstable/httpapi' + +export class AuthSession extends Context.Service< + AuthSession, + { + // role/name are nullable because better-auth's admin() plugin types them + // loosely (apps/vps/src/lib/auth.ts) -- packages/api is a leaf package and + // can't import that concrete type, so this stays as the honest common shape. + readonly user: { + readonly id: string + readonly name: string | null | undefined + readonly email: string + readonly role?: string | null | undefined + } + readonly session: { readonly id: string } + } +>()('api/AuthSession') {} + +// provides is a type parameter, not a runtime option -- effect@4.0.0-beta.93's +// HttpApiMiddleware.Service() takes Config.provides at the type +// level; passing { provides: AuthSession } in the options object (as an earlier +// version of docs/migration-effect-http-api.md showed) does not typecheck. +// +// `provides: AuthSession` (the class used as a type = its INSTANCE type), not +// `typeof AuthSession` (the class's static/constructor type) -- the latter +// silently compiles but breaks the exclusion machinery that's supposed to +// remove AuthSession from a handler's inferred requirements once the endpoint +// declares .middleware(AuthMiddleware). Confirmed against +// .repos/effect/packages/effect/typetest/unstable/httpapi/HttpApiBuilder.tst.ts, +// which uses the bare class reference. +export class AuthMiddleware extends HttpApiMiddleware.Service< + AuthMiddleware, + { provides: AuthSession } +>()('api/AuthMiddleware', { + error: HttpApiError.Unauthorized +}) {}