History

fix(oauth): retry a refresh grant without scope when the AS refuses it (#1982)

Railway answers every scope-bearing refresh with `invalid_scope: refresh token missing requested scope` when its stored grant is narrower than the authorization it echoed back, so a connection whose refresh token was still live failed every call as `oauth_refresh_failed` and only a hand re-authorization recovered it. RFC 6749 §6 defines omitting `scope` as "the scope originally granted", so retry the grant once without it. Only `invalid_scope` qualifies: `invalid_grant` means the token is dead, and retrying that spends a rotating refresh token to learn nothing.

mmarabel committed 2 months ago

Commit cc0fd8f1 parent e9055c15 files · +551 −63Browse files

Showing the changes to e2e (1 of 5 files). Show all

Unified · Split

e2e/scenarios/oauth-refresh-scope-fallback.test.ts+297−0
@@ -0,0 +1,297 @@
1+// Cross-target: an authorization server that refuses a scope-bearing refresh
2+// grant must not turn a live refresh token into a dead connection.
3+//
4+// Railway answers any refresh grant carrying a `scope` parameter with
5+// `invalid_scope: refresh token missing requested scope` when the refresh
6+// token's own record is narrower than the authorization response it echoed
7+// back (issue #1969). Echoing the recorded grant is legal under RFC 6749 §6,
8+// and so is omitting `scope`, which the spec defines as "the scope originally
9+// granted". Before the fallback, the refusal reached the sandbox as
10+// `oauth_refresh_failed` with `retryable: false`, so a connection whose refresh
11+// token was perfectly alive stayed unusable until someone re-authorized it by
12+// hand — the reported symptom was a saved Railway connection failing every
13+// call.
14+//
15+// The journey: an OpenAPI integration completes a real authorization-code flow
16+// against a live test AS that mints instantly-expiring access tokens and holds
17+// a narrower grant on the refresh token than the authorization echoed. The
18+// first tool call must therefore refresh, the scope-bearing grant is refused,
19+// executor retries WITHOUT `scope`, and the call succeeds — proven from the
20+// AS's own request ledger, which records both the refused request and the
21+// accepted retry.
22+import { randomBytes } from "node:crypto";
23+import { createServer } from "node:http";
24+
25+import { expect } from "@effect/vitest";
26+import { Effect } from "effect";
27+import { composePluginApi } from "@executor-js/api/server";
28+import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
29+import {
30+ AuthTemplateSlug,
31+ ConnectionName,
32+ IntegrationSlug,
33+ OAuthClientSlug,
34+} from "@executor-js/sdk/shared";
35+import { serveOAuthTestServer } from "@executor-js/sdk/testing";
36+
37+import { scenario } from "../src/scenario";
38+import { Api, Mcp, Target } from "../src/services";
39+
40+const api = composePluginApi([openApiHttpPlugin()] as const);
41+
42+const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`;
43+
44+/** Upstream on 127.0.0.1: `GET /issues` is 200 for any bearer and returns one
45+ * issue, so "the call succeeded" is distinguishable from "the call returned
46+ * an empty body". */
47+const serveUpstream = () =>
48+ Effect.acquireRelease(
49+ Effect.callback<{ readonly url: string; readonly close: () => void }>((resume) => {
50+ const server = createServer((request, response) => {
51+ if (request.method === "GET" && (request.url ?? "").startsWith("/issues")) {
52+ response.writeHead(200, { "content-type": "application/json" });
53+ response.end(JSON.stringify({ issues: [{ id: "issue-1", title: "Scope drift" }] }));
54+ return;
55+ }
56+ response.writeHead(404, { "content-type": "application/json" });
57+ response.end(JSON.stringify({ error: "not_found" }));
58+ });
59+ server.listen(0, "127.0.0.1", () => {
60+ const address = server.address();
61+ const port = typeof address === "object" && address ? address.port : 0;
62+ resume(
63+ Effect.succeed({
64+ url: `http://127.0.0.1:${port}`,
65+ close: () => {
66+ server.close();
67+ server.closeAllConnections();
68+ },
69+ }),
70+ );
71+ });
72+ }),
73+ (server) => Effect.sync(server.close),
74+ );
75+
76+const spec = (
77+ baseUrl: string,
78+ oauth: { readonly authorizationEndpoint: string; readonly tokenEndpoint: string },
79+): string =>
80+ JSON.stringify({
81+ openapi: "3.0.3",
82+ info: { title: "Issues API", version: "1.0.0" },
83+ servers: [{ url: baseUrl }],
84+ paths: {
85+ "/issues": {
86+ get: {
87+ operationId: "listIssues",
88+ summary: "List issues",
89+ security: [{ oauth: ["issues.read"] }],
90+ responses: { "200": { description: "issues" } },
91+ },
92+ },
93+ },
94+ components: {
95+ securitySchemes: {
96+ oauth: {
97+ type: "oauth2",
98+ flows: {
99+ authorizationCode: {
100+ authorizationUrl: oauth.authorizationEndpoint,
101+ tokenUrl: oauth.tokenEndpoint,
102+ scopes: { "issues.read": "Read issues", "issues.write": "Write issues" },
103+ },
104+ },
105+ },
106+ },
107+ },
108+ });
109+
110+const invokeByAddressCode = (address: string, args: unknown) => `
111+const segments = ${JSON.stringify(address)}.split(".").slice(1);
112+let node = tools;
113+for (const segment of segments) node = node[segment];
114+const result = await node(${JSON.stringify(args)});
115+return JSON.stringify(result);
116+`;
117+
118+type ToolEnvelope = {
119+ readonly ok: boolean;
120+ readonly data?: unknown;
121+ readonly error?: {
122+ readonly code?: string;
123+ readonly message?: string;
124+ };
125+};
126+
127+scenario(
128+ "Auth failures · a refresh refused for its scope is retried without one, so a scope-drifted connection keeps working",
129+ {},
130+ Effect.scoped(
131+ Effect.gen(function* () {
132+ const target = yield* Target;
133+ const { client: makeClient } = yield* Api;
134+ const mcp = yield* Mcp;
135+ const identity = yield* target.newIdentity();
136+ const client = yield* makeClient(api, identity);
137+ const upstream = yield* serveUpstream();
138+ // The AS grants both scopes at authorization and echoes them, but the
139+ // refresh token it stores covers only `issues.read` — the divergence
140+ // that makes the recorded grant look like a request for more access.
141+ const oauth = yield* serveOAuthTestServer({
142+ scopes: ["issues.read", "issues.write"],
143+ refreshGrantScopes: ["issues.read"],
144+ tokenExpiresInSeconds: 0,
145+ });
146+ const slug = unique("refreshscope");
147+ const clientSlug = OAuthClientSlug.make(unique("refreshscopec"));
148+
149+ yield* Effect.ensuring(
150+ Effect.gen(function* () {
151+ yield* client.openapi.addSpec({
152+ payload: {
153+ spec: { kind: "blob", value: spec(upstream.url, oauth) },
154+ slug,
155+ baseUrl: upstream.url,
156+ authenticationTemplate: [
157+ {
158+ slug: "oauth",
159+ kind: "oauth2",
160+ authorizationUrl: oauth.authorizationEndpoint,
161+ tokenUrl: oauth.tokenEndpoint,
162+ scopes: ["issues.read", "issues.write"],
163+ },
164+ ],
165+ },
166+ });
167+ yield* client.oauth.createClient({
168+ payload: {
169+ owner: "org",
170+ slug: clientSlug,
171+ grant: "authorization_code",
172+ authorizationUrl: oauth.authorizationEndpoint,
173+ tokenUrl: oauth.tokenEndpoint,
174+ clientId: "test-client",
175+ clientSecret: "test-secret",
176+ originIntegration: IntegrationSlug.make(slug),
177+ },
178+ });
179+
180+ const started = yield* client.oauth.start({
181+ payload: {
182+ client: clientSlug,
183+ clientOwner: "org",
184+ owner: "org",
185+ name: ConnectionName.make("main"),
186+ integration: IntegrationSlug.make(slug),
187+ template: AuthTemplateSlug.make("oauth"),
188+ },
189+ });
190+ expect(started.status, "oauth.start redirects to the authorization server").toBe(
191+ "redirect",
192+ );
193+ if (started.status !== "redirect") return yield* Effect.die("no redirect");
194+
195+ // Drive the test IdP's consent by hand (authorize → login → code).
196+ const code = yield* Effect.promise(async () => {
197+ const authorize = await fetch(started.authorizationUrl, { redirect: "manual" });
198+ const loginUrl = authorize.headers.get("location");
199+ if (!loginUrl) throw new Error(`authorize did not redirect: ${authorize.status}`);
200+ const login = await fetch(loginUrl, {
201+ method: "POST",
202+ headers: {
203+ authorization: `Basic ${Buffer.from("alice:password").toString("base64")}`,
204+ },
205+ redirect: "manual",
206+ });
207+ const callbackUrl = login.headers.get("location");
208+ if (!callbackUrl) throw new Error(`login did not redirect: ${login.status}`);
209+ const minted = new URL(callbackUrl).searchParams.get("code");
210+ if (!minted) throw new Error("callback carried no authorization code");
211+ return minted;
212+ });
213+ yield* client.oauth.complete({ payload: { state: started.state, code } });
214+
215+ const tools = yield* client.tools.list({ query: {} });
216+ const address = tools
217+ .filter((tool) => String(tool.integration) === slug)
218+ .map((tool) => String(tool.address))
219+ .find((addr) => addr.endsWith("listIssues"));
220+ expect(address, "the listIssues tool is in the catalog").toBeDefined();
221+
222+ // Call through the real MCP surface, the channel the reported
223+ // failure was seen on.
224+ const session = mcp.session(identity);
225+ let called = yield* session.call("execute", {
226+ code: invokeByAddressCode(address!, {}),
227+ });
228+ // Approval-gated tools pause the execution once per gated call.
229+ let guard = 0;
230+ while (called.text.includes("executionId:") && guard < 10) {
231+ called = yield* session.approvePaused(called.text);
232+ guard += 1;
233+ }
234+ expect(
235+ called.ok,
236+ `the MCP execute call itself completed (got: ${called.text.slice(0, 400)})`,
237+ ).toBe(true);
238+ const envelope = JSON.parse(called.text) as ToolEnvelope;
239+
240+ // THE guarantee: the caller gets data, not `oauth_refresh_failed`.
241+ // The access token had already expired, so this call could only
242+ // succeed by refreshing.
243+ expect(
244+ envelope.ok,
245+ `the tool call succeeded (got: ${JSON.stringify(envelope.error ?? {}).slice(0, 400)})`,
246+ ).toBe(true);
247+ expect(
248+ JSON.stringify(envelope.data ?? {}),
249+ "the upstream's payload came back, so the retried token really worked upstream",
250+ ).toContain("issue-1");
251+
252+ // Proven from the AS's own ledger: the scope-bearing grant was
253+ // refused, and the scope-less retry is what succeeded.
254+ const refreshGrants = (yield* oauth.requests).filter(
255+ (request) =>
256+ request.path === "/token" &&
257+ request.method === "POST" &&
258+ request.body.includes("grant_type=refresh_token"),
259+ );
260+ expect(
261+ refreshGrants.length,
262+ "the refused refresh and the retry are both on the ledger (2 requests)",
263+ ).toBe(2);
264+ const refused = refreshGrants[0]!;
265+ const retried = refreshGrants[1]!;
266+ expect(
267+ new URLSearchParams(refused.body).get("scope"),
268+ "the first refresh echoed the grant the connection recorded",
269+ ).toBe("issues.read issues.write");
270+ expect(
271+ new URLSearchParams(retried.body).has("scope"),
272+ "the retry omitted scope, the form RFC 6749 §6 defines as 'the grant originally issued'",
273+ ).toBe(false);
274+ expect(
275+ new URLSearchParams(retried.body).get("refresh_token"),
276+ "the retry replayed the same still-live refresh token",
277+ ).toBe(new URLSearchParams(refused.body).get("refresh_token"));
278+ }),
279+ Effect.gen(function* () {
280+ yield* client.connections
281+ .remove({
282+ params: {
283+ owner: "org",
284+ integration: IntegrationSlug.make(slug),
285+ name: ConnectionName.make("main"),
286+ },
287+ })
288+ .pipe(Effect.ignore);
289+ yield* client.oauth
290+ .removeClient({ params: { slug: clientSlug }, payload: { owner: "org" } })
291+ .pipe(Effect.ignore);
292+ yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore);
293+ }),
294+ );
295+ }),
296+ ),
297+);