inspector icon indicating copy to clipboard operation
inspector copied to clipboard

When using Streamable HTTP transport, the oauth flow is not triggered when the server returns 401

Open ashubham opened this issue 8 months ago • 1 comments

Describe the bug When using Streamable HTTP transport, the oauth flow is not triggered when the server returns 401. The sse transport supports this and the browser correctly initiates the oauth flow as per the spec.

To Reproduce Steps to reproduce the behavior:

  1. Point against an MCP server with Streamable HTTP + Oauth flow. Like this one (https://thoughtspot-mcp-server.thoughtspot-485.workers.dev/mcp)

Expected behavior When the server returns 401 the inspector should trigger the oauth flow.

Logs When using streamable HTTP:

Created streamable web app transport 9b453c43-3a2c-4e67-a18f-29824475847e
Error from MCP server: Error: Error POSTing to endpoint (HTTP 401)
-- the inspector hangs here --

When using sse:

SSE transport: url=<server sse endpoint>, headers=Accept
Received 401 Unauthorized from MCP server: SSE error: Non-200 status code (401)
-- the oauth flow is triggered ---

Additional context There is discussion on this in #339, and is identified as a follow up task.

ashubham avatar Apr 29 '25 21:04 ashubham

I am also experiencing this issue (i.e no authorization flow for streamable http). I did a little digging into the code. Hopefully this will help find the root cause and will help the authors of the StreamableHTTPClientTransport to make a fix.

Looking at the sse case in the SSEClientTransport class the start function calls _startOrAuth function which checks for 401 and throws an exception that gets picked up in index.ts

    async start() {
        if (this._eventSource) {
            throw new Error("SSEClientTransport already started! If using Client class, note that connect() calls start() automatically.");
        }
        return await this._startOrAuth();
    }

exception get caught here in index.ts

index.ts code

app.get("/sse", async (req, res) => {
  try {
    console.log(
      "New SSE connection. NOTE: The sse transport is deprecated and has been replaced by streamable-http",
    );

    try {
      await backingServerTransport?.close();
      backingServerTransport = await createTransport(req);
    } catch (error) {
      if (error instanceof SseError && error.code === 401) {
        console.error(
          "Received 401 Unauthorized from MCP server:",
          error.message,
        );
        res.status(401).json(error);
        return;
      }

      throw error;
    }

However, the StreamableHTTPClientTransport also has a start function but that doesnt call the startOrAuth so the exception to send back 401 to the client is never thrown. There is a funtion called _startOrAuthSse that appears to be used elsewhere in the StreamableHTTPClientTransport but possibly the authors have overlooked the initial 401 exception.

    async start() {
        if (this._abortController) {
            throw new Error("StreamableHTTPClientTransport already started! If using Client class, note that connect() calls start() automatically.");
        }
        this._abortController = new AbortController();
    }

and therefore the exception never gets thrown so the 401 is never returned to the client

index.ts code

    try {
      console.log("New streamable-http connection");
      try {
        await backingServerTransport?.close();
        backingServerTransport = await createTransport(req);
      } catch (error) {
        if (error instanceof SseError && error.code === 401) {
          console.error(
            "Received 401 Unauthorized from MCP server:",
            error.message,
          );
          res.status(401).json(error);
          return;
        }

        throw error;
      }

oidebrett avatar May 06 '25 21:05 oidebrett

I did some more investigation and I think this is an easy fix but I think its dependent on the mcp sdk streamablehttp. I am just capturing my investigation here.

  1. The start function in node_modules/@modelcontextprotocol/sdk/dist/esm/client/streamableHttp.js

should have the following lines instead of this._abortController = new AbortController();

    async start() {
        if (this._abortController) {
            throw new Error("StreamableHTTPClientTransport already started! If using Client class, note that connect() calls start() automatically.");
        }
        try {
            await this._startOrAuthSse({ resumptionToken: undefined })
        }
        catch (error) {
            throw new StreamableHTTPError(error.code, error.message);
        }
        //this._abortController = new AbortController();
    }

  1. the index.ts (server/src/index.ts) file should pick up the StreamableHttpError so that the 401 is returned to the client
app.post("/mcp", async (req, res) => {
  const sessionId = req.headers["mcp-session-id"] as string | undefined;
  console.log(`Received POST message for sessionId ${sessionId}`);
  if (!sessionId) {
    try {
      console.log("New streamable-http connection");
      try {
        await backingServerTransport?.close();
        backingServerTransport = await createTransport(req);
      } catch (error) {
        if ((error instanceof SseError || error instanceof StreamableHTTPError) && error.code === 401) {
          console.error(
            "Received 401 Unauthorized from MCP server:",
            error.message,
          );
          res.status(401).json(error);
          return;
        }

        throw error;
      }

I got it Oauth flow to kick off now but now theres a known bug in how the inspector is query for the Oauth meta data. Theres already an issue raised on this here: https://github.com/modelcontextprotocol/inspector/issues/390

I am happy to do create a PR once the above issue is fixed so that I can do a full end to end test. My knowledge of the new streamable http is only recent so maybe I am missing something so I would appreciate if theres an expert on streamable http and authentication that could verify if this fix is ok.

oidebrett avatar May 11 '25 16:05 oidebrett

I havent played with this new commit https://github.com/modelcontextprotocol/inspector/pull/355 but I think it may solve this problem. I will test it on more streamable http use cases.

oidebrett avatar May 15 '25 20:05 oidebrett

I believe this issue was resolved in a released version of Inspector, please re-open if anyone is still seeing this issue with the latest version.

olaservo avatar May 30 '25 12:05 olaservo