Skip to content

[auth] refactor resource selection to not include resource if PRM is not present #693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
231 changes: 225 additions & 6 deletions src/client/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -954,10 +954,19 @@ describe("OAuth Authorization", () => {
});

it("passes resource parameter through authorization flow", async () => {
// Mock successful metadata discovery
// Mock successful metadata discovery - need to include protected resource metadata
mockFetch.mockImplementation((url) => {
const urlString = url.toString();
if (urlString.includes("/.well-known/oauth-authorization-server")) {
if (urlString.includes("/.well-known/oauth-protected-resource")) {
return Promise.resolve({
ok: true,
status: 200,
json: async () => ({
resource: "https://api.example.com/mcp-server",
authorization_servers: ["https://auth.example.com"],
}),
});
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
return Promise.resolve({
ok: true,
status: 200,
Expand Down Expand Up @@ -1002,11 +1011,20 @@ describe("OAuth Authorization", () => {
});

it("includes resource in token exchange when authorization code is provided", async () => {
// Mock successful metadata discovery and token exchange
// Mock successful metadata discovery and token exchange - need protected resource metadata
mockFetch.mockImplementation((url) => {
const urlString = url.toString();

if (urlString.includes("/.well-known/oauth-authorization-server")) {
if (urlString.includes("/.well-known/oauth-protected-resource")) {
return Promise.resolve({
ok: true,
status: 200,
json: async () => ({
resource: "https://api.example.com/mcp-server",
authorization_servers: ["https://auth.example.com"],
}),
});
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
return Promise.resolve({
ok: true,
status: 200,
Expand Down Expand Up @@ -1062,11 +1080,20 @@ describe("OAuth Authorization", () => {
});

it("includes resource in token refresh", async () => {
// Mock successful metadata discovery and token refresh
// Mock successful metadata discovery and token refresh - need protected resource metadata
mockFetch.mockImplementation((url) => {
const urlString = url.toString();

if (urlString.includes("/.well-known/oauth-authorization-server")) {
if (urlString.includes("/.well-known/oauth-protected-resource")) {
return Promise.resolve({
ok: true,
status: 200,
json: async () => ({
resource: "https://api.example.com/mcp-server",
authorization_servers: ["https://auth.example.com"],
}),
});
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
return Promise.resolve({
ok: true,
status: 200,
Expand Down Expand Up @@ -1244,5 +1271,197 @@ describe("OAuth Authorization", () => {
// Should use the PRM's resource value, not the full requested URL
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/");
});

it("excludes resource parameter when Protected Resource Metadata is not present", async () => {
// Mock metadata discovery where protected resource metadata is not available (404)
// but authorization server metadata is available
mockFetch.mockImplementation((url) => {
const urlString = url.toString();

if (urlString.includes("/.well-known/oauth-protected-resource")) {
// Protected resource metadata not available
return Promise.resolve({
ok: false,
status: 404,
});
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
return Promise.resolve({
ok: true,
status: 200,
json: async () => ({
issuer: "https://auth.example.com",
authorization_endpoint: "https://auth.example.com/authorize",
token_endpoint: "https://auth.example.com/token",
response_types_supported: ["code"],
code_challenge_methods_supported: ["S256"],
}),
});
}

return Promise.resolve({ ok: false, status: 404 });
});

// Mock provider methods
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
});
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
(mockProvider.redirectToAuthorization as jest.Mock).mockResolvedValue(undefined);

// Call auth - should not include resource parameter
const result = await auth(mockProvider, {
serverUrl: "https://api.example.com/mcp-server",
});

expect(result).toBe("REDIRECT");

// Verify the authorization URL does NOT include the resource parameter
expect(mockProvider.redirectToAuthorization).toHaveBeenCalledWith(
expect.objectContaining({
searchParams: expect.any(URLSearchParams),
})
);

const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
const authUrl: URL = redirectCall[0];
// Resource parameter should not be present when PRM is not available
expect(authUrl.searchParams.has("resource")).toBe(false);
});

it("excludes resource parameter in token exchange when Protected Resource Metadata is not present", async () => {
// Mock metadata discovery - no protected resource metadata, but auth server metadata available
mockFetch.mockImplementation((url) => {
const urlString = url.toString();

if (urlString.includes("/.well-known/oauth-protected-resource")) {
return Promise.resolve({
ok: false,
status: 404,
});
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
return Promise.resolve({
ok: true,
status: 200,
json: async () => ({
issuer: "https://auth.example.com",
authorization_endpoint: "https://auth.example.com/authorize",
token_endpoint: "https://auth.example.com/token",
response_types_supported: ["code"],
code_challenge_methods_supported: ["S256"],
}),
});
} else if (urlString.includes("/token")) {
return Promise.resolve({
ok: true,
status: 200,
json: async () => ({
access_token: "access123",
token_type: "Bearer",
expires_in: 3600,
refresh_token: "refresh123",
}),
});
}

return Promise.resolve({ ok: false, status: 404 });
});

// Mock provider methods for token exchange
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
});
(mockProvider.codeVerifier as jest.Mock).mockResolvedValue("test-verifier");
(mockProvider.saveTokens as jest.Mock).mockResolvedValue(undefined);

// Call auth with authorization code
const result = await auth(mockProvider, {
serverUrl: "https://api.example.com/mcp-server",
authorizationCode: "auth-code-123",
});

expect(result).toBe("AUTHORIZED");

// Find the token exchange call
const tokenCall = mockFetch.mock.calls.find(call =>
call[0].toString().includes("/token")
);
expect(tokenCall).toBeDefined();

const body = tokenCall![1].body as URLSearchParams;
// Resource parameter should not be present when PRM is not available
expect(body.has("resource")).toBe(false);
expect(body.get("code")).toBe("auth-code-123");
});

it("excludes resource parameter in token refresh when Protected Resource Metadata is not present", async () => {
// Mock metadata discovery - no protected resource metadata, but auth server metadata available
mockFetch.mockImplementation((url) => {
const urlString = url.toString();

if (urlString.includes("/.well-known/oauth-protected-resource")) {
return Promise.resolve({
ok: false,
status: 404,
});
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
return Promise.resolve({
ok: true,
status: 200,
json: async () => ({
issuer: "https://auth.example.com",
authorization_endpoint: "https://auth.example.com/authorize",
token_endpoint: "https://auth.example.com/token",
response_types_supported: ["code"],
code_challenge_methods_supported: ["S256"],
}),
});
} else if (urlString.includes("/token")) {
return Promise.resolve({
ok: true,
status: 200,
json: async () => ({
access_token: "new-access123",
token_type: "Bearer",
expires_in: 3600,
}),
});
}

return Promise.resolve({ ok: false, status: 404 });
});

// Mock provider methods for token refresh
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
client_id: "test-client",
client_secret: "test-secret",
});
(mockProvider.tokens as jest.Mock).mockResolvedValue({
access_token: "old-access",
refresh_token: "refresh123",
});
(mockProvider.saveTokens as jest.Mock).mockResolvedValue(undefined);

// Call auth with existing tokens (should trigger refresh)
const result = await auth(mockProvider, {
serverUrl: "https://api.example.com/mcp-server",
});

expect(result).toBe("AUTHORIZED");

// Find the token refresh call
const tokenCall = mockFetch.mock.calls.find(call =>
call[0].toString().includes("/token")
);
expect(tokenCall).toBeDefined();

const body = tokenCall![1].body as URLSearchParams;
// Resource parameter should not be present when PRM is not available
expect(body.has("resource")).toBe(false);
expect(body.get("grant_type")).toBe("refresh_token");
expect(body.get("refresh_token")).toBe("refresh123");
});
});
});
27 changes: 16 additions & 11 deletions src/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,19 +198,24 @@ export async function auth(
}

export async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
let resource = resourceUrlFromServerUrl(serverUrl);
const defaultResource = resourceUrlFromServerUrl(serverUrl);

// If provider has custom validation, delegate to it
if (provider.validateResourceURL) {
return await provider.validateResourceURL(resource, resourceMetadata?.resource);
} else if (resourceMetadata) {
if (checkResourceAllowed({ requestedResource: resource, configuredResource: resourceMetadata.resource })) {
// If the resource mentioned in metadata is valid, prefer it since it is what the server is telling us to request.
resource = new URL(resourceMetadata.resource);
} else {
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${resource} (or origin)`);
}
return await provider.validateResourceURL(defaultResource, resourceMetadata?.resource);
}

// Only include resource parameter when Protected Resource Metadata is present
if (!resourceMetadata) {
return undefined;
}

return resource;
// Validate that the metadata's resource is compatible with our request
if (!checkResourceAllowed({ requestedResource: defaultResource, configuredResource: resourceMetadata.resource })) {
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin)`);
}
// Prefer the resource from metadata since it's what the server is telling us to request
return new URL(resourceMetadata.resource);
}

/**
Expand Down Expand Up @@ -360,7 +365,7 @@ export async function discoverOAuthMetadata(
try {
const rootUrl = new URL("/.well-known/oauth-authorization-server", issuer);
response = await tryMetadataDiscovery(rootUrl, protocolVersion);

if (response.status === 404) {
return undefined;
}
Expand Down