Merge branch 'v2/main' into v2/feat/1005-schema-lint

clients/web/src/test/integration/mcp/inspectorClient-oauth-e2e.test.ts

@@ -1383,6 +1383,158 @@ describe("InspectorClient OAuth E2E", () => {
       if (!authUrl) throw new Error("Expected authorization URL");
       const scope = authUrl.searchParams.get("scope") ?? "";
       expect(scope.split(/\s+/)).toContain("offline_access");
+      // The consequence of that scope, and the actual failure in #2068: the SDK
+      // forces an interactive consent prompt. Asserted here so the opt-out case
+      // below is a real before/after pair rather than a scope check alone.
+      expect(authUrl.searchParams.get("prompt")).toBe("consent");
+    });
+
+    // #2068 — the same server, the same AS advertising `offline_access`, with
+    // the grant declined. This is the end-to-end proof the unit tests cannot
+    // give: they assert our `grant_types` array, while the behavior that
+    // actually unblocks an Entra tenant is what the SDK puts in the authorize
+    // URL. If the SDK ever stopped keying its augmentation off `grant_types`,
+    // every other test here would still pass and AADSTS90094 would remain.
+    it("omits offline_access and prompt=consent when the refresh-token grant is declined", async () => {
+      const transport = transports[1]!;
+      const staticClientId = "test-no-offline-access-client";
+      const staticClientSecret = "test-no-offline-access-secret";
+      const serverConfig = {
+        ...getDefaultServerConfig(),
+        serverType: transport.serverType,
+        ...createOAuthTestServerConfig({
+          requireAuth: true,
+          supportRefreshTokens: true,
+          scopesSupported: ["mcp", "offline_access"],
+          staticClients: [
+            {
+              clientId: staticClientId,
+              clientSecret: staticClientSecret,
+              redirectUris: [testRedirectUrl],
+            },
+          ],
+        }),
+      };
+      server = new TestServerHttp(serverConfig);
+      const port = await server.start();
+      const serverUrl = `http://localhost:${port}`;
+      await waitForOAuthWellKnown(serverUrl);
+
+      const oauthConfig = createTestOAuthConfig({
+        mode: "static",
+        clientId: staticClientId,
+        clientSecret: staticClientSecret,
+        redirectUrl: testRedirectUrl,
+        scope: "mcp",
+      });
+      client = new InspectorClient(
+        {
+          type: transport.clientType,
+          url: `${serverUrl}${transport.endpoint}`,
+        } as MCPServerConfig,
+        {
+          environment: {
+            transport: createTransportNode,
+            oauth: {
+              storage: oauthConfig.storage,
+              navigation: oauthConfig.navigation,
+              redirectUrlProvider: oauthConfig.redirectUrlProvider,
+            },
+          },
+          oauth: {
+            clientId: oauthConfig.clientId,
+            clientSecret: oauthConfig.clientSecret,
+            scope: oauthConfig.scope,
+            requestRefreshToken: false,
+          },
+        },
+      );
+
+      const authUrl = await client.authenticate();
+      if (!authUrl) throw new Error("Expected authorization URL");
+      const scope = authUrl.searchParams.get("scope") ?? "";
+      expect(scope.split(/\s+/)).not.toContain("offline_access");
+      expect(authUrl.searchParams.get("prompt")).toBeNull();
+      // The configured scope still goes out — the opt-out drops the SDK's
+      // addition, not what the user asked for.
+      expect(scope.split(/\s+/)).toContain("mcp");
+    });
+
+    // #2068 — the case that makes the opt-out worth anything to the user who
+    // reported it: a server that already authorized successfully. That grant
+    // persists the AS-granted scope, `offline_access` included, and the
+    // provider reloads it on the next connect. Handing it back unfiltered puts
+    // the token in scope again and `startAuthorization` re-adds
+    // `prompt=consent` off the scope alone — box unchecked, AADSTS90094 intact.
+    it("drops an offline_access inherited from an earlier grant's stored scope", async () => {
+      const transport = transports[1]!;
+      const staticClientId = "test-inherited-offline-client";
+      const staticClientSecret = "test-inherited-offline-secret";
+      const serverConfig = {
+        ...getDefaultServerConfig(),
+        serverType: transport.serverType,
+        ...createOAuthTestServerConfig({
+          requireAuth: true,
+          supportRefreshTokens: true,
+          scopesSupported: ["mcp", "offline_access"],
+          staticClients: [
+            {
+              clientId: staticClientId,
+              clientSecret: staticClientSecret,
+              redirectUris: [testRedirectUrl],
+            },
+          ],
+        }),
+      };
+      server = new TestServerHttp(serverConfig);
+      const port = await server.start();
+      const serverUrl = `http://localhost:${port}`;
+      await waitForOAuthWellKnown(serverUrl);
+
+      const oauthConfig = createTestOAuthConfig({
+        mode: "static",
+        clientId: staticClientId,
+        clientSecret: staticClientSecret,
+        redirectUrl: testRedirectUrl,
+        scope: "mcp",
+      });
+      // Exactly what a completed default-on authorization leaves behind (see
+      // `resolvePersistedScopeAfterGrant`): the scope the AS granted.
+      const mcpUrl = `${serverUrl}${transport.endpoint}`;
+      await oauthConfig.storage.saveScope(mcpUrl, "mcp offline_access");
+
+      client = new InspectorClient(
+        { type: transport.clientType, url: mcpUrl } as MCPServerConfig,
+        {
+          environment: {
+            transport: createTransportNode,
+            oauth: {
+              storage: oauthConfig.storage,
+              navigation: oauthConfig.navigation,
+              redirectUrlProvider: oauthConfig.redirectUrlProvider,
+            },
+          },
+          oauth: {
+            clientId: oauthConfig.clientId,
+            clientSecret: oauthConfig.clientSecret,
+            scope: oauthConfig.scope,
+            requestRefreshToken: false,
+          },
+        },
+      );
+
+      const authUrl = await client.authenticate();
+      if (!authUrl) throw new Error("Expected authorization URL");
+      const scope = authUrl.searchParams.get("scope") ?? "";
+      expect(scope.split(/\s+/)).not.toContain("offline_access");
+      expect(authUrl.searchParams.get("prompt")).toBeNull();
+      expect(scope.split(/\s+/)).toContain("mcp");
+
+      // Filtered on the way out, not in storage — re-checking the box has to
+      // restore the old behavior rather than find the scope destroyed.
+      expect(await oauthConfig.storage.getScope(mcpUrl)).toBe(
+        "mcp offline_access",
+      );
     });
   });
 

