-
Notifications
You must be signed in to change notification settings - Fork 29
[Bugfix] Fix OpenAI authentication header formatting #99
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
base: main
Are you sure you want to change the base?
Conversation
…nai-compatible providers
Summary of ChangesHello @WillemMomma, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug that prevented proper authentication with OpenAI-compatible API providers due to an incorrectly formatted Authorization header. By standardizing the header generation process, the change ensures seamless integration and resolves authentication failures, thereby broadening the system's compatibility and reliability across different API endpoints. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request correctly fixes a bug in the OpenAI authentication header formatting by delegating header creation to the ModelAuthProvider. This is a good architectural improvement. The associated test changes are also appropriate, ensuring the fix is covered. I have one suggestion to improve the robustness of the header merging logic in openai_user.py to prevent potential conflicts with headers provided by the authentication provider.
| self.headers = { | ||
| "Authorization": f"Bearer {self.auth_provider.get_credentials()}", | ||
| **auth_headers, | ||
| "Content-Type": "application/json", | ||
| } |
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.
While this change correctly fixes the Authorization header issue, the current dictionary unpacking order will always override the Content-Type header from the authentication provider. This could cause issues if an auth provider requires a specific Content-Type.
To make this more robust and give the ModelAuthProvider full control over headers, consider setting Content-Type as a default that can be overridden by the provider. This aligns better with the goal of delegating header management.
| self.headers = { | |
| "Authorization": f"Bearer {self.auth_provider.get_credentials()}", | |
| **auth_headers, | |
| "Content-Type": "application/json", | |
| } | |
| self.headers = { | |
| "Content-Type": "application/json", | |
| **auth_headers, | |
| } |
Description
Fixes authentication errors when using OpenAI-compatible API providers/routers like LiteLLM. The Authorization header was incorrectly sending a dictionary representation instead of the API key string.
Issue: In
openai_user.py,get_credentials()returns{"api_key": "sk-..."}but was used directly in an f-string, resulting in:instead of:
Fix: Use
auth_provider.get_headers()which properly formats the Authorization header.PR Type / Label
/kind bug
Related Issue
N/A - Bug discovered during manual testing with LiteLLM endpoint
Changes
openai_user.py: Useget_headers()instead of manually constructing Authorization headertest_openai_user.py: Addedget_headers()mock to test fixtureCorrectness Tests
tests/user/test_openai_user.pyChecklist
git pull origin main)make checkto ensure code is properly formatted and passes all lint checksmake testto verify test coverage (~90% required)Additional Information
This fix ensures compatibility with any OpenAI-compatible API endpoint, not just the official OpenAI API.