LibreChat/api/server/controllers/__tests__/maybeUninstallOAuthMCP.spec.js
Danny Avila 4cce88be42
🪟 feat: Add allowedAddresses Exemption List For SSRF-Guarded Targets (#12933)
* 🪟 feat: Add allowedAddresses Exemption List For SSRF-Guarded Targets

LibreChat already blocks SSRF-prone targets (private IPs, loopback,
link-local, .internal/.local TLDs) at every server-side fetch site
that consumes user-controllable URLs — custom-endpoint baseURLs, MCP
servers, OpenAPI Actions, and OAuth endpoints. The only existing
escape hatch is `allowedDomains`, but that flips the field into a
strict whitelist: adding `127.0.0.1` to permit a self-hosted Ollama
also blocks every public destination that isn't in the list.

Introduce `allowedAddresses` as the orthogonal primitive: a private-
IP-space exemption list. When a hostname or its resolved IP appears
in the list, the SSRF block is bypassed for that target. Public
destinations remain reachable. Operators can now run self-hosted
LLMs / MCP servers / Action endpoints on private addresses without
weakening the default-deny posture for everything else.

Schema additions in `packages/data-provider/src/config.ts`:
- `endpoints.allowedAddresses` (new — gates `validateEndpointURL`)
- `mcpSettings.allowedAddresses` (parallel to `allowedDomains`)
- `actions.allowedAddresses` (parallel to `allowedDomains`)

Core changes in `packages/api/src/auth/`:
- New `isAddressAllowed(hostnameOrIP, allowedAddresses)` — pure,
  case-insensitive, bracket-stripped literal match.
- Threaded the list through `isSSRFTarget`, `resolveHostnameSSRF`,
  `isDomainAllowedCore`, `isActionDomainAllowed`, `isMCPDomainAllowed`,
  `isOAuthUrlAllowed`, and `validateEndpointURL`.
- Extended `createSSRFSafeAgents` and `createSSRFSafeUndiciConnect`
  to accept the list, building an SSRF-safe DNS lookup that exempts
  matching hostnames/IPs at TCP connect time (TOCTOU-safe).

Wiring:
- Custom and OpenAI endpoint initialize sites pass
  `endpoints.allowedAddresses` to `validateEndpointURL`.
- `MCPServersRegistry` stores `allowedAddresses` and exposes it via
  `getAllowedAddresses()`. The factory, connection class, manager,
  `UserConnectionManager`, and `ConnectionsRepository` all thread
  it through to the SSRF utilities.
- `MCPOAuthHandler.initiateOAuthFlow`, `refreshOAuthTokens`, and
  `validateOAuthUrl` accept the list and consult it on every URL
  validation along the OAuth chain.
- `ToolService`, `ActionService`, and the assistants/agents action
  routes pass `actions.allowedAddresses` to `isActionDomainAllowed`
  and to `createSSRFSafeAgents` for runtime action calls.
- `initializeMCPs.js` reads `mcpSettings.allowedAddresses` from the
  app config and forwards it to the registry constructor.

Documentation:
- `librechat.example.yaml` shows the new field next to each existing
  `allowedDomains` block, with a note clarifying that
  `allowedAddresses` is an exemption list (not a whitelist).

Tests:
- Unit tests for `isAddressAllowed` covering literal IPs, hostnames,
  IPv6 brackets, case insensitivity, and partial-match rejection.
- Exemption tests for every entry point: `isSSRFTarget`,
  `resolveHostnameSSRF`, `validateEndpointURL`, `isActionDomainAllowed`,
  `isMCPDomainAllowed`, `isOAuthUrlAllowed`.
- Existing tests updated to reflect the new optional parameter.

Default behavior is unchanged: omitted = empty list = no exemptions.

* 🩹 fix: Plumb allowedAddresses Through AppConfig endpoints Type

The initial PR added `endpoints.allowedAddresses` to the
data-provider config schema and consumed it in the endpoint
initialize sites, but the runtime `AppConfig.endpoints` shape in
`@librechat/data-schemas` was a hand-maintained subset that didn't
include the new field — so `tsc` rejected `appConfig.endpoints.allowedAddresses`.

Add the field to `AppConfig['endpoints']` in
`packages/data-schemas/src/types/app.ts` and forward it from the
loaded config in `packages/data-schemas/src/app/endpoints.ts` so the
runtime config carries the value.

Update `initializeMCPs.spec.js` to expect the third positional
argument (`allowedAddresses`) on the `createMCPServersRegistry` call.

* 🩹 fix: Enforce allowedDomains Before allowedAddresses In isOAuthUrlAllowed

The initial implementation checked the address exemption first, so a
URL whose hostname appeared in `allowedAddresses` would return true
even when the admin had configured `allowedDomains` as a strict bound
on OAuth endpoints. A malicious MCP server could advertise OAuth
metadata, token, or revocation URLs at any address the admin had
permitted for an unrelated reason (a self-hosted LLM at `127.0.0.1`,
for example) and pass validation, expanding SSRF reach beyond the
configured domain whitelist.

Reorder: when `allowedDomains` is set, treat it as authoritative —
return true only if the URL matches a domain entry, otherwise fall
through to false. The address exemption only applies when no
`allowedDomains` is configured (mirrors how the downstream SSRF check
in `validateOAuthUrl` consults `allowedAddresses`).

Add a regression test asserting that an `allowedAddresses` entry does
not broaden a configured `allowedDomains` list.

Reported by chatgpt-codex-connector on PR #12933.

* 🩹 fix: Forward allowedAddresses To Remaining OAuth Callers

Two `MCPOAuthHandler` callers still used the pre-feature signatures and
were silently dropping the new `allowedAddresses` argument:

- `api/server/routes/mcp.js` invoked `initiateOAuthFlow` with the old
  5-argument shape, so OAuth flows initiated through the route handler
  ignored the registry's `getAllowedAddresses()` and would reject any
  metadata/authorization/token URL on a permitted private host.
- `api/server/controllers/UserController.js#maybeUninstallOAuthMCP`
  invoked `revokeOAuthToken` without the address exemption, so
  uninstalling an OAuth-backed MCP server on a permitted private host
  would fail at the revocation step even though the rest of the MCP
  connection path now permits it.

Both sites now read `allowedAddresses` from the registry alongside
`allowedDomains` and forward it. Reported by Copilot on PR #12933.

* 🩹 fix: Update Test Mocks And Assertions For OAuth allowedAddresses

The previous commit started passing `allowedAddresses` to
`MCPOAuthHandler.initiateOAuthFlow` from `api/server/routes/mcp.js`
and to `MCPOAuthHandler.revokeOAuthToken` from
`api/server/controllers/UserController.js`, but the corresponding
test files mocked the registry without `getAllowedAddresses` (causing
`TypeError`s) and asserted the old positional shape on
`toHaveBeenCalledWith`.

Update the mocks and assertions to match the new arity:

- `api/server/routes/__tests__/mcp.spec.js`: add
  `getAllowedDomains`/`getAllowedAddresses` to the registry mock and
  expect the additional positional args on `initiateOAuthFlow`.
- `api/server/controllers/__tests__/maybeUninstallOAuthMCP.spec.js`:
  add a `getAllowedAddresses` mock alongside the existing
  `getAllowedDomains` and seed it in `setupOAuthServerFound`.
- `api/server/controllers/__tests__/UserController.mcpOAuth.spec.js`:
  add `getAllowedAddresses` to the registry mock and expect the
  trailing `null` arg on the three `revokeOAuthToken` assertions.

* 🛡️ fix: Address Comprehensive Review — Scope allowedAddresses To Private IP Space

Major findings from the comprehensive PR review (severity → fix):

**CRITICAL — `validateOAuthUrl` SSRF fallback bypass.** When `allowedDomains`
is configured and a URL fails the whitelist, the SSRF fallback in
`validateOAuthUrl` was still passing `allowedAddresses` to `isSSRFTarget` /
`resolveHostnameSSRF`, letting a malicious MCP server advertise OAuth
endpoints at any address the admin had permitted for an unrelated reason.
Suppress `allowedAddresses` in the fallback when `allowedDomains` is active —
the address exemption is opt-in for the no-whitelist mode only.

**MAJOR — WebSocket transport SSRF check ignored exemptions.** The
`constructTransport` WebSocket branch called `resolveHostnameSSRF(wsHostname)`
without `this.allowedAddresses`, so a permitted private MCP server would
pass `isMCPDomainAllowed` but be blocked at transport creation. Forward
the exemption.

**Scope `allowedAddresses` to private IP space only (operator directive).**
The exemption list is for permitting private/internal targets; it must not
be a back-door to broaden trust to public destinations.
- Schema (`packages/data-provider/src/config.ts`): new
  `allowedAddressesSchema` rejects URLs (`://`), paths/CIDR (`/`),
  whitespace, and public IPv4/IPv6 literals at config-load time. Wired
  into `endpoints`, `mcpSettings`, and `actions`.
- Runtime (`packages/api/src/auth/domain.ts`): `isAddressAllowed` now
  drops public-IP candidates and public-IP entries on the match path —
  defense in depth so a misconfigured runtime list never grants exemption.
- Hot path (`packages/api/src/auth/agent.ts`): `buildSSRFSafeLookup`
  pre-normalizes the list into a `Set<string>` once at construction and
  applies the same scoping filter, so the connect-time DNS lookup is an
  O(1) Set membership check instead of a full re-iterate-and-normalize on
  every outbound request.

**Test coverage for the connect-time and OAuth-fallback paths.**
- `agent.spec.ts`: new describe block exercising `buildSSRFSafeLookup` and
  `createSSRFSafe*` with `allowedAddresses` — hostname-literal exemption,
  resolved-IP exemption, public-IP scoping, URL/CIDR/whitespace rejection,
  and the default no-list block.
- `handler.allowedAddresses.test.ts` (new): integration tests for
  `validateOAuthUrl` — covers both the no-domains-set "permit private"
  path and the strict-bound regression where `allowedAddresses` must NOT
  bypass `allowedDomains`.

**Documentation & cleanup.**
- `connection.ts` redirect SSRF check: explicit comment that
  `allowedAddresses` is intentionally NOT consulted for redirect targets
  (server-controlled, must not inherit the admin's exemption).
- `MCPConnectionFactory.test.ts`: replaced an `eslint-disable` with a
  proper `import { getTenantId } from '@librechat/data-schemas'`. The
  disable was added to make a pre-existing `require()` quiet — the cleaner
  fix is to use the existing top-level import.

Updated `MCPConnectionSSRF.test.ts` WebSocket SSRF assertions to match the
new two-argument call shape (`hostname, allowedAddresses`).

* 🩹 fix: Require Absolute URL Before allowedAddresses Trust Bypass In isOAuthUrlAllowed

`parseDomainSpec` is lenient — it silently prepends `https://` to
schemeless inputs so it can match patterns like bare `example.com`.
That leniency leaked into `isOAuthUrlAllowed`'s new
`allowedAddresses` short-circuit: a value like `10.0.0.5/oauth` (no
scheme) would parse successfully via the prepended default, hit the
address-exemption path, return `true`, and skip `validateOAuthUrl`'s
strict `new URL(url)` parse-or-throw — only to fail later in OAuth
discovery with a less clear runtime error.

Add a strict `new URL(url)` gate at the top of `isOAuthUrlAllowed`.
Schemeless inputs now fall through to `validateOAuthUrl`'s explicit
"Invalid OAuth <field>" rejection. Tests added in both
`auth/domain.spec.ts` (unit) and the OAuth handler integration spec
(end-to-end).

Reported by chatgpt-codex-connector (P2) on PR #12933.

* 🛡️ fix: Address Follow-Up Comprehensive Review — Schema Tests, Shared Normalization, host:port

Auditing the second comprehensive review:

**F1 MAJOR — schema validation untested.** `allowedAddressesSchema` had
zero coverage, so a regression in the three refinement stages or the
three wiring locations (`endpoints` / `mcpSettings` / `actions`) would
silently let invalid entries reach the runtime. Added a dedicated
`describe('allowedAddressesSchema')` block in `config.spec.ts` covering:
valid private IPs (v4 + v6, including the previously-missed 192.0.0.0/24
range), accepted hostnames, all rejection categories (URLs, CIDR, paths,
whitespace tabs/newlines, host:port, public IP literals), and full
`configSchema.parse()` integration at each of the three nesting points.

**F2 MINOR — `isPrivateIPv4Literal` divergence.** The schema reimpl in
`packages/data-provider` was discarding the `c` octet, so the
`192.0.0.0/24` (RFC 5736 IETF protocol assignments) range that the
authoritative `isPrivateIPv4` accepts was being rejected with a
misleading "public IP" error. Destructure `c` and add the missing range
check; covered by the new schema tests.

**F3 MINOR — DRY violation across `domain.ts` and `agent.ts`.** Both
files had independent normalization implementations with a subtle
whitespace-check divergence (`/\s/` vs `.includes(' ')`). Extracted the
shared logic into a new `packages/api/src/auth/allowedAddresses.ts`
module that both consumers import:
  - `normalizeAddressEntry(entry)` — single-entry shape check
  - `looksLikeHostPort(entry)` — host:port detector (used by F4)
  - `normalizeAllowedAddressesSet(list)` — pre-normalized Set for the
    connect-time hot path
  - `isAddressInAllowedSet(candidate, set)` — membership check that
    enforces private-IP scoping on the candidate

Both `isAddressAllowed` (preflight) and `buildSSRFSafeLookup` (connect)
now go through the same primitives; the whitespace divergence is gone.

To break the import cycle (`allowedAddresses` needs `isPrivateIP`,
`domain` previously owned it), extracted IP private-range detection
into a leaf `auth/ip.ts` module. `domain.ts` re-exports `isPrivateIP`
for backward compatibility with existing call sites.

**F4 MINOR — `host:port` silently misclassified.** Entries like
`localhost:8080` previously slipped through the URL/path guard, were
mis-detected as IPv6, failed `isPrivateIP`, and were silently dropped
with a misleading "public IP" schema error. Added an explicit
`looksLikeHostPort` check with a clear error: "allowedAddresses
entries must not include a port — list the bare hostname or IP only."
Bare `::1`, `[::1]`, and other valid IPv6 literals are intentionally
not matched (regex distinguishes by colon count and the bracketed
`[ipv6]:port` form).

**F5 MINOR — hostname-trust documentation gap.** Hostname entries
short-circuit `resolveHostnameSSRF` before any DNS lookup — that's a
deliberate design (admin trusts the name) but it means the exemption
follows whatever the name resolves to at runtime. Added an explicit
note in `librechat.example.yaml` for both `mcpSettings.allowedAddresses`
and `endpoints.allowedAddresses`: "a hostname entry trusts whatever IP
that name resolves to. Only list hostnames whose DNS you control.
Prefer literal IPs when you can."

**F6** (8 positional params) is flagged for follow-up; refactor to an
options object is a breaking-API change deferred to a separate PR.
**F7** (redirect/WebSocket asymmetry, NIT, conf 40) — skipping; the
existing inline comment is sufficient.

* 🧹 chore: Address Follow-Up NITs — Import Order And Mirror-Function Naming

Three NITs from the latest comprehensive review:

**NIT #1 (conf 85) — local import order.** AGENTS.md requires local
imports sorted longest-to-shortest. Both `domain.ts` and `agent.ts`
had `./ip` (shorter) before `./allowedAddresses` (longer). Swapped.

**NIT #2 (conf 60) — missing cross-reference.** The schema-side
`isHostPortShape` in `packages/data-provider/src/config.ts` had no
note pointing at the canonical runtime mirror. Added a JSDoc paragraph
explaining the mirror relationship and why a local copy exists (the
data-provider package can't import from `@librechat/api` without
creating a circular dependency).

**NIT #3 (conf 50) — naming inconsistency.** Renamed
`isHostPortShape` → `looksLikeHostPort` so the schema mirror matches
the runtime helper exactly. Kept as a separate function (not a shared
import) for the same circular-dependency reason; the matching name
makes it obvious they should stay in lockstep.
2026-05-03 21:43:59 -04:00

300 lines
11 KiB
JavaScript

const mockGetTokens = jest.fn();
const mockDeleteUserTokens = jest.fn();
const mockGetClientInfoAndMetadata = jest.fn();
const mockRevokeOAuthToken = jest.fn();
const mockGetServerConfig = jest.fn();
const mockGetOAuthServers = jest.fn();
const mockGetAllowedDomains = jest.fn();
const mockGetAllowedAddresses = jest.fn();
const mockDeleteFlow = jest.fn();
const mockGetLogStores = jest.fn();
const mockFindToken = jest.fn();
const mockDeleteTokens = jest.fn();
const mockLoggerInfo = jest.fn();
const mockLoggerWarn = jest.fn();
const mockLoggerError = jest.fn();
jest.mock('@librechat/data-schemas', () => ({
logger: { info: mockLoggerInfo, warn: mockLoggerWarn, error: mockLoggerError },
webSearchKeys: [],
}));
jest.mock('@librechat/api', () => {
return {
MCPOAuthHandler: {
revokeOAuthToken: (...args) => mockRevokeOAuthToken(...args),
generateFlowId: (userId, serverName) => `${userId}:${serverName}`,
},
MCPTokenStorage: {
getTokens: (...args) => mockGetTokens(...args),
getClientInfoAndMetadata: (...args) => mockGetClientInfoAndMetadata(...args),
deleteUserTokens: (...args) => mockDeleteUserTokens(...args),
},
normalizeHttpError: jest.fn(),
extractWebSearchEnvVars: jest.fn(),
needsRefresh: jest.fn(),
getNewS3URL: jest.fn(),
};
});
jest.mock('librechat-data-provider', () => ({
Tools: {},
CacheKeys: { FLOWS: 'flows' },
Constants: { mcp_delimiter: '::', mcp_prefix: 'mcp_' },
FileSources: {},
ResourceType: {},
}));
jest.mock('~/config', () => ({
getMCPManager: jest.fn(),
getFlowStateManager: jest.fn(() => ({
deleteFlow: (...args) => mockDeleteFlow(...args),
})),
getMCPServersRegistry: jest.fn(() => ({
getServerConfig: (...args) => mockGetServerConfig(...args),
getOAuthServers: (...args) => mockGetOAuthServers(...args),
getAllowedDomains: (...args) => mockGetAllowedDomains(...args),
getAllowedAddresses: (...args) => mockGetAllowedAddresses(...args),
})),
}));
jest.mock('~/cache', () => ({
getLogStores: (...args) => mockGetLogStores(...args),
}));
jest.mock('~/server/services/PluginService', () => ({
updateUserPluginAuth: jest.fn(),
deleteUserPluginAuth: jest.fn(),
}));
jest.mock('~/server/services/twoFactorService', () => ({
verifyOTPOrBackupCode: jest.fn(),
}));
jest.mock('~/server/services/AuthService', () => ({
verifyEmail: jest.fn(),
resendVerificationEmail: jest.fn(),
}));
jest.mock('~/server/services/Config/getCachedTools', () => ({
invalidateCachedTools: jest.fn(),
}));
jest.mock('~/server/services/Files/process', () => ({
processDeleteRequest: jest.fn(),
}));
jest.mock('~/server/services/Config', () => ({
getAppConfig: jest.fn(),
}));
jest.mock('~/models', () => ({
findToken: (...args) => mockFindToken(...args),
deleteTokens: (...args) => mockDeleteTokens(...args),
updateUser: jest.fn(),
deleteAllUserSessions: jest.fn(),
deleteAllSharedLinks: jest.fn(),
updateUserPlugins: jest.fn(),
deleteUserById: jest.fn(),
deleteMessages: jest.fn(),
deletePresets: jest.fn(),
deleteUserKey: jest.fn(),
getUserById: jest.fn(),
deleteConvos: jest.fn(),
deleteFiles: jest.fn(),
getFiles: jest.fn(),
deleteToolCalls: jest.fn(),
deleteUserAgents: jest.fn(),
deleteUserPrompts: jest.fn(),
deleteTransactions: jest.fn(),
deleteBalances: jest.fn(),
deleteAllAgentApiKeys: jest.fn(),
deleteAssistants: jest.fn(),
deleteConversationTags: jest.fn(),
deleteAllUserMemories: jest.fn(),
deleteActions: jest.fn(),
removeUserFromAllGroups: jest.fn(),
deleteAclEntries: jest.fn(),
getSoleOwnedResourceIds: jest.fn().mockResolvedValue([]),
}));
const { maybeUninstallOAuthMCP } = require('~/server/controllers/UserController');
const userId = 'user-123';
const pluginKey = 'mcp_acme';
const serverName = 'acme';
const serverConfig = {
url: 'https://acme.example.com',
oauth: {
revocation_endpoint: 'https://acme.example.com/revoke',
revocation_endpoint_auth_methods_supported: ['client_secret_basic'],
},
oauth_headers: { 'X-Tenant': 'acme' },
};
const appConfig = {
mcpServers: { acme: serverConfig },
};
const clientInfo = { client_id: 'cid', client_secret: 'csec' };
const clientMetadata = {};
function setupOAuthServerFound() {
mockGetServerConfig.mockResolvedValue(serverConfig);
mockGetOAuthServers.mockResolvedValue(new Set([serverName]));
mockGetAllowedDomains.mockReturnValue(['https://acme.example.com']);
mockGetAllowedAddresses.mockReturnValue(null);
mockGetClientInfoAndMetadata.mockResolvedValue({ clientInfo, clientMetadata });
}
describe('maybeUninstallOAuthMCP', () => {
beforeEach(() => {
jest.clearAllMocks();
});
test('is a no-op when pluginKey is not an MCP key', async () => {
await maybeUninstallOAuthMCP(userId, 'plugin_google_calendar', appConfig);
expect(mockGetServerConfig).not.toHaveBeenCalled();
expect(mockGetTokens).not.toHaveBeenCalled();
expect(mockDeleteUserTokens).not.toHaveBeenCalled();
expect(mockDeleteFlow).not.toHaveBeenCalled();
});
test('clears stored state when the MCP server is not an OAuth server', async () => {
mockGetServerConfig.mockResolvedValue(serverConfig);
mockGetOAuthServers.mockResolvedValue(new Set(['other']));
await maybeUninstallOAuthMCP(userId, pluginKey, appConfig);
expect(mockGetClientInfoAndMetadata).not.toHaveBeenCalled();
expect(mockGetTokens).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 is missing', async () => {
setupOAuthServerFound();
mockGetClientInfoAndMetadata.mockResolvedValue(null);
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);
});
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 () => {
setupOAuthServerFound();
mockGetTokens.mockResolvedValue({
access_token: 'access-abc',
refresh_token: 'refresh-xyz',
});
mockRevokeOAuthToken.mockResolvedValue(undefined);
mockDeleteUserTokens.mockResolvedValue(undefined);
mockDeleteFlow.mockResolvedValue(undefined);
await maybeUninstallOAuthMCP(userId, pluginKey, appConfig);
expect(mockRevokeOAuthToken).toHaveBeenCalledTimes(2);
expect(mockRevokeOAuthToken.mock.calls[0][1]).toBe('access-abc');
expect(mockRevokeOAuthToken.mock.calls[0][2]).toBe('access');
expect(mockRevokeOAuthToken.mock.calls[1][1]).toBe('refresh-xyz');
expect(mockRevokeOAuthToken.mock.calls[1][2]).toBe('refresh');
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
expect(mockDeleteUserTokens.mock.calls[0][0]).toMatchObject({ userId, serverName });
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
expect(mockDeleteFlow.mock.calls[0][1]).toBe('mcp_get_tokens');
expect(mockDeleteFlow.mock.calls[1][1]).toBe('mcp_oauth');
});
test('skips revocation but still runs cleanup when token retrieval fails', async () => {
setupOAuthServerFound();
mockGetTokens.mockRejectedValue(new Error('missing'));
mockDeleteUserTokens.mockResolvedValue(undefined);
mockDeleteFlow.mockResolvedValue(undefined);
await expect(maybeUninstallOAuthMCP(userId, pluginKey, appConfig)).resolves.toBeUndefined();
expect(mockRevokeOAuthToken).not.toHaveBeenCalled();
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
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 () => {
setupOAuthServerFound();
mockGetTokens.mockRejectedValue(new Error('boom: unreachable'));
mockDeleteUserTokens.mockResolvedValue(undefined);
mockDeleteFlow.mockResolvedValue(undefined);
await expect(maybeUninstallOAuthMCP(userId, pluginKey, appConfig)).resolves.toBeUndefined();
expect(mockRevokeOAuthToken).not.toHaveBeenCalled();
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
expect(mockLoggerWarn).toHaveBeenCalledWith(
`[maybeUninstallOAuthMCP] Unable to load OAuth tokens for ${serverName}; clearing local token state.`,
expect.any(Error),
);
});
test('continues cleanup when only one token type is present', async () => {
setupOAuthServerFound();
mockGetTokens.mockResolvedValue({ access_token: 'only-access' });
mockRevokeOAuthToken.mockResolvedValue(undefined);
mockDeleteUserTokens.mockResolvedValue(undefined);
mockDeleteFlow.mockResolvedValue(undefined);
await maybeUninstallOAuthMCP(userId, pluginKey, appConfig);
expect(mockRevokeOAuthToken).toHaveBeenCalledTimes(1);
expect(mockRevokeOAuthToken.mock.calls[0][2]).toBe('access');
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
});
test('still runs cleanup even when both revocation calls fail', async () => {
setupOAuthServerFound();
mockGetTokens.mockResolvedValue({
access_token: 'a',
refresh_token: 'r',
});
mockRevokeOAuthToken.mockRejectedValue(new Error('network down'));
mockDeleteUserTokens.mockResolvedValue(undefined);
mockDeleteFlow.mockResolvedValue(undefined);
await expect(maybeUninstallOAuthMCP(userId, pluginKey, appConfig)).resolves.toBeUndefined();
expect(mockRevokeOAuthToken).toHaveBeenCalledTimes(2);
expect(mockDeleteUserTokens).toHaveBeenCalledTimes(1);
expect(mockDeleteFlow).toHaveBeenCalledTimes(2);
expect(mockLoggerError).toHaveBeenCalled();
});
});