Skip to content

Commit 23266fe

Browse files
waleedlatif1claude
andcommitted
fix(mcp): clear OAuth tokens on POST upsert; validate https; OAuthClientProvider conformance
- POST upsert now clears mcp_server_oauth rows when URL or client credentials change - Validate https: scheme on authorizationUrl before window.open to prevent javascript: URI execution - SimMcpOauthProvider now declares 'implements OAuthClientProvider' so SDK upgrades surface as compile errors - Edit form only sends oauthClientId when changed, mirroring oauthClientSecret behavior Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent e37f10a commit 23266fe

4 files changed

Lines changed: 47 additions & 5 deletions

File tree

apps/sim/app/api/mcp/servers/route.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
22
import { db } from '@sim/db'
3-
import { mcpServers } from '@sim/db/schema'
3+
import { mcpServerOauth, mcpServers } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
55
import { toError } from '@sim/utils/errors'
66
import { generateId } from '@sim/utils/id'
77
import { and, eq, isNull } from 'drizzle-orm'
88
import type { NextRequest } from 'next/server'
99
import { createMcpServerBodySchema, deleteMcpServerByQuerySchema } from '@/lib/api/contracts/mcp'
1010
import { validationErrorResponse } from '@/lib/api/server'
11-
import { encryptSecret } from '@/lib/core/security/encryption'
11+
import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
1212
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1313
import {
1414
McpDnsResolutionError,
@@ -133,7 +133,13 @@ export const POST = withRouteHandler(
133133
const oauthClientId = body.oauthClientId || null
134134

135135
const [existingServer] = await db
136-
.select({ id: mcpServers.id, deletedAt: mcpServers.deletedAt })
136+
.select({
137+
id: mcpServers.id,
138+
deletedAt: mcpServers.deletedAt,
139+
url: mcpServers.url,
140+
oauthClientId: mcpServers.oauthClientId,
141+
oauthClientSecret: mcpServers.oauthClientSecret,
142+
})
137143
.from(mcpServers)
138144
.where(and(eq(mcpServers.id, serverId), eq(mcpServers.workspaceId, workspaceId)))
139145
.limit(1)
@@ -143,6 +149,29 @@ export const POST = withRouteHandler(
143149
`[${requestId}] Server with ID ${serverId} already exists, updating instead of creating`
144150
)
145151

152+
const urlChanged = existingServer.url !== body.url
153+
const clientIdChanged = (oauthClientId || null) !== (existingServer.oauthClientId ?? null)
154+
let clientSecretChanged = false
155+
if (body.oauthClientSecret) {
156+
if (!existingServer.oauthClientSecret) {
157+
clientSecretChanged = true
158+
} else {
159+
const currentPlaintext = (await decryptSecret(existingServer.oauthClientSecret))
160+
.decrypted
161+
clientSecretChanged = currentPlaintext !== body.oauthClientSecret
162+
}
163+
} else if (existingServer.oauthClientSecret) {
164+
clientSecretChanged = true
165+
}
166+
const oauthCredsChanged = clientIdChanged || clientSecretChanged
167+
168+
if (urlChanged || oauthCredsChanged) {
169+
await db.delete(mcpServerOauth).where(eq(mcpServerOauth.mcpServerId, serverId))
170+
logger.info(
171+
`[${requestId}] Cleared OAuth credentials for server ${serverId} due to ${urlChanged ? 'URL' : 'OAuth credential'} change`
172+
)
173+
}
174+
146175
await db
147176
.update(mcpServers)
148177
.set({

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,8 @@ export function McpServerFormModal({
507507
const headers = headersToRecord(formData.headers)
508508
const oauthClientId = formData.oauthClientId?.trim()
509509
const oauthClientSecret = formData.oauthClientSecret?.trim()
510+
const originalClientId = (originalData.oauthClientId || '').trim()
511+
const oauthClientIdChanged = (oauthClientId || '') !== originalClientId
510512

511513
const connectionResult = await testConnection({
512514
name: formData.name,
@@ -538,7 +540,12 @@ export function McpServerFormModal({
538540
url: formData.url!,
539541
headers,
540542
timeout: formData.timeout || 30000,
541-
oauthClientId: mode === 'edit' ? (oauthClientId ?? '') : oauthClientId || undefined,
543+
oauthClientId:
544+
mode === 'edit'
545+
? oauthClientIdChanged
546+
? (oauthClientId ?? '')
547+
: undefined
548+
: oauthClientId || undefined,
542549
oauthClientSecret:
543550
mode === 'edit'
544551
? oauthClientSecretTouched
@@ -565,6 +572,7 @@ export function McpServerFormModal({
565572
onOpenChange,
566573
mode,
567574
oauthClientSecretTouched,
575+
originalData,
568576
])
569577

570578
const handleSubmitJson = useCallback(async () => {

apps/sim/hooks/queries/mcp.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,10 @@ export function useStartMcpOauth() {
185185
query: { serverId, workspaceId },
186186
})
187187
if (result.status === 'redirect') {
188+
const parsedUrl = new URL(result.authorizationUrl)
189+
if (parsedUrl.protocol !== 'https:') {
190+
throw new Error('Authorization URL must use HTTPS')
191+
}
188192
const popup = window.open(
189193
result.authorizationUrl,
190194
'mcp-oauth',

apps/sim/lib/mcp/oauth/provider.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { OAuthClientProvider } from '@modelcontextprotocol/sdk/client/auth.js'
12
import type {
23
OAuthClientInformationMixed,
34
OAuthClientMetadata,
@@ -43,7 +44,7 @@ interface SimMcpOauthProviderInit {
4344
preregistered?: PreregisteredClient
4445
}
4546

46-
export class SimMcpOauthProvider {
47+
export class SimMcpOauthProvider implements OAuthClientProvider {
4748
private row: McpOauthRow
4849
private readonly scope?: string
4950
private readonly preregistered?: PreregisteredClient

0 commit comments

Comments
 (0)