📌 fix: Improve robustness for missing token and key expiry fields#65
Open
programmvilli wants to merge 1 commit into
Open
📌 fix: Improve robustness for missing token and key expiry fields#65programmvilli wants to merge 1 commit into
programmvilli wants to merge 1 commit into
Conversation
Ensure `id_token` is populated by falling back to the raw bearer token if explicitly missing from the IdP response. Allow user-provided API keys to be resolved even when the `expiresAt` field is absent.
📝 WalkthroughWalkthroughThis PR addresses two independent concerns: OpenID JWT token handling now falls back to raw Bearer tokens when ID tokens are missing, and OpenAI initialization decouples user input presence from expiry validation, with test coverage for missing expiry fields. ChangesOpenID JWT Token Fallback
OpenAI Initialization Expiry Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/api/src/endpoints/openai/initialize.spec.ts (1)
118-143: ⚡ Quick winAssert that expiry validation is explicitly skipped in the missing-expiry path.
This test should also verify
checkUserKeyExpiryis not called, so the exact guard behavior is locked down.Suggested patch
import type { BaseInitializeParams } from '~/types'; const mockValidateEndpointURL = jest.fn(); +const mockCheckUserKeyExpiry = jest.fn(); jest.mock('~/auth', () => ({ validateEndpointURL: (...args: unknown[]) => mockValidateEndpointURL(...args), })); @@ jest.mock('~/utils', () => ({ getAzureCredentials: jest.fn(), resolveHeaders: jest.fn(() => ({})), isUserProvided: (val: string) => val === 'user_provided', - checkUserKeyExpiry: jest.fn(), + checkUserKeyExpiry: (...args: unknown[]) => mockCheckUserKeyExpiry(...args), })); @@ expect(params.db.getUserKeyValues).toHaveBeenCalledWith({ userId: 'user-1', name: EModelEndpoint.openAI, }); + expect(mockCheckUserKeyExpiry).not.toHaveBeenCalled(); expect(mockValidateEndpointURL).not.toHaveBeenCalled(); expect(mockGetOpenAIConfig).toHaveBeenCalledWith( 'sk-user-key',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/api/src/endpoints/openai/initialize.spec.ts` around lines 118 - 143, Add an assertion in the test "should resolve a stored user key even when the expiry field is missing" to assert that checkUserKeyExpiry is not invoked; locate the test in initialize.spec.ts and after the initializeOpenAI call (but before restoring mocks) add expect(checkUserKeyExpiry).not.toHaveBeenCalled() so the missing-expiry code path explicitly skips expiry validation; reference the checkUserKeyExpiry symbol and the initializeOpenAI test block to place the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/api/src/endpoints/openai/initialize.spec.ts`:
- Around line 118-143: Add an assertion in the test "should resolve a stored
user key even when the expiry field is missing" to assert that
checkUserKeyExpiry is not invoked; locate the test in initialize.spec.ts and
after the initializeOpenAI call (but before restoring mocks) add
expect(checkUserKeyExpiry).not.toHaveBeenCalled() so the missing-expiry code
path explicitly skips expiry validation; reference the checkUserKeyExpiry symbol
and the initializeOpenAI test block to place the assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72fe85bc-2482-495f-a37c-6ff238a5e7da
📒 Files selected for processing (4)
api/strategies/openIdJwtStrategy.jsapi/strategies/openIdJwtStrategy.spec.jspackages/api/src/endpoints/openai/initialize.spec.tspackages/api/src/endpoints/openai/initialize.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Template
Summary
Please provide a brief summary of your changes and the related issue. Include any motivation and context that is relevant to your changes. If there are any dependencies necessary for your changes, please list them here.
Change Type
Please delete any irrelevant options.
Testing
Please describe your test process and include instructions so that we can reproduce your test. If there are any important variables for your testing configuration, list them here.
Test Configuration:
We tested OIDC Bearer fallback and initialization under missing key expiration parameters utilizing the Jest test framework.
Test Process:
Scenario 1: Simulated selective cookie deletion (missing openid_id_token cookie) under an active browser session. Verified LibreChat falls back directly to the validated raw bearer token.
Scenario 2: Evaluated endpoint credentials with no expiration field in the request parameters. Verified retrieved credentials decrypted successfully without crashing.
How to Reproduce:
Run these commands from the root directory of your project:
Verify OIDC Token Strategy Fallback:
(Checks user fallback logic in openIdJwtStrategy.js)
Verify Endpoint Key Initialization:
(Checks key logic in [initialize.ts]
Variables:
NODE_ENV: test (Sets test profile context)
OPENAI_API_KEY: user_provided (Triggers user key collection)
OPENAI_REVERSE_PROXY: https://user-proxy.example.com/v1 (Provides a valid mock endpoint verification)
Checklist
Please delete any irrelevant options.
Summary by CodeRabbit
Release Notes