diff --git a/api/server/controllers/mcp.js b/api/server/controllers/mcp.js index d58e1feacf..02e29d7596 100644 --- a/api/server/controllers/mcp.js +++ b/api/server/controllers/mcp.js @@ -7,13 +7,20 @@ */ const { logger } = require('@librechat/data-schemas'); const { + checkAccess, MCPErrorCodes, redactServerSecrets, redactAllServerSecrets, isMCPDomainNotAllowedError, isMCPInspectionFailedError, } = require('@librechat/api'); -const { Constants, MCPServerUserInputSchema } = require('librechat-data-provider'); +const { + Constants, + Permissions, + PermissionTypes, + MCPServerUserInputSchema, + MCP_USER_INPUT_FIELDS, +} = require('librechat-data-provider'); const { resolveConfigServers, resolveMcpConfigNames, @@ -21,6 +28,7 @@ const { } = require('~/server/services/MCP'); const { cacheMCPServerTools, getMCPServerTools } = require('~/server/services/Config'); const { getMCPManager, getMCPServersRegistry } = require('~/config'); +const db = require('~/models'); /** * Handles MCP-specific errors and sends appropriate HTTP responses. @@ -202,6 +210,63 @@ const getMCPServersList = async (req, res) => { } }; +/** + * Returns true when the request body's parsed config configures OBO. We block + * non-permission holders from creating or updating any DB-stored MCP server + * that mints per-user delegated tokens. + */ +function configHasObo(parsedConfig) { + return ( + !!parsedConfig && + typeof parsedConfig === 'object' && + 'obo' in parsedConfig && + parsedConfig.obo != null + ); +} + +/** + * Fields a user without `CONFIGURE_OBO` may modify on an OBO server (allowlist). + * Any field not on this list is locked: changes to it (add, modify, or remove) + * require the permission. Allowlisting is fail-closed — when upstream introduces + * a new MCP server config field, it lands in the locked set by default until + * explicitly opted in here. Anything that could redirect the OBO token flow + * (`url`, `proxy`, `headers`), change scopes (`obo`), or reroute auth (`oauth`, + * `apiKey`, `customUserVars`) MUST stay locked. + */ +const OBO_USER_EDITABLE_FIELDS = new Set(['title', 'description', 'iconPath']); + +/** + * Returns true when any non-allowlisted user-input field differs between the + * existing server config and the new payload. Treats add, remove, and modify + * as changes (stable JSON compare, with absence on either side counting as a + * change unless both sides are absent). The comparison surface is + * `MCP_USER_INPUT_FIELDS` (schema-derived from `MCPServerUserInputSchema`), + * so new fields on the schema are picked up automatically and stay locked + * by default until added to the allowlist above. + */ +function violatesOboLockdown(existingConfig, newConfig) { + for (const field of MCP_USER_INPUT_FIELDS) { + if (OBO_USER_EDITABLE_FIELDS.has(field)) continue; + const existing = existingConfig?.[field]; + const next = newConfig?.[field]; + if (existing === undefined && next === undefined) continue; + if (JSON.stringify(existing) !== JSON.stringify(next)) { + return true; + } + } + return false; +} + +async function callerCanConfigureObo(req) { + return checkAccess({ + req, + user: req.user, + permissionType: PermissionTypes.MCP_SERVERS, + permissions: [Permissions.CONFIGURE_OBO], + getRoleByName: db.getRoleByName, + }); +} + /** * Create MCP server * @route POST /api/mcp/servers @@ -218,6 +283,14 @@ const createMCPServerController = async (req, res) => { errors: validation.error.errors, }); } + if (configHasObo(validation.data) && !(await callerCanConfigureObo(req))) { + logger.warn( + `[createMCPServer] User ${userId} attempted to configure OBO without ${Permissions.CONFIGURE_OBO} permission`, + ); + return res + .status(403) + .json({ message: 'Forbidden: Insufficient permissions to configure OBO' }); + } const reservedServerNames = await resolveMcpConfigNames(req); const result = await getMCPServersRegistry().addServer( 'temp_server_name', @@ -285,6 +358,36 @@ const updateMCPServerController = async (req, res) => { errors: validation.error.errors, }); } + + /** + * On an existing OBO server, lock down every user-input field except the + * cosmetic allowlist (title, description, iconPath) for callers without + * CONFIGURE_OBO. This closes the OBO redirect vector — without it, a user + * with UPDATE could change `url` (or `proxy`/`headers`/`customUserVars`) + * to point OBO-minted tokens at an attacker-controlled endpoint. Adds, + * modifies, and removes are all caught. + */ + const existingConfig = await getMCPServersRegistry().getServerConfig(serverName, userId); + if (configHasObo(existingConfig) && !(await callerCanConfigureObo(req))) { + if (violatesOboLockdown(existingConfig, validation.data)) { + logger.warn( + `[updateMCPServer] User ${userId} attempted to modify a locked field on OBO server '${serverName}' without ${Permissions.CONFIGURE_OBO} permission`, + ); + return res + .status(403) + .json({ message: 'Forbidden: Insufficient permissions to configure OBO' }); + } + } else if (configHasObo(validation.data) && !(await callerCanConfigureObo(req))) { + // Adding OBO to a non-OBO server (or first-time configuration) still + // requires the permission, even if existing has no OBO. + logger.warn( + `[updateMCPServer] User ${userId} attempted to add OBO to '${serverName}' without ${Permissions.CONFIGURE_OBO} permission`, + ); + return res + .status(403) + .json({ message: 'Forbidden: Insufficient permissions to configure OBO' }); + } + const parsedConfig = await getMCPServersRegistry().updateServer( serverName, validation.data, diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index b86d339dac..96e8ff9d9e 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -154,6 +154,7 @@ describe('MCP Routes', () => { let app; let mongoServer; let mcpRouter; + let currentUser; beforeAll(async () => { mongoServer = await MongoMemoryServer.create(); @@ -168,7 +169,7 @@ describe('MCP Routes', () => { app.use(cookieParser()); app.use((req, res, next) => { - req.user = { id: 'test-user-id' }; + req.user = currentUser ?? { id: 'test-user-id' }; next(); }); @@ -182,9 +183,20 @@ describe('MCP Routes', () => { beforeEach(() => { jest.clearAllMocks(); + currentUser = undefined; mockResolveAllMcpConfigs.mockResolvedValue({}); mockResolveMcpConfigNames.mockResolvedValue([]); mockMCPUseAllowed = true; + /** + * Reset registry method implementations every test. `clearAllMocks` resets + * call records but NOT implementations, so a `.mockRejectedValue(...)` set + * by an earlier test leaks into later ones — including the new + * `getServerConfig` lookup in updateMCPServerController. + */ + mockRegistryInstance.getServerConfig.mockReset().mockResolvedValue(undefined); + mockRegistryInstance.addServer.mockReset(); + mockRegistryInstance.updateServer.mockReset(); + mockRegistryInstance.removeServer.mockReset(); }); describe('GET /:serverName/oauth/initiate', () => { @@ -2340,6 +2352,224 @@ describe('MCP Routes', () => { expect(response.body).toEqual({ message: 'Database connection failed' }); }); + describe('OBO permission gate', () => { + const oboConfig = { + type: 'streamable-http', + url: 'https://mcp-server.example.com/mcp', + title: 'OBO Server', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }; + const db = require('~/models'); + + beforeEach(() => { + currentUser = { id: 'test-user-id', role: 'USER' }; + mockRegistryInstance.addServer.mockResolvedValue({ + serverName: 'obo-server', + config: oboConfig, + }); + }); + + it('rejects POST with obo body when role lacks CONFIGURE_OBO', async () => { + db.getRoleByName.mockResolvedValue({ + name: 'USER', + permissions: { + MCP_SERVERS: { + USE: true, + CREATE: true, + SHARE: false, + SHARE_PUBLIC: false, + CONFIGURE_OBO: false, + }, + }, + }); + + const response = await request(app).post('/api/mcp/servers').send({ config: oboConfig }); + + expect(response.status).toBe(403); + expect(response.body.message).toMatch(/Insufficient permissions to configure OBO/); + expect(mockRegistryInstance.addServer).not.toHaveBeenCalled(); + }); + + it('allows POST with obo body when role has CONFIGURE_OBO', async () => { + db.getRoleByName.mockResolvedValue({ + name: 'USER', + permissions: { + MCP_SERVERS: { + USE: true, + CREATE: true, + SHARE: false, + SHARE_PUBLIC: false, + CONFIGURE_OBO: true, + }, + }, + }); + + const response = await request(app).post('/api/mcp/servers').send({ config: oboConfig }); + + expect(response.status).toBe(201); + expect(mockRegistryInstance.addServer).toHaveBeenCalled(); + }); + + it('allows POST without obo body regardless of CONFIGURE_OBO', async () => { + db.getRoleByName.mockResolvedValue({ + name: 'USER', + permissions: { + MCP_SERVERS: { + USE: true, + CREATE: true, + CONFIGURE_OBO: false, + }, + }, + }); + + const nonOboConfig = { + type: 'streamable-http', + url: 'https://mcp-server.example.com/mcp', + title: 'Plain Server', + }; + mockRegistryInstance.addServer.mockResolvedValue({ + serverName: 'plain-server', + config: nonOboConfig, + }); + + const response = await request(app).post('/api/mcp/servers').send({ config: nonOboConfig }); + + expect(response.status).toBe(201); + expect(db.getRoleByName).not.toHaveBeenCalled(); + expect(mockRegistryInstance.addServer).toHaveBeenCalled(); + }); + + it('rejects PATCH with obo body when role lacks CONFIGURE_OBO', async () => { + db.getRoleByName.mockResolvedValue({ + name: 'USER', + permissions: { + MCP_SERVERS: { + USE: true, + CREATE: true, + CONFIGURE_OBO: false, + }, + }, + }); + + const response = await request(app) + .patch('/api/mcp/servers/obo-server') + .send({ config: oboConfig }); + + expect(response.status).toBe(403); + expect(response.body.message).toMatch(/Insufficient permissions to configure OBO/); + expect(mockRegistryInstance.updateServer).not.toHaveBeenCalled(); + }); + + it('allows PATCH without CONFIGURE_OBO when OBO is unchanged', async () => { + // Editor without CONFIGURE_OBO should still be able to edit non-OBO fields + // (title, URL, description) on an OBO server as long as the OBO block is + // re-sent unchanged. Closes the regression where any save of an OBO server + // by such a user was rejected even when OBO itself was not being modified. + db.getRoleByName.mockResolvedValue({ + name: 'USER', + permissions: { + MCP_SERVERS: { + USE: true, + CREATE: true, + CONFIGURE_OBO: false, + }, + }, + }); + mockRegistryInstance.getServerConfig.mockResolvedValue({ + ...oboConfig, + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }); + mockRegistryInstance.updateServer.mockResolvedValue({ + ...oboConfig, + title: 'Renamed OBO Server', + }); + + const response = await request(app) + .patch('/api/mcp/servers/obo-server') + .send({ + config: { + ...oboConfig, + title: 'Renamed OBO Server', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }, + }); + + expect(response.status).toBe(200); + expect(mockRegistryInstance.updateServer).toHaveBeenCalled(); + }); + + it('rejects PATCH that removes OBO from an existing OBO server without CONFIGURE_OBO', async () => { + // Closes the silent-downgrade vector: a user with UPDATE but not + // CONFIGURE_OBO must not be able to convert an OBO server to non-OBO, + // because doing so de-secures the server end-to-end. + db.getRoleByName.mockResolvedValue({ + name: 'USER', + permissions: { + MCP_SERVERS: { + USE: true, + CREATE: true, + CONFIGURE_OBO: false, + }, + }, + }); + mockRegistryInstance.getServerConfig.mockResolvedValue({ + ...oboConfig, + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }); + + // Submit body that omits the obo field (auth_type changed away from OBO) + const downgradePayload = { + type: 'streamable-http', + url: 'https://mcp-server.example.com/mcp', + title: 'OBO Server', + }; + const response = await request(app) + .patch('/api/mcp/servers/obo-server') + .send({ config: downgradePayload }); + + expect(response.status).toBe(403); + expect(response.body.message).toMatch(/Insufficient permissions to configure OBO/); + expect(mockRegistryInstance.updateServer).not.toHaveBeenCalled(); + }); + + it('rejects PATCH that redirects the URL of an existing OBO server without CONFIGURE_OBO', async () => { + // Closes the OBO redirect vector — the original trust-boundary concern + // CONFIGURE_OBO was introduced to address. A user with UPDATE but + // without the permission must not be able to point an existing OBO + // server at an attacker-controlled endpoint, which would cause OBO + // tokens minted for other users to be exfiltrated to that endpoint. + // The same allowlist policy also covers `proxy`, `headers`, transport + // type, and auth blocks. + db.getRoleByName.mockResolvedValue({ + name: 'USER', + permissions: { + MCP_SERVERS: { + USE: true, + CREATE: true, + CONFIGURE_OBO: false, + }, + }, + }); + mockRegistryInstance.getServerConfig.mockResolvedValue({ + ...oboConfig, + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }); + + const redirectPayload = { + ...oboConfig, + url: 'https://attacker.example.com/mcp', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }; + const response = await request(app) + .patch('/api/mcp/servers/obo-server') + .send({ config: redirectPayload }); + + expect(response.status).toBe(403); + expect(response.body.message).toMatch(/Insufficient permissions to configure OBO/); + expect(mockRegistryInstance.updateServer).not.toHaveBeenCalled(); + }); + }); + it('should fail closed when config-managed names cannot be resolved', async () => { const validConfig = { type: 'sse', diff --git a/api/server/services/GraphTokenService.js b/api/server/services/GraphTokenService.js index 843adbe5a2..5eba0f2d0e 100644 --- a/api/server/services/GraphTokenService.js +++ b/api/server/services/GraphTokenService.js @@ -1,77 +1,19 @@ -const client = require('openid-client'); const { logger } = require('@librechat/data-schemas'); -const { CacheKeys } = require('librechat-data-provider'); -const { getOpenIdConfig } = require('~/strategies/openidStrategy'); -const getLogStores = require('~/cache/getLogStores'); +const { exchangeOboToken } = require('./OboTokenService'); /** - * Get Microsoft Graph API token using existing token exchange mechanism + * Get Microsoft Graph API token using the On-Behalf-Of flow. + * Thin wrapper around the generic OBO exchange for Graph-specific error context. + * * @param {Object} user - User object with OpenID information * @param {string} accessToken - Federated access token used as OBO assertion * @param {string} scopes - Graph API scopes for the token - * @param {boolean} fromCache - Whether to try getting token from cache first + * @param {boolean} [fromCache=true] - Whether to try getting token from cache first * @returns {Promise} Graph API token response with access_token and expires_in */ async function getGraphApiToken(user, accessToken, scopes, fromCache = true) { try { - if (!user.openidId) { - throw new Error('User must be authenticated via Entra ID to access Microsoft Graph'); - } - - if (!accessToken) { - throw new Error('Access token is required for token exchange'); - } - - if (!scopes) { - throw new Error('Graph API scopes are required for token exchange'); - } - - const config = getOpenIdConfig(); - if (!config) { - throw new Error('OpenID configuration not available'); - } - - const cacheKey = `${user.openidId}:${scopes}`; - const tokensCache = getLogStores(CacheKeys.OPENID_EXCHANGED_TOKENS); - - if (fromCache) { - const cachedToken = await tokensCache.get(cacheKey); - if (cachedToken) { - logger.debug(`[GraphTokenService] Using cached Graph API token for user: ${user.openidId}`); - return cachedToken; - } - } - - logger.debug(`[GraphTokenService] Requesting new Graph API token for user: ${user.openidId}`); - logger.debug(`[GraphTokenService] Requested scopes: ${scopes}`); - - const grantResponse = await client.genericGrantRequest( - config, - 'urn:ietf:params:oauth:grant-type:jwt-bearer', - { - scope: scopes, - assertion: accessToken, - requested_token_use: 'on_behalf_of', - }, - ); - - const tokenResponse = { - access_token: grantResponse.access_token, - token_type: 'Bearer', - expires_in: grantResponse.expires_in || 3600, - scope: scopes, - }; - - await tokensCache.set( - cacheKey, - tokenResponse, - (grantResponse.expires_in || 3600) * 1000, // Convert to milliseconds - ); - - logger.debug( - `[GraphTokenService] Successfully obtained and cached Graph API token for user: ${user.openidId}`, - ); - return tokenResponse; + return await exchangeOboToken(user, accessToken, scopes, fromCache); } catch (error) { logger.error( `[GraphTokenService] Failed to acquire Graph API token for user ${user.openidId}:`, diff --git a/api/server/services/MCP.js b/api/server/services/MCP.js index e8f6820d6e..8035bcc3df 100644 --- a/api/server/services/MCP.js +++ b/api/server/services/MCP.js @@ -34,6 +34,8 @@ const { const db = require('~/models'); const { findToken, createToken, updateToken, deleteTokens } = db; const { getGraphApiToken } = require('./GraphTokenService'); +const { exchangeOboToken } = require('./OboTokenService'); +const { createOboTrustChecker } = require('./OboPolicyService'); const { reinitMCPServer } = require('./Tools/mcp'); const { getAppConfig } = require('./Config'); const { getLogStores } = require('~/cache'); @@ -738,6 +740,8 @@ function createToolInstance({ oauthStart, oauthEnd, graphTokenResolver: getGraphApiToken, + oboTokenResolver: exchangeOboToken, + oboTrustChecker: createOboTrustChecker(), }); if (isAssistantsEndpoint(provider) && Array.isArray(result)) { diff --git a/api/server/services/OboPolicyService.js b/api/server/services/OboPolicyService.js new file mode 100644 index 0000000000..99949a2701 --- /dev/null +++ b/api/server/services/OboPolicyService.js @@ -0,0 +1,43 @@ +const { isOboConfigStillTrusted } = require('@librechat/api'); +const db = require('~/models'); + +/** + * Checks whether a parsed MCP server config is DB-sourced (user-created) using + * the same `isUserSourced` heuristics as the rest of the MCP layer: an explicit + * `source` is authoritative when present; otherwise `dbId` presence is used. + */ +function isDbSourced({ source, dbId }) { + if (source != null) { + return source === 'user'; + } + return !!dbId; +} + +/** + * Builds the predicate the MCP runtime calls before performing an OBO token exchange. + * + * YAML/Config-sourced configs (admin-defined) bypass the check — admins are + * already trusted at the deployment level. DB-sourced configs (created via the + * UI) are gated on the original author still holding `MCP_SERVERS.CONFIGURE_OBO`, + * so retained configs fail closed when an author's role is downgraded. + */ +function createOboTrustChecker() { + return async ({ source, author, dbId }) => { + if (!isDbSourced({ source, dbId })) { + return true; + } + return isOboConfigStillTrusted({ + authorId: author, + getUserRoleByAuthorId: async (userId) => { + const user = await db.findUser({ _id: userId }, 'role'); + return user?.role; + }, + getRolePermissions: async (roleName) => { + const role = await db.getRoleByName(roleName); + return role?.permissions; + }, + }); + }; +} + +module.exports = { createOboTrustChecker }; diff --git a/api/server/services/OboTokenService.js b/api/server/services/OboTokenService.js new file mode 100644 index 0000000000..be269be993 --- /dev/null +++ b/api/server/services/OboTokenService.js @@ -0,0 +1,194 @@ +const client = require('openid-client'); +const { logger } = require('@librechat/data-schemas'); +const { CacheKeys } = require('librechat-data-provider'); +const { getOpenIdConfig } = require('~/strategies/openidStrategy'); +const getLogStores = require('~/cache/getLogStores'); + +const RETRYABLE_STATUS_CODES = new Set([429, 500, 502, 503, 504]); +const RETRYABLE_ERROR_CODES = new Set(['ETIMEDOUT', 'ECONNRESET', 'EAI_AGAIN', 'ENOTFOUND']); +const OBO_RETRY_DELAY_MS = 300; + +/** + * In-flight OBO exchanges keyed by `${openidId}:${scopes}`. + * + * Without coalescing, parallel tool calls that arrive on a cache miss each issue + * their own jwt-bearer request to the IdP. Under fan-out, Entra intermittently + * returns errors that look non-retryable, surfacing as "identity provider + * rejected the OBO token exchange." A user retry then hits the populated cache + * and succeeds, which matches the observed flakiness. Sharing a single upstream + * exchange per key removes the thundering herd. + */ +const inFlightExchanges = new Map(); + +function getErrorStatus(error) { + return error?.status ?? error?.statusCode ?? error?.response?.status; +} + +function getErrorCode(error) { + return typeof error?.code === 'string' ? error.code.toUpperCase() : undefined; +} + +function isRetryableOboExchangeError(error) { + const status = getErrorStatus(error); + if (status != null && RETRYABLE_STATUS_CODES.has(status)) { + return true; + } + + const code = getErrorCode(error); + if (code != null && RETRYABLE_ERROR_CODES.has(code)) { + return true; + } + + const message = String(error?.message ?? '').toLowerCase(); + return ( + message.includes('timed out') || + message.includes('timeout') || + message.includes('econnreset') || + message.includes('socket hang up') || + message.includes('temporarily unavailable') || + message.includes('too many requests') || + message.includes('service unavailable') + ); +} + +function tagOboExchangeError(error, retryable) { + if (error && typeof error === 'object') { + error.retryable = retryable; + error.oboFailureReason = 'exchange_failed'; + } + return error; +} + +async function delay(ms) { + await new Promise((resolve) => setTimeout(resolve, ms)); +} + +async function performOboExchange({ user, accessToken, scopes, config, tokensCache, cacheKey }) { + const requestGrant = async () => + client.genericGrantRequest(config, 'urn:ietf:params:oauth:grant-type:jwt-bearer', { + scope: scopes, + assertion: accessToken, + requested_token_use: 'on_behalf_of', + }); + + let grantResponse; + try { + grantResponse = await requestGrant(); + } catch (error) { + const retryable = isRetryableOboExchangeError(error); + if (!retryable) { + throw tagOboExchangeError(error, false); + } + + logger.warn( + `[OboTokenService] Transient OBO exchange failure for user: ${user.openidId}, retrying once`, + error, + ); + await delay(OBO_RETRY_DELAY_MS); + + try { + grantResponse = await requestGrant(); + } catch (retryError) { + throw tagOboExchangeError(retryError, isRetryableOboExchangeError(retryError)); + } + } + + const tokenResponse = { + access_token: grantResponse.access_token, + token_type: 'Bearer', + expires_in: grantResponse.expires_in || 3600, + scope: scopes, + }; + + await tokensCache.set(cacheKey, tokenResponse, (grantResponse.expires_in || 3600) * 1000); + + logger.debug( + `[OboTokenService] Successfully obtained and cached OBO token for user: ${user.openidId}`, + ); + return tokenResponse; +} + +/** + * Exchange a user's access token for a downstream-scoped token via the + * OAuth 2.0 On-Behalf-Of (jwt-bearer) grant. + * + * Concurrent callers for the same `${openidId}:${scopes}` key share a single + * upstream exchange (see `inFlightExchanges`) so a fan-out of tool calls right + * after a cache miss does not produce N parallel requests to the IdP. + * + * @param {Object} user - User object with OpenID information + * @param {string} accessToken - Federated access token used as OBO assertion + * @param {string} scopes - Scopes to request for the downstream service + * @param {boolean} [fromCache=true] - When true, read from cache and join any + * in-flight exchange. When false, bypass both and force a fresh exchange. + * @returns {Promise} Token response with access_token and expires_in + */ +async function exchangeOboToken(user, accessToken, scopes, fromCache = true) { + if (!user.openidId) { + throw new Error('User must be authenticated via OpenID to perform OBO token exchange'); + } + + if (!accessToken) { + throw new Error('Access token is required for OBO exchange'); + } + + if (!scopes) { + throw new Error('Scopes are required for OBO exchange'); + } + + const config = getOpenIdConfig(); + if (!config) { + throw new Error('OpenID configuration not available'); + } + + const cacheKey = `${user.openidId}:${scopes}`; + const tokensCache = getLogStores(CacheKeys.OPENID_EXCHANGED_TOKENS); + + if (fromCache) { + const cachedToken = await tokensCache.get(cacheKey); + if (cachedToken) { + logger.debug(`[OboTokenService] Using cached token for user: ${user.openidId}`); + return cachedToken; + } + + const inFlight = inFlightExchanges.get(cacheKey); + if (inFlight) { + logger.debug(`[OboTokenService] Joining in-flight OBO exchange for user: ${user.openidId}`); + return inFlight; + } + } + + logger.debug( + `[OboTokenService] Requesting new OBO token for user: ${user.openidId}, scopes: ${scopes}`, + ); + + const exchangePromise = performOboExchange({ + user, + accessToken, + scopes, + config, + tokensCache, + cacheKey, + }); + + if (fromCache) { + inFlightExchanges.set(cacheKey, exchangePromise); + exchangePromise + .finally(() => { + if (inFlightExchanges.get(cacheKey) === exchangePromise) { + inFlightExchanges.delete(cacheKey); + } + }) + .catch(() => { + /* The original rejection is delivered to the awaiting caller; this + * chain exists only to run cleanup, so swallow it here to avoid an + * unhandled-rejection warning on the cleanup promise. */ + }); + } + + return exchangePromise; +} + +module.exports = { + exchangeOboToken, +}; diff --git a/api/server/services/OboTokenService.spec.js b/api/server/services/OboTokenService.spec.js new file mode 100644 index 0000000000..122ddee4b2 --- /dev/null +++ b/api/server/services/OboTokenService.spec.js @@ -0,0 +1,342 @@ +jest.mock('~/strategies/openidStrategy'); +jest.mock('~/cache/getLogStores'); +jest.mock('@librechat/data-schemas', () => ({ + ...jest.requireActual('@librechat/data-schemas'), + logger: { + error: jest.fn(), + debug: jest.fn(), + warn: jest.fn(), + }, +})); + +const client = require('openid-client'); +const { getOpenIdConfig } = require('~/strategies/openidStrategy'); +const getLogStores = require('~/cache/getLogStores'); +const { exchangeOboToken } = require('./OboTokenService'); + +describe('OboTokenService', () => { + let mockTokensCache; + let mockOpenIdConfig; + + const mockUser = { + openidId: 'oidc-sub-123', + email: 'test@example.com', + name: 'Test User', + }; + + beforeEach(() => { + jest.clearAllMocks(); + + mockTokensCache = { + get: jest.fn().mockResolvedValue(null), + set: jest.fn().mockResolvedValue(undefined), + }; + getLogStores.mockReturnValue(mockTokensCache); + + mockOpenIdConfig = { + client_id: 'test-client-id', + issuer: 'https://login.microsoftonline.com/tenant-id/v2.0', + }; + getOpenIdConfig.mockReturnValue(mockOpenIdConfig); + + client.genericGrantRequest.mockResolvedValue({ + access_token: 'exchanged-obo-token', + expires_in: 3600, + }); + }); + + describe('input validation', () => { + it('should throw when user has no openidId', async () => { + await expect( + exchangeOboToken({ email: 'test@example.com' }, 'access-token', 'api://scope'), + ).rejects.toThrow('User must be authenticated via OpenID to perform OBO token exchange'); + }); + + it('should throw when accessToken is missing', async () => { + await expect(exchangeOboToken(mockUser, '', 'api://scope')).rejects.toThrow( + 'Access token is required for OBO exchange', + ); + }); + + it('should throw when scopes are missing', async () => { + await expect(exchangeOboToken(mockUser, 'access-token', '')).rejects.toThrow( + 'Scopes are required for OBO exchange', + ); + }); + + it('should throw when OpenID config is not available', async () => { + getOpenIdConfig.mockReturnValue(null); + + await expect(exchangeOboToken(mockUser, 'access-token', 'api://scope')).rejects.toThrow( + 'OpenID configuration not available', + ); + }); + }); + + describe('cache behavior', () => { + it('should return cached token when fromCache is true and cache hit', async () => { + const cachedToken = { + access_token: 'cached-obo-token', + token_type: 'Bearer', + expires_in: 1800, + scope: 'api://mcp-server/Scope.Read', + }; + mockTokensCache.get.mockResolvedValue(cachedToken); + + const result = await exchangeOboToken( + mockUser, + 'access-token', + 'api://mcp-server/Scope.Read', + true, + ); + + expect(result).toBe(cachedToken); + expect(mockTokensCache.get).toHaveBeenCalledWith('oidc-sub-123:api://mcp-server/Scope.Read'); + expect(client.genericGrantRequest).not.toHaveBeenCalled(); + }); + + it('should skip cache when fromCache is false', async () => { + const cachedToken = { access_token: 'cached-obo-token' }; + mockTokensCache.get.mockResolvedValue(cachedToken); + + const result = await exchangeOboToken( + mockUser, + 'access-token', + 'api://mcp-server/Scope.Read', + false, + ); + + expect(mockTokensCache.get).not.toHaveBeenCalled(); + expect(client.genericGrantRequest).toHaveBeenCalled(); + expect(result.access_token).toBe('exchanged-obo-token'); + }); + + it('should default fromCache to true', async () => { + mockTokensCache.get.mockResolvedValue(null); + + await exchangeOboToken(mockUser, 'access-token', 'api://scope'); + + expect(mockTokensCache.get).toHaveBeenCalledWith('oidc-sub-123:api://scope'); + }); + }); + + describe('OBO token exchange', () => { + it('should call genericGrantRequest with jwt-bearer grant type', async () => { + await exchangeOboToken(mockUser, 'user-access-token', 'api://mcp-server/Tools.ReadWrite'); + + expect(client.genericGrantRequest).toHaveBeenCalledWith( + mockOpenIdConfig, + 'urn:ietf:params:oauth:grant-type:jwt-bearer', + { + scope: 'api://mcp-server/Tools.ReadWrite', + assertion: 'user-access-token', + requested_token_use: 'on_behalf_of', + }, + ); + }); + + it('should return token response with correct structure', async () => { + const result = await exchangeOboToken( + mockUser, + 'access-token', + 'api://mcp-server/Tools.ReadWrite', + ); + + expect(result).toEqual({ + access_token: 'exchanged-obo-token', + token_type: 'Bearer', + expires_in: 3600, + scope: 'api://mcp-server/Tools.ReadWrite', + }); + }); + + it('should cache the exchanged token with correct TTL', async () => { + client.genericGrantRequest.mockResolvedValue({ + access_token: 'new-obo-token', + expires_in: 1800, + }); + + await exchangeOboToken(mockUser, 'access-token', 'api://scope'); + + expect(mockTokensCache.set).toHaveBeenCalledWith( + 'oidc-sub-123:api://scope', + { + access_token: 'new-obo-token', + token_type: 'Bearer', + expires_in: 1800, + scope: 'api://scope', + }, + 1800 * 1000, + ); + }); + + it('should default expires_in to 3600 when not in response', async () => { + client.genericGrantRequest.mockResolvedValue({ + access_token: 'no-expiry-token', + }); + + const result = await exchangeOboToken(mockUser, 'access-token', 'api://scope'); + + expect(result.expires_in).toBe(3600); + expect(mockTokensCache.set).toHaveBeenCalledWith( + 'oidc-sub-123:api://scope', + expect.objectContaining({ expires_in: 3600 }), + 3600 * 1000, + ); + }); + + it('should propagate errors from genericGrantRequest', async () => { + client.genericGrantRequest.mockRejectedValue( + new Error('invalid_grant: AADSTS50013: Assertion failed signature validation'), + ); + + await expect(exchangeOboToken(mockUser, 'bad-token', 'api://scope')).rejects.toThrow( + 'invalid_grant: AADSTS50013: Assertion failed signature validation', + ); + }); + + it('should retry once for transient Entra failures and succeed on the second attempt', async () => { + const transientError = Object.assign(new Error('Service unavailable'), { status: 503 }); + const setTimeoutSpy = jest.spyOn(global, 'setTimeout').mockImplementation((callback) => { + callback(); + return 0; + }); + + try { + client.genericGrantRequest.mockRejectedValueOnce(transientError).mockResolvedValueOnce({ + access_token: 'retried-obo-token', + expires_in: 1800, + }); + + const result = await exchangeOboToken(mockUser, 'access-token', 'api://scope'); + + expect(client.genericGrantRequest).toHaveBeenCalledTimes(2); + expect(result).toEqual({ + access_token: 'retried-obo-token', + token_type: 'Bearer', + expires_in: 1800, + scope: 'api://scope', + }); + } finally { + setTimeoutSpy.mockRestore(); + } + }); + + it('should not retry permanent OBO exchange failures', async () => { + const permanentError = new Error( + 'invalid_grant: AADSTS50013: Assertion failed signature validation', + ); + client.genericGrantRequest.mockRejectedValue(permanentError); + + await expect(exchangeOboToken(mockUser, 'bad-token', 'api://scope')).rejects.toThrow( + 'invalid_grant: AADSTS50013: Assertion failed signature validation', + ); + + expect(client.genericGrantRequest).toHaveBeenCalledTimes(1); + }); + }); + + describe('cache key isolation', () => { + it('should use different cache keys for different scopes', async () => { + await exchangeOboToken(mockUser, 'access-token', 'api://server-a/Scope.A'); + await exchangeOboToken(mockUser, 'access-token', 'api://server-b/Scope.B'); + + expect(mockTokensCache.get).toHaveBeenCalledWith('oidc-sub-123:api://server-a/Scope.A'); + expect(mockTokensCache.get).toHaveBeenCalledWith('oidc-sub-123:api://server-b/Scope.B'); + }); + + it('should use different cache keys for different users', async () => { + const otherUser = { openidId: 'oidc-sub-456', email: 'other@example.com' }; + + await exchangeOboToken(mockUser, 'access-token', 'api://scope'); + await exchangeOboToken(otherUser, 'access-token', 'api://scope'); + + expect(mockTokensCache.get).toHaveBeenCalledWith('oidc-sub-123:api://scope'); + expect(mockTokensCache.get).toHaveBeenCalledWith('oidc-sub-456:api://scope'); + }); + }); + + describe('single-flight coalescing', () => { + /** Yields long enough for both pending callers to advance past their cache lookup. */ + const flushMicrotasks = () => new Promise((resolve) => setImmediate(resolve)); + + it('coalesces concurrent exchanges for the same key into one IdP call', async () => { + let resolveGrant; + client.genericGrantRequest.mockReturnValueOnce( + new Promise((resolve) => { + resolveGrant = resolve; + }), + ); + + const callA = exchangeOboToken(mockUser, 'access-token', 'api://shared'); + const callB = exchangeOboToken(mockUser, 'access-token', 'api://shared'); + + await flushMicrotasks(); + expect(client.genericGrantRequest).toHaveBeenCalledTimes(1); + + resolveGrant({ access_token: 'shared-obo-token', expires_in: 3600 }); + + const [resultA, resultB] = await Promise.all([callA, callB]); + expect(resultA).toEqual(resultB); + expect(resultA.access_token).toBe('shared-obo-token'); + expect(client.genericGrantRequest).toHaveBeenCalledTimes(1); + expect(mockTokensCache.set).toHaveBeenCalledTimes(1); + }); + + it('does not coalesce exchanges for different keys', async () => { + await Promise.all([ + exchangeOboToken(mockUser, 'access-token', 'api://scope-a'), + exchangeOboToken(mockUser, 'access-token', 'api://scope-b'), + ]); + + expect(client.genericGrantRequest).toHaveBeenCalledTimes(2); + }); + + it('clears the in-flight slot after a successful exchange', async () => { + await exchangeOboToken(mockUser, 'access-token', 'api://scope'); + expect(client.genericGrantRequest).toHaveBeenCalledTimes(1); + + await exchangeOboToken(mockUser, 'access-token', 'api://scope'); + expect(client.genericGrantRequest).toHaveBeenCalledTimes(2); + }); + + it('clears the in-flight slot after a failed exchange', async () => { + client.genericGrantRequest + .mockRejectedValueOnce( + new Error('invalid_grant: AADSTS50013: Assertion failed signature validation'), + ) + .mockResolvedValueOnce({ access_token: 'fresh-token', expires_in: 3600 }); + + await expect(exchangeOboToken(mockUser, 'access-token', 'api://scope')).rejects.toThrow( + 'invalid_grant', + ); + + const result = await exchangeOboToken(mockUser, 'access-token', 'api://scope'); + expect(result.access_token).toBe('fresh-token'); + expect(client.genericGrantRequest).toHaveBeenCalledTimes(2); + }); + + it('bypasses in-flight coalescing when fromCache is false', async () => { + let resolveFirst; + client.genericGrantRequest + .mockReturnValueOnce( + new Promise((resolve) => { + resolveFirst = resolve; + }), + ) + .mockResolvedValueOnce({ access_token: 'forced-fresh-token', expires_in: 3600 }); + + const callA = exchangeOboToken(mockUser, 'access-token', 'api://scope', true); + await flushMicrotasks(); + + const callB = exchangeOboToken(mockUser, 'access-token', 'api://scope', false); + expect(client.genericGrantRequest).toHaveBeenCalledTimes(2); + + resolveFirst({ access_token: 'in-flight-token', expires_in: 3600 }); + + const [resultA, resultB] = await Promise.all([callA, callB]); + expect(resultA.access_token).toBe('in-flight-token'); + expect(resultB.access_token).toBe('forced-fresh-token'); + }); + }); +}); diff --git a/api/server/services/Tools/mcp.js b/api/server/services/Tools/mcp.js index 4b654fa11a..e13fa9989b 100644 --- a/api/server/services/Tools/mcp.js +++ b/api/server/services/Tools/mcp.js @@ -3,6 +3,8 @@ const { getMissingCustomUserVars } = require('@librechat/api'); const { CacheKeys, Constants } = require('librechat-data-provider'); const { getMCPManager, getMCPServersRegistry, getFlowStateManager } = require('~/config'); const { findToken, createToken, updateToken, deleteTokens } = require('~/models'); +const { exchangeOboToken } = require('~/server/services/OboTokenService'); +const { createOboTrustChecker } = require('~/server/services/OboPolicyService'); const { updateMCPServerTools } = require('~/server/services/Config'); const { getLogStores } = require('~/cache'); @@ -133,6 +135,8 @@ async function reinitMCPServer({ customUserVars, connectionTimeout, serverConfig, + oboTokenResolver: exchangeOboToken, + oboTrustChecker: createOboTrustChecker(), }); logger.info(`[MCP Reinitialize] Successfully established connection for ${serverName}`); @@ -166,6 +170,8 @@ async function reinitMCPServer({ customUserVars, connectionTimeout, configServers, + oboTokenResolver: exchangeOboToken, + oboTrustChecker: createOboTrustChecker(), }); if (discoveryResult.tools && discoveryResult.tools.length > 0) { diff --git a/api/server/services/__tests__/MCP.spec.js b/api/server/services/__tests__/MCP.spec.js index cfc71b7752..ca3d5eea9a 100644 --- a/api/server/services/__tests__/MCP.spec.js +++ b/api/server/services/__tests__/MCP.spec.js @@ -43,6 +43,12 @@ jest.mock('~/models', () => ({ jest.mock('~/server/services/GraphTokenService', () => ({ getGraphApiToken: jest.fn(), })); +jest.mock('~/server/services/OboTokenService', () => ({ + exchangeOboToken: jest.fn(), +})); +jest.mock('~/server/services/OboPolicyService', () => ({ + createOboTrustChecker: jest.fn(() => async () => true), +})); jest.mock('~/server/services/Tools/mcp', () => ({ reinitMCPServer: jest.fn(), })); diff --git a/client/src/components/SidePanel/MCPBuilder/MCPAdminSettings.tsx b/client/src/components/SidePanel/MCPBuilder/MCPAdminSettings.tsx index af677999bc..617502a00d 100644 --- a/client/src/components/SidePanel/MCPBuilder/MCPAdminSettings.tsx +++ b/client/src/components/SidePanel/MCPBuilder/MCPAdminSettings.tsx @@ -10,6 +10,10 @@ const permissions: PermissionConfig[] = [ { permission: Permissions.CREATE, labelKey: 'com_ui_mcp_servers_allow_create' }, { permission: Permissions.SHARE, labelKey: 'com_ui_mcp_servers_allow_share' }, { permission: Permissions.SHARE_PUBLIC, labelKey: 'com_ui_mcp_servers_allow_share_public' }, + { + permission: Permissions.CONFIGURE_OBO, + labelKey: 'com_ui_mcp_servers_allow_configure_obo', + }, ]; const MCPAdminSettings = () => { diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/MCPServerForm.tsx b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/MCPServerForm.tsx index d4096ea96a..ac078cae6f 100644 --- a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/MCPServerForm.tsx +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/MCPServerForm.tsx @@ -1,5 +1,8 @@ -import { FormProvider } from 'react-hook-form'; -import type { useMCPServerForm } from './hooks/useMCPServerForm'; +import { FormProvider, useWatch } from 'react-hook-form'; +import { Permissions, PermissionTypes } from 'librechat-data-provider'; +import { useHasAccess } from '~/hooks'; +import type { useMCPServerForm, MCPServerFormData } from './hooks/useMCPServerForm'; +import { AuthTypeEnum } from './hooks/useMCPServerForm'; import ConnectionSection from './sections/ConnectionSection'; import BasicInfoSection from './sections/BasicInfoSection'; import TransportSection from './sections/TransportSection'; @@ -13,18 +16,40 @@ interface MCPServerFormProps { export default function MCPServerForm({ formHook }: MCPServerFormProps) { const { methods, isEditMode, server } = formHook; + const canConfigureObo = useHasAccess({ + permissionType: PermissionTypes.MCP_SERVERS, + permission: Permissions.CONFIGURE_OBO, + }); + + /** + * Lockdown applies when a user without CONFIGURE_OBO opens an OBO server in + * edit mode. Mirrors the backend allowlist policy: every field outside title, + * description, and iconPath is read-only (URL, transport, auth, trust). Any + * other change would let the user redirect OBO tokens to an arbitrary endpoint. + */ + const authType = useWatch({ + control: methods.control, + name: 'auth.auth_type', + }); + const isOboLockedReadOnly = isEditMode && authType === AuthTypeEnum.OBO && !canConfigureObo; + return (
+
+ - + - + - - - + +
); diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/hooks/useMCPServerForm.ts b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/hooks/useMCPServerForm.ts index 9522f450df..7f42eee19b 100644 --- a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/hooks/useMCPServerForm.ts +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/hooks/useMCPServerForm.ts @@ -16,6 +16,7 @@ export enum AuthTypeEnum { None = 'none', ServiceHttp = 'service_http', OAuth = 'oauth', + OBO = 'obo', } // Authorization type enum @@ -37,6 +38,7 @@ export interface AuthConfig { oauth_authorization_url?: string; oauth_token_url?: string; oauth_scope?: string; + obo_scopes?: string; server_id?: string; } @@ -77,7 +79,9 @@ export function useMCPServerForm({ server, onSuccess, onClose }: UseMCPServerFor const defaultValues = useMemo(() => { if (server) { let authType = AuthTypeEnum.None; - if (server.config.oauth) { + if ('obo' in server.config && server.config.obo) { + authType = AuthTypeEnum.OBO; + } else if (server.config.oauth) { authType = AuthTypeEnum.OAuth; } else if ('apiKey' in server.config && server.config.apiKey) { authType = AuthTypeEnum.ServiceHttp; @@ -104,6 +108,7 @@ export function useMCPServerForm({ server, onSuccess, onClose }: UseMCPServerFor oauth_authorization_url: server.config.oauth?.authorization_url || '', oauth_token_url: server.config.oauth?.token_url || '', oauth_scope: server.config.oauth?.scope || '', + obo_scopes: 'obo' in server.config && server.config.obo ? server.config.obo.scopes : '', server_id: server.serverName, }, trust: true, // Pre-checked for existing servers @@ -127,6 +132,7 @@ export function useMCPServerForm({ server, onSuccess, onClose }: UseMCPServerFor oauth_authorization_url: '', oauth_token_url: '', oauth_scope: '', + obo_scopes: '', }, trust: false, }; @@ -142,7 +148,6 @@ export function useMCPServerForm({ server, onSuccess, onClose }: UseMCPServerFor // Watch URL for auto-fill const watchedUrl = watch('url'); - const watchedTitle = watch('title'); // Auto-fill title from URL when title is empty const handleUrlChange = useCallback( @@ -221,6 +226,10 @@ export function useMCPServerForm({ server, onSuccess, onClose }: UseMCPServerFor }; } + if (formData.auth.auth_type === AuthTypeEnum.OBO && formData.auth.obo_scopes) { + config.obo = { scopes: formData.auth.obo_scopes }; + } + const params: MCPServerCreateParams = { config }; const result = server diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx index 4c99c793b0..fb31c95949 100644 --- a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx @@ -1,10 +1,11 @@ import { useMemo, useState } from 'react'; import { Copy, CopyCheck } from 'lucide-react'; import { useFormContext, useWatch } from 'react-hook-form'; +import { Permissions, PermissionTypes } from 'librechat-data-provider'; import { Label, Input, Checkbox, SecretInput, Radio, useToastContext } from '@librechat/client'; import { AuthTypeEnum, AuthorizationTypeEnum } from '../hooks/useMCPServerForm'; import type { MCPServerFormData } from '../hooks/useMCPServerForm'; -import { useLocalize, useCopyToClipboard } from '~/hooks'; +import { useLocalize, useCopyToClipboard, useHasAccess } from '~/hooks'; import { cn } from '~/utils'; interface AuthSectionProps { @@ -23,6 +24,11 @@ export default function AuthSection({ isEditMode, serverName }: AuthSectionProps const [isCopying, setIsCopying] = useState(false); + const canConfigureObo = useHasAccess({ + permissionType: PermissionTypes.MCP_SERVERS, + permission: Permissions.CONFIGURE_OBO, + }); + const authType = useWatch({ name: 'auth.auth_type', }) as AuthTypeEnum; @@ -41,14 +47,31 @@ export default function AuthSection({ isEditMode, serverName }: AuthSectionProps const copyLink = useCopyToClipboard({ text: redirectUri }); - const authTypeOptions = useMemo( - () => [ + /** + * Show OBO as a selectable auth option only when the caller has the permission. + * Edit-mode carve-out: if the form was loaded for a server that already uses OBO + * and the caller has since lost the permission, surface OBO in the radio so the + * current auth state is visible — but mark every OBO control disabled (and + * disable the radio itself, so the user can't switch away) since the backend + * also rejects modifying OBO without the permission. + */ + const showOboOption = canConfigureObo || authType === AuthTypeEnum.OBO; + const isOboLockedReadOnly = !canConfigureObo && authType === AuthTypeEnum.OBO; + + const authTypeOptions = useMemo(() => { + const options = [ { value: AuthTypeEnum.None, label: localize('com_ui_no_auth') }, { value: AuthTypeEnum.ServiceHttp, label: localize('com_ui_api_key') }, { value: AuthTypeEnum.OAuth, label: 'OAuth' }, - ], - [localize], - ); + ]; + if (showOboOption) { + options.push({ + value: AuthTypeEnum.OBO, + label: localize('com_ui_obo'), + }); + } + return options; + }, [localize, showOboOption]); const headerFormatOptions = useMemo( () => [ @@ -73,6 +96,7 @@ export default function AuthSection({ isEditMode, serverName }: AuthSectionProps value={authType || AuthTypeEnum.None} onChange={(val) => setValue('auth.auth_type', val as AuthTypeEnum)} fullWidth + disabled={isOboLockedReadOnly} aria-labelledby="auth-type-label" /> @@ -265,6 +289,48 @@ export default function AuthSection({ isEditMode, serverName }: AuthSectionProps )} )} + + {/* OBO Fields */} + {authType === AuthTypeEnum.OBO && ( +
+
+ + + {errors.auth?.obo_scopes && ( +

+ {localize('com_ui_field_required')} +

+ )} + {canConfigureObo ? ( +

+ {localize('com_ui_obo_scopes_description')} +

+ ) : ( +

+ {localize('com_ui_obo_readonly_no_permission')} +

+ )} +
+
+ )} ); } diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 098154c9da..5969eb7535 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1196,6 +1196,7 @@ "com_ui_mcp_server_url_placeholder": "https://mcp.example.com", "com_ui_mcp_servers": "MCP Servers", "com_ui_mcp_servers_allow_create": "Allow users to create MCP servers", + "com_ui_mcp_servers_allow_configure_obo": "Allow users to configure On-Behalf-Of (OBO) on MCP servers", "com_ui_mcp_servers_allow_share": "Allow users to share MCP servers", "com_ui_mcp_servers_allow_share_public": "Allow users to share MCP servers publicly", "com_ui_mcp_servers_allow_use": "Allow users to use MCP servers", @@ -1286,6 +1287,10 @@ "com_ui_none": "None", "com_ui_not_used": "Not Used", "com_ui_nothing_found": "Nothing found", + "com_ui_obo": "On-Behalf-Of (OBO)", + "com_ui_obo_scopes": "OBO Scopes", + "com_ui_obo_scopes_description": "Scopes to request for the downstream MCP server via the On-Behalf-Of flow. Requires OpenID Connect authentication (e.g., Entra ID).", + "com_ui_obo_readonly_no_permission": "OBO is configured for this server but your role no longer has permission to modify it. Contact an administrator to make changes.", "com_ui_oauth": "OAuth", "com_ui_oauth_connected_to": "Connected to", "com_ui_oauth_error_callback_failed": "Authentication callback failed. Please try again.", diff --git a/packages/api/src/app/permissions.spec.ts b/packages/api/src/app/permissions.spec.ts index 01a6b11e42..4cbb59dfe2 100644 --- a/packages/api/src/app/permissions.spec.ts +++ b/packages/api/src/app/permissions.spec.ts @@ -99,6 +99,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: false, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: false, @@ -152,6 +153,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: true, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: true, @@ -269,6 +271,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: false, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: false, @@ -322,6 +325,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: true, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: true, @@ -425,6 +429,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: false, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: false, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: false, @@ -478,6 +483,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: true, [Permissions.SHARE_PUBLIC]: true, + [Permissions.CONFIGURE_OBO]: true, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: true, @@ -594,6 +600,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: false, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: false, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: false, @@ -647,6 +654,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: true, [Permissions.SHARE_PUBLIC]: true, + [Permissions.CONFIGURE_OBO]: true, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: true, @@ -750,6 +758,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: false, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: false, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: false, @@ -803,6 +812,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: true, [Permissions.SHARE_PUBLIC]: true, + [Permissions.CONFIGURE_OBO]: true, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: true, @@ -911,6 +921,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: false, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: false, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: false, @@ -952,6 +963,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: true, [Permissions.SHARE_PUBLIC]: true, + [Permissions.CONFIGURE_OBO]: true, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: true, @@ -1077,6 +1089,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: false, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: false, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: false, @@ -1122,6 +1135,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: true, [Permissions.SHARE_PUBLIC]: true, + [Permissions.CONFIGURE_OBO]: true, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: true, @@ -2180,6 +2194,111 @@ describe('updateInterfacePermissions - permissions', () => { expect(userCall[1][PermissionTypes.MCP_SERVERS]).not.toHaveProperty(Permissions.SHARE); }); + it('should backfill MCP_SERVERS.CONFIGURE_OBO for existing roles (post-OBO permission addition)', async () => { + // Existing deployment: MCP_SERVERS row already present from before CONFIGURE_OBO existed. + // initializeRoles only fills missing permission *types*, not sub-keys, so backfill is needed. + mockGetRoleByName.mockImplementation(async (roleName: string) => { + if (roleName === SystemRoles.USER) { + return { + permissions: { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: false, + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + // CONFIGURE_OBO intentionally absent + }, + }, + }; + } + return { + permissions: { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: true, + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: true, + // CONFIGURE_OBO intentionally absent + }, + }, + }; + }); + + const config = { interface: {} }; + const configDefaults = { interface: {} } as TConfigDefaults; + const interfaceConfig = await loadDefaultInterface({ config, configDefaults }); + const appConfig = { config, interfaceConfig } as unknown as AppConfig; + + await updateInterfacePermissions({ + appConfig, + getRoleByName: mockGetRoleByName, + updateAccessPermissions: mockUpdateAccessPermissions, + }); + + const userCall = mockUpdateAccessPermissions.mock.calls.find( + (call) => call[0] === SystemRoles.USER, + ); + const adminCall = mockUpdateAccessPermissions.mock.calls.find( + (call) => call[0] === SystemRoles.ADMIN, + ); + + expect(userCall?.[1]?.[PermissionTypes.MCP_SERVERS]?.[Permissions.CONFIGURE_OBO]).toBe(false); + expect(adminCall?.[1]?.[PermissionTypes.MCP_SERVERS]?.[Permissions.CONFIGURE_OBO]).toBe(true); + }); + + it('should not overwrite an admin-set CONFIGURE_OBO value during backfill', async () => { + // Operator explicitly set USER.CONFIGURE_OBO=true via the role permissions editor. + mockGetRoleByName.mockImplementation(async (roleName: string) => { + if (roleName === SystemRoles.USER) { + return { + permissions: { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: false, + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: true, + }, + }, + }; + } + return { + permissions: { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.USE]: true, + [Permissions.CREATE]: true, + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: true, + [Permissions.CONFIGURE_OBO]: true, + }, + }, + }; + }); + + const config = { interface: {} }; + const configDefaults = { interface: {} } as TConfigDefaults; + const interfaceConfig = await loadDefaultInterface({ config, configDefaults }); + const appConfig = { config, interfaceConfig } as unknown as AppConfig; + + await updateInterfacePermissions({ + appConfig, + getRoleByName: mockGetRoleByName, + updateAccessPermissions: mockUpdateAccessPermissions, + }); + + const userCall = mockUpdateAccessPermissions.mock.calls.find( + (call) => call[0] === SystemRoles.USER, + ); + + // Either no update was queued, or the queued update does not flip CONFIGURE_OBO. + if (userCall) { + const queuedMcp = userCall[1]?.[PermissionTypes.MCP_SERVERS]; + if (queuedMcp && Permissions.CONFIGURE_OBO in queuedMcp) { + expect(queuedMcp[Permissions.CONFIGURE_OBO]).toBe(true); + } + } + }); + it('should apply explicit remoteAgents config to USER permissions (regression: loadDefaultInterface omission)', async () => { const config = { interface: { @@ -2451,6 +2570,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: false, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: false, }); expect(adminCall[1][PermissionTypes.MCP_SERVERS]).toEqual({ @@ -2458,6 +2578,7 @@ describe('updateInterfacePermissions - permissions', () => { [Permissions.CREATE]: true, [Permissions.SHARE]: true, [Permissions.SHARE_PUBLIC]: true, + [Permissions.CONFIGURE_OBO]: true, }); }); @@ -2490,6 +2611,7 @@ describe('updateInterfacePermissions - permissions', () => { expect(userCall[1][PermissionTypes.MCP_SERVERS]).toEqual({ [Permissions.CREATE]: false, + [Permissions.CONFIGURE_OBO]: false, }); }); @@ -2531,10 +2653,15 @@ describe('updateInterfacePermissions - permissions', () => { expect(userCall[1][PermissionTypes.MCP_SERVERS]).toEqual({ [Permissions.CREATE]: false, + [Permissions.CONFIGURE_OBO]: false, }); expect(userCall[1]).not.toHaveProperty(PermissionTypes.AGENTS); - expect(adminCall[1]).not.toHaveProperty(PermissionTypes.MCP_SERVERS); + // Admin's MCP_SERVERS doesn't migrate CREATE (already true) but does receive + // the CONFIGURE_OBO backfill since the mocked role doc lacked that sub-key. + expect(adminCall[1][PermissionTypes.MCP_SERVERS]).toEqual({ + [Permissions.CONFIGURE_OBO]: true, + }); expect(adminCall[1]).not.toHaveProperty(PermissionTypes.AGENTS); }); diff --git a/packages/api/src/app/permissions.ts b/packages/api/src/app/permissions.ts index 5c5433f427..c72863d95e 100644 --- a/packages/api/src/app/permissions.ts +++ b/packages/api/src/app/permissions.ts @@ -402,6 +402,17 @@ export async function updateInterfacePermissions({ ), } : {}), + ...((typeof interfaceConfig?.mcpServers === 'object' && + 'configureObo' in interfaceConfig.mcpServers) || + !existingPermissions?.[PermissionTypes.MCP_SERVERS] + ? { + [Permissions.CONFIGURE_OBO]: getPermissionValue( + loadedInterface.mcpServers?.configureObo, + defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.CONFIGURE_OBO], + undefined, + ), + } + : {}), }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: getPermissionValue( @@ -621,6 +632,39 @@ export async function updateInterfacePermissions({ } } + /** + * Backfill MCP_SERVERS.CONFIGURE_OBO for existing roles that pre-date the permission. + * The MCP_SERVERS permission type already exists on these role docs, so the + * `addPermissionIfNeeded` block above does not re-seed it. Only fill in the field + * when it is literally absent — never overwrite an admin-set value. + */ + { + const existingMcpPerms = existingPermissions?.[PermissionTypes.MCP_SERVERS]; + const oboExplicit = + typeof interfaceConfig?.mcpServers === 'object' && + 'configureObo' in interfaceConfig.mcpServers; + const alreadyQueued = + permissionsToUpdate[PermissionTypes.MCP_SERVERS]?.[Permissions.CONFIGURE_OBO] !== undefined; + if ( + existingMcpPerms && + existingMcpPerms[Permissions.CONFIGURE_OBO] === undefined && + !oboExplicit && + !alreadyQueued + ) { + const backfillValue = + defaultPerms[PermissionTypes.MCP_SERVERS]?.[Permissions.CONFIGURE_OBO]; + if (backfillValue !== undefined) { + logger.debug( + `Role '${roleName}': Backfilling MCP_SERVERS.CONFIGURE_OBO=${backfillValue}`, + ); + permissionsToUpdate[PermissionTypes.MCP_SERVERS] = { + ...permissionsToUpdate[PermissionTypes.MCP_SERVERS], + [Permissions.CONFIGURE_OBO]: backfillValue, + }; + } + } + } + // Update permissions if any need updating if (Object.keys(permissionsToUpdate).length > 0) { await updateAccessPermissions(roleName, permissionsToUpdate, existingRole); diff --git a/packages/api/src/mcp/ConnectionsRepository.ts b/packages/api/src/mcp/ConnectionsRepository.ts index 2c535af820..a6e3e07a67 100644 --- a/packages/api/src/mcp/ConnectionsRepository.ts +++ b/packages/api/src/mcp/ConnectionsRepository.ts @@ -2,7 +2,7 @@ import { logger } from '@librechat/data-schemas'; import type * as t from './types'; import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; -import { hasCustomUserVars, isUserSourced } from './utils'; +import { isUserSourced, requiresUserScopedConnection } from './utils'; import { MCPConnection } from './connection'; const CONNECT_CONCURRENCY = 3; @@ -147,6 +147,8 @@ export class ConnectionsRepository { * App-level (shared) connections cannot serve servers that need per-user context: * env/header placeholders like `{{MY_KEY}}` are only resolved by `processMCPEnv()` * when real `customUserVars` values exist — which requires a user-level connection. + * OBO servers also require a user-level connection because each tool call + * uses the current user's bearer token. */ private isAllowedToConnectToServer(config: t.ParsedServerConfig) { if (config.inspectionFailed) { @@ -154,7 +156,7 @@ export class ConnectionsRepository { } if ( this.ownerId === undefined && - (config.startup === false || config.requiresOAuth || hasCustomUserVars(config)) + (config.startup === false || requiresUserScopedConnection(config)) ) { return false; } diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index 760325cbba..47474d482f 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -3,9 +3,16 @@ import type { OAuthClientInformation } from '@modelcontextprotocol/sdk/shared/au import type { Tool } from '@modelcontextprotocol/sdk/types.js'; import type { TokenMethods } from '@librechat/data-schemas'; import type { MCPOAuthTokens, OAuthMetadata, MCPOAuthFlowMetadata } from '~/mcp/oauth'; +import type { OboTokenResolver, OboTrustChecker } from '~/mcp/oauth/obo'; import type { FlowStateManager } from '~/flow/manager'; import type * as t from './types'; -import { MCPTokenStorage, MCPOAuthHandler, ReauthenticationRequiredError } from '~/mcp/oauth'; +import { + MCPTokenStorage, + MCPOAuthHandler, + OboTokenResolutionError, + ReauthenticationRequiredError, + resolveOboToken, +} from '~/mcp/oauth'; import { PENDING_STALE_MS, normalizeExpiresAt } from '~/flow/manager'; import { sanitizeUrlForLogging, isClientRejectionMessage, isOAuthServer } from './utils'; import { withTimeout } from '~/utils/promise'; @@ -35,6 +42,7 @@ export class MCPConnectionFactory { // OAuth-related properties (only set when useOAuth is true) protected readonly userId?: string; + protected readonly user?: t.OAuthConnectionOptions['user']; protected readonly flowManager?: FlowStateManager; protected readonly tokenMethods?: TokenMethods; protected readonly signal?: AbortSignal; @@ -42,6 +50,8 @@ export class MCPConnectionFactory { protected readonly oauthEnd?: () => Promise; protected readonly returnOnOAuth?: boolean; protected readonly connectionTimeout?: number; + protected readonly oboTokenResolver?: OboTokenResolver; + protected readonly oboTrustChecker?: OboTrustChecker; /** Creates a new MCP connection with optional OAuth support */ static async create( @@ -73,7 +83,12 @@ export class MCPConnectionFactory { const oauthUrl: string | null = null; let oauthRequired = false; - const oauthTokens = this.useOAuth ? await this.getOAuthTokens() : null; + let oauthTokens: MCPOAuthTokens | null = null; + if (this.usesObo) { + oauthTokens = await this.getOboTokens(); + } else if (this.useOAuth) { + oauthTokens = await this.getOAuthTokens(); + } const connection = new MCPConnection({ serverName: this.serverName, serverConfig: this.serverConfig, @@ -208,6 +223,8 @@ export class MCPConnectionFactory { ? `[MCP][${basic.serverName}][${options.user.id}]` : `[MCP][${basic.serverName}]`; + this.user = options?.user; + if (options != null && 'useOAuth' in options) { this.useOAuth = true; this.userId = options.user?.id; @@ -217,14 +234,78 @@ export class MCPConnectionFactory { this.oauthStart = options.oauthStart; this.oauthEnd = options.oauthEnd; this.returnOnOAuth = options.returnOnOAuth; + this.oboTokenResolver = options.oboTokenResolver; + this.oboTrustChecker = options.oboTrustChecker; } else { this.useOAuth = false; } } + /** Resolves OBO tokens when the server config specifies obo, returns null otherwise */ + protected async getOboTokens(): Promise { + const oboConfig = this.serverConfig.obo; + if (!oboConfig || !this.oboTokenResolver || !this.user) { + return null; + } + + if (this.oboTrustChecker) { + const config = this.serverConfig as t.ParsedServerConfig; + const trusted = await this.oboTrustChecker({ + source: config.source, + author: config.author, + dbId: config.dbId, + }); + if (!trusted) { + logger.warn( + `${this.logPrefix} OBO config not trusted (author lacks CONFIGURE_OBO permission); skipping OBO token exchange`, + ); + return null; + } + } + + logger.info(`${this.logPrefix} Resolving OBO token for scopes: ${oboConfig.scopes}`); + return resolveOboToken(this.user, oboConfig, this.oboTokenResolver); + } + + /** Returns true if this server uses OBO instead of standard OAuth */ + protected get usesObo(): boolean { + return !!this.serverConfig.obo && !!this.oboTokenResolver && !!this.user; + } + + protected createOboConnectionError(error: OboTokenResolutionError): Error { + let recoveryHint = 'Re-authenticate the user and retry.'; + + if (error.retryable) { + recoveryHint = 'Please retry.'; + } else if (error.reason === 'exchange_failed') { + recoveryHint = 'Re-authenticate the user or verify the configured OBO scopes and retry.'; + } + + return new Error( + `${error.userMessage} Unable to connect to OBO server "${this.serverName}". ${recoveryHint}`, + ); + } + /** Creates the base MCP connection with OAuth tokens */ protected async createConnection(): Promise { - const oauthTokens = this.useOAuth ? await this.getOAuthTokens() : null; + let oauthTokens: MCPOAuthTokens | null = null; + + if (this.usesObo) { + try { + oauthTokens = await this.getOboTokens(); + } catch (error) { + if (error instanceof OboTokenResolutionError) { + throw this.createOboConnectionError(error); + } + throw error; + } + if (!oauthTokens) { + throw new Error(`OBO token exchange failed for "${this.serverName}".`); + } + } else if (this.useOAuth) { + oauthTokens = await this.getOAuthTokens(); + } + const connection = new MCPConnection({ serverName: this.serverName, serverConfig: this.serverConfig, @@ -235,7 +316,7 @@ export class MCPConnectionFactory { }); let cleanupOAuthHandlers: (() => void) | null = null; - if (this.useOAuth) { + if (this.useOAuth && !this.usesObo) { cleanupOAuthHandlers = this.handleOAuthEvents(connection); } else { const nonOAuthHandler = () => { diff --git a/packages/api/src/mcp/MCPManager.ts b/packages/api/src/mcp/MCPManager.ts index 7b0127b475..1830375ae5 100644 --- a/packages/api/src/mcp/MCPManager.ts +++ b/packages/api/src/mcp/MCPManager.ts @@ -1,8 +1,10 @@ import pick from 'lodash/pick'; import { logger } from '@librechat/data-schemas'; +import { Permissions, PermissionTypes } from 'librechat-data-provider'; import { CallToolResultSchema, ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import type { RequestOptions } from '@modelcontextprotocol/sdk/shared/protocol.js'; import type { TokenMethods, IUser } from '@librechat/data-schemas'; +import type { OboTokenResolver, OboTrustChecker } from '~/mcp/oauth/obo'; import type { GraphTokenResolver } from '~/utils/graph'; import type { FlowStateManager } from '~/flow/manager'; import type { MCPOAuthTokens } from './oauth'; @@ -14,11 +16,28 @@ import { MCPServersRegistry } from './registry/MCPServersRegistry'; import { UserConnectionManager } from './UserConnectionManager'; import { ConnectionsRepository } from './ConnectionsRepository'; import { MCPConnectionFactory } from './MCPConnectionFactory'; +import { OboTokenResolutionError, resolveOboToken } from '~/mcp/oauth'; import { preProcessGraphTokens } from '~/utils/graph'; import { formatToolContent } from './parsers'; import { MCPConnection } from './connection'; import { processMCPEnv } from '~/utils/env'; -import { isUserSourced, isOAuthServer } from './utils'; +import { isUserSourced, requiresOAuthMachinery, requiresUserScopedConnection } from './utils'; + +function createOboToolCallErrorMessage( + logPrefix: string, + toolName: string, + error: OboTokenResolutionError, +): string { + let failureSuffix = 'Re-authenticate the user and retry.'; + + if (error.retryable) { + failureSuffix = 'Please retry.'; + } else if (error.reason === 'exchange_failed') { + failureSuffix = 'Re-authenticate the user or verify the configured OBO scopes and retry.'; + } + + return `${logPrefix} ${error.userMessage} Cannot execute tool ${toolName}. ${failureSuffix}`; +} /** * Centralized manager for MCP server connections and tool execution. @@ -58,12 +77,29 @@ export class MCPManager extends UserConnectionManager { serverConfig?: t.ParsedServerConfig; } & Omit, ): Promise { + const userId = args.user?.id; + const effectiveConfig = + args.serverConfig ?? + (userId + ? await MCPServersRegistry.getInstance().getServerConfig(args.serverName, userId) + : undefined); + + if (effectiveConfig && userId && requiresUserScopedConnection(effectiveConfig)) { + return this.getUserConnection({ + ...args, + serverConfig: effectiveConfig, + } as Parameters[0]); + } + //the get method checks if the config is still valid as app level const existingAppConnection = await this.appConnections!.get(args.serverName); if (existingAppConnection) { return existingAppConnection; - } else if (args.user?.id) { - return this.getUserConnection(args as Parameters[0]); + } else if (userId) { + return this.getUserConnection({ + ...args, + serverConfig: effectiveConfig, + } as Parameters[0]); } else { throw new McpError( ErrorCode.InvalidRequest, @@ -102,7 +138,7 @@ export class MCPManager extends UserConnectionManager { return { tools: null, oauthRequired: false, oauthUrl: null }; } - const useOAuth = isOAuthServer(serverConfig); + const useOAuth = requiresOAuthMachinery(serverConfig); const registry = MCPServersRegistry.getInstance(); const useSSRFProtection = registry.shouldEnableSSRFProtection(); @@ -147,6 +183,8 @@ export class MCPManager extends UserConnectionManager { customUserVars: args.customUserVars, requestBody: args.requestBody, connectionTimeout: args.connectionTimeout, + oboTokenResolver: args.oboTokenResolver, + oboTrustChecker: args.oboTrustChecker, }); return { tools: result.tools, oauthRequired: result.oauthRequired, oauthUrl: result.oauthUrl }; @@ -274,6 +312,8 @@ Please follow these instructions when using tools from the respective MCP server oauthEnd, customUserVars, graphTokenResolver, + oboTokenResolver, + oboTrustChecker, }: { user?: IUser; serverName: string; @@ -290,6 +330,8 @@ Please follow these instructions when using tools from the respective MCP server oauthStart?: (authURL: string) => Promise; oauthEnd?: () => Promise; graphTokenResolver?: GraphTokenResolver; + oboTokenResolver?: OboTokenResolver; + oboTrustChecker?: OboTrustChecker; }): Promise { /** User-specific connection */ let connection: MCPConnection | undefined; @@ -306,6 +348,8 @@ Please follow these instructions when using tools from the respective MCP server tokenMethods, oauthStart, oauthEnd, + oboTokenResolver, + oboTrustChecker, signal: options?.signal, customUserVars, requestBody, @@ -346,10 +390,53 @@ Please follow these instructions when using tools from the respective MCP server options: graphProcessedConfig, customUserVars, }); - if ('headers' in currentOptions) { - connection.setRequestHeaders(currentOptions.headers || {}); + + const resolvedHeaders: Record = + 'headers' in currentOptions ? { ...(currentOptions.headers || {}) } : {}; + + /** Refresh OBO token on each tool call to ensure it's current */ + const oboConfig = rawConfig.obo; + if (oboConfig && oboTokenResolver && user) { + const oboTrusted = oboTrustChecker + ? await oboTrustChecker({ + source: rawConfig.source, + author: rawConfig.author, + dbId: rawConfig.dbId, + }) + : true; + if (!oboTrusted) { + logger.warn( + `${logPrefix} OBO config not trusted (author lacks ${PermissionTypes.MCP_SERVERS}.${Permissions.CONFIGURE_OBO}); refusing to mint a downstream token`, + ); + throw new McpError( + ErrorCode.InternalError, + `${logPrefix} OBO is not permitted for server "${serverName}". The user who configured it no longer has permission to use OBO.`, + ); + } + let oboTokens: MCPOAuthTokens; + try { + oboTokens = await resolveOboToken(user, oboConfig, oboTokenResolver); + } catch (error) { + if (error instanceof OboTokenResolutionError) { + throw new McpError( + ErrorCode.InternalError, + createOboToolCallErrorMessage(logPrefix, toolName, error), + ); + } + throw error; + } + + if (!oboTokens.access_token) { + throw new McpError( + ErrorCode.InternalError, + `${logPrefix} OBO token refresh failed. Cannot execute tool ${toolName}. Re-authenticate the user and retry.`, + ); + } + resolvedHeaders['Authorization'] = `Bearer ${oboTokens.access_token}`; } + connection.setRequestHeaders(resolvedHeaders); + const result = await connection.client.request( { method: 'tools/call', diff --git a/packages/api/src/mcp/UserConnectionManager.ts b/packages/api/src/mcp/UserConnectionManager.ts index 2a420861c3..a295b4b0b8 100644 --- a/packages/api/src/mcp/UserConnectionManager.ts +++ b/packages/api/src/mcp/UserConnectionManager.ts @@ -4,7 +4,7 @@ import type * as t from './types'; import { MCPServersRegistry } from '~/mcp/registry/MCPServersRegistry'; import { ConnectionsRepository } from '~/mcp/ConnectionsRepository'; import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; -import { isUserSourced, isOAuthServer } from './utils'; +import { isUserSourced, requiresOAuthMachinery } from './utils'; import { MCPConnection } from './connection'; import { mcpConfig } from './mcpConfig'; @@ -78,6 +78,8 @@ export abstract class UserConnectionManager { tokenMethods, oauthStart, oauthEnd, + oboTokenResolver, + oboTrustChecker, signal, returnOnOAuth = false, connectionTimeout, @@ -159,7 +161,7 @@ export abstract class UserConnectionManager { allowedAddresses: registry.getAllowedAddresses(), }; - const useOAuth = isOAuthServer(config); + const useOAuth = requiresOAuthMachinery(config); let connectionOptions: t.OAuthConnectionOptions | t.UserConnectionContext; if (useOAuth) { if (!flowManager) { @@ -178,6 +180,8 @@ export abstract class UserConnectionManager { signal: signal, oauthStart: oauthStart, oauthEnd: oauthEnd, + oboTokenResolver: oboTokenResolver, + oboTrustChecker: oboTrustChecker, returnOnOAuth: returnOnOAuth, requestBody: requestBody, connectionTimeout: connectionTimeout, diff --git a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts index 0a012bb4b5..bd52537778 100644 --- a/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts +++ b/packages/api/src/mcp/__tests__/ConnectionsRepository.test.ts @@ -430,6 +430,18 @@ describe('ConnectionsRepository', () => { expect(await repository.has('customVarStartupServer')).toBe(false); }); + it('should NOT allow connection to OBO servers', async () => { + mockServerConfigs.oboServer = { + type: 'streamable-http', + url: 'http://example.com', + obo: { + scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite', + }, + }; + + expect(await repository.has('oboServer')).toBe(false); + }); + it('should disconnect existing connection when server becomes not allowed', async () => { // Initially setup as regular server mockServerConfigs.changingServer = { @@ -523,6 +535,18 @@ describe('ConnectionsRepository', () => { expect(await repository.has('customVarServer')).toBe(true); }); + it('should allow connection to OBO servers', async () => { + mockServerConfigs.oboServer = { + type: 'streamable-http', + url: 'http://example.com', + obo: { + scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite', + }, + }; + + expect(await repository.has('oboServer')).toBe(true); + }); + it('should return null from get() when server config does not exist', async () => { const connection = await repository.get('nonexistent'); expect(connection).toBeNull(); diff --git a/packages/api/src/mcp/__tests__/MCPManager.test.ts b/packages/api/src/mcp/__tests__/MCPManager.test.ts index cd30a75318..b8792fb8ef 100644 --- a/packages/api/src/mcp/__tests__/MCPManager.test.ts +++ b/packages/api/src/mcp/__tests__/MCPManager.test.ts @@ -8,6 +8,7 @@ import { MCPConnectionFactory } from '~/mcp/MCPConnectionFactory'; import { ConnectionsRepository } from '~/mcp/ConnectionsRepository'; import { MCPConnection } from '~/mcp/connection'; import { MCPManager } from '~/mcp/MCPManager'; +import { OboTokenResolutionError, resolveOboToken } from '~/mcp/oauth'; import * as graphUtils from '~/utils/graph'; // Mock external dependencies @@ -25,6 +26,11 @@ jest.mock('~/utils/graph', () => ({ preProcessGraphTokens: jest.fn(), })); +jest.mock('~/mcp/oauth', () => ({ + ...jest.requireActual('~/mcp/oauth'), + resolveOboToken: jest.fn(), +})); + jest.mock('~/utils/env', () => ({ processMCPEnv: jest.fn((params) => params.options), })); @@ -55,6 +61,7 @@ jest.mock('~/mcp/ConnectionsRepository'); jest.mock('~/mcp/MCPConnectionFactory'); const mockLogger = logger as jest.Mocked; +const mockResolveOboToken = resolveOboToken as jest.MockedFunction; describe('MCPManager', () => { const userId = 'test-user-123'; @@ -793,6 +800,296 @@ describe('MCPManager', () => { }); }); + describe('callTool - OBO Integration', () => { + const mockUser: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + }; + + const mockFlowManager = { + getState: jest.fn(), + setState: jest.fn(), + clearState: jest.fn(), + }; + + const mockConnection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + timeout: 30000, + client: { + request: jest.fn().mockResolvedValue({ + content: [{ type: 'text', text: 'Tool result' }], + isError: false, + }), + }, + } as unknown as MCPConnection; + + const mockOboTokenResolver = jest.fn(); + + const serverConfig: t.SSEOptions & { obo: { scopes: string } } = { + type: 'sse', + url: 'https://api.example.com', + headers: { + Authorization: 'Bearer bootstrap-token', + }, + obo: { + scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite', + }, + }; + + beforeEach(() => { + mockResolveOboToken.mockReset(); + }); + + it('should bypass shared app connections for OBO servers and use a user-scoped connection', async () => { + const sharedAppConnection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + timeout: 30000, + client: { + request: jest.fn().mockResolvedValue({ + content: [{ type: 'text', text: 'Shared tool result' }], + isError: false, + }), + }, + } as unknown as MCPConnection; + + const userConnection = { + isConnected: jest.fn().mockResolvedValue(true), + setRequestHeaders: jest.fn(), + timeout: 30000, + client: { + request: jest.fn().mockResolvedValue({ + content: [{ type: 'text', text: 'User tool result' }], + isError: false, + }), + }, + } as unknown as MCPConnection; + + const appConnections = { + get: jest.fn().mockResolvedValue(sharedAppConnection), + }; + + mockResolveOboToken.mockResolvedValue({ + access_token: 'fresh-obo-token', + token_type: 'Bearer', + obtained_at: Date.now(), + expires_at: Date.now() + 3600_000, + }); + + mockAppConnections(appConnections); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(serverConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const getUserConnectionSpy = jest + .spyOn(manager, 'getUserConnection') + .mockResolvedValue(userConnection); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + oboTokenResolver: mockOboTokenResolver, + }); + + expect(appConnections.get).not.toHaveBeenCalled(); + expect(getUserConnectionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + serverName, + serverConfig, + user: mockUser, + }), + ); + expect(userConnection.setRequestHeaders as jest.Mock).toHaveBeenCalledWith( + expect.objectContaining({ + Authorization: 'Bearer fresh-obo-token', + }), + ); + expect(sharedAppConnection.setRequestHeaders as jest.Mock).not.toHaveBeenCalled(); + expect(userConnection.client.request as jest.Mock).toHaveBeenCalled(); + expect(sharedAppConnection.client.request as jest.Mock).not.toHaveBeenCalled(); + }); + + it('should replace Authorization with the refreshed OBO token on each tool call', async () => { + mockResolveOboToken.mockResolvedValue({ + access_token: 'fresh-obo-token', + token_type: 'Bearer', + obtained_at: Date.now(), + expires_at: Date.now() + 3600_000, + }); + + const appConnections = { + get: jest.fn().mockResolvedValue(mockConnection), + }; + + mockAppConnections(appConnections); + + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(serverConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const getUserConnectionSpy = jest + .spyOn(manager, 'getUserConnection') + .mockResolvedValue(mockConnection); + + await manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + oboTokenResolver: mockOboTokenResolver, + }); + + expect(mockResolveOboToken).toHaveBeenCalledWith( + mockUser, + serverConfig.obo, + mockOboTokenResolver, + ); + expect(appConnections.get).not.toHaveBeenCalled(); + expect(getUserConnectionSpy).toHaveBeenCalled(); + expect(mockConnection.setRequestHeaders).toHaveBeenCalledWith( + expect.objectContaining({ + Authorization: 'Bearer fresh-obo-token', + }), + ); + expect(mockConnection.client.request).toHaveBeenCalled(); + }); + + it('should fail closed with a retryable message when per-call OBO refresh has a transient failure', async () => { + mockResolveOboToken.mockRejectedValue( + new OboTokenResolutionError( + 'exchange_failed', + 'Temporary OBO token exchange failure.', + true, + ), + ); + + const appConnections = { + get: jest.fn().mockResolvedValue(mockConnection), + }; + + mockAppConnections(appConnections); + + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(serverConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const getUserConnectionSpy = jest + .spyOn(manager, 'getUserConnection') + .mockResolvedValue(mockConnection); + + await expect( + manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + oboTokenResolver: mockOboTokenResolver, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('Temporary OBO token exchange failure.'), + }); + + expect(appConnections.get).not.toHaveBeenCalled(); + expect(getUserConnectionSpy).toHaveBeenCalled(); + expect(mockConnection.setRequestHeaders).not.toHaveBeenCalled(); + expect(mockConnection.client.request).not.toHaveBeenCalled(); + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining('[test_tool] Tool call failed'), + expect.anything(), + ); + }); + + it('should fail closed with a re-authentication message when per-call OBO refresh has a permanent failure', async () => { + mockResolveOboToken.mockRejectedValue( + new OboTokenResolutionError( + 'exchange_failed', + 'The identity provider rejected the OBO token exchange.', + ), + ); + + const appConnections = { + get: jest.fn().mockResolvedValue(mockConnection), + }; + + mockAppConnections(appConnections); + + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(serverConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const getUserConnectionSpy = jest + .spyOn(manager, 'getUserConnection') + .mockResolvedValue(mockConnection); + + await expect( + manager.callTool({ + user: mockUser as IUser, + serverName, + toolName: 'test_tool', + provider: 'openai', + flowManager: mockFlowManager as unknown as Parameters< + typeof manager.callTool + >[0]['flowManager'], + oboTokenResolver: mockOboTokenResolver, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('verify the configured OBO scopes'), + }); + + expect(appConnections.get).not.toHaveBeenCalled(); + expect(getUserConnectionSpy).toHaveBeenCalled(); + expect(mockConnection.setRequestHeaders).not.toHaveBeenCalled(); + expect(mockConnection.client.request).not.toHaveBeenCalled(); + }); + }); + + describe('getConnection', () => { + const mockUser: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + }; + + it('should continue using shared app connections for non-OBO servers', async () => { + const appConnection = { + isConnected: jest.fn().mockResolvedValue(true), + } as unknown as MCPConnection; + + const appConnections = { + get: jest.fn().mockResolvedValue(appConnection), + }; + + const nonOboConfig: t.SSEOptions = { + type: 'sse', + url: 'https://api.example.com', + }; + + mockAppConnections(appConnections); + (mockRegistryInstance.getServerConfig as jest.Mock).mockResolvedValue(nonOboConfig); + + const manager = await MCPManager.createInstance(newMCPServersConfig()); + const getUserConnectionSpy = jest.spyOn(manager, 'getUserConnection'); + + const connection = await manager.getConnection({ + serverName, + user: mockUser as IUser, + }); + + expect(connection).toBe(appConnection); + expect(appConnections.get).toHaveBeenCalledWith(serverName); + expect(getUserConnectionSpy).not.toHaveBeenCalled(); + }); + }); + describe('discoverServerTools', () => { const mockTools = [ { name: 'tool1', description: 'First tool', inputSchema: { type: 'object' } }, diff --git a/packages/api/src/mcp/__tests__/utils.test.ts b/packages/api/src/mcp/__tests__/utils.test.ts index ed626cdb45..c40273440d 100644 --- a/packages/api/src/mcp/__tests__/utils.test.ts +++ b/packages/api/src/mcp/__tests__/utils.test.ts @@ -3,6 +3,7 @@ import { normalizeServerName, redactAllServerSecrets, redactServerSecrets, + requiresUserScopedConnection, isInvalidClientMessage, isClientRejectionMessage, getMissingCustomUserVars, @@ -249,6 +250,17 @@ describe('redactServerSecrets', () => { expect((redacted as Record).someNewSensitiveField).toBeUndefined(); expect(redacted.title).toBe('Test'); }); + + it('should preserve obo config', () => { + const config: ParsedServerConfig = { + type: 'sse', + url: 'https://example.com/mcp', + title: 'OBO Server', + obo: { scopes: 'api://client-id/.default' }, + }; + const redacted = redactServerSecrets(config); + expect(redacted.obo).toEqual({ scopes: 'api://client-id/.default' }); + }); }); describe('redactAllServerSecrets', () => { @@ -343,6 +355,39 @@ describe('isUserSourced', () => { }); }); +describe('requiresUserScopedConnection', () => { + it('returns true for OAuth servers', () => { + expect(requiresUserScopedConnection({ requiresOAuth: true })).toBe(true); + }); + + it('returns true for OBO servers', () => { + expect( + requiresUserScopedConnection({ + obo: { scopes: 'api://client-id/.default' }, + }), + ).toBe(true); + }); + + it('returns true for servers with customUserVars', () => { + expect( + requiresUserScopedConnection({ + customUserVars: { + API_KEY: { title: 'API Key', description: 'Your key' }, + }, + }), + ).toBe(true); + }); + + it('returns false for app-shareable servers', () => { + expect( + requiresUserScopedConnection({ + requiresOAuth: false, + customUserVars: {}, + }), + ).toBe(false); + }); +}); + describe('getMissingCustomUserVars', () => { const configWithVars = (keys: string[]): Pick => ({ customUserVars: Object.fromEntries( diff --git a/packages/api/src/mcp/oauth/index.ts b/packages/api/src/mcp/oauth/index.ts index 1c4d98213e..12fd25f56c 100644 --- a/packages/api/src/mcp/oauth/index.ts +++ b/packages/api/src/mcp/oauth/index.ts @@ -3,3 +3,4 @@ export * from './handler'; export * from './tokens'; export * from './detectOAuth'; export * from './methods'; +export * from './obo'; diff --git a/packages/api/src/mcp/oauth/obo.spec.ts b/packages/api/src/mcp/oauth/obo.spec.ts new file mode 100644 index 0000000000..039f13da7d --- /dev/null +++ b/packages/api/src/mcp/oauth/obo.spec.ts @@ -0,0 +1,270 @@ +import type { IUser } from '@librechat/data-schemas'; +import { Permissions, PermissionTypes } from 'librechat-data-provider'; +import type { OboTokenResolver } from './obo'; +import { isOboConfigStillTrusted, resolveOboToken } from './obo'; + +jest.mock('@librechat/data-schemas', () => ({ + logger: { + warn: jest.fn(), + error: jest.fn(), + info: jest.fn(), + }, +})); + +jest.mock('~/utils/oidc', () => ({ + extractOpenIDTokenInfo: jest.fn(), + isOpenIDTokenValid: jest.fn(), +})); + +import { extractOpenIDTokenInfo, isOpenIDTokenValid } from '~/utils/oidc'; + +const mockExtractOpenIDTokenInfo = extractOpenIDTokenInfo as jest.Mock; +const mockIsOpenIDTokenValid = isOpenIDTokenValid as jest.Mock; + +describe('resolveOboToken', () => { + const mockUser: Partial = { + id: 'user-123', + provider: 'openid', + openidId: 'oidc-sub-456', + email: 'test@example.com', + name: 'Test User', + federatedTokens: { + access_token: 'federated-access-token', + id_token: 'federated-id-token', + expires_at: Math.floor(Date.now() / 1000) + 3600, + }, + }; + + const oboConfig = { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }; + + const mockResolver: OboTokenResolver = jest.fn().mockResolvedValue({ + access_token: 'exchanged-mcp-token', + expires_in: 3600, + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should throw when user has no valid OpenID token info', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue(null); + + await expect(resolveOboToken(mockUser as IUser, oboConfig, mockResolver)).rejects.toMatchObject( + { + reason: 'missing_upstream_token', + retryable: false, + }, + ); + expect(mockResolver).not.toHaveBeenCalled(); + }); + + it('should throw when OpenID token is not valid (expired)', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue({ accessToken: 'some-token' }); + mockIsOpenIDTokenValid.mockReturnValue(false); + + await expect(resolveOboToken(mockUser as IUser, oboConfig, mockResolver)).rejects.toMatchObject( + { + reason: 'missing_upstream_token', + retryable: false, + }, + ); + expect(mockResolver).not.toHaveBeenCalled(); + }); + + it('should throw when access token is missing from token info', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue({ userId: 'user-123' }); + mockIsOpenIDTokenValid.mockReturnValue(true); + + await expect(resolveOboToken(mockUser as IUser, oboConfig, mockResolver)).rejects.toMatchObject( + { + reason: 'missing_upstream_access_token', + retryable: false, + }, + ); + expect(mockResolver).not.toHaveBeenCalled(); + }); + + it('should call the resolver with correct arguments and return MCPOAuthTokens', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue({ accessToken: 'federated-access-token' }); + mockIsOpenIDTokenValid.mockReturnValue(true); + + const beforeCall = Date.now(); + const result = await resolveOboToken(mockUser as IUser, oboConfig, mockResolver); + const afterCall = Date.now(); + + expect(mockResolver).toHaveBeenCalledWith( + mockUser, + 'federated-access-token', + 'api://mcp-server-id/Mcp.Tools.ReadWrite', + true, + ); + + expect(result).not.toBeNull(); + expect(result!.access_token).toBe('exchanged-mcp-token'); + expect(result!.token_type).toBe('Bearer'); + expect(result!.obtained_at).toBeGreaterThanOrEqual(beforeCall); + expect(result!.obtained_at).toBeLessThanOrEqual(afterCall); + expect(result!.expires_at).toBe(result!.obtained_at + 3600 * 1000); + }); + + it('should default expires_in to 3600 when not provided by resolver', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue({ accessToken: 'federated-access-token' }); + mockIsOpenIDTokenValid.mockReturnValue(true); + + const resolverNoExpiry: OboTokenResolver = jest.fn().mockResolvedValue({ + access_token: 'exchanged-token', + }); + + const result = await resolveOboToken(mockUser as IUser, oboConfig, resolverNoExpiry); + + expect(result).not.toBeNull(); + expect(result!.expires_at).toBe(result!.obtained_at + 3600 * 1000); + }); + + it('should throw when resolver returns no access_token', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue({ accessToken: 'federated-access-token' }); + mockIsOpenIDTokenValid.mockReturnValue(true); + + const emptyResolver: OboTokenResolver = jest.fn().mockResolvedValue({}); + await expect( + resolveOboToken(mockUser as IUser, oboConfig, emptyResolver), + ).rejects.toMatchObject({ + reason: 'empty_exchange_response', + retryable: false, + }); + }); + + it('should throw a retryable error when resolver reports a transient failure', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue({ accessToken: 'federated-access-token' }); + mockIsOpenIDTokenValid.mockReturnValue(true); + + const failingResolver: OboTokenResolver = jest + .fn() + .mockRejectedValue(Object.assign(new Error('temporary timeout'), { retryable: true })); + + await expect( + resolveOboToken(mockUser as IUser, oboConfig, failingResolver), + ).rejects.toMatchObject({ + reason: 'exchange_failed', + retryable: true, + userMessage: 'Temporary OBO token exchange failure.', + }); + }); + + it('should throw a non-retryable error when resolver reports a permanent failure', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue({ accessToken: 'federated-access-token' }); + mockIsOpenIDTokenValid.mockReturnValue(true); + + const failingResolver: OboTokenResolver = jest + .fn() + .mockRejectedValue(new Error('invalid_grant: assertion invalid')); + + await expect( + resolveOboToken(mockUser as IUser, oboConfig, failingResolver), + ).rejects.toMatchObject({ + reason: 'exchange_failed', + retryable: false, + userMessage: 'The identity provider rejected the OBO token exchange.', + }); + }); + + it('should use the correct scopes from oboConfig', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue({ accessToken: 'federated-access-token' }); + mockIsOpenIDTokenValid.mockReturnValue(true); + + const customConfig = { scopes: 'api://other-app/Custom.Scope' }; + await resolveOboToken(mockUser as IUser, customConfig, mockResolver); + + expect(mockResolver).toHaveBeenCalledWith( + mockUser, + 'federated-access-token', + 'api://other-app/Custom.Scope', + true, + ); + }); + + it('should respect custom expires_in from resolver', async () => { + mockExtractOpenIDTokenInfo.mockReturnValue({ accessToken: 'federated-access-token' }); + mockIsOpenIDTokenValid.mockReturnValue(true); + + const shortLivedResolver: OboTokenResolver = jest.fn().mockResolvedValue({ + access_token: 'short-lived-token', + expires_in: 300, + }); + + const result = await resolveOboToken(mockUser as IUser, oboConfig, shortLivedResolver); + + expect(result).not.toBeNull(); + expect(result!.expires_at).toBe(result!.obtained_at + 300 * 1000); + }); +}); + +describe('isOboConfigStillTrusted', () => { + const adminPerms = { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.CONFIGURE_OBO]: true, + }, + }; + const userPerms = { + [PermissionTypes.MCP_SERVERS]: { + [Permissions.CONFIGURE_OBO]: false, + }, + }; + const noOboPerms = { + [PermissionTypes.MCP_SERVERS]: {}, + }; + + it('returns false when authorId is missing', async () => { + const result = await isOboConfigStillTrusted({ + authorId: undefined, + getUserRoleByAuthorId: jest.fn(), + getRolePermissions: jest.fn(), + }); + expect(result).toBe(false); + }); + + it('returns false when the author has no role (deleted user)', async () => { + const result = await isOboConfigStillTrusted({ + authorId: 'gone', + getUserRoleByAuthorId: jest.fn().mockResolvedValue(null), + getRolePermissions: jest.fn(), + }); + expect(result).toBe(false); + }); + + it('returns false when role lookup throws', async () => { + const result = await isOboConfigStillTrusted({ + authorId: 'u1', + getUserRoleByAuthorId: jest.fn().mockResolvedValue('ADMIN'), + getRolePermissions: jest.fn().mockRejectedValue(new Error('db down')), + }); + expect(result).toBe(false); + }); + + it('returns false when the role lacks CONFIGURE_OBO sub-key (older deployment)', async () => { + const result = await isOboConfigStillTrusted({ + authorId: 'u1', + getUserRoleByAuthorId: jest.fn().mockResolvedValue('ADMIN'), + getRolePermissions: jest.fn().mockResolvedValue(noOboPerms), + }); + expect(result).toBe(false); + }); + + it('returns false when CONFIGURE_OBO is explicitly false', async () => { + const result = await isOboConfigStillTrusted({ + authorId: 'u1', + getUserRoleByAuthorId: jest.fn().mockResolvedValue('USER'), + getRolePermissions: jest.fn().mockResolvedValue(userPerms), + }); + expect(result).toBe(false); + }); + + it('returns true when the author still has CONFIGURE_OBO', async () => { + const result = await isOboConfigStillTrusted({ + authorId: 'u1', + getUserRoleByAuthorId: jest.fn().mockResolvedValue('ADMIN'), + getRolePermissions: jest.fn().mockResolvedValue(adminPerms), + }); + expect(result).toBe(true); + }); +}); diff --git a/packages/api/src/mcp/oauth/obo.ts b/packages/api/src/mcp/oauth/obo.ts new file mode 100644 index 0000000000..91b994a682 --- /dev/null +++ b/packages/api/src/mcp/oauth/obo.ts @@ -0,0 +1,245 @@ +import { logger } from '@librechat/data-schemas'; +import { Permissions, PermissionTypes } from 'librechat-data-provider'; +import type { IUser } from '@librechat/data-schemas'; +import { extractOpenIDTokenInfo, isOpenIDTokenValid } from '~/utils/oidc'; +import type { MCPOAuthTokens } from './types'; + +export interface OboConfig { + scopes: string; +} + +/** + * Function type for performing OBO token exchange. + * Injected from the main API layer since it requires OpenID configuration and caching. + */ +export type OboTokenResolver = ( + user: IUser, + accessToken: string, + scopes: string, + fromCache?: boolean, +) => Promise<{ access_token: string; expires_in?: number }>; + +export type OboTokenResolutionReason = + | 'missing_upstream_token' + | 'missing_upstream_access_token' + | 'empty_exchange_response' + | 'exchange_failed'; + +const RETRYABLE_OBO_STATUS_CODES = new Set([429, 500, 502, 503, 504]); +const RETRYABLE_OBO_ERROR_CODES = new Set(['ETIMEDOUT', 'ECONNRESET', 'EAI_AGAIN', 'ENOTFOUND']); + +function getErrorStatus(error: unknown): number | undefined { + if (!error || typeof error !== 'object') { + return undefined; + } + + const candidate = error as { + status?: number; + statusCode?: number; + response?: { status?: number }; + }; + + return candidate.status ?? candidate.statusCode ?? candidate.response?.status; +} + +function getErrorCode(error: unknown): string | undefined { + if (!error || typeof error !== 'object' || !('code' in error)) { + return undefined; + } + + const code = (error as { code?: unknown }).code; + return typeof code === 'string' ? code.toUpperCase() : undefined; +} + +function getErrorRetryableFlag(error: unknown): boolean | undefined { + if (!error || typeof error !== 'object' || !('retryable' in error)) { + return undefined; + } + + const retryable = (error as { retryable?: unknown }).retryable; + return typeof retryable === 'boolean' ? retryable : undefined; +} + +export class OboTokenResolutionError extends Error { + public readonly reason: OboTokenResolutionReason; + public readonly retryable: boolean; + public readonly userMessage: string; + public override readonly cause?: unknown; + + constructor( + reason: OboTokenResolutionReason, + userMessage: string, + retryable = false, + cause?: unknown, + ) { + super(userMessage); + this.name = 'OboTokenResolutionError'; + this.reason = reason; + this.retryable = retryable; + this.userMessage = userMessage; + this.cause = cause; + } +} + +function isRetryableOboExchangeError(error: unknown): boolean { + const taggedRetryable = getErrorRetryableFlag(error); + if (taggedRetryable != null) { + return taggedRetryable; + } + + const status = getErrorStatus(error); + if (status != null && RETRYABLE_OBO_STATUS_CODES.has(status)) { + return true; + } + + const code = getErrorCode(error); + if (code != null && RETRYABLE_OBO_ERROR_CODES.has(code)) { + return true; + } + + if (!error || typeof error !== 'object' || !('message' in error)) { + return false; + } + + const message = String((error as { message?: unknown }).message ?? '').toLowerCase(); + return ( + message.includes('timed out') || + message.includes('timeout') || + message.includes('econnreset') || + message.includes('socket hang up') || + message.includes('temporarily unavailable') || + message.includes('too many requests') || + message.includes('service unavailable') + ); +} + +/** + * Performs an OBO token exchange for the given user and MCP server OBO config. + * Returns MCPOAuthTokens suitable for injection into the MCP connection. + */ +export async function resolveOboToken( + user: IUser, + oboConfig: OboConfig, + oboTokenResolver: OboTokenResolver, +): Promise { + const tokenInfo = extractOpenIDTokenInfo(user); + if (!tokenInfo || !isOpenIDTokenValid(tokenInfo)) { + logger.warn( + `[OBO] No valid OpenID token available for OBO exchange (provider: ${user.provider}, hasOpenidId: ${!!user.openidId}, hasFederatedTokens: ${!!user.federatedTokens})`, + ); + throw new OboTokenResolutionError( + 'missing_upstream_token', + 'No valid OpenID access token is available for OBO exchange.', + ); + } + + if (!tokenInfo.accessToken) { + logger.warn('[OBO] OpenID token info present but access_token is missing'); + throw new OboTokenResolutionError( + 'missing_upstream_access_token', + 'The upstream OpenID access token is missing for OBO exchange.', + ); + } + + try { + const response = await oboTokenResolver(user, tokenInfo.accessToken, oboConfig.scopes, true); + + if (!response?.access_token) { + logger.warn('[OBO] Token exchange did not return an access token'); + throw new OboTokenResolutionError( + 'empty_exchange_response', + 'The identity provider returned no access token for the OBO exchange.', + ); + } + + const now = Date.now(); + const expiresIn = response.expires_in ?? 3600; + + return { + access_token: response.access_token, + token_type: 'Bearer', + obtained_at: now, + expires_at: now + expiresIn * 1000, + }; + } catch (error) { + if (error instanceof OboTokenResolutionError) { + throw error; + } + + logger.error('[OBO] Failed to exchange token:', error); + const retryable = isRetryableOboExchangeError(error); + throw new OboTokenResolutionError( + 'exchange_failed', + retryable + ? 'Temporary OBO token exchange failure.' + : 'The identity provider rejected the OBO token exchange.', + retryable, + error, + ); + } +} + +/** + * Re-evaluates whether the original author of a DB-stored OBO config still has + * permission to configure OBO. The connection layer calls this before performing + * an OBO token exchange so that retained configs fail closed if the author's role + * is downgraded after the server was created. + * + * Returns true when the author's role grants `MCP_SERVERS.CONFIGURE_OBO`. Any of + * the following degraded states return false (fail closed): + * - missing author id + * - user lookup miss / no role + * - role lookup miss + * - role missing the CONFIGURE_OBO bit + */ +export type GetUserRoleByAuthorId = (authorId: string) => Promise; +export type GetRolePermissions = ( + roleName: string, +) => Promise> | null | undefined>; + +export async function isOboConfigStillTrusted({ + authorId, + getUserRoleByAuthorId, + getRolePermissions, +}: { + authorId: string | undefined; + getUserRoleByAuthorId: GetUserRoleByAuthorId; + getRolePermissions: GetRolePermissions; +}): Promise { + if (!authorId) { + return false; + } + let roleName: string | null | undefined; + try { + roleName = await getUserRoleByAuthorId(authorId); + } catch (err) { + logger.warn('[OBO] Failed to resolve author role for OBO trust check', err); + return false; + } + if (!roleName) { + return false; + } + let permissions: Record> | null | undefined; + try { + permissions = await getRolePermissions(roleName); + } catch (err) { + logger.warn('[OBO] Failed to load role permissions for OBO trust check', err); + return false; + } + return permissions?.[PermissionTypes.MCP_SERVERS]?.[Permissions.CONFIGURE_OBO] === true; +} + +/** + * Async predicate injected into MCP runtime to gate OBO exchanges per server. + * Returns true when OBO is allowed for the given config, false to fail closed. + * + * The runtime passes `source`, `author`, and `dbId` so the implementation can + * use the same `isUserSourced` semantics as the rest of the MCP layer (a + * missing `source` field on a legacy cached config still falls back to + * `dbId`-presence heuristics). + */ +export type OboTrustChecker = (config: { + source?: string; + author?: string; + dbId?: string; +}) => Promise; diff --git a/packages/api/src/mcp/registry/MCPServerInspector.ts b/packages/api/src/mcp/registry/MCPServerInspector.ts index fde1c7d521..2ec4723085 100644 --- a/packages/api/src/mcp/registry/MCPServerInspector.ts +++ b/packages/api/src/mcp/registry/MCPServerInspector.ts @@ -68,7 +68,8 @@ export class MCPServerInspector { if ( this.config.startup !== false && !this.config.requiresOAuth && - !hasCustomUserVars(this.config) + !hasCustomUserVars(this.config) && + !this.config.obo ) { let tempConnection = false; if (!this.connection) { diff --git a/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts b/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts index 2012f82e31..3937d700f0 100644 --- a/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts +++ b/packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.ts @@ -148,6 +148,51 @@ describe('MCPServerInspector', () => { expect(result.toolFunctions).toBeUndefined(); }); + it('should skip capabilities fetch when obo is configured', async () => { + // OBO servers mint per-user delegated tokens at tool-call time; an + // unauthenticated probe at inspection has no valid bearer to attach, + // so the upstream rejects the MCP `initialize` handshake and the + // create/update fails with MCP_INSPECTION_FAILED. Treat `obo` as + // user-scoped auth alongside requiresOAuth and customUserVars. + mockDetectOAuthRequirement.mockResolvedValue({ + requiresOAuth: false, + method: 'no-metadata-found', + }); + + const rawConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://mcp-server.example.com/mcp', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }; + + const result = await MCPServerInspector.inspect('test_server', rawConfig, mockConnection); + + expect(result.obo).toEqual({ scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }); + expect(result.requiresOAuth).toBe(false); + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + expect(mockConnection.disconnect).not.toHaveBeenCalled(); + }); + + it('should NOT create a temp connection when obo is configured and no connection is provided', async () => { + mockDetectOAuthRequirement.mockResolvedValue({ + requiresOAuth: false, + method: 'no-metadata-found', + }); + + const rawConfig: t.MCPOptions = { + type: 'streamable-http', + url: 'https://mcp-server.example.com/mcp', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }; + + const result = await MCPServerInspector.inspect('test_server', rawConfig); + + expect(MCPConnectionFactory.create).not.toHaveBeenCalled(); + expect(result.requiresOAuth).toBe(false); + expect(result.capabilities).toBeUndefined(); + expect(result.toolFunctions).toBeUndefined(); + }); + it('should keep custom serverInstructions string and not fetch from server', async () => { const rawConfig: t.MCPOptions = { type: 'stdio', diff --git a/packages/api/src/mcp/registry/db/ServerConfigsDB.ts b/packages/api/src/mcp/registry/db/ServerConfigsDB.ts index bae25a96aa..8994d6b529 100644 --- a/packages/api/src/mcp/registry/db/ServerConfigsDB.ts +++ b/packages/api/src/mcp/registry/db/ServerConfigsDB.ts @@ -455,11 +455,16 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface { private async mapDBServerToParsedConfig( serverDBDoc: MCPServerDocument, ): Promise { + const authorId = + serverDBDoc.author != null + ? (serverDBDoc.author as unknown as Types.ObjectId | string).toString() + : undefined; const config: ParsedServerConfig = { ...serverDBDoc.config, dbId: (serverDBDoc._id as Types.ObjectId).toString(), source: 'user', updatedAt: serverDBDoc.updatedAt?.getTime(), + ...(authorId ? { author: authorId } : {}), }; return sanitizeUserManagedOAuthConfig(await this.decryptConfig(config)); } diff --git a/packages/api/src/mcp/types/index.ts b/packages/api/src/mcp/types/index.ts index ed85c13fa9..25b9d7f244 100644 --- a/packages/api/src/mcp/types/index.ts +++ b/packages/api/src/mcp/types/index.ts @@ -22,6 +22,7 @@ import type { LCTool } from '@librechat/agents'; import type { FlowStateManager } from '~/flow/manager'; import type { RequestBody } from '~/types/http'; import type * as o from '~/mcp/oauth/types'; +import type { OboTokenResolver, OboTrustChecker } from '~/mcp/oauth/obo'; export type StdioOptions = z.infer; export type WebSocketOptions = z.infer; @@ -168,6 +169,12 @@ export type ParsedServerConfig = MCPOptions & { consumeOnly?: boolean; /** True when inspection failed at startup; the server is known but not fully initialized */ inspectionFailed?: boolean; + /** + * User-id of the creating user (DB-sourced configs only). Used at runtime to gate + * OBO token exchanges by re-checking the author's CONFIGURE_OBO permission, so a + * stored config remains safe if the author's role is downgraded. + */ + author?: string; }; export type AddServerResult = { @@ -202,6 +209,8 @@ export interface OAuthConnectionOptions extends UserConnectionContext { oauthStart?: (authURL: string) => Promise; oauthEnd?: () => Promise; returnOnOAuth?: boolean; + oboTokenResolver?: OboTokenResolver; + oboTrustChecker?: OboTrustChecker; } /** Options accepted by UserConnectionManager.getUserConnection. OAuth fields are optional. */ @@ -215,6 +224,8 @@ export interface UserMCPConnectionOptions extends UserConnectionContext { oauthStart?: (authURL: string) => Promise; oauthEnd?: () => Promise; returnOnOAuth?: boolean; + oboTokenResolver?: OboTokenResolver; + oboTrustChecker?: OboTrustChecker; } export interface ToolDiscoveryOptions { @@ -229,6 +240,8 @@ export interface ToolDiscoveryOptions { connectionTimeout?: number; /** Pre-resolved config-source servers for tenant-scoped lookup */ configServers?: Record; + oboTokenResolver?: OboTokenResolver; + oboTrustChecker?: OboTrustChecker; } export interface ToolDiscoveryResult { diff --git a/packages/api/src/mcp/utils.ts b/packages/api/src/mcp/utils.ts index a92755e1a1..86efcc919c 100644 --- a/packages/api/src/mcp/utils.ts +++ b/packages/api/src/mcp/utils.ts @@ -13,11 +13,38 @@ export function isOAuthServer( return config.requiresOAuth === true || config.oauth != null; } +/** + * Whether a server needs the OAuth-style connection wiring (flow manager, + * token methods, OBO/OAuth resolvers). Distinct from `isOAuthServer`: OBO + * servers reuse the same wiring even though they don't run an OAuth handshake, + * because the runtime needs `oboTokenResolver`/`oboTrustChecker` plumbed through. + * + * Without this, an OBO server with `requiresOAuth: false` would land in the + * non-OAuth branch of MCPManager.discoverServerTools / UserConnectionManager, + * which omits the OBO resolver — `usesObo` then evaluates to false in the + * factory and the connection sends a bare request that the upstream rejects. + */ +export function requiresOAuthMachinery( + config: Pick, +): boolean { + return isOAuthServer(config) || config.obo != null; +} + /** Checks that `customUserVars` is present AND non-empty (guards against truthy `{}`) */ export function hasCustomUserVars(config: Pick): boolean { return !!config.customUserVars && Object.keys(config.customUserVars).length > 0; } +/** + * Returns true when a server requires a per-user connection instead of an + * app-shared connection. + */ +export function requiresUserScopedConnection( + config: Pick, +): boolean { + return config.requiresOAuth === true || config.obo != null || hasCustomUserVars(config); +} + /** * Returns the names of `customUserVars` declared on the server config for which * the user has not supplied a non-blank value (unset, empty, or whitespace-only @@ -94,6 +121,10 @@ export function redactServerSecrets(config: ParsedServerConfig): Partial v !== undefined), ) as Partial; diff --git a/packages/data-provider/specs/mcp.spec.ts b/packages/data-provider/specs/mcp.spec.ts index 54c424c74d..e5b7d6566a 100644 --- a/packages/data-provider/specs/mcp.spec.ts +++ b/packages/data-provider/specs/mcp.spec.ts @@ -3,8 +3,50 @@ import { SSEOptionsSchema, StreamableHTTPOptionsSchema, MCPServerUserInputSchema, + MCP_USER_INPUT_FIELDS, } from '../src/mcp'; +describe('MCPOptionsSchema', () => { + describe('OBO transport support', () => { + it('should accept obo on SSE transport', () => { + const result = MCPOptionsSchema.safeParse({ + type: 'sse', + url: 'https://mcp-server.com/sse', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }); + expect(result.success).toBe(true); + }); + + it('should accept obo on streamable-http transport', () => { + const result = MCPOptionsSchema.safeParse({ + type: 'streamable-http', + url: 'https://mcp-server.com/http', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }); + expect(result.success).toBe(true); + }); + + it('should reject obo on WebSocket transport', () => { + const result = MCPOptionsSchema.safeParse({ + type: 'websocket', + url: 'wss://mcp-server.com/ws', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }); + expect(result.success).toBe(false); + }); + + it('should reject obo on stdio transport', () => { + const result = MCPOptionsSchema.safeParse({ + type: 'stdio', + command: 'node', + args: ['server.js'], + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }); + expect(result.success).toBe(false); + }); + }); +}); + describe('MCP schemas', () => { describe('env variable exfiltration prevention', () => { it('should confirm admin schema resolves env vars (attack vector baseline)', () => { @@ -203,6 +245,69 @@ describe('MCP schemas', () => { }); }); + describe('OBO configuration', () => { + it('should accept obo field with valid scopes', () => { + const result = MCPServerUserInputSchema.safeParse({ + type: 'sse', + url: 'https://mcp-server.com/sse', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.obo).toEqual({ + scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite', + }); + } + }); + + it('should accept obo on streamable-http transport', () => { + const result = MCPServerUserInputSchema.safeParse({ + type: 'streamable-http', + url: 'https://mcp-server.com/http', + obo: { scopes: 'api://other-app/Custom.Scope' }, + }); + expect(result.success).toBe(true); + }); + + it('should reject obo on WebSocket transport', () => { + const result = MCPServerUserInputSchema.safeParse({ + type: 'websocket', + url: 'wss://mcp-server.com/ws', + obo: { scopes: 'api://mcp-server-id/Mcp.Tools.ReadWrite' }, + }); + expect(result.success).toBe(false); + }); + + it('should reject obo with empty scopes', () => { + const result = MCPServerUserInputSchema.safeParse({ + type: 'sse', + url: 'https://mcp-server.com/sse', + obo: { scopes: '' }, + }); + expect(result.success).toBe(false); + }); + + it('should reject obo without scopes property', () => { + const result = MCPServerUserInputSchema.safeParse({ + type: 'sse', + url: 'https://mcp-server.com/sse', + obo: {}, + }); + expect(result.success).toBe(false); + }); + + it('should accept config without obo (optional)', () => { + const result = MCPServerUserInputSchema.safeParse({ + type: 'sse', + url: 'https://mcp-server.com/sse', + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.obo).toBeUndefined(); + } + }); + }); + describe('user-managed OAuth audience restrictions', () => { it('should reject audience from user-managed OAuth configuration', () => { const result = MCPServerUserInputSchema.safeParse({ @@ -437,3 +542,37 @@ describe('MCP schemas', () => { }); }); }); + +describe('MCP_USER_INPUT_FIELDS', () => { + it('includes the expected user-input fields and excludes server-managed ones', () => { + // Sanity check on the schema-derived field set. This is the comparison + // surface for the OBO lockdown check in updateMCPServerController; if it + // drifts unexpectedly, the lockdown could miss a new field. Add new + // entries here when you add new user-input fields to the schema. + expect(MCP_USER_INPUT_FIELDS.has('type')).toBe(true); + expect(MCP_USER_INPUT_FIELDS.has('url')).toBe(true); + expect(MCP_USER_INPUT_FIELDS.has('title')).toBe(true); + expect(MCP_USER_INPUT_FIELDS.has('description')).toBe(true); + expect(MCP_USER_INPUT_FIELDS.has('iconPath')).toBe(true); + expect(MCP_USER_INPUT_FIELDS.has('oauth')).toBe(true); + expect(MCP_USER_INPUT_FIELDS.has('apiKey')).toBe(true); + expect(MCP_USER_INPUT_FIELDS.has('obo')).toBe(true); + expect(MCP_USER_INPUT_FIELDS.has('proxy')).toBe(true); + expect(MCP_USER_INPUT_FIELDS.has('headers')).toBe(true); + + // Server-managed fields should NOT be in this set — they're stripped by + // omitServerManagedFields() before MCPServerUserInputSchema is built. + expect(MCP_USER_INPUT_FIELDS.has('startup')).toBe(false); + expect(MCP_USER_INPUT_FIELDS.has('timeout')).toBe(false); + expect(MCP_USER_INPUT_FIELDS.has('chatMenu')).toBe(false); + expect(MCP_USER_INPUT_FIELDS.has('requiresOAuth')).toBe(false); + expect(MCP_USER_INPUT_FIELDS.has('customUserVars')).toBe(false); + expect(MCP_USER_INPUT_FIELDS.has('oauth_headers')).toBe(false); + + // Stdio is intentionally excluded from MCPServerUserInputSchema (security + // posture), so its transport-only fields should not be in the set either. + expect(MCP_USER_INPUT_FIELDS.has('command')).toBe(false); + expect(MCP_USER_INPUT_FIELDS.has('args')).toBe(false); + expect(MCP_USER_INPUT_FIELDS.has('env')).toBe(false); + }); +}); diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index b027fd266b..999365e8ff 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -901,6 +901,7 @@ const mcpServersSchema = z create: z.boolean().optional(), share: z.boolean().optional(), public: z.boolean().optional(), + configureObo: z.boolean().optional(), trustCheckbox: z .object({ label: localizedStringSchema.optional(), diff --git a/packages/data-provider/src/mcp.ts b/packages/data-provider/src/mcp.ts index 21c3c1df7a..430cd4332b 100644 --- a/packages/data-provider/src/mcp.ts +++ b/packages/data-provider/src/mcp.ts @@ -127,6 +127,11 @@ const UserOAuthOptionsSchema = OAuthOptionsBaseSchema.omit({ }) .superRefine(validateOAuthClientCredentials); +const OboOptionsSchema = z.object({ + /** Scopes to request for the downstream MCP server (e.g., "api:///Mcp.Tools.ReadWrite") */ + scopes: z.string().min(1), +}); + const BaseOptionsSchema = z.object({ /** Display name for the MCP server - only letters, numbers, and spaces allowed */ title: z @@ -224,6 +229,7 @@ const ProxyUrlSchema = z export const StdioOptionsSchema = BaseOptionsSchema.extend({ type: z.literal('stdio').default('stdio'), + obo: z.undefined().optional(), /** * The executable to run to start the server. */ @@ -264,6 +270,7 @@ export const StdioOptionsSchema = BaseOptionsSchema.extend({ export const WebSocketOptionsSchema = BaseOptionsSchema.extend({ type: z.literal('websocket').default('websocket'), + obo: z.undefined().optional(), url: z .string() .transform((val: string) => extractEnvVariable(val)) @@ -282,6 +289,14 @@ export const WebSocketOptionsSchema = BaseOptionsSchema.extend({ export const SSEOptionsSchema = BaseOptionsSchema.extend({ type: z.literal('sse').default('sse'), headers: z.record(z.string(), z.string()).optional(), + /** + * On-Behalf-Of (OBO) token exchange configuration. + * When configured, LibreChat exchanges the logged-in user's federated access token + * for a token scoped to this MCP server via the OAuth 2.0 OBO flow (jwt-bearer grant). + * The exchanged token is injected as a Bearer Authorization header automatically. + * Requires the user to be authenticated via OpenID Connect (e.g., Entra ID). + */ + obo: OboOptionsSchema.optional(), /** Optional outbound proxy URL for this remote MCP transport */ proxy: ProxyUrlSchema.optional(), url: z @@ -302,6 +317,14 @@ export const SSEOptionsSchema = BaseOptionsSchema.extend({ export const StreamableHTTPOptionsSchema = BaseOptionsSchema.extend({ type: z.union([z.literal('streamable-http'), z.literal('http')]), headers: z.record(z.string(), z.string()).optional(), + /** + * On-Behalf-Of (OBO) token exchange configuration. + * When configured, LibreChat exchanges the logged-in user's federated access token + * for a token scoped to this MCP server via the OAuth 2.0 OBO flow (jwt-bearer grant). + * The exchanged token is injected as a Bearer Authorization header automatically. + * Requires the user to be authenticated via OpenID Connect (e.g., Entra ID). + */ + obo: OboOptionsSchema.optional(), /** Optional outbound proxy URL for this remote MCP transport */ proxy: ProxyUrlSchema.optional(), url: z @@ -400,3 +423,28 @@ export const MCPServerUserInputSchema = z.union([ ]); export type MCPServerUserInput = z.infer; + +/** + * Set of every field name that may appear in a user-submitted MCP server config, + * derived from `MCPServerUserInputSchema`'s union members. Used as the comparison + * surface for the OBO lockdown check in `updateMCPServerController` so that + * server-managed fields on the existing config (`dbId`, `source`, `author`, + * `requiresOAuth`, `oauthMetadata`, etc.) don't show up as differences and + * cause spurious 403s on legitimate saves. + * + * Schema-derived rather than hand-maintained: when a new field is added to + * `BaseOptionsSchema` or any transport variant, it flows into this set + * automatically. The OBO lockdown then locks the new field by default + * (since it won't be in the hand-curated `OBO_USER_EDITABLE_FIELDS` + * allowlist), preventing a silent privilege regression. + */ +export const MCP_USER_INPUT_FIELDS: ReadonlySet = (() => { + const fields = new Set(); + for (const variant of MCPServerUserInputSchema.options) { + const shape = (variant as unknown as { shape: Record }).shape; + for (const key of Object.keys(shape)) { + fields.add(key); + } + } + return fields; +})(); diff --git a/packages/data-provider/src/permissions.ts b/packages/data-provider/src/permissions.ts index 3f4590af3a..340f599a49 100644 --- a/packages/data-provider/src/permissions.ts +++ b/packages/data-provider/src/permissions.ts @@ -98,13 +98,14 @@ export const INTERFACE_PERMISSION_FIELDS = new Set(Object.values(PERMISSION_TYPE * DB overrides — other sub-keys (like `placeholder`, `trustCheckbox`) are UI-only and pass through. * * Mapping to Permissions enum: - * 'use' → Permissions.USE (agents, prompts, mcpServers, remoteAgents, marketplace) - * 'create' → Permissions.CREATE (agents, prompts, mcpServers, remoteAgents) - * 'share' → Permissions.SHARE (agents, prompts, mcpServers, remoteAgents) - * 'public' → Permissions.SHARE_PUBLIC (agents, prompts, mcpServers, remoteAgents) - * 'users' → Permissions.VIEW_USERS (peoplePicker only) - * 'groups' → Permissions.VIEW_GROUPS (peoplePicker only) - * 'roles' → Permissions.VIEW_ROLES (peoplePicker only) + * 'use' → Permissions.USE (agents, prompts, mcpServers, remoteAgents, marketplace) + * 'create' → Permissions.CREATE (agents, prompts, mcpServers, remoteAgents) + * 'share' → Permissions.SHARE (agents, prompts, mcpServers, remoteAgents) + * 'public' → Permissions.SHARE_PUBLIC (agents, prompts, mcpServers, remoteAgents) + * 'users' → Permissions.VIEW_USERS (peoplePicker only) + * 'groups' → Permissions.VIEW_GROUPS (peoplePicker only) + * 'roles' → Permissions.VIEW_ROLES (peoplePicker only) + * 'configureObo' → Permissions.CONFIGURE_OBO (mcpServers only) */ export const PERMISSION_SUB_KEYS = new Set([ 'use', @@ -114,6 +115,7 @@ export const PERMISSION_SUB_KEYS = new Set([ 'users', 'groups', 'roles', + 'configureObo', ]); /** @@ -133,6 +135,12 @@ export enum Permissions { VIEW_ROLES = 'VIEW_ROLES', /** Can share resources publicly (with everyone) */ SHARE_PUBLIC = 'SHARE_PUBLIC', + /** + * Can configure MCP server On-Behalf-Of (OBO) token exchange. Gates the + * `obo` field on MCP server configs because OBO silently mints and forwards + * per-user delegated tokens to whatever URL the server points at. + */ + CONFIGURE_OBO = 'CONFIGURE_OBO', } export const promptPermissionsSchema = z.object({ @@ -212,6 +220,7 @@ export const mcpServersPermissionsSchema = z.object({ [Permissions.CREATE]: z.boolean().default(true), [Permissions.SHARE]: z.boolean().default(false), [Permissions.SHARE_PUBLIC]: z.boolean().default(false), + [Permissions.CONFIGURE_OBO]: z.boolean().default(false), }); export type TMcpServersPermissions = z.infer; diff --git a/packages/data-provider/src/roles.spec.ts b/packages/data-provider/src/roles.spec.ts index db94c393f4..6b5603522a 100644 --- a/packages/data-provider/src/roles.spec.ts +++ b/packages/data-provider/src/roles.spec.ts @@ -156,4 +156,20 @@ describe('roleDefaults', () => { }); }); }); + + describe('MCP_SERVERS.CONFIGURE_OBO defaults', () => { + it('grants ADMIN CONFIGURE_OBO by default', () => { + const adminMcp = roleDefaults[SystemRoles.ADMIN].permissions[ + PermissionTypes.MCP_SERVERS + ] as Record; + expect(adminMcp[Permissions.CONFIGURE_OBO]).toBe(true); + }); + + it('does not grant CONFIGURE_OBO to USER by default — gates the OBO config layer', () => { + const userMcp = roleDefaults[SystemRoles.USER].permissions[ + PermissionTypes.MCP_SERVERS + ] as Record; + expect(userMcp[Permissions.CONFIGURE_OBO]).toBe(false); + }); + }); }); diff --git a/packages/data-provider/src/roles.ts b/packages/data-provider/src/roles.ts index 600c017402..6d5f997808 100644 --- a/packages/data-provider/src/roles.ts +++ b/packages/data-provider/src/roles.ts @@ -97,6 +97,7 @@ const defaultRolesSchema = z.object({ [Permissions.CREATE]: z.boolean().default(true), [Permissions.SHARE]: z.boolean().default(true), [Permissions.SHARE_PUBLIC]: z.boolean().default(true), + [Permissions.CONFIGURE_OBO]: z.boolean().default(true), }), [PermissionTypes.REMOTE_AGENTS]: remoteAgentsPermissionsSchema.extend({ [Permissions.USE]: z.boolean().default(true), @@ -185,6 +186,7 @@ export const roleDefaults = defaultRolesSchema.parse({ [Permissions.CREATE]: true, [Permissions.SHARE]: true, [Permissions.SHARE_PUBLIC]: true, + [Permissions.CONFIGURE_OBO]: true, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: true, @@ -236,6 +238,7 @@ export const roleDefaults = defaultRolesSchema.parse({ [Permissions.CREATE]: false, [Permissions.SHARE]: false, [Permissions.SHARE_PUBLIC]: false, + [Permissions.CONFIGURE_OBO]: false, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: false, diff --git a/packages/data-schemas/src/schema/role.ts b/packages/data-schemas/src/schema/role.ts index ab1fc582e9..ed2ae58379 100644 --- a/packages/data-schemas/src/schema/role.ts +++ b/packages/data-schemas/src/schema/role.ts @@ -60,6 +60,7 @@ const rolePermissionsSchema = new Schema( [Permissions.CREATE]: { type: Boolean }, [Permissions.SHARE]: { type: Boolean }, [Permissions.SHARE_PUBLIC]: { type: Boolean }, + [Permissions.CONFIGURE_OBO]: { type: Boolean }, }, [PermissionTypes.REMOTE_AGENTS]: { [Permissions.USE]: { type: Boolean },