Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e2fb78d
feat(oauth): make the refresh_token grant opt-out per server
cliffhall Aug 24, 2026
f6fca59
docs(oauth): scope what the refresh-token opt-out actually changes
cliffhall Aug 25, 2026
fca3f1c
fix(oauth): stop claiming the opt-out removes every offline_access
cliffhall Aug 25, 2026
6303f8a
test(oauth): prove the opt-out against a real authorization server
cliffhall Aug 25, 2026
89dedd8
fix(oauth): drop an inherited offline_access when the grant is declined
cliffhall Aug 25, 2026
1bbdef0
fix(oauth): keep the scope filter from overwriting persisted storage
cliffhall Aug 25, 2026
f153591
fix(oauth): honor a newly configured offline_access against stale sto…
cliffhall Aug 25, 2026
87fe7b2
refactor(web): extract the refresh-token change handler
cliffhall Aug 25, 2026
d29ff8b
refactor(web): shorten the refresh-token checkbox description
cliffhall Aug 25, 2026
6968738
Merge branch 'v2/main' into v2/feat/2068-refresh-token-toggle
cliffhall Aug 25, 2026
26b6f03
fix(oauth): filter the step-up scope union too, not just the provider
cliffhall Aug 25, 2026
40149a2
fix(web): name the Scopes control in the offline_access warning
cliffhall Aug 25, 2026
fef8f5c
fix(oauth): pass the raw challenge scopes to the step-up scope filter
cliffhall Aug 25, 2026
b339ad4
fix(oauth): persist the filtered scope after an ordinary grant
cliffhall Aug 25, 2026
99a0748
docs: say when the filtered scope reaches storage
cliffhall Aug 25, 2026
40cfb51
Merge branch 'v2/main' into v2/feat/2068-refresh-token-toggle
cliffhall Aug 25, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions clients/web/src/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,104 @@ const fetchLogInstances = (
FetchLogModule as unknown as { __fetchLogInstances: EventTarget[] }
).__fetchLogInstances;

// #2068 — the web connection seam for the refresh-token opt-out. The provider
// and manager tests start from an already-built OAuth config and the runner test
// covers only the CLI/TUI leg, so without this case deleting the
// `requestRefreshToken` spread from App's `oauthFromServer` would leave the
// checkbox persisted but inert with every other new test still green.
describe("App wires the refresh-token opt-out into the client (#2068)", () => {
const HTTP_SERVER = {
id: "A",
name: "PlotRocket",
config: { type: "streamable-http", url: "https://api.example.com/mcp" },
connection: { status: "disconnected" },
};

function mockServersWith(settings?: Partial<InspectorServerSettings>) {
vi.mocked(useServers).mockReturnValue({
servers: [
{
...HTTP_SERVER,
...(settings ? { settings } : {}),
} as unknown as ServerEntry,
],
Comment thread
cliffhall marked this conversation as resolved.
Outdated
loading: false,
error: undefined,
refresh: vi.fn().mockResolvedValue(undefined),
addServer: addServerSpy,
updateServer: updateServerSpy,
updateServerSettings: updateServerSettingsSpy,
removeServer: vi.fn(),
reorderServers: vi.fn(),
importSource: vi.fn().mockResolvedValue({ servers: {} }),
});
}

/** The `oauth` option the mocked InspectorClient constructor was given. */
function constructedOAuth(): { requestRefreshToken?: boolean } | undefined {
const call = vi.mocked(McpIndex.InspectorClient).mock.calls[0];
const options = call?.[1] as
| { oauth?: { requestRefreshToken?: boolean } }
| undefined;
return options?.oauth;
}

let previousUseServers: typeof useServers | undefined;

beforeEach(() => {
vi.clearAllMocks();
clientInstances.length = 0;
previousUseServers = vi.mocked(useServers).getMockImplementation();
vi.mocked(useInspectorClient).mockReturnValue(DEFAULT_USE_INSPECTOR_CLIENT);
});

afterEach(() => {
if (previousUseServers) {
vi.mocked(useServers).mockImplementation(previousUseServers);
}
});

it("passes requestRefreshToken: false when the server opted out", async () => {
const user = userEvent.setup();
mockServersWith({ oauthRequestRefreshToken: false });
renderWithMantine(<App />);

await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));

expect(constructedOAuth()?.requestRefreshToken).toBe(false);
});

// The default must not send the key at all — the provider's own default is
// what declares the grant, and a stray `true` here would mask its removal.
it("omits requestRefreshToken when the setting is on", async () => {
const user = userEvent.setup();
mockServersWith({ oauthScopes: "openid" });
renderWithMantine(<App />);

await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));

const oauth = constructedOAuth();
expect(oauth).toBeDefined();
expect(oauth).not.toHaveProperty("requestRefreshToken");
});

// The opt-out is the only OAuth field set, so it must be enough on its own to
// materialize the `oauth` option — otherwise it is dropped for any server
// without credentials or scopes, which is the common case here.
it("builds the oauth option from the opt-out alone", async () => {
const user = userEvent.setup();
mockServersWith({ oauthRequestRefreshToken: false });
renderWithMantine(<App />);

await user.click(screen.getByText("connect"));
await waitFor(() => expect(clientInstances).toHaveLength(1));

expect(constructedOAuth()).toEqual({ requestRefreshToken: false });
});
});

describe("App failed-connection card border (#1621)", () => {
beforeEach(() => {
clientInstances.length = 0;
Expand Down
8 changes: 7 additions & 1 deletion clients/web/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2602,7 +2602,8 @@ function App() {
savedSettings.oauthScopes ||
serverAuthorizationParams ||
serverEndpointOverrides ||
savedSettings.enterpriseManaged)
savedSettings.enterpriseManaged ||
savedSettings.oauthRequestRefreshToken === false)
? {
...(savedSettings.oauthClientId && {
clientId: savedSettings.oauthClientId,
Expand All @@ -2620,6 +2621,11 @@ function App() {
...(savedSettings.enterpriseManaged && {
enterpriseManaged: true,
}),
// #2068: only the explicit opt-out is forwarded; omitting the key
// leaves the provider's default (declare `refresh_token`) in place.
...(savedSettings.oauthRequestRefreshToken === false && {
requestRefreshToken: false,
}),
Comment thread
cliffhall marked this conversation as resolved.
Comment thread
cliffhall marked this conversation as resolved.
}
: undefined;
const oauth =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -706,9 +706,118 @@ describe("ServerSettingsForm", () => {
authorizationUrl: "",
tokenUrl: "",
enterpriseManaged: false,
requestRefreshToken: true,
});
});

// #2068 — the refresh-token opt-out. On by default; unchecking it is what
// stops the SDK adding `offline_access` and then `prompt=consent`.
it("renders Request refresh token checked by default", () => {
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
settings={emptySettings}
expandedSections={["oauth"]}
/>,
);
expect(screen.getByLabelText("Request refresh token")).toBeChecked();
});

it("renders Request refresh token unchecked when the server opted out", () => {
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
settings={{ ...emptySettings, oauthRequestRefreshToken: false }}
expandedSections={["oauth"]}
/>,
);
expect(screen.getByLabelText("Request refresh token")).not.toBeChecked();
});

it("toggles Request refresh token through onOAuthChange", async () => {
const user = userEvent.setup();
const onOAuthChange = vi.fn();
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
onOAuthChange={onOAuthChange}
settings={emptySettings}
expandedSections={["oauth"]}
/>,
);
await user.click(screen.getByLabelText("Request refresh token"));
expect(onOAuthChange).toHaveBeenLastCalledWith(
expect.objectContaining({ requestRefreshToken: false }),
);
});

it("warns when opting out while offline_access is still in Scopes", () => {
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
settings={{
...emptySettings,
oauthRequestRefreshToken: false,
oauthScopes: "openid offline_access",
}}
expandedSections={["oauth"]}
/>,
);
expect(
screen.getByText("offline_access is still requested"),
).toBeInTheDocument();
});

it("does not warn when opted out and Scopes omits offline_access", () => {
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
settings={{
...emptySettings,
oauthRequestRefreshToken: false,
oauthScopes: "openid",
}}
expandedSections={["oauth"]}
/>,
);
expect(
screen.queryByText("offline_access is still requested"),
).not.toBeInTheDocument();
});

// The scope alone is not a problem — with the grant declared, requesting
// offline_access is exactly what the default configuration does.
it("does not warn when offline_access is in Scopes but the grant is on", () => {
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
settings={{ ...emptySettings, oauthScopes: "openid offline_access" }}
expandedSections={["oauth"]}
/>,
);
expect(
screen.queryByText("offline_access is still requested"),
).not.toBeInTheDocument();
});

// A substring must not trip it — `offline_access_extra` is a different scope.
it("matches offline_access as a whole scope token, not a substring", () => {
renderWithMantine(
<ServerSettingsForm
{...baseHandlers}
settings={{
...emptySettings,
oauthRequestRefreshToken: false,
oauthScopes: "openid offline_access_extra",
}}
expandedSections={["oauth"]}
/>,
);
expect(
screen.queryByText("offline_access is still requested"),
).not.toBeInTheDocument();
});

// #2018 — custom authorization-request parameters.
it("shows the empty hint for authorization parameters when none are set", () => {
renderWithMantine(
Expand Down Expand Up @@ -1112,6 +1221,7 @@ describe("ServerSettingsForm", () => {
authorizationUrl: "",
tokenUrl: "",
enterpriseManaged: true,
requestRefreshToken: true,
});
});

Expand Down Expand Up @@ -1219,6 +1329,7 @@ describe("ServerSettingsForm", () => {
authorizationUrl: "",
tokenUrl: "",
enterpriseManaged: false,
requestRefreshToken: true,
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,8 @@ export function ServerSettingsForm({
tokenUrl: settings.oauthTokenUrl ?? "",
enterpriseManaged: settings.enterpriseManaged ?? false,
onInsufficientScope: settings.oauthOnInsufficientScope,
// Unset means the default, on — only an explicit opt-out is stored.
requestRefreshToken: settings.oauthRequestRefreshToken ?? true,
};
}

Expand All @@ -528,6 +530,15 @@ export function ServerSettingsForm({
? " Not applied while Enterprise-managed authorization is on: that flow authorizes against the enterprise IdP, a different authorization server."
: "";

// #2068 — the opt-out only removes the SDK's *automatic* `offline_access`.
// `prompt=consent` keys off the scope, not the grant, so an `offline_access`
// the user typed here still forces the prompt the checkbox claims to avoid.
// Flag that combination rather than letting it read as a fixed configuration.
const scopesIncludeOfflineAccess = (settings.oauthScopes ?? "")
.split(/\s+/)
.includes("offline_access");
const refreshTokenOptedOut = settings.oauthRequestRefreshToken === false;

const rejectedParamKeys = authorizationParams
.map((p) => p.key.trim())
.filter((key) => isReservedAuthorizationParam(key));
Expand Down Expand Up @@ -879,6 +890,25 @@ export function ServerSettingsForm({
) : null
}
/>
<Checkbox
label="Request refresh token"
description="When checked, the Inspector registers with the refresh_token grant so it can renew access tokens without a new sign-in. Unchecking it stops the SDK adding offline_access to the requested scope, and so stops the prompt=consent it forces — on Microsoft Entra ID that prompt routes a non-admin user into the admin-consent workflow (AADSTS90094) even after a tenant admin has consented. It removes only that automatic scope: an offline_access you configure in Scopes still forces the prompt. Applies on the next connect, and does not revoke a refresh token already issued or reach a registration the authorization server already holds."
checked={settings.oauthRequestRefreshToken ?? true}
onChange={(e) =>
onOAuthChange({
...currentOAuth(),
requestRefreshToken: e.currentTarget.checked,
})
}
/>
{refreshTokenOptedOut && scopesIncludeOfflineAccess ? (
<Alert color="yellow" title="offline_access is still requested">
Scopes above lists offline_access, and prompt=consent is added
Comment thread
cliffhall marked this conversation as resolved.
Outdated
whenever that scope is requested — regardless of this setting.
Remove it from Scopes as well, or the consent prompt this
checkbox is meant to avoid still goes out.
</Alert>
) : null}
<Stack gap="xs">
<FieldLabel>Additional authorization parameters</FieldLabel>
<FieldDescription>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,43 @@ describe("ServerSettingsModal", () => {
);
});

// #2068 — only the opt-out is persisted; re-checking the box clears the field
// rather than writing `true`, so a default-on server keeps a minimal entry.
it("maps the refresh-token opt-out into settings, and back to unset", async () => {
const user = userEvent.setup();
const onSettingsChange = vi.fn();
const { rerender } = renderWithMantine(
<ServerSettingsModal
opened
settings={initialSettings}
serverType="streamable-http"
isStdio={false}
onClose={vi.fn()}
onSettingsChange={onSettingsChange}
/>,
);
await user.click(screen.getByRole("button", { name: /OAuth Settings/i }));
await user.click(screen.getByLabelText("Request refresh token"));
expect(onSettingsChange).toHaveBeenLastCalledWith(
expect.objectContaining({ oauthRequestRefreshToken: false }),
);

rerender(
<ServerSettingsModal
opened
settings={{ ...initialSettings, oauthRequestRefreshToken: false }}
serverType="streamable-http"
isStdio={false}
onClose={vi.fn()}
onSettingsChange={onSettingsChange}
/>,
);
await user.click(screen.getByLabelText("Request refresh token"));
expect(onSettingsChange).toHaveBeenLastCalledWith(
expect.objectContaining({ oauthRequestRefreshToken: undefined }),
);
});

it("maps the selected protocol era into settings (#1626)", async () => {
const user = userEvent.setup();
const onSettingsChange = vi.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,10 @@ export function ServerSettingsModal({
// the SDK's `reauthorize` behavior without writing a spurious field.
oauthOnInsufficientScope:
oauth.onInsufficientScope === "throw" ? "throw" : undefined,
// #2068: persist only the non-default (off). `undefined` means on, so a
// server that never touched the switch writes no field at all.
oauthRequestRefreshToken:
oauth.requestRefreshToken === false ? false : undefined,
});
}

Expand Down
Loading