mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🧹 fix: Clear MCP OAuth Tokens On Revoke
Fixes #12912.\n\n- Clear stored MCP OAuth tokens and flow state on revoke cleanup-only paths.\n- Keep provider revocation best-effort when token and client metadata are available.\n- Add controller and function coverage for stale metadata, missing config, and cleanup failure paths.
This commit is contained in:
parent
eb22bb6969
commit
4e45e8e17c
3 changed files with 530 additions and 62 deletions
|
|
@ -7,7 +7,6 @@ const {
|
|||
MCPTokenStorage,
|
||||
normalizeHttpError,
|
||||
extractWebSearchEnvVars,
|
||||
ReauthenticationRequiredError,
|
||||
} = require('@librechat/api');
|
||||
const {
|
||||
Tools,
|
||||
|
|
@ -376,9 +375,48 @@ const resendVerificationController = async (req, res) => {
|
|||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* OAuth MCP specific uninstall logic
|
||||
*/
|
||||
/** Best-effort cleanup of stored MCP OAuth tokens and flow state. */
|
||||
const clearStoredMCPOAuthState = async (userId, serverName) => {
|
||||
try {
|
||||
await MCPTokenStorage.deleteUserTokens({
|
||||
userId,
|
||||
serverName,
|
||||
deleteToken: async (filter) => {
|
||||
await db.deleteTokens(filter);
|
||||
},
|
||||
});
|
||||
} catch (error) {
|
||||
logger.warn(
|
||||
`[clearStoredMCPOAuthState] Failed to delete MCP OAuth tokens for ${serverName}:`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
|
||||
try {
|
||||
const flowsCache = getLogStores(CacheKeys.FLOWS);
|
||||
const flowManager = getFlowStateManager(flowsCache);
|
||||
const flowId = MCPOAuthHandler.generateFlowId(userId, serverName);
|
||||
const results = await Promise.allSettled([
|
||||
flowManager.deleteFlow(flowId, 'mcp_get_tokens'),
|
||||
flowManager.deleteFlow(flowId, 'mcp_oauth'),
|
||||
]);
|
||||
for (const result of results) {
|
||||
if (result.status === 'rejected') {
|
||||
logger.warn(
|
||||
`[clearStoredMCPOAuthState] Failed to clear MCP OAuth flow state for ${serverName}:`,
|
||||
result.reason,
|
||||
);
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
logger.warn(
|
||||
`[clearStoredMCPOAuthState] Failed to clear MCP OAuth flow state for ${serverName}:`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
/** Revokes MCP OAuth tokens at the provider when possible, then clears local state. */
|
||||
const maybeUninstallOAuthMCP = async (userId, pluginKey, appConfig) => {
|
||||
if (!pluginKey.startsWith(Constants.mcp_prefix)) {
|
||||
// this is not an MCP server, so nothing to do here
|
||||
|
|
@ -390,29 +428,37 @@ const maybeUninstallOAuthMCP = async (userId, pluginKey, appConfig) => {
|
|||
(await getMCPServersRegistry().getServerConfig(serverName, userId)) ??
|
||||
appConfig?.mcpServers?.[serverName];
|
||||
const oauthServers = await getMCPServersRegistry().getOAuthServers(userId);
|
||||
if (!oauthServers.has(serverName)) {
|
||||
// this server does not use OAuth, so nothing to do here as well
|
||||
if (!oauthServers.has(serverName) || !serverConfig) {
|
||||
await clearStoredMCPOAuthState(userId, serverName);
|
||||
return;
|
||||
}
|
||||
|
||||
// 1. get client info used for revocation (client id, secret)
|
||||
const clientTokenData = await MCPTokenStorage.getClientInfoAndMetadata({
|
||||
userId,
|
||||
serverName,
|
||||
findToken: db.findToken,
|
||||
});
|
||||
let clientTokenData = null;
|
||||
try {
|
||||
clientTokenData = await MCPTokenStorage.getClientInfoAndMetadata({
|
||||
userId,
|
||||
serverName,
|
||||
findToken: db.findToken,
|
||||
});
|
||||
} catch (error) {
|
||||
logger.warn(
|
||||
`[maybeUninstallOAuthMCP] Unable to load OAuth client metadata for ${serverName}; clearing local MCP OAuth state only.`,
|
||||
error,
|
||||
);
|
||||
await clearStoredMCPOAuthState(userId, serverName);
|
||||
return;
|
||||
}
|
||||
if (clientTokenData == null) {
|
||||
logger.info(
|
||||
`[maybeUninstallOAuthMCP] Missing OAuth client metadata for ${serverName}; clearing local MCP OAuth state only.`,
|
||||
);
|
||||
await clearStoredMCPOAuthState(userId, serverName);
|
||||
return;
|
||||
}
|
||||
const { clientInfo, clientMetadata } = clientTokenData;
|
||||
|
||||
// 2. get decrypted tokens before deletion.
|
||||
// Token retrieval can throw ReauthenticationRequiredError (or other
|
||||
// errors) when the refresh token is missing/expired — exactly the
|
||||
// state that triggers a user-initiated revoke. Swallow it here so
|
||||
// the DB and flow-state cleanup below always runs. Revocation is
|
||||
// best-effort and the individual calls already wrap their own
|
||||
// try/catch.
|
||||
// 2. get decrypted tokens before deletion
|
||||
let tokens = null;
|
||||
try {
|
||||
tokens = await MCPTokenStorage.getTokens({
|
||||
|
|
@ -421,16 +467,10 @@ const maybeUninstallOAuthMCP = async (userId, pluginKey, appConfig) => {
|
|||
findToken: db.findToken,
|
||||
});
|
||||
} catch (error) {
|
||||
if (error instanceof ReauthenticationRequiredError) {
|
||||
logger.info(
|
||||
`[maybeUninstallOAuthMCP] No usable tokens for ${serverName} — skipping revocation, continuing cleanup`,
|
||||
);
|
||||
} else {
|
||||
logger.warn(
|
||||
`[maybeUninstallOAuthMCP] Unexpected error retrieving tokens for ${serverName}:`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
logger.warn(
|
||||
`[maybeUninstallOAuthMCP] Unable to load OAuth tokens for ${serverName}; clearing local token state.`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
|
||||
// 3. revoke OAuth tokens at the provider
|
||||
|
|
@ -459,7 +499,10 @@ const maybeUninstallOAuthMCP = async (userId, pluginKey, appConfig) => {
|
|||
allowedDomains,
|
||||
);
|
||||
} catch (error) {
|
||||
logger.error(`Error revoking OAuth access token for ${serverName}:`, error);
|
||||
logger.error(
|
||||
`[maybeUninstallOAuthMCP] Error revoking OAuth access token for ${serverName}:`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -480,25 +523,15 @@ const maybeUninstallOAuthMCP = async (userId, pluginKey, appConfig) => {
|
|||
allowedDomains,
|
||||
);
|
||||
} catch (error) {
|
||||
logger.error(`Error revoking OAuth refresh token for ${serverName}:`, error);
|
||||
logger.error(
|
||||
`[maybeUninstallOAuthMCP] Error revoking OAuth refresh token for ${serverName}:`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// 4. delete tokens from the DB after revocation attempts
|
||||
await MCPTokenStorage.deleteUserTokens({
|
||||
userId,
|
||||
serverName,
|
||||
deleteToken: async (filter) => {
|
||||
await db.deleteTokens(filter);
|
||||
},
|
||||
});
|
||||
|
||||
// 5. clear the flow state for the OAuth tokens
|
||||
const flowsCache = getLogStores(CacheKeys.FLOWS);
|
||||
const flowManager = getFlowStateManager(flowsCache);
|
||||
const flowId = MCPOAuthHandler.generateFlowId(userId, serverName);
|
||||
await flowManager.deleteFlow(flowId, 'mcp_get_tokens');
|
||||
await flowManager.deleteFlow(flowId, 'mcp_oauth');
|
||||
// 4. delete tokens from the DB and clear the flow state after revocation attempts
|
||||
await clearStoredMCPOAuthState(userId, serverName);
|
||||
};
|
||||
|
||||
module.exports = {
|
||||
|
|
|
|||
419
api/server/controllers/__tests__/UserController.mcpOAuth.spec.js
Normal file
419
api/server/controllers/__tests__/UserController.mcpOAuth.spec.js
Normal file
|
|
@ -0,0 +1,419 @@
|
|||
const mockUpdateUserPlugins = jest.fn();
|
||||
const mockFindToken = jest.fn();
|
||||
const mockDeleteUserPluginAuth = jest.fn();
|
||||
const mockGetAppConfig = jest.fn();
|
||||
const mockInvalidateCachedTools = jest.fn();
|
||||
const mockGetLogStores = jest.fn();
|
||||
const mockGetMCPManager = jest.fn();
|
||||
const mockGetFlowStateManager = jest.fn();
|
||||
const mockGetMCPServersRegistry = jest.fn();
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
logger: { error: jest.fn(), info: jest.fn(), warn: jest.fn() },
|
||||
webSearchKeys: [],
|
||||
}));
|
||||
|
||||
jest.mock('librechat-data-provider', () => ({
|
||||
Tools: {},
|
||||
CacheKeys: { FLOWS: 'flows' },
|
||||
Constants: { mcp_delimiter: '_mcp_', mcp_prefix: 'mcp_' },
|
||||
FileSources: {},
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/api', () => ({
|
||||
MCPOAuthHandler: {
|
||||
generateFlowId: jest.fn(() => 'user-1:test-server'),
|
||||
revokeOAuthToken: jest.fn(),
|
||||
},
|
||||
MCPTokenStorage: {
|
||||
getClientInfoAndMetadata: jest.fn(),
|
||||
getTokens: jest.fn(),
|
||||
deleteUserTokens: jest.fn().mockResolvedValue(undefined),
|
||||
},
|
||||
normalizeHttpError: jest.fn((error) => error),
|
||||
extractWebSearchEnvVars: jest.fn((params) => params.keys),
|
||||
needsRefresh: jest.fn(),
|
||||
getNewS3URL: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/models', () => ({
|
||||
updateUserPlugins: (...args) => mockUpdateUserPlugins(...args),
|
||||
findToken: mockFindToken,
|
||||
deleteTokens: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/PluginService', () => ({
|
||||
updateUserPluginAuth: jest.fn(),
|
||||
deleteUserPluginAuth: (...args) => mockDeleteUserPluginAuth(...args),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/twoFactorService', () => ({
|
||||
verifyOTPOrBackupCode: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/AuthService', () => ({
|
||||
verifyEmail: jest.fn(),
|
||||
resendVerificationEmail: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/config', () => ({
|
||||
getMCPManager: (...args) => mockGetMCPManager(...args),
|
||||
getFlowStateManager: (...args) => mockGetFlowStateManager(...args),
|
||||
getMCPServersRegistry: (...args) => mockGetMCPServersRegistry(...args),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Config/getCachedTools', () => ({
|
||||
invalidateCachedTools: (...args) => mockInvalidateCachedTools(...args),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Files/process', () => ({
|
||||
processDeleteRequest: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Config', () => ({
|
||||
getAppConfig: (...args) => mockGetAppConfig(...args),
|
||||
}));
|
||||
|
||||
jest.mock('~/cache', () => ({
|
||||
getLogStores: (...args) => mockGetLogStores(...args),
|
||||
}));
|
||||
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { MCPTokenStorage, MCPOAuthHandler } = require('@librechat/api');
|
||||
const { updateUserPluginsController } = require('~/server/controllers/UserController');
|
||||
|
||||
function createResponse() {
|
||||
const res = {};
|
||||
res.status = jest.fn().mockReturnValue(res);
|
||||
res.json = jest.fn().mockReturnValue(res);
|
||||
res.send = jest.fn().mockReturnValue(res);
|
||||
return res;
|
||||
}
|
||||
|
||||
function createRequest() {
|
||||
return {
|
||||
user: {
|
||||
id: 'user-1',
|
||||
_id: 'user-1',
|
||||
plugins: [],
|
||||
role: 'USER',
|
||||
},
|
||||
body: {
|
||||
pluginKey: 'mcp_test-server',
|
||||
action: 'uninstall',
|
||||
auth: {},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function setupMCPMocks() {
|
||||
const flowManager = {
|
||||
deleteFlow: jest.fn().mockResolvedValue(true),
|
||||
};
|
||||
const mcpManager = {
|
||||
disconnectUserConnection: jest.fn().mockResolvedValue(),
|
||||
};
|
||||
const registry = {
|
||||
getServerConfig: jest.fn().mockResolvedValue({
|
||||
url: 'https://example.com/mcp',
|
||||
oauth: {},
|
||||
oauth_headers: {},
|
||||
}),
|
||||
getOAuthServers: jest.fn().mockResolvedValue(new Set(['test-server'])),
|
||||
getAllowedDomains: jest.fn().mockReturnValue([]),
|
||||
};
|
||||
|
||||
mockGetAppConfig.mockResolvedValue({});
|
||||
mockUpdateUserPlugins.mockResolvedValue();
|
||||
mockDeleteUserPluginAuth.mockResolvedValue();
|
||||
mockInvalidateCachedTools.mockResolvedValue();
|
||||
mockGetLogStores.mockReturnValue({});
|
||||
mockGetFlowStateManager.mockReturnValue(flowManager);
|
||||
mockGetMCPManager.mockReturnValue(mcpManager);
|
||||
mockGetMCPServersRegistry.mockReturnValue(registry);
|
||||
|
||||
return { flowManager, mcpManager, registry };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('updateUserPluginsController MCP OAuth cleanup', () => {
|
||||
it('clears stored OAuth token state when client metadata is missing', async () => {
|
||||
const { flowManager, mcpManager } = setupMCPMocks();
|
||||
MCPTokenStorage.getClientInfoAndMetadata.mockResolvedValue(null);
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(MCPTokenStorage.getClientInfoAndMetadata).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
findToken: mockFindToken,
|
||||
});
|
||||
expect(MCPTokenStorage.deleteUserTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
deleteToken: expect.any(Function),
|
||||
});
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_get_tokens');
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_oauth');
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).not.toHaveBeenCalled();
|
||||
expect(mcpManager.disconnectUserConnection).toHaveBeenCalledWith('user-1', 'test-server');
|
||||
});
|
||||
|
||||
it('still clears OAuth flow state when stored token deletion fails', async () => {
|
||||
const { flowManager } = setupMCPMocks();
|
||||
const cleanupError = new Error('DB down');
|
||||
MCPTokenStorage.getClientInfoAndMetadata.mockResolvedValue(null);
|
||||
MCPTokenStorage.deleteUserTokens.mockRejectedValueOnce(cleanupError);
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_get_tokens');
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_oauth');
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
'[clearStoredMCPOAuthState] Failed to delete MCP OAuth tokens for test-server:',
|
||||
cleanupError,
|
||||
);
|
||||
});
|
||||
|
||||
it('logs all flow cleanup failures without failing MCP OAuth cleanup', async () => {
|
||||
const { flowManager } = setupMCPMocks();
|
||||
const getTokensFlowError = new Error('get tokens flow cache down');
|
||||
const oauthFlowError = new Error('oauth flow cache down');
|
||||
MCPTokenStorage.getClientInfoAndMetadata.mockResolvedValue(null);
|
||||
flowManager.deleteFlow
|
||||
.mockRejectedValueOnce(getTokensFlowError)
|
||||
.mockRejectedValueOnce(oauthFlowError);
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_get_tokens');
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_oauth');
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
'[clearStoredMCPOAuthState] Failed to clear MCP OAuth flow state for test-server:',
|
||||
getTokensFlowError,
|
||||
);
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
'[clearStoredMCPOAuthState] Failed to clear MCP OAuth flow state for test-server:',
|
||||
oauthFlowError,
|
||||
);
|
||||
});
|
||||
|
||||
it('clears stored OAuth token state when client metadata cannot be loaded', async () => {
|
||||
const { flowManager } = setupMCPMocks();
|
||||
MCPTokenStorage.getClientInfoAndMetadata.mockRejectedValue(new Error('invalid client info'));
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
'[maybeUninstallOAuthMCP] Unable to load OAuth client metadata for test-server; clearing local MCP OAuth state only.',
|
||||
expect.any(Error),
|
||||
);
|
||||
expect(MCPTokenStorage.deleteUserTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
deleteToken: expect.any(Function),
|
||||
});
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_get_tokens');
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_oauth');
|
||||
expect(MCPTokenStorage.getTokens).not.toHaveBeenCalled();
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('clears stored OAuth token state when server config is missing', async () => {
|
||||
const { flowManager, registry } = setupMCPMocks();
|
||||
registry.getServerConfig.mockResolvedValue(undefined);
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(MCPTokenStorage.deleteUserTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
deleteToken: expect.any(Function),
|
||||
});
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_get_tokens');
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_oauth');
|
||||
expect(MCPTokenStorage.getClientInfoAndMetadata).not.toHaveBeenCalled();
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('clears stored OAuth token state when server no longer requires OAuth', async () => {
|
||||
const { flowManager, registry } = setupMCPMocks();
|
||||
registry.getOAuthServers.mockResolvedValue(new Set());
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(MCPTokenStorage.deleteUserTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
deleteToken: expect.any(Function),
|
||||
});
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_get_tokens');
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_oauth');
|
||||
expect(MCPTokenStorage.getClientInfoAndMetadata).not.toHaveBeenCalled();
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('clears stored OAuth token state when token loading fails before provider revocation', async () => {
|
||||
const { flowManager } = setupMCPMocks();
|
||||
MCPTokenStorage.getClientInfoAndMetadata.mockResolvedValue({
|
||||
clientInfo: { client_id: 'client-1' },
|
||||
clientMetadata: {},
|
||||
});
|
||||
MCPTokenStorage.getTokens.mockRejectedValue(new Error('token lookup failed'));
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(MCPTokenStorage.getTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
findToken: mockFindToken,
|
||||
});
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
'[maybeUninstallOAuthMCP] Unable to load OAuth tokens for test-server; clearing local token state.',
|
||||
expect.any(Error),
|
||||
);
|
||||
expect(MCPTokenStorage.deleteUserTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
deleteToken: expect.any(Function),
|
||||
});
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_get_tokens');
|
||||
expect(flowManager.deleteFlow).toHaveBeenCalledWith('user-1:test-server', 'mcp_oauth');
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('revokes provider tokens before clearing local token state when token data is available', async () => {
|
||||
setupMCPMocks();
|
||||
MCPTokenStorage.getClientInfoAndMetadata.mockResolvedValue({
|
||||
clientInfo: { client_id: 'client-1', client_secret: 'secret-1' },
|
||||
clientMetadata: { revocation_endpoint: 'https://example.com/revoke' },
|
||||
});
|
||||
MCPTokenStorage.getTokens.mockResolvedValue({
|
||||
access_token: 'access-token',
|
||||
refresh_token: 'refresh-token',
|
||||
});
|
||||
MCPOAuthHandler.revokeOAuthToken.mockResolvedValue();
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(MCPTokenStorage.getTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
findToken: mockFindToken,
|
||||
});
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).toHaveBeenCalledWith(
|
||||
'test-server',
|
||||
'access-token',
|
||||
'access',
|
||||
{
|
||||
serverUrl: 'https://example.com/mcp',
|
||||
clientId: 'client-1',
|
||||
clientSecret: 'secret-1',
|
||||
revocationEndpoint: 'https://example.com/revoke',
|
||||
revocationEndpointAuthMethodsSupported: undefined,
|
||||
},
|
||||
{},
|
||||
[],
|
||||
);
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).toHaveBeenCalledWith(
|
||||
'test-server',
|
||||
'refresh-token',
|
||||
'refresh',
|
||||
{
|
||||
serverUrl: 'https://example.com/mcp',
|
||||
clientId: 'client-1',
|
||||
clientSecret: 'secret-1',
|
||||
revocationEndpoint: 'https://example.com/revoke',
|
||||
revocationEndpointAuthMethodsSupported: undefined,
|
||||
},
|
||||
{},
|
||||
[],
|
||||
);
|
||||
expect(MCPTokenStorage.deleteUserTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
deleteToken: expect.any(Function),
|
||||
});
|
||||
});
|
||||
|
||||
it('revokes only the access token when refresh token data is absent', async () => {
|
||||
setupMCPMocks();
|
||||
MCPTokenStorage.getClientInfoAndMetadata.mockResolvedValue({
|
||||
clientInfo: { client_id: 'client-1', client_secret: 'secret-1' },
|
||||
clientMetadata: {},
|
||||
});
|
||||
MCPTokenStorage.getTokens.mockResolvedValue({
|
||||
access_token: 'access-token',
|
||||
});
|
||||
MCPOAuthHandler.revokeOAuthToken.mockResolvedValue();
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).toHaveBeenCalledTimes(1);
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).toHaveBeenCalledWith(
|
||||
'test-server',
|
||||
'access-token',
|
||||
'access',
|
||||
expect.objectContaining({ clientId: 'client-1' }),
|
||||
{},
|
||||
[],
|
||||
);
|
||||
expect(MCPTokenStorage.deleteUserTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
deleteToken: expect.any(Function),
|
||||
});
|
||||
});
|
||||
|
||||
it('revokes only the refresh token when access token data is absent', async () => {
|
||||
setupMCPMocks();
|
||||
MCPTokenStorage.getClientInfoAndMetadata.mockResolvedValue({
|
||||
clientInfo: { client_id: 'client-1', client_secret: 'secret-1' },
|
||||
clientMetadata: {},
|
||||
});
|
||||
MCPTokenStorage.getTokens.mockResolvedValue({
|
||||
refresh_token: 'refresh-token',
|
||||
});
|
||||
MCPOAuthHandler.revokeOAuthToken.mockResolvedValue();
|
||||
|
||||
const res = createResponse();
|
||||
await updateUserPluginsController(createRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).toHaveBeenCalledTimes(1);
|
||||
expect(MCPOAuthHandler.revokeOAuthToken).toHaveBeenCalledWith(
|
||||
'test-server',
|
||||
'refresh-token',
|
||||
'refresh',
|
||||
expect.objectContaining({ clientId: 'client-1' }),
|
||||
{},
|
||||
[],
|
||||
);
|
||||
expect(MCPTokenStorage.deleteUserTokens).toHaveBeenCalledWith({
|
||||
userId: 'user-1',
|
||||
serverName: 'test-server',
|
||||
deleteToken: expect.any(Function),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -19,12 +19,6 @@ jest.mock('@librechat/data-schemas', () => ({
|
|||
}));
|
||||
|
||||
jest.mock('@librechat/api', () => {
|
||||
class ReauthenticationRequiredError extends Error {
|
||||
constructor(serverName, reason) {
|
||||
super(`Re-authentication required for "${serverName}": ${reason}`);
|
||||
this.name = 'ReauthenticationRequiredError';
|
||||
}
|
||||
}
|
||||
return {
|
||||
MCPOAuthHandler: {
|
||||
revokeOAuthToken: (...args) => mockRevokeOAuthToken(...args),
|
||||
|
|
@ -35,7 +29,6 @@ jest.mock('@librechat/api', () => {
|
|||
getClientInfoAndMetadata: (...args) => mockGetClientInfoAndMetadata(...args),
|
||||
deleteUserTokens: (...args) => mockDeleteUserTokens(...args),
|
||||
},
|
||||
ReauthenticationRequiredError,
|
||||
normalizeHttpError: jest.fn(),
|
||||
extractWebSearchEnvVars: jest.fn(),
|
||||
needsRefresh: jest.fn(),
|
||||
|
|
@ -124,7 +117,6 @@ jest.mock('~/models', () => ({
|
|||
}));
|
||||
|
||||
const { maybeUninstallOAuthMCP } = require('~/server/controllers/UserController');
|
||||
const { ReauthenticationRequiredError } = require('@librechat/api');
|
||||
|
||||
const userId = 'user-123';
|
||||
const pluginKey = 'mcp_acme';
|
||||
|
|
@ -167,7 +159,7 @@ describe('maybeUninstallOAuthMCP', () => {
|
|||
expect(mockDeleteFlow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('is a no-op when the MCP server is not an OAuth server', async () => {
|
||||
test('clears stored state when the MCP server is not an OAuth server', async () => {
|
||||
mockGetServerConfig.mockResolvedValue(serverConfig);
|
||||
mockGetOAuthServers.mockResolvedValue(new Set(['other']));
|
||||
|
||||
|
|
@ -175,18 +167,39 @@ describe('maybeUninstallOAuthMCP', () => {
|
|||
|
||||
expect(mockGetClientInfoAndMetadata).not.toHaveBeenCalled();
|
||||
expect(mockGetTokens).not.toHaveBeenCalled();
|
||||
expect(mockDeleteUserTokens).not.toHaveBeenCalled();
|
||||
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
|
||||
expect(mockDeleteUserTokens.mock.calls[0][0]).toMatchObject({ userId, serverName });
|
||||
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
test('returns early when client info is missing', async () => {
|
||||
test('clears stored state when client info is missing', async () => {
|
||||
setupOAuthServerFound();
|
||||
mockGetClientInfoAndMetadata.mockResolvedValue(null);
|
||||
|
||||
await maybeUninstallOAuthMCP(userId, pluginKey, appConfig);
|
||||
|
||||
expect(mockGetTokens).not.toHaveBeenCalled();
|
||||
expect(mockDeleteUserTokens).not.toHaveBeenCalled();
|
||||
expect(mockDeleteFlow).not.toHaveBeenCalled();
|
||||
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
|
||||
expect(mockDeleteUserTokens.mock.calls[0][0]).toMatchObject({ userId, serverName });
|
||||
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
test('clears stored state when client info cannot be loaded', async () => {
|
||||
setupOAuthServerFound();
|
||||
mockGetClientInfoAndMetadata.mockRejectedValue(new Error('bad client data'));
|
||||
mockDeleteUserTokens.mockResolvedValue(undefined);
|
||||
mockDeleteFlow.mockResolvedValue(undefined);
|
||||
|
||||
await maybeUninstallOAuthMCP(userId, pluginKey, appConfig);
|
||||
|
||||
expect(mockGetTokens).not.toHaveBeenCalled();
|
||||
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
|
||||
expect(mockDeleteUserTokens.mock.calls[0][0]).toMatchObject({ userId, serverName });
|
||||
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
|
||||
expect(mockLoggerWarn).toHaveBeenCalledWith(
|
||||
`[maybeUninstallOAuthMCP] Unable to load OAuth client metadata for ${serverName}; clearing local MCP OAuth state only.`,
|
||||
expect.any(Error),
|
||||
);
|
||||
});
|
||||
|
||||
test('revokes both tokens and runs cleanup on happy path', async () => {
|
||||
|
|
@ -215,9 +228,9 @@ describe('maybeUninstallOAuthMCP', () => {
|
|||
expect(mockDeleteFlow.mock.calls[1][1]).toBe('mcp_oauth');
|
||||
});
|
||||
|
||||
test('skips revocation but still runs cleanup when getTokens throws ReauthenticationRequiredError', async () => {
|
||||
test('skips revocation but still runs cleanup when token retrieval fails', async () => {
|
||||
setupOAuthServerFound();
|
||||
mockGetTokens.mockRejectedValue(new ReauthenticationRequiredError(serverName, 'missing'));
|
||||
mockGetTokens.mockRejectedValue(new Error('missing'));
|
||||
mockDeleteUserTokens.mockResolvedValue(undefined);
|
||||
mockDeleteFlow.mockResolvedValue(undefined);
|
||||
|
||||
|
|
@ -226,7 +239,10 @@ describe('maybeUninstallOAuthMCP', () => {
|
|||
expect(mockRevokeOAuthToken).not.toHaveBeenCalled();
|
||||
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
|
||||
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
|
||||
expect(mockLoggerInfo).toHaveBeenCalledWith(expect.stringContaining('No usable tokens'));
|
||||
expect(mockLoggerWarn).toHaveBeenCalledWith(
|
||||
`[maybeUninstallOAuthMCP] Unable to load OAuth tokens for ${serverName}; clearing local token state.`,
|
||||
expect.any(Error),
|
||||
);
|
||||
});
|
||||
|
||||
test('skips revocation, logs warn, and still runs cleanup on unexpected token-retrieval error', async () => {
|
||||
|
|
@ -241,7 +257,7 @@ describe('maybeUninstallOAuthMCP', () => {
|
|||
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
|
||||
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
|
||||
expect(mockLoggerWarn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Unexpected error retrieving tokens'),
|
||||
`[maybeUninstallOAuthMCP] Unable to load OAuth tokens for ${serverName}; clearing local token state.`,
|
||||
expect.any(Error),
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue