From eae00250dfeb930a58bdb1830ab080019667ec2c Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 3 Jun 2026 09:21:46 -0400 Subject: [PATCH 001/160] =?UTF-8?q?=F0=9F=8C=8D=20i18n:=20Update=20transla?= =?UTF-8?q?tion.json=20with=20latest=20translations=20(#13482)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- client/src/locales/lv/translation.json | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/client/src/locales/lv/translation.json b/client/src/locales/lv/translation.json index 87c02fd32b8..583a4136308 100644 --- a/client/src/locales/lv/translation.json +++ b/client/src/locales/lv/translation.json @@ -500,7 +500,7 @@ "com_nav_font_size_sm": "Mazs", "com_nav_font_size_xl": "Īpaši liels", "com_nav_font_size_xs": "Īpaši mazs", - "com_nav_help_faq": "Palīdzība un bieži uzdotie jautājumi", + "com_nav_help_faq": "Lietošanas instrukcija un palīdzība", "com_nav_info_balance": "Bilance parāda, cik daudz tokenu kredītu jums ir atlicis izmantot. Tokenu kredīti tiek pārvērsti naudas vērtībā (piemēram, 1000 kredīti = 0,001 USD).", "com_nav_info_code_artifacts": "Iespējo eksperimentāla koda artefaktu rādīšanu blakus sarunai", "com_nav_info_code_artifacts_agent": "Iespējo koda artefaktu izmantošanu šim aģentam. Pēc noklusējuma tiek pievienotas papildu instrukcijas, kas attiecas uz artefaktu izmantošanu, ja vien nav iespējots \"Pielāgots uzvednes režīms\".", @@ -613,7 +613,7 @@ "com_nav_theme": "Tēma", "com_nav_theme_dark": "Tumšs", "com_nav_theme_light": "Gaišs", - "com_nav_theme_system": "Sistēmas uzstādījums", + "com_nav_theme_system": "Sistēmas", "com_nav_tool_dialog": "Asistenta rīki", "com_nav_tool_dialog_agents": "Aģenta rīki", "com_nav_tool_dialog_description": "Lai saglabātu rīku atlasi, ir jāsaglabā asistents.", @@ -725,12 +725,12 @@ "com_ui_agent_subagents_allow_self_info": "Ļaujiet šim aģentam deleģēt fokusētos apakšuzdevumus paša kopijai izolētā konteksta logā.", "com_ui_agent_subagents_empty": "Nav pieejami apakšaģenti: iespējojiet paš-izveidi vai pievienojiet vismaz vienu aģentu.", "com_ui_agent_subagents_enable": "Iespējot apakšaģentus", - "com_ui_agent_subagents_info": "Apakšaģenti darbojas izolētos konteksta logos. Izvērstā rīka izvade paliek pieejama bērnam; tikai kopsavilkums atgriežas pie šī aģenta.", + "com_ui_agent_subagents_info": "Apakšaģenti darbojas izolētos konteksta logos. Izvērstā rīka izvade paliek pieejama apakšaģentam; tikai kopsavilkums atgriežas pie šī aģenta.", "com_ui_agent_subagents_info_2": "Iespējojot bez uzskaitītajiem aģentiem, joprojām ir iespējams veikt paš-izveidi, tāpēc šis aģents var deleģēt fokusētus apakšuzdevumus svaigai savai kopijai.", "com_ui_agent_subagents_max": "Maksimums {{0}} apakšaģentu ir sasniegts.", "com_ui_agent_subagents_remove": "Noņemt {{0}} no apakšaģentiem", "com_ui_agent_url_copied": "Aģenta URL kopēts starpliktuvē", - "com_ui_agent_var": "{{0}} aģents", + "com_ui_agent_var": "{{0}} aģentu", "com_ui_agent_version": "Versija", "com_ui_agent_version_active": "Aktīvā versija", "com_ui_agent_version_empty": "Nav pieejamu versiju", @@ -1197,6 +1197,7 @@ "com_ui_mcp_server_updated": "MCP serveris veiksmīgi atjaunināts", "com_ui_mcp_server_url_placeholder": "https://mcp.piemers.lv", "com_ui_mcp_servers": "MCP serveri", + "com_ui_mcp_servers_allow_configure_obo": "Ļaut lietotājiem konfigurēt On-Behalf-Of (OBO) MCP serveros", "com_ui_mcp_servers_allow_create": "Atļaut lietotājiem izveidot MCP serverus", "com_ui_mcp_servers_allow_share": "Atļaut lietotājiem koplietot MCP serverus", "com_ui_mcp_servers_allow_share_public": "Ļaujiet lietotājiem publiski koplietot MCP serverus", @@ -1298,6 +1299,10 @@ "com_ui_oauth_error_title": "Autentifikācija neizdevās", "com_ui_oauth_success_description": "Jūsu autentifikācija bija veiksmīga. Šis logs aizvērsies pēc", "com_ui_oauth_success_title": "Autentifikācija veiksmīga", + "com_ui_obo": "On-Behalf-Of (OBO)", + "com_ui_obo_readonly_no_permission": "OBO ir konfigurēts šim serverim, bet jūsu lomai vairs nav atļaujas to modificēt. Sazinieties ar administratoru, lai veiktu izmaiņas.", + "com_ui_obo_scopes": "OBO darbības jomas", + "com_ui_obo_scopes_description": "Jomas, kas jāpieprasa lejupstraumes MCP serverim, izmantojot On-Behalf-Of plūsmu. Nepieciešama OpenID Connect autentifikācija (piemēram, Entra ID).", "com_ui_of": "no", "com_ui_off": "Izslēgts", "com_ui_offline": "Bezsaistē", From c50b3c58d5b240fee7692bb73e7d74910de8dad3 Mon Sep 17 00:00:00 2001 From: "Joohan(Lucas)" Date: Wed, 3 Jun 2026 09:45:37 -0400 Subject: [PATCH 002/160] =?UTF-8?q?=F0=9F=8F=B7=EF=B8=8F=20fix:=20Prevent?= =?UTF-8?q?=20Bedrock=20Cache=20Tokens=20from=20Inflating=20Completion=20C?= =?UTF-8?q?ount=20(#13468)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🐛 fix: prevent Bedrock cache tokens from inflating completion count * style: fix prettier formatting --- packages/api/src/agents/usage.spec.ts | 101 ++++++++++++++++++++++++++ packages/api/src/agents/usage.ts | 38 +++++++--- 2 files changed, 130 insertions(+), 9 deletions(-) diff --git a/packages/api/src/agents/usage.spec.ts b/packages/api/src/agents/usage.spec.ts index 4521de8eb4f..38e2fb5476d 100644 --- a/packages/api/src/agents/usage.spec.ts +++ b/packages/api/src/agents/usage.spec.ts @@ -988,6 +988,107 @@ describe('recordCollectedUsage', () => { }); }); + describe('Bedrock prompt caching — completion token inflation regression', () => { + it('does not fold cache_creation into completion on the first cached step', async () => { + // Bedrock: total = input + output + cache_creation (additive, not subset). + // Before fix: resolveCompletionTokens returned output + cache_creation (5500) + // instead of output (500). + const collectedUsage: UsageMetadata[] = [ + { + input_tokens: 100, + output_tokens: 500, + total_tokens: 5600, + cache_creation_input_tokens: 5000, + cache_read_input_tokens: 0, + model: 'claude-sonnet-4-6', + }, + ]; + + const result = await recordCollectedUsage(deps, { ...baseParams, collectedUsage }); + + expect(mockSpendStructuredTokens).toHaveBeenCalledWith( + expect.objectContaining({ model: 'claude-sonnet-4-6' }), + { + promptTokens: { input: 100, write: 5000, read: 0 }, + completionTokens: 500, + }, + ); + expect(result?.output_tokens).toBe(500); + }); + + it('does not fold cache_read into completion on subsequent cached steps', async () => { + // Bedrock: total = input + output + cache_read on every read step. + // Before fix: each step returned output + cache_read instead of output. + const collectedUsage: UsageMetadata[] = [ + { + input_tokens: 200, + output_tokens: 300, + total_tokens: 4500, + cache_read_input_tokens: 4000, + cache_creation_input_tokens: 0, + model: 'claude-sonnet-4-6', + }, + ]; + + const result = await recordCollectedUsage(deps, { ...baseParams, collectedUsage }); + + expect(mockSpendStructuredTokens).toHaveBeenCalledWith( + expect.objectContaining({ model: 'claude-sonnet-4-6' }), + { + promptTokens: { input: 200, write: 0, read: 4000 }, + completionTokens: 300, + }, + ); + expect(result?.output_tokens).toBe(300); + }); + + it('handles cache tokens in input_token_details format (alternate field path)', async () => { + const collectedUsage: UsageMetadata[] = [ + { + input_tokens: 200, + output_tokens: 300, + total_tokens: 4500, + input_token_details: { cache_read: 4000, cache_creation: 0 }, + model: 'claude-sonnet-4-6', + }, + ]; + + const result = await recordCollectedUsage(deps, { ...baseParams, collectedUsage }); + + expect(result?.output_tokens).toBe(300); + }); + + it('accumulates only true output across a multi-step cached agent run', async () => { + // 1 write step + 4 read steps. Without the fix, each step folds its + // cache tokens into completion, inflating the total by the full cache size. + const writeStep: UsageMetadata = { + input_tokens: 100, + output_tokens: 500, + total_tokens: 5600, + cache_creation_input_tokens: 5000, + cache_read_input_tokens: 0, + model: 'claude-sonnet-4-6', + }; + const readSteps: UsageMetadata[] = Array.from({ length: 4 }, (_, i) => ({ + input_tokens: 200, + output_tokens: 300 + i * 50, + total_tokens: 200 + (300 + i * 50) + 5000, + cache_read_input_tokens: 5000, + cache_creation_input_tokens: 0, + model: 'claude-sonnet-4-6', + })); + + const result = await recordCollectedUsage(deps, { + ...baseParams, + collectedUsage: [writeStep, ...readSteps], + }); + + // True output: 500 + 300 + 350 + 400 + 450 = 2000 + const trueOutput = 500 + readSteps.reduce((sum, s) => sum + (s.output_tokens ?? 0), 0); + expect(result?.output_tokens).toBe(trueOutput); + }); + }); + describe('bulk write with summarization usage', () => { let mockInsertMany: jest.Mock; let mockUpdateBalance: jest.Mock; diff --git a/packages/api/src/agents/usage.ts b/packages/api/src/agents/usage.ts index 3d8ddf37991..cc151685471 100644 --- a/packages/api/src/agents/usage.ts +++ b/packages/api/src/agents/usage.ts @@ -58,22 +58,42 @@ function inputTokensIncludesCache(provider?: string): boolean { * `total_tokens === input_tokens + output_tokens`. Compliant providers * (OpenAI, Anthropic, Google API via agents' `CustomChatGoogleGenerativeAI`) * include any reasoning/thinking tokens inside `output_tokens` already, - * so the invariant holds. + * so the invariant holds and this function is a no-op for them. * - * Vertex AI Gemini through `@langchain/google-common`'s streaming path - * emits `output_tokens = candidatesTokenCount` and drops `thoughtsTokenCount`, - * leaving `total - input > output`. When that gap shows up we use the - * invariant to recover the correct billable output (`total - input`). - * Compliant providers have a zero gap, so this is a no-op for them. + * **Vertex AI undercount (issue #13006):** `@langchain/google-common`'s streaming + * path emits `output_tokens = candidatesTokenCount` and drops `thoughtsTokenCount`, + * so `total - input > output`. The gap is recovered as `total - input`. * - * Tracked in: https://github.com/danny-avila/LibreChat/issues/13006 + * **Bedrock / Anthropic cache inflation:** additive providers keep cache tokens + * separate from `input_tokens`, making + * `total = input + output + cache_read + cache_creation`. Without adjustment + * the Vertex recovery fires on every cached step and returns + * `output + cache_read + cache_creation` instead of `output`, inflating + * completion counts by orders of magnitude. The fix subtracts the cache + * adjustment before the gap test — but only for additive providers; subset + * providers (Google, OpenAI, …) already include cache inside `input_tokens` + * so their `cacheAdjustment` is zero and the Vertex recovery is unaffected. */ function resolveCompletionTokens(usage: UsageMetadata): number { const output = Number(usage.output_tokens) || 0; const total = Number(usage.total_tokens) || 0; const input = Number(usage.input_tokens) || 0; - if (total > input + output) { - return total - input; + + // For additive providers (Bedrock, Anthropic), cache tokens are separate + // from input_tokens and are included in total_tokens, widening the gap + // independently of any missing thinking tokens. Subtract them so the gap + // check only fires when output_tokens genuinely undercounts (Vertex case). + // Subset providers fold cache into input_tokens, so their adjustment is 0. + const cacheRead = + Number(usage.input_token_details?.cache_read) || Number(usage.cache_read_input_tokens) || 0; + const cacheCreation = + Number(usage.input_token_details?.cache_creation) || + Number(usage.cache_creation_input_tokens) || + 0; + const cacheAdjustment = inputTokensIncludesCache(usage.provider) ? 0 : cacheRead + cacheCreation; + + if (total > input + output + cacheAdjustment) { + return total - input - cacheAdjustment; } return output; } From 1fa28ec45b52964efd3316544eb0a4192ff68f81 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 3 Jun 2026 13:45:46 -0400 Subject: [PATCH 003/160] =?UTF-8?q?=F0=9F=9B=B0=EF=B8=8F=20feat:=20Add=20A?= =?UTF-8?q?uth=20Fallback=20Observability=20(#13488)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add auth fallback observability * refactor: move auth log helpers to api package * test: harden auth log context handling * fix: keep auth logs low cardinality * fix: render auth log context in debug messages * fix: lower plain jwt auth failures to debug --- .../__tests__/requireJwtAuth.spec.js | 450 +++++++++++++++++- api/server/middleware/requireJwtAuth.js | 79 +++ packages/api/src/middleware/auth.spec.ts | 206 ++++++++ packages/api/src/middleware/auth.ts | 228 +++++++++ packages/api/src/middleware/index.ts | 1 + 5 files changed, 963 insertions(+), 1 deletion(-) create mode 100644 packages/api/src/middleware/auth.spec.ts create mode 100644 packages/api/src/middleware/auth.ts diff --git a/api/server/middleware/__tests__/requireJwtAuth.spec.js b/api/server/middleware/__tests__/requireJwtAuth.spec.js index 4059be24098..873a8158748 100644 --- a/api/server/middleware/__tests__/requireJwtAuth.spec.js +++ b/api/server/middleware/__tests__/requireJwtAuth.spec.js @@ -43,6 +43,7 @@ jest.mock('@librechat/data-schemas', () => { getTenantId: () => tenantStorage.getStore()?.tenantId, getUserId: () => tenantStorage.getStore()?.userId, getRequestId: () => tenantStorage.getStore()?.requestId, + logger: { debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn() }, tenantStorage, }; }); @@ -53,6 +54,152 @@ jest.mock('@librechat/data-schemas', () => { // primitives. The real implementation is covered by packages/api tenant.spec.ts. jest.mock('@librechat/api', () => { const { tenantStorage } = require('@librechat/data-schemas'); + const normalizeAuthLogValue = (value) => { + if (value == null) { + return undefined; + } + if (Array.isArray(value)) { + for (const entry of value) { + const normalized = normalizeAuthLogValue(entry); + if (normalized) { + return normalized; + } + } + return undefined; + } + if (typeof value === 'string') { + const trimmed = value.trim(); + return trimmed || undefined; + } + if (typeof value === 'number' || typeof value === 'boolean') { + return String(value); + } + return undefined; + }; + const normalizeAuthLogContextValue = (value) => { + if (value == null) { + return undefined; + } + if (Array.isArray(value)) { + const values = value + .map((entry) => normalizeAuthLogValue(entry)) + .filter((entry) => entry !== undefined); + return values.length > 0 ? values : undefined; + } + if (typeof value === 'string') { + return normalizeAuthLogValue(value); + } + if (typeof value === 'number') { + return Number.isFinite(value) ? value : undefined; + } + if (typeof value === 'boolean') { + return value; + } + return undefined; + }; + const getAuthFailureField = (source, field) => { + if (!source) { + return undefined; + } + if (typeof source === 'string') { + return field === 'message' ? source : undefined; + } + if (typeof source === 'object') { + try { + return source[field]; + } catch { + return undefined; + } + } + return undefined; + }; + const getAuthFailureReason = (err, info, fallback = 'Unauthorized') => + normalizeAuthLogValue(getAuthFailureField(info, 'message')) ?? + normalizeAuthLogValue(getAuthFailureField(err, 'message')) ?? + fallback; + const getAuthFailureErrorName = (err, info) => + normalizeAuthLogValue(getAuthFailureField(info, 'name')) ?? + normalizeAuthLogValue(getAuthFailureField(err, 'name')); + const getSafeTokenProvider = (tokenProvider) => { + const normalized = normalizeAuthLogValue(tokenProvider); + if (!normalized) { + return undefined; + } + return normalized === 'openid' || normalized === 'librechat' ? normalized : 'other'; + }; + const normalizeRoutePath = (path) => { + if (typeof path === 'string') { + return normalizeAuthLogValue(path); + } + if (Array.isArray(path)) { + for (const entry of path) { + const normalized = normalizeRoutePath(entry); + if (normalized) { + return normalized; + } + } + } + return undefined; + }; + const joinRoutePath = (baseUrl, routePath) => { + const normalizedRoute = routePath === '/' ? '' : routePath; + if (!baseUrl) { + return normalizedRoute || '/'; + } + if (!normalizedRoute) { + return baseUrl; + } + return `${baseUrl.replace(/\/$/, '')}/${normalizedRoute.replace(/^\//, '')}`; + }; + const bucketConcretePath = (path) => { + const queryless = path?.split('?')[0]; + if (!queryless) { + return undefined; + } + const segments = queryless.split('/').filter(Boolean); + if (segments.length === 0) { + return '/'; + } + if (segments[0] === 'api' && segments[1]) { + return `/${segments.slice(0, 2).join('/')}`; + } + return `/${segments[0]}`; + }; + const getRequestPath = (req) => { + const baseUrl = normalizeAuthLogValue(req.baseUrl); + const routePath = normalizeRoutePath(req.route?.path); + if (routePath) { + return joinRoutePath(baseUrl, routePath); + } + if (baseUrl) { + return baseUrl; + } + const path = + normalizeAuthLogValue(req.path) ?? normalizeAuthLogValue(req.originalUrl ?? req.url); + return bucketConcretePath(path); + }; + const compactAuthLogContext = (log) => + Object.fromEntries( + Object.entries(log) + .map(([key, value]) => [key, normalizeAuthLogContextValue(value)]) + .filter(([, value]) => value !== undefined), + ); + const buildSafeAuthLogContext = (req, authState, extra = {}) => + compactAuthLogContext({ + ...extra, + request_id: + normalizeAuthLogValue(req.requestId) ?? + normalizeAuthLogValue(req.id) ?? + normalizeAuthLogValue(req.headers?.['x-request-id']) ?? + normalizeAuthLogValue(req.headers?.['x-correlation-id']), + method: normalizeAuthLogValue(req.method), + path: getRequestPath(req), + token_provider: getSafeTokenProvider(authState.tokenProvider), + openid_reuse_enabled: authState.openidReuseEnabled, + openid_jwt_available: authState.openidJwtAvailable, + has_openid_reuse_user_id: authState.hasOpenIdReuseUserId, + }); + const formatAuthLogMessage = (message, context) => `${message} ${JSON.stringify(context)}`; const normalizeContextValue = (value) => { const trimmed = value?.trim?.(); return trimmed || undefined; @@ -66,6 +213,10 @@ jest.mock('@librechat/api', () => { normalizeContextValue(req.headers?.['x-correlation-id']); return { isEnabled: jest.fn(() => false), + getAuthFailureReason, + getAuthFailureErrorName, + buildSafeAuthLogContext, + formatAuthLogMessage, maybeRefreshCloudFrontAuthCookiesMiddleware: jest.fn((req, res, next) => next()), tenantContextMiddleware: (req, res, next) => { const context = { @@ -84,7 +235,7 @@ jest.mock('@librechat/api', () => { // ── Helpers ───────────────────────────────────────────────────────────── const requireJwtAuth = require('../requireJwtAuth'); -const { getTenantId, getUserId } = require('@librechat/data-schemas'); +const { getTenantId, getUserId, logger } = require('@librechat/data-schemas'); const { isEnabled, maybeRefreshCloudFrontAuthCookiesMiddleware } = require('@librechat/api'); const passport = require('passport'); @@ -127,6 +278,10 @@ describe('requireJwtAuth tenant context chaining', () => { mockRegisteredStrategies = new Set(['jwt']); isEnabled.mockReturnValue(false); maybeRefreshCloudFrontAuthCookiesMiddleware.mockClear(); + logger.debug.mockClear(); + logger.info.mockClear(); + logger.warn.mockClear(); + logger.error.mockClear(); passport.authenticate.mockClear(); passport._strategy.mockClear(); if (originalJwtSecret === undefined) { @@ -204,6 +359,207 @@ describe('requireJwtAuth tenant context chaining', () => { expect(next).not.toHaveBeenCalled(); expect(res.status).toHaveBeenCalledWith(401); expect(getTenantId()).toBeUndefined(); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] Authentication failed after all strategies'), + expect.objectContaining({ + primary_strategy: 'jwt', + fallback_attempted: false, + fallback_succeeded: false, + attempted_strategies: ['jwt'], + final_strategy: 'jwt', + reason: 'Unauthorized', + status: 401, + }), + ); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('logs OpenID JWT expiry when JWT fallback succeeds', () => { + isEnabled.mockReturnValue(true); + mockRegisteredStrategies.add('openidJwt'); + const req = mockReq(undefined, { + requestId: 'req-expired-success', + method: 'GET', + path: '/api/messages', + headers: { + cookie: `token_provider=openid; openid_user_id=${signedOpenIdUserCookie('user-jwt')}`, + }, + _mockStrategies: { + openidJwt: { + user: false, + info: { message: 'jwt expired', name: 'TokenExpiredError' }, + status: 401, + }, + jwt: { user: { id: 'user-jwt', tenantId: 'tenant-jwt', role: 'user' } }, + }, + }); + const res = mockRes(); + const next = jest.fn(); + + requireJwtAuth(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(req.authStrategy).toBe('jwt'); + expect(res.status).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] OpenID JWT auth failed; trying fallback'), + expect.objectContaining({ + request_id: 'req-expired-success', + method: 'GET', + path: '/api/messages', + token_provider: 'openid', + openid_reuse_enabled: true, + openid_jwt_available: true, + has_openid_reuse_user_id: true, + primary_strategy: 'openidJwt', + fallback_strategy: 'jwt', + fallback_attempted: true, + reason: 'jwt expired', + error_name: 'TokenExpiredError', + status: 401, + }), + ); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] JWT fallback succeeded after OpenID JWT failure'), + expect.objectContaining({ + request_id: 'req-expired-success', + auth_strategy: 'jwt', + primary_strategy: 'openidJwt', + fallback_strategy: 'jwt', + fallback_attempted: true, + fallback_succeeded: true, + primary_failure_reason: 'jwt expired', + reason: 'jwt expired', + error_name: 'TokenExpiredError', + }), + ); + expect(logger.debug.mock.calls[0][0]).toContain('"reason":"jwt expired"'); + expect(logger.debug.mock.calls[0][0]).toContain('"fallback_attempted":true'); + expect(logger.debug.mock.calls[1][0]).toContain('"fallback_succeeded":true'); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('does not let malformed Passport info break JWT fallback logging', () => { + isEnabled.mockReturnValue(true); + mockRegisteredStrategies.add('openidJwt'); + const info = {}; + Object.defineProperties(info, { + message: { + get() { + throw new TypeError('message getter failed'); + }, + }, + name: { + get() { + throw new TypeError('name getter failed'); + }, + }, + }); + const req = mockReq(undefined, { + requestId: 'req-malformed-info', + method: 'GET', + path: '/api/messages', + headers: { + cookie: `token_provider=openid; openid_user_id=${signedOpenIdUserCookie('user-jwt')}`, + }, + _mockStrategies: { + openidJwt: { + user: false, + info, + status: 401, + }, + jwt: { user: { id: 'user-jwt', tenantId: 'tenant-jwt', role: 'user' } }, + }, + }); + const res = mockRes(); + const next = jest.fn(); + + expect(() => requireJwtAuth(req, res, next)).not.toThrow(); + + expect(next).toHaveBeenCalled(); + expect(req.authStrategy).toBe('jwt'); + expect(res.status).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] OpenID JWT auth failed; trying fallback'), + expect.objectContaining({ + request_id: 'req-malformed-info', + fallback_attempted: true, + reason: 'Unauthorized', + status: 401, + }), + ); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] JWT fallback succeeded after OpenID JWT failure'), + expect.objectContaining({ + request_id: 'req-malformed-info', + fallback_succeeded: true, + primary_failure_reason: 'Unauthorized', + }), + ); + }); + + it('logs OpenID JWT expiry when JWT fallback fails', () => { + isEnabled.mockReturnValue(true); + mockRegisteredStrategies.add('openidJwt'); + const req = mockReq(undefined, { + id: 'req-expired-fail', + method: 'POST', + originalUrl: '/api/ask?access_token=hidden', + headers: { + cookie: `token_provider=openid; openid_user_id=${signedOpenIdUserCookie('user-jwt')}`, + }, + _mockStrategies: { + openidJwt: { + user: false, + info: { message: 'jwt expired', name: 'TokenExpiredError' }, + status: 401, + }, + jwt: { + user: false, + info: { message: 'invalid signature', name: 'JsonWebTokenError' }, + status: 401, + }, + }, + }); + const res = mockRes(); + const next = jest.fn(); + + requireJwtAuth(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] OpenID JWT auth failed; trying fallback'), + expect.objectContaining({ + request_id: 'req-expired-fail', + method: 'POST', + path: '/api/ask', + fallback_attempted: true, + reason: 'jwt expired', + error_name: 'TokenExpiredError', + status: 401, + }), + ); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] Authentication failed after all strategies'), + expect.objectContaining({ + request_id: 'req-expired-fail', + method: 'POST', + path: '/api/ask', + token_provider: 'openid', + attempted_strategies: ['openidJwt', 'jwt'], + final_strategy: 'jwt', + primary_strategy: 'openidJwt', + fallback_strategy: 'jwt', + fallback_attempted: true, + fallback_succeeded: false, + reason: 'invalid signature', + error_name: 'JsonWebTokenError', + status: 401, + }), + ); + expect(logger.warn.mock.calls[0][0]).toContain('"reason":"invalid signature"'); + expect(logger.warn.mock.calls[0][0]).toContain('"path":"/api/ask"'); }); it('does not fall back to OpenID JWT for bearer-only reuse requests', () => { @@ -263,6 +619,98 @@ describe('requireJwtAuth tenant context chaining', () => { ); }); + it('logs OpenID user-id mismatch when JWT fallback succeeds', () => { + isEnabled.mockReturnValue(true); + mockRegisteredStrategies.add('openidJwt'); + const req = mockReq(undefined, { + requestId: 'req-mismatch-success', + method: 'GET', + path: '/api/auth/me', + headers: { + cookie: `token_provider=openid; openid_user_id=${signedOpenIdUserCookie('user-a')}`, + }, + _mockStrategies: { + openidJwt: { user: { id: 'user-b', tenantId: 'tenant-openid', role: 'user' } }, + jwt: { user: { id: 'user-a', tenantId: 'tenant-jwt', role: 'user' } }, + }, + }); + const res = mockRes(); + const next = jest.fn(); + + requireJwtAuth(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(req.authStrategy).toBe('jwt'); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] OpenID JWT auth failed; trying fallback'), + expect.objectContaining({ + request_id: 'req-mismatch-success', + primary_strategy: 'openidJwt', + fallback_strategy: 'jwt', + fallback_attempted: true, + reason: 'openid user-id mismatch', + status: 401, + }), + ); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] JWT fallback succeeded after OpenID JWT failure'), + expect.objectContaining({ + request_id: 'req-mismatch-success', + auth_strategy: 'jwt', + fallback_attempted: true, + fallback_succeeded: true, + primary_failure_reason: 'openid user-id mismatch', + reason: 'openid user-id mismatch', + }), + ); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('logs OpenID user-id mismatch when JWT fallback fails', () => { + isEnabled.mockReturnValue(true); + mockRegisteredStrategies.add('openidJwt'); + const req = mockReq(undefined, { + requestId: 'req-mismatch-fail', + method: 'GET', + path: '/api/auth/me', + headers: { + cookie: `token_provider=openid; openid_user_id=${signedOpenIdUserCookie('user-a')}`, + }, + _mockStrategies: { + openidJwt: { user: { id: 'user-b', tenantId: 'tenant-openid', role: 'user' } }, + jwt: { user: false, info: { message: 'Unauthorized' }, status: 401 }, + }, + }); + const res = mockRes(); + const next = jest.fn(); + + requireJwtAuth(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] OpenID JWT auth failed; trying fallback'), + expect.objectContaining({ + request_id: 'req-mismatch-fail', + fallback_attempted: true, + reason: 'openid user-id mismatch', + status: 401, + }), + ); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('[requireJwtAuth] Authentication failed after all strategies'), + expect.objectContaining({ + request_id: 'req-mismatch-fail', + attempted_strategies: ['openidJwt', 'jwt'], + final_strategy: 'jwt', + fallback_attempted: true, + fallback_succeeded: false, + reason: 'Unauthorized', + status: 401, + }), + ); + }); + it('does not authenticate OpenID JWT when the reuse cookie belongs to another user', () => { isEnabled.mockReturnValue(true); mockRegisteredStrategies.add('openidJwt'); diff --git a/api/server/middleware/requireJwtAuth.js b/api/server/middleware/requireJwtAuth.js index 935957e913d..a34dcd69838 100644 --- a/api/server/middleware/requireJwtAuth.js +++ b/api/server/middleware/requireJwtAuth.js @@ -1,9 +1,14 @@ const cookies = require('cookie'); const jwt = require('jsonwebtoken'); const passport = require('passport'); +const { logger } = require('@librechat/data-schemas'); const { isEnabled, tenantContextMiddleware, + getAuthFailureReason, + getAuthFailureErrorName, + buildSafeAuthLogContext, + formatAuthLogMessage, maybeRefreshCloudFrontAuthCookiesMiddleware, } = require('@librechat/api'); @@ -48,6 +53,66 @@ const requireJwtAuth = (req, res, next) => { const useOpenIdJwt = tokenProvider === 'openid' && openidJwtAvailable && openIdReuseUserId != null; const strategies = useOpenIdJwt ? ['openidJwt', 'jwt'] : ['jwt']; + const authLogState = { + tokenProvider, + openidReuseEnabled, + openidJwtAvailable, + hasOpenIdReuseUserId: openIdReuseUserId != null, + }; + let primaryFailureReason; + let primaryFailureErrorName; + let fallbackAttempted = false; + + const logOpenIdFallbackAttempt = ({ fallbackStrategy, reason, errorName, status }) => { + primaryFailureReason = reason; + primaryFailureErrorName = errorName; + fallbackAttempted = true; + const message = '[requireJwtAuth] OpenID JWT auth failed; trying fallback'; + const context = buildSafeAuthLogContext(req, authLogState, { + primary_strategy: 'openidJwt', + fallback_strategy: fallbackStrategy, + fallback_attempted: true, + reason, + error_name: errorName, + status, + }); + logger.debug(formatAuthLogMessage(message, context), context); + }; + + const logAuthenticationFailure = ({ strategy, info, status, err }) => { + const message = '[requireJwtAuth] Authentication failed after all strategies'; + const context = buildSafeAuthLogContext(req, authLogState, { + primary_strategy: strategies[0], + fallback_strategy: strategies[1], + fallback_attempted: fallbackAttempted, + fallback_succeeded: false, + attempted_strategies: strategies, + final_strategy: strategy, + reason: getAuthFailureReason(err, info), + error_name: getAuthFailureErrorName(err, info), + status: status || 401, + }); + const log = fallbackAttempted ? logger.warn : logger.debug; + log.call(logger, formatAuthLogMessage(message, context), context); + }; + + const logFallbackSuccess = (strategy) => { + if (!fallbackAttempted || strategy !== 'jwt') { + return; + } + const message = '[requireJwtAuth] JWT fallback succeeded after OpenID JWT failure'; + const context = buildSafeAuthLogContext(req, authLogState, { + auth_strategy: 'jwt', + primary_strategy: 'openidJwt', + fallback_strategy: 'jwt', + fallback_attempted: true, + fallback_succeeded: true, + primary_failure_reason: primaryFailureReason, + reason: primaryFailureReason, + error_name: primaryFailureErrorName, + }); + logger.debug(formatAuthLogMessage(message, context), context); + }; const authenticateWithStrategy = (index) => { const strategy = strategies[index]; @@ -57,20 +122,34 @@ const requireJwtAuth = (req, res, next) => { } if (!user) { if (index + 1 < strategies.length) { + logOpenIdFallbackAttempt({ + fallbackStrategy: strategies[index + 1], + reason: getAuthFailureReason(err, info), + errorName: getAuthFailureErrorName(err, info), + status: status || 401, + }); return authenticateWithStrategy(index + 1); } + logAuthenticationFailure({ strategy, info, status, err }); return res.status(status || 401).json({ message: info?.message || 'Unauthorized', }); } if (strategy === 'openidJwt' && getAuthenticatedUserId(user) !== openIdReuseUserId) { if (index + 1 < strategies.length) { + logOpenIdFallbackAttempt({ + fallbackStrategy: strategies[index + 1], + reason: 'openid user-id mismatch', + status: 401, + }); return authenticateWithStrategy(index + 1); } + logAuthenticationFailure({ strategy, info, status: 401, err }); return res.status(401).json({ message: 'Unauthorized' }); } req.user = user; req.authStrategy = strategy; + logFallbackSuccess(strategy); tenantContextMiddleware(req, res, (tenantErr) => { if (tenantErr) { return next(tenantErr); diff --git a/packages/api/src/middleware/auth.spec.ts b/packages/api/src/middleware/auth.spec.ts new file mode 100644 index 00000000000..1c39b387df3 --- /dev/null +++ b/packages/api/src/middleware/auth.spec.ts @@ -0,0 +1,206 @@ +import { + buildSafeAuthLogContext, + formatAuthLogMessage, + getAuthFailureErrorName, + getAuthFailureReason, +} from './auth'; +import type { AuthLogRequest, AuthLogState } from './auth'; + +function createRequest(overrides: Partial = {}): AuthLogRequest { + return { + headers: {}, + method: 'GET', + path: '/api/messages', + originalUrl: '/api/messages', + ...overrides, + }; +} + +function createAuthState(overrides: Partial = {}): AuthLogState { + return { + tokenProvider: 'openid', + openidReuseEnabled: true, + openidJwtAvailable: true, + hasOpenIdReuseUserId: true, + ...overrides, + }; +} + +describe('auth middleware logging helpers', () => { + it('builds safe auth log context without raw query strings or user identifiers', () => { + const log = buildSafeAuthLogContext( + createRequest({ + id: 'request-id', + path: undefined, + originalUrl: '/api/ask?access_token=secret-token', + }), + createAuthState(), + { + attempted_strategies: ['openidJwt', 'jwt'], + fallback_attempted: true, + fallback_succeeded: false, + reason: 'jwt expired', + error_name: 'TokenExpiredError', + status: 401, + }, + ); + + expect(log).toEqual({ + request_id: 'request-id', + method: 'GET', + path: '/api/ask', + token_provider: 'openid', + openid_reuse_enabled: true, + openid_jwt_available: true, + has_openid_reuse_user_id: true, + attempted_strategies: ['openidJwt', 'jwt'], + fallback_attempted: true, + fallback_succeeded: false, + reason: 'jwt expired', + error_name: 'TokenExpiredError', + status: 401, + }); + expect(JSON.stringify(log)).not.toContain('secret-token'); + }); + + it('uses request headers when request ids are not directly set', () => { + const log = buildSafeAuthLogContext( + createRequest({ + headers: { + 'x-request-id': ['header-request-id'], + }, + }), + createAuthState({ + tokenProvider: null, + openidReuseEnabled: false, + openidJwtAvailable: false, + hasOpenIdReuseUserId: false, + }), + ); + + expect(log).toEqual({ + request_id: 'header-request-id', + method: 'GET', + path: '/api/messages', + openid_reuse_enabled: false, + openid_jwt_available: false, + has_openid_reuse_user_id: false, + }); + }); + + it('buckets unknown token providers to keep auth logs low-cardinality', () => { + const log = buildSafeAuthLogContext( + createRequest(), + createAuthState({ + tokenProvider: 'attacker-controlled-provider', + }), + ); + + expect(log.token_provider).toBe('other'); + }); + + it('prefers route buckets over concrete dynamic request paths', () => { + const log = buildSafeAuthLogContext( + createRequest({ + baseUrl: '/api/messages', + path: '/conversation-123/message-456', + originalUrl: '/api/messages/conversation-123/message-456?access_token=secret-token', + }), + createAuthState(), + ); + + expect(log.path).toBe('/api/messages'); + expect(JSON.stringify(log)).not.toContain('conversation-123'); + expect(JSON.stringify(log)).not.toContain('message-456'); + expect(JSON.stringify(log)).not.toContain('secret-token'); + }); + + it('logs route templates when Express exposes them', () => { + const log = buildSafeAuthLogContext( + createRequest({ + baseUrl: '/api/share', + path: '/link/conversation-123', + route: { path: '/link/:conversationId' }, + }), + createAuthState(), + ); + + expect(log.path).toBe('/api/share/link/:conversationId'); + }); + + it('drops unsupported extra values and keeps safe arrays primitive', () => { + const log = buildSafeAuthLogContext(createRequest({ id: 'request-id' }), createAuthState(), { + attempted_strategies: ['openidJwt', '', { strategy: 'jwt' }, 'jwt'], + fallback_attempted: true, + path: { unsafe: true }, + request_id: { unsafe: true }, + status: Number.NaN, + unsafe_object: { token: 'secret-token' }, + reason: ' jwt expired ', + }); + + expect(log).toEqual({ + request_id: 'request-id', + method: 'GET', + path: '/api/messages', + token_provider: 'openid', + openid_reuse_enabled: true, + openid_jwt_available: true, + has_openid_reuse_user_id: true, + attempted_strategies: ['openidJwt', 'jwt'], + fallback_attempted: true, + reason: 'jwt expired', + }); + expect(JSON.stringify(log)).not.toContain('secret-token'); + }); + + it('formats auth log messages with serialized safe context for stdout collectors', () => { + const log = buildSafeAuthLogContext(createRequest({ id: 'request-id' }), createAuthState(), { + fallback_attempted: true, + reason: 'jwt expired', + error_name: 'TokenExpiredError', + status: 401, + }); + + expect( + formatAuthLogMessage('[requireJwtAuth] OpenID JWT auth failed; trying fallback', log), + ).toBe( + '[requireJwtAuth] OpenID JWT auth failed; trying fallback {"fallback_attempted":true,"reason":"jwt expired","error_name":"TokenExpiredError","status":401,"request_id":"request-id","method":"GET","path":"/api/messages","token_provider":"openid","openid_reuse_enabled":true,"openid_jwt_available":true,"has_openid_reuse_user_id":true}', + ); + }); + + it('prefers Passport info fields for auth failure reason and error name', () => { + const err = Object.assign(new Error('outer failure'), { name: 'OuterError' }); + const info = { message: 'jwt expired', name: 'TokenExpiredError' }; + + expect(getAuthFailureReason(err, info)).toBe('jwt expired'); + expect(getAuthFailureErrorName(err, info)).toBe('TokenExpiredError'); + }); + + it('falls back to Error fields when Passport info is absent', () => { + const err = Object.assign(new Error('invalid signature'), { name: 'JsonWebTokenError' }); + + expect(getAuthFailureReason(err, undefined)).toBe('invalid signature'); + expect(getAuthFailureErrorName(err, undefined)).toBe('JsonWebTokenError'); + }); + + it('does not throw when Passport failure objects expose throwing getters', () => { + const err = Object.assign(new Error('invalid signature'), { name: 'JsonWebTokenError' }); + const info = {}; + Object.defineProperties(info, { + message: { + get() { + throw new TypeError('message getter failed'); + }, + }, + name: { + get() { + throw new TypeError('name getter failed'); + }, + }, + }); + + expect(getAuthFailureReason(err, info)).toBe('invalid signature'); + expect(getAuthFailureErrorName(err, info)).toBe('JsonWebTokenError'); + }); +}); diff --git a/packages/api/src/middleware/auth.ts b/packages/api/src/middleware/auth.ts new file mode 100644 index 00000000000..2ae429f6a05 --- /dev/null +++ b/packages/api/src/middleware/auth.ts @@ -0,0 +1,228 @@ +type AuthFailureLike = { + message?: unknown; + name?: unknown; +}; + +type AuthLogValue = string | number | boolean | readonly string[]; +type AuthLogHeaderValue = string | string[] | undefined; +type AuthRoutePath = string | RegExp | readonly (string | RegExp)[]; + +export type AuthLogRequest = { + headers?: Record; + method?: string; + path?: string; + originalUrl?: string; + url?: string; + baseUrl?: string; + route?: { + path?: AuthRoutePath; + }; + id?: string; + requestId?: string; +}; + +export type AuthLogState = { + tokenProvider?: string | null; + openidReuseEnabled: boolean; + openidJwtAvailable: boolean; + hasOpenIdReuseUserId: boolean; +}; + +export type AuthLogContext = Record; + +function normalizeAuthLogValue(value: unknown): string | undefined { + if (value == null) { + return undefined; + } + + if (Array.isArray(value)) { + for (const entry of value) { + const normalized = normalizeAuthLogValue(entry); + if (normalized) { + return normalized; + } + } + return undefined; + } + + if (typeof value === 'string') { + const trimmed = value.trim(); + return trimmed || undefined; + } + + if (typeof value === 'number' || typeof value === 'boolean') { + return String(value); + } + + return undefined; +} + +function normalizeAuthLogContextValue(value: unknown): AuthLogValue | undefined { + if (value == null) { + return undefined; + } + + if (Array.isArray(value)) { + const values = value + .map((entry) => normalizeAuthLogValue(entry)) + .filter((entry): entry is string => entry !== undefined); + return values.length > 0 ? values : undefined; + } + + if (typeof value === 'string') { + return normalizeAuthLogValue(value); + } + + if (typeof value === 'number') { + return Number.isFinite(value) ? value : undefined; + } + + if (typeof value === 'boolean') { + return value; + } + + return undefined; +} + +function getRequestId(req: AuthLogRequest): string | undefined { + return ( + normalizeAuthLogValue(req.requestId) ?? + normalizeAuthLogValue(req.id) ?? + normalizeAuthLogValue(req.headers?.['x-request-id']) ?? + normalizeAuthLogValue(req.headers?.['x-correlation-id']) + ); +} + +function normalizeRoutePath(path: AuthRoutePath | undefined): string | undefined { + if (typeof path === 'string') { + return normalizeAuthLogValue(path); + } + + if (Array.isArray(path)) { + for (const entry of path) { + const normalized = normalizeRoutePath(entry); + if (normalized) { + return normalized; + } + } + } + + return undefined; +} + +function joinRoutePath(baseUrl: string | undefined, routePath: string): string { + const normalizedRoute = routePath === '/' ? '' : routePath; + if (!baseUrl) { + return normalizedRoute || '/'; + } + if (!normalizedRoute) { + return baseUrl; + } + return `${baseUrl.replace(/\/$/, '')}/${normalizedRoute.replace(/^\//, '')}`; +} + +function bucketConcretePath(path: string | undefined): string | undefined { + const queryless = path?.split('?')[0]; + if (!queryless) { + return undefined; + } + + const segments = queryless.split('/').filter(Boolean); + if (segments.length === 0) { + return '/'; + } + if (segments[0] === 'api' && segments[1]) { + return `/${segments.slice(0, 2).join('/')}`; + } + return `/${segments[0]}`; +} + +function getRequestPath(req: AuthLogRequest): string | undefined { + const baseUrl = normalizeAuthLogValue(req.baseUrl); + const routePath = normalizeRoutePath(req.route?.path); + if (routePath) { + return joinRoutePath(baseUrl, routePath); + } + if (baseUrl) { + return baseUrl; + } + + const path = normalizeAuthLogValue(req.path) ?? normalizeAuthLogValue(req.originalUrl ?? req.url); + return bucketConcretePath(path); +} + +function getAuthFailureField(source: unknown, field: keyof AuthFailureLike): unknown { + if (!source) { + return undefined; + } + if (typeof source === 'string') { + return field === 'message' ? source : undefined; + } + if (typeof source === 'object') { + try { + return (source as AuthFailureLike)[field]; + } catch { + return undefined; + } + } + return undefined; +} + +function compactAuthLogContext(log: Record): AuthLogContext { + const compacted: Partial = {}; + for (const key of Object.keys(log)) { + const value = normalizeAuthLogContextValue(log[key]); + if (value !== undefined) { + Object.assign(compacted, { [key]: value }); + } + } + return compacted as AuthLogContext; +} + +export function getAuthFailureReason( + err: unknown, + info: unknown, + fallback = 'Unauthorized', +): string { + return ( + normalizeAuthLogValue(getAuthFailureField(info, 'message')) ?? + normalizeAuthLogValue(getAuthFailureField(err, 'message')) ?? + fallback + ); +} + +export function getAuthFailureErrorName(err: unknown, info: unknown): string | undefined { + return ( + normalizeAuthLogValue(getAuthFailureField(info, 'name')) ?? + normalizeAuthLogValue(getAuthFailureField(err, 'name')) + ); +} + +function getSafeTokenProvider(tokenProvider: unknown): string | undefined { + const normalized = normalizeAuthLogValue(tokenProvider); + if (!normalized) { + return undefined; + } + return normalized === 'openid' || normalized === 'librechat' ? normalized : 'other'; +} + +export function buildSafeAuthLogContext( + req: AuthLogRequest, + authState: AuthLogState, + extra: Record = {}, +): AuthLogContext { + return compactAuthLogContext({ + ...extra, + request_id: getRequestId(req), + method: normalizeAuthLogValue(req.method), + path: getRequestPath(req), + token_provider: getSafeTokenProvider(authState.tokenProvider), + openid_reuse_enabled: authState.openidReuseEnabled, + openid_jwt_available: authState.openidJwtAvailable, + has_openid_reuse_user_id: authState.hasOpenIdReuseUserId, + }); +} + +export function formatAuthLogMessage(message: string, context: AuthLogContext): string { + return `${message} ${JSON.stringify(context)}`; +} diff --git a/packages/api/src/middleware/index.ts b/packages/api/src/middleware/index.ts index cac5d637b97..6ac95683fe2 100644 --- a/packages/api/src/middleware/index.ts +++ b/packages/api/src/middleware/index.ts @@ -5,6 +5,7 @@ export * from './notFound'; export * from './balance'; export * from './json'; export * from './capabilities'; +export * from './auth'; export { tenantContextMiddleware, restoreTenantContextFromReq, From 86fe79c37d148a08c3ff2cda6b4aaa6dea57c16b Mon Sep 17 00:00:00 2001 From: Atef Bellaaj Date: Wed, 3 Jun 2026 20:17:17 +0200 Subject: [PATCH 004/160] =?UTF-8?q?=F0=9F=94=97=20feat:=20Add=20Granular?= =?UTF-8?q?=20Access=20Control=20to=20Shared=20Links=20via=20ACL=20System?= =?UTF-8?q?=20(#13051)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Add granular access control to shared links via ACL system * fix(shared-links): preserve isPublic on failed migration grants Transient ACL failures during auto-migration permanently stranded links — $unset ran unconditionally, removing the legacy flag that triggers retry. Now only $unset isPublic after all grants succeed. * fix(config): skip isPublic unset for failed ACL grants Bulk migration unconditionally removed isPublic from all links, even those whose ACL writes failed. Failed links then lost the legacy marker needed for auto-migration retry. Now tracks failed link IDs per-batch and excludes them from the $unset step. Also adds sharedLink to AccessRole resourceType schema enum — was missing, only worked because seedDefaultRoles uses findOneAndUpdate which bypasses validation. * ci(config): add jest config and PR workflow for migration tests config/__tests__/ specs depend on api/jest.config.js module mappings but had no dedicated runner. Adds config/jest.config.js extending api config with absolutized paths, npm test:config script, and a GitHub Actions workflow triggered by changes to config/, api/models/, api/db/, or packages/ ACL code. * fix(permissions): honor boolean sharedLinks config SHARED_LINKS has no USE permission, so boolean config produced an empty update payload — gate conditions only matched object form, making `sharedLinks: false` a no-op on existing perms. * fix(share): resolve role before creating shared link Role lookup between create and grant left an orphaned link without ACL entries if getRoleByName threw — retry then hit "Share already exists" with no recovery path. * fix: Restore Public ACL Access Checks * fix: Type Public ACL Lookup * fix: Preserve Private Legacy Shared Links * chore: Promote Shared Link Permission Migration * fix: Address Shared Link Review Findings * fix: Repair Shared Link CI Follow-Up * fix: Narrow Shared Link Mongoose Test Mock * fix: Address Shared Link Review Follow-Ups * fix: Close Shared Link Review Gaps * fix: Guard Missing Shared Link Permission Backfill * test: Add Shared Link Mock E2E * test: Stabilize Shared Link Mock E2E --------- Co-authored-by: Danny Avila --- .github/workflows/config-review.yml | 88 +++++ .../controllers/PermissionsController.js | 6 +- api/server/controllers/UserController.js | 3 +- .../controllers/__tests__/deleteUser.spec.js | 3 + .../deleteUserResourceCoverage.spec.js | 1 + api/server/middleware/canAccessSharedLink.js | 6 + .../__test-utils__/convos-route-mocks.js | 2 + api/server/routes/__tests__/convos.spec.js | 99 +++-- api/server/routes/__tests__/share.spec.js | 67 +++- api/server/routes/accessPermissions.js | 66 +++- .../accessPermissions.sharePolicy.test.js | 109 ++++++ api/server/routes/convos.js | 6 +- api/server/routes/share.js | 71 ++-- .../components/Chat/ExportAndShareMenu.tsx | 11 +- .../ConvoOptions/ConvoOptions.tsx | 14 +- .../ConvoOptions/ShareButton.tsx | 7 +- .../ConvoOptions/SharedLinkButton.tsx | 46 ++- .../Nav/SettingsTabs/Data/SharedLinks.tsx | 1 - client/src/components/Share/MessagesView.tsx | 2 +- .../components/Sharing/AccessRolesPicker.tsx | 10 +- .../Sharing/GenericGrantAccessDialog.tsx | 8 +- .../PeoplePicker/SelectedPrincipalsList.tsx | 48 ++- .../Sharing/PeoplePickerAdminSettings.tsx | 2 +- .../Sharing/PublicSharingToggle.tsx | 3 +- client/src/data-provider/queries.ts | 5 +- client/src/hooks/Sharing/useCanSharePublic.ts | 3 +- client/src/locales/en/translation.json | 2 + client/src/utils/resources.ts | 10 + client/src/utils/roles.ts | 9 + .../migrate-shared-link-permissions.spec.js | 218 +++++++++++ config/jest.config.js | 18 + config/migrate-shared-link-permissions.js | 357 +++++++++++++++++ e2e/specs/mock/shared-links.spec.ts | 187 +++++++++ librechat.example.yaml | 6 + package.json | 4 + .../api/src/acl/accessControlService.spec.ts | 60 +++ packages/api/src/acl/accessControlService.ts | 40 +- packages/api/src/app/permissions.spec.ts | 144 +++++++ packages/api/src/app/permissions.ts | 60 +++ packages/api/src/index.ts | 3 + packages/api/src/middleware/share.spec.ts | 40 +- packages/api/src/middleware/share.ts | 13 +- packages/api/src/shared-links/access.test.ts | 325 +++++++++++++++ packages/api/src/shared-links/access.ts | 137 +++++++ packages/api/src/shared-links/service.test.ts | 369 ++++++++++++++++++ packages/api/src/shared-links/service.ts | 324 +++++++++++++++ .../data-provider/src/accessPermissions.ts | 9 +- packages/data-provider/src/api-endpoints.ts | 3 +- packages/data-provider/src/config.ts | 15 + packages/data-provider/src/data-service.ts | 6 +- packages/data-provider/src/permissions.ts | 13 + packages/data-provider/src/roles.spec.ts | 3 +- packages/data-provider/src/roles.ts | 16 + packages/data-provider/src/schemas.ts | 1 - packages/data-provider/src/types.ts | 4 +- packages/data-provider/src/types/queries.ts | 2 - .../data-schemas/src/admin/capabilities.ts | 6 + packages/data-schemas/src/app/interface.ts | 1 + .../src/methods/accessRole.spec.ts | 2 + .../data-schemas/src/methods/accessRole.ts | 14 + packages/data-schemas/src/methods/aclEntry.ts | 6 +- .../data-schemas/src/methods/share.test.ts | 132 +------ packages/data-schemas/src/methods/share.ts | 60 +-- .../data-schemas/src/schema/accessRole.ts | 11 +- packages/data-schemas/src/schema/aclEntry.ts | 4 + packages/data-schemas/src/schema/role.ts | 5 + packages/data-schemas/src/schema/share.ts | 5 - packages/data-schemas/src/types/aclEntry.ts | 2 + packages/data-schemas/src/types/role.ts | 5 + packages/data-schemas/src/types/share.ts | 7 +- 70 files changed, 3057 insertions(+), 288 deletions(-) create mode 100644 .github/workflows/config-review.yml create mode 100644 api/server/middleware/canAccessSharedLink.js create mode 100644 config/__tests__/migrate-shared-link-permissions.spec.js create mode 100644 config/jest.config.js create mode 100644 config/migrate-shared-link-permissions.js create mode 100644 e2e/specs/mock/shared-links.spec.ts create mode 100644 packages/api/src/shared-links/access.test.ts create mode 100644 packages/api/src/shared-links/access.ts create mode 100644 packages/api/src/shared-links/service.test.ts create mode 100644 packages/api/src/shared-links/service.ts diff --git a/.github/workflows/config-review.yml b/.github/workflows/config-review.yml new file mode 100644 index 00000000000..e08662aa1c3 --- /dev/null +++ b/.github/workflows/config-review.yml @@ -0,0 +1,88 @@ +name: Config Migration Tests +on: + pull_request: + paths: + - 'config/**' + - 'api/models/**' + - 'api/db/**' + - 'packages/data-schemas/src/**' + - 'packages/data-provider/src/**' + - 'packages/api/src/acl/**' + - 'packages/api/src/shared-links/**' + +env: + NODE_ENV: CI + NODE_OPTIONS: '--max-old-space-size=${{ secrets.NODE_MAX_OLD_SPACE_SIZE || 6144 }}' + +jobs: + test-config: + name: 'Tests: config migrations' + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@v4 + + - name: Use Node.js 20.19 + uses: actions/setup-node@v4 + with: + node-version: '20.19' + + - name: Restore node_modules cache + id: cache-node-modules + uses: actions/cache@v4 + with: + path: | + node_modules + api/node_modules + packages/api/node_modules + packages/data-provider/node_modules + packages/data-schemas/node_modules + key: node-modules-backend-${{ runner.os }}-20.19-${{ hashFiles('package-lock.json') }} + + - name: Install dependencies + if: steps.cache-node-modules.outputs.cache-hit != 'true' + run: npm ci + + - name: Restore data-provider build cache + id: cache-data-provider + uses: actions/cache@v4 + with: + path: packages/data-provider/dist + key: build-data-provider-${{ runner.os }}-${{ hashFiles('packages/data-provider/src/**', 'packages/data-provider/tsconfig*.json', 'packages/data-provider/rollup.config.js', 'packages/data-provider/package.json') }} + + - name: Build data-provider + if: steps.cache-data-provider.outputs.cache-hit != 'true' + run: npm run build:data-provider + + - name: Restore data-schemas build cache + id: cache-data-schemas + uses: actions/cache@v4 + with: + path: packages/data-schemas/dist + key: build-data-schemas-${{ runner.os }}-${{ hashFiles('packages/data-schemas/src/**', 'packages/data-schemas/tsconfig*.json', 'packages/data-schemas/rollup.config.js', 'packages/data-schemas/package.json', 'packages/data-provider/src/**', 'packages/data-provider/tsconfig*.json', 'packages/data-provider/rollup.config.js', 'packages/data-provider/package.json') }} + + - name: Build data-schemas + if: steps.cache-data-schemas.outputs.cache-hit != 'true' + run: npm run build:data-schemas + + - name: Restore api build cache + id: cache-api + uses: actions/cache@v4 + with: + path: packages/api/dist + key: build-api-${{ runner.os }}-${{ hashFiles('packages/api/src/**', 'packages/api/tsconfig*.json', 'packages/api/server-rollup.config.js', 'packages/api/package.json', 'packages/data-provider/src/**', 'packages/data-provider/tsconfig*.json', 'packages/data-provider/rollup.config.js', 'packages/data-provider/package.json', 'packages/data-schemas/src/**', 'packages/data-schemas/tsconfig*.json', 'packages/data-schemas/rollup.config.js', 'packages/data-schemas/package.json') }} + + - name: Build api + if: steps.cache-api.outputs.cache-hit != 'true' + run: npm run build:api + + - name: Create empty auth.json file + run: | + mkdir -p api/data + echo '{}' > api/data/auth.json + + - name: Prepare .env.test file + run: cp api/test/.env.test.example api/test/.env.test + + - name: Run config migration tests + run: npm run test:config diff --git a/api/server/controllers/PermissionsController.js b/api/server/controllers/PermissionsController.js index ffe159a82cc..000bba7671c 100644 --- a/api/server/controllers/PermissionsController.js +++ b/api/server/controllers/PermissionsController.js @@ -134,8 +134,8 @@ const updateResourcePermissions = async (req, res) => { revokedPrincipals.push(...removed); } - // If public is disabled, add public to revoked list - if (!isPublic) { + // If public is explicitly disabled, add public to revoked list + if (isPublic === false) { revokedPrincipals.push({ type: PrincipalType.PUBLIC, id: null, @@ -167,7 +167,7 @@ const updateResourcePermissions = async (req, res) => { message: 'Permissions updated successfully', results: { principals: results.granted, - public: isPublic || false, + ...(isPublic !== undefined ? { public: isPublic } : {}), publicAccessRoleId: isPublic ? publicAccessRoleId : undefined, }, }; diff --git a/api/server/controllers/UserController.js b/api/server/controllers/UserController.js index ca560389e6a..7884d0a6111 100644 --- a/api/server/controllers/UserController.js +++ b/api/server/controllers/UserController.js @@ -7,6 +7,7 @@ const { MCPTokenStorage, normalizeHttpError, extractWebSearchEnvVars, + deleteAllSharedLinksWithCleanup, } = require('@librechat/api'); const { Tools, @@ -359,7 +360,7 @@ const deleteUserController = async (req, res) => { } await deleteUserPluginAuth(user.id, null, true); await db.deleteUserById(user.id); - await db.deleteAllSharedLinks(user.id); + await deleteAllSharedLinksWithCleanup(user.id); await deleteUserFiles(req); await db.deleteFiles(null, user.id); await db.deleteToolCalls(user.id); diff --git a/api/server/controllers/__tests__/deleteUser.spec.js b/api/server/controllers/__tests__/deleteUser.spec.js index 1d7c8521531..6198122bd05 100644 --- a/api/server/controllers/__tests__/deleteUser.spec.js +++ b/api/server/controllers/__tests__/deleteUser.spec.js @@ -3,6 +3,7 @@ const mockDeleteMessages = jest.fn(); const mockDeleteAllUserSessions = jest.fn(); const mockDeleteUserById = jest.fn(); const mockDeleteAllSharedLinks = jest.fn(); +const mockDeleteAllSharedLinksWithCleanup = jest.fn(); const mockDeletePresets = jest.fn(); const mockDeleteUserKey = jest.fn(); const mockDeleteConvos = jest.fn(); @@ -38,6 +39,7 @@ jest.mock('@librechat/api', () => ({ extractWebSearchEnvVars: jest.fn(), needsRefresh: jest.fn(), getNewS3URL: jest.fn(), + deleteAllSharedLinksWithCleanup: (...args) => mockDeleteAllSharedLinksWithCleanup(...args), })); jest.mock('~/models', () => ({ @@ -126,6 +128,7 @@ function stubDeletionMocks() { mockDeleteUserPluginAuth.mockResolvedValue(); mockDeleteUserById.mockResolvedValue(); mockDeleteAllSharedLinks.mockResolvedValue(); + mockDeleteAllSharedLinksWithCleanup.mockResolvedValue({ deletedCount: 0 }); mockGetFiles.mockResolvedValue([]); mockProcessDeleteRequest.mockResolvedValue({ deletedFileIds: [], failedFileIds: [] }); mockDeleteFiles.mockResolvedValue(); diff --git a/api/server/controllers/__tests__/deleteUserResourceCoverage.spec.js b/api/server/controllers/__tests__/deleteUserResourceCoverage.spec.js index 78fcfa16b0b..1bd5b2efaa5 100644 --- a/api/server/controllers/__tests__/deleteUserResourceCoverage.spec.js +++ b/api/server/controllers/__tests__/deleteUserResourceCoverage.spec.js @@ -16,6 +16,7 @@ const HANDLED_RESOURCE_TYPES = { [ResourceType.PROMPTGROUP]: 'deleteUserPrompts', [ResourceType.MCPSERVER]: 'deleteUserMcpServers', [ResourceType.SKILL]: 'deleteUserSkills', + [ResourceType.SHARED_LINK]: 'deleteAllSharedLinksWithCleanup', }; /** diff --git a/api/server/middleware/canAccessSharedLink.js b/api/server/middleware/canAccessSharedLink.js new file mode 100644 index 00000000000..79fd93e486c --- /dev/null +++ b/api/server/middleware/canAccessSharedLink.js @@ -0,0 +1,6 @@ +const mongoose = require('mongoose'); +const { createSharedLinkAccessMiddleware } = require('@librechat/api'); + +const canAccessSharedLink = createSharedLinkAccessMiddleware({ mongoose }); + +module.exports = canAccessSharedLink; diff --git a/api/server/routes/__test-utils__/convos-route-mocks.js b/api/server/routes/__test-utils__/convos-route-mocks.js index a3718addff0..a0eb6fe3128 100644 --- a/api/server/routes/__test-utils__/convos-route-mocks.js +++ b/api/server/routes/__test-utils__/convos-route-mocks.js @@ -12,6 +12,8 @@ module.exports = { })), logAxiosError: jest.fn(), restoreTenantContextFromReq: jest.fn((req, res, next) => next()), + deleteConvoSharedLinksWithCleanup: jest.fn(), + deleteAllSharedLinksWithCleanup: jest.fn(), ...overrides, }), diff --git a/api/server/routes/__tests__/convos.spec.js b/api/server/routes/__tests__/convos.spec.js index 23978f28e9b..2f766694606 100644 --- a/api/server/routes/__tests__/convos.spec.js +++ b/api/server/routes/__tests__/convos.spec.js @@ -21,13 +21,11 @@ jest.mock('~/server/services/Endpoints/assistants', () => require(MOCKS).assista describe('Convos Routes', () => { let app; let convosRouter; + const { deleteToolCalls, deleteConvos, saveConvo } = require('~/models'); const { - deleteAllSharedLinks, - deleteConvoSharedLink, - deleteToolCalls, - deleteConvos, - saveConvo, - } = require('~/models'); + deleteAllSharedLinksWithCleanup, + deleteConvoSharedLinksWithCleanup, + } = require('@librechat/api'); beforeAll(() => { convosRouter = require('../convos'); @@ -57,7 +55,7 @@ describe('Convos Routes', () => { deleteConvos.mockResolvedValue(mockDbResponse); deleteToolCalls.mockResolvedValue({ deletedCount: 10 }); - deleteAllSharedLinks.mockResolvedValue({ + deleteAllSharedLinksWithCleanup.mockResolvedValue({ message: 'All shared links deleted successfully', deletedCount: 3, }); @@ -75,12 +73,12 @@ describe('Convos Routes', () => { expect(deleteToolCalls).toHaveBeenCalledWith('test-user-123'); expect(deleteToolCalls).toHaveBeenCalledTimes(1); - /** Verify deleteAllSharedLinks was called with correct userId */ - expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-123'); - expect(deleteAllSharedLinks).toHaveBeenCalledTimes(1); + /** Verify deleteAllSharedLinksWithCleanup was called with correct userId */ + expect(deleteAllSharedLinksWithCleanup).toHaveBeenCalledWith('test-user-123'); + expect(deleteAllSharedLinksWithCleanup).toHaveBeenCalledTimes(1); }); - it('should call deleteAllSharedLinks even when no conversations exist', async () => { + it('should call deleteAllSharedLinksWithCleanup even when no conversations exist', async () => { const mockDbResponse = { deletedCount: 0, message: 'No conversations to delete', @@ -88,7 +86,7 @@ describe('Convos Routes', () => { deleteConvos.mockResolvedValue(mockDbResponse); deleteToolCalls.mockResolvedValue({ deletedCount: 0 }); - deleteAllSharedLinks.mockResolvedValue({ + deleteAllSharedLinksWithCleanup.mockResolvedValue({ message: 'All shared links deleted successfully', deletedCount: 0, }); @@ -96,7 +94,7 @@ describe('Convos Routes', () => { const response = await request(app).delete('/api/convos/all'); expect(response.status).toBe(201); - expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-123'); + expect(deleteAllSharedLinksWithCleanup).toHaveBeenCalledWith('test-user-123'); }); it('should return 500 if deleteConvos fails', async () => { @@ -123,10 +121,10 @@ describe('Convos Routes', () => { expect(response.text).toBe('Error clearing conversations'); }); - it('should return 500 if deleteAllSharedLinks fails', async () => { + it('should return 500 if deleteAllSharedLinksWithCleanup fails', async () => { deleteConvos.mockResolvedValue({ deletedCount: 5 }); deleteToolCalls.mockResolvedValue({ deletedCount: 10 }); - deleteAllSharedLinks.mockRejectedValue(new Error('Shared links deletion failed')); + deleteAllSharedLinksWithCleanup.mockRejectedValue(new Error('Shared links deletion failed')); const response = await request(app).delete('/api/convos/all'); @@ -138,12 +136,12 @@ describe('Convos Routes', () => { /** First user */ deleteConvos.mockResolvedValue({ deletedCount: 3 }); deleteToolCalls.mockResolvedValue({ deletedCount: 5 }); - deleteAllSharedLinks.mockResolvedValue({ deletedCount: 2 }); + deleteAllSharedLinksWithCleanup.mockResolvedValue({ deletedCount: 2 }); let response = await request(app).delete('/api/convos/all'); expect(response.status).toBe(201); - expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-123'); + expect(deleteAllSharedLinksWithCleanup).toHaveBeenCalledWith('test-user-123'); jest.clearAllMocks(); @@ -158,12 +156,12 @@ describe('Convos Routes', () => { deleteConvos.mockResolvedValue({ deletedCount: 7 }); deleteToolCalls.mockResolvedValue({ deletedCount: 12 }); - deleteAllSharedLinks.mockResolvedValue({ deletedCount: 4 }); + deleteAllSharedLinksWithCleanup.mockResolvedValue({ deletedCount: 4 }); response = await request(app2).delete('/api/convos/all'); expect(response.status).toBe(201); - expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-456'); + expect(deleteAllSharedLinksWithCleanup).toHaveBeenCalledWith('test-user-456'); }); it('should execute deletions in correct sequence', async () => { @@ -179,15 +177,19 @@ describe('Convos Routes', () => { return Promise.resolve({ deletedCount: 10 }); }); - deleteAllSharedLinks.mockImplementation(() => { - executionOrder.push('deleteAllSharedLinks'); + deleteAllSharedLinksWithCleanup.mockImplementation(() => { + executionOrder.push('deleteAllSharedLinksWithCleanup'); return Promise.resolve({ deletedCount: 3 }); }); await request(app).delete('/api/convos/all'); /** Verify all three functions were called */ - expect(executionOrder).toEqual(['deleteConvos', 'deleteToolCalls', 'deleteAllSharedLinks']); + expect(executionOrder).toEqual([ + 'deleteConvos', + 'deleteToolCalls', + 'deleteAllSharedLinksWithCleanup', + ]); }); it('should maintain data integrity by cleaning up shared links when conversations are deleted', async () => { @@ -201,17 +203,17 @@ describe('Convos Routes', () => { deleteConvos.mockResolvedValue(mockConvosDeleted); deleteToolCalls.mockResolvedValue(mockToolCallsDeleted); - deleteAllSharedLinks.mockResolvedValue(mockSharedLinksDeleted); + deleteAllSharedLinksWithCleanup.mockResolvedValue(mockSharedLinksDeleted); const response = await request(app).delete('/api/convos/all'); expect(response.status).toBe(201); /** Verify that shared links cleanup was called for the same user */ - expect(deleteAllSharedLinks).toHaveBeenCalledWith('test-user-123'); + expect(deleteAllSharedLinksWithCleanup).toHaveBeenCalledWith('test-user-123'); /** Verify no shared links remain for deleted conversations */ - expect(deleteAllSharedLinks).toHaveBeenCalledAfter(deleteConvos); + expect(deleteAllSharedLinksWithCleanup).toHaveBeenCalledAfter(deleteConvos); }); }); @@ -225,7 +227,7 @@ describe('Convos Routes', () => { deleteConvos.mockResolvedValue(mockDbResponse); deleteToolCalls.mockResolvedValue({ deletedCount: 3 }); - deleteConvoSharedLink.mockResolvedValue({ + deleteConvoSharedLinksWithCleanup.mockResolvedValue({ message: 'Shared links deleted successfully', deletedCount: 1, }); @@ -249,11 +251,14 @@ describe('Convos Routes', () => { /** Verify deleteToolCalls was called */ expect(deleteToolCalls).toHaveBeenCalledWith('test-user-123', mockConversationId); - /** Verify deleteConvoSharedLink was called */ - expect(deleteConvoSharedLink).toHaveBeenCalledWith('test-user-123', mockConversationId); + /** Verify deleteConvoSharedLinksWithCleanup was called */ + expect(deleteConvoSharedLinksWithCleanup).toHaveBeenCalledWith( + 'test-user-123', + mockConversationId, + ); }); - it('should not call deleteConvoSharedLink when no conversationId provided', async () => { + it('should not call deleteConvoSharedLinksWithCleanup when no conversationId provided', async () => { deleteConvos.mockResolvedValue({ deletedCount: 0 }); deleteToolCalls.mockResolvedValue({ deletedCount: 0 }); @@ -266,7 +271,7 @@ describe('Convos Routes', () => { }); expect(response.status).toBe(200); - expect(deleteConvoSharedLink).not.toHaveBeenCalled(); + expect(deleteConvoSharedLinksWithCleanup).not.toHaveBeenCalled(); }); it('should handle deletion of conversation without shared links', async () => { @@ -274,7 +279,7 @@ describe('Convos Routes', () => { deleteConvos.mockResolvedValue({ deletedCount: 1 }); deleteToolCalls.mockResolvedValue({ deletedCount: 0 }); - deleteConvoSharedLink.mockResolvedValue({ + deleteConvoSharedLinksWithCleanup.mockResolvedValue({ message: 'Shared links deleted successfully', deletedCount: 0, }); @@ -288,7 +293,10 @@ describe('Convos Routes', () => { }); expect(response.status).toBe(201); - expect(deleteConvoSharedLink).toHaveBeenCalledWith('test-user-123', mockConversationId); + expect(deleteConvoSharedLinksWithCleanup).toHaveBeenCalledWith( + 'test-user-123', + mockConversationId, + ); }); it('should return 400 when no parameters provided', async () => { @@ -299,7 +307,7 @@ describe('Convos Routes', () => { expect(response.status).toBe(400); expect(response.body).toEqual({ error: 'no parameters provided' }); expect(deleteConvos).not.toHaveBeenCalled(); - expect(deleteConvoSharedLink).not.toHaveBeenCalled(); + expect(deleteConvoSharedLinksWithCleanup).not.toHaveBeenCalled(); }); it('should return 400 when request body is empty (DoS prevention)', async () => { @@ -336,12 +344,14 @@ describe('Convos Routes', () => { expect(deleteConvos).not.toHaveBeenCalled(); }); - it('should return 500 if deleteConvoSharedLink fails', async () => { + it('should return 500 if deleteConvoSharedLinksWithCleanup fails', async () => { const mockConversationId = 'conv-error'; deleteConvos.mockResolvedValue({ deletedCount: 1 }); deleteToolCalls.mockResolvedValue({ deletedCount: 2 }); - deleteConvoSharedLink.mockRejectedValue(new Error('Failed to delete shared links')); + deleteConvoSharedLinksWithCleanup.mockRejectedValue( + new Error('Failed to delete shared links'), + ); const response = await request(app) .delete('/api/convos') @@ -369,8 +379,8 @@ describe('Convos Routes', () => { return Promise.resolve({ deletedCount: 2 }); }); - deleteConvoSharedLink.mockImplementation(() => { - executionOrder.push('deleteConvoSharedLink'); + deleteConvoSharedLinksWithCleanup.mockImplementation(() => { + executionOrder.push('deleteConvoSharedLinksWithCleanup'); return Promise.resolve({ deletedCount: 1 }); }); @@ -382,7 +392,11 @@ describe('Convos Routes', () => { }, }); - expect(executionOrder).toEqual(['deleteConvos', 'deleteToolCalls', 'deleteConvoSharedLink']); + expect(executionOrder).toEqual([ + 'deleteConvos', + 'deleteToolCalls', + 'deleteConvoSharedLinksWithCleanup', + ]); }); it('should prevent orphaned shared links when deleting single conversation', async () => { @@ -390,7 +404,7 @@ describe('Convos Routes', () => { deleteConvos.mockResolvedValue({ deletedCount: 1 }); deleteToolCalls.mockResolvedValue({ deletedCount: 4 }); - deleteConvoSharedLink.mockResolvedValue({ + deleteConvoSharedLinksWithCleanup.mockResolvedValue({ message: 'Shared links deleted successfully', deletedCount: 2, }); @@ -406,10 +420,13 @@ describe('Convos Routes', () => { expect(response.status).toBe(201); /** Verify shared links were deleted for the specific conversation */ - expect(deleteConvoSharedLink).toHaveBeenCalledWith('test-user-123', mockConversationId); + expect(deleteConvoSharedLinksWithCleanup).toHaveBeenCalledWith( + 'test-user-123', + mockConversationId, + ); /** Verify it was called after the conversation was deleted */ - expect(deleteConvoSharedLink).toHaveBeenCalledAfter(deleteConvos); + expect(deleteConvoSharedLinksWithCleanup).toHaveBeenCalledAfter(deleteConvos); }); }); diff --git a/api/server/routes/__tests__/share.spec.js b/api/server/routes/__tests__/share.spec.js index 541ae451c64..02941c84537 100644 --- a/api/server/routes/__tests__/share.spec.js +++ b/api/server/routes/__tests__/share.spec.js @@ -3,9 +3,18 @@ const request = require('supertest'); const mongoose = require('mongoose'); const mockGetSharedLinkExpiration = jest.fn(); +const mockGrantCreationPermissions = jest.fn(); +const mockUpdateSharedLinkPermissionsExpiration = jest.fn(); +const mockSharedLinksAccess = jest.fn((_req, _res, next) => next()); jest.mock('@librechat/api', () => ({ isEnabled: jest.fn(() => true), + generateCheckAccess: jest.fn(() => mockSharedLinksAccess), + grantCreationPermissions: (...args) => mockGrantCreationPermissions(...args), + updateSharedLinkPermissionsExpiration: (...args) => + mockUpdateSharedLinkPermissionsExpiration(...args), + ensureLinkPermissions: jest.fn(), + deleteSharedLinkWithCleanup: jest.fn(), getSharedLinkExpiration: (...args) => mockGetSharedLinkExpiration(...args), isActiveExpirationDate: jest.fn((expiredAt) => expiredAt > new Date()), })); @@ -16,6 +25,13 @@ jest.mock('@librechat/data-schemas', () => ({ })); jest.mock('librechat-data-provider', () => ({ + PermissionTypes: { + SHARED_LINKS: 'SHARED_LINKS', + }, + Permissions: { + CREATE: 'CREATE', + SHARE_PUBLIC: 'SHARE_PUBLIC', + }, RetentionMode: { ALL: 'all', TEMPORARY: 'temporary', @@ -40,13 +56,17 @@ jest.mock('~/models', () => ({ deleteSharedLink: jest.fn(), getSharedLinks: jest.fn(), getSharedLink: jest.fn(), + getRoleByName: jest.fn(), })); +jest.mock('~/server/middleware/canAccessSharedLink', () => (_req, _res, next) => next()); +jest.mock('~/server/middleware/optionalJwtAuth', () => (req, _res, next) => next()); jest.mock('~/server/middleware/requireJwtAuth', () => (req, res, next) => next()); const { RetentionMode } = require('librechat-data-provider'); const { createTempChatExpirationDate, logger } = require('@librechat/data-schemas'); -const { createSharedLink, updateSharedLink } = require('~/models'); +const { deleteSharedLinkWithCleanup } = require('@librechat/api'); +const { createSharedLink, updateSharedLink, getRoleByName } = require('~/models'); const shareRouter = require('../share'); const activeExpiration = new Date('2030-01-01T00:00:00.000Z'); @@ -71,11 +91,19 @@ const buildApp = ({ retentionMode = RetentionMode.TEMPORARY } = {}) => { describe('share routes retention', () => { beforeEach(() => { jest.clearAllMocks(); + getRoleByName.mockResolvedValue({ + permissions: { + SHARED_LINKS: { + SHARE_PUBLIC: true, + }, + }, + }); + mockGrantCreationPermissions.mockResolvedValue(undefined); }); it('expires new shares for retained non-temporary conversations', async () => { mockGetSharedLinkExpiration.mockResolvedValue(activeExpiration); - createSharedLink.mockResolvedValue({ shareId: 'share-123' }); + createSharedLink.mockResolvedValue({ _id: 'link-123', shareId: 'share-123' }); const response = await request(buildApp()) .post('/api/share/convo-123') @@ -106,11 +134,18 @@ describe('share routes retention', () => { 'msg-123', new Date('2030-01-01T00:00:00.000Z'), ); + expect(mockGrantCreationPermissions).toHaveBeenCalledWith( + 'link-123', + 'user-123', + true, + new Date('2030-01-01T00:00:00.000Z'), + ); + expect(mockSharedLinksAccess).toHaveBeenCalled(); }); it('rejects new shares when the retained conversation expired', async () => { mockGetSharedLinkExpiration.mockResolvedValue(expiredExpiration); - createSharedLink.mockResolvedValue({ shareId: 'share-123' }); + createSharedLink.mockResolvedValue({ _id: 'link-123', shareId: 'share-123' }); const response = await request(buildApp()) .post('/api/share/convo-123') @@ -122,7 +157,7 @@ describe('share routes retention', () => { it('rejects new shares for expired conversations in all retention mode', async () => { mockGetSharedLinkExpiration.mockResolvedValue(expiredExpiration); - createSharedLink.mockResolvedValue({ shareId: 'share-123' }); + createSharedLink.mockResolvedValue({ _id: 'link-123', shareId: 'share-123' }); const response = await request(buildApp({ retentionMode: RetentionMode.ALL })) .post('/api/share/convo-123') @@ -135,7 +170,7 @@ describe('share routes retention', () => { it('expires updated shares for retained non-temporary conversations', async () => { mongoose.models.SharedLink.findOne.mockReturnValue(lean({ conversationId: 'convo-123' })); mockGetSharedLinkExpiration.mockResolvedValue(activeExpiration); - updateSharedLink.mockResolvedValue({ shareId: 'share-456' }); + updateSharedLink.mockResolvedValue({ _id: 'link-456', shareId: 'share-456' }); const response = await request(buildApp()).patch('/api/share/share-123'); @@ -162,6 +197,10 @@ describe('share routes retention', () => { undefined, new Date('2030-01-01T00:00:00.000Z'), ); + expect(mockUpdateSharedLinkPermissionsExpiration).toHaveBeenCalledWith( + 'link-456', + new Date('2030-01-01T00:00:00.000Z'), + ); }); it('rejects updated shares when the retained conversation expired', async () => { @@ -195,12 +234,14 @@ describe('share routes retention', () => { it('clears updated share expiration when the conversation is no longer retained', async () => { mongoose.models.SharedLink.findOne.mockReturnValue(lean({ conversationId: 'convo-123' })); mockGetSharedLinkExpiration.mockResolvedValue(null); - updateSharedLink.mockResolvedValue({ shareId: 'share-456' }); + updateSharedLink.mockResolvedValue({ _id: 'link-456', shareId: 'share-456' }); const response = await request(buildApp()).patch('/api/share/share-123'); expect(response.status).toBe(200); expect(updateSharedLink).toHaveBeenCalledWith('user-123', 'share-123', undefined, null); + expect(mockUpdateSharedLinkPermissionsExpiration).toHaveBeenCalledWith('link-456', null); + expect(mockSharedLinksAccess).not.toHaveBeenCalled(); }); it('preserves updated share expiration when the conversation cannot be found', async () => { @@ -212,6 +253,7 @@ describe('share routes retention', () => { expect(response.status).toBe(200); expect(updateSharedLink).toHaveBeenCalledWith('user-123', 'share-123', undefined, undefined); + expect(mockUpdateSharedLinkPermissionsExpiration).not.toHaveBeenCalled(); }); it('clears updated share expiration when creating a new expiration throws', async () => { @@ -221,7 +263,7 @@ describe('share routes retention', () => { dependencies.logger.error('[getSharedLinkExpiration] Error creating expiration date:', error); return null; }); - updateSharedLink.mockResolvedValue({ shareId: 'share-456' }); + updateSharedLink.mockResolvedValue({ _id: 'link-456', shareId: 'share-456' }); const response = await request(buildApp()).patch('/api/share/share-123'); @@ -231,6 +273,7 @@ describe('share routes retention', () => { error, ); expect(updateSharedLink).toHaveBeenCalledWith('user-123', 'share-123', undefined, null); + expect(mockUpdateSharedLinkPermissionsExpiration).toHaveBeenCalledWith('link-456', null); }); it('updates share target message while applying retention expiration', async () => { @@ -259,4 +302,14 @@ describe('share routes retention', () => { expect(response.status).toBe(400); expect(updateSharedLink).not.toHaveBeenCalled(); }); + + it('allows deleting existing shares without CREATE permission gate', async () => { + deleteSharedLinkWithCleanup.mockResolvedValue({ shareId: 'share-123' }); + + const response = await request(buildApp()).delete('/api/share/share-123'); + + expect(response.status).toBe(200); + expect(mockSharedLinksAccess).not.toHaveBeenCalled(); + expect(deleteSharedLinkWithCleanup).toHaveBeenCalledWith('user-123', 'share-123'); + }); }); diff --git a/api/server/routes/accessPermissions.js b/api/server/routes/accessPermissions.js index e53d0ef1a77..6ef731daba7 100644 --- a/api/server/routes/accessPermissions.js +++ b/api/server/routes/accessPermissions.js @@ -1,5 +1,11 @@ +const mongoose = require('mongoose'); const express = require('express'); -const { ResourceType, PermissionBits } = require('librechat-data-provider'); +const { + AccessRoleIds, + PrincipalType, + ResourceType, + PermissionBits, +} = require('librechat-data-provider'); const { getUserEffectivePermissions, getAllEffectivePermissions, @@ -82,6 +88,12 @@ const checkResourcePermissionAccess = (requiredPermission) => (req, res, next) = resourceIdParam: 'resourceId', idResolver: getSkillById, }); + } else if (resourceType === ResourceType.SHARED_LINK) { + middleware = canAccessResource({ + resourceType: ResourceType.SHARED_LINK, + requiredPermission, + resourceIdParam: 'resourceId', + }); } else { return res.status(400).json({ error: 'Bad Request', @@ -93,6 +105,57 @@ const checkResourcePermissionAccess = (requiredPermission) => (req, res, next) = middleware(req, res, next); }; +const rejectSharedLinkOwnerPermissionChanges = async (req, res, next) => { + if (req.params.resourceType !== ResourceType.SHARED_LINK) { + return next(); + } + + const updated = Array.isArray(req.body?.updated) ? req.body.updated : []; + const removed = Array.isArray(req.body?.removed) ? req.body.removed : []; + const grantsOwner = updated.some( + (principal) => principal?.accessRoleId === AccessRoleIds.SHARED_LINK_OWNER, + ); + const grantsPublicOwner = req.body?.publicAccessRoleId === AccessRoleIds.SHARED_LINK_OWNER; + + if (grantsOwner || grantsPublicOwner) { + return res.status(400).json({ + error: 'Bad Request', + message: 'Shared link owner permissions cannot be changed', + }); + } + + const userMutations = [...updated, ...removed].filter( + (principal) => principal?.type === PrincipalType.USER && principal?.id, + ); + + if (userMutations.length === 0) { + return next(); + } + + try { + const SharedLink = mongoose.models.SharedLink; + const link = await SharedLink.findById(req.params.resourceId, 'user').lean(); + const ownerId = link?.user?.toString(); + const touchesOwner = ownerId + ? userMutations.some((principal) => principal.id?.toString() === ownerId) + : false; + + if (touchesOwner) { + return res.status(400).json({ + error: 'Bad Request', + message: 'Shared link owner permissions cannot be changed', + }); + } + } catch (_error) { + return res.status(500).json({ + error: 'Internal Server Error', + message: 'Failed to validate shared link owner permissions', + }); + } + + return next(); +}; + /** * GET /api/permissions/{resourceType}/{resourceId} * Get all permissions for a specific resource @@ -115,6 +178,7 @@ router.put( checkResourcePermissionAccess(PermissionBits.SHARE), checkShareAccess, checkSharePublicAccess, + rejectSharedLinkOwnerPermissionChanges, updateResourcePermissions, ); diff --git a/api/server/routes/accessPermissions.sharePolicy.test.js b/api/server/routes/accessPermissions.sharePolicy.test.js index 0fc7a90deac..ed17a044529 100644 --- a/api/server/routes/accessPermissions.sharePolicy.test.js +++ b/api/server/routes/accessPermissions.sharePolicy.test.js @@ -30,6 +30,7 @@ jest.mock('~/server/controllers/PermissionsController', () => ({ const express = require('express'); const request = require('supertest'); +const mongoose = require('mongoose'); const { SystemRoles, ResourceType, @@ -48,6 +49,8 @@ const { getRoleByName } = require('~/models'); describe('Access permissions share policy', () => { let app; + const mockSharedLinkFindById = jest.fn(); + const originalSharedLinkModel = mongoose.models.SharedLink; const resourceId = '507f1f77bcf86cd799439011'; const sharePolicyCases = [ @@ -116,8 +119,30 @@ describe('Access permissions share policy', () => { accessRoleId, }); + const allowSharedLinkSharing = () => { + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.SHARED_LINKS]: { + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: true, + }, + }, + }); + }; + + const mockSharedLinkOwner = (ownerId = 'owner-user') => { + mockSharedLinkFindById.mockReturnValue({ + lean: jest.fn().mockResolvedValue({ user: ownerId }), + }); + }; + beforeEach(() => { jest.clearAllMocks(); + if (mongoose.models.SharedLink) { + mongoose.models.SharedLink.findById = mockSharedLinkFindById; + } else { + mongoose.models.SharedLink = { findById: mockSharedLinkFindById }; + } hasCapability.mockResolvedValue(false); app = express(); @@ -129,6 +154,14 @@ describe('Access permissions share policy', () => { app.use('/api/permissions', accessPermissionsRouter); }); + afterAll(() => { + if (originalSharedLinkModel) { + mongoose.models.SharedLink = originalSharedLinkModel; + } else { + delete mongoose.models.SharedLink; + } + }); + it.each(sharePolicyCases)( 'blocks non-public $label sharing when ACL SHARE passes but role SHARE is disabled', async ({ resourceType, permissionType, accessRoleId, middlewareOptions }) => { @@ -208,4 +241,80 @@ describe('Access permissions share policy', () => { }); expect(updateResourcePermissions).not.toHaveBeenCalled(); }); + + it('blocks granting shared-link owner through generic permission updates', async () => { + allowSharedLinkSharing(); + mockSharedLinkOwner(); + + const response = await request(app) + .put(`/api/permissions/${ResourceType.SHARED_LINK}/${resourceId}`) + .send({ + updated: [ + { + type: PrincipalType.USER, + id: 'target-user', + accessRoleId: AccessRoleIds.SHARED_LINK_OWNER, + }, + ], + public: false, + }); + + expect(response.status).toBe(400); + expect(response.body.message).toBe('Shared link owner permissions cannot be changed'); + expect(updateResourcePermissions).not.toHaveBeenCalled(); + }); + + it('blocks granting shared-link owner to the public principal', async () => { + allowSharedLinkSharing(); + + const response = await request(app) + .put(`/api/permissions/${ResourceType.SHARED_LINK}/${resourceId}`) + .send({ + public: true, + publicAccessRoleId: AccessRoleIds.SHARED_LINK_OWNER, + }); + + expect(response.status).toBe(400); + expect(response.body.message).toBe('Shared link owner permissions cannot be changed'); + expect(updateResourcePermissions).not.toHaveBeenCalled(); + expect(mockSharedLinkFindById).not.toHaveBeenCalled(); + }); + + it('blocks removing the canonical shared-link owner', async () => { + allowSharedLinkSharing(); + mockSharedLinkOwner('owner-user'); + + const response = await request(app) + .put(`/api/permissions/${ResourceType.SHARED_LINK}/${resourceId}`) + .send({ + updated: [], + removed: [{ type: PrincipalType.USER, id: 'owner-user' }], + public: false, + }); + + expect(response.status).toBe(400); + expect(response.body.message).toBe('Shared link owner permissions cannot be changed'); + expect(updateResourcePermissions).not.toHaveBeenCalled(); + }); + + it('allows viewer grants for non-owner shared-link users', async () => { + allowSharedLinkSharing(); + mockSharedLinkOwner('owner-user'); + + const response = await request(app) + .put(`/api/permissions/${ResourceType.SHARED_LINK}/${resourceId}`) + .send({ + updated: [ + { + type: PrincipalType.USER, + id: 'target-user', + accessRoleId: AccessRoleIds.SHARED_LINK_VIEWER, + }, + ], + public: false, + }); + + expect(response.status).toBe(200); + expect(updateResourcePermissions).toHaveBeenCalledTimes(1); + }); }); diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js index dc59482afa7..8dfe8621efa 100644 --- a/api/server/routes/convos.js +++ b/api/server/routes/convos.js @@ -5,6 +5,8 @@ const { isEnabled, resolveImportMaxFileSize, restoreTenantContextFromReq, + deleteAllSharedLinksWithCleanup, + deleteConvoSharedLinksWithCleanup, } = require('@librechat/api'); const { logger } = require('@librechat/data-schemas'); const { CacheKeys, EModelEndpoint } = require('librechat-data-provider'); @@ -133,7 +135,7 @@ router.delete('/', async (req, res) => { const dbResponse = await db.deleteConvos(req.user.id, filter); if (filter.conversationId) { await db.deleteToolCalls(req.user.id, filter.conversationId); - await db.deleteConvoSharedLink(req.user.id, filter.conversationId); + await deleteConvoSharedLinksWithCleanup(req.user.id, filter.conversationId); } res.status(201).json(dbResponse); } catch (error) { @@ -146,7 +148,7 @@ router.delete('/all', async (req, res) => { try { const dbResponse = await db.deleteConvos(req.user.id, {}); await db.deleteToolCalls(req.user.id); - await db.deleteAllSharedLinks(req.user.id); + await deleteAllSharedLinksWithCleanup(req.user.id); res.status(201).json(dbResponse); } catch (error) { logger.error('Error clearing conversations', error); diff --git a/api/server/routes/share.js b/api/server/routes/share.js index ce4dee1a1f4..19c760007af 100644 --- a/api/server/routes/share.js +++ b/api/server/routes/share.js @@ -1,18 +1,36 @@ const mongoose = require('mongoose'); const express = require('express'); -const { isEnabled, isActiveExpirationDate, getSharedLinkExpiration } = require('@librechat/api'); +const { + isEnabled, + generateCheckAccess, + grantCreationPermissions, + ensureLinkPermissions, + deleteSharedLinkWithCleanup, + updateSharedLinkPermissionsExpiration, + isActiveExpirationDate, + getSharedLinkExpiration, +} = require('@librechat/api'); const { logger, createTempChatExpirationDate } = require('@librechat/data-schemas'); +const { PermissionTypes, Permissions } = require('librechat-data-provider'); const { getSharedMessages, createSharedLink, updateSharedLink, - deleteSharedLink, getSharedLinks, getSharedLink, + getRoleByName, } = require('~/models'); +const canAccessSharedLink = require('~/server/middleware/canAccessSharedLink'); +const optionalJwtAuth = require('~/server/middleware/optionalJwtAuth'); const requireJwtAuth = require('~/server/middleware/requireJwtAuth'); const router = express.Router(); +const checkSharedLinksAccess = generateCheckAccess({ + permissionType: PermissionTypes.SHARED_LINKS, + permissions: [Permissions.CREATE], + getRoleByName, +}); + const resolveSharedLinkExpiration = (req, conversationId) => getSharedLinkExpiration( { req, conversationId }, @@ -36,25 +54,19 @@ const allowSharedLinks = process.env.ALLOW_SHARED_LINKS === undefined || isEnabled(process.env.ALLOW_SHARED_LINKS); if (allowSharedLinks) { - const allowSharedLinksPublic = isEnabled(process.env.ALLOW_SHARED_LINKS_PUBLIC); - router.get( - '/:shareId', - allowSharedLinksPublic ? (req, res, next) => next() : requireJwtAuth, - async (req, res) => { - try { - const share = await getSharedMessages(req.params.shareId); - - if (share) { - res.status(200).json(share); - } else { - res.status(404).end(); - } - } catch (error) { - logger.error('Error getting shared messages:', error); - res.status(500).json({ message: 'Error getting shared messages' }); + router.get('/:shareId', optionalJwtAuth, canAccessSharedLink, async (req, res) => { + try { + const share = await getSharedMessages(req.params.shareId, req.shareResourceId); + if (share) { + res.status(200).json(share); + } else { + res.status(404).end(); } - }, - ); + } catch (error) { + logger.error('Error getting shared messages:', error); + res.status(500).json({ message: 'Error getting shared messages' }); + } + }); } /** @@ -65,7 +77,6 @@ router.get('/', requireJwtAuth, async (req, res) => { const params = { pageParam: req.query.cursor, pageSize: Math.max(1, parseInt(req.query.pageSize) || 10), - isPublic: isEnabled(req.query.isPublic), sortBy: ['createdAt', 'title'].includes(req.query.sortBy) ? req.query.sortBy : 'createdAt', sortDirection: ['asc', 'desc'].includes(req.query.sortDirection) ? req.query.sortDirection @@ -77,7 +88,6 @@ router.get('/', requireJwtAuth, async (req, res) => { req.user.id, params.pageParam, params.pageSize, - params.isPublic, params.sortBy, params.sortDirection, params.search, @@ -101,7 +111,12 @@ router.get('/link/:conversationId', requireJwtAuth, async (req, res) => { try { const share = await getSharedLink(req.user.id, req.params.conversationId); + if (share._id && share.success) { + await ensureLinkPermissions(share._id, req.user.id); + } + return res.status(200).json({ + _id: share._id, success: share.success, shareId: share.shareId, targetMessageId: share.targetMessageId, @@ -113,7 +128,7 @@ router.get('/link/:conversationId', requireJwtAuth, async (req, res) => { } }); -router.post('/:conversationId', requireJwtAuth, async (req, res) => { +router.post('/:conversationId', requireJwtAuth, checkSharedLinksAccess, async (req, res) => { try { const { targetMessageId } = req.body; const expiredAt = await resolveSharedLinkExpiration(req, req.params.conversationId); @@ -121,6 +136,10 @@ router.post('/:conversationId', requireJwtAuth, async (req, res) => { return res.status(404).end(); } + const role = await getRoleByName(req.user.role); + const sharedLinksPerms = role?.permissions?.[PermissionTypes.SHARED_LINKS] || {}; + const grantPublic = sharedLinksPerms[Permissions.SHARE_PUBLIC] === true; + const created = await createSharedLink( req.user.id, req.params.conversationId, @@ -128,6 +147,7 @@ router.post('/:conversationId', requireJwtAuth, async (req, res) => { expiredAt, ); if (created) { + await grantCreationPermissions(created._id, req.user.id, grantPublic, expiredAt); res.status(200).json(created); } else { res.status(404).end(); @@ -165,6 +185,9 @@ router.patch('/:shareId', requireJwtAuth, async (req, res) => { expiredAt, ); if (updatedShare) { + if (updatedShare._id && expiredAt !== undefined) { + await updateSharedLinkPermissionsExpiration(updatedShare._id, expiredAt); + } res.status(200).json(updatedShare); } else { res.status(404).end(); @@ -177,7 +200,7 @@ router.patch('/:shareId', requireJwtAuth, async (req, res) => { router.delete('/:shareId', requireJwtAuth, async (req, res) => { try { - const result = await deleteSharedLink(req.user.id, req.params.shareId); + const result = await deleteSharedLinkWithCleanup(req.user.id, req.params.shareId); if (!result) { return res.status(404).json({ message: 'Share not found' }); diff --git a/client/src/components/Chat/ExportAndShareMenu.tsx b/client/src/components/Chat/ExportAndShareMenu.tsx index 739f2c497b6..5dcbe9ea139 100644 --- a/client/src/components/Chat/ExportAndShareMenu.tsx +++ b/client/src/components/Chat/ExportAndShareMenu.tsx @@ -2,11 +2,12 @@ import { useState, useId, useRef } from 'react'; import { useRecoilValue } from 'recoil'; import * as Ariakit from '@ariakit/react'; import { Upload, Share2 } from 'lucide-react'; +import { PermissionTypes, Permissions } from 'librechat-data-provider'; import { DropdownPopup, TooltipAnchor, useMediaQuery } from '@librechat/client'; import type * as t from '~/common'; import ExportModal from '~/components/Nav/ExportConversation/ExportModal'; import { ShareButton } from '~/components/Conversations/ConvoOptions'; -import { useLocalize } from '~/hooks'; +import { useHasAccess, useLocalize } from '~/hooks'; import store from '~/store'; export default function ExportAndShareMenu({ @@ -22,6 +23,10 @@ export default function ExportAndShareMenu({ const menuId = useId(); const shareButtonRef = useRef(null); const exportButtonRef = useRef(null); + const canCreateSharedLinks = useHasAccess({ + permissionType: PermissionTypes.SHARED_LINKS, + permission: Permissions.CREATE, + }); const isSmallScreen = useMediaQuery('(max-width: 768px)'); const conversation = useRecoilValue(store.conversationByIndex(0)); @@ -48,11 +53,11 @@ export default function ExportAndShareMenu({ label: localize('com_ui_share'), onClick: shareHandler, icon: , - show: isSharedButtonEnabled, + show: isSharedButtonEnabled && canCreateSharedLinks, /** NOTE: THE FOLLOWING PROPS ARE REQUIRED FOR MENU ITEMS THAT OPEN DIALOGS */ hideOnClick: false, ref: shareButtonRef, - render: (props) => )} /> + + {canManageAccess && ( + + ( + + )} + /> + + )} )} (-1); return ( -
+
role.accessRoleId !== ownerRoleId) + : accessRoles || []; + const selectedRole = accessRoles?.find((role) => role.accessRoleId === selectedRoleId); const selectedRoleInfo = selectedRole ? getLocalizedRoleInfo(selectedRole.accessRoleId) : null; @@ -44,7 +50,7 @@ export default function AccessRolesPicker({ return ; } - const dropdownItems: t.MenuItemProps[] = accessRoles.map((role: AccessRole) => { + const dropdownItems: t.MenuItemProps[] = filteredRoles.map((role: AccessRole) => { const localizedInfo = getLocalizedRoleInfo(role.accessRoleId); return { id: role.accessRoleId, diff --git a/client/src/components/Sharing/GenericGrantAccessDialog.tsx b/client/src/components/Sharing/GenericGrantAccessDialog.tsx index ce7fdbdc143..24e9d1ae9ba 100644 --- a/client/src/components/Sharing/GenericGrantAccessDialog.tsx +++ b/client/src/components/Sharing/GenericGrantAccessDialog.tsx @@ -180,14 +180,18 @@ export default function GenericGrantAccessDialog({ return !allSharesMap.has(key); }); + const publicChanged = isPublic !== currentIsPublic; + const publicRoleChanged = isPublic && publicRole !== currentPublicRole; + const sendPublicUpdate = publicChanged || publicRoleChanged; + await updatePermissionsMutation.mutateAsync({ resourceType, resourceId: resourceDbId, data: { updated, removed, - public: isPublic, - publicAccessRoleId: isPublic ? publicRole : undefined, + ...(sendPublicUpdate ? { public: isPublic } : {}), + ...(sendPublicUpdate && isPublic ? { publicAccessRoleId: publicRole } : {}), }, }); diff --git a/client/src/components/Sharing/PeoplePicker/SelectedPrincipalsList.tsx b/client/src/components/Sharing/PeoplePicker/SelectedPrincipalsList.tsx index b74b2126696..05ee3704a5c 100644 --- a/client/src/components/Sharing/PeoplePicker/SelectedPrincipalsList.tsx +++ b/client/src/components/Sharing/PeoplePicker/SelectedPrincipalsList.tsx @@ -5,6 +5,7 @@ import { ResourceType } from 'librechat-data-provider'; import type { TPrincipal, AccessRoleIds } from 'librechat-data-provider'; import AccessRolesPicker from '~/components/Sharing/AccessRolesPicker'; import PrincipalAvatar from '~/components/Sharing/PrincipalAvatar'; +import { RESOURCE_CONFIGS } from '~/utils/resources'; import { useLocalize } from '~/hooks'; interface SelectedPrincipalsListProps { @@ -50,6 +51,10 @@ export default function SelectedPrincipalsList({
{principles.map((share) => { const { displayName, subtitle } = getPrincipalDisplayInfo(share); + const ownerRoleId = RESOURCE_CONFIGS[resourceType]?.defaultOwnerRoleId; + const isOwner = share.accessRoleId === ownerRoleId; + const isSharedLink = resourceType === ResourceType.SHARED_LINK; + const lockOwner = isSharedLink && isOwner; return (
- {!!share.accessRoleId && !!onRoleChange && ( - { - onRoleChange?.(share.idOnTheSource!, newRole); - }} - className="min-w-0" - /> + {lockOwner ? ( + + {localize('com_ui_role_owner')} + + ) : ( + !!share.accessRoleId && + !!onRoleChange && ( + { + onRoleChange?.(share.idOnTheSource!, newRole); + }} + className="min-w-0" + /> + ) + )} + {!lockOwner && ( + )} -
); diff --git a/client/src/components/Sharing/PeoplePickerAdminSettings.tsx b/client/src/components/Sharing/PeoplePickerAdminSettings.tsx index 5f334f3764d..ab3f9f5517c 100644 --- a/client/src/components/Sharing/PeoplePickerAdminSettings.tsx +++ b/client/src/components/Sharing/PeoplePickerAdminSettings.tsx @@ -140,7 +140,7 @@ const PeoplePickerAdminSettings = () => {
diff --git a/client/src/components/Chat/Input/ChatForm.tsx b/client/src/components/Chat/Input/ChatForm.tsx index 884a9251c42..f8c058cae0c 100644 --- a/client/src/components/Chat/Input/ChatForm.tsx +++ b/client/src/components/Chat/Input/ChatForm.tsx @@ -42,6 +42,7 @@ import store from '~/store'; interface ChatFormProps { index: number; + placeholder?: string; /** From ChatContext — individual values so memo can compare them */ files: Map; setFiles: FileSetter; @@ -55,6 +56,7 @@ interface ChatFormProps { const ChatForm = memo(function ChatForm({ index, + placeholder, files, setFiles, conversation, @@ -177,6 +179,7 @@ const ChatForm = memo(function ChatForm({ submitButtonRef, setIsScrollable, disabled: disableInputs, + placeholder, }); useQueryParams({ textAreaRef }); @@ -413,7 +416,7 @@ ChatForm.displayName = 'ChatForm'; * to the memo'd ChatForm. This prevents ChatForm from re-rendering on every * streaming chunk — it only re-renders when the specific values it uses change. */ -function ChatFormWrapper({ index = 0 }: { index?: number }) { +function ChatFormWrapper({ index = 0, placeholder }: { index?: number; placeholder?: string }) { const { files, setFiles, @@ -465,6 +468,7 @@ function ChatFormWrapper({ index = 0 }: { index?: number }) { return ( data?.pages.flatMap((page) => page.projects) ?? [], [data?.pages]); + const items = useMemo( + () => + projects.map((item) => ({ + label: item.name, + value: item._id, + icon:
); }); @@ -114,7 +156,6 @@ DateLabel.displayName = 'DateLabel'; type FlattenedItem = | { type: 'favorites' } - | { type: 'chats-header' } | { type: 'header'; groupName: string } | { type: 'convo'; convo: TConversation } | { type: 'loading' }; @@ -145,6 +186,7 @@ const MemoizedConvo = memo( prevProps.conversation.conversationId === nextProps.conversation.conversationId && prevProps.conversation.title === nextProps.conversation.title && prevProps.conversation.endpoint === nextProps.conversation.endpoint && + prevProps.conversation.chatProjectId === nextProps.conversation.chatProjectId && prevProps.isGenerating === nextProps.isGenerating ); }, @@ -160,6 +202,7 @@ const Conversations: FC = ({ isSearchLoading, isChatsExpanded, setIsChatsExpanded, + showFavorites = true, }) => { const localize = useLocalize(); const search = useRecoilValue(store.search); @@ -179,7 +222,9 @@ const Conversations: FC = ({ // Determine if FavoritesList will render content const shouldShowFavorites = - !search.query && (isFavoritesLoading || favorites.length > 0 || showAgentMarketplace); + showFavorites && + !search.query && + (isFavoritesLoading || favorites.length > 0 || showAgentMarketplace); favoritesContentKeyRef.current = `${favorites.length}-${showAgentMarketplace ? 1 : 0}-${isFavoritesLoading ? 1 : 0}`; @@ -199,7 +244,6 @@ const Conversations: FC = ({ if (shouldShowFavorites) { items.push({ type: 'favorites' }); } - items.push({ type: 'chats-header' }); if (isChatsExpanded) { groupedConversations.forEach(([groupName, convos]) => { @@ -232,9 +276,6 @@ const Conversations: FC = ({ if (item.type === 'favorites') { return `favorites-${favoritesContentKeyRef.current}`; } - if (item.type === 'chats-header') { - return 'chats-header'; - } if (item.type === 'header') { return `header-${item.groupName}`; } @@ -297,22 +338,9 @@ const Conversations: FC = ({ ); } - if (item.type === 'chats-header') { - return ( - - setIsChatsExpanded(!isChatsExpanded)} - /> - - ); - } - if (item.type === 'header') { - // First date header index depends on whether favorites row is included - // With favorites: [favorites, chats-header, first-header] → index 2 - // Without favorites: [chats-header, first-header] → index 1 - const firstHeaderIndex = shouldShowFavorites ? 2 : 1; + // First date header index depends on whether the favorites row is included + const firstHeaderIndex = shouldShowFavorites ? 1 : 0; return ( @@ -336,17 +364,7 @@ const Conversations: FC = ({ return null; }, - [ - cache, - flattenedItems, - moveToTop, - toggleNav, - isSmallScreen, - isChatsExpanded, - setIsChatsExpanded, - shouldShowFavorites, - activeJobIds, - ], + [cache, flattenedItems, moveToTop, toggleNav, isSmallScreen, shouldShowFavorites, activeJobIds], ); const getRowHeight = useCallback( @@ -370,6 +388,12 @@ const Conversations: FC = ({ return (
+
+ setIsChatsExpanded(!isChatsExpanded)} + /> +
{isSearchLoading ? (
diff --git a/client/src/components/Conversations/Convo.tsx b/client/src/components/Conversations/Convo.tsx index 46e3be09ebc..923397f623f 100644 --- a/client/src/components/Conversations/Convo.tsx +++ b/client/src/components/Conversations/Convo.tsx @@ -171,6 +171,7 @@ export default function Conversation({ renameHandler: handleRename, isActiveConvo, conversationId, + chatProjectId: conversation.chatProjectId, isPopoverActive, setIsPopoverActive: handlePopoverOpenChange, isShiftHeld: isActiveConvo ? isShiftHeld : false, diff --git a/client/src/components/Conversations/ConvoOptions/ConvoOptions.tsx b/client/src/components/Conversations/ConvoOptions/ConvoOptions.tsx index 4286a2f588d..7916de77704 100644 --- a/client/src/components/Conversations/ConvoOptions/ConvoOptions.tsx +++ b/client/src/components/Conversations/ConvoOptions/ConvoOptions.tsx @@ -3,12 +3,22 @@ import * as Ariakit from '@ariakit/react'; import { useQueryClient } from '@tanstack/react-query'; import { useParams, useNavigate } from 'react-router-dom'; import { DropdownPopup, Spinner, useToastContext } from '@librechat/client'; -import { Ellipsis, Share2, CopyPlus, Archive, Pen, Trash } from 'lucide-react'; +import { + Ellipsis, + Share2, + CopyPlus, + Archive, + FolderInput, + FolderX, + Pen, + Trash, +} from 'lucide-react'; import { QueryKeys, PermissionTypes, Permissions } from 'librechat-data-provider'; import type { MouseEvent } from 'react'; import type { TMessage } from 'librechat-data-provider'; import { useDuplicateConversationMutation, + useAssignConversationToProjectMutation, useDeleteConversationMutation, useGetStartupConfig, useArchiveConvoMutation, @@ -17,11 +27,13 @@ import { useHasAccess, useLocalize, useNavigateToConvo, useNewConvo } from '~/ho import { NotificationSeverity } from '~/common'; import { useChatContext } from '~/Providers'; import DeleteButton from './DeleteButton'; +import ProjectButton from './ProjectButton'; import ShareButton from './ShareButton'; import { cn } from '~/utils'; function ConvoOptions({ conversationId, + chatProjectId, title, retainView, renameHandler, @@ -31,6 +43,7 @@ function ConvoOptions({ isShiftHeld = false, }: { conversationId: string | null; + chatProjectId?: string | null; title: string | null; retainView: () => void; renameHandler: (e: MouseEvent) => void; @@ -53,8 +66,10 @@ function ConvoOptions({ const menuId = useId(); const shareButtonRef = useRef(null); const deleteButtonRef = useRef(null); + const projectButtonRef = useRef(null); const [showShareDialog, setShowShareDialog] = useState(false); const [showDeleteDialog, setShowDeleteDialog] = useState(false); + const [showProjectDialog, setShowProjectDialog] = useState(false); const [announcement, setAnnouncement] = useState(''); const canCreateSharedLinks = useHasAccess({ @@ -63,6 +78,7 @@ function ConvoOptions({ }); const archiveConvoMutation = useArchiveConvoMutation(); + const assignConversationToProject = useAssignConversationToProjectMutation(); const deleteMutation = useDeleteConversationMutation({ onSuccess: () => { @@ -121,6 +137,37 @@ function ConvoOptions({ setShowDeleteDialog(true); }, []); + const projectHandler = useCallback(() => { + setShowProjectDialog(true); + }, []); + + const removeProjectHandler = useCallback(() => { + const convoId = conversationId ?? ''; + if (!convoId) { + return; + } + assignConversationToProject.mutate( + { conversationId: convoId, projectId: null }, + { + onSuccess: () => { + setIsPopoverActive(false); + showToast({ + message: localize('com_ui_project_updated'), + severity: NotificationSeverity.SUCCESS, + showIcon: true, + }); + }, + onError: () => { + showToast({ + message: localize('com_ui_project_update_error'), + severity: NotificationSeverity.ERROR, + showIcon: true, + }); + }, + }, + ); + }, [assignConversationToProject, conversationId, localize, setIsPopoverActive, showToast]); + const handleInstantDelete = useCallback( (e: MouseEvent) => { e.stopPropagation(); @@ -217,6 +264,27 @@ function ConvoOptions({