clients/web/src/test/integration/mcp/remote/server-extra-coverage.test.ts

@@ -631,6 +631,14 @@ describe("server.ts supplemental coverage", () => {
       expect((await res.json()).error).toMatch(/enterpriseManaged/);
     });
 
+    it("rejects a non-boolean oauthRequestRefreshToken (#2068)", async () => {
+      const res = await postSettings({
+        ...base,
+        oauthRequestRefreshToken: "no",
+      });
+      expect((await res.json()).error).toMatch(/oauthRequestRefreshToken/);
+    });
+
     it("rejects malformed roots", async () => {
       const res = await postSettings({ ...base, roots: [{ uri: 1 }] });
       expect((await res.json()).error).toMatch(/roots/);
@@ -667,6 +675,39 @@ describe("server.ts supplemental coverage", () => {
       expect(res.status).toBe(200);
     });
 
+    // #2068 — a 200 only proves the payload validated. Without reading the
+    // saved entry back, deleting the `oauthRequestRefreshToken` line from
+    // `normalizeSettings` leaves every other test green while saves through
+    // this route silently revert to the default.
+    it("persists the refresh-token opt-out through a save", async () => {
+      expect(
+        (await postSettings({ ...base, oauthRequestRefreshToken: false }))
+          .status,
+      ).toBe(200);
+
+      const res = await fetch(`${h.baseUrl}/api/servers`);
+      const body = (await res.json()) as {
+        mcpServers: Record<
+          string,
+          { oauth?: { requestRefreshToken?: boolean } }
+        >;
+      };
+      expect(body.mcpServers.srv?.oauth?.requestRefreshToken).toBe(false);
+    });
+
+    it("writes no refresh-token field when the setting is on", async () => {
+      expect(
+        (await postSettings({ ...base, oauthRequestRefreshToken: true }))
+          .status,
+      ).toBe(200);
+
+      const res = await fetch(`${h.baseUrl}/api/servers`);
+      const body = (await res.json()) as {
+        mcpServers: Record<string, { oauth?: Record<string, unknown> }>;
+      };
+      expect(body.mcpServers.srv?.oauth?.requestRefreshToken).toBeUndefined();
+    });
+
     it("accepts a fully-populated valid settings payload", async () => {
       const res = await postSettings({
         ...base,
@@ -682,6 +723,7 @@ describe("server.ts supplemental coverage", () => {
         oauthClientId: "cid",
         oauthScopes: "a b",
         enterpriseManaged: true,
+        oauthRequestRefreshToken: false,
         roots: [{ uri: "file:///x", name: "x" }],
       });
       expect(res.status).toBe(200);
@@ -824,6 +866,57 @@ describe("server.ts supplemental coverage", () => {
       }
     });
 
+    // #2068 — same all-or-nothing rule for the refresh-token opt-out: a
+    // non-boolean drops the whole `oauth` node rather than reaching the
+    // provider, where only an explicit `false` means anything.
+    it("drops oauth whose requestRefreshToken is not a boolean", async () => {
+      const h = await start({
+        seedConfig: JSON.stringify({
+          mcpServers: {
+            srv: {
+              type: "streamable-http",
+              url: "https://x.test/mcp",
+              oauth: { requestRefreshToken: "no" },
+            },
+          },
+        }),
+      });
+      try {
+        const res = await fetch(`${h.baseUrl}/api/servers`);
+        const body = (await res.json()) as {
+          mcpServers: Record<string, Record<string, unknown>>;
+        };
+        expect(body.mcpServers.srv).not.toHaveProperty("oauth");
+      } finally {
+        await stop(h);
+      }
+    });
+
+    it("keeps a well-formed requestRefreshToken opt-out on read (#2068)", async () => {
+      const h = await start({
+        seedConfig: JSON.stringify({
+          mcpServers: {
+            srv: {
+              type: "streamable-http",
+              url: "https://x.test/mcp",
+              oauth: { requestRefreshToken: false },
+            },
+          },
+        }),
+      });
+      try {
+        const res = await fetch(`${h.baseUrl}/api/servers`);
+        const body = (await res.json()) as {
+          mcpServers: Record<string, Record<string, unknown>>;
+        };
+        expect(body.mcpServers.srv?.oauth).toEqual({
+          requestRefreshToken: false,
+        });
+      } finally {
+        await stop(h);
+      }
+    });
+
     it("keeps a well-formed authorizationParams record on read", async () => {
       const h = await start({
         seedConfig: JSON.stringify({