Skip to content

Commit eedc242

Browse files
committed
refactor(sso): write the two merge-sensitive SAML fields the same way
idpMetadata and identifierFormat both exist to defeat Better Auth's `??` merge, which silently keeps a stored value when a key is omitted, but they were written differently — one always, one only when defined. Both are now always written, empty when unset, under one comment explaining why and noting that each is falsy-guarded downstream. Also drops a redundant saveDisabled: false; the prop already defaults to false.
1 parent c564441 commit eedc242

2 files changed

Lines changed: 18 additions & 22 deletions

File tree

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

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -523,28 +523,25 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
523523
},
524524
}
525525

526-
/**
527-
* Persist only IdP metadata the admin actually supplied, and always write the
528-
* key so clearing it takes effect. Two failures sat here: a document generated
529-
* from `cert` + `entryPoint` used to be stored unconditionally, and the form
530-
* loads metadata back and resends it, so it won over the certificate and a
531-
* cert rotation silently did nothing. Omitting the key instead is no fix —
532-
* Better Auth merges SAML config with `??`, so a previously stored document
533-
* would survive. An empty string is written instead, which `createIdP`
534-
* falsy-guards, falling back to issuer/entryPoint/cert — the fields the form
535-
* actually edits.
536-
*/
537-
samlConfig.idpMetadata = { metadata: idpMetadata ?? '' }
538-
539526
if (audience) samlConfig.audience = audience
540527
if (wantAssertionsSigned !== undefined) samlConfig.wantAssertionsSigned = wantAssertionsSigned
541528
if (signatureAlgorithm) samlConfig.signatureAlgorithm = signatureAlgorithm
542529
if (digestAlgorithm) samlConfig.digestAlgorithm = digestAlgorithm
543-
// Forward an explicit empty string rather than dropping it: Better Auth
544-
// merges SAML config with `??`, so omitting the key would retain a
545-
// previously stored format while the caller asked for the provider default.
546-
// samlify falsy-guards nameIDFormat, so '' correctly reads as unset.
547-
if (identifierFormat !== undefined) samlConfig.identifierFormat = identifierFormat
530+
531+
/**
532+
* These two are always written, empty when unset, rather than omitted.
533+
* Better Auth merges SAML config with `??`, so an omitted key silently keeps
534+
* whatever was stored — clearing either field would never take effect. Both
535+
* are falsy-guarded downstream: `createIdP` falls back to
536+
* issuer/entryPoint/cert without metadata, and `createSP` omits nameIDFormat.
537+
*
538+
* Metadata in particular must not be generated here. Storing a document built
539+
* from cert + entryPoint made re-saving destructive, because the form loads it
540+
* back, resends it, and it then outranks the certificate — so rotating a SAML
541+
* cert appeared to succeed and changed nothing.
542+
*/
543+
samlConfig.idpMetadata = { metadata: idpMetadata ?? '' }
544+
samlConfig.identifierFormat = identifierFormat ?? ''
548545
// Better Auth reads the attribute mapping from samlConfig.mapping.
549546
if (mapping) samlConfig.mapping = mapping
550547

apps/sim/ee/sso/components/sso-settings.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -531,10 +531,9 @@ function OrganizationSsoSettings({ organizationId }: SSOProps) {
531531
...saveDiscardActions({
532532
dirty: hasChanges,
533533
saving: configureSSOMutation.isPending,
534-
// Deliberately not disabled on validation errors: showErrors is only
535-
// set by handleSubmit, so a disabled Save left the admin with a greyed
536-
// out button and no message. Clicking now reveals what is wrong.
537-
saveDisabled: false,
534+
// Never disabled on validation errors: showErrors is only set by
535+
// handleSubmit, so a disabled Save left the admin with a greyed out
536+
// button and no message. Clicking now reveals what is wrong.
538537
saveLabel: isEditing ? 'Update' : 'Save',
539538
savingLabel: isEditing ? 'Updating...' : 'Saving...',
540539
onSave: () => void handleSubmit(),

0 commit comments

Comments
 (0)