Skip to content

fix(openid): distinguish ID tokens from access tokens in federated auth#62

Closed
busla wants to merge 1 commit intomainfrom
fix/openid-token-distinction
Closed

fix(openid): distinguish ID tokens from access tokens in federated auth#62
busla wants to merge 1 commit intomainfrom
fix/openid-token-distinction

Conversation

@busla
Copy link

@busla busla commented Feb 10, 2026

What

Fix OpenID Connect token handling to properly distinguish ID tokens from access tokens. ID tokens and access tokens are now stored and propagated separately, preventing LIBRECHAT_OPENID_ID_TOKEN and LIBRECHAT_OPENID_ACCESS_TOKEN placeholders from resolving to identical values.

Why

Previously, ID tokens were not stored separately from access tokens in the authentication flow. This caused both token types to resolve to the same value when used in placeholder substitution, breaking token propagation in MCP configurations and HTTP headers that require distinct ID and access tokens.

Changes

  • AuthService.js: Added idToken field to session storage alongside accessToken
  • openIdJwtStrategy.js: Updated to read idToken from session and use it in federatedTokens
  • openidStrategy.js: Explicitly included id_token from tokenset in federatedTokens object
  • Test suites: Added comprehensive test coverage including:
    • Unit tests validating distinct token values in federatedTokens
    • Placeholder resolution tests confirming LIBRECHAT_OPENID_ID_TOKEN and LIBRECHAT_OPENID_ACCESS_TOKEN resolve to different values
    • Integration tests for MCP environment variables and HTTP headers with distinct tokens

Testing

  • Run existing OpenID Connect test suites to verify backward compatibility
  • Verify token placeholders in MCP configurations resolve to correct distinct values
  • Confirm HTTP headers with OpenID tokens are populated correctly with separate ID and access tokens

Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • OpenID Connect authentication now properly stores and exposes ID tokens separately from access tokens throughout the authentication flow.
  • Tests

    • Added and updated tests to validate ID token handling and ensure tokens are correctly processed and preserved.

Fix OpenID Connect token handling to properly distinguish ID tokens from access tokens. ID tokens and access tokens are now stored and propagated separately, preventing token placeholders from resolving to identical values.

- AuthService.js: Added idToken field to session storage
- openIdJwtStrategy.js: Updated to read idToken from session
- openidStrategy.js: Explicitly included id_token in federatedTokens
- Test suites: Added comprehensive test coverage for token distinction

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 10, 2026

📝 Walkthrough

Walkthrough

The changes add id_token (OpenID Connect identity token) support across the authentication pipeline, enabling storage, propagation, and extraction of identity tokens alongside existing access and refresh tokens in OpenID flows.

Changes

Cohort / File(s) Summary
Server-side token storage
api/server/services/AuthService.js
Now stores idToken from the OpenID tokenset into the server-side session alongside existing accessToken, refreshToken, and expiresAt fields.
OpenID strategy token propagation
api/strategies/openidStrategy.js, api/strategies/openIdJwtStrategy.js
Modified to include id_token in federatedTokens object; openIdJwtStrategy extracts idToken from session and uses it when constructing federated tokens, with backward-compatibility fallback logic preserved.
Test coverage
api/strategies/openidStrategy.spec.js, packages/api/src/utils/oidc.spec.ts
Extended test suite to verify id_token is properly stored, propagated, and distinct from access_token; new integration test ensures LIBRECHAT_OPENID_ACCESS_TOKEN and LIBRECHAT_OPENID_ID_TOKEN placeholders resolve to distinct values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🔐 The id_token finds its way,
Through sessions, strategies at play,
Alongside tokens old and true,
Identity flows, both fresh and new,
A OpenID tale, secure and tight! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and accurately summarizes the main change: distinguishing ID tokens from access tokens in OpenID federated authentication.
Description check ✅ Passed The description covers all critical sections: What, Why, Changes, and Testing. It aligns well with the template requirements and provides clear context for the modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/openid-token-distinction

Tip

We've launched Issue Planner and it is currently in beta. Please try it out and share your feedback on Discord!


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
api/server/services/AuthService.js (1)

452-473: ⚠️ Potential issue | 🟡 Minor

idToken is not persisted in the cookie fallback path.

When req.session is unavailable (lines 459–472), idToken is not stored in a cookie, so downstream consumers (e.g., openIdJwtStrategy.js) will never see an id_token for cookie-based sessions. If this is intentional—since ID tokens can be large and the cookie path is a legacy fallback—a brief comment explaining the omission would help future readers. If it's unintentional, it's a gap that breaks the feature for cookie-fallback users.

@busla
Copy link
Author

busla commented Feb 10, 2026

Recreating as a PR against upstream (danny-avila/LibreChat)

@busla busla closed this Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant