Fix HttpApiBuilder security middleware cache reuse (#1837)

Co-authored-by: Tim Smart <hello@timsmart.co>

Author: kitlangton

.changeset/fix-httpapi-security-middleware-cache.md

@@ -0,0 +1,5 @@
+---
+"effect": patch
+---
+
+Fix `HttpApiBuilder` security middleware caching so separate handler builds do not reuse the first provided middleware implementation.

packages/effect/src/unstable/httpapi/HttpApiBuilder.ts

@@ -652,21 +652,22 @@ const applyMiddleware = <A extends Effect.Effect<any, any, any>>(
 }
 
 const securityMiddlewareCache = new WeakMap<
-  any,
+  object,
   (effect: Effect.Effect<any, any, any>, options: any) => Effect.Effect<any, any, any>
 >()
 
 const makeSecurityMiddleware = (
   key: HttpApiMiddleware.AnyServiceSecurity,
   service: HttpApiMiddleware.HttpApiMiddlewareSecurity<any, any, any, any>
 ): (effect: Effect.Effect<any, any, any>, options: any) => Effect.Effect<any, any, any> => {
-  if (securityMiddlewareCache.has(key)) {
-    return securityMiddlewareCache.get(key)!
+  const cached = securityMiddlewareCache.get(service)
+  if (cached !== undefined) {
+    return cached
   }
 
-  const entries = Object.entries(key.security).map(([key, security]) => ({
+  const entries = Object.entries(key.security).map(([securityKey, security]) => ({
     decode: securityDecode(security),
-    middleware: service[key]
+    middleware: service[securityKey]
   }))
   if (entries.length === 0) {
     return identity
@@ -694,7 +695,7 @@ const makeSecurityMiddleware = (
     return yield* lastResult!.asEffect()
   })
 
-  securityMiddlewareCache.set(key, middleware)
+  securityMiddlewareCache.set(service, middleware)
   return middleware
 }
 

packages/platform-node/test/HttpApi.test.ts

@@ -484,6 +484,81 @@ describe("HttpApi", () => {
       }).pipe(Effect.provide(ApiLive))
     })
 
+    it("security middleware cache does not reuse the first service impl", async () => {
+      class CurrentMarker extends ServiceMap.Service<CurrentMarker, string>()("CurrentMarker") {}
+
+      class M extends HttpApiMiddleware.Service<M, {
+        provides: CurrentMarker
+      }>()("Server/Security/Repro", {
+        security: {
+          apiKey: HttpApiSecurity.apiKey({
+            in: "header",
+            key: "authorization"
+          })
+        }
+      }) {}
+
+      const Api = HttpApi.make("api").add(
+        HttpApiGroup.make("group")
+          .add(HttpApiEndpoint.get("a", "/a", {
+            success: Schema.Struct({
+              marker: Schema.String
+            })
+          }))
+          .middleware(M)
+      )
+
+      const makeWebHandler = (marker: string) => {
+        const GroupLive = HttpApiBuilder.group(
+          Api,
+          "group",
+          (handlers) => handlers.handle("a", () => Effect.map(CurrentMarker.asEffect(), (marker) => ({ marker })))
+        )
+        const MLive = Layer.succeed(M)({
+          apiKey: (effect) => Effect.provideService(effect, CurrentMarker, marker)
+        })
+
+        return HttpRouter.toWebHandler(
+          Layer.mergeAll(
+            HttpApiBuilder.layer(Api).pipe(
+              Layer.provide(GroupLive),
+              Layer.provide(MLive),
+              Layer.provide(HttpServer.layerServices)
+            )
+          ),
+          { disableLogger: true }
+        )
+      }
+
+      const first = makeWebHandler("first")
+      const second = makeWebHandler("second")
+
+      try {
+        const firstResponse = await first.handler(
+          new Request("http://localhost/a", {
+            headers: {
+              authorization: "token"
+            }
+          })
+        )
+        assert.strictEqual(firstResponse.status, 200)
+        assert.deepStrictEqual(await firstResponse.json(), { marker: "first" })
+
+        const secondResponse = await second.handler(
+          new Request("http://localhost/a", {
+            headers: {
+              authorization: "token"
+            }
+          })
+        )
+        assert.strictEqual(secondResponse.status, 200)
+        assert.deepStrictEqual(await secondResponse.json(), { marker: "second" })
+      } finally {
+        await first.dispose()
+        await second.dispose()
+      }
+    })
+
     it.effect("addHttpApi + middleware works across merged groups", () => {
       class M1 extends HttpApiMiddleware.Service<M1>()("Http/M1") {}
       class M2 extends HttpApiMiddleware.Service<M2>()("Http/M2") {}