inspector icon indicating copy to clipboard operation
inspector copied to clipboard

Fix to the Authorization header bug for Streamable-HTTP

Open santthosh opened this issue 8 months ago • 4 comments

Fixed the Streamable HTTP transport to properly propagate authorization headers by including transport options in the StreamableHTTPClientTransport initialization.

Motivation and Context

Bug referenced in 369

The Streamable HTTP transport was not properly passing authorization headers to the server, resulting in 401 Unauthorized errors. This was happening because the transport options containing the authorization headers were not being passed to the StreamableHTTPClientTransport constructor. This fix ensures that authentication headers are properly propagated through the Streamable HTTP transport, maintaining consistent authentication behavior across all transport types.

How Has This Been Tested?

  • Tested with a Streamable HTTP connection using bearer token authentication
  • Verified that authorization headers are properly passed through to the server
  • Confirmed that 401 Unauthorized errors are no longer occurring when valid authentication is provided

Successfully connected after screenshot

Screenshot 2025-04-30 at 10 30 53 PM

Logs (after fix)

[1] 2025-05-01T05:30:34.803Z router dispatching OPTIONS /mcp?url=https%3A%2F%2Fdevelopment.gateway.cloudmcp.dev%2Fmcp-gateway-test%2Fmcp&transportType=streamable-http
[1] 2025-05-01T05:30:34.803Z router corsMiddleware  : /mcp?url=https%3A%2F%2Fdevelopment.gateway.cloudmcp.dev%2Fmcp-gateway-test%2Fmcp&transportType=streamable-http
[1] 2025-05-01T05:30:34.804Z router dispatching POST /mcp?url=https%3A%2F%2Fdevelopment.gateway.cloudmcp.dev%2Fmcp-gateway-test%2Fmcp&transportType=streamable-http
[1] 2025-05-01T05:30:34.804Z router corsMiddleware  : /mcp?url=https%3A%2F%2Fdevelopment.gateway.cloudmcp.dev%2Fmcp-gateway-test%2Fmcp&transportType=streamable-http
[1] 2025-05-01T05:30:34.804Z router <anonymous>  : /mcp?url=https%3A%2F%2Fdevelopment.gateway.cloudmcp.dev%2Fmcp-gateway-test%2Fmcp&transportType=streamable-http
[1] Received POST message for sessionId f91063e9-35b4-43af-bc32-37c590dfc7a3
[1] 2025-05-01T05:30:34.806Z router dispatching GET /mcp?url=https%3A%2F%2Fdevelopment.gateway.cloudmcp.dev%2Fmcp-gateway-test%2Fmcp&transportType=streamable-http
[1] 2025-05-01T05:30:34.807Z router corsMiddleware  : /mcp?url=https%3A%2F%2Fdevelopment.gateway.cloudmcp.dev%2Fmcp-gateway-test%2Fmcp&transportType=streamable-http
[1] 2025-05-01T05:30:34.807Z router <anonymous>  : /mcp?url=https%3A%2F%2Fdevelopment.gateway.cloudmcp.dev%2Fmcp-gateway-test%2Fmcp&transportType=streamable-http
[1] Received GET message for sessionId f91063e9-35b4-43af-bc32-37c590dfc7a3

Breaking Changes

No breaking changes. This is a bug fix that maintains backward compatibility while fixing the authorization header propagation issue.

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Documentation update

Checklist

  • [x] I have read the MCP Documentation
  • [x] My code follows the repository's style guidelines
  • [x] New and existing tests pass locally
  • [ ] I have added appropriate error handling
  • [ ] I have added or updated documentation as needed

santthosh avatar May 01 '25 05:05 santthosh

Is there an opportunity here to be more type-specific about the transport options for each transport? (i.e. StreamableHTTPClientTransportOptions, SSEClientTransportOptions)

It appears that the SDK will handle either gracefully so that may be overkill (and not necessary to actually fix the issue).

Actually, good call on this. The options we are sending in right now match either of those interfaces. However,

  • Both fail to include authProvider: serverAuthProvider even though we create it and the transport would like to see it.
  • StreamableHTTPClientTransportOptions has additional reconnectionOptions and sessionId properties. I feel like at a minimum we want to send an object with default reconnectionOptions settings, with a TODO to expose them in configuration.

I have a patch to this PR that I want to make, but am having trouble testing with @max-stytch's Todo app. I can't get that one to work rn.

cliffhall avatar May 02 '25 21:05 cliffhall

Awesome @cliffhall I'm going to test this with my service locally and post results

santthosh avatar May 02 '25 22:05 santthosh

@cliffhall - Did a full retest with and without the Authorization header and Bearer Token and worked great. Also verified all the unit tests passed locally. Thanks for the changes.

santthosh avatar May 03 '25 01:05 santthosh

Do we know if this also fixes/mitigates this? https://github.com/modelcontextprotocol/typescript-sdk/issues/436

evalstate avatar May 03 '25 08:05 evalstate

LLC

Code w

Thaikoma avatar May 08 '25 11:05 Thaikoma

Do we know if this also fixes/mitigates this? modelcontextprotocol/typescript-sdk#436

@evalstate It does something very similar to the workaround you posted there. I do think that that the suggestion in that issue (to include a spread of ...this.requestInit.headers in the returned HeadersInit object in sse.ts as it is done in streamableHttp.ts is correct and should be done directly in the SDK. Problem being that if the Inspector is doing what amounts to your workaround, it will be masking a problem in the SDK that will show up in the field with clients that don't do the same.

cliffhall avatar May 08 '25 16:05 cliffhall