Skip to content

Commit decbcba

Browse files
committed
fix(auth): address review on SSO registration, link start errors, and migration 0448
- Keep the SSO plugin's parsed registration body, as before 1.7. - Compare issuers exactly when carrying over a token auth method. - Surface linkSocial start errors in the connect hook and desktop launcher. - Accept whitespace around WantAssertionsSigned in migration 0448 and the script. - The register script refuses custom SP metadata that contradicts its signing flag and prints the stored SAML ACS URL.
1 parent 29b171e commit decbcba

6 files changed

Lines changed: 36 additions & 16 deletions

File tree

‎apps/sim/app/desktop/connect/connect-launcher.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export function ConnectLauncher({ providerId, completeUrl }: ConnectLauncherProp
3333
setError(null)
3434
try {
3535
const scopes = getPerRequestOAuthLinkScopes(providerId)
36-
await client.linkSocial({
36+
const result = await client.linkSocial({
3737
provider: providerId,
3838
callbackURL: completeUrl,
3939
...(scopes && { scopes }),
@@ -44,6 +44,9 @@ export function ConnectLauncher({ providerId, completeUrl }: ConnectLauncherProp
4444
// that the complete page can't read — so it would look like success.
4545
errorCallbackURL: completeUrl,
4646
})
47+
if (result.error) {
48+
setError(getErrorMessage(result.error.message, 'Could not start the connection.'))
49+
}
4750
} catch (err) {
4851
setError(getErrorMessage(err, 'Could not start the connection.'))
4952
}

‎apps/sim/hooks/queries/oauth/connections.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createLogger } from '@sim/logger'
2+
import { getErrorMessage } from '@sim/utils/errors'
23
import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
34
import { requestJson } from '@/lib/api/client/request'
45
import {
@@ -205,11 +206,19 @@ export function useConnectOAuthService() {
205206
}
206207

207208
const scopes = getPerRequestOAuthLinkScopes(providerId)
208-
await client.linkSocial({
209+
const result = await client.linkSocial({
209210
provider: providerId,
210211
callbackURL: stateCallbackUrl.toString(),
211212
...(scopes && { scopes }),
212213
})
214+
if (result.error) {
215+
throw new Error(
216+
getErrorMessage(
217+
result.error.message,
218+
result.error.statusText || 'Failed to start OAuth connection'
219+
)
220+
)
221+
}
213222

214223
return { success: true }
215224
},

‎apps/sim/lib/auth/sso/application/provider-registration.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,7 @@ export const saveSsoProvider = defineOrganizationConfigurationUseCase({
329329
clientSecret = secret
330330
}
331331
const existingTokenMethod =
332-
existing && trimTrailingSlashes(existing.issuer) === trimTrailingSlashes(issuer)
333-
? stored.tokenEndpointAuthentication
334-
: undefined
332+
existing?.issuer === issuer ? stored.tokenEndpointAuthentication : undefined
335333

336334
const oidcConfig: NonNullable<SsoProviderConfig['oidcConfig']> = {
337335
clientId,
@@ -679,7 +677,7 @@ export const saveSsoProvider = defineOrganizationConfigurationUseCase({
679677
providerConfig.oidcConfig.clientSecret = secret
680678
}
681679
if (
682-
trimTrailingSlashes(existing.issuer) === trimTrailingSlashes(issuer) &&
680+
existing.issuer === providerConfig.issuer &&
683681
(current.tokenEndpointAuthentication === 'client_secret_basic' ||
684682
current.tokenEndpointAuthentication === 'client_secret_post')
685683
)

‎apps/sim/lib/auth/sso/provider-repository.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,12 @@ export function createSsoProviderRepository(
202202
}
203203

204204
return {
205-
async register(body: SsoProviderConfig) {
206-
plugin.endpoints.registerSSOProvider.options.body.parse(body)
205+
async register(input: SsoProviderConfig) {
206+
// Keep the parser's output so its defaults apply; its declared output type is untyped.
207+
const body: SsoProviderConfig = {
208+
...input,
209+
...plugin.endpoints.registerSSOProvider.options.body.parse(input),
210+
}
207211
if (body.organizationId !== organizationId)
208212
throw new APIError('BAD_REQUEST', {
209213
message: 'Provider organization does not match its authorized scope',

‎packages/db/migrations/0448_sso_saml_signin_authority.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ BEGIN
4949
END IF;
5050

5151
IF config->'wantAssertionsSigned' = 'true'::jsonb
52-
AND coalesce(config #>> '{spMetadata,metadata}', '') !~ '\sWantAssertionsSigned=(["''])(true|1)\1' THEN
52+
AND coalesce(config #>> '{spMetadata,metadata}', '') !~ '\sWantAssertionsSigned\s*=\s*(["''])(true|1)\1' THEN
5353
updated := jsonb_set(updated, '{wantAssertionsSigned}', 'false'::jsonb);
5454
END IF;
5555

‎packages/db/scripts/register-sso-provider.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,15 @@ function buildSSOConfigFromEnv(): SSOProviderConfig | null {
270270
const wantAssertionsSigned = process.env.SSO_SAML_WANT_ASSERTIONS_SIGNED === 'true'
271271

272272
let spMetadata = process.env.SSO_SAML_SP_METADATA
273+
if (
274+
spMetadata &&
275+
wantAssertionsSigned &&
276+
!/\sWantAssertionsSigned\s*=\s*(["'])(true|1)\1/.test(spMetadata)
277+
) {
278+
throw new Error(
279+
'SSO_SAML_SP_METADATA must declare WantAssertionsSigned="true" when SSO_SAML_WANT_ASSERTIONS_SIGNED=true'
280+
)
281+
}
273282
if (!spMetadata) {
274283
spMetadata = `<?xml version="1.0" encoding="UTF-8"?>
275284
<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="${escapeXml(appBaseUrl)}">
@@ -726,13 +735,10 @@ async function registerSSOProvider(): Promise<boolean> {
726735

727736
const baseUrl =
728737
process.env.NEXT_PUBLIC_APP_URL || process.env.BETTER_AUTH_URL || 'https://your-domain.com'
729-
const callbackPath =
730-
ssoConfig.providerType === 'saml'
731-
? `api/auth/sso/saml2/sp/acs/${ssoConfig.providerId}`
732-
: `api/auth/sso/callback/${ssoConfig.providerId}`
733-
logger.info(
734-
`📋 Callback URL (configure this in your identity provider): ${baseUrl}/${callbackPath}`
735-
)
738+
const callbackUrl =
739+
ssoConfig.samlConfig?.callbackUrl ??
740+
`${baseUrl}/api/auth/sso/callback/${ssoConfig.providerId}`
741+
logger.info(`📋 Callback URL (configure this in your identity provider): ${callbackUrl}`)
736742

737743
return true
738744
} catch (error) {

0 commit comments

Comments
 (0)