Skip to content

Commit e9949e9

Browse files
committed
fix(sso): always write SAML IdP metadata so clearing it takes effect on update
Not storing generated metadata fixed new providers but not existing ones: Better Auth merges SAML config with `??`, so omitting the key let a previously stored document survive and keep overriding the certificate. The key is now always written, empty when the admin supplied none. createIdP falsy-guards it and falls back to issuer/entryPoint/cert, so clearing the field actually clears it.
1 parent 2318480 commit e9949e9

2 files changed

Lines changed: 13 additions & 9 deletions

File tree

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ describe('POST /api/auth/sso/register', () => {
361361
* it back, resent it, and it then won over the certificate — so rotating a SAML
362362
* cert through the form silently did nothing.
363363
*/
364-
it('does not persist generated IdP metadata when the admin supplied none', async () => {
364+
it('writes empty IdP metadata when the admin supplied none, so a stored one clears', async () => {
365365
queueMembers([{ organizationId: 'org1', role: 'owner' }])
366366
queueProviders([])
367367
await POST(
@@ -376,7 +376,9 @@ describe('POST /api/auth/sso/register', () => {
376376
})
377377
)
378378
const sent = mockRegisterSSOProvider.mock.calls[0][0].body
379-
expect(sent.samlConfig.idpMetadata).toBeUndefined()
379+
// Written as empty rather than omitted: Better Auth merges with `??`, so an
380+
// omitted key would retain a previously stored document on update.
381+
expect(sent.samlConfig.idpMetadata).toEqual({ metadata: '' })
380382
expect(sent.samlConfig.cert).toBe('ORIGINAL-CERT')
381383
})
382384

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -524,15 +524,17 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
524524
}
525525

526526
/**
527-
* Only persist IdP metadata the admin actually supplied. Storing a document
528-
* generated from `cert` + `entryPoint` made re-saving destructive: the form
529-
* loads it back into the optional metadata field, resends it, and it then
530-
* wins over the certificate — so a cert rotation entered through the form
531-
* silently did nothing. With no metadata stored, Better Auth's createIdP
532-
* builds the IdP from issuer/entryPoint/cert, which are the fields the form
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
533535
* actually edits.
534536
*/
535-
if (idpMetadata) samlConfig.idpMetadata = { metadata: idpMetadata }
537+
samlConfig.idpMetadata = { metadata: idpMetadata ?? '' }
536538

537539
if (audience) samlConfig.audience = audience
538540
if (wantAssertionsSigned !== undefined) samlConfig.wantAssertionsSigned = wantAssertionsSigned

0 commit comments

Comments
 (0)