Skip to content

🔏 fix: Remove Federated Tokens from OpenID Refresh Response#12264

Merged
danny-avila merged 2 commits into
devfrom
fix/refresh-token-exposure
Mar 16, 2026
Merged

🔏 fix: Remove Federated Tokens from OpenID Refresh Response#12264
danny-avila merged 2 commits into
devfrom
fix/refresh-token-exposure

Conversation

@danny-avila

@danny-avila danny-avila commented Mar 16, 2026

Copy link
Copy Markdown
Owner

Summary

Fixed two credential-exposure bugs in the OpenID refreshController response path where server-side-protected tokens and sensitive user fields were being serialized into the JSON response body.

  • Removed the explicit user.federatedTokens assignment before res.send({ token, user }) in the OpenID refresh path, which was leaking access_token, id_token, refresh_token, and expires_at into the response body — tokens already stored server-side by setOpenIDAuthTokens (express session + HttpOnly cookies) and re-attached to req.user by the JWT strategy on every authenticated request.
  • Destructured out password, __v, totpSecret, and backupCodes from the findOpenIDUser result before serializing, aligning the OpenID path with the non-OpenID path's explicit getUserById projection (-password -__v -totpSecret -backupCodes), which previously left 2FA secrets and password hashes exposed for migrated users.
  • Strengthened the regression test: replaced expect.not.objectContaining({ federatedTokens: expect.anything() }) with not.toHaveProperty(...) calls (which correctly catch null/undefined values), added a positive objectContaining shape assertion to prevent false-green passes on empty response objects, populated defaultUser with realistic sensitive field values, and extracted the duplicated mock fixture into a shared defaultUser constant with findOpenIDUser setup moved to beforeEach.

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

Verified with the Jest suite in the api workspace (cd api && npx jest AuthController). The updated refreshController – OpenID path describe block covers the fix via the should not expose sensitive fields or federatedTokens in refresh response test, which:

  • Seeds findOpenIDUser with a defaultUser containing populated password, totpSecret, backupCodes, and __v fields
  • Asserts the sent payload contains the expected safe fields (_id, email, openidId) via objectContaining
  • Asserts all five sensitive properties are absent via not.toHaveProperty, catching both omission and explicit null/undefined assignment

Test Configuration:

  • OPENID_REUSE_TOKENS=true (simulated via isEnabled.mockReturnValue(true))
  • No live IdP required — openid-client, @librechat/api, and all model dependencies are mocked

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes

The refresh controller was attaching federatedTokens (including the
refresh_token) to the user object returned in the JSON response,
exposing HttpOnly-protected tokens to client-side JavaScript.
The tokens are already stored server-side by setOpenIDAuthTokens
and re-attached by the JWT strategy on authenticated requests.
Copilot AI review requested due to automatic review settings March 16, 2026 12:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Removes OpenID federated token data (including refresh tokens) from the /refresh endpoint’s JSON response to prevent exposure of HttpOnly-protected credentials to client-side JavaScript.

Changes:

  • Stop attaching user.federatedTokens in the OpenID refresh controller response.
  • Add a Jest test asserting federatedTokens is not included in the refresh response user payload.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
api/server/controllers/AuthController.js Removes user.federatedTokens construction before sending the refresh response.
api/server/controllers/AuthController.spec.js Adds a regression test to ensure refresh responses don’t include federatedTokens.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +256 to +261
expect(res.send).toHaveBeenCalledWith({
token: 'new-app-token',
user: expect.not.objectContaining({
federatedTokens: expect.anything(),
}),
});
The OpenID refresh path returned the raw findOpenIDUser result without
field projection, unlike the non-OpenID path which excludes password,
__v, totpSecret, and backupCodes via getUserById projection. Destructure
out sensitive fields before serializing.

Also strengthens the regression test: uses not.toHaveProperty for true
property-absence checks (expect.anything() misses null/undefined), adds
positive shape assertion, and DRYs up duplicated mock user setup.
@danny-avila danny-avila changed the title 🔒 fix: Remove OpenID federated tokens from refresh endpoint response 🔏 fix: Remove Federated Tokens from OpenID Refresh Response Mar 16, 2026
@danny-avila danny-avila merged commit d17ac8f into dev Mar 16, 2026
9 checks passed
@danny-avila danny-avila deleted the fix/refresh-token-exposure branch March 16, 2026 13:23
jcbartle pushed a commit to jcbartle/LibreChat that referenced this pull request May 11, 2026
…ila#12264)

* 🔒 fix: Remove OpenID federated tokens from refresh endpoint response

The refresh controller was attaching federatedTokens (including the
refresh_token) to the user object returned in the JSON response,
exposing HttpOnly-protected tokens to client-side JavaScript.
The tokens are already stored server-side by setOpenIDAuthTokens
and re-attached by the JWT strategy on authenticated requests.

* 🔒 fix: Strip sensitive fields from OpenID refresh response user object

The OpenID refresh path returned the raw findOpenIDUser result without
field projection, unlike the non-OpenID path which excludes password,
__v, totpSecret, and backupCodes via getUserById projection. Destructure
out sensitive fields before serializing.

Also strengthens the regression test: uses not.toHaveProperty for true
property-absence checks (expect.anything() misses null/undefined), adds
positive shape assertion, and DRYs up duplicated mock user setup.
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.

2 participants