Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions backend/__tests__/unit/routes/hostedMcpConnect.callback.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,20 @@ describe('hosted-mcp connect: callback', () => {
expect(Integration.findOne).not.toHaveBeenCalled();
});

it.each([
['access_denied', 'consent_denied'],
['temporarily_unavailable', 'vendor_error'],
])('maps vendor error %s to fixed code %s without spending the pending flow', async (vendorError, expectedCode) => {
const query = `state=st-1&error=${vendorError}&error_description=${encodeURIComponent('<img src=x onerror=alert(1)>')}`;
const res = await callback(query);
expect(outcome(res)).toEqual({ status: 302, hostedMcp: 'error', code: expectedCode });
expect(res.headers.location).not.toContain('error_description');
expect(res.headers.location).not.toContain('img');
expect(Integration.findOne).not.toHaveBeenCalled();
expect(Integration.findOneAndUpdate).not.toHaveBeenCalled();
expect(global.fetch).not.toHaveBeenCalled();
});

it('reports a refused exchange as a vendor outcome, not as a connected row', async () => {
global.fetch.mockResolvedValue({ ok: false, status: 400, json: async () => ({}) });
const res = await callback();
Expand All @@ -863,6 +877,16 @@ describe('hosted-mcp connect: callback', () => {
expect(Integration.findOneAndUpdate).toHaveBeenCalledTimes(1);
});

it('names invalid_client separately from a provider outage', async () => {
global.fetch.mockResolvedValue({
ok: false, status: 401, json: async () => ({ error: 'invalid_client', error_description: 'vendor text' }),
});
const res = await callback();
expect(outcome(res)).toEqual({ status: 302, hostedMcp: 'error', code: 'client_auth_refused' });
expect(res.headers.location).not.toContain('vendor text');
expect(connectorSecrets.put).not.toHaveBeenCalled();
});

it('reports an exchange that returned no token as incomplete, not as connected', async () => {
global.fetch.mockResolvedValue({ ok: true, json: async () => ({ scope: 'read' }) });
const res = await callback();
Expand Down
10 changes: 10 additions & 0 deletions backend/routes/hostedMcpConnect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,12 @@ router.get('/:entryId/callback', callbackRateLimit, async (req: Request, res: Re
}
const state = typeof req.query.state === 'string' ? req.query.state : '';
const code = typeof req.query.code === 'string' ? req.query.code : '';
const vendorError = typeof req.query.error === 'string' ? req.query.error : '';
if (vendorError) {
// The provider denied authorization before issuing a code. Leave the
// pending flow intact so the member can retry; never forward vendor text.
return callbackRedirect(res, 'error', vendorError === 'access_denied' ? 'consent_denied' : 'vendor_error');
}
if (!state || !code) {
return callbackRedirect(res, 'error', 'invalid_state');
}
Expand Down Expand Up @@ -422,6 +428,10 @@ router.get('/:entryId/callback', callbackRateLimit, async (req: Request, res: Re
body: tokenRequest.body,
});
if (!response.ok) {
const refusal = await response.json().catch(() => null) as { error?: unknown } | null;
if (refusal?.error === 'invalid_client') {
return callbackRedirect(res, 'error', 'client_auth_refused');
}
return callbackRedirect(res, 'error', 'exchange_refused');
}
tokens = (await response.json()) as TokenResponse;
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1942,7 +1942,19 @@
"gatesTitle": "Pods that reach this channel",
"hostedMcpManualRevoke": "Your previous {{provider}} account may still have Commonly connected. Switch to it on {{provider}}, then remove Commonly there.",
"hostedMcpReviewAuthorization": "Open {{provider}}'s connected apps",
"hostedMcpAuthenticationRequired": "Sign in to Commonly, then try connecting again.",
"hostedMcpClientSetup": "This connector’s sign-in setup needs attention. Ask an administrator to check it.",
"hostedMcpConsentDenied": "You didn’t approve this connection. Nothing was saved.",
"hostedMcpExchangeIncomplete": "The provider returned an incomplete authorization. Nothing was saved.",
"hostedMcpExchangeRefused": "The provider refused this connection. Try again or check its permissions.",
"hostedMcpExchangeUnreachable": "Couldn’t reach the provider to finish connecting. Try again.",
"hostedMcpInvalidState": "This connection attempt expired or couldn’t be verified. Start again.",
"hostedMcpIssuerUnavailable": "The provider’s sign-in service is unavailable. Try again later.",
"hostedMcpRateLimited": "Too many connection attempts. Wait a minute, then try again.",
"hostedMcpUnknownEntry": "This connector is no longer available.",
"hostedMcpUnknownError": "Couldn’t finish connecting. Try again.",
"hostedMcpUnrequestedScope": "The provider granted access Commonly didn’t ask for, so the connection wasn’t saved.",
"hostedMcpVendorError": "The provider couldn’t finish authorization. Try again.",
"installInProgress": "Still setting up — try again in a moment.",
"installInProgressForPod": "Still setting up for {{pod}} — try again in a moment.",
"leadAny": "Any agent leads",
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/i18n/locales/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -1934,7 +1934,19 @@
"gatesTitle": "接入此频道的 Pod",
"hostedMcpManualRevoke": "你之前的 {{provider}} 账号可能仍保留对 Commonly 的授权。请在 {{provider}} 切换到那个账号,再在那里移除 Commonly。",
"hostedMcpReviewAuthorization": "打开 {{provider}} 的已连接应用",
"hostedMcpAuthenticationRequired": "请先登录 Commonly,再重新连接。",
"hostedMcpClientSetup": "此连接器的登录配置有问题,请联系管理员检查。",
"hostedMcpConsentDenied": "你没有同意此次授权。连接未保存。",
"hostedMcpExchangeIncomplete": "提供方返回的授权信息不完整。连接未保存。",
"hostedMcpExchangeRefused": "提供方拒绝了连接。请重试或检查授权设置。",
"hostedMcpExchangeUnreachable": "无法联系提供方以完成连接,请重试。",
"hostedMcpInvalidState": "此次连接已过期或无法验证,请重新开始。",
"hostedMcpIssuerUnavailable": "提供方的登录服务暂时不可用,请稍后重试。",
"hostedMcpRateLimited": "连接尝试次数过多,请稍等一分钟再试。",
"hostedMcpUnknownEntry": "此连接器已不可用。",
"hostedMcpUnknownError": "无法完成连接,请重试。",
"hostedMcpUnrequestedScope": "提供方授予了 Commonly 未请求的访问权限,因此我们没有保存你的连接。",
"hostedMcpVendorError": "提供方未能完成授权,请重试。",
"installInProgress": "仍在设置中,请稍后再试。",
"installInProgressForPod": "{{pod}} 仍在设置中,请稍后再试。",
"leadAny": "任一智能体都可主导",
Expand Down
20 changes: 20 additions & 0 deletions frontend/src/v2/__tests__/V2ConnectorTools.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -679,3 +679,23 @@ test('step 7: the page\'s call to start a hosted connect is credentialed', async
open.mockRestore();
}
});

test('a hosted connect start error uses fixed copy instead of response text', async () => {
mockApi([{ ...linearEntry, connections: [] }]);
axios.post.mockRejectedValue({
response: { data: { error: 'client_not_configured', message: 'spoofed vendor text' } },
});
const opened = { opener: null, location: { assign: jest.fn() }, close: jest.fn() };
const open = jest.spyOn(window, 'open').mockReturnValue(opened as never);
try {
renderTools();
fireEvent.click(await screen.findByRole('button', { name: 'Connect' }));
expect(await screen.findByRole('alert')).toHaveTextContent(
'This connector’s sign-in setup needs attention. Ask an administrator to check it.',
);
expect(screen.queryByText(/spoofed vendor text/)).toBeNull();
expect(opened.close).toHaveBeenCalled();
} finally {
open.mockRestore();
}
});
34 changes: 28 additions & 6 deletions frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -687,15 +687,37 @@ describe('V2ConnectorsPage', () => {
expect(screen.queryByText('unrequested_scope')).toBeNull();
});

it('does not treat a different hosted-MCP callback code as an unrequested-scope refusal', () => {
window.history.replaceState({}, '', '/v2/connectors?hostedMcp=error&code=exchange_refused');
it.each([
['unknown_entry', 'hostedMcpUnknownEntry'],
['invalid_state', 'hostedMcpInvalidState'],
['state_expired', 'hostedMcpInvalidState'],
['state_consumed', 'hostedMcpInvalidState'],
['browser_mismatch', 'hostedMcpInvalidState'],
['issuer_mismatch', 'hostedMcpInvalidState'],
['consent_denied', 'hostedMcpConsentDenied'],
['vendor_error', 'hostedMcpVendorError'],
['rate_limited', 'hostedMcpRateLimited'],
['issuer_unreachable', 'hostedMcpIssuerUnavailable'],
['issuer_metadata_incomplete', 'hostedMcpIssuerUnavailable'],
['client_not_configured', 'hostedMcpClientSetup'],
['client_registration_required', 'hostedMcpClientSetup'],
['client_auth_unsupported', 'hostedMcpClientSetup'],
['client_auth_refused', 'hostedMcpClientSetup'],
['authorization_parameter_conflict', 'hostedMcpClientSetup'],
['exchange_refused', 'hostedMcpExchangeRefused'],
['exchange_unreachable', 'hostedMcpExchangeUnreachable'],
['exchange_incomplete', 'hostedMcpExchangeIncomplete'],
['exchange_invalid_scope', 'hostedMcpExchangeIncomplete'],
['unrequested_scope', 'hostedMcpUnrequestedScope'],
['authentication_required', 'hostedMcpAuthenticationRequired'],
])('shows a fixed notice for hosted-MCP code %s and consumes the query', async (code, noticeKey) => {
window.history.replaceState({}, '', `/v2/connectors?hostedMcp=error&code=${code}&error_description=spoofed`);
mockGets([]);
renderPage();

expect(screen.queryByText(
"The provider granted access Commonly didn’t ask for, so the connection wasn’t saved.",
)).toBeNull();
expect(window.location.search).toBe('?hostedMcp=error&code=exchange_refused');
expect(await screen.findByRole('alert')).toHaveTextContent(en.connectors[noticeKey]);
expect(window.location.search).toBe('');
expect(screen.queryByText(/spoofed/)).toBeNull();
});

it('reloads connectors after a successful Slack callback and clears its query', async () => {
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/v2/components/V2ConnectorTools.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { useAuth } from '../../context/AuthContext';
import { V2Pod } from '../hooks/useV2Pods';
import { PlatformGlyph } from '../icons/platforms';
import { ActGlyph } from '../icons/glyphs';
import { hostedMcpErrorNotice } from './hostedMcpErrorNotice';

export type GrantWriteMode = 'read' | 'write' | 'write-with-confirm';

Expand Down Expand Up @@ -507,7 +508,7 @@ const V2ConnectorTools: React.FC<Props> = ({ pods, manualRevokeNotice = null })
} catch (err) {
authorizationWindow?.close();
const data = (err as { response?: { data?: { error?: string; message?: string } } })?.response?.data;
setError(data?.message || data?.error || t('tools.connectError', { defaultValue: 'Could not begin connecting. Try again in a moment.' }));
setError(hostedMcpErrorNotice(t, data?.error));
} finally {
setBusy(false);
}
Expand Down Expand Up @@ -999,6 +1000,9 @@ const V2ConnectorTools: React.FC<Props> = ({ pods, manualRevokeNotice = null })
))}
</div>
</div>
{error && !draft && !selected && !githubAppSetup && (
<p className="v2-connector-aside__note" role="alert">{error}</p>
)}
<div className="v2-connectors__rows">
{rows.map(renderRow)}
{notYet.map(renderNotYet)}
Expand Down
7 changes: 5 additions & 2 deletions frontend/src/v2/components/V2ConnectorsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { V2Pod, V2PodMember } from '../hooks/useV2Pods';
import { PlatformGlyph } from '../icons/platforms';
import { ActGlyph, MarkGlyph, MarkName } from '../icons/glyphs';
import V2ConnectorTools, { type HostedMcpManualRevokeNotice } from './V2ConnectorTools';
import { hostedMcpErrorNotice } from './hostedMcpErrorNotice';
import { localizeRelativeTime } from '../utils/localizeRelativeTime';

interface ConnectorGate {
Expand Down Expand Up @@ -385,11 +386,13 @@ const V2ConnectorsPage: React.FC = () => {
// Consume the callback result after displaying a fixed notice.
useEffect(() => {
const query = new URLSearchParams(window.location.search);
if (query.get('hostedMcp') !== 'error' || query.get('code') !== 'unrequested_scope') return;
setHostedMcpCallbackError(t('connectors.hostedMcpUnrequestedScope'));
if (query.get('hostedMcp') !== 'error') return;
setHostedMcpCallbackError(hostedMcpErrorNotice(t, query.get('code')));
query.delete('hostedMcp');
query.delete('code');
query.delete('extraScope');
query.delete('error');
query.delete('error_description');
const remaining = query.toString();
window.history.replaceState(
window.history.state,
Expand Down
32 changes: 32 additions & 0 deletions frontend/src/v2/components/hostedMcpErrorNotice.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import type { TFunction } from 'i18next';

// Only reviewed, fixed copy reaches the UI. Callback URLs and start errors
// carry codes, never text from the provider.
const noticeKeyByCode: Record<string, string> = {
unknown_entry: 'hostedMcpUnknownEntry',
invalid_state: 'hostedMcpInvalidState',
state_expired: 'hostedMcpInvalidState',
state_consumed: 'hostedMcpInvalidState',
browser_mismatch: 'hostedMcpInvalidState',
issuer_mismatch: 'hostedMcpInvalidState',
consent_denied: 'hostedMcpConsentDenied',
vendor_error: 'hostedMcpVendorError',
rate_limited: 'hostedMcpRateLimited',
issuer_unreachable: 'hostedMcpIssuerUnavailable',
issuer_metadata_incomplete: 'hostedMcpIssuerUnavailable',
client_not_configured: 'hostedMcpClientSetup',
client_registration_required: 'hostedMcpClientSetup',
client_auth_unsupported: 'hostedMcpClientSetup',
client_auth_refused: 'hostedMcpClientSetup',
authorization_parameter_conflict: 'hostedMcpClientSetup',
exchange_refused: 'hostedMcpExchangeRefused',
exchange_unreachable: 'hostedMcpExchangeUnreachable',
exchange_incomplete: 'hostedMcpExchangeIncomplete',
exchange_invalid_scope: 'hostedMcpExchangeIncomplete',
unrequested_scope: 'hostedMcpUnrequestedScope',
authentication_required: 'hostedMcpAuthenticationRequired',
};

export const hostedMcpErrorNotice = (t: TFunction, code: string | null | undefined): string => (
t(`connectors.${noticeKeyByCode[code || ''] || 'hostedMcpUnknownError'}`)
);
Loading