Skip to content
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
5 changes: 5 additions & 0 deletions .changeset/mighty-badgers-vanish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"agents": patch
---

Fix OAuth state parameter security vulnerability by replacing client_id with secure random tokens
7 changes: 5 additions & 2 deletions packages/agents/src/mcp/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,13 @@ export class MCPClientManager {
);
}
const code = url.searchParams.get("code");
const clientId = url.searchParams.get("state");
const state = url.searchParams.get("state");
const urlParams = urlMatch.split("/");
const serverId = urlParams[urlParams.length - 1];
if (!code) {
throw new Error("Unauthorized: no code provided");
}
if (!clientId) {
if (!state) {
throw new Error("Unauthorized: no state provided");
}

Expand All @@ -228,6 +228,9 @@ export class MCPClientManager {
);
}

// Get clientId from auth provider (stored during redirectToAuthorization) or fallback to state for backward compatibility
const clientId = conn.options.transport.authProvider.clientId || state;

// Set the OAuth credentials
conn.options.transport.authProvider.clientId = clientId;
conn.options.transport.authProvider.serverId = serverId;
Expand Down
10 changes: 4 additions & 6 deletions packages/agents/src/mcp/do-oauth-client-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
OAuthClientMetadata,
OAuthTokens
} from "@modelcontextprotocol/sdk/shared/auth.js";
import { nanoid } from "nanoid";

// A slight extension to the standard OAuthClientProvider interface because `redirectToAuthorization` doesn't give us the interface we need
// This allows us to track authentication for a specific server and associated dynamic client registration
Expand Down Expand Up @@ -122,12 +123,9 @@ export class DurableObjectOAuthClientProvider implements AgentsOAuthProvider {
* and require user interact to initiate the redirect flow
*/
async redirectToAuthorization(authUrl: URL): Promise<void> {
// We want to track the client ID in state here because the typescript SSE client sometimes does
// a dynamic client registration AFTER generating this redirect URL.
const client_id = authUrl.searchParams.get("client_id");
if (client_id) {
authUrl.searchParams.append("state", client_id);
}
// Generate secure random token for state parameter
const stateToken = nanoid();
authUrl.searchParams.set("state", stateToken);
this._authUrl_ = authUrl.toString();
}

Expand Down
Loading