diff --git a/apps/web/src/app/api/mcp-oauth/callback/__tests__/route.test.ts b/apps/web/src/app/api/mcp-oauth/callback/__tests__/route.test.ts index daac6838b..abf30c14c 100644 --- a/apps/web/src/app/api/mcp-oauth/callback/__tests__/route.test.ts +++ b/apps/web/src/app/api/mcp-oauth/callback/__tests__/route.test.ts @@ -47,6 +47,7 @@ vi.mock('@/lib/server/bootstrap-runtime-env', () => ({ vi.mock('@/lib/server/mcp-linear', () => ({ hydrateLinearMcpConnectionAfterOauth: hydrateLinearMcpConnectionAfterOauthMock, + LinearReplayIdentityMismatchError: class extends Error {}, })); vi.mock('@/lib/server/logger', () => ({ @@ -90,6 +91,8 @@ vi.mock('@roomote/types', () => ({ isSelfServeMcpIntegration: isSelfServeMcpIntegrationMock, })); +import { LinearReplayIdentityMismatchError } from '@/lib/server/mcp-linear'; + import { GET } from '../route'; const CONNECTION_ID = 'conn-linear-1'; @@ -237,10 +240,16 @@ describe('GET /api/mcp-oauth/callback', () => { { client_id: 'client-1' }, PUBLIC_CALLBACK, ); - expect(storeTokensMock).toHaveBeenCalledWith(CONNECTION_ID, { - access_token: 'access-token', - refresh_token: 'refresh-token', + expect(hydrateLinearMcpConnectionAfterOauthMock).toHaveBeenCalledWith({ + connection: expect.objectContaining({ id: CONNECTION_ID }), + tokens: { + access_token: 'access-token', + refresh_token: 'refresh-token', + }, + replayToken: null, + enabledByUserId: 'user-1', }); + expect(storeTokensMock).not.toHaveBeenCalled(); }); it('redirects oauth errors to the public settings host', async () => { @@ -293,17 +302,14 @@ describe('GET /api/mcp-oauth/callback', () => { ); }); - it('distinguishes Linear workspace metadata failures after token storage', async () => { + it('does not store Linear tokens when identity metadata validation fails', async () => { hydrateLinearMcpConnectionAfterOauthMock.mockRejectedValueOnce( new Error('viewer lookup failed'), ); const response = await GET(buildRequest('?code=auth-code&state=state-1')); - expect(storeTokensMock).toHaveBeenCalledWith(CONNECTION_ID, { - access_token: 'access-token', - refresh_token: 'refresh-token', - }); + expect(storeTokensMock).not.toHaveBeenCalled(); expect(response.headers.get('location')).toBe( 'https://customer.example/settings?mcp=error&reason=linear_metadata_failed', ); @@ -318,4 +324,18 @@ describe('GET /api/mcp-oauth/callback', () => { 'MCP OAuth callback failed', ); }); + + it('preserves the existing connection status on replay identity mismatch', async () => { + hydrateLinearMcpConnectionAfterOauthMock.mockRejectedValueOnce( + new LinearReplayIdentityMismatchError(), + ); + + const response = await GET(buildRequest('?code=auth-code&state=state-1')); + + expect(response.headers.get('location')).toBe( + 'https://customer.example/settings?mcp=error&reason=linear_metadata_failed', + ); + expect(storeTokensMock).not.toHaveBeenCalled(); + expect(updateAuthStatusMock).not.toHaveBeenCalled(); + }); }); diff --git a/apps/web/src/app/api/mcp-oauth/callback/route.ts b/apps/web/src/app/api/mcp-oauth/callback/route.ts index 444944ff6..c8e42bb25 100644 --- a/apps/web/src/app/api/mcp-oauth/callback/route.ts +++ b/apps/web/src/app/api/mcp-oauth/callback/route.ts @@ -25,7 +25,10 @@ import { authorize } from '@/lib/server'; import { bootstrapWebRuntimeEnv } from '@/lib/server/bootstrap-runtime-env'; import { getPublicAppUrl } from '@/lib/server/get-public-app-url'; import { logger } from '@/lib/server/logger'; -import { hydrateLinearMcpConnectionAfterOauth } from '@/lib/server/mcp-linear'; +import { + hydrateLinearMcpConnectionAfterOauth, + LinearReplayIdentityMismatchError, +} from '@/lib/server/mcp-linear'; import type { McpOAuthErrorReason, McpOAuthResult, @@ -317,17 +320,17 @@ export async function GET(request: NextRequest) { redirectUri, ); - failureStage = 'token_storage'; - await storeTokens(resolvedConnectionId, tokens); - if (integration.id === 'linear') { failureStage = 'linear_metadata'; await hydrateLinearMcpConnectionAfterOauth({ connection, - accessToken: tokens.access_token, + tokens, replayToken: oauthState.replayToken, enabledByUserId: userId, }); + } else { + failureStage = 'token_storage'; + await storeTokens(resolvedConnectionId, tokens); } if (requiresOrgAdmin) { @@ -365,7 +368,7 @@ export async function GET(request: NextRequest) { 'MCP OAuth callback failed', ); - if (connectionId) { + if (connectionId && !(error instanceof LinearReplayIdentityMismatchError)) { try { await updateAuthStatus(connectionId, 'error'); } catch (statusError) { diff --git a/apps/web/src/app/api/mcp-oauth/replay/[token]/__tests__/route.test.ts b/apps/web/src/app/api/mcp-oauth/replay/[token]/__tests__/route.test.ts index f721cb9a4..12541c098 100644 --- a/apps/web/src/app/api/mcp-oauth/replay/[token]/__tests__/route.test.ts +++ b/apps/web/src/app/api/mcp-oauth/replay/[token]/__tests__/route.test.ts @@ -8,6 +8,7 @@ const { getMcpIntegrationMock, getMcpOauthReplayMock, insertReturningMock, + onConflictDoUpdateMock, updateMcpOauthReplayMock, } = vi.hoisted(() => ({ authorizeMock: vi.fn(), @@ -17,6 +18,7 @@ const { getMcpIntegrationMock: vi.fn(), getMcpOauthReplayMock: vi.fn(), insertReturningMock: vi.fn(), + onConflictDoUpdateMock: vi.fn(), updateMcpOauthReplayMock: vi.fn(), })); @@ -28,9 +30,9 @@ vi.mock('@roomote/db/server', () => ({ db: { insert: vi.fn(() => ({ values: vi.fn(() => ({ - onConflictDoUpdate: vi.fn(() => ({ + onConflictDoUpdate: onConflictDoUpdateMock.mockReturnValue({ returning: insertReturningMock, - })), + }), })), })), }, @@ -88,6 +90,10 @@ describe('GET /api/mcp-oauth/replay/[token]', () => { expect(response.headers.get('location')).toBe( 'https://roomote.example/api/mcp-oauth/initiate/connection-1?redirectTo=%2Fsettings%2Fpersonal&replayToken=replay-token', ); + expect(onConflictDoUpdateMock).toHaveBeenCalledWith({ + target: ['userId', 'mcpId', 'connectionRole'], + set: { updatedAt: expect.any(Date) }, + }); }); it('sends signed-out users to sign in on the public app origin', async () => { diff --git a/apps/web/src/app/api/mcp-oauth/replay/[token]/route.ts b/apps/web/src/app/api/mcp-oauth/replay/[token]/route.ts index fde0ff07a..b47041d2c 100644 --- a/apps/web/src/app/api/mcp-oauth/replay/[token]/route.ts +++ b/apps/web/src/app/api/mcp-oauth/replay/[token]/route.ts @@ -76,10 +76,6 @@ export async function GET( mcpConnections.connectionRole, ], set: { - userId: targetUserId, - authConfig: null, - enabled: false, - authStatus: 'pending', updatedAt: new Date(), }, }) diff --git a/apps/web/src/lib/server/mcp-linear.test.ts b/apps/web/src/lib/server/mcp-linear.test.ts new file mode 100644 index 000000000..631c80a30 --- /dev/null +++ b/apps/web/src/lib/server/mcp-linear.test.ts @@ -0,0 +1,139 @@ +const { + consumeMcpOauthReplayMock, + dbUpdateSetMock, + dbUpdateMock, + getMcpOauthReplayMock, + linearViewerMock, +} = vi.hoisted(() => ({ + consumeMcpOauthReplayMock: vi.fn(), + dbUpdateSetMock: vi.fn(), + dbUpdateMock: vi.fn(), + getMcpOauthReplayMock: vi.fn(), + linearViewerMock: vi.fn(), +})); + +vi.mock('@linear/sdk', () => ({ + LinearClient: class { + get viewer() { + return linearViewerMock(); + } + }, +})); + +vi.mock('@roomote/db/server', () => ({ + db: { + query: { mcpConnections: { findFirst: vi.fn() } }, + update: dbUpdateMock, + }, + deploymentMcpEnablements: {}, + eq: vi.fn(), + mcpConnections: { id: 'id' }, +})); + +vi.mock('@roomote/sdk/server', () => ({ + consumeMcpOauthReplay: consumeMcpOauthReplayMock, + findLinearDeploymentMcpConnection: vi.fn(), + getLinearDeploymentMetadata: vi.fn(), + getMcpOauthReplay: getMcpOauthReplayMock, + getValidAccessToken: vi.fn(), + LINEAR_ORG_CONNECTION_ROLE: 'deployment', + LINEAR_USER_CONNECTION_ROLE: 'user', +})); + +vi.mock('@roomote/linear', () => ({ + createLinearAgentRun: vi.fn(), + createLinearClient: vi.fn(), + enrichSessionComments: vi.fn(), + parseAgentSessionEventPayload: vi.fn(), +})); + +import { hydrateLinearMcpConnectionAfterOauth } from './mcp-linear'; + +describe('hydrateLinearMcpConnectionAfterOauth', () => { + beforeEach(() => { + vi.clearAllMocks(); + linearViewerMock.mockResolvedValue({ + id: 'linear-user-1', + organization: Promise.resolve({ id: 'linear-org-1' }), + }); + dbUpdateMock.mockReturnValue({ + set: dbUpdateSetMock.mockReturnValue({ where: vi.fn() }), + }); + }); + + it.each([ + { + name: 'user', + metadata: { + linearUserId: 'linear-user-2', + linearOrganizationId: 'linear-org-1', + }, + }, + { + name: 'organization', + metadata: { + linearUserId: 'linear-user-1', + linearOrganizationId: 'linear-org-2', + }, + }, + ])( + 'rejects a replay authorized by a different Linear $name', + async ({ metadata }) => { + getMcpOauthReplayMock.mockResolvedValue({ + mcpId: 'linear', + metadata, + }); + + await expect( + hydrateLinearMcpConnectionAfterOauth({ + connection: { + id: 'connection-1', + connectionRole: 'user', + userId: 'roomote-user-1', + } as never, + tokens: { access_token: 'access-token' }, + replayToken: 'replay-token', + }), + ).rejects.toThrow( + 'The authorized Linear account does not match the requested session', + ); + + expect(dbUpdateMock).not.toHaveBeenCalled(); + expect(consumeMcpOauthReplayMock).not.toHaveBeenCalled(); + }, + ); + + it('stores Linear identity metadata and OAuth tokens in one update', async () => { + await hydrateLinearMcpConnectionAfterOauth({ + connection: { + id: 'connection-1', + connectionRole: 'user', + userId: 'roomote-user-1', + authConfig: { type: 'oauth_client', client_id: 'client-1' }, + } as never, + tokens: { + access_token: 'access-token', + refresh_token: 'refresh-token', + expires_in: 3600, + scope: 'read write', + }, + }); + + expect(dbUpdateMock).toHaveBeenCalledTimes(1); + expect(dbUpdateSetMock).toHaveBeenCalledWith( + expect.objectContaining({ + accessToken: 'access-token', + refreshToken: 'refresh-token', + tokenExpiresAt: expect.any(Date), + scopes: ['read', 'write'], + authStatus: 'authenticated', + enabled: true, + authConfig: expect.objectContaining({ + client_id: 'client-1', + linearOrganizationId: 'linear-org-1', + linearUserId: 'linear-user-1', + }), + }), + ); + }); +}); diff --git a/apps/web/src/lib/server/mcp-linear.ts b/apps/web/src/lib/server/mcp-linear.ts index ce96ddfdc..92aa04380 100644 --- a/apps/web/src/lib/server/mcp-linear.ts +++ b/apps/web/src/lib/server/mcp-linear.ts @@ -9,6 +9,7 @@ import { import { consumeMcpOauthReplay, findLinearDeploymentMcpConnection, + getMcpOauthReplay, getValidAccessToken, getLinearDeploymentMetadata, LINEAR_ORG_CONNECTION_ROLE, @@ -20,19 +21,50 @@ import { enrichSessionComments, parseAgentSessionEventPayload, } from '@roomote/linear'; +import type { OAuthTokens } from '@roomote/types'; type McpConnectionRecord = Awaited< ReturnType >; -async function updateLinearConnectionMetadata(input: { +export class LinearReplayIdentityMismatchError extends Error { + constructor() { + super('The authorized Linear account does not match the requested session'); + this.name = 'LinearReplayIdentityMismatchError'; + } +} + +function replayMatchesLinearIdentity( + replay: { mcpId: string; metadata: unknown }, + identity: { linearOrganizationId: string; linearUserId: string }, +) { + if ( + replay.mcpId !== 'linear' || + !replay.metadata || + typeof replay.metadata !== 'object' + ) { + return false; + } + + const metadata = replay.metadata as Record; + return ( + metadata.linearOrganizationId === identity.linearOrganizationId && + metadata.linearUserId === identity.linearUserId + ); +} + +async function storeLinearConnection(input: { connection: NonNullable; + tokens: OAuthTokens; linearOrganizationId: string; linearOrganizationName?: string | null; linearOrganizationUrlKey?: string | null; appUserId?: string; linearUserId?: string; }) { + const tokenExpiresAt = input.tokens.expires_in + ? new Date(Date.now() + input.tokens.expires_in * 1000) + : null; const authConfig = input.connection.authConfig && typeof input.connection.authConfig === 'object' @@ -50,6 +82,14 @@ async function updateLinearConnectionMetadata(input: { ...(input.appUserId ? { appUserId: input.appUserId } : {}), ...(input.linearUserId ? { linearUserId: input.linearUserId } : {}), } as NonNullable['authConfig'], + accessToken: input.tokens.access_token, + refreshToken: input.tokens.refresh_token || null, + tokenExpiresAt, + scopes: input.tokens.scope + ? input.tokens.scope.split(/[\s,]+/).filter(Boolean) + : [], + authStatus: 'authenticated', + enabled: true, updatedAt: new Date(), }) .where(eq(mcpConnections.id, input.connection.id)); @@ -58,13 +98,15 @@ async function updateLinearConnectionMetadata(input: { async function resumeLinearReplay(input: { replayToken: string; userId: string; + linearOrganizationId: string; + linearUserId: string; }) { const replay = await consumeMcpOauthReplay(input.replayToken); if (!replay) { return; } - if (replay.mcpId !== 'linear') { + if (!replayMatchesLinearIdentity(replay, input)) { return; } @@ -130,11 +172,13 @@ async function resumeLinearReplay(input: { export async function hydrateLinearMcpConnectionAfterOauth(input: { connection: NonNullable; - accessToken: string; + tokens: OAuthTokens; replayToken?: string | null; enabledByUserId?: string; }) { - const viewerClient = new LinearClient({ accessToken: input.accessToken }); + const viewerClient = new LinearClient({ + accessToken: input.tokens.access_token, + }); const viewer = await viewerClient.viewer; const organization = await viewer.organization; @@ -143,8 +187,9 @@ export async function hydrateLinearMcpConnectionAfterOauth(input: { } if (input.connection.connectionRole === LINEAR_ORG_CONNECTION_ROLE) { - await updateLinearConnectionMetadata({ + await storeLinearConnection({ connection: input.connection, + tokens: input.tokens, linearOrganizationId: organization.id, linearOrganizationName: organization.name, linearOrganizationUrlKey: organization.urlKey ?? null, @@ -173,8 +218,22 @@ export async function hydrateLinearMcpConnectionAfterOauth(input: { } if (input.connection.connectionRole === LINEAR_USER_CONNECTION_ROLE) { - await updateLinearConnectionMetadata({ + if (input.replayToken) { + const replay = await getMcpOauthReplay(input.replayToken); + if ( + !replay || + !replayMatchesLinearIdentity(replay, { + linearOrganizationId: organization.id, + linearUserId: viewer.id, + }) + ) { + throw new LinearReplayIdentityMismatchError(); + } + } + + await storeLinearConnection({ connection: input.connection, + tokens: input.tokens, linearOrganizationId: organization.id, linearUserId: viewer.id, }); @@ -183,6 +242,8 @@ export async function hydrateLinearMcpConnectionAfterOauth(input: { await resumeLinearReplay({ replayToken: input.replayToken, userId: input.connection.userId, + linearOrganizationId: organization.id, + linearUserId: viewer.id, }); } }