From ddc763595ab0c448752060dd49796742d79b0448 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 23 Jun 2026 08:32:28 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=8D=AA=20fix:=20Validate=20Shared-File=20?= =?UTF-8?q?Cookie=20Auth=20Against=20the=20Live=20Refresh=20Session=20(#13?= =?UTF-8?q?908)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: validate shared file cookie sessions * fix: run shared file session lookup as system --- .../middleware/optionalShareFileAuth.js | 50 ++++++++----- .../middleware/optionalShareFileAuth.spec.js | 70 ++++++++++++++++--- 2 files changed, 95 insertions(+), 25 deletions(-) diff --git a/api/server/middleware/optionalShareFileAuth.js b/api/server/middleware/optionalShareFileAuth.js index f5c4ac07a5..bebf087d20 100644 --- a/api/server/middleware/optionalShareFileAuth.js +++ b/api/server/middleware/optionalShareFileAuth.js @@ -3,9 +3,9 @@ const jwt = require('jsonwebtoken'); const { isEnabled } = require('@librechat/api'); const { logger, runAsSystem } = require('@librechat/data-schemas'); const { SystemRoles } = require('librechat-data-provider'); -const { getUserById } = require('~/models'); +const { getUserById, findSession } = require('~/models'); -const verifyRefreshToken = (token) => { +const verifySignedUserId = (token) => { try { const payload = jwt.verify(token, process.env.JWT_REFRESH_SECRET); return typeof payload?.id === 'string' ? payload.id : null; @@ -14,13 +14,36 @@ const verifyRefreshToken = (token) => { } }; +const getRefreshTokenUserId = async (token) => { + const userId = verifySignedUserId(token); + if (!userId) { + return null; + } + + const session = await runAsSystem(() => findSession({ userId, refreshToken: token })); + return session ? userId : null; +}; + +const getOpenIdUserId = (parsed, req) => { + if (parsed.token_provider !== 'openid' || !isEnabled(process.env.OPENID_REUSE_TOKENS)) { + return null; + } + + const sessionRefreshToken = req.session?.openidTokens?.refreshToken; + if (!parsed.refreshToken || parsed.refreshToken !== sessionRefreshToken) { + return null; + } + + return verifySignedUserId(parsed.openid_user_id); +}; + /** * Fallback auth for share file routes that are hit by ``/anchor requests, * which can't carry the bearer access token. Resolves the viewer from the - * `refreshToken` cookie (or the signed `openid_user_id` cookie) — the same - * mechanism secure image links use — so non-public shared links can authorize - * the viewer's ACL. Never blocks: on any failure it leaves `req.user` unset and - * lets `canAccessSharedLink` decide (public access, 401, or 403). + * `refreshToken` cookie (or an active OpenID session plus signed `openid_user_id` + * cookie) so non-public shared links can authorize the viewer's ACL. Never + * blocks: on any failure it leaves `req.user` unset and lets + * `canAccessSharedLink` decide (public access, 401, or 403). */ const optionalShareFileAuth = async (req, res, next) => { if (req.user) { @@ -34,22 +57,17 @@ const optionalShareFileAuth = async (req, res, next) => { } const parsed = cookie.parse(cookieHeader); - const useOpenId = - parsed.token_provider === 'openid' && isEnabled(process.env.OPENID_REUSE_TOKENS); - const token = useOpenId ? parsed.openid_user_id : parsed.refreshToken; - if (!token) { - return next(); - } - - const userId = verifyRefreshToken(token); + const userId = + getOpenIdUserId(parsed, req) || + (parsed.refreshToken ? await getRefreshTokenUserId(parsed.refreshToken) : null); if (!userId) { return next(); } // Resolve in system context: this runs before canAccessSharedLink establishes // the share tenant, so under strict tenant isolation a tenant-scoped User - // query would otherwise throw. The viewer's id comes from their own verified - // refresh token; the share's tenant-scoped ACL check still gates access. + // query would otherwise throw. The viewer's id comes from verified, active + // cookie auth; the share's tenant-scoped ACL check still gates access. const user = await runAsSystem(() => getUserById(userId, '-password -__v -totpSecret -backupCodes'), ); diff --git a/api/server/middleware/optionalShareFileAuth.spec.js b/api/server/middleware/optionalShareFileAuth.spec.js index 96e147198b..ffc0cf5cfc 100644 --- a/api/server/middleware/optionalShareFileAuth.spec.js +++ b/api/server/middleware/optionalShareFileAuth.spec.js @@ -1,14 +1,27 @@ const mockVerify = jest.fn(); const mockGetUserById = jest.fn(); +const mockFindSession = jest.fn(); +const mockRunAsSystem = jest.fn((fn) => fn()); jest.mock('jsonwebtoken', () => ({ verify: (...args) => mockVerify(...args) })); -jest.mock('@librechat/api', () => ({ isEnabled: (v) => v === 'true' || v === true })); -jest.mock('@librechat/data-schemas', () => ({ - logger: { warn: jest.fn(), error: jest.fn() }, - runAsSystem: (fn) => fn(), +jest.mock('@librechat/api', () => ({ isEnabled: (v) => v === 'true' || v === true }), { + virtual: true, +}); +jest.mock( + '@librechat/data-schemas', + () => ({ + logger: { warn: jest.fn(), error: jest.fn() }, + runAsSystem: (...args) => mockRunAsSystem(...args), + }), + { virtual: true }, +); +jest.mock('librechat-data-provider', () => ({ SystemRoles: { USER: 'USER' } }), { + virtual: true, +}); +jest.mock('~/models', () => ({ + getUserById: (...args) => mockGetUserById(...args), + findSession: (...args) => mockFindSession(...args), })); -jest.mock('librechat-data-provider', () => ({ SystemRoles: { USER: 'USER' } })); -jest.mock('~/models', () => ({ getUserById: (...args) => mockGetUserById(...args) })); const optionalShareFileAuth = require('./optionalShareFileAuth'); @@ -30,20 +43,25 @@ describe('optionalShareFileAuth', () => { expect(next).toHaveBeenCalledTimes(1); expect(mockVerify).not.toHaveBeenCalled(); expect(mockGetUserById).not.toHaveBeenCalled(); + expect(mockFindSession).not.toHaveBeenCalled(); }); - it('resolves the viewer from a valid refreshToken cookie', async () => { + it('resolves the viewer from a valid refreshToken cookie with a live session', async () => { mockVerify.mockReturnValue({ id: 'viewer-1' }); + mockFindSession.mockResolvedValue({ _id: 'session-1' }); mockGetUserById.mockResolvedValue({ _id: 'viewer-1', role: 'USER' }); const req = { headers: { cookie: 'refreshToken=good.jwt' } }; const next = await run(req); expect(next).toHaveBeenCalledTimes(1); expect(mockVerify).toHaveBeenCalledWith('good.jwt', 'test-secret'); + expect(mockFindSession).toHaveBeenCalledWith({ userId: 'viewer-1', refreshToken: 'good.jwt' }); + expect(mockRunAsSystem).toHaveBeenCalledTimes(2); expect(req.user).toMatchObject({ id: 'viewer-1', role: 'USER' }); }); it('defaults the role to USER when the record has none', async () => { mockVerify.mockReturnValue({ id: 'viewer-2' }); + mockFindSession.mockResolvedValue({ _id: 'session-2' }); mockGetUserById.mockResolvedValue({ _id: 'viewer-2' }); const req = { headers: { cookie: 'refreshToken=good.jwt' } }; await run(req); @@ -58,6 +76,21 @@ describe('optionalShareFileAuth', () => { expect(mockGetUserById).not.toHaveBeenCalled(); }); + it('leaves req.user unset when the refresh token has no live session', async () => { + mockVerify.mockReturnValue({ id: 'viewer-3' }); + mockFindSession.mockResolvedValue(null); + const req = { headers: { cookie: 'refreshToken=revoked.jwt' } }; + const next = await run(req); + expect(next).toHaveBeenCalledTimes(1); + expect(req.user).toBeUndefined(); + expect(mockFindSession).toHaveBeenCalledWith({ + userId: 'viewer-3', + refreshToken: 'revoked.jwt', + }); + expect(mockRunAsSystem).toHaveBeenCalledTimes(1); + expect(mockGetUserById).not.toHaveBeenCalled(); + }); + it('leaves req.user unset when the token is invalid', async () => { mockVerify.mockImplementation(() => { throw new Error('bad token'); @@ -69,16 +102,35 @@ describe('optionalShareFileAuth', () => { expect(mockGetUserById).not.toHaveBeenCalled(); }); - it('uses the signed openid_user_id cookie for OpenID-reuse sessions', async () => { + it('uses the signed openid_user_id cookie only for active OpenID-reuse sessions', async () => { process.env.OPENID_REUSE_TOKENS = 'true'; mockVerify.mockReturnValue({ id: 'oidc-1' }); mockGetUserById.mockResolvedValue({ _id: 'oidc-1', role: 'USER' }); const req = { - headers: { cookie: 'token_provider=openid; openid_user_id=signed.jwt' }, + headers: { + cookie: 'token_provider=openid; refreshToken=stored-refresh; openid_user_id=signed.jwt', + }, + session: { openidTokens: { refreshToken: 'stored-refresh' } }, }; await run(req); expect(mockVerify).toHaveBeenCalledWith('signed.jwt', 'test-secret'); + expect(mockFindSession).not.toHaveBeenCalled(); expect(req.user).toMatchObject({ id: 'oidc-1' }); delete process.env.OPENID_REUSE_TOKENS; }); + + it('leaves req.user unset for OpenID-reuse cookies without an active matching session', async () => { + process.env.OPENID_REUSE_TOKENS = 'true'; + mockVerify.mockReturnValue({ id: 'oidc-2' }); + const req = { + headers: { + cookie: 'token_provider=openid; refreshToken=stale-refresh; openid_user_id=signed.jwt', + }, + session: { openidTokens: { refreshToken: 'current-refresh' } }, + }; + await run(req); + expect(req.user).toBeUndefined(); + expect(mockGetUserById).not.toHaveBeenCalled(); + delete process.env.OPENID_REUSE_TOKENS; + }); });