-
Notifications
You must be signed in to change notification settings - Fork 465
feat: support custom authorization headers, fix #395 #400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions and suggestions
- Avoid calling .lowerCase twice - Keep 'Bearer ' prefix when header equals 'Authorization' - Remove unnecessary Array type checks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few requests...
- Remove unnecessary auth header name check - Extract operations on headers into a function
Sorry to be jumping in a little late in the game, want to cc @pcarleton for extra 👀 . For anything auth-related I think we want to be careful that we're not introducing any unintentional inconsistencies here with security best practices, etc. that are being shared alongside the official docs + tools. I think for this ^^ PR it would be OK to merge with the caveat that we should follow up with some version of this feature, which would just make headers in general more flexible to add (including custom auth headers). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Just one thing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Littly Looks good, except it needs a prettier run to pass CI.
Local build and tests are good. Manually tested with a custom auth header against all three transport types.
SSE

StreamableHttp

STDIO

Express converts all header names to lowercase, which prevents custom headers with uppercase characters from being properly passed to the target service. This fix ensures that custom headers preserve their original capitalization when forwarded through the proxy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍
Added custom header support.
Motivation and Context
See #395
How Has This Been Tested?
The header parser implementation has been tested with a server driven by Spring-AI locally.
Breaking Changes
This change will impact services that rely on the "Bearer " prefix for authentication.
Types of changes
Checklist