Skip to content

Commit ac991d4

Browse files
authored
fix(sso): removed provider specific OIDC logic from SSO registration & deregistration scripts (#2896)
* fix(sso): updated registration & deregistration script for explicit support for Entra ID * cleanup * ack PR comment * ack PR comment * tested edge cases, ack'd PR comments * remove trailing slash
1 parent 69614d2 commit ac991d4

File tree

6 files changed

+204
-117
lines changed

6 files changed

+204
-117
lines changed

apps/sim/app/api/auth/sso/providers/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { eq } from 'drizzle-orm'
44
import { NextResponse } from 'next/server'
55
import { getSession } from '@/lib/auth'
66

7-
const logger = createLogger('SSO-Providers')
7+
const logger = createLogger('SSOProvidersRoute')
88

99
export async function GET() {
1010
try {

apps/sim/app/api/auth/sso/register/route.ts

Lines changed: 107 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { hasSSOAccess } from '@/lib/billing'
66
import { env } from '@/lib/core/config/env'
77
import { REDACTED_MARKER } from '@/lib/core/security/redaction'
88

9-
const logger = createLogger('SSO-Register')
9+
const logger = createLogger('SSORegisterRoute')
1010

1111
const mappingSchema = z
1212
.object({
@@ -43,6 +43,10 @@ const ssoRegistrationSchema = z.discriminatedUnion('providerType', [
4343
])
4444
.default(['openid', 'profile', 'email']),
4545
pkce: z.boolean().default(true),
46+
authorizationEndpoint: z.string().url().optional(),
47+
tokenEndpoint: z.string().url().optional(),
48+
userInfoEndpoint: z.string().url().optional(),
49+
jwksEndpoint: z.string().url().optional(),
4650
}),
4751
z.object({
4852
providerType: z.literal('saml'),
@@ -64,12 +68,10 @@ const ssoRegistrationSchema = z.discriminatedUnion('providerType', [
6468

6569
export async function POST(request: NextRequest) {
6670
try {
67-
// SSO plugin must be enabled in Better Auth
6871
if (!env.SSO_ENABLED) {
6972
return NextResponse.json({ error: 'SSO is not enabled' }, { status: 400 })
7073
}
7174

72-
// Check plan access (enterprise) or env var override
7375
const session = await getSession()
7476
if (!session?.user?.id) {
7577
return NextResponse.json({ error: 'Authentication required' }, { status: 401 })
@@ -116,7 +118,16 @@ export async function POST(request: NextRequest) {
116118
}
117119

118120
if (providerType === 'oidc') {
119-
const { clientId, clientSecret, scopes, pkce } = body
121+
const {
122+
clientId,
123+
clientSecret,
124+
scopes,
125+
pkce,
126+
authorizationEndpoint,
127+
tokenEndpoint,
128+
userInfoEndpoint,
129+
jwksEndpoint,
130+
} = body
120131

121132
const oidcConfig: any = {
122133
clientId,
@@ -127,48 +138,102 @@ export async function POST(request: NextRequest) {
127138
pkce: pkce ?? true,
128139
}
129140

130-
// Add manual endpoints for providers that might need them
131-
// Common patterns for OIDC providers that don't support discovery properly
141+
oidcConfig.authorizationEndpoint = authorizationEndpoint
142+
oidcConfig.tokenEndpoint = tokenEndpoint
143+
oidcConfig.userInfoEndpoint = userInfoEndpoint
144+
oidcConfig.jwksEndpoint = jwksEndpoint
145+
146+
const needsDiscovery =
147+
!oidcConfig.authorizationEndpoint || !oidcConfig.tokenEndpoint || !oidcConfig.jwksEndpoint
148+
149+
if (needsDiscovery) {
150+
const discoveryUrl = `${issuer.replace(/\/$/, '')}/.well-known/openid-configuration`
151+
try {
152+
logger.info('Fetching OIDC discovery document for missing endpoints', {
153+
discoveryUrl,
154+
hasAuthEndpoint: !!oidcConfig.authorizationEndpoint,
155+
hasTokenEndpoint: !!oidcConfig.tokenEndpoint,
156+
hasJwksEndpoint: !!oidcConfig.jwksEndpoint,
157+
})
158+
159+
const discoveryResponse = await fetch(discoveryUrl, {
160+
headers: { Accept: 'application/json' },
161+
})
162+
163+
if (!discoveryResponse.ok) {
164+
logger.error('Failed to fetch OIDC discovery document', {
165+
status: discoveryResponse.status,
166+
statusText: discoveryResponse.statusText,
167+
})
168+
return NextResponse.json(
169+
{
170+
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Status: ${discoveryResponse.status}. Provide all endpoints explicitly or verify the issuer URL.`,
171+
},
172+
{ status: 400 }
173+
)
174+
}
175+
176+
const discovery = await discoveryResponse.json()
177+
178+
oidcConfig.authorizationEndpoint =
179+
oidcConfig.authorizationEndpoint || discovery.authorization_endpoint
180+
oidcConfig.tokenEndpoint = oidcConfig.tokenEndpoint || discovery.token_endpoint
181+
oidcConfig.userInfoEndpoint = oidcConfig.userInfoEndpoint || discovery.userinfo_endpoint
182+
oidcConfig.jwksEndpoint = oidcConfig.jwksEndpoint || discovery.jwks_uri
183+
184+
logger.info('Merged OIDC endpoints (user-provided + discovery)', {
185+
providerId,
186+
issuer,
187+
authorizationEndpoint: oidcConfig.authorizationEndpoint,
188+
tokenEndpoint: oidcConfig.tokenEndpoint,
189+
userInfoEndpoint: oidcConfig.userInfoEndpoint,
190+
jwksEndpoint: oidcConfig.jwksEndpoint,
191+
})
192+
} catch (error) {
193+
logger.error('Error fetching OIDC discovery document', {
194+
error: error instanceof Error ? error.message : 'Unknown error',
195+
discoveryUrl,
196+
})
197+
return NextResponse.json(
198+
{
199+
error: `Failed to fetch OIDC discovery document from ${discoveryUrl}. Please verify the issuer URL is correct or provide all endpoints explicitly.`,
200+
},
201+
{ status: 400 }
202+
)
203+
}
204+
} else {
205+
logger.info('Using explicitly provided OIDC endpoints (all present)', {
206+
providerId,
207+
issuer,
208+
authorizationEndpoint: oidcConfig.authorizationEndpoint,
209+
tokenEndpoint: oidcConfig.tokenEndpoint,
210+
userInfoEndpoint: oidcConfig.userInfoEndpoint,
211+
jwksEndpoint: oidcConfig.jwksEndpoint,
212+
})
213+
}
214+
132215
if (
133-
issuer.includes('okta.com') ||
134-
issuer.includes('auth0.com') ||
135-
issuer.includes('identityserver')
216+
!oidcConfig.authorizationEndpoint ||
217+
!oidcConfig.tokenEndpoint ||
218+
!oidcConfig.jwksEndpoint
136219
) {
137-
const baseUrl = issuer.includes('/oauth2/default')
138-
? issuer.replace('/oauth2/default', '')
139-
: issuer.replace('/oauth', '').replace('/v2.0', '').replace('/oauth2', '')
140-
141-
// Okta-style endpoints
142-
if (issuer.includes('okta.com')) {
143-
oidcConfig.authorizationEndpoint = `${baseUrl}/oauth2/default/v1/authorize`
144-
oidcConfig.tokenEndpoint = `${baseUrl}/oauth2/default/v1/token`
145-
oidcConfig.userInfoEndpoint = `${baseUrl}/oauth2/default/v1/userinfo`
146-
oidcConfig.jwksEndpoint = `${baseUrl}/oauth2/default/v1/keys`
147-
}
148-
// Auth0-style endpoints
149-
else if (issuer.includes('auth0.com')) {
150-
oidcConfig.authorizationEndpoint = `${baseUrl}/authorize`
151-
oidcConfig.tokenEndpoint = `${baseUrl}/oauth/token`
152-
oidcConfig.userInfoEndpoint = `${baseUrl}/userinfo`
153-
oidcConfig.jwksEndpoint = `${baseUrl}/.well-known/jwks.json`
154-
}
155-
// Generic OIDC endpoints (IdentityServer, etc.)
156-
else {
157-
oidcConfig.authorizationEndpoint = `${baseUrl}/connect/authorize`
158-
oidcConfig.tokenEndpoint = `${baseUrl}/connect/token`
159-
oidcConfig.userInfoEndpoint = `${baseUrl}/connect/userinfo`
160-
oidcConfig.jwksEndpoint = `${baseUrl}/.well-known/jwks`
161-
}
220+
const missing: string[] = []
221+
if (!oidcConfig.authorizationEndpoint) missing.push('authorizationEndpoint')
222+
if (!oidcConfig.tokenEndpoint) missing.push('tokenEndpoint')
223+
if (!oidcConfig.jwksEndpoint) missing.push('jwksEndpoint')
162224

163-
logger.info('Using manual OIDC endpoints for provider', {
164-
providerId,
165-
provider: issuer.includes('okta.com')
166-
? 'Okta'
167-
: issuer.includes('auth0.com')
168-
? 'Auth0'
169-
: 'Generic',
170-
authEndpoint: oidcConfig.authorizationEndpoint,
225+
logger.error('Missing required OIDC endpoints after discovery merge', {
226+
missing,
227+
authorizationEndpoint: oidcConfig.authorizationEndpoint,
228+
tokenEndpoint: oidcConfig.tokenEndpoint,
229+
jwksEndpoint: oidcConfig.jwksEndpoint,
171230
})
231+
return NextResponse.json(
232+
{
233+
error: `Missing required OIDC endpoints: ${missing.join(', ')}. Please provide these explicitly or verify the issuer supports OIDC discovery.`,
234+
},
235+
{ status: 400 }
236+
)
172237
}
173238

174239
providerConfig.oidcConfig = oidcConfig

apps/sim/lib/copilot/tools/client/workflow/deploy-api.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
ClientToolCallState,
77
} from '@/lib/copilot/tools/client/base-tool'
88
import { registerToolUIConfig } from '@/lib/copilot/tools/client/ui-config'
9+
import { getBaseUrl } from '@/lib/core/utils/urls'
910
import { getInputFormatExample } from '@/lib/workflows/operations/deployment-utils'
1011
import { useCopilotStore } from '@/stores/panel/copilot/store'
1112
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
@@ -36,7 +37,6 @@ export class DeployApiClientTool extends BaseClientTool {
3637

3738
const action = params?.action || 'deploy'
3839

39-
// Check if workflow is already deployed
4040
const workflowId = params?.workflowId || useWorkflowRegistry.getState().activeWorkflowId
4141
const isAlreadyDeployed = workflowId
4242
? useWorkflowRegistry.getState().getWorkflowDeploymentStatus(workflowId)?.isDeployed
@@ -89,7 +89,6 @@ export class DeployApiClientTool extends BaseClientTool {
8989
getDynamicText: (params, state) => {
9090
const action = params?.action === 'undeploy' ? 'undeploy' : 'deploy'
9191

92-
// Check if workflow is already deployed
9392
const workflowId = params?.workflowId || useWorkflowRegistry.getState().activeWorkflowId
9493
const isAlreadyDeployed = workflowId
9594
? useWorkflowRegistry.getState().getWorkflowDeploymentStatus(workflowId)?.isDeployed
@@ -231,10 +230,7 @@ export class DeployApiClientTool extends BaseClientTool {
231230
}
232231

233232
if (action === 'deploy') {
234-
const appUrl =
235-
typeof window !== 'undefined'
236-
? window.location.origin
237-
: process.env.NEXT_PUBLIC_APP_URL || 'https://app.sim.ai'
233+
const appUrl = getBaseUrl()
238234
const apiEndpoint = `${appUrl}/api/workflows/${workflowId}/execute`
239235
const apiKeyPlaceholder = '$SIM_API_KEY'
240236

apps/sim/tools/http/utils.ts

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
1-
import { createLogger } from '@sim/logger'
2-
import { isTest } from '@/lib/core/config/feature-flags'
31
import { getBaseUrl } from '@/lib/core/utils/urls'
42
import { transformTable } from '@/tools/shared/table'
53
import type { TableRow } from '@/tools/types'
64

7-
const logger = createLogger('HTTPRequestUtils')
8-
95
/**
106
* Creates a set of default headers used in HTTP requests
117
* @param customHeaders Additional user-provided headers to include
@@ -30,7 +26,6 @@ export const getDefaultHeaders = (
3026
...customHeaders,
3127
}
3228

33-
// Add Host header if not provided and URL is valid
3429
if (url) {
3530
try {
3631
const hostname = new URL(url).host
@@ -57,26 +52,21 @@ export const processUrl = (
5752
pathParams?: Record<string, string>,
5853
queryParams?: TableRow[] | null
5954
): string => {
60-
// Strip any surrounding quotes
6155
if ((url.startsWith('"') && url.endsWith('"')) || (url.startsWith("'") && url.endsWith("'"))) {
6256
url = url.slice(1, -1)
6357
}
6458

65-
// Replace path parameters
6659
if (pathParams) {
6760
Object.entries(pathParams).forEach(([key, value]) => {
6861
url = url.replace(`:${key}`, encodeURIComponent(value))
6962
})
7063
}
7164

72-
// Handle query parameters
7365
if (queryParams) {
7466
const queryParamsObj = transformTable(queryParams)
7567

76-
// Verify if URL already has query params to use proper separator
7768
const separator = url.includes('?') ? '&' : '?'
7869

79-
// Build query string manually to avoid double-encoding issues
8070
const queryParts: string[] = []
8171

8272
for (const [key, value] of Object.entries(queryParamsObj)) {
@@ -92,31 +82,3 @@ export const processUrl = (
9282

9383
return url
9484
}
95-
96-
// Check if a URL needs proxy to avoid CORS/method restrictions
97-
export const shouldUseProxy = (url: string): boolean => {
98-
// Skip proxying in test environment
99-
if (isTest) {
100-
return false
101-
}
102-
103-
// Only consider proxying in browser environment
104-
if (typeof window === 'undefined') {
105-
return false
106-
}
107-
108-
try {
109-
const _urlObj = new URL(url)
110-
const currentOrigin = window.location.origin
111-
112-
// Don't proxy same-origin or localhost requests
113-
if (url.startsWith(currentOrigin) || url.includes('localhost')) {
114-
return false
115-
}
116-
117-
return true // Proxy all cross-origin requests for consistency
118-
} catch (e) {
119-
logger.warn('URL parsing failed:', e)
120-
return false
121-
}
122-
}

packages/db/scripts/deregister-sso-provider.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import { drizzle } from 'drizzle-orm/postgres-js'
1818
import postgres from 'postgres'
1919
import { ssoProvider, user } from '../schema'
2020

21-
// Simple console logger
2221
const logger = {
2322
info: (message: string, meta?: any) => {
2423
const timestamp = new Date().toISOString()
@@ -43,7 +42,6 @@ const logger = {
4342
},
4443
}
4544

46-
// Get database URL from environment
4745
const CONNECTION_STRING = process.env.POSTGRES_URL ?? process.env.DATABASE_URL
4846
if (!CONNECTION_STRING) {
4947
console.error('❌ POSTGRES_URL or DATABASE_URL environment variable is required')
@@ -88,15 +86,13 @@ async function deregisterSSOProvider(): Promise<boolean> {
8886
return false
8987
}
9088

91-
// Get user
9289
const targetUser = await getUser(userEmail)
9390
if (!targetUser) {
9491
return false
9592
}
9693

9794
logger.info(`Found user: ${targetUser.email} (ID: ${targetUser.id})`)
9895

99-
// Get SSO providers for this user
10096
const providers = await db
10197
.select()
10298
.from(ssoProvider)
@@ -112,11 +108,9 @@ async function deregisterSSOProvider(): Promise<boolean> {
112108
logger.info(` - Provider ID: ${provider.providerId}, Domain: ${provider.domain}`)
113109
}
114110

115-
// Check if specific provider ID was requested
116111
const specificProviderId = process.env.SSO_PROVIDER_ID
117112

118113
if (specificProviderId) {
119-
// Delete specific provider
120114
const providerToDelete = providers.find((p) => p.providerId === specificProviderId)
121115
if (!providerToDelete) {
122116
logger.error(`Provider '${specificProviderId}' not found for user ${targetUser.email}`)
@@ -133,7 +127,6 @@ async function deregisterSSOProvider(): Promise<boolean> {
133127
`✅ Successfully deleted SSO provider '${specificProviderId}' for user ${targetUser.email}`
134128
)
135129
} else {
136-
// Delete all providers for this user
137130
await db.delete(ssoProvider).where(eq(ssoProvider.userId, targetUser.id))
138131

139132
logger.info(
@@ -171,7 +164,6 @@ async function main() {
171164
}
172165
}
173166

174-
// Handle script execution
175167
main().catch((error) => {
176168
logger.error('Script execution failed:', { error })
177169
process.exit(1)

0 commit comments

Comments
 (0)