fix: enhance OAuth token handling in useConnection hook to prevent infinite auth loops

[kentcdodds]

Fix OAuth token injection when default "Bearer " header is present, preventing infinite auth loops

Closes #829

Motivation and Context

This change fixes a critical authentication bug that caused infinite OAuth loops in the inspector. The issue occurred when users had the default customHeaders configuration with an empty "Bearer " placeholder header. The authentication logic would send "Bearer " (without an actual token) to the server, which would return a 401 error, triggering OAuth flow, but the same empty header would be sent again, creating an infinite loop.

The root cause was that the OAuth token injection logic only triggered when finalHeaders.length === 0, but the default configuration always included a placeholder header { name: "Authorization", value: "Bearer ", enabled: true }, so the condition was never met.

How Has This Been Tested?

• Added a specific regression test case "replaces empty Bearer token placeholder with OAuth token" that verifies the fix
• All existing tests in the useConnection test suite pass (36/36 tests)
• The test verifies that when a default "Bearer " header is present, it gets replaced with the actual OAuth token "Bearer mock-token"

Breaking Changes

None. This is a bug fix that preserves existing behavior while fixing the authentication issue.

Types of changes

• 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

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

Additional context

I'm actually open to instead changing the default header to exclude the empty Bearer . Arguably that's a more appropriate fix. But I figured there's a reason that default was added so I decided to just handle it gracefully.

[kentcdodds]

I've verified this fixes the issues by publishing it under: @kentcdodds/[email protected] ✅

[olaservo]

Thank you @kentcdodds ! Do you think it would be a better long term solution to avoid this happening in the first place, rather than filtering? I think this is fine as a quick fix but I just tripped over this being turned on by default too and breaking my auth testing.

So maybe we need to:

1. Not have this added by default? and/or
2. Show a more obvious validation error if Inspector tries to pass a placeholder with just Bearer? Otherwise it may also silently fail on scenarios where someone is trying to pass that header but missed adding the token?

[kentcdodds]

Yeah, I noted that I would be happy with changing it so that it's just not on by default. And having some kind of validation error in the inspector would be good I think.

[kentcdodds]

I'm much happier with this 👍

[cliffhall]

@kentcdodds Left a comment that I think makes sense, leaving the handiness of the predefined header, but not having it enabled by default. Requesting the change, but will approve if you think that's still problematic.

[cliffhall]

Would an better way to do this be to leave it, but disable it? So for the common use case of adding an authorization header they just enable it and add their token, but otherwise it's just ignored when headers are sent? Makes it handy, but removes the footgun. With the robust checking you've added above in useConnection, we can be confident in the proper behavior should they switch it on but fail to add a token.

[kentcdodds]

I wonder if there's a situation where someone would want to test the lack of an auth header without actually removing the auth token from their client 🤔

Disabling the Auth header would be how I would want to do that.

[olaservo]

So instead of removing this chunk, should we just set the default to enabled: false?

[kentcdodds]

I'm a little confused why this chunk was added in the first place.

[kentcdodds]

I guess for myself, I always just use the auth flow that's built into the inspector. But I suppose some people have something more complicated and they have to get an auth token through some other means. But to me, that seems like more of an edge case. And a person in that situation would know how to add a new header called authorization with a bearer prefix. I don't think that they need our help to do that.

[olaservo]

Just speaking for myself and a few others I've seen chime in on the original feature request - I've worked with quite a few servers that don't support OAuth and where I need to pass something like a Personal Access Token through the authorization header, so this UI option seems more intuitive for that scenario?

[kentcdodds]

I'm happy to go whatever direction you all want with this. Just seems odd to me to have the headers include an invalid auth header (even if it's disabled) when it's quite easy to add and then it even saves to localstorage once you have 🤷‍♂️

Additionally, I do think this is a much less common case so we are increasing confusion for more folks to make things a tiny bit easier for fewer folks 🤔

[cliffhall]

@kentcdodds can we just leave it and turn it off by default? The point is that it saves the person who needs to add a bearer token some typing and possible fat-finger errors. I don't see how it's really going to sow confusion. Anyone monkeying with headers will know the bearer token header pattern, and if they don't, they'll probably just leave it turned off. If they do just turn it on and mash connect, they'll get a toast about it being incomplete and see the error of their ways. It's not a footgun, it's just a convenience.

[kentcdodds]

Sounds good. I changed it though so it does not automatically replace an empty auth header to reduce "magic." People will see the toast and go disable the header. I'm happy with that :)

[cliffhall]

One last change for clarity

Fixed

[cliffhall]

LGTM! 👍