Drives a real authorization-code flow against the test AS with long-lived tokens, so the proactive expiry check never fires and the upstream's 401 is the only thing that can trigger a refresh. The upstream then revokes the bearer it was issued; the next call must still succeed. Asserted from the upstream's own request ledger: three calls, the second carrying the revoked token and the third a different one the AS confirms it issued, plus exactly one refresh grant redeemed. Verified fail-before/pass-after: with the fix reverted the scenario fails with connection_rejected / HTTP 401. Also corrects the refresh span comment, which claimed to stamp the AS error code while the attribute stamps the failure kind.
Unified · Split
| @@ -0,0 +1,326 @@ | |||
| 1 | // Cross-target: when the upstream rejects a token executor still believes is | ||
| 2 | // valid, executor re-mints it and retries — the caller never sees the 401. | ||
| 3 | // | ||
| 4 | // `expires_at` only ever holds the authorization server's ADVERTISED lifetime. | ||
| 5 | // A token can stop working well before that: server-side revocation, an | ||
| 6 | // identity provider idle-timeout policy shorter than the token lifetime, or an | ||
| 7 | // AS that never advertised an expiry at all. Before the reactive path, the | ||
| 8 | // proactive expiry check was the only thing that triggered a refresh, so every | ||
| 9 | // one of those cases surfaced to the user as "re-authenticate" even though the | ||
| 10 | // stored refresh token would have worked. | ||
| 11 | // | ||
| 12 | // The journey: an OpenAPI integration completes a real authorization-code flow | ||
| 13 | // against a live test AS that issues LONG-LIVED tokens (so the proactive check | ||
| 14 | // never fires). The upstream then revokes the bearer it was given. The next | ||
| 15 | // tool call gets a 401, executor re-mints against the AS, retries with the new | ||
| 16 | // token, and the call succeeds — proven from the upstream's own request ledger, | ||
| 17 | // which records both the rejected bearer and the accepted one. | ||
| 18 | import { randomBytes } from "node:crypto"; | ||
| 19 | import { createServer } from "node:http"; | ||
| 20 | |||
| 21 | import { expect } from "@effect/vitest"; | ||
| 22 | import { Effect } from "effect"; | ||
| 23 | import { composePluginApi } from "@executor-js/api/server"; | ||
| 24 | import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; | ||
| 25 | import { | ||
| 26 | AuthTemplateSlug, | ||
| 27 | ConnectionName, | ||
| 28 | IntegrationSlug, | ||
| 29 | OAuthClientSlug, | ||
| 30 | } from "@executor-js/sdk/shared"; | ||
| 31 | import { serveOAuthTestServer } from "@executor-js/sdk/testing"; | ||
| 32 | |||
| 33 | import { scenario } from "../src/scenario"; | ||
| 34 | import { Api, Mcp, Target } from "../src/services"; | ||
| 35 | |||
| 36 | const api = composePluginApi([openApiHttpPlugin()] as const); | ||
| 37 | |||
| 38 | const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; | ||
| 39 | |||
| 40 | type UpstreamHandle = { | ||
| 41 | readonly url: string; | ||
| 42 | /** Every bearer the upstream saw, oldest first — the evidence that a retry | ||
| 43 | * happened and that it carried a DIFFERENT token than the rejected call. */ | ||
| 44 | readonly bearers: () => readonly string[]; | ||
| 45 | /** Stop honouring the bearer(s) seen so far, exactly as a revocation or an | ||
| 46 | * idle-timeout policy would. Anything minted afterwards still works. */ | ||
| 47 | readonly revokeSeenBearers: () => void; | ||
| 48 | readonly close: () => void; | ||
| 49 | }; | ||
| 50 | |||
| 51 | /** Upstream on 127.0.0.1 that authenticates for real: `GET /issues` is 200 for | ||
| 52 | * any bearer it has not been told to reject, and 401 for one it has. */ | ||
| 53 | const serveUpstream = () => | ||
| 54 | Effect.acquireRelease( | ||
| 55 | Effect.callback<UpstreamHandle>((resume) => { | ||
| 56 | const seen: string[] = []; | ||
| 57 | const revoked = new Set<string>(); | ||
| 58 | const server = createServer((request, response) => { | ||
| 59 | const bearer = (request.headers.authorization ?? "").replace(/^Bearer\s+/i, ""); | ||
| 60 | if (request.method === "GET" && (request.url ?? "").startsWith("/issues")) { | ||
| 61 | seen.push(bearer); | ||
| 62 | if (revoked.has(bearer)) { | ||
| 63 | response.writeHead(401, { "content-type": "application/json" }); | ||
| 64 | response.end(JSON.stringify({ error: "invalid_token" })); | ||
| 65 | return; | ||
| 66 | } | ||
| 67 | response.writeHead(200, { "content-type": "application/json" }); | ||
| 68 | response.end(JSON.stringify({ issues: [{ id: 1, title: "first" }] })); | ||
| 69 | return; | ||
| 70 | } | ||
| 71 | response.writeHead(404, { "content-type": "application/json" }); | ||
| 72 | response.end(JSON.stringify({ error: "not_found" })); | ||
| 73 | }); | ||
| 74 | server.listen(0, "127.0.0.1", () => { | ||
| 75 | const address = server.address(); | ||
| 76 | const port = typeof address === "object" && address ? address.port : 0; | ||
| 77 | resume( | ||
| 78 | Effect.succeed({ | ||
| 79 | url: `http://127.0.0.1:${port}`, | ||
| 80 | bearers: () => [...seen], | ||
| 81 | revokeSeenBearers: () => { | ||
| 82 | for (const bearer of seen) revoked.add(bearer); | ||
| 83 | }, | ||
| 84 | close: () => { | ||
| 85 | server.close(); | ||
| 86 | server.closeAllConnections(); | ||
| 87 | }, | ||
| 88 | }), | ||
| 89 | ); | ||
| 90 | }); | ||
| 91 | }), | ||
| 92 | (server) => Effect.sync(server.close), | ||
| 93 | ); | ||
| 94 | |||
| 95 | const spec = ( | ||
| 96 | baseUrl: string, | ||
| 97 | oauth: { readonly authorizationEndpoint: string; readonly tokenEndpoint: string }, | ||
| 98 | ): string => | ||
| 99 | JSON.stringify({ | ||
| 100 | openapi: "3.0.3", | ||
| 101 | info: { title: "Issues API", version: "1.0.0" }, | ||
| 102 | servers: [{ url: baseUrl }], | ||
| 103 | paths: { | ||
| 104 | "/issues": { | ||
| 105 | get: { | ||
| 106 | operationId: "listIssues", | ||
| 107 | summary: "List issues", | ||
| 108 | security: [{ oauth: ["issues.read"] }], | ||
| 109 | responses: { "200": { description: "issues" } }, | ||
| 110 | }, | ||
| 111 | }, | ||
| 112 | }, | ||
| 113 | components: { | ||
| 114 | securitySchemes: { | ||
| 115 | oauth: { | ||
| 116 | type: "oauth2", | ||
| 117 | flows: { | ||
| 118 | authorizationCode: { | ||
| 119 | authorizationUrl: oauth.authorizationEndpoint, | ||
| 120 | tokenUrl: oauth.tokenEndpoint, | ||
| 121 | scopes: { "issues.read": "Read issues" }, | ||
| 122 | }, | ||
| 123 | }, | ||
| 124 | }, | ||
| 125 | }, | ||
| 126 | }, | ||
| 127 | }); | ||
| 128 | |||
| 129 | const invokeByAddressCode = (address: string, args: unknown) => ` | ||
| 130 | const segments = ${JSON.stringify(address)}.split(".").slice(1); | ||
| 131 | let node = tools; | ||
| 132 | for (const segment of segments) node = node[segment]; | ||
| 133 | const result = await node(${JSON.stringify(args)}); | ||
| 134 | return JSON.stringify(result); | ||
| 135 | `; | ||
| 136 | |||
| 137 | type ToolEnvelope = { | ||
| 138 | readonly ok: boolean; | ||
| 139 | readonly data?: unknown; | ||
| 140 | readonly error?: { | ||
| 141 | readonly code?: string; | ||
| 142 | readonly message?: string; | ||
| 143 | readonly status?: number; | ||
| 144 | }; | ||
| 145 | }; | ||
| 146 | |||
| 147 | scenario( | ||
| 148 | "Auth failures · a token the upstream rejects is re-minted and the call retried, so a revoked credential recovers without a reconnect", | ||
| 149 | {}, | ||
| 150 | Effect.scoped( | ||
| 151 | Effect.gen(function* () { | ||
| 152 | const target = yield* Target; | ||
| 153 | const { client: makeClient } = yield* Api; | ||
| 154 | const mcp = yield* Mcp; | ||
| 155 | const identity = yield* target.newIdentity(); | ||
| 156 | const client = yield* makeClient(api, identity); | ||
| 157 | const upstream = yield* serveUpstream(); | ||
| 158 | // LONG-LIVED tokens are the point: the proactive expiry check must never | ||
| 159 | // fire, so the only thing that can trigger a refresh here is the | ||
| 160 | // upstream's 401. Refresh support is on, so the re-mint can succeed. | ||
| 161 | const oauth = yield* serveOAuthTestServer({ | ||
| 162 | scopes: ["issues.read"], | ||
| 163 | tokenExpiresInSeconds: 3600, | ||
| 164 | }); | ||
| 165 | const slug = unique("refresh401"); | ||
| 166 | const clientSlug = OAuthClientSlug.make(unique("refresh401c")); | ||
| 167 | |||
| 168 | yield* Effect.ensuring( | ||
| 169 | Effect.gen(function* () { | ||
| 170 | yield* client.openapi.addSpec({ | ||
| 171 | payload: { | ||
| 172 | spec: { kind: "blob", value: spec(upstream.url, oauth) }, | ||
| 173 | slug, | ||
| 174 | baseUrl: upstream.url, | ||
| 175 | authenticationTemplate: [ | ||
| 176 | { | ||
| 177 | slug: "oauth", | ||
| 178 | kind: "oauth2", | ||
| 179 | authorizationUrl: oauth.authorizationEndpoint, | ||
| 180 | tokenUrl: oauth.tokenEndpoint, | ||
| 181 | scopes: ["issues.read"], | ||
| 182 | }, | ||
| 183 | ], | ||
| 184 | }, | ||
| 185 | }); | ||
| 186 | yield* client.oauth.createClient({ | ||
| 187 | payload: { | ||
| 188 | owner: "org", | ||
| 189 | slug: clientSlug, | ||
| 190 | grant: "authorization_code", | ||
| 191 | authorizationUrl: oauth.authorizationEndpoint, | ||
| 192 | tokenUrl: oauth.tokenEndpoint, | ||
| 193 | clientId: "test-client", | ||
| 194 | clientSecret: "test-secret", | ||
| 195 | originIntegration: IntegrationSlug.make(slug), | ||
| 196 | }, | ||
| 197 | }); | ||
| 198 | |||
| 199 | const started = yield* client.oauth.start({ | ||
| 200 | payload: { | ||
| 201 | client: clientSlug, | ||
| 202 | clientOwner: "org", | ||
| 203 | owner: "org", | ||
| 204 | name: ConnectionName.make("main"), | ||
| 205 | integration: IntegrationSlug.make(slug), | ||
| 206 | template: AuthTemplateSlug.make("oauth"), | ||
| 207 | }, | ||
| 208 | }); | ||
| 209 | expect(started.status, "oauth.start redirects to the authorization server").toBe( | ||
| 210 | "redirect", | ||
| 211 | ); | ||
| 212 | if (started.status !== "redirect") return yield* Effect.die("no redirect"); | ||
| 213 | |||
| 214 | // Drive the test IdP's consent by hand (authorize → login → code). | ||
| 215 | const code = yield* Effect.promise(async () => { | ||
| 216 | const authorize = await fetch(started.authorizationUrl, { redirect: "manual" }); | ||
| 217 | const loginUrl = authorize.headers.get("location"); | ||
| 218 | if (!loginUrl) throw new Error(`authorize did not redirect: ${authorize.status}`); | ||
| 219 | const login = await fetch(loginUrl, { | ||
| 220 | method: "POST", | ||
| 221 | headers: { | ||
| 222 | authorization: `Basic ${Buffer.from("alice:password").toString("base64")}`, | ||
| 223 | }, | ||
| 224 | redirect: "manual", | ||
| 225 | }); | ||
| 226 | const callbackUrl = login.headers.get("location"); | ||
| 227 | if (!callbackUrl) throw new Error(`login did not redirect: ${login.status}`); | ||
| 228 | const minted = new URL(callbackUrl).searchParams.get("code"); | ||
| 229 | if (!minted) throw new Error("callback carried no authorization code"); | ||
| 230 | return minted; | ||
| 231 | }); | ||
| 232 | yield* client.oauth.complete({ payload: { state: started.state, code } }); | ||
| 233 | |||
| 234 | const tools = yield* client.tools.list({ query: {} }); | ||
| 235 | const address = tools | ||
| 236 | .filter((tool) => String(tool.integration) === slug) | ||
| 237 | .map((tool) => String(tool.address)) | ||
| 238 | .find((addr) => addr.endsWith("listIssues")); | ||
| 239 | expect(address, "the listIssues tool is in the catalog").toBeDefined(); | ||
| 240 | |||
| 241 | const session = mcp.session(identity); | ||
| 242 | const call = (): Effect.Effect<ToolEnvelope, unknown> => | ||
| 243 | Effect.gen(function* () { | ||
| 244 | let called = yield* session.call("execute", { | ||
| 245 | code: invokeByAddressCode(address!, {}), | ||
| 246 | }); | ||
| 247 | // Approval-gated tools pause the execution once per gated call. | ||
| 248 | let guard = 0; | ||
| 249 | while (called.text.includes("executionId:") && guard < 10) { | ||
| 250 | called = yield* session.approvePaused(called.text); | ||
| 251 | guard += 1; | ||
| 252 | } | ||
| 253 | expect( | ||
| 254 | called.ok, | ||
| 255 | `the MCP execute call itself completed (got: ${called.text.slice(0, 400)})`, | ||
| 256 | ).toBe(true); | ||
| 257 | return JSON.parse(called.text) as ToolEnvelope; | ||
| 258 | }); | ||
| 259 | |||
| 260 | // Baseline: the connection works, and the upstream records the bearer. | ||
| 261 | const first = yield* call(); | ||
| 262 | expect(first.ok, "the first call succeeds on the freshly minted token").toBe(true); | ||
| 263 | const bearersAfterFirst = upstream.bearers(); | ||
| 264 | expect(bearersAfterFirst.length, "the upstream saw exactly one call").toBe(1); | ||
| 265 | |||
| 266 | // Revoke it upstream. Executor's stored `expires_at` still says this | ||
| 267 | // token is good for an hour — the divergence this whole path exists | ||
| 268 | // for. Nothing in executor's state changes here. | ||
| 269 | upstream.revokeSeenBearers(); | ||
| 270 | |||
| 271 | const second = yield* call(); | ||
| 272 | |||
| 273 | // THE guarantee: the caller sees a successful call, not a 401 and not | ||
| 274 | // a re-authenticate wall. Executor absorbed the rejection by | ||
| 275 | // re-minting and retrying. | ||
| 276 | expect( | ||
| 277 | second.ok, | ||
| 278 | `the call succeeded after the upstream rejected the stored token (got: ${JSON.stringify(second.error ?? {}).slice(0, 400)})`, | ||
| 279 | ).toBe(true); | ||
| 280 | |||
| 281 | // Proven from the upstream's own ledger: the retry really happened, | ||
| 282 | // and it carried a DIFFERENT bearer than the one that was rejected. | ||
| 283 | const bearers = upstream.bearers(); | ||
| 284 | expect(bearers.length, "the upstream saw the rejected call and the retry (3 total)").toBe( | ||
| 285 | 3, | ||
| 286 | ); | ||
| 287 | const rejected = bearers[1]!; | ||
| 288 | const retried = bearers[2]!; | ||
| 289 | expect(rejected, "the second call reused the token the upstream had revoked").toBe( | ||
| 290 | bearersAfterFirst[0], | ||
| 291 | ); | ||
| 292 | expect(retried, "the retry carried a freshly minted token").not.toBe(rejected); | ||
| 293 | expect( | ||
| 294 | yield* oauth.acceptsAccessToken(retried), | ||
| 295 | "the retry's token is one the authorization server actually issued", | ||
| 296 | ).toBe(true); | ||
| 297 | |||
| 298 | // A refresh grant really was redeemed — not a silent reuse of | ||
| 299 | // something already cached. | ||
| 300 | const refreshGrants = (yield* oauth.requests).filter( | ||
| 301 | (request) => | ||
| 302 | request.path === "/token" && | ||
| 303 | request.method === "POST" && | ||
| 304 | request.body.includes("grant_type=refresh_token"), | ||
| 305 | ); | ||
| 306 | expect(refreshGrants.length, "exactly one refresh grant was redeemed").toBe(1); | ||
| 307 | }), | ||
| 308 | Effect.gen(function* () { | ||
| 309 | yield* client.connections | ||
| 310 | .remove({ | ||
| 311 | params: { | ||
| 312 | owner: "org", | ||
| 313 | integration: IntegrationSlug.make(slug), | ||
| 314 | name: ConnectionName.make("main"), | ||
| 315 | }, | ||
| 316 | }) | ||
| 317 | .pipe(Effect.ignore); | ||
| 318 | yield* client.oauth | ||
| 319 | .removeClient({ params: { slug: clientSlug }, payload: { owner: "org" } }) | ||
| 320 | .pipe(Effect.ignore); | ||
| 321 | yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore); | ||
| 322 | }), | ||
| 323 | ); | ||
| 324 | }), | ||
| 325 | ), | ||
| 326 | ); | ||
| @@ -1662,10 +1662,17 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 1661 unmodified lines | |||
| 1662 | }).pipe( | 1662 | }).pipe( |
| 1663 | // The refresh path was previously invisible to telemetry: no span, no | 1663 | // The refresh path was previously invisible to telemetry: no span, no |
| 1664 | // log, no metric. When a customer reported "my OAuth just died", there | 1664 | // log, no metric. When a customer reported "my OAuth just died", there |
| 1665 | // was no way to answer "did a refresh even fire, and what did the AS | 1665 | // was no way to answer "did a refresh even fire, and did it work?" |
| 1666 | // say?" without a repro. Stamp the outcome and the AS's own error code | 1666 | // without a repro. Stamp the outcome and the failure KIND — enumerable |
| 1667 | // (an enumerable RFC 6749 §5.2 identifier, never user content or token | 1667 | // identifiers only, never user content or token material. |
| 1668 | // material) so refresh health is greppable in traces. | 1668 | // |
| 1669 | // The AS's own RFC 6749 §5.2 code is not stamped here: it survives only | ||
| 1670 | // inside the error's message, and that message embeds the token | ||
| 1671 | // endpoint's response URL and a body preview, so it can't go on a span | ||
| 1672 | // attribute. Making that code a queryable dimension needs it lifted to | ||
| 1673 | // a typed field on CredentialResolutionError first — worth doing, since | ||
| 1674 | // invalid_client (a rotated secret, fleet-wide) currently looks exactly | ||
| 1675 | // like a transient server_error from a trace. | ||
| 1669 | Effect.tap(() => Effect.annotateCurrentSpan({ "executor.oauth.refresh.outcome": "ok" })), | 1676 | Effect.tap(() => Effect.annotateCurrentSpan({ "executor.oauth.refresh.outcome": "ok" })), |
| 1670 | Effect.tapError((error: StorageFailure | CredentialResolutionError) => | 1677 | Effect.tapError((error: StorageFailure | CredentialResolutionError) => |
| 1671 | Effect.annotateCurrentSpan({ | 1678 | Effect.annotateCurrentSpan({ |