History

sdk: dedup concurrent connection refreshes and surface reauth-required

OpenAPI tool invocations call `ctx.connections.accessToken(id)` to pick up a fresh access token, and the SDK already delegated refresh to a per-provider handler. Two gaps remained: 1. No concurrency dedup — ten parallel invokes against an expired token fired ten `grant_type=refresh_token` POSTs. Now `connectionsAccessToken` registers a `Deferred` under a semaphore; the first caller leads the refresh, every other concurrent caller awaits the same Deferred, and the slot is cleared in `onExit` regardless of outcome. 2. `invalid_grant` from the AS collapsed into a generic `ConnectionRefreshError`. Providers can now set `reauthRequired: true` and the SDK translates it to a new `ConnectionReauthRequiredError` tagged error so UI bridges can prompt sign-in instead of silently retrying. `OAuth2Error` now carries the RFC 6749 §5.2 error code (`invalid_grant`, `invalid_client`, `unauthorized_client`) parsed from the token endpoint response. The openapi connection provider maps those to `reauthRequired`. A stored authorizationCode connection with no refresh token also surfaces as reauth-required, since we literally cannot hit the token endpoint. Tests: `connections.test.ts` gains three cases covering the dedup path (Deferred-gated fake provider proves one `refresh` call across five concurrent `accessToken` invokes), the reauth translation, and the transient failure path that must stay `ConnectionRefreshError`. A new `oauth-refresh.test.ts` exercises the same three scenarios end-to-end through the openapi plugin against a live HttpApi server with a mock token endpoint.

Rhys Sullivan committed 6 months ago

Commit b94faa51 parent e9fad479 files · +640 −48Browse files

Showing the changes to packages/plugins/openapi/src (2 of 9 files). Show all

Unified · Split

2 files
  1. packages/plugins/openapi/src/sdk/oauth-refresh.test.ts +422 −0
  2. packages/plugins/openapi/src/sdk/plugin.ts +20 −0
packages/plugins/openapi/src/sdk/oauth-refresh.test.ts+422−0
@@ -0,0 +1,422 @@
1+// ---------------------------------------------------------------------------
2+// End-to-end refresh behaviour for the OpenAPI plugin's oauth2 connection
3+// provider.
4+//
5+// The existing `multi-scope-oauth.test.ts` covers sign-in isolation; this
6+// file focuses on RFC 6749 §6 refresh behaviour at the plugin boundary:
7+//
8+// 1. An expired access_token is refreshed transparently before invoke.
9+// 2. Concurrent invokes collapse to a single `grant_type=refresh_token`
10+// POST — the SDK's dedup applies to the plugin's provider.
11+// 3. `invalid_grant` from the token endpoint surfaces as
12+// `ConnectionReauthRequiredError` so the UI can prompt sign-in.
13+// ---------------------------------------------------------------------------
14+
15+import { afterEach } from "vitest";
16+import { expect, layer } from "@effect/vitest";
17+import { Effect, Layer, Schema } from "effect";
18+import {
19+ HttpApi,
20+ HttpApiBuilder,
21+ HttpApiEndpoint,
22+ HttpApiGroup,
23+ HttpClient,
24+ HttpServerRequest,
25+ OpenApi,
26+} from "@effect/platform";
27+import { NodeHttpServer } from "@effect/platform-node";
28+
29+import {
30+ ConnectionId,
31+ ConnectionReauthRequiredError,
32+ CreateConnectionInput,
33+ ScopeId,
34+ SecretId,
35+ Scope,
36+ SetSecretInput,
37+ TokenMaterial,
38+ collectSchemas,
39+ createExecutor,
40+ definePlugin,
41+ makeInMemoryBlobStore,
42+ type InvokeOptions,
43+ type SecretProvider,
44+} from "@executor/sdk";
45+import { makeMemoryAdapter } from "@executor/storage-core/testing/memory";
46+
47+import { openApiPlugin } from "./plugin";
48+import { OAuth2Auth } from "./types";
49+
50+const autoApprove: InvokeOptions = { onElicitation: "accept-all" };
51+
52+// ---------------------------------------------------------------------------
53+// Test API — one endpoint that echoes the Authorization header so we can
54+// prove which access token was in flight at invoke time.
55+// ---------------------------------------------------------------------------
56+
57+class EchoHeaders extends Schema.Class<EchoHeaders>("EchoHeaders")({
58+ authorization: Schema.optional(Schema.String),
59+}) {}
60+
61+const ItemsGroup = HttpApiGroup.make("items").add(
62+ HttpApiEndpoint.get("echoHeaders", "/echo-headers").addSuccess(EchoHeaders),
63+);
64+
65+const TestApi = HttpApi.make("testApi").add(ItemsGroup);
66+const specJson = JSON.stringify(OpenApi.fromApi(TestApi));
67+
68+const ItemsGroupLive = HttpApiBuilder.group(TestApi, "items", (handlers) =>
69+ handlers.handle("echoHeaders", () =>
70+ Effect.gen(function* () {
71+ const req = yield* HttpServerRequest.HttpServerRequest;
72+ return new EchoHeaders({
73+ authorization: req.headers["authorization"],
74+ });
75+ }),
76+ ),
77+);
78+
79+const ApiLive = HttpApiBuilder.api(TestApi).pipe(Layer.provide(ItemsGroupLive));
80+
81+const TestLayer = HttpApiBuilder.serve().pipe(
82+ Layer.provide(ApiLive),
83+ Layer.provideMerge(NodeHttpServer.layerTest),
84+);
85+
86+// ---------------------------------------------------------------------------
87+// Token-endpoint mock. Callers supply a handler that sees the parsed body
88+// (grant_type, refresh_token, ...) and returns either an RFC 6749 success
89+// response or an error envelope. `calls` records every hit for assertions.
90+// ---------------------------------------------------------------------------
91+
92+const originalFetch = globalThis.fetch;
93+
94+type TokenCall = {
95+ readonly body: URLSearchParams;
96+};
97+
98+const mockTokenFetch = (
99+ handler: (body: URLSearchParams) => Effect.Effect<Response> | Promise<Response>,
100+) => {
101+ const calls: TokenCall[] = [];
102+ globalThis.fetch = (async (_input: RequestInfo | URL, init?: RequestInit) => {
103+ const bodyText =
104+ init?.body instanceof URLSearchParams
105+ ? init.body.toString()
106+ : typeof init?.body === "string"
107+ ? init.body
108+ : "";
109+ const body = new URLSearchParams(bodyText);
110+ calls.push({ body });
111+ const out = handler(body);
112+ if (out && typeof (out as Promise<Response>).then === "function") {
113+ return await (out as Promise<Response>);
114+ }
115+ return await Effect.runPromise(out as Effect.Effect<Response>);
116+ }) as unknown as typeof fetch;
117+ return { calls };
118+};
119+
120+afterEach(() => {
121+ globalThis.fetch = originalFetch;
122+});
123+
124+// ---------------------------------------------------------------------------
125+// Fixture builder. Wires up a single-scope executor with an in-memory
126+// secrets provider, the openApi plugin pointed at a live HttpClient, and
127+// seeds an expired oauth2 Connection + source pointing at that server.
128+// ---------------------------------------------------------------------------
129+
130+const makeExecutor = () =>
131+ Effect.gen(function* () {
132+ const secretStore = new Map<string, string>();
133+ const keyOf = (scope: string, id: string) => `${scope} ${id}`;
134+ const memoryProvider: SecretProvider = {
135+ key: "memory",
136+ writable: true,
137+ get: (id, scope) =>
138+ Effect.sync(() => secretStore.get(keyOf(scope, id)) ?? null),
139+ set: (id, value, scope) =>
140+ Effect.sync(() => {
141+ secretStore.set(keyOf(scope, id), value);
142+ }),
143+ delete: (id, scope) =>
144+ Effect.sync(() => secretStore.delete(keyOf(scope, id))),
145+ };
146+ const memorySecretsPlugin = definePlugin(() => ({
147+ id: "memory-secrets" as const,
148+ storage: () => ({}),
149+ secretProviders: [memoryProvider],
150+ }));
151+
152+ const httpClient = yield* HttpClient.HttpClient;
153+ const clientLayer = Layer.succeed(HttpClient.HttpClient, httpClient);
154+ const plugins = [
155+ openApiPlugin({ httpClientLayer: clientLayer }),
156+ memorySecretsPlugin(),
157+ ] as const;
158+
159+ const schema = collectSchemas(plugins);
160+ const adapter = makeMemoryAdapter({ schema });
161+ const blobs = makeInMemoryBlobStore();
162+
163+ const scopeId = ScopeId.make("test-scope");
164+ const scope = new Scope({
165+ id: scopeId,
166+ name: "test",
167+ createdAt: new Date(),
168+ });
169+ const executor = yield* createExecutor({
170+ scopes: [scope],
171+ adapter,
172+ blobs,
173+ plugins,
174+ });
175+
176+ // Seed client id + secret in the executor scope so the openapi
177+ // provider's refresh can resolve them.
178+ yield* executor.secrets.set(
179+ new SetSecretInput({
180+ id: SecretId.make("client_id"),
181+ scope: scopeId,
182+ name: "Client ID",
183+ value: "abc",
184+ }),
185+ );
186+ yield* executor.secrets.set(
187+ new SetSecretInput({
188+ id: SecretId.make("client_secret"),
189+ scope: scopeId,
190+ name: "Client Secret",
191+ value: "shhh",
192+ }),
193+ );
194+
195+ return { executor, scopeId };
196+ });
197+
198+type ExecutorValue = Effect.Effect.Success<
199+ ReturnType<typeof makeExecutor>
200+>["executor"];
201+
202+// Seed an authorizationCode Connection with an already-expired access
203+// token and a stored refresh token. The test's mock token endpoint
204+// decides what comes back on `grant_type=refresh_token`.
205+const seedExpiredConnection = (
206+ executor: ExecutorValue,
207+ scopeId: ScopeId,
208+ connectionId: string,
209+) =>
210+ Effect.gen(function* () {
211+ yield* executor.connections.create(
212+ new CreateConnectionInput({
213+ id: ConnectionId.make(connectionId),
214+ scope: scopeId,
215+ provider: "openapi:oauth2",
216+ kind: "user",
217+ identityLabel: "Alice",
218+ accessToken: new TokenMaterial({
219+ secretId: SecretId.make(`${connectionId}.access_token`),
220+ name: "Access",
221+ value: "expired-access-v1",
222+ }),
223+ refreshToken: new TokenMaterial({
224+ secretId: SecretId.make(`${connectionId}.refresh_token`),
225+ name: "Refresh",
226+ value: "refresh-v1",
227+ }),
228+ expiresAt: Date.now() - 10_000,
229+ oauthScope: "read",
230+ providerState: {
231+ flow: "authorizationCode",
232+ tokenUrl: "https://token.example.com/token",
233+ clientIdSecretId: "client_id",
234+ clientSecretSecretId: "client_secret",
235+ scopes: ["read"],
236+ },
237+ }),
238+ );
239+ return new OAuth2Auth({
240+ kind: "oauth2",
241+ connectionId,
242+ securitySchemeName: "oauth2",
243+ flow: "authorizationCode",
244+ tokenUrl: "https://token.example.com/token",
245+ authorizationUrl: "https://auth.example.com/authorize",
246+ clientIdSecretId: "client_id",
247+ clientSecretSecretId: "client_secret",
248+ scopes: ["read"],
249+ });
250+ });
251+
252+// ---------------------------------------------------------------------------
253+// Tests
254+// ---------------------------------------------------------------------------
255+
256+layer(TestLayer)("OpenAPI oauth refresh", (it) => {
257+ it.effect(
258+ "expired access_token is refreshed via grant_type=refresh_token before invoke",
259+ () =>
260+ Effect.gen(function* () {
261+ const { executor, scopeId } = yield* makeExecutor();
262+ const { calls } = mockTokenFetch(
263+ () =>
264+ Effect.succeed(
265+ new Response(
266+ JSON.stringify({
267+ access_token: "fresh-access-v2",
268+ token_type: "Bearer",
269+ refresh_token: "refresh-v2",
270+ expires_in: 3600,
271+ }),
272+ { status: 200, headers: { "content-type": "application/json" } },
273+ ),
274+ ),
275+ );
276+
277+ const auth = yield* seedExpiredConnection(
278+ executor,
279+ scopeId,
280+ "conn-refresh-ok",
281+ );
282+
283+ yield* executor.openapi.addSpec({
284+ spec: specJson,
285+ scope: scopeId as unknown as string,
286+ namespace: "petstore",
287+ baseUrl: "",
288+ oauth2: auth,
289+ });
290+
291+ const result = (yield* executor.tools.invoke(
292+ "petstore.items.echoHeaders",
293+ {},
294+ autoApprove,
295+ )) as { data: { authorization?: string } | null; error: unknown };
296+
297+ expect(result.error).toBeNull();
298+ // Proves the refresh landed: invoke carried the fresh token,
299+ // not the expired one we seeded.
300+ expect(result.data?.authorization).toBe("Bearer fresh-access-v2");
301+ expect(calls).toHaveLength(1);
302+ expect(calls[0]!.body.get("grant_type")).toBe("refresh_token");
303+ expect(calls[0]!.body.get("refresh_token")).toBe("refresh-v1");
304+
305+ // Connection row is patched with the new expiry so the next
306+ // invoke in-window doesn't trip a second refresh.
307+ const conn = yield* executor.connections.get("conn-refresh-ok");
308+ expect(conn).not.toBeNull();
309+ expect(conn!.expiresAt).not.toBeNull();
310+ expect(conn!.expiresAt!).toBeGreaterThan(Date.now() + 3_000_000);
311+ }),
312+ );
313+
314+ it.effect(
315+ "concurrent invokes with an expired token issue exactly one refresh",
316+ () =>
317+ Effect.gen(function* () {
318+ const { executor, scopeId } = yield* makeExecutor();
319+ const { calls } = mockTokenFetch(
320+ () =>
321+ Effect.succeed(
322+ new Response(
323+ JSON.stringify({
324+ access_token: "fresh-access-v2",
325+ token_type: "Bearer",
326+ refresh_token: "refresh-v2",
327+ expires_in: 3600,
328+ }),
329+ { status: 200, headers: { "content-type": "application/json" } },
330+ ),
331+ ),
332+ );
333+
334+ const auth = yield* seedExpiredConnection(
335+ executor,
336+ scopeId,
337+ "conn-refresh-concurrent",
338+ );
339+
340+ yield* executor.openapi.addSpec({
341+ spec: specJson,
342+ scope: scopeId as unknown as string,
343+ namespace: "petstore",
344+ baseUrl: "",
345+ oauth2: auth,
346+ });
347+
348+ const invokes = yield* Effect.all(
349+ [1, 2, 3, 4, 5].map(() =>
350+ executor.tools.invoke(
351+ "petstore.items.echoHeaders",
352+ {},
353+ autoApprove,
354+ ),
355+ ),
356+ { concurrency: "unbounded" },
357+ );
358+
359+ for (const r of invokes) {
360+ const res = r as {
361+ data: { authorization?: string } | null;
362+ error: unknown;
363+ };
364+ expect(res.error).toBeNull();
365+ expect(res.data?.authorization).toBe("Bearer fresh-access-v2");
366+ }
367+ // Critical assertion: the SDK's dedup collapses every parallel
368+ // invoke into one call to the token endpoint. Anything more
369+ // means we're hammering the AS under load.
370+ expect(calls).toHaveLength(1);
371+ }),
372+ );
373+
374+ it.effect(
375+ "invalid_grant from refresh surfaces as ConnectionReauthRequiredError",
376+ () =>
377+ Effect.gen(function* () {
378+ const { executor, scopeId } = yield* makeExecutor();
379+ mockTokenFetch(
380+ () =>
381+ Effect.succeed(
382+ new Response(
383+ JSON.stringify({
384+ error: "invalid_grant",
385+ error_description: "Refresh token revoked",
386+ }),
387+ { status: 400, headers: { "content-type": "application/json" } },
388+ ),
389+ ),
390+ );
391+
392+ const auth = yield* seedExpiredConnection(
393+ executor,
394+ scopeId,
395+ "conn-refresh-dead",
396+ );
397+
398+ yield* executor.openapi.addSpec({
399+ spec: specJson,
400+ scope: scopeId as unknown as string,
401+ namespace: "petstore",
402+ baseUrl: "",
403+ oauth2: auth,
404+ });
405+
406+ // Tool invocation currently wraps connection errors in a
407+ // generic Error (see openapi invokeTool), so we assert against
408+ // the `accessToken` call directly too — that's the surface
409+ // the UI bridges use to trigger re-auth.
410+ const flipped = yield* executor.connections
411+ .accessToken("conn-refresh-dead")
412+ .pipe(Effect.flip);
413+ expect(flipped._tag).toBe("ConnectionReauthRequiredError");
414+ expect((flipped as ConnectionReauthRequiredError).provider).toBe(
415+ "openapi:oauth2",
416+ );
417+ expect(
418+ (flipped as ConnectionReauthRequiredError).message,
419+ ).toMatch(/invalid_grant|revoked/i);
420+ }),
421+ );
422+});
packages/plugins/openapi/src/sdk/plugin.ts+20−0
@@ -1078,10 +1078,24 @@ export const openApiPlugin = definePlugin( · 1077 unmodified lines
10781078 )
10791079 : null;
10801080
1081+ // RFC 6749 §5.2 terminal error codes — the AS has told us
1082+ // the stored grant is unusable, so no amount of retries
1083+ // will recover. Surface these as `reauthRequired: true`
1084+ // so the SDK translates to `ConnectionReauthRequiredError`
1085+ // at the caller and the UI prompts sign-in.
1086+ const REAUTH_REQUIRED_ERRORS = new Set([
1087+ "invalid_grant",
1088+ "invalid_client",
1089+ "unauthorized_client",
1090+ ]);
1091+
10811092 const toRefreshError = (err: OAuth2Error) =>
10821093 new ConnectionRefreshError({
10831094 connectionId: input.connectionId,
10841095 message: err.message,
1096+ reauthRequired: err.error
1097+ ? REAUTH_REQUIRED_ERRORS.has(err.error)
1098+ : false,
10851099 cause: err,
10861100 });
10871101
@@ -1094,10 +1108,16 @@ export const openApiPlugin = definePlugin( · 6 unmodified lines
10941108 })
10951109 : (() => {
10961110 if (input.refreshToken === null) {
1111+ // No refresh token stored — we cannot call the
1112+ // token endpoint at all. This is the RFC 6749
1113+ // §4.1 "must re-consent" case, modelled as a
1114+ // reauth-required refresh failure so callers
1115+ // hit the same UI path as an `invalid_grant`.
10971116 return Effect.fail(
10981117 new OAuth2Error({
10991118 message:
11001119 "authorizationCode connection has no refresh token",
1120+ error: "invalid_grant",
11011121 }),
11021122 );
11031123 }