Skip to content
115 changes: 115 additions & 0 deletions clients/web/src/test/core/mcp/oauthManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,92 @@ describe("OAuthManager", () => {
});
});

describe("persisting the requested scope after an ordinary grant (#2117)", () => {
function capturedUrlWithScope(scope?: string): URL {
const url = new URL("https://auth.example.com/authorize?state=abc");
if (scope !== undefined) {
url.searchParams.set("scope", scope);
}
return url;
}

async function authorizeThenComplete(
storedScope: string | undefined,
tokenScope: string | undefined,
authorizeUrlScope?: string,
) {
mockedMcpAuth.mockResolvedValue("REDIRECT");
const params = createMockParams();
storageOf(params).getScope.mockResolvedValue(storedScope);
storageOf(params).getClientInformation.mockResolvedValue({
client_id: "cid",
});
const manager = new OAuthManager(params);
const captureSpy = vi
.spyOn(
(await import("@inspector/core/auth/providers.js"))
.BaseOAuthClientProvider.prototype,
"getCapturedAuthUrl",
)
.mockReturnValue(capturedUrlWithScope(authorizeUrlScope));

await manager.authenticate();

mockedMcpAuth.mockResolvedValue("AUTHORIZED");
storageOf(params).getTokens.mockResolvedValue({
access_token: "access",
token_type: "Bearer",
...(tokenScope === undefined ? {} : { scope: tokenScope }),
});
storageOf(params).saveScope.mockClear();

await manager.completeOAuthFlow("code");
captureSpy.mockRestore();
return params;
}

it("persists the requested scope when the token response omits scope", async () => {
const params = await authorizeThenComplete("mcp weather:read", undefined);

expect(storageOf(params).saveScope).toHaveBeenCalledWith(
SERVER_URL,
"mcp weather:read",
);
});

it("still prefers the granted scope when the AS echoes one", async () => {
const params = await authorizeThenComplete("mcp weather:read", "mcp");

expect(storageOf(params).saveScope).toHaveBeenCalledWith(
SERVER_URL,
"mcp",
);
});

it("persists nothing when neither the request nor the response names a scope", async () => {
const params = await authorizeThenComplete(undefined, undefined);

expect(storageOf(params).saveScope).not.toHaveBeenCalled();
});

it("persists the scope the authorize URL actually carried, not the provider's", async () => {
// The SDK augments the request with `offline_access` when the AS
// advertises it and the client wants a refresh token, so the authorize
// URL is a superset of `provider.scope`. Persisting the provider's copy
// would understate the grant the AS implied by staying silent.
const params = await authorizeThenComplete(
"mcp",
undefined,
"mcp offline_access",
);

expect(storageOf(params).saveScope).toHaveBeenCalledWith(
SERVER_URL,
"mcp offline_access",
);
});
});

describe("completeOAuthFlow (EMA)", () => {
it("mints resource tokens via the EMA path and dispatches complete", async () => {
const tokens = { access_token: "EMA", token_type: "Bearer" };
Expand All @@ -762,6 +848,35 @@ describe("OAuthManager", () => {
expect(manager.getOAuthFlowStep()).toBe("complete");
mintSpy.mockRestore();
});

it("persists the minted scope request when the mint response omits scope (#2117)", async () => {
const params = createMockParams({
enterpriseManagedAuth: {
idp: {
issuer: "https://idp.example.com",
clientId: "app-client",
clientSecret: "secret",
},
},
});
params.initialConfig.scope = "mcp weather:read";
storageOf(params).getScope.mockResolvedValue(undefined);
const manager = new OAuthManager(params);
manager.setOAuthConfig({ enterpriseManaged: true });

const mintSpy = vi
.spyOn(emaFlow, "completeEmaIdpAuthorizationAndMint")
.mockResolvedValue({ access_token: "EMA", token_type: "Bearer" });
storageOf(params).saveScope.mockClear();

await manager.completeOAuthFlow("ema-code");

expect(storageOf(params).saveScope).toHaveBeenCalledWith(
SERVER_URL,
"mcp weather:read",
);
mintSpy.mockRestore();
});
});

describe("trySilentEnterpriseManagedAuth", () => {
Expand Down
25 changes: 22 additions & 3 deletions core/mcp/oauthManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,10 @@ export class OAuthManager {
resourceMetadataUrl,
});

const requestedScope = provider.scope;
const result = await mcpAuth(provider, {
serverUrl,
scope: provider.scope,
scope: requestedScope,
fetchFn: this.params.effectiveAuthFetch,
resourceMetadataUrl,
});
Expand All @@ -323,6 +324,21 @@ export class OAuthManager {
throw new Error("Failed to capture authorization URL");
}

// RFC 6749 §5.1: an omitted `scope` in the token response means the grant
// is identical to what was requested. Carry the request across the
// redirect so completeOAuthFlow has that fallback to persist (#2117) —
// the step-up paths already do this, and without it an ordinary grant
// whose AS echoes no scope leaves the previous stored scope standing as
// if it were still what the current token carries.
//
// Read it off the authorize URL rather than from `provider.scope`: the
// SDK augments the request with `offline_access` when the AS advertises
// it and the client asks for a refresh token, so the provider's scope can
// be a strict subset of what was actually asked for — and persisting the
// subset would understate the grant in exactly the way this fixes.
this.pendingAuthorizationScope =
capturedUrl.searchParams.get("scope")?.trim() || requestedScope;
Comment thread
cliffhall marked this conversation as resolved.
Outdated

const stateParam = capturedUrl.searchParams.get("state");
if (stateParam && this.params.onBeforeOAuthRedirect) {
const parsedState = parseOAuthState(stateParam);
Expand Down Expand Up @@ -357,10 +373,13 @@ export class OAuthManager {
authorizationCode,
iss,
);
const requestedScope = this.pendingAuthorizationScope;
// `scopeForMint` — not `pendingAuthorizationScope` — is what the
// mint actually asked for, so it is the RFC 6749 §5.1 fallback here
// (the two differ on the ordinary leg, where only the config carries
// a scope).
const scopeToPersist = resolvePersistedScopeAfterGrant(
Comment thread
cliffhall marked this conversation as resolved.
Outdated
tokens.scope,
requestedScope,
scopeForMint,
);
if (scopeToPersist) {
await this.requireStorage().saveScope(
Expand Down