-
Notifications
You must be signed in to change notification settings - Fork 4k
openai-adapters: allow overriding authorization header #8684
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
Enable overriding OpenAI client's Authorization header by removing it if
the user provides a custom authorization header with `requestOptions: {
headers: { Authorization: Basic ... } }`
OpenAI client will always send an `Authorization: Bearer` header since
`apiKey` is a mandatory parameter. Previously, both the OpenAI header
and the custom header were sent, breaking at least vLLM set up with
basic auth. And it seems like multiple Authorization headers is a breach
of the HTTP RFC specs, so there should be no justifiable use case to
send more than one.
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.
No issues found across 1 file
|
Oh, I forgot to mention that this is an attempt to fix issue #7047. |
RomneyDa
left a comment
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.
@visadb thanks for the contribution!
|
🎉 This PR is included in version 1.34.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
This fixes issue #7047 for me.
Enable overriding OpenAI client's Authorization header by removing it if the user provides a custom authorization header with
requestOptions: { headers: { Authorization: Basic ... } }OpenAI client will always send an
Authorization: Bearerheader sinceapiKeyis a mandatory parameter. Previously, both the OpenAI header and the custom header were sent, breaking at least vLLM set up with basic auth. And it seems like multiple Authorization headers is a breach of the HTTP RFC specs, so there should be no justifiable use case to send more than one.This is try 2 after #7803 failed to fix the issue. This time I had time to set up a development environment and actually test that the fixed IntelliJ IDEA plugin works against my Basic authenticated vLLM instance.
AI Code Review
@continue-reviewChecklist
Screen recording or screenshot
This is what headers used to look like before this fix:
And now the
Authorization: Bearerone is removed if a custom authorization header is provided.Tests
I couldn't find an appropriate place to test this customFetch function for its authorization header behaviour. Testing suggestions are welcome!
Summary by cubic
Allows a custom Authorization header to replace the default Bearer header in openai-adapters, fixing duplicate headers and restoring compatibility with Basic-auth setups like vLLM. Prevents sending multiple Authorization headers.
Written for commit 6ad9330. Summary will update automatically on new commits.