Pass AuthInfo from middleware to Mcp Server + Error handling

[allenzhou101]

A common use case when building Mcp Servers is to pass the auth info from the OAuth token through to the Mcp Server tool call. This PR adjust the middleware to set the auth object on the request so that it's automatically handled, as implemented in the Anthropic Mcp TS SDK here and here.

Also, this PR makes the error handling of the withMcpAuth function more robust, accounting for insufficient scopes, etc. The return type of the withMcpAuth function is adjusted to be the AuthInfo object instead of boolean to support this case.

[quuu]

hey @allenzhou101 thanks for the PR! looks good codewise, and checked out the linked PR's -- makes sense!

wondering if you have a recommended way to minimally test this? when i try this out -- I don't see the global auth being passed in on the request?

[allenzhou101]

Hey @quuu! Thanks for the review!

I tested it using this PR/branch of the MCP Inspector that supports the draft auth spec: modelcontextprotocol/inspector#469

On the MCP Server side I used Descope as the OAuth IdP. One weird IdP-specific note was that the AS Issuer included in the Protected Resource Metadata was used to construct the Authorization Server Metadata weirdly, as mentioned here, but there's a straightforward workaround.

[allenzhou101]

And I focused on testing Streamable HTTP in this PR. Also, here's the server implementation I'm testing with: https://github.com/allenzhou101/mcp-server-nextjs

[allenzhou101]

Switched from resource metadata param from path to url since the prm spec doesn't mandate that the url be relative to the root/origin.

Also, wonder if the types from the Anthropic MCP TS SDK should be exported out of this to the fullest extent possible so only installing this package is required, even for more advanced use cases.

[allenzhou101]

Probably the adapter should also handle hosting the PRM well known, but can be a different PR

[quuu]

Probably the adapter should also handle hosting the PRM well known, but can be a different PR

i was wondering this too, but to abide by the .well-known/... pattern it wouldn't be that easy to route everything together through this

overall this looks pretty good! was wondering if the AuthInfo type should be a generic? does the shape of the auth object changes based on the underlying auth provider?

[allenzhou101]

overall this looks pretty good! was wondering if the AuthInfo type should be a generic? does the shape of the auth object changes based on the underlying auth provider?

It should be agnostic to any auth provider, since it really just requires the token and client id and these should exist for any provider--the client id may just be hardcoded or dynamic based on client. scopes can easily be left empty or not based on the type of token or existence of scopes.

[quuu]

overall this looks pretty good! was wondering if the AuthInfo type should be a generic? does the shape of the auth object changes based on the underlying auth provider?

It should be agnostic to any auth provider, since it really just requires the token and client id and these should exist for any provider--the client id may just be hardcoded or dynamic based on client. scopes can easily be left empty or not based on the type of token or existence of scopes.

makes sense -- looks great!

I just merged another PR into main: #39

and it had some conflicts, mind rebasing on top of it -- specifically to add in your more robust error handling?

[allenzhou101]

@quuu Interesting. Why the choice in #39 to keep only the req param instead of token in the verifyToken function? If anything, I would think that the function should accept token only, since the MCP spec mandates using the Authorization header: https://modelcontextprotocol.io/specification/2025-03-26/basic/authorization#2-6-1-token-requirements

verifyToken: (req: Request, token: string) => Promise<AuthInfo>,

That way this one piece of boilerplate that everyone needs with auth wouldn't have to be reimplemented.

[quuu]

@quuu Interesting. Why the choice in #39 to keep only the req param instead of token in the verifyToken function? If anything, I would think that the function should accept token only, since the MCP spec mandates using the Authorization header: modelcontextprotocol.io/specification/2025-03-26/basic/authorization#2-6-1-token-requirements

verifyToken: (req: Request, token: string) => Promise<AuthInfo>,

That way this one piece of boilerplate that everyone needs with auth wouldn't have to be reimplemented.

good point, im open to including the token in the signature -- it was removed to give control to the server developer to decide how to handle the token, as some auth services have more complex than string tokens (several strings or objects)

[allenzhou101]

maybe overloading is a good compromise?

export function verifyToken(token: string): Promise<AuthInfo>;
export function verifyToken(token: string, req: Request): Promise<AuthInfo>;

Would say token only should account for most use cases and the req can be included for more advanced?

[dvoytenko]

say token only should account for most use cases and the req can be included for more a

I think extracting token from the Authorization header and passing it as a second argument is a good idea. But IMHO throwing errors on it is not. E.g.

• Authorization is not the only way to propagate auth. Cookies could be used, session auth, etc.
• Authorization: Bearer is not the only token type that can be used.

[dvoytenko]

say token only should account for most use cases and the req can be included for more a

I think extracting token from the Authorization header and passing it as a second argument is a good idea. But IMHO throwing errors on it is not. E.g.

• Authorization is not the only way to propagate auth. Cookies could be used, session auth, etc.
• Authorization: Bearer is not the only token type that can be used.

But also to reiterate: it's good to have defaults.

[quuu]

say token only should account for most use cases and the req can be included for more a

I think extracting token from the Authorization header and passing it as a second argument is a good idea. But IMHO throwing errors on it is not. E.g.

• Authorization is not the only way to propagate auth. Cookies could be used, session auth, etc.
• Authorization: Bearer is not the only token type that can be used.

@allenzhou101 as an added compromise, how about this signature:

  verifyToken: (
    req: Request,
    bearerToken?: string
  ) => AuthInfo | undefined | Promise<AuthInfo | undefined>,
  


    const authHeader = req.headers.get("Authorization");
    const [_, bearerToken] = authHeader?.split(" ") || [];

    const authInfo = await verifyToken(req, bearerToken);

so it's explicit that it's Bearer

i agree with you that based on the official spec, Bearer tokens should be first class supported:

https://modelcontextprotocol.io/specification/2025-03-26/basic/authorization#2-6-1-token-requirements

[quuu]

@allenzhou101
#42

[allenzhou101]

say token only should account for most use cases and the req can be included for more a

I think extracting token from the Authorization header and passing it as a second argument is a good idea. But IMHO throwing errors on it is not. E.g.

• Authorization is not the only way to propagate auth. Cookies could be used, session auth, etc.
• Authorization: Bearer is not the only token type that can be used.

I agree at large, but with MCP in particular, the official spec defines the auth header in this way as the intended mechanism, so I'm inclined to support it and throw errors by default, maybe with a way to override and prevent.

However, I suppose ultimately either way is probably fine.

[quuu]

This looks good to me! cc @dvoytenko

[quuu]

@allenzhou101 can you run npm changeset to make this PR generate a patch change?

[allenzhou101]

@quuu Done!

[quuu]

Thanks for contributing @allenzhou101 !

[quuu]

Will update here after I test + bump the package version

[allenzhou101]

Thank y'all for reviewing! @quuu @dvoytenko

[quuu]

Deployed! Try the latest 0.8.3 @allenzhou101

[allenzhou101]

Awesome! Looks great.

[allenzhou101]

One note while using this is that maybe the 'required' field shouldn't exist, or it should default true--probably the latter. I'd expect token verification to be required by default when I'm adding the auth function.

This is how I've seen it work in the official implementation as well.