Merge branch 'main' into aron/data-retention-upstream

api/package.json

@@ -118,7 +118,7 @@
   },
   "devDependencies": {
     "jest": "^30.2.0",
-    "mongodb-memory-server": "^10.1.4",
+    "mongodb-memory-server": "^11.0.1",
     "nodemon": "^3.0.3",
     "supertest": "^7.1.0"
   }

api/server/routes/__tests__/mcp.spec.js

@@ -311,6 +311,87 @@ describe('MCP Routes', () => {
       expect(response.headers.location).toBe(`${basePath}/oauth/error?error=access_denied`);
     });
 
+    describe('OAuth error callback failFlow', () => {
+      it('should fail the flow when OAuth error is received with valid CSRF cookie', async () => {
+        const flowId = 'test-user-id:test-server';
+        const mockFlowManager = {
+          failFlow: jest.fn().mockResolvedValue(true),
+        };
+
+        getLogStores.mockReturnValueOnce({});
+        require('~/config').getFlowStateManager.mockReturnValueOnce(mockFlowManager);
+        MCPOAuthHandler.resolveStateToFlowId.mockResolvedValueOnce(flowId);
+
+        const csrfToken = generateTestCsrfToken(flowId);
+        const response = await request(app)
+          .get('/api/mcp/test-server/oauth/callback')
+          .set('Cookie', [`oauth_csrf=${csrfToken}`])
+          .query({
+            error: 'invalid_client',
+            state: flowId,
+          });
+        const basePath = getBasePath();
+
+        expect(response.status).toBe(302);
+        expect(response.headers.location).toBe(`${basePath}/oauth/error?error=invalid_client`);
+        expect(mockFlowManager.failFlow).toHaveBeenCalledWith(
+          flowId,
+          'mcp_oauth',
+          'invalid_client',
+        );
+      });
+
+      it('should fail the flow when OAuth error is received with valid session cookie', async () => {
+        const flowId = 'test-user-id:test-server';
+        const mockFlowManager = {
+          failFlow: jest.fn().mockResolvedValue(true),
+        };
+
+        getLogStores.mockReturnValueOnce({});
+        require('~/config').getFlowStateManager.mockReturnValueOnce(mockFlowManager);
+        MCPOAuthHandler.resolveStateToFlowId.mockResolvedValueOnce(flowId);
+
+        const sessionToken = generateTestCsrfToken('test-user-id');
+        const response = await request(app)
+          .get('/api/mcp/test-server/oauth/callback')
+          .set('Cookie', [`oauth_session=${sessionToken}`])
+          .query({
+            error: 'invalid_client',
+            state: flowId,
+          });
+        const basePath = getBasePath();
+
+        expect(response.status).toBe(302);
+        expect(response.headers.location).toBe(`${basePath}/oauth/error?error=invalid_client`);
+        expect(mockFlowManager.failFlow).toHaveBeenCalledWith(
+          flowId,
+          'mcp_oauth',
+          'invalid_client',
+        );
+      });
+
+      it('should NOT fail the flow when OAuth error is received without cookies (DoS prevention)', async () => {
+        const flowId = 'test-user-id:test-server';
+        const mockFlowManager = {
+          failFlow: jest.fn(),
+        };
+
+        getLogStores.mockReturnValueOnce({});
+        require('~/config').getFlowStateManager.mockReturnValueOnce(mockFlowManager);
+        MCPOAuthHandler.resolveStateToFlowId.mockResolvedValueOnce(flowId);
+
+        const response = await request(app).get('/api/mcp/test-server/oauth/callback').query({
+          error: 'invalid_client',
+          state: flowId,
+        });
+        const basePath = getBasePath();
+
+        expect(response.status).toBe(302);
+        expect(response.headers.location).toBe(`${basePath}/oauth/error?error=invalid_client`);
+        expect(mockFlowManager.failFlow).not.toHaveBeenCalled();
+      });
+    });
+
     it('should redirect to error page when code is missing', async () => {
       const response = await request(app).get('/api/mcp/test-server/oauth/callback').query({
         state: 'test-user-id:test-server',

api/server/routes/__tests__/roles.spec.js

@@ -0,0 +1,155 @@
+const express = require('express');
+const request = require('supertest');
+const { SystemRoles, roleDefaults } = require('librechat-data-provider');
+
+const mockGetRoleByName = jest.fn();
+const mockHasCapability = jest.fn();
+
+jest.mock('~/server/middleware', () => ({
+  requireJwtAuth: (_req, _res, next) => next(),
+}));
+
+jest.mock('~/server/middleware/roles/capabilities', () => ({
+  hasCapability: (...args) => mockHasCapability(...args),
+  requireCapability: () => (_req, _res, next) => next(),
+}));
+
+jest.mock('~/models', () => ({
+  getRoleByName: (...args) => mockGetRoleByName(...args),
+  updateRoleByName: jest.fn(),
+}));
+
+const rolesRouter = require('../roles');
+
+function createApp(user) {
+  const app = express();
+  app.use(express.json());
+  app.use((req, _res, next) => {
+    req.user = user;
+    next();
+  });
+  app.use('/api/roles', rolesRouter);
+  return app;
+}
+
+const staffRole = {
+  name: 'STAFF',
+  permissions: {
+    PROMPTS: { USE: true, CREATE: false },
+  },
+};
+
+const userRole = roleDefaults[SystemRoles.USER];
+const adminRole = roleDefaults[SystemRoles.ADMIN];
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  mockHasCapability.mockResolvedValue(false);
+  mockGetRoleByName.mockResolvedValue(null);
+});
+
+describe('GET /api/roles/:roleName — isOwnRole authorization', () => {
+  it('allows a custom role user to fetch their own role', async () => {
+    mockGetRoleByName.mockResolvedValue(staffRole);
+    const app = createApp({ id: 'u1', role: 'STAFF' });
+
+    const res = await request(app).get('/api/roles/STAFF');
+
+    expect(res.status).toBe(200);
+    expect(res.body.name).toBe('STAFF');
+    expect(mockGetRoleByName).toHaveBeenCalledWith('STAFF', '-_id -__v');
+  });
+
+  it('returns 403 when a custom role user requests a different custom role', async () => {
+    const app = createApp({ id: 'u1', role: 'STAFF' });
+
+    const res = await request(app).get('/api/roles/MANAGER');
+
+    expect(res.status).toBe(403);
+    expect(mockGetRoleByName).not.toHaveBeenCalled();
+  });
+
+  it('returns 403 when a custom role user requests ADMIN', async () => {
+    const app = createApp({ id: 'u1', role: 'STAFF' });
+
+    const res = await request(app).get('/api/roles/ADMIN');
+
+    expect(res.status).toBe(403);
+    expect(mockGetRoleByName).not.toHaveBeenCalled();
+  });
+
+  it('allows USER to fetch the USER role (roleDefaults key)', async () => {
+    mockGetRoleByName.mockResolvedValue(userRole);
+    const app = createApp({ id: 'u1', role: SystemRoles.USER });
+
+    const res = await request(app).get(`/api/roles/${SystemRoles.USER}`);
+
+    expect(res.status).toBe(200);
+  });
+
+  it('returns 403 when USER requests the ADMIN role', async () => {
+    const app = createApp({ id: 'u1', role: SystemRoles.USER });
+
+    const res = await request(app).get(`/api/roles/${SystemRoles.ADMIN}`);
+
+    expect(res.status).toBe(403);
+  });
+
+  it('allows ADMIN user to fetch their own ADMIN role via isOwnRole', async () => {
+    mockHasCapability.mockResolvedValue(false);
+    mockGetRoleByName.mockResolvedValue(adminRole);
+    const app = createApp({ id: 'u1', role: SystemRoles.ADMIN });
+
+    const res = await request(app).get(`/api/roles/${SystemRoles.ADMIN}`);
+
+    expect(res.status).toBe(200);
+  });
+
+  it('allows any user with READ_ROLES capability to fetch any role', async () => {
+    mockHasCapability.mockResolvedValue(true);
+    mockGetRoleByName.mockResolvedValue(staffRole);
+    const app = createApp({ id: 'u1', role: SystemRoles.USER });
+
+    const res = await request(app).get('/api/roles/STAFF');
+
+    expect(res.status).toBe(200);
+    expect(res.body.name).toBe('STAFF');
+  });
+
+  it('returns 404 when the requested role does not exist', async () => {
+    mockGetRoleByName.mockResolvedValue(null);
+    const app = createApp({ id: 'u1', role: 'GHOST' });
+
+    const res = await request(app).get('/api/roles/GHOST');
+
+    expect(res.status).toBe(404);
+  });
+
+  it('returns 500 when getRoleByName throws', async () => {
+    mockGetRoleByName.mockRejectedValue(new Error('db error'));
+    const app = createApp({ id: 'u1', role: SystemRoles.USER });
+
+    const res = await request(app).get(`/api/roles/${SystemRoles.USER}`);
+
+    expect(res.status).toBe(500);
+  });
+
+  it('returns 403 for prototype property names like constructor (no prototype pollution)', async () => {
+    const app = createApp({ id: 'u1', role: 'STAFF' });
+
+    const res = await request(app).get('/api/roles/constructor');
+
+    expect(res.status).toBe(403);
+    expect(mockGetRoleByName).not.toHaveBeenCalled();
+  });
+
+  it('treats hasCapability failure as no capability (does not 500)', async () => {
+    mockHasCapability.mockRejectedValue(new Error('capability check failed'));
+    const app = createApp({ id: 'u1', role: 'STAFF' });
+    mockGetRoleByName.mockResolvedValue(staffRole);
+
+    const res = await request(app).get('/api/roles/STAFF');
+
+    expect(res.status).toBe(200);
+  });
+});

api/server/routes/admin/auth.js

@@ -3,7 +3,12 @@ const passport = require('passport');
 const crypto = require('node:crypto');
 const { CacheKeys } = require('librechat-data-provider');
 const { logger, SystemCapabilities } = require('@librechat/data-schemas');
-const { getAdminPanelUrl, exchangeAdminCode, createSetBalanceConfig } = require('@librechat/api');
+const {
+  getAdminPanelUrl,
+  exchangeAdminCode,
+  createSetBalanceConfig,
+  storeAndStripChallenge,
+} = require('@librechat/api');
 const { loginController } = require('~/server/controllers/auth/LoginController');
 const { requireCapability } = require('~/server/middleware/roles/capabilities');
 const { createOAuthHandler } = require('~/server/controllers/auth/oauth');
@@ -73,11 +78,6 @@ router.get('/oauth/openid/check', (req, res) => {
   res.status(200).json({ message: 'OpenID check successful' });
 });
 
-/** PKCE challenge cache TTL: 5 minutes (enough for user to authenticate with IdP) */
-const PKCE_CHALLENGE_TTL = 5 * 60 * 1000;
-/** Regex pattern for valid PKCE challenges: 64 hex characters (SHA-256 hex digest) */
-const PKCE_CHALLENGE_PATTERN = /^[a-f0-9]{64}$/;
-
 /**
  * Generates a random hex state string for OAuth flows.
  * @returns {string} A 32-byte random hex string.
@@ -86,27 +86,6 @@ function generateState() {
   return crypto.randomBytes(32).toString('hex');
 }
 
-/**
- * Stores a PKCE challenge in cache keyed by state.
- * @param {string} state - The OAuth state value.
- * @param {string | undefined} codeChallenge - The PKCE code_challenge from query params.
- * @param {string} provider - Provider name for logging.
- * @returns {Promise<boolean>} True if stored successfully or no challenge provided.
- */
-async function storePkceChallenge(state, codeChallenge, provider) {
-  if (typeof codeChallenge !== 'string' || !PKCE_CHALLENGE_PATTERN.test(codeChallenge)) {
-    return true;
-  }
-  try {
-    const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE);
-    await cache.set(`pkce:${state}`, codeChallenge, PKCE_CHALLENGE_TTL);
-    return true;
-  } catch (err) {
-    logger.error(`[admin/oauth/${provider}] Failed to store PKCE challenge:`, err);
-    return false;
-  }
-}
-
 /**
  * Middleware to retrieve PKCE challenge from cache using the OAuth state.
  * Reads state from req.oauthState (set by a preceding middleware).
@@ -148,7 +127,8 @@ function retrievePkceChallenge(provider) {
 
 router.get('/oauth/openid', async (req, res, next) => {
   const state = generateState();
-  const stored = await storePkceChallenge(state, req.query.code_challenge, 'openid');
+  const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE);
+  const stored = await storeAndStripChallenge(cache, req, state, 'openid');
   if (!stored) {
     return res.redirect(
       `${getAdminPanelUrl()}/auth/openid/callback?error=pkce_store_failed&error_description=Failed+to+store+PKCE+challenge`,
@@ -185,7 +165,8 @@ router.get(
 
 router.get('/oauth/saml', async (req, res, next) => {
   const state = generateState();
-  const stored = await storePkceChallenge(state, req.query.code_challenge, 'saml');
+  const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE);
+  const stored = await storeAndStripChallenge(cache, req, state, 'saml');
   if (!stored) {
     return res.redirect(
       `${getAdminPanelUrl()}/auth/saml/callback?error=pkce_store_failed&error_description=Failed+to+store+PKCE+challenge`,
@@ -222,7 +203,8 @@ router.post(
 
 router.get('/oauth/google', async (req, res, next) => {
   const state = generateState();
-  const stored = await storePkceChallenge(state, req.query.code_challenge, 'google');
+  const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE);
+  const stored = await storeAndStripChallenge(cache, req, state, 'google');
   if (!stored) {
     return res.redirect(
       `${getAdminPanelUrl()}/auth/google/callback?error=pkce_store_failed&error_description=Failed+to+store+PKCE+challenge`,
@@ -260,7 +242,8 @@ router.get(
 
 router.get('/oauth/github', async (req, res, next) => {
   const state = generateState();
-  const stored = await storePkceChallenge(state, req.query.code_challenge, 'github');
+  const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE);
+  const stored = await storeAndStripChallenge(cache, req, state, 'github');
   if (!stored) {
     return res.redirect(
       `${getAdminPanelUrl()}/auth/github/callback?error=pkce_store_failed&error_description=Failed+to+store+PKCE+challenge`,
@@ -298,7 +281,8 @@ router.get(
 
 router.get('/oauth/discord', async (req, res, next) => {
   const state = generateState();
-  const stored = await storePkceChallenge(state, req.query.code_challenge, 'discord');
+  const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE);
+  const stored = await storeAndStripChallenge(cache, req, state, 'discord');
   if (!stored) {
     return res.redirect(
       `${getAdminPanelUrl()}/auth/discord/callback?error=pkce_store_failed&error_description=Failed+to+store+PKCE+challenge`,
@@ -336,7 +320,8 @@ router.get(
 
 router.get('/oauth/facebook', async (req, res, next) => {
   const state = generateState();
-  const stored = await storePkceChallenge(state, req.query.code_challenge, 'facebook');
+  const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE);
+  const stored = await storeAndStripChallenge(cache, req, state, 'facebook');
   if (!stored) {
     return res.redirect(
       `${getAdminPanelUrl()}/auth/facebook/callback?error=pkce_store_failed&error_description=Failed+to+store+PKCE+challenge`,
@@ -374,7 +359,8 @@ router.get(
 
 router.get('/oauth/apple', async (req, res, next) => {
   const state = generateState();
-  const stored = await storePkceChallenge(state, req.query.code_challenge, 'apple');
+  const cache = getLogStores(CacheKeys.ADMIN_OAUTH_EXCHANGE);
+  const stored = await storeAndStripChallenge(cache, req, state, 'apple');
   if (!stored) {
     return res.redirect(
       `${getAdminPanelUrl()}/auth/apple/callback?error=pkce_store_failed&error_description=Failed+to+store+PKCE+challenge`,

api/server/routes/mcp.js

@@ -149,6 +149,29 @@ router.get('/:serverName/oauth/callback', async (req, res) => {
 
     if (oauthError) {
       logger.error('[MCP OAuth] OAuth error received', { error: oauthError });
+      // Gate failFlow behind callback validation to prevent DoS via leaked state
+      if (state && typeof state === 'string') {
+        try {
+          const flowsCache = getLogStores(CacheKeys.FLOWS);
+          const flowManager = getFlowStateManager(flowsCache);
+          const flowId = await MCPOAuthHandler.resolveStateToFlowId(state, flowManager);
+          if (flowId) {
+            const flowParts = flowId.split(':');
+            const [flowUserId] = flowParts;
+            const hasCsrf = validateOAuthCsrf(req, res, flowId, OAUTH_CSRF_COOKIE_PATH);
+            const hasSession = !hasCsrf && validateOAuthSession(req, flowUserId);
+            if (hasCsrf || hasSession) {
+              await flowManager.failFlow(flowId, 'mcp_oauth', String(oauthError));
+              logger.debug('[MCP OAuth] Marked flow as FAILED with OAuth error', {
+                flowId,
+                error: oauthError,
+              });
+            }
+          }
+        } catch (err) {
+          logger.debug('[MCP OAuth] Could not mark flow as failed', err);
+        }
+      }
       return res.redirect(
         `${basePath}/oauth/error?error=${encodeURIComponent(String(oauthError))}`,
       );

api/server/routes/roles.js

@@ -71,9 +71,7 @@ const createPermissionUpdateHandler = (permissionKey) => {
   const config = permissionConfigs[permissionKey];
 
   return async (req, res) => {
-    const { roleName: _r } = req.params;
-    // TODO: TEMP, use a better parsing for roleName
-    const roleName = _r.toUpperCase();
+    const { roleName } = req.params;
     const updates = req.body;
 
     try {
@@ -110,9 +108,7 @@ const createPermissionUpdateHandler = (permissionKey) => {
  * Get a specific role by name
  */
 router.get('/:roleName', async (req, res) => {
-  const { roleName: _r } = req.params;
-  // TODO: TEMP, use a better parsing for roleName
-  const roleName = _r.toUpperCase();
+  const { roleName } = req.params;
 
   try {
     let hasReadRoles = false;
@@ -121,7 +117,9 @@ router.get('/:roleName', async (req, res) => {
     } catch (err) {
       logger.warn(`[GET /roles/:roleName] capability check failed: ${err.message}`);
     }
-    if (!hasReadRoles && (roleName === SystemRoles.ADMIN || !roleDefaults[roleName])) {
+    const isOwnRole = req.user?.role === roleName;
+    const isDefaultRole = Object.hasOwn(roleDefaults, roleName);
+    if (!hasReadRoles && !isOwnRole && (roleName === SystemRoles.ADMIN || !isDefaultRole)) {
       return res.status(403).send({ message: 'Unauthorized' });
     }
 

api/server/services/MCP.js

@@ -23,7 +23,7 @@ const {
   getFlowStateManager,
   getMCPManager,
 } = require('~/config');
-const { findToken, createToken, updateToken } = require('~/models');
+const { findToken, createToken, updateToken, deleteTokens } = require('~/models');
 const { getGraphApiToken } = require('./GraphTokenService');
 const { reinitMCPServer } = require('./Tools/mcp');
 const { getAppConfig } = require('./Config');
@@ -644,6 +644,7 @@ function createToolInstance({
           findToken,
           createToken,
           updateToken,
+          deleteTokens,
         },
         oauthStart,
         oauthEnd,