Skip to content

Commit 2318480

Browse files
committed
fix(sso): stop persisting generated IdP metadata so SAML cert rotation works
The route stored an IdP metadata document built from cert + entryPoint even when the admin supplied none. The form loads that document back into its optional metadata field and resends it, and on the next save it wins over the certificate — so rotating a SAML signing certificate through the form appeared to succeed and changed nothing. Only metadata the admin actually pasted is persisted now. With none stored, Better Auth's createIdP builds the IdP from issuer, entryPoint and cert, which are the fields the form edits. No SAML providers exist in production, so this changes no live tenant.
1 parent 833932a commit 2318480

2 files changed

Lines changed: 56 additions & 25 deletions

File tree

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,51 @@ describe('POST /api/auth/sso/register', () => {
356356
expect(sent.samlConfig).toHaveProperty('identifierFormat', '')
357357
})
358358

359+
/**
360+
* Persisting generated IdP metadata made re-saving destructive: the form loaded
361+
* it back, resent it, and it then won over the certificate — so rotating a SAML
362+
* cert through the form silently did nothing.
363+
*/
364+
it('does not persist generated IdP metadata when the admin supplied none', async () => {
365+
queueMembers([{ organizationId: 'org1', role: 'owner' }])
366+
queueProviders([])
367+
await POST(
368+
request({
369+
providerType: 'saml',
370+
providerId: 'acme-saml',
371+
issuer: 'https://idp.acme.com',
372+
domain: 'acme.com',
373+
orgId: 'org1',
374+
entryPoint: 'https://idp.acme.com/sso',
375+
cert: 'ORIGINAL-CERT',
376+
})
377+
)
378+
const sent = mockRegisterSSOProvider.mock.calls[0][0].body
379+
expect(sent.samlConfig.idpMetadata).toBeUndefined()
380+
expect(sent.samlConfig.cert).toBe('ORIGINAL-CERT')
381+
})
382+
383+
it('persists IdP metadata the admin did supply', async () => {
384+
queueMembers([{ organizationId: 'org1', role: 'owner' }])
385+
queueProviders([])
386+
await POST(
387+
request({
388+
providerType: 'saml',
389+
providerId: 'acme-saml',
390+
issuer: 'https://idp.acme.com',
391+
domain: 'acme.com',
392+
orgId: 'org1',
393+
entryPoint: 'https://idp.acme.com/sso',
394+
cert: 'CERT',
395+
idpMetadata: '<EntityDescriptor>supplied</EntityDescriptor>',
396+
})
397+
)
398+
const sent = mockRegisterSSOProvider.mock.calls[0][0].body
399+
expect(sent.samlConfig.idpMetadata).toEqual({
400+
metadata: '<EntityDescriptor>supplied</EntityDescriptor>',
401+
})
402+
})
403+
359404
it('nests the attribute mapping inside oidcConfig (Better Auth reads it there)', async () => {
360405
queueMembers([{ organizationId: 'org1', role: 'owner' }])
361406
await POST(

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

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -514,40 +514,26 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
514514
</md:SPSSODescriptor>
515515
</md:EntityDescriptor>`
516516

517-
const certBase64 = cert
518-
.replace(/-----BEGIN CERTIFICATE-----/g, '')
519-
.replace(/-----END CERTIFICATE-----/g, '')
520-
.replace(/\s/g, '')
521-
522-
const computedIdpMetadataXml =
523-
idpMetadata ||
524-
`<?xml version="1.0"?>
525-
<EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" entityID="${escapeXml(issuer)}">
526-
<IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
527-
<KeyDescriptor use="signing">
528-
<ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
529-
<ds:X509Data>
530-
<ds:X509Certificate>${certBase64}</ds:X509Certificate>
531-
</ds:X509Data>
532-
</ds:KeyInfo>
533-
</KeyDescriptor>
534-
<SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="${escapeXml(entryPoint)}"/>
535-
<SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="${escapeXml(entryPoint)}"/>
536-
</IDPSSODescriptor>
537-
</EntityDescriptor>`
538-
539517
const samlConfig: any = {
540518
entryPoint,
541519
cert,
542520
callbackUrl: computedCallbackUrl,
543521
spMetadata: {
544522
metadata: spMetadataXml,
545523
},
546-
idpMetadata: {
547-
metadata: computedIdpMetadataXml,
548-
},
549524
}
550525

526+
/**
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
533+
* actually edits.
534+
*/
535+
if (idpMetadata) samlConfig.idpMetadata = { metadata: idpMetadata }
536+
551537
if (audience) samlConfig.audience = audience
552538
if (wantAssertionsSigned !== undefined) samlConfig.wantAssertionsSigned = wantAssertionsSigned
553539
if (signatureAlgorithm) samlConfig.signatureAlgorithm = signatureAlgorithm

0 commit comments

Comments
 (0)