History

Update OAuth refresh reauth assertion

Rhys Sullivan committed 6 months ago

Commit 418b0961 parent 70d01de1 file · +173 −193Browse files

Unified · Split

packages/plugins/openapi/src/sdk/oauth-refresh.test.ts+173−193
@@ -13,14 +13,19 @@ · 12 unmodified lines
1313 // ---------------------------------------------------------------------------
1414
1515 import { afterEach, expect, layer } from "@effect/vitest";
16−import { Effect, Layer, Schema } from "effect";
17−import { HttpApi, HttpApiBuilder, HttpApiEndpoint, HttpApiGroup, OpenApi } from "effect/unstable/httpapi";
16+import { Effect, Layer, Predicate, Schema } from "effect";
17+import {
18+ HttpApi,
19+ HttpApiBuilder,
20+ HttpApiEndpoint,
21+ HttpApiGroup,
22+ OpenApi,
23+} from "effect/unstable/httpapi";
1824 import { FetchHttpClient, HttpRouter, HttpServer, HttpServerRequest } from "effect/unstable/http";
1925 import * as NodeHttpServer from "@effect/platform-node/NodeHttpServer";
2026
2127 import {
2228 ConnectionId,
23− ConnectionReauthRequiredError,
2429 CreateConnectionInput,
2530 ScopeId,
2631 SecretId,
@@ -90,23 +95,26 @@ const mockTokenFetch = ( · 63 unmodified lines
9095 handler: (body: URLSearchParams) => Effect.Effect<Response, never, never> | Promise<Response>,
9196 ) => {
9297 const calls: TokenCall[] = [];
93− globalThis.fetch = Object.assign(async (_input: RequestInfo | URL, init?: RequestInit) => {
94− const url = typeof _input === "string" ? _input : _input.toString();
95− if (!url.includes("token.example.com")) {
96− return originalFetch(_input, init);
97− }
98− const bodyText =
99− init?.body instanceof URLSearchParams
100− ? init.body.toString()
101− : typeof init?.body === "string"
102− ? init.body
103− : "";
104− const body = new URLSearchParams(bodyText);
105− calls.push({ body });
106− const out = handler(body);
107− if (Effect.isEffect(out)) return await Effect.runPromise(out);
108− return await out;
109− }, { preconnect: originalFetch.preconnect });
98+ globalThis.fetch = Object.assign(
99+ async (_input: RequestInfo | URL, init?: RequestInit) => {
100+ const url = typeof _input === "string" ? _input : _input.toString();
101+ if (!url.includes("token.example.com")) {
102+ return originalFetch(_input, init);
103+ }
104+ const bodyText =
105+ init?.body instanceof URLSearchParams
106+ ? init.body.toString()
107+ : typeof init?.body === "string"
108+ ? init.body
109+ : "";
110+ const body = new URLSearchParams(bodyText);
111+ calls.push({ body });
112+ const out = handler(body);
113+ if (Effect.isEffect(out)) return await Effect.runPromise(out);
114+ return await out;
115+ },
116+ { preconnect: originalFetch.preconnect },
117+ );
110118 return { calls };
111119 };
112120
@@ -127,25 +135,26 @@ const makeExecutor = () => · 14 unmodified lines
127135 const memoryProvider: SecretProvider = {
128136 key: "memory",
129137 writable: true,
130− get: (id, scope) =>
131− Effect.sync(() => secretStore.get(keyOf(scope, id)) ?? null),
138+ get: (id, scope) => Effect.sync(() => secretStore.get(keyOf(scope, id)) ?? null),
132139 set: (id, value, scope) =>
133140 Effect.sync(() => {
134141 secretStore.set(keyOf(scope, id), value);
135142 }),
136− delete: (id, scope) =>
137− Effect.sync(() => secretStore.delete(keyOf(scope, id))),
143+ delete: (id, scope) => Effect.sync(() => secretStore.delete(keyOf(scope, id))),
138144 };
139145 const memorySecretsPlugin = definePlugin(() => ({
140146 id: "memory-secrets" as const,
141147 storage: () => ({}),
142148 secretProviders: [memoryProvider],
143149 }));
144− const clientLayer = FetchHttpClient.layer;
145− const server = yield* HttpServer.HttpServer;
146− const address = server.address;
147− if (address._tag !== "TcpAddress") return yield* Effect.die("test server must bind to TCP");
148− const baseUrl = `http://127.0.0.1:${address.port}`;
150+ const clientLayer = FetchHttpClient.layer;
151+ const server = yield* HttpServer.HttpServer;
152+ const address = server.address;
153+ if (!Predicate.isTagged("TcpAddress")(address)) {
154+ // oxlint-disable-next-line executor/no-effect-escape-hatch -- boundary: test harness cannot continue without a TCP test server address
155+ return yield* Effect.die("test server must bind to TCP");
156+ }
157+ const baseUrl = `http://127.0.0.1:${address.port}`;
149158 const plugins = [
150159 openApiPlugin({ httpClientLayer: clientLayer }),
151160 memorySecretsPlugin(),
@@ -198,11 +207,7 @@ type ExecutorValue = EffectSuccess<ReturnType<typeof makeExecutor>>["executor"]; · 46 unmodified lines
198207 // Seed an authorizationCode Connection with an already-expired access
199208 // token and a stored refresh token. The test's mock token endpoint
200209 // decides what comes back on `grant_type=refresh_token`.
201−const seedExpiredConnection = (
202− executor: ExecutorValue,
203− scopeId: ScopeId,
204− connectionId: string,
205−) =>
210+const seedExpiredConnection = (executor: ExecutorValue, scopeId: ScopeId, connectionId: string) =>
206211 Effect.gen(function* () {
207212 yield* executor.connections.create(
208213 new CreateConnectionInput({
@@ -249,169 +254,144 @@ const seedExpiredConnection = ( · 40 unmodified lines
249254 // ---------------------------------------------------------------------------
250255
251256 layer(TestLayer)("OpenAPI oauth refresh", (it) => {
252− it.effect(
253− "expired access_token is refreshed via grant_type=refresh_token before invoke",
254− () =>
255− Effect.gen(function* () {
256− const { executor, scopeId, baseUrl } = yield* makeExecutor();
257− const { calls } = mockTokenFetch(
258− () =>
259− Effect.succeed(
260− new Response(
261− JSON.stringify({
262− access_token: "fresh-access-v2",
263− token_type: "Bearer",
264− refresh_token: "refresh-v2",
265− expires_in: 3600,
266− }),
267− { status: 200, headers: { "content-type": "application/json" } },
268− ),
269− ),
270− );
271−
272− const auth = yield* seedExpiredConnection(
273− executor,
274− scopeId,
275− "conn-refresh-ok",
276− );
277−
278− yield* executor.openapi.addSpec({
279− spec: specJson,
280− scope: String(scopeId),
281− namespace: "petstore",
282− baseUrl,
283− oauth2: auth,
284− });
285−
286− const result = (yield* executor.tools.invoke(
287− "petstore.items.echoHeaders",
288− {},
289− autoApprove,
290− )) as { data: { authorization?: string } | null; error: unknown };
291−
292− expect(result.error).toBeNull();
293− // Proves the refresh landed: invoke carried the fresh token,
294− // not the expired one we seeded.
295− expect(result.data?.authorization).toBe("Bearer fresh-access-v2");
296− expect(calls).toHaveLength(1);
297− expect(calls[0]!.body.get("grant_type")).toBe("refresh_token");
298− expect(calls[0]!.body.get("refresh_token")).toBe("refresh-v1");
299−
300− // Connection row is patched with the new expiry so the next
301− // invoke in-window doesn't trip a second refresh.
302− const conn = yield* executor.connections.get("conn-refresh-ok");
303− expect(conn).not.toBeNull();
304− expect(conn!.expiresAt).not.toBeNull();
305− expect(conn!.expiresAt!).toBeGreaterThan(Date.now() + 3_000_000);
306− }),
257+ it.effect("expired access_token is refreshed via grant_type=refresh_token before invoke", () =>
258+ Effect.gen(function* () {
259+ const { executor, scopeId, baseUrl } = yield* makeExecutor();
260+ const { calls } = mockTokenFetch(() =>
261+ Effect.succeed(
262+ new Response(
263+ JSON.stringify({
264+ access_token: "fresh-access-v2",
265+ token_type: "Bearer",
266+ refresh_token: "refresh-v2",
267+ expires_in: 3600,
268+ }),
269+ { status: 200, headers: { "content-type": "application/json" } },
270+ ),
271+ ),
272+ );
273+
274+ const auth = yield* seedExpiredConnection(executor, scopeId, "conn-refresh-ok");
275+
276+ yield* executor.openapi.addSpec({
277+ spec: specJson,
278+ scope: String(scopeId),
279+ namespace: "petstore",
280+ baseUrl,
281+ oauth2: auth,
282+ });
283+
284+ const result = (yield* executor.tools.invoke(
285+ "petstore.items.echoHeaders",
286+ {},
287+ autoApprove,
288+ )) as { data: { authorization?: string } | null; error: unknown };
289+
290+ expect(result.error).toBeNull();
291+ // Proves the refresh landed: invoke carried the fresh token,
292+ // not the expired one we seeded.
293+ expect(result.data?.authorization).toBe("Bearer fresh-access-v2");
294+ expect(calls).toHaveLength(1);
295+ expect(calls[0]!.body.get("grant_type")).toBe("refresh_token");
296+ expect(calls[0]!.body.get("refresh_token")).toBe("refresh-v1");
297+
298+ // Connection row is patched with the new expiry so the next
299+ // invoke in-window doesn't trip a second refresh.
300+ const conn = yield* executor.connections.get("conn-refresh-ok");
301+ expect(conn).not.toBeNull();
302+ expect(conn!.expiresAt).not.toBeNull();
303+ expect(conn!.expiresAt!).toBeGreaterThan(Date.now() + 3_000_000);
304+ }),
307305 );
308306
309− it.effect(
310− "concurrent invokes with an expired token issue exactly one refresh",
311− () =>
312− Effect.gen(function* () {
313− const { executor, scopeId, baseUrl } = yield* makeExecutor();
314− const { calls } = mockTokenFetch(
315− () =>
316− Effect.succeed(
317− new Response(
318− JSON.stringify({
319− access_token: "fresh-access-v2",
320− token_type: "Bearer",
321− refresh_token: "refresh-v2",
322− expires_in: 3600,
323− }),
324− { status: 200, headers: { "content-type": "application/json" } },
325− ),
326− ),
327− );
328−
329− const auth = yield* seedExpiredConnection(
330− executor,
331− scopeId,
332− "conn-refresh-concurrent",
333− );
334−
335− yield* executor.openapi.addSpec({
336− spec: specJson,
337− scope: String(scopeId),
338− namespace: "petstore",
339− baseUrl,
340− oauth2: auth,
341− });
342−
343− const invokes = yield* Effect.all(
344− [1, 2, 3, 4, 5].map(() =>
345− executor.tools.invoke(
346− "petstore.items.echoHeaders",
347− {},
348− autoApprove,
349− ),
307+ it.effect("concurrent invokes with an expired token issue exactly one refresh", () =>
308+ Effect.gen(function* () {
309+ const { executor, scopeId, baseUrl } = yield* makeExecutor();
310+ const { calls } = mockTokenFetch(() =>
311+ Effect.succeed(
312+ new Response(
313+ JSON.stringify({
314+ access_token: "fresh-access-v2",
315+ token_type: "Bearer",
316+ refresh_token: "refresh-v2",
317+ expires_in: 3600,
318+ }),
319+ { status: 200, headers: { "content-type": "application/json" } },
350320 ),
351− { concurrency: "unbounded" },
352− );
353−
354− for (const r of invokes) {
355− const res = r as {
356− data: { authorization?: string } | null;
357− error: unknown;
358− };
359− expect(res.error).toBeNull();
360− expect(res.data?.authorization).toBe("Bearer fresh-access-v2");
361− }
362− // Critical assertion: the SDK's dedup collapses every parallel
363− // invoke into one call to the token endpoint. Anything more
364− // means we're hammering the AS under load.
365− expect(calls).toHaveLength(1);
366− }),
321+ ),
322+ );
323+
324+ const auth = yield* seedExpiredConnection(executor, scopeId, "conn-refresh-concurrent");
325+
326+ yield* executor.openapi.addSpec({
327+ spec: specJson,
328+ scope: String(scopeId),
329+ namespace: "petstore",
330+ baseUrl,
331+ oauth2: auth,
332+ });
333+
334+ const invokes = yield* Effect.all(
335+ [1, 2, 3, 4, 5].map(() =>
336+ executor.tools.invoke("petstore.items.echoHeaders", {}, autoApprove),
337+ ),
338+ { concurrency: "unbounded" },
339+ );
340+
341+ for (const r of invokes) {
342+ const res = r as {
343+ data: { authorization?: string } | null;
344+ error: unknown;
345+ };
346+ expect(res.error).toBeNull();
347+ expect(res.data?.authorization).toBe("Bearer fresh-access-v2");
348+ }
349+ // Critical assertion: the SDK's dedup collapses every parallel
350+ // invoke into one call to the token endpoint. Anything more
351+ // means we're hammering the AS under load.
352+ expect(calls).toHaveLength(1);
353+ }),
367354 );
368355
369− it.effect(
370− "invalid_grant from refresh surfaces as ConnectionReauthRequiredError",
371− () =>
372− Effect.gen(function* () {
373− const { executor, scopeId, baseUrl } = yield* makeExecutor();
374− mockTokenFetch(
375− () =>
376− Effect.succeed(
377− new Response(
378− JSON.stringify({
379− error: "invalid_grant",
380− error_description: "Refresh token revoked",
381− }),
382− { status: 400, headers: { "content-type": "application/json" } },
383− ),
384− ),
385− );
386−
387− const auth = yield* seedExpiredConnection(
388− executor,
389− scopeId,
390− "conn-refresh-dead",
391− );
392−
393− yield* executor.openapi.addSpec({
394− spec: specJson,
395− scope: String(scopeId),
396− namespace: "petstore",
397− baseUrl,
398− oauth2: auth,
399− });
400−
401− // Tool invocation currently wraps connection errors in a
402− // generic Error (see openapi invokeTool), so we assert against
403− // the `accessToken` call directly too — that's the surface
404− // the UI bridges use to trigger re-auth.
405− const flipped = yield* executor.connections
406− .accessToken("conn-refresh-dead")
407− .pipe(Effect.flip);
408− expect(flipped._tag).toBe("ConnectionReauthRequiredError");
409− expect((flipped as ConnectionReauthRequiredError).provider).toBe(
410− "openapi:oauth2",
411− );
412− expect(
413− (flipped as ConnectionReauthRequiredError).message,
414− ).toMatch(/invalid_grant|revoked/i);
415− }),
356+ it.effect("invalid_grant from refresh surfaces as ConnectionReauthRequiredError", () =>
357+ Effect.gen(function* () {
358+ const { executor, scopeId, baseUrl } = yield* makeExecutor();
359+ mockTokenFetch(() =>
360+ Effect.succeed(
361+ new Response(
362+ JSON.stringify({
363+ error: "invalid_grant",
364+ error_description: "Refresh token revoked",
365+ }),
366+ { status: 400, headers: { "content-type": "application/json" } },
367+ ),
368+ ),
369+ );
370+
371+ const auth = yield* seedExpiredConnection(executor, scopeId, "conn-refresh-dead");
372+
373+ yield* executor.openapi.addSpec({
374+ spec: specJson,
375+ scope: String(scopeId),
376+ namespace: "petstore",
377+ baseUrl,
378+ oauth2: auth,
379+ });
380+
381+ // Tool invocation currently wraps connection errors in a
382+ // generic Error (see openapi invokeTool), so we assert against
383+ // the `accessToken` call directly too — that's the surface
384+ // the UI bridges use to trigger re-auth.
385+ const flipped = yield* executor.connections.accessToken("conn-refresh-dead").pipe(
386+ Effect.flip,
387+ Effect.flatMap((error) =>
388+ Predicate.isTagged("ConnectionReauthRequiredError")(error)
389+ ? Effect.succeed(error)
390+ : Effect.fail(error),
391+ ),
392+ );
393+ expect(flipped.provider).toBe("openapi:oauth2");
394+ expect(flipped.message).toBe("OAuth refresh failed");
395+ }),
416396 );
417397 });