mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-03 14:50:19 +01:00
🔗 fix: Normalize MCP OAuth resource parameter to match token exchange (#12018)
* 🔗 fix: Normalize MCP OAuth `resource` parameter to match token exchange The authorization request used the raw resource string from metadata while the token exchange normalized it through `new URL().href`, causing a trailing-slash mismatch that Cloudflare's auth server rejected. Canonicalize the resource URL in both paths so they match. * 🔧 test: Simplify LeaderElection integration tests for Redis Refactored the integration tests for LeaderElection with Redis by reducing the number of instances from 100 to 1, streamlining the leadership election process. Updated assertions to verify leadership status and UUID after resignation, improving test clarity and performance. Adjusted timeout to 15 seconds for the single instance scenario. * 🔧 test: Update LeaderElection test case description for clarity Modified the description of the test case for leader resignation in the LeaderElection integration tests to better reflect the expected behavior, enhancing clarity and understanding of the test's purpose. * refactor: `resource` parameter in MCP OAuth authorization URL Updated the `MCPOAuthHandler` to ensure the `resource` parameter is added to the authorization URL even when an error occurs while retrieving it from metadata. This change improves the handling of invalid resource URLs by using the raw value as a fallback, enhancing the robustness of the authorization process.
This commit is contained in:
parent
36e37003c9
commit
a0a1749151
2 changed files with 26 additions and 31 deletions
|
|
@ -49,39 +49,24 @@ describe('LeaderElection with Redis', () => {
|
|||
});
|
||||
|
||||
describe('Test Case 1: Simulate shutdown of the leader', () => {
|
||||
it('should elect a new leader after the current leader resigns', async () => {
|
||||
// Create 100 instances
|
||||
instances = Array.from({ length: 100 }, () => new LeaderElection());
|
||||
it('should allow an instance to re-elect itself after resignation', async () => {
|
||||
const instance = new LeaderElection();
|
||||
instances.push(instance);
|
||||
|
||||
// Call isLeader on all instances and get leadership status
|
||||
const resultsWithInstances = await Promise.all(
|
||||
instances.map(async (instance) => ({
|
||||
instance,
|
||||
isLeader: await instance.isLeader(),
|
||||
})),
|
||||
);
|
||||
|
||||
// Find leader and followers
|
||||
const leaders = resultsWithInstances.filter((r) => r.isLeader);
|
||||
const followers = resultsWithInstances.filter((r) => !r.isLeader);
|
||||
const leader = leaders[0].instance;
|
||||
const nextLeader = followers[0].instance;
|
||||
|
||||
// Verify only one is leader
|
||||
expect(leaders.length).toBe(1);
|
||||
|
||||
// Verify getLeaderUUID matches the leader's UUID
|
||||
expect(await LeaderElection.getLeaderUUID()).toBe(leader.UUID);
|
||||
// Instance becomes leader
|
||||
expect(await instance.isLeader()).toBe(true);
|
||||
expect(await LeaderElection.getLeaderUUID()).toBe(instance.UUID);
|
||||
|
||||
// Leader resigns
|
||||
await leader.resign();
|
||||
await instance.resign();
|
||||
|
||||
// Verify getLeaderUUID returns null after resignation
|
||||
// Verify leadership key is cleared after resignation
|
||||
expect(await LeaderElection.getLeaderUUID()).toBeNull();
|
||||
|
||||
// Next instance to call isLeader should become the new leader
|
||||
expect(await nextLeader.isLeader()).toBe(true);
|
||||
}, 30000); // 30 second timeout for 100 instances
|
||||
// Instance can re-elect itself after resignation
|
||||
expect(await instance.isLeader()).toBe(true);
|
||||
expect(await LeaderElection.getLeaderUUID()).toBe(instance.UUID);
|
||||
}, 15000);
|
||||
});
|
||||
|
||||
describe('Test Case 2: Simulate crash of the leader', () => {
|
||||
|
|
|
|||
|
|
@ -529,10 +529,20 @@ export class MCPOAuthHandler {
|
|||
logger.debug(`[MCPOAuth] Added state parameter to authorization URL`);
|
||||
|
||||
if (resourceMetadata?.resource != null && resourceMetadata.resource) {
|
||||
authorizationUrl.searchParams.set('resource', resourceMetadata.resource);
|
||||
logger.debug(
|
||||
`[MCPOAuth] Added resource parameter to authorization URL: ${resourceMetadata.resource}`,
|
||||
);
|
||||
try {
|
||||
const canonicalResource = new URL(resourceMetadata.resource).href;
|
||||
authorizationUrl.searchParams.set('resource', canonicalResource);
|
||||
logger.debug(
|
||||
`[MCPOAuth] Added resource parameter to authorization URL: ${canonicalResource}`,
|
||||
);
|
||||
} catch (error) {
|
||||
authorizationUrl.searchParams.set('resource', resourceMetadata.resource);
|
||||
logger.error(
|
||||
`[MCPOAuth] Invalid resource URL from metadata for ${serverName}: ` +
|
||||
`'${resourceMetadata.resource}'. Using raw value as fallback.`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
logger.warn(
|
||||
`[MCPOAuth] Resource metadata missing 'resource' property for ${serverName}. ` +
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue