fix: include MCP OAuth resource indicator
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed MCP OAuth authorization and token requests to include the required `resource` indicator for the target MCP server.
|
||||
|
||||
## [15.10.12] - 2026-06-10
|
||||
|
||||
### Added
|
||||
|
||||
@@ -36,6 +36,7 @@ export interface MCPServer {
|
||||
tokenUrl?: string;
|
||||
clientId?: string;
|
||||
clientSecret?: string;
|
||||
resource?: string;
|
||||
};
|
||||
/** OAuth configuration (clientId, clientSecret, redirectUri, callbackPort, callbackPath) for servers requiring explicit client credentials */
|
||||
oauth?: {
|
||||
|
||||
@@ -62,6 +62,10 @@
|
||||
"clientSecret": {
|
||||
"type": "string",
|
||||
"description": "OAuth client secret persisted for refresh."
|
||||
},
|
||||
"resource": {
|
||||
"type": "string",
|
||||
"description": "MCP resource URI persisted for OAuth resource indicators."
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -1174,12 +1174,15 @@ export class MCPManager {
|
||||
const shouldRefresh =
|
||||
forceRefresh || (credential.expires && Date.now() >= credential.expires - REFRESH_BUFFER_MS);
|
||||
if (shouldRefresh && credential.refresh && auth.tokenUrl) {
|
||||
const resource =
|
||||
auth.resource ?? (config.type === "http" || config.type === "sse" ? config.url : undefined);
|
||||
try {
|
||||
const refreshed = await refreshMCPOAuthToken(
|
||||
auth.tokenUrl,
|
||||
credential.refresh,
|
||||
auth.clientId,
|
||||
auth.clientSecret,
|
||||
resource,
|
||||
);
|
||||
const refreshedCredential = { type: "oauth" as const, ...refreshed };
|
||||
await this.#authStorage.set(credentialId, refreshedCredential);
|
||||
|
||||
@@ -11,6 +11,7 @@ export interface OAuthEndpoints {
|
||||
tokenUrl: string;
|
||||
clientId?: string;
|
||||
scopes?: string;
|
||||
resource?: string;
|
||||
}
|
||||
|
||||
export interface AuthDetectionResult {
|
||||
@@ -94,7 +95,12 @@ export function extractOAuthEndpoints(error: Error): OAuthEndpoints | null {
|
||||
(obj.default_client_id as string | undefined) ||
|
||||
(obj.public_client_id as string | undefined);
|
||||
|
||||
return { authorizationUrl, tokenUrl, clientId, scopes };
|
||||
const resource =
|
||||
(obj.resource as string | undefined) ||
|
||||
(obj.resource_uri as string | undefined) ||
|
||||
(obj.resourceUri as string | undefined);
|
||||
|
||||
return { authorizationUrl, tokenUrl, clientId, scopes, resource };
|
||||
};
|
||||
|
||||
const clientIdFromAuthUrl = (authorizationUrl: string): string | undefined => {
|
||||
@@ -161,6 +167,7 @@ export function extractOAuthEndpoints(error: Error): OAuthEndpoints | null {
|
||||
challengeValues.get("realm");
|
||||
const tokenUrl =
|
||||
challengeValues.get("token_url") || challengeValues.get("token_uri") || challengeValues.get("token_endpoint");
|
||||
const resource = challengeValues.get("resource") || challengeValues.get("resource_uri");
|
||||
|
||||
if (authorizationUrl && tokenUrl) {
|
||||
return {
|
||||
@@ -168,6 +175,7 @@ export function extractOAuthEndpoints(error: Error): OAuthEndpoints | null {
|
||||
tokenUrl,
|
||||
clientId: challengeValues.get("client_id") || clientIdFromAuthUrl(authorizationUrl),
|
||||
scopes: challengeValues.get("scope") || challengeValues.get("scopes") || scopeFromAuthUrl(authorizationUrl),
|
||||
resource,
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -250,7 +258,7 @@ export async function discoverOAuthEndpoints(
|
||||
serverUrl: string,
|
||||
authServerUrl?: string,
|
||||
resourceMetadataUrl?: string,
|
||||
opts?: { fetch?: FetchImpl },
|
||||
opts?: { fetch?: FetchImpl; protectedResource?: string },
|
||||
): Promise<OAuthEndpoints | null> {
|
||||
const fetchImpl: FetchImpl = opts?.fetch ?? fetch;
|
||||
const wellKnownPaths = [
|
||||
@@ -264,6 +272,8 @@ export async function discoverOAuthEndpoints(
|
||||
const urlsToQuery: string[] = [];
|
||||
const visitedAuthServers = new Set<string>();
|
||||
|
||||
let protectedResource = opts?.protectedResource;
|
||||
|
||||
// Step 1: If a resource_metadata URL was provided, fetch it to discover auth servers.
|
||||
// This follows the RFC 9728 chain: resource_metadata → authorization_servers.
|
||||
if (resourceMetadataUrl && !visitedAuthServers.has(resourceMetadataUrl)) {
|
||||
@@ -276,6 +286,9 @@ export async function discoverOAuthEndpoints(
|
||||
});
|
||||
if (metaResp.ok) {
|
||||
const meta = (await metaResp.json()) as Record<string, unknown>;
|
||||
if (typeof meta.resource === "string" && meta.resource.trim() !== "") {
|
||||
protectedResource = meta.resource;
|
||||
}
|
||||
const authServers = Array.isArray(meta.authorization_servers)
|
||||
? meta.authorization_servers.filter((entry): entry is string => typeof entry === "string")
|
||||
: [];
|
||||
@@ -304,6 +317,8 @@ export async function discoverOAuthEndpoints(
|
||||
const scopesSupported = Array.isArray(metadata.scopes_supported)
|
||||
? metadata.scopes_supported.filter((scope): scope is string => typeof scope === "string").join(" ")
|
||||
: undefined;
|
||||
const resource = typeof metadata.resource === "string" ? metadata.resource : protectedResource;
|
||||
|
||||
return {
|
||||
authorizationUrl: String(metadata.authorization_endpoint),
|
||||
tokenUrl: String(metadata.token_endpoint),
|
||||
@@ -324,12 +339,15 @@ export async function discoverOAuthEndpoints(
|
||||
: typeof metadata.scope === "string"
|
||||
? metadata.scope
|
||||
: undefined),
|
||||
resource,
|
||||
};
|
||||
}
|
||||
|
||||
if (metadata.oauth || metadata.authorization || metadata.auth) {
|
||||
const oauthData = (metadata.oauth || metadata.authorization || metadata.auth) as Record<string, unknown>;
|
||||
if (typeof oauthData.authorization_url === "string" && typeof oauthData.token_url === "string") {
|
||||
const resource = typeof oauthData.resource === "string" ? oauthData.resource : protectedResource;
|
||||
|
||||
return {
|
||||
authorizationUrl: oauthData.authorization_url || String(oauthData.authorizationUrl),
|
||||
tokenUrl: oauthData.token_url || String(oauthData.tokenUrl),
|
||||
@@ -349,6 +367,7 @@ export async function discoverOAuthEndpoints(
|
||||
: typeof oauthData.scope === "string"
|
||||
? oauthData.scope
|
||||
: undefined,
|
||||
resource,
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -378,12 +397,18 @@ export async function discoverOAuthEndpoints(
|
||||
? metadata.authorization_servers.filter((entry): entry is string => typeof entry === "string")
|
||||
: [];
|
||||
|
||||
const discoveredProtectedResource =
|
||||
typeof metadata.resource === "string" && metadata.resource.trim() !== ""
|
||||
? metadata.resource
|
||||
: protectedResource;
|
||||
|
||||
for (const discoveredAuthServer of authServers) {
|
||||
if (visitedAuthServers.has(discoveredAuthServer)) {
|
||||
continue;
|
||||
}
|
||||
const discovered = await discoverOAuthEndpoints(serverUrl, discoveredAuthServer, undefined, {
|
||||
fetch: fetchImpl,
|
||||
protectedResource: discoveredProtectedResource,
|
||||
});
|
||||
if (discovered) return discovered;
|
||||
}
|
||||
|
||||
@@ -98,6 +98,23 @@ function resolveCallbackOptions(config: MCPOAuthConfig): OAuthCallbackFlowOption
|
||||
};
|
||||
}
|
||||
|
||||
function resolveResourceUri(resource: string | undefined): string | undefined {
|
||||
const trimmed = resource?.trim();
|
||||
if (!trimmed) return undefined;
|
||||
if (trimmed !== resource) {
|
||||
throw new Error("OAuth resource URI must not include surrounding whitespace");
|
||||
}
|
||||
|
||||
const parsed = new URL(trimmed);
|
||||
if (parsed.protocol !== "http:" && parsed.protocol !== "https:") {
|
||||
throw new Error("OAuth resource URI must use http or https");
|
||||
}
|
||||
if (parsed.hash) {
|
||||
throw new Error("OAuth resource URI must not include a fragment");
|
||||
}
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
export interface MCPOAuthConfig {
|
||||
/** Authorization endpoint URL */
|
||||
authorizationUrl: string;
|
||||
@@ -115,6 +132,8 @@ export interface MCPOAuthConfig {
|
||||
callbackPort?: number;
|
||||
/** Custom callback path (default: /callback or redirectUri pathname) */
|
||||
callbackPath?: string;
|
||||
/** MCP resource URI for RFC 8707 resource indicators */
|
||||
resource?: string;
|
||||
/** Fetch implementation for token exchange and discovery requests. */
|
||||
fetch?: FetchImpl;
|
||||
}
|
||||
@@ -128,6 +147,7 @@ export class MCPOAuthFlow extends OAuthCallbackFlow {
|
||||
#registeredClientSecret?: string;
|
||||
#codeVerifier?: string;
|
||||
#fetch: FetchImpl;
|
||||
#resource?: string;
|
||||
|
||||
constructor(
|
||||
private config: MCPOAuthConfig,
|
||||
@@ -136,6 +156,9 @@ export class MCPOAuthFlow extends OAuthCallbackFlow {
|
||||
super(ctrl, resolveCallbackOptions(config));
|
||||
this.#resolvedClientId = this.#resolveClientId(config);
|
||||
this.#fetch = config.fetch ?? ctrl.fetch ?? fetch;
|
||||
this.#resource = resolveResourceUri(
|
||||
config.resource ?? this.#resourceFromAuthorizationUrl(config.authorizationUrl),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -157,6 +180,9 @@ export class MCPOAuthFlow extends OAuthCallbackFlow {
|
||||
get registeredClientSecret(): string | undefined {
|
||||
return this.#registeredClientSecret;
|
||||
}
|
||||
get resource(): string | undefined {
|
||||
return this.#resource;
|
||||
}
|
||||
|
||||
async generateAuthUrl(state: string, redirectUri: string): Promise<{ url: string; instructions?: string }> {
|
||||
if (!this.#resolvedClientId) {
|
||||
@@ -176,6 +202,12 @@ export class MCPOAuthFlow extends OAuthCallbackFlow {
|
||||
if (this.config.scopes && !params.get("scope")) {
|
||||
params.set("scope", this.config.scopes);
|
||||
}
|
||||
const existingResource = params.get("resource")?.trim();
|
||||
if (existingResource) {
|
||||
this.#resource = resolveResourceUri(existingResource);
|
||||
} else if (this.#resource) {
|
||||
params.set("resource", this.#resource);
|
||||
}
|
||||
params.set("redirect_uri", redirectUri);
|
||||
params.set("state", state);
|
||||
|
||||
@@ -212,6 +244,9 @@ export class MCPOAuthFlow extends OAuthCallbackFlow {
|
||||
this.#codeVerifier = undefined;
|
||||
|
||||
// Add client secret if provided
|
||||
if (this.#resource) {
|
||||
params.set("resource", this.#resource);
|
||||
}
|
||||
const clientSecret = this.config.clientSecret ?? this.#registeredClientSecret;
|
||||
if (clientSecret) {
|
||||
params.set("client_secret", clientSecret);
|
||||
@@ -285,6 +320,13 @@ export class MCPOAuthFlow extends OAuthCallbackFlow {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
#resourceFromAuthorizationUrl(authorizationUrl: string): string | undefined {
|
||||
try {
|
||||
return new URL(authorizationUrl).searchParams.get("resource") ?? undefined;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Try OAuth dynamic client registration when provider requires a client_id.
|
||||
@@ -407,14 +449,18 @@ export async function refreshMCPOAuthToken(
|
||||
refreshToken: string,
|
||||
clientId?: string,
|
||||
clientSecret?: string,
|
||||
resourceOrOpts?: string | { fetch?: FetchImpl },
|
||||
opts?: { fetch?: FetchImpl },
|
||||
): Promise<OAuthCredentials> {
|
||||
const fetchImpl: FetchImpl = opts?.fetch ?? fetch;
|
||||
const fetchImpl: FetchImpl = (typeof resourceOrOpts === "string" ? opts?.fetch : resourceOrOpts?.fetch) ?? fetch;
|
||||
const resource = typeof resourceOrOpts === "string" ? resourceOrOpts : undefined;
|
||||
const params = new URLSearchParams({
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: refreshToken,
|
||||
});
|
||||
if (clientId) params.set("client_id", clientId);
|
||||
const resolvedResource = resolveResourceUri(resource);
|
||||
if (resolvedResource) params.set("resource", resolvedResource);
|
||||
if (clientSecret) params.set("client_secret", clientSecret);
|
||||
|
||||
const response = await fetchImpl(tokenUrl, {
|
||||
|
||||
@@ -55,6 +55,8 @@ export interface MCPAuthConfig {
|
||||
clientId?: string;
|
||||
/** Client secret — persisted for token refresh */
|
||||
clientSecret?: string;
|
||||
/** MCP resource URI — persisted for OAuth resource indicators during refresh */
|
||||
resource?: string;
|
||||
}
|
||||
|
||||
/** Base server config with shared options */
|
||||
|
||||
@@ -57,6 +57,7 @@ export interface MCPAddWizardOAuthResult {
|
||||
credentialId: string;
|
||||
clientId?: string;
|
||||
clientSecret?: string;
|
||||
resource?: string;
|
||||
}
|
||||
|
||||
interface WizardState {
|
||||
@@ -71,6 +72,7 @@ interface WizardState {
|
||||
oauthClientId: string;
|
||||
oauthClientSecret: string;
|
||||
oauthScopes: string;
|
||||
oauthResource: string;
|
||||
oauthCredentialId: string | null;
|
||||
apiKey: string;
|
||||
authLocation: AuthLocation | null;
|
||||
@@ -101,6 +103,7 @@ export class MCPAddWizard extends Container {
|
||||
oauthClientId: "",
|
||||
oauthClientSecret: "",
|
||||
oauthScopes: "",
|
||||
oauthResource: "",
|
||||
oauthCredentialId: null,
|
||||
apiKey: "",
|
||||
authLocation: null,
|
||||
@@ -122,6 +125,7 @@ export class MCPAddWizard extends Container {
|
||||
clientId: string,
|
||||
clientSecret: string,
|
||||
scopes: string,
|
||||
resource?: string,
|
||||
) => Promise<MCPAddWizardOAuthResult>)
|
||||
| null = null;
|
||||
#onTestConnectionCallback: ((config: MCPServerConfig) => Promise<void>) | null = null;
|
||||
@@ -136,6 +140,7 @@ export class MCPAddWizard extends Container {
|
||||
clientId: string,
|
||||
clientSecret: string,
|
||||
scopes: string,
|
||||
resource?: string,
|
||||
) => Promise<MCPAddWizardOAuthResult>,
|
||||
onTestConnection?: (config: MCPServerConfig) => Promise<void>,
|
||||
onRender?: () => void,
|
||||
@@ -987,6 +992,7 @@ export class MCPAddWizard extends Container {
|
||||
this.#state.oauthTokenUrl = oauth.tokenUrl;
|
||||
this.#state.oauthClientId = oauth.clientId || "";
|
||||
this.#state.oauthScopes = oauth.scopes || "";
|
||||
this.#state.oauthResource = oauth.resource || (this.#state.transport === "stdio" ? "" : this.#state.url);
|
||||
this.#state.authMethod = "oauth";
|
||||
|
||||
this.#contentContainer.clear();
|
||||
@@ -1054,6 +1060,7 @@ export class MCPAddWizard extends Container {
|
||||
type: "oauth",
|
||||
credentialId: this.#state.oauthCredentialId,
|
||||
tokenUrl: this.#state.oauthTokenUrl || undefined,
|
||||
resource: this.#state.oauthResource || undefined,
|
||||
clientId: this.#state.oauthClientId || undefined,
|
||||
clientSecret: this.#state.oauthClientSecret || undefined,
|
||||
};
|
||||
@@ -1081,6 +1088,7 @@ export class MCPAddWizard extends Container {
|
||||
type: "oauth",
|
||||
credentialId: this.#state.oauthCredentialId,
|
||||
tokenUrl: this.#state.oauthTokenUrl || undefined,
|
||||
resource: this.#state.oauthResource || undefined,
|
||||
clientId: this.#state.oauthClientId || undefined,
|
||||
clientSecret: this.#state.oauthClientSecret || undefined,
|
||||
};
|
||||
@@ -1142,12 +1150,14 @@ export class MCPAddWizard extends Container {
|
||||
|
||||
try {
|
||||
// Call OAuth handler
|
||||
const oauthResource = this.#state.oauthResource || (this.#state.transport === "stdio" ? "" : this.#state.url);
|
||||
const oauthResult = await this.#onOAuthCallback(
|
||||
this.#state.oauthAuthUrl,
|
||||
this.#state.oauthTokenUrl,
|
||||
this.#state.oauthClientId,
|
||||
this.#state.oauthClientSecret,
|
||||
this.#state.oauthScopes,
|
||||
oauthResource || undefined,
|
||||
);
|
||||
|
||||
// Store credential ID + any dynamically-registered client credentials,
|
||||
@@ -1155,6 +1165,7 @@ export class MCPAddWizard extends Container {
|
||||
this.#state.oauthCredentialId = oauthResult.credentialId;
|
||||
if (oauthResult.clientId) this.#state.oauthClientId = oauthResult.clientId;
|
||||
if (oauthResult.clientSecret) this.#state.oauthClientSecret = oauthResult.clientSecret;
|
||||
this.#state.oauthResource = oauthResult.resource ?? oauthResource;
|
||||
|
||||
// Show success message
|
||||
this.#contentContainer.clear();
|
||||
@@ -1284,6 +1295,7 @@ export class MCPAddWizard extends Container {
|
||||
type: "oauth",
|
||||
credentialId: this.#state.oauthCredentialId,
|
||||
tokenUrl: this.#state.oauthTokenUrl || undefined,
|
||||
resource: this.#state.oauthResource || undefined,
|
||||
clientId: this.#state.oauthClientId || undefined,
|
||||
clientSecret: this.#state.oauthClientSecret || undefined,
|
||||
};
|
||||
@@ -1312,6 +1324,7 @@ export class MCPAddWizard extends Container {
|
||||
type: "oauth",
|
||||
credentialId: this.#state.oauthCredentialId,
|
||||
tokenUrl: this.#state.oauthTokenUrl || undefined,
|
||||
resource: this.#state.oauthResource || undefined,
|
||||
clientId: this.#state.oauthClientId || undefined,
|
||||
clientSecret: this.#state.oauthClientSecret || undefined,
|
||||
};
|
||||
|
||||
@@ -127,6 +127,7 @@ interface OAuthFlowResult {
|
||||
credentialId: string;
|
||||
clientId?: string;
|
||||
clientSecret?: string;
|
||||
resource?: string;
|
||||
}
|
||||
|
||||
type MCPAddScope = "user" | "project";
|
||||
@@ -490,6 +491,7 @@ export class MCPCommandController {
|
||||
|
||||
try {
|
||||
const oauthClientSecret = finalConfig.oauth?.clientSecret ?? "";
|
||||
const oauthResource = oauth.resource ?? finalConfig.url;
|
||||
const oauthResult = await this.#handleOAuthFlow(
|
||||
oauth.authorizationUrl,
|
||||
oauth.tokenUrl,
|
||||
@@ -499,15 +501,18 @@ export class MCPCommandController {
|
||||
finalConfig.oauth?.callbackPort,
|
||||
finalConfig.oauth?.callbackPath,
|
||||
finalConfig.oauth?.redirectUri,
|
||||
oauthResource,
|
||||
);
|
||||
const persistedClientId = oauthResult.clientId ?? oauth.clientId ?? finalConfig.oauth?.clientId;
|
||||
const persistedClientSecret = oauthResult.clientSecret ?? finalConfig.oauth?.clientSecret;
|
||||
const persistedResource = oauthResult.resource ?? oauthResource;
|
||||
finalConfig = {
|
||||
...finalConfig,
|
||||
auth: {
|
||||
type: "oauth",
|
||||
credentialId: oauthResult.credentialId,
|
||||
tokenUrl: oauth.tokenUrl,
|
||||
resource: persistedResource,
|
||||
clientId: persistedClientId,
|
||||
clientSecret: persistedClientSecret,
|
||||
},
|
||||
@@ -548,8 +553,25 @@ export class MCPCommandController {
|
||||
done();
|
||||
this.#handleWizardCancel();
|
||||
},
|
||||
async (authUrl: string, tokenUrl: string, clientId: string, clientSecret: string, scopes: string) => {
|
||||
return await this.#handleOAuthFlow(authUrl, tokenUrl, clientId, clientSecret, scopes);
|
||||
async (
|
||||
authUrl: string,
|
||||
tokenUrl: string,
|
||||
clientId: string,
|
||||
clientSecret: string,
|
||||
scopes: string,
|
||||
resource?: string,
|
||||
) => {
|
||||
return await this.#handleOAuthFlow(
|
||||
authUrl,
|
||||
tokenUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
scopes,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
resource,
|
||||
);
|
||||
},
|
||||
async (config: MCPServerConfig) => {
|
||||
return await this.#handleTestConnection(config);
|
||||
@@ -579,6 +601,7 @@ export class MCPCommandController {
|
||||
callbackPort?: number,
|
||||
callbackPath?: string,
|
||||
redirectUri?: string,
|
||||
resource?: string,
|
||||
): Promise<OAuthFlowResult> {
|
||||
const authStorage = this.ctx.session.modelRegistry.authStorage;
|
||||
let parsedAuthUrl: URL;
|
||||
@@ -617,6 +640,7 @@ export class MCPCommandController {
|
||||
redirectUri,
|
||||
callbackPort,
|
||||
callbackPath,
|
||||
resource,
|
||||
},
|
||||
{
|
||||
onAuth: (info: { url: string; instructions?: string }) => {
|
||||
@@ -704,6 +728,7 @@ export class MCPCommandController {
|
||||
credentialId,
|
||||
clientId: flow.resolvedClientId,
|
||||
clientSecret: flow.registeredClientSecret,
|
||||
resource: flow.resource,
|
||||
};
|
||||
} catch (error) {
|
||||
const errorMsg = error instanceof Error ? error.message : String(error);
|
||||
@@ -804,6 +829,7 @@ export class MCPCommandController {
|
||||
tokenUrl: string;
|
||||
clientId?: string;
|
||||
scopes?: string;
|
||||
resource?: string;
|
||||
}> {
|
||||
// First test if server actually needs auth by connecting without OAuth
|
||||
let connectionSucceeded = false;
|
||||
@@ -1415,6 +1441,9 @@ export class MCPCommandController {
|
||||
|
||||
this.#showMessage(["", theme.fg("muted", `Reauthorizing "${name}"...`), ""].join("\n"));
|
||||
|
||||
const oauthResource =
|
||||
oauth.resource ?? currentAuth?.resource ?? ("url" in baseConfig ? baseConfig.url : undefined);
|
||||
|
||||
const oauthResult = await this.#handleOAuthFlow(
|
||||
oauth.authorizationUrl,
|
||||
oauth.tokenUrl,
|
||||
@@ -1424,10 +1453,12 @@ export class MCPCommandController {
|
||||
found.config.oauth?.callbackPort,
|
||||
found.config.oauth?.callbackPath,
|
||||
found.config.oauth?.redirectUri,
|
||||
oauthResource,
|
||||
);
|
||||
|
||||
const persistedClientId = oauthResult.clientId ?? oauth.clientId ?? found.config.oauth?.clientId;
|
||||
const persistedClientSecret = oauthResult.clientSecret ?? (oauthClientSecret || undefined);
|
||||
const persistedResource = oauthResult.resource ?? oauthResource;
|
||||
|
||||
const updated: MCPServerConfig = {
|
||||
...baseConfig,
|
||||
@@ -1435,6 +1466,7 @@ export class MCPCommandController {
|
||||
type: "oauth",
|
||||
credentialId: oauthResult.credentialId,
|
||||
tokenUrl: oauth.tokenUrl,
|
||||
resource: persistedResource,
|
||||
clientId: persistedClientId,
|
||||
clientSecret: persistedClientSecret,
|
||||
},
|
||||
|
||||
@@ -77,6 +77,13 @@ describe("MCPManager OAuth refresh failure", () => {
|
||||
const prepared = await manager.prepareConfig(serverConfig);
|
||||
|
||||
expect(refreshSpy).toHaveBeenCalledTimes(1);
|
||||
expect(refreshSpy).toHaveBeenCalledWith(
|
||||
TOKEN_URL,
|
||||
STALE_REFRESH,
|
||||
undefined,
|
||||
undefined,
|
||||
"https://logfire.example.com/mcp",
|
||||
);
|
||||
// The poisoned Bearer must not be re-injected — that is the loop the user
|
||||
// reported (#1908).
|
||||
expect(getAuthorizationHeader(prepared)).toBeUndefined();
|
||||
|
||||
@@ -213,6 +213,7 @@ describe("resource_metadata chain", () => {
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
authorization_servers: ["https://gateway.example.com/my-service"],
|
||||
resource: "https://gateway.example.com/my-service/mcp",
|
||||
}),
|
||||
{ status: 200, headers: { "Content-Type": "application/json" } },
|
||||
);
|
||||
@@ -245,10 +246,60 @@ describe("resource_metadata chain", () => {
|
||||
expect(oauth).toEqual({
|
||||
authorizationUrl: "https://gateway.example.com/my-service/oauth",
|
||||
tokenUrl: "https://gateway.example.com/my-service/token",
|
||||
resource: "https://gateway.example.com/my-service/mcp",
|
||||
});
|
||||
// resource_metadata fetched first
|
||||
expect(calls[0]).toBe("https://gateway.example.com/my-service/.well-known/oauth-protected-resource");
|
||||
});
|
||||
|
||||
it("carries resource from fallback protected-resource discovery", async () => {
|
||||
const calls: string[] = [];
|
||||
const fetchImpl = mockFetch((input: FetchInput) => {
|
||||
const url = String(input);
|
||||
calls.push(url);
|
||||
|
||||
if (url === "https://gateway.example.com/.well-known/oauth-protected-resource") {
|
||||
return new Response("not found", { status: 404 });
|
||||
}
|
||||
if (url === "https://gateway.example.com/my-service/.well-known/oauth-protected-resource") {
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
authorization_servers: ["https://auth.example.com/my-service"],
|
||||
resource: "https://gateway.example.com/my-service/custom-resource",
|
||||
}),
|
||||
{ status: 200, headers: { "Content-Type": "application/json" } },
|
||||
);
|
||||
}
|
||||
if (url === "https://gateway.example.com/.well-known/oauth-authorization-server") {
|
||||
return new Response("not found", { status: 404 });
|
||||
}
|
||||
if (url === "https://gateway.example.com/my-service/.well-known/oauth-authorization-server") {
|
||||
return new Response("not found", { status: 404 });
|
||||
}
|
||||
if (url === "https://auth.example.com/my-service/.well-known/oauth-authorization-server") {
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
authorization_endpoint: "https://auth.example.com/my-service/oauth",
|
||||
token_endpoint: "https://auth.example.com/my-service/token",
|
||||
}),
|
||||
{ status: 200, headers: { "Content-Type": "application/json" } },
|
||||
);
|
||||
}
|
||||
|
||||
return new Response("not found", { status: 404 });
|
||||
});
|
||||
|
||||
const oauth = await discoverOAuthEndpoints("https://gateway.example.com/my-service/mcp", undefined, undefined, {
|
||||
fetch: fetchImpl,
|
||||
});
|
||||
|
||||
expect(oauth).toEqual({
|
||||
authorizationUrl: "https://auth.example.com/my-service/oauth",
|
||||
tokenUrl: "https://auth.example.com/my-service/token",
|
||||
resource: "https://gateway.example.com/my-service/custom-resource",
|
||||
});
|
||||
expect(calls).toContain("https://gateway.example.com/my-service/.well-known/oauth-protected-resource");
|
||||
});
|
||||
});
|
||||
|
||||
describe("relative Mcp-Auth-Server URL", () => {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { afterEach, describe, expect, it, vi } from "bun:test";
|
||||
import type { FetchImpl } from "@oh-my-pi/pi-ai/types";
|
||||
import { MCPOAuthFlow } from "@oh-my-pi/pi-coding-agent/mcp/oauth-flow";
|
||||
import { MCPOAuthFlow, refreshMCPOAuthToken } from "@oh-my-pi/pi-coding-agent/mcp/oauth-flow";
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
@@ -108,6 +108,77 @@ describe("mcp oauth flow", () => {
|
||||
refresh: "refresh-token",
|
||||
});
|
||||
});
|
||||
it("sends MCP resource indicator in authorization and token requests", async () => {
|
||||
let authResource = "";
|
||||
let tokenRequestBody = "";
|
||||
|
||||
const flow = new MCPOAuthFlow(
|
||||
{
|
||||
authorizationUrl: "https://provider.example/authorize",
|
||||
tokenUrl: "https://provider.example/token",
|
||||
clientId: "client-id",
|
||||
resource: "https://mcp.example.com/mcp",
|
||||
callbackPort: 14572,
|
||||
fetch: mockProviderTokenEndpoint(body => {
|
||||
tokenRequestBody = body;
|
||||
}),
|
||||
},
|
||||
{
|
||||
onAuth: info => {
|
||||
const authUrl = new URL(info.url);
|
||||
authResource = authUrl.searchParams.get("resource") ?? "";
|
||||
const redirectUri = authUrl.searchParams.get("redirect_uri") ?? "";
|
||||
const state = authUrl.searchParams.get("state") ?? "";
|
||||
queueMicrotask(() => {
|
||||
void fetch(`${redirectUri}?code=test-code&state=${state}`);
|
||||
});
|
||||
},
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
},
|
||||
);
|
||||
|
||||
await flow.login();
|
||||
const tokenParams = new URLSearchParams(tokenRequestBody);
|
||||
|
||||
expect(authResource).toBe("https://mcp.example.com/mcp");
|
||||
expect(tokenParams.get("resource")).toBe("https://mcp.example.com/mcp");
|
||||
});
|
||||
it("uses an authorization URL resource for the matching token request", async () => {
|
||||
let authResource = "";
|
||||
let tokenRequestBody = "";
|
||||
|
||||
const flow = new MCPOAuthFlow(
|
||||
{
|
||||
authorizationUrl:
|
||||
"https://provider.example/authorize?resource=https%3A%2F%2Fauth-url-resource.example%2Fmcp",
|
||||
tokenUrl: "https://provider.example/token",
|
||||
clientId: "client-id",
|
||||
resource: "https://config-resource.example/mcp",
|
||||
callbackPort: 14573,
|
||||
fetch: mockProviderTokenEndpoint(body => {
|
||||
tokenRequestBody = body;
|
||||
}),
|
||||
},
|
||||
{
|
||||
onAuth: info => {
|
||||
const authUrl = new URL(info.url);
|
||||
authResource = authUrl.searchParams.get("resource") ?? "";
|
||||
const redirectUri = authUrl.searchParams.get("redirect_uri") ?? "";
|
||||
const state = authUrl.searchParams.get("state") ?? "";
|
||||
queueMicrotask(() => {
|
||||
void fetch(`${redirectUri}?code=test-code&state=${state}`);
|
||||
});
|
||||
},
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
},
|
||||
);
|
||||
|
||||
await flow.login();
|
||||
const tokenParams = new URLSearchParams(tokenRequestBody);
|
||||
|
||||
expect(authResource).toBe("https://auth-url-resource.example/mcp");
|
||||
expect(tokenParams.get("resource")).toBe("https://auth-url-resource.example/mcp");
|
||||
});
|
||||
|
||||
it("uses exact redirectUri and clientSecret for provider requests", async () => {
|
||||
let observedRedirectUri = "";
|
||||
@@ -367,6 +438,7 @@ describe("mcp oauth flow", () => {
|
||||
},
|
||||
onManualCodeInput: async () => {
|
||||
const authUrl = new URL(manualAuthUrl);
|
||||
|
||||
const redirectUri = authUrl.searchParams.get("redirect_uri") ?? "";
|
||||
const state = authUrl.searchParams.get("state") ?? "";
|
||||
return `${redirectUri}?code=manual-code&state=${encodeURIComponent(state)}`;
|
||||
@@ -381,4 +453,39 @@ describe("mcp oauth flow", () => {
|
||||
expect(credentials.access).toBe("access-token");
|
||||
expect(tokenParams.get("code")).toBe("manual-code");
|
||||
});
|
||||
|
||||
it("sends MCP resource indicator when refreshing tokens", async () => {
|
||||
let tokenRequestBody = "";
|
||||
|
||||
const credentials = await refreshMCPOAuthToken(
|
||||
"https://provider.example/token",
|
||||
"refresh-token",
|
||||
"client-id",
|
||||
"client-secret",
|
||||
"https://mcp.example.com/mcp",
|
||||
{
|
||||
fetch: mockProviderTokenEndpoint(body => {
|
||||
tokenRequestBody = body;
|
||||
}),
|
||||
},
|
||||
);
|
||||
const tokenParams = new URLSearchParams(tokenRequestBody);
|
||||
|
||||
expect(credentials.access).toBe("access-token");
|
||||
expect(tokenParams.get("grant_type")).toBe("refresh_token");
|
||||
expect(tokenParams.get("resource")).toBe("https://mcp.example.com/mcp");
|
||||
});
|
||||
it("keeps the legacy refresh options position when no resource is provided", async () => {
|
||||
let tokenRequestBody = "";
|
||||
|
||||
await refreshMCPOAuthToken("https://provider.example/token", "refresh-token", undefined, undefined, {
|
||||
fetch: mockProviderTokenEndpoint(body => {
|
||||
tokenRequestBody = body;
|
||||
}),
|
||||
});
|
||||
const tokenParams = new URLSearchParams(tokenRequestBody);
|
||||
|
||||
expect(tokenParams.get("grant_type")).toBe("refresh_token");
|
||||
expect(tokenParams.get("resource")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user