From e81307c38594148a3d4926fbcc1a90e043f1c01e Mon Sep 17 00:00:00 2001 From: Cemal Kilic Date: Wed, 10 Jun 2026 13:45:38 +0200 Subject: [PATCH 1/3] feat(saml): add PrivateKeyNext support for zero-downtime key rotation --- internal/conf/configuration.go | 1 + internal/conf/saml.go | 63 ++++++++++++++++++++++++++- internal/conf/saml_test.go | 79 ++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 1 deletion(-) diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 9b0ec63ba4..f6e8712a8f 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -962,6 +962,7 @@ func (config *GlobalConfiguration) PopulateGlobal() error { if err := config.SAML.PopulateFields(config.API.ExternalURL); err != nil { return err } + config.SAML.PrivateKeyNext = "" } else { config.SAML.PrivateKey = "" } diff --git a/internal/conf/saml.go b/internal/conf/saml.go index 9f0ec1ce15..8a49ef29dd 100644 --- a/internal/conf/saml.go +++ b/internal/conf/saml.go @@ -17,6 +17,7 @@ import ( type SAMLConfiguration struct { Enabled bool `json:"enabled"` PrivateKey string `json:"-" split_words:"true"` + PrivateKeyNext string `json:"-" split_words:"true"` AllowEncryptedAssertions bool `json:"allow_encrypted_assertions" split_words:"true"` RelayStateValidityPeriod time.Duration `json:"relay_state_validity_period" split_words:"true"` @@ -24,6 +25,10 @@ type SAMLConfiguration struct { RSAPublicKey *rsa.PublicKey `json:"-"` Certificate *x509.Certificate `json:"-"` + RSAPrivateKeyNext *rsa.PrivateKey `json:"-"` + RSAPublicKeyNext *rsa.PublicKey `json:"-"` + CertificateNext *x509.Certificate `json:"-"` + ExternalURL string `json:"external_url,omitempty" split_words:"true"` RateLimitAssertion float64 `default:"15" split_words:"true"` @@ -57,6 +62,26 @@ func (c *SAMLConfiguration) Validate() error { return errors.New("SAML private key must be at least RSA 2048") } + if c.PrivateKeyNext != "" { + nextBytes, err := base64.StdEncoding.DecodeString(c.PrivateKeyNext) + if err != nil { + return errors.New("SAML next private key not in standard Base64 format") + } + + nextKey, err := x509.ParsePKCS1PrivateKey(nextBytes) + if err != nil { + return errors.New("SAML next private key not in PKCS#1 format") + } + + if nextKey.E != 0x10001 { + return errors.New("SAML next private key should use the 65537 (0x10001) RSA public exponent") + } + + if nextKey.N.BitLen() < 2048 { + return errors.New("SAML next private key must be at least RSA 2048") + } + } + if c.RelayStateValidityPeriod < 0 { return errors.New("SAML RelayState validity period should be a positive duration") } @@ -79,7 +104,15 @@ func (c *SAMLConfiguration) PopulateFields(externalURL string) error { if err != nil { return err } - return c.createCertificate(certTemplate) + if err := c.createCertificate(certTemplate); err != nil { + return err + } + if c.PrivateKeyNext != "" { + if err := c.populateNextKey(certTemplate); err != nil { + return err + } + } + return nil } // PopulateFields fills the configuration details based off the provided @@ -159,3 +192,31 @@ func (c *SAMLConfiguration) parseCertificateDer(certDer []byte) error { } return nil } + +func (c *SAMLConfiguration) populateNextKey(certTemplate *x509.Certificate) error { + bytes, err := base64.StdEncoding.DecodeString(c.PrivateKeyNext) + if err != nil { + return fmt.Errorf("saml: PopulateFields: next key invalid base64: %w", err) + } + + privateKey, err := x509.ParsePKCS1PrivateKey(bytes) + if err != nil { + return fmt.Errorf("saml: PopulateFields: next key not in PKCS#1 format: %w", err) + } + + c.RSAPrivateKeyNext = privateKey + c.RSAPublicKeyNext = privateKey.Public().(*rsa.PublicKey) + + certDer, err := x509.CreateCertificate(nil, certTemplate, certTemplate, c.RSAPublicKeyNext, c.RSAPrivateKeyNext) + if err != nil { + return err + } + + cert, err := x509.ParseCertificate(certDer) + if err != nil { + return err + } + + c.CertificateNext = cert + return nil +} diff --git a/internal/conf/saml_test.go b/internal/conf/saml_test.go index 24657a9345..154560d5a5 100644 --- a/internal/conf/saml_test.go +++ b/internal/conf/saml_test.go @@ -193,6 +193,75 @@ func TestSAMLConfigurationValidate(t *testing.T) { } } +func TestSAMLConfigurationNextKey(t *testing.T) { + const externalURL = "https://projectref.supabase.co" + + t.Run("Absent", func(t *testing.T) { + c := &SAMLConfiguration{ + Enabled: true, + PrivateKey: validPrivateKey, + } + require.NoError(t, c.PopulateFields(externalURL)) + require.Nil(t, c.CertificateNext) + require.Nil(t, c.RSAPrivateKeyNext) + require.Nil(t, c.RSAPublicKeyNext) + }) + + t.Run("Valid", func(t *testing.T) { + c := &SAMLConfiguration{ + Enabled: true, + PrivateKey: validPrivateKey, + PrivateKeyNext: validPrivateKeyNext, + } + require.NoError(t, c.PopulateFields(externalURL)) + require.NotNil(t, c.RSAPrivateKeyNext) + require.NotNil(t, c.RSAPublicKeyNext) + require.NotNil(t, c.CertificateNext) + require.NotEqual(t, c.Certificate.Raw, c.CertificateNext.Raw, "next cert should be distinct from primary cert") + }) + + t.Run("SameAsCurrent", func(t *testing.T) { + c := &SAMLConfiguration{ + Enabled: true, + PrivateKey: validPrivateKey, + PrivateKeyNext: validPrivateKey, + } + require.NoError(t, c.Validate()) + require.NoError(t, c.PopulateFields(externalURL)) + require.NotNil(t, c.CertificateNext) + }) +} + +func TestSAMLConfigurationValidateNextKey(t *testing.T) { + invalidExamples := []*SAMLConfiguration{ + { + Enabled: true, + PrivateKey: validPrivateKey, + PrivateKeyNext: "InvalidBase64!", + }, + { + Enabled: true, + PrivateKey: validPrivateKey, + PrivateKeyNext: base64.StdEncoding.EncodeToString([]byte("not PKCS#1")), + }, + { + Enabled: true, + PrivateKey: validPrivateKey, + PrivateKeyNext: invalidRSA1024Key, + }, + { + Enabled: true, + PrivateKey: validPrivateKey, + PrivateKeyNext: invalidWrongExponentKey, + }, + } + + for i, example := range invalidExamples { + err := example.Validate() + require.Error(t, err, "Invalid next key example %d was regarded as valid", i) + } +} + func TestSAMLConfigurationDeterministicCertificate(t *testing.T) { a := &SAMLConfiguration{ Enabled: true, @@ -215,4 +284,14 @@ func TestSAMLConfigurationDeterministicCertificate(t *testing.T) { const ( validPrivateKey = "MIIEowIBAAKCAQEAsBuxTUWFrfy0qYXaqNSeVWcJOd6TQ4+4b/3N4p/58r1d/kMU+K+BGR+tF0GKHGYngTF6puvNDff2wgW3dp3LUSMjxOhC3sK0uL90vd+IR6v1EDDGLyQNo6EjP/x5Gp/PcL2s6hZb8iLBEq4FksPnEhWqf9Nsmgf1YPJV4AvaaWe3oBFo9zJobSs3etTVitc3qEH2DpgYFtrCKhMWv5qoZtZTyZRE3LU3rvInDgYw6HDGF1G4y4Fvah6VpRmTdyMR81r1tCLmGvk61QJp7i4HteazQ6Raqh2EZ1sH/UfEp8mrwYRaRdgLDQ/Q6/YlO8NTQwzp6YwwAybhMBnOrABLCQIDAQABAoIBADqobq0DPByQsIhKmmNjtn1RvYP1++0kANXknuAeUv2kT5tyMpkGtCRvJZM6dEszR3NDzMuufPVrI1jK2Kn8sw0KfE6I4kUaa2Gh+7uGqfjdcNn8tPZctuJKuNgGOzxAALNXqjGqUuPa6Z5UMm0JLX0blFfRTzoa7oNlFG9040H6CRjJQQGfYyPS8xeo+RUR009sK/222E5jz6ThIiCrOU/ZGm5Ws9y3AAIASqJd9QPy7qxKoFZ1qKZ/cDaf1txCKq9VBXH6ypZoU1dQibhyLCIJ3tYapBtV4p8V12oHhITXb6Vbo1P9bQSVz+2rQ0nJkjdXX/N4aHE01ecbu8MpMxUCgYEA5P4ZCAdpkTaOSJi7GyL4AcZ5MN26eifFnRO/tbmw07f6vi//vdqzC9T7kxmZ8e1OvhX5OMGNb3nsXm78WgS2EVLTkaTInG6XhlOeYj9BHAQZDBr7rcAxrVQxVgaGDiZpYun++kXw+39iq3gxuYuC9mM0AQze3SjTRIM9WWXJSqMCgYEAxODfXcWMk2P/WfjE3u+8fhjc3cvqyWSyThEZC9YzpN59dL73SE7BRkMDyZO19fFvVO9mKsRfsTio0ceC5XQOO6hUxAm4gAEvMpeapQgXTxIxF5FAQ0vGmBMxT+xg7lX8HTTJX/UCttKo3BdIJQeTf8bKVzJCoLFh8Rcv5qI6umMCgYAEuj44DTcfuVmcpBKQz9sA5mEQIjO8W9/Xi1XU4Z2F8XFqxcDo4X/6yY3cDpZACV8ry3ZWtqA94e2AUZhCH4DGwMf/ZMCDgkD8k/NcIeQtOORvfIsfni0oX+mY1g+kcSSR1zTdY95CwvF9isC0DO5KOegT8XkUZchezLrSgqhyMwKBgQCvS0mWRH6V/UMu6MDhfrNl0t1U3mt+RZo8yBx03ZO+CBvMBvxF9VlBJgoJQOuSwBVQmpdtHMvXD4vAvNNfWaYSmB5hLgaIcoWDlliq+DlIvfnX8gw13xJD9VLCxsTHcOe5WXazaYOxJIAU9uXVkplR+73NRYLtcQKzluGfiHKh4QKBgFpPtOqcAbkMsV+1qPYvvvX7E4+l52Odb4tbxGBYV8tzCqMRETqMPVxFWwsj+EQ8lyAu15rCRH7DKHVK5zL6JvIZEjt0tptKqSL2o3ovS6y3DmD6t+YpvjKME7a+vunOoJWe9pWl3wZmodfyZMpAdDLvDGhPR7Jlhun41tbMMaQF" + + // A second valid RSA 2048 key used to test the "next key" rotation feature. + // This is the key used in TestSAMLConfigurationDeterministicCertificate. + validPrivateKeyNext = "MIIEowIBAAKCAQEAt7dS8iM5MsQ+1mVkNpoaUnL8BCdxSrSx8jsSnvqN/GIJ4ipqbdrTgLpFVklVTqfaa5CykGVEV577l6AWkpkm2p7SvSkCQglmyAMMjY9glmztytAnfBpm+cQ6ZVTHC4XKlUG1aJigEuXPcZUU3FiBHWEuV2huYy2bLOtIY1v9N0i2v61QCdG+SM/Yb5t86KzApRl7VyHqquge6vvRuchfF0msv/2LW32hwxg3Gt4zkAF0SJqCCcfAPZ9pQwmbdUhoX16dRFU98nyIvuR8LH/wONZe/YyywFFHDEwkFa4XEzjCEm+AD+xvK7eEu55w21xB8JKMLEBy8uRuI3bIEG4pawIDAQABAoIBADw4IT4xgYw8e4R3U7P6K2qfOjB6ZU5hkHqgFmh6JJR35ll2IdDEi9OEOzofa5EOwC/GDGH8b7xw5nM7DGsdPHko2lca3BydTE1/glvchYKJTiDOvkKVvO9d/O4+Lch/IHpwQXB5pu7K2YaXoXDgqeHhevk3yAdGabj9norDGmtGIeU/x1hialKbw6L080CdbxpjeAsM/w+G/VtwvyOKYFBYxBflRW+sS8UeclVqKRAvaXKd1JGleWzH3hFZyFI54x5LyyjPI1JyVXRjNbf8xcS6eRaN849grL1+wBxEs/lQFn4JLhAcNi912iJ3lhxvkNleXZw7B7JAM8x4wUbK7zECgYEA6SYmu3YH8XuLUfT8MMCp+ETjPkNMOJGQmTXOkW6zuXP3J8iCPIxtuz09cGIro+yJU23yPUzOVCDZMmnMWBmkoTKAFoFL9TX0Eyqn/t1MD77i3NdkMp16yI5fwOO6yX1bZgLiG00W2E5/IGgNfTtEafU/mre95JBnTgxS3sAvz8UCgYEAybjfBVt+1X0vSVAGKYHI9wtzoSx3dIGE8G5LIchPTdNDZ0ke0QCRffhyCGKy6bPos0P2z5nLgWSePBPZQowpwZiQVXdWE05ID641E2zGULdYL1yVHDt6tVTpSzTAy89BiS1G8HvgpQyaBTmvmF11Fyd/YbrDxEIHN+qQdDkM928CgYEA4lJ4ksz21QF6sqpADQtZc3lbplspqFgVp8RFq4Nsz3+00lefpSskcff2phuGBXBdtjEqTzs5pwzkCj4NcRAjcZ9WG4KTu4sOTXTA83TamwZPrtUfnMqmH/2lEdd+wI0BpjryRlJE9ODuIwUe4wwfU0QQ5B2tJizPO0JXR4gEYYkCgYBzqidm4QGm1DLq7JG79wkObmiMv/x2t1VMr1ExO7QNQdfiP1EGMjc6bdyk5kMEMf5527yHaP4BYXpBpHfs6oV+1kXcW6LlSvuS0iboznQgECDmd0WgfJJtqxRh5QuvUVWYnHeSqNU0jjc6S8tdqCjdb+5gUUCzJdERxNOzcIr4zQKBgAqcBQwlWy0PdlZ06JhJUYlwX1pOU8mWPz9LIF0wrSm9LEtAl37zZJaD3uscvk/fCixAGHOktkDGVO7aUYIAlX9iD49huGkeRTn9tz7Wanw6am04Xj0y7H1oPPV7k5nJ4s9AOWq/gkZEhrRIis2anAczsx1YHSjq/M05+AbuRzvs" + + // RSA 1024 key — too short, used to verify rejection. + invalidRSA1024Key = "MIICXQIBAAKBgQDFa3SgzWZpcoONv3Iq3FxNieks2u2TmykxxxeggI9aNpHpuCzwGQO8wqXGVvFNlkE3GSPcz7rklzfyj577Z47lfWdBP1OAefralA3tS2mafqpZ32JwDynX4as+xauLVdP4iOR96b3L2eOb6rDpr4wBJuNqO533xsjcbNPINEDkSwIDAQABAoGASggBtEtSHDjVHFKufWQlOO5+glOWw8Nrrz75nTaYizvre7mVIHRA8ogLolT4KCAwVHkY+bTsYMxULqGs/JnY+40suHECYQ2u76PTQlvJnhJANGtCxuV4lSK6B8QBJhjGExsnAOwMMKz0p5kVftx2GA+/Rz2De7DR9keNECjcAAECQQDtr5cdkEdnIffvi782843EvX/g8615AKZeUYVUl0gVXujjpIVZXDtytPHINvIW1Z2mOm2rlJukwiKYYJ8IjsxlAkEA1KGbJ9EI6AOUcnpy7FYdGkbINTDngCqVOoaddlHS+1SaofpYXZPueXXIqIG3viksxmq/Q0IY6+JRkGo/RpGq7wJARD+BAqok9oYYbR4RX7P7ZxyKlYsiqnX3T2nVAP8XYZuI/6SD7a7AGyW9ryGnzcq0o8BvMS9QqbRcvqgvwgNOyQJBAL2ZVMaOSIjKGGZz9WHz74Nstj1n3CWW0vYa7vGASMc/S5s/pefbbvvzIPfQo0z3XiuXJ/ELUTmU1vIVK1L7tRUCQQCsuE7xckZ8H/523jdWWy9zZVP1z4c5dVLDR5RY+YQNForgb6kkSv4Gzn/FRUOxqn2MEWJLla31D4EuS+XKuwZR" + + // RSA 2048 with public exponent 0x11 instead of 0x10001 — used to verify exponent check. + invalidWrongExponentKey = "MIIEowIBAAKCAQEAyMvTanPoiorCpIQCl70qXF34FIPOkKaInr1vw+3/0nik5CDUo761E02uTrK4/8JXr5NLGmy/fQmagNsBOdKewciRB3xxs+sPNncptG4rpCBjxSJdVl+mYZaw2kdvFY7TvNTlr7qG1Q0kV/3lBgpMlyM9OqBrjuG0UUzB5hlg08KLNflkQAkoJGWNVWULi2VceP3I3QsH9uNUQkgaM9Z6rl0BaRAkobHTTvquAqqj1AlNmSh24rrIbV4hYcNnesIpG4+LDd8XfpOwTp+jUl8akF6xcRBJjiPDJGN9ety29DcCxjo2i0b+TWYU+Pex08uOeOdulsgecbIVxLUEgRHcFQIBEQKCAQBefgkjCV5fUFuYtpfO75t2ws8Ytn9TITE7pHDUrDwmz1ynlvqnaM2uuyTZvYQ8HzhSn6rfQjv+mxuH7pcqRP9qQEQ/whdjuekKkm36jjKnlsWJ8g3OSyEe3YBmuDRGYVSVGOSO7l2Rb5ih4OQ/E+fOpyvfWoz38b5EYFs/GwBjpgJG+9cdCLYKOax8WDifWkjHdrogAlE8do/QF6RZoSvhAbRkpuxYActmKU8rIORrq8dLidSjBG2aoRH+RCN4ONZ3R4iHbYF2zWfqDFdSIX64kChaOZVhtTyTnF7/1v4VF3UwByEs8hTSckFH2jW6T7RZoatpgsv5zx/roRPDBWNRAoGBAPGphQwX9GF56XVmqD9rQMD9a0FuGCNGgiFkt2OUTFKmr4rTNVE8uJqEXjQwbybTnF2iJ1ApL1zNHg0cpnpMt7ZpcWG4Bu2UsXlwBL/ZwY4Spk6tHNdTsg/wuoWRSIGNanNS6CI5EUA4cxGNUt0G+dF4LaMHZuIAU7avs+kwDMzHAoGBANS1nS8KYkPUwYlmgVPNhMDTtjvq7fgP5UFDXnlhE6rJidc/+B0p9WiRhLGWlZebn+h2fELfIgK3qc4IzCHOkar0pic2D3bNbboNQKnqFl81hg0EORTK0JJ5/K4J61l5+rZtQu3Ss1HVwDiy9SKg6F3CQj9PK0r+hjtAStFSmZxDAoGBAMcEEzciyUE3OLsJP0NJRGKylJA8jFlJH99D4lIBqEQQzMyt76xQH45O5Cr6teO9U5hna6ttNhAwcxnbW+w/LeGEAwUuI9K2sEXjx60NrnUATLlDRO2QOElc1ddolhBWV6pERrLFlbxquR2DcWq6c2E1yzr3CW7TF8OfwVagCoqFAoGBAK8sJxeuMs5y+bxyiJ9d9NsItDFYD0TBy9tkqCe5W32W6fyPCJB86Df/XjflbCKAKVYHOSgDDPMt1yIlXNCL/326arbhOeld4eSDYm3P1jBKMijWTSAujaXN3yXqDRyCkjvhgmmAV3CR6Zga5/5mZQHrRZ2MfgGGUG0HxSTanJ7NAoGBAOhZBGtFsBdtEawvCh4Z8NaMC2nU+Ru9hEsZSy6rZQrPvja9aBUk5QUdh04TYtu8PzQ1EghZy71rtwDAvxXWJ1mWcZn0kD06tZKudmZpMVXCp3SFah6DDUCFSmQ2U60yh6XOzpS2+Z97Ngi02UFph8sSQA6Dl/lmaf4bfQHCYc5Z" ) From 7e3f4e3f8bebc4c8cb70f500a52c5e9847b49836 Mon Sep 17 00:00:00 2001 From: Cemal Kilic Date: Wed, 10 Jun 2026 14:14:07 +0200 Subject: [PATCH 2/3] feat: use certificatenext --- internal/api/saml.go | 33 +++++++++++- internal/api/saml_test.go | 109 +++++++++++++++++++++++++++++++++++++- internal/api/settings.go | 22 ++++---- internal/conf/saml.go | 6 +++ 4 files changed, 158 insertions(+), 12 deletions(-) diff --git a/internal/api/saml.go b/internal/api/saml.go index 1e8b9a7427..21a600ad7a 100644 --- a/internal/api/saml.go +++ b/internal/api/saml.go @@ -1,6 +1,7 @@ package api import ( + "encoding/base64" "encoding/xml" "net/http" "net/url" @@ -92,16 +93,46 @@ func (a *API) SAMLMetadata(w http.ResponseWriter, r *http.Request) error { } } + // During key rotation, advertise the next certificate so IdPs can + // cache it before we promote it to primary. + if a.config.SAML.CertificateNext != nil { + nextCertData := base64.StdEncoding.EncodeToString(a.config.SAML.CertificateNext.Raw) + nextKD := saml.KeyDescriptor{ + Use: "signing", + KeyInfo: saml.KeyInfo{ + X509Data: saml.X509Data{ + X509Certificates: []saml.X509Certificate{ + {Data: nextCertData}, + }, + }, + }, + } + keyDescriptors = append(keyDescriptors, nextKD) + if a.config.SAML.AllowEncryptedAssertions { + encKD := nextKD + encKD.Use = "encryption" + keyDescriptors = append(keyDescriptors, encKD) + } + } + spd.KeyDescriptors = keyDescriptors } + // Reduce cache aggressiveness during key rotation so IdPs and CDNs pick + // up the updated metadata before we promote the next key. + cacheControl := "public, max-age=600" + if a.config.SAML.CertificateNext != nil { + metadata.CacheDuration = time.Hour + cacheControl = "public, max-age=60" + } + metadataXML, err := xml.Marshal(metadata) if err != nil { return err } w.Header().Set("Content-Type", "application/xml") - w.Header().Set("Cache-Control", "public, max-age=600") // cache at CDN for 10 minutes + w.Header().Set("Cache-Control", cacheControl) if r.FormValue("download") == "true" { w.Header().Set("Content-Disposition", "attachment; filename=\"metadata.xml\"") diff --git a/internal/api/saml_test.go b/internal/api/saml_test.go index cb40b4022d..40d4d9e24b 100644 --- a/internal/api/saml_test.go +++ b/internal/api/saml_test.go @@ -13,12 +13,18 @@ import ( "github.com/supabase/auth/internal/conf/confload" ) +// samlTestPrimaryKey is an RSA 2048 key used as the primary SAML signing key in tests. +const samlTestPrimaryKey = "MIIEowIBAAKCAQEAszrVveMQcSsa0Y+zN1ZFb19cRS0jn4UgIHTprW2tVBmO2PABzjY3XFCfx6vPirMAPWBYpsKmXrvm1tr0A6DZYmA8YmJd937VUQ67fa6DMyppBYTjNgGEkEhmKuszvF3MARsIKCGtZqUrmS7UG4404wYxVppnr2EYm3RGtHlkYsXu20MBqSDXP47bQP+PkJqC3BuNGk3xt5UHl2FSFpTHelkI6lBynw16B+lUT1F96SERNDaMqi/TRsZdGe5mB/29ngC/QBMpEbRBLNRir5iUevKS7Pn4aph9Qjaxx/97siktK210FJT23KjHpgcUfjoQ6BgPBTLtEeQdRyDuc/CgfwIDAQABAoIBAGYDWOEpupQPSsZ4mjMnAYJwrp4ZISuMpEqVAORbhspVeb70bLKonT4IDcmiexCg7cQBcLQKGpPVM4CbQ0RFazXZPMVq470ZDeWDEyhoCfk3bGtdxc1Zc9CDxNMs6FeQs6r1beEZug6weG5J/yRn/qYxQife3qEuDMl+lzfl2EN3HYVOSnBmdt50dxRuX26iW3nqqbMRqYn9OHuJ1LvRRfYeyVKqgC5vgt/6Tf7DAJwGe0dD7q08byHV8DBZ0pnMVU0bYpf1GTgMibgjnLjK//EVWafFHtN+RXcjzGmyJrk3+7ZyPUpzpDjO21kpzUQLrpEkkBRnmg6bwHnSrBr8avECgYEA3pq1PTCAOuLQoIm1CWR9/dhkbJQiKTJevlWV8slXQLR50P0WvI2RdFuSxlWmA4xZej8s4e7iD3MYye6SBsQHygOVGc4efvvEZV8/XTlDdyj7iLVGhnEmu2r7AFKzy8cOvXx0QcLg+zNd7vxZv/8D3Qj9Jje2LjLHKM5n/dZ3RzUCgYEAzh5Lo2anc4WN8faLGt7rPkGQF+7/18ImQE11joHWa3LzAEy7FbeOGpE/vhOv5umq5M/KlWFIRahMEQv4RusieHWI19ZLIP+JwQFxWxS+cPp3xOiGcquSAZnlyVSxZ//dlVgaZq2o2MfrxECcovRlaknl2csyf+HjFFwKlNxHm2MCgYAr//R3BdEy0oZeVRndo2lr9YvUEmu2LOihQpWDCd0fQw0ZDA2kc28eysL2RROte95r1XTvq6IvX5a0w11FzRWlDpQ4J4/LlcQ6LVt+98SoFwew+/PWuyLmxLycUbyMOOpm9eSc4wJJZNvaUzMCSkvfMtmm5jgyZYMMQ9A2Ul/9SQKBgB9mfh9mhBwVPIqgBJETZMMXOdxrjI5SBYHGSyJqpT+5Q0vIZLfqPrvNZOiQFzwWXPJ+tV4Mc/YorW3rZOdo6tdvEGnRO6DLTTEaByrY/io3/gcBZXoSqSuVRmxleqFdWWRnB56c1hwwWLqNHU+1671FhL6pNghFYVK4suP6qu4BAoGBAMk+VipXcIlD67mfGrET/xDqiWWBZtgTzTMjTpODhDY1GZck1eb4CQMP5j5V3gFJ4cSgWDJvnWg8rcz0unz/q4aeMGl1rah5WNDWj1QKWMS6vJhMHM/rqN1WHWR0ZnV83svYgtg0zDnQKlLujqW4JmGXLMU7ur6a+e6lpa1fvLsP" + +// samlTestNextKey is a second RSA 2048 key used to test next-key rotation in tests. +const samlTestNextKey = "MIIEowIBAAKCAQEAt7dS8iM5MsQ+1mVkNpoaUnL8BCdxSrSx8jsSnvqN/GIJ4ipqbdrTgLpFVklVTqfaa5CykGVEV577l6AWkpkm2p7SvSkCQglmyAMMjY9glmztytAnfBpm+cQ6ZVTHC4XKlUG1aJigEuXPcZUU3FiBHWEuV2huYy2bLOtIY1v9N0i2v61QCdG+SM/Yb5t86KzApRl7VyHqquge6vvRuchfF0msv/2LW32hwxg3Gt4zkAF0SJqCCcfAPZ9pQwmbdUhoX16dRFU98nyIvuR8LH/wONZe/YyywFFHDEwkFa4XEzjCEm+AD+xvK7eEu55w21xB8JKMLEBy8uRuI3bIEG4pawIDAQABAoIBADw4IT4xgYw8e4R3U7P6K2qfOjB6ZU5hkHqgFmh6JJR35ll2IdDEi9OEOzofa5EOwC/GDGH8b7xw5nM7DGsdPHko2lca3BydTE1/glvchYKJTiDOvkKVvO9d/O4+Lch/IHpwQXB5pu7K2YaXoXDgqeHhevk3yAdGabj9norDGmtGIeU/x1hialKbw6L080CdbxpjeAsM/w+G/VtwvyOKYFBYxBflRW+sS8UeclVqKRAvaXKd1JGleWzH3hFZyFI54x5LyyjPI1JyVXRjNbf8xcS6eRaN849grL1+wBxEs/lQFn4JLhAcNi912iJ3lhxvkNleXZw7B7JAM8x4wUbK7zECgYEA6SYmu3YH8XuLUfT8MMCp+ETjPkNMOJGQmTXOkW6zuXP3J8iCPIxtuz09cGIro+yJU23yPUzOVCDZMmnMWBmkoTKAFoFL9TX0Eyqn/t1MD77i3NdkMp16yI5fwOO6yX1bZgLiG00W2E5/IGgNfTtEafU/mre95JBnTgxS3sAvz8UCgYEAybjfBVt+1X0vSVAGKYHI9wtzoSx3dIGE8G5LIchPTdNDZ0ke0QCRffhyCGKy6bPos0P2z5nLgWSePBPZQowpwZiQVXdWE05ID641E2zGULdYL1yVHDt6tVTpSzTAy89BiS1G8HvgpQyaBTmvmF11Fyd/YbrDxEIHN+qQdDkM928CgYEA4lJ4ksz21QF6sqpADQtZc3lbplspqFgVp8RFq4Nsz3+00lefpSskcff2phuGBXBdtjEqTzs5pwzkCj4NcRAjcZ9WG4KTu4sOTXTA83TamwZPrtUfnMqmH/2lEdd+wI0BpjryRlJE9ODuIwUe4wwfU0QQ5B2tJizPO0JXR4gEYYkCgYBzqidm4QGm1DLq7JG79wkObmiMv/x2t1VMr1ExO7QNQdfiP1EGMjc6bdyk5kMEMf5527yHaP4BYXpBpHfs6oV+1kXcW6LlSvuS0iboznQgECDmd0WgfJJtqxRh5QuvUVWYnHeSqNU0jjc6S8tdqCjdb+5gUUCzJdERxNOzcIr4zQKBgAqcBQwlWy0PdlZ06JhJUYlwX1pOU8mWPz9LIF0wrSm9LEtAl37zZJaD3uscvk/fCixAGHOktkDGVO7aUYIAlX9iD49huGkeRTn9tz7Wanw6am04Xj0y7H1oPPV7k5nJ4s9AOWq/gkZEhrRIis2anAczsx1YHSjq/M05+AbuRzvs" + func TestSAMLMetadataWithAPI(t *tst.T) { config, err := confload.LoadGlobal(apiTestConfig) require.NoError(t, err) config.API.ExternalURL = "https://projectref.supabase.co/auth/v1/" config.SAML.Enabled = true - config.SAML.PrivateKey = "MIIEowIBAAKCAQEAszrVveMQcSsa0Y+zN1ZFb19cRS0jn4UgIHTprW2tVBmO2PABzjY3XFCfx6vPirMAPWBYpsKmXrvm1tr0A6DZYmA8YmJd937VUQ67fa6DMyppBYTjNgGEkEhmKuszvF3MARsIKCGtZqUrmS7UG4404wYxVppnr2EYm3RGtHlkYsXu20MBqSDXP47bQP+PkJqC3BuNGk3xt5UHl2FSFpTHelkI6lBynw16B+lUT1F96SERNDaMqi/TRsZdGe5mB/29ngC/QBMpEbRBLNRir5iUevKS7Pn4aph9Qjaxx/97siktK210FJT23KjHpgcUfjoQ6BgPBTLtEeQdRyDuc/CgfwIDAQABAoIBAGYDWOEpupQPSsZ4mjMnAYJwrp4ZISuMpEqVAORbhspVeb70bLKonT4IDcmiexCg7cQBcLQKGpPVM4CbQ0RFazXZPMVq470ZDeWDEyhoCfk3bGtdxc1Zc9CDxNMs6FeQs6r1beEZug6weG5J/yRn/qYxQife3qEuDMl+lzfl2EN3HYVOSnBmdt50dxRuX26iW3nqqbMRqYn9OHuJ1LvRRfYeyVKqgC5vgt/6Tf7DAJwGe0dD7q08byHV8DBZ0pnMVU0bYpf1GTgMibgjnLjK//EVWafFHtN+RXcjzGmyJrk3+7ZyPUpzpDjO21kpzUQLrpEkkBRnmg6bwHnSrBr8avECgYEA3pq1PTCAOuLQoIm1CWR9/dhkbJQiKTJevlWV8slXQLR50P0WvI2RdFuSxlWmA4xZej8s4e7iD3MYye6SBsQHygOVGc4efvvEZV8/XTlDdyj7iLVGhnEmu2r7AFKzy8cOvXx0QcLg+zNd7vxZv/8D3Qj9Jje2LjLHKM5n/dZ3RzUCgYEAzh5Lo2anc4WN8faLGt7rPkGQF+7/18ImQE11joHWa3LzAEy7FbeOGpE/vhOv5umq5M/KlWFIRahMEQv4RusieHWI19ZLIP+JwQFxWxS+cPp3xOiGcquSAZnlyVSxZ//dlVgaZq2o2MfrxECcovRlaknl2csyf+HjFFwKlNxHm2MCgYAr//R3BdEy0oZeVRndo2lr9YvUEmu2LOihQpWDCd0fQw0ZDA2kc28eysL2RROte95r1XTvq6IvX5a0w11FzRWlDpQ4J4/LlcQ6LVt+98SoFwew+/PWuyLmxLycUbyMOOpm9eSc4wJJZNvaUzMCSkvfMtmm5jgyZYMMQ9A2Ul/9SQKBgB9mfh9mhBwVPIqgBJETZMMXOdxrjI5SBYHGSyJqpT+5Q0vIZLfqPrvNZOiQFzwWXPJ+tV4Mc/YorW3rZOdo6tdvEGnRO6DLTTEaByrY/io3/gcBZXoSqSuVRmxleqFdWWRnB56c1hwwWLqNHU+1671FhL6pNghFYVK4suP6qu4BAoGBAMk+VipXcIlD67mfGrET/xDqiWWBZtgTzTMjTpODhDY1GZck1eb4CQMP5j5V3gFJ4cSgWDJvnWg8rcz0unz/q4aeMGl1rah5WNDWj1QKWMS6vJhMHM/rqN1WHWR0ZnV83svYgtg0zDnQKlLujqW4JmGXLMU7ur6a+e6lpa1fvLsP" + config.SAML.PrivateKey = samlTestPrimaryKey config.API.MaxRequestDuration = 5 * time.Second require.NoError(t, config.ApplyDefaults()) @@ -57,3 +63,104 @@ func TestSAMLMetadataWithAPI(t *tst.T) { require.Equal(t, metadata.SPSSODescriptors[0].NameIDFormats[0], saml.EmailAddressNameIDFormat) require.Equal(t, metadata.SPSSODescriptors[0].NameIDFormats[1], saml.PersistentNameIDFormat) } + +// newSAMLTestAPI builds a minimal API with SAML enabled using the given key(s). +func newSAMLTestAPI(t *tst.T, primaryKey, nextKey string, allowEncrypted bool) (*API, error) { + t.Helper() + config, err := confload.LoadGlobal(apiTestConfig) + if err != nil { + return nil, err + } + config.API.ExternalURL = "https://projectref.supabase.co/auth/v1/" + config.API.MaxRequestDuration = 5 * time.Second + config.SAML.Enabled = true + config.SAML.PrivateKey = primaryKey + config.SAML.PrivateKeyNext = nextKey + config.SAML.AllowEncryptedAssertions = allowEncrypted + if err := config.ApplyDefaults(); err != nil { + return nil, err + } + if err := config.SAML.PopulateFields(config.API.ExternalURL); err != nil { + return nil, err + } + return NewAPI(config, nil), nil +} + +// samlMetadataRequest issues a GET /sso/saml/metadata and parses the response. +func samlMetadataRequest(t *tst.T, api *API) (*saml.EntityDescriptor, http.Header) { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "http://localhost/sso/saml/metadata", nil) + w := httptest.NewRecorder() + api.handler.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + var metadata saml.EntityDescriptor + require.NoError(t, xml.Unmarshal(w.Body.Bytes(), &metadata)) + return &metadata, w.Header() +} + +func TestSAMLMetadata_SingleKey(t *tst.T) { + api, err := newSAMLTestAPI(t, samlTestPrimaryKey, "", false) + require.NoError(t, err) + + metadata, headers := samlMetadataRequest(t, api) + + require.Len(t, metadata.SPSSODescriptors, 1) + kds := metadata.SPSSODescriptors[0].KeyDescriptors + require.Len(t, kds, 1) + require.Equal(t, "signing", kds[0].Use) + + // Cache TTL stays at 600s when no next key is set + require.Equal(t, "public, max-age=600", headers.Get("Cache-Control")) + require.Equal(t, time.Duration(0), metadata.CacheDuration) +} + +func TestSAMLMetadata_DualKey(t *tst.T) { + api, err := newSAMLTestAPI(t, samlTestPrimaryKey, samlTestNextKey, false) + require.NoError(t, err) + + metadata, headers := samlMetadataRequest(t, api) + + require.Len(t, metadata.SPSSODescriptors, 1) + kds := metadata.SPSSODescriptors[0].KeyDescriptors + + // Exactly two signing descriptors, primary cert first + require.Len(t, kds, 2) + require.Equal(t, "signing", kds[0].Use) + require.Equal(t, "signing", kds[1].Use) + + // The two certificates must be distinct + cert0 := kds[0].KeyInfo.X509Data.X509Certificates[0].Data + cert1 := kds[1].KeyInfo.X509Data.X509Certificates[0].Data + require.NotEqual(t, cert0, cert1) + + // Cache TTL is reduced to 60s during rotation + require.Equal(t, "public, max-age=60", headers.Get("Cache-Control")) + require.Equal(t, time.Hour, metadata.CacheDuration) +} + +func TestSAMLMetadata_DualKey_Encryption(t *tst.T) { + api, err := newSAMLTestAPI(t, samlTestPrimaryKey, samlTestNextKey, true) + require.NoError(t, err) + + metadata, headers := samlMetadataRequest(t, api) + + require.Len(t, metadata.SPSSODescriptors, 1) + kds := metadata.SPSSODescriptors[0].KeyDescriptors + + // Two signing + two encryption descriptors + var signing, encryption []saml.KeyDescriptor + for _, kd := range kds { + switch kd.Use { + case "signing": + signing = append(signing, kd) + case "encryption": + encryption = append(encryption, kd) + } + } + require.Len(t, signing, 2) + require.Len(t, encryption, 2) + + require.Equal(t, "public, max-age=60", headers.Get("Cache-Control")) + require.Equal(t, time.Hour, metadata.CacheDuration) + _ = headers +} diff --git a/internal/api/settings.go b/internal/api/settings.go index be7ac504f8..7fb21a05f0 100644 --- a/internal/api/settings.go +++ b/internal/api/settings.go @@ -32,13 +32,14 @@ type ProviderSettings struct { } type Settings struct { - ExternalProviders ProviderSettings `json:"external"` - DisableSignup bool `json:"disable_signup"` - MailerAutoconfirm bool `json:"mailer_autoconfirm"` - PhoneAutoconfirm bool `json:"phone_autoconfirm"` - SmsProvider string `json:"sms_provider"` - SAMLEnabled bool `json:"saml_enabled"` - PasskeysEnabled bool `json:"passkeys_enabled"` + ExternalProviders ProviderSettings `json:"external"` + DisableSignup bool `json:"disable_signup"` + MailerAutoconfirm bool `json:"mailer_autoconfirm"` + PhoneAutoconfirm bool `json:"phone_autoconfirm"` + SmsProvider string `json:"sms_provider"` + SAMLEnabled bool `json:"saml_enabled"` + SAMLPrivateKeyNextConfigured bool `json:"saml_private_key_next_configured"` + PasskeysEnabled bool `json:"passkeys_enabled"` } func (a *API) Settings(w http.ResponseWriter, r *http.Request) error { @@ -76,8 +77,9 @@ func (a *API) Settings(w http.ResponseWriter, r *http.Request) error { DisableSignup: config.DisableSignup, MailerAutoconfirm: config.Mailer.Autoconfirm, PhoneAutoconfirm: config.Sms.Autoconfirm, - SmsProvider: config.Sms.Provider, - SAMLEnabled: config.SAML.Enabled, - PasskeysEnabled: config.Passkey.Enabled, + SmsProvider: config.Sms.Provider, + SAMLEnabled: config.SAML.Enabled, + SAMLPrivateKeyNextConfigured: config.SAML.CertificateNext != nil, + PasskeysEnabled: config.Passkey.Enabled, }) } diff --git a/internal/conf/saml.go b/internal/conf/saml.go index 8a49ef29dd..62f40bae6f 100644 --- a/internal/conf/saml.go +++ b/internal/conf/saml.go @@ -111,6 +111,12 @@ func (c *SAMLConfiguration) PopulateFields(externalURL string) error { if err := c.populateNextKey(certTemplate); err != nil { return err } + } else { + // envconfig allocates zero-value structs for nil pointer fields; reset + // them explicitly so CertificateNext == nil when no next key is set. + c.RSAPrivateKeyNext = nil + c.RSAPublicKeyNext = nil + c.CertificateNext = nil } return nil } From da5be932febdee21203ea8e86fb61fc8eda614bc Mon Sep 17 00:00:00 2001 From: Cemal Kilic Date: Fri, 12 Jun 2026 13:18:25 +0200 Subject: [PATCH 3/3] feat(saml): support zero-downtime SP key rotation --- docs/saml_key_rotation.md | 162 +++++++++++++++++++++++++++++++++++ internal/api/saml.go | 50 +++++------ internal/api/samlacs.go | 25 +++++- internal/api/samlacs_test.go | 119 +++++++++++++++++++++++++ internal/api/sso.go | 2 +- 5 files changed, 323 insertions(+), 35 deletions(-) create mode 100644 docs/saml_key_rotation.md create mode 100644 internal/api/samlacs_test.go diff --git a/docs/saml_key_rotation.md b/docs/saml_key_rotation.md new file mode 100644 index 0000000000..8232e613e5 --- /dev/null +++ b/docs/saml_key_rotation.md @@ -0,0 +1,162 @@ +# SAML SP Key Rotation Runbook + +Zero-downtime rotation of the SAML Service Provider signing (and optional encryption) key. + +--- + +## Overview + +GoTrue advertises its SP public key inside SAML metadata. Identity Providers (IdPs) cache this +metadata and use the embedded certificate to verify SP-signed AuthnRequests and (when encryption +is enabled) to encrypt assertions sent back to the SP. + +A rotation therefore has two concerns: + +1. **Signing** — IdPs must trust the new certificate before GoTrue starts signing with it. +2. **Encryption** (only when `GOTRUE_SAML_ALLOW_ENCRYPTED_ASSERTIONS=true`) — GoTrue must be + able to decrypt assertions that were encrypted with the *old* certificate while the IdP's + cache still points to it. + +Both concerns are handled automatically once you follow the steps below. + +--- + +## Prerequisites + +- Access to the GoTrue environment variables / secrets store. +- Ability to trigger a rolling restart or redeploy of GoTrue. +- `openssl` available locally (or equivalent). + +--- + +## Step 1 — Generate the new key + +```bash +# Produces a PKCS#1 DER key encoded as standard Base64 (no line breaks). +openssl genrsa 2048 | openssl rsa -outform DER | base64 | tr -d '\n' +``` + +Store the output somewhere safe (secret manager, vault). This is the **new key** value. + +> **Requirement:** RSA 2048 or larger, public exponent 65537 (the `openssl genrsa` default). + +--- + +## Step 2 — Announce the new certificate (dual-key window) + +Set the new key as the *next* key **without** touching the primary key: + +``` +GOTRUE_SAML_PRIVATE_KEY= +GOTRUE_SAML_PRIVATE_KEY_NEXT= +``` + +Redeploy / restart GoTrue. + +**What happens:** + +- Both certificates appear in SP metadata under ``. +- The primary certificate remains first, so IdPs that already trust it continue to work. +- `Cache-Control` drops to `max-age=60` and the XML `cacheDuration` is set to `PT1H` so IdPs + re-fetch metadata sooner. +- The `/settings` endpoint returns `"saml_private_key_next_configured": true`. +- If encrypted assertions are enabled, both certificates also appear as `use="encryption"` + descriptors, and GoTrue will automatically retry decryption with the old key if the primary + key fails. + +**Verify:** + +```bash +curl -s https:///auth/v1/sso/saml/metadata \ + | xmllint --xpath 'count(//md:KeyDescriptor[@use="signing"])' \ + --noout - 2>/dev/null +# Expected: 2 +``` + +--- + +## Step 3 — Wait for IdP caches to drain + +IdPs must re-fetch metadata and import the new certificate before you promote it. The safe +window is determined by the *largest* cache TTL among your IdPs. + +**Minimum wait:** 1 hour (the `cacheDuration=PT1H` advertised in metadata). + +For IdPs with longer cache windows or manual metadata import workflows, trigger a metadata +refresh in their admin console before proceeding, or contact the IdP admin to confirm the new +certificate is imported. + +Confirm the new certificate is trusted by performing a test login with an affected IdP if +possible. + +--- + +## Step 4 — Promote the new key + +Swap the values and remove `_NEXT`: + +``` +GOTRUE_SAML_PRIVATE_KEY= +GOTRUE_SAML_PRIVATE_KEY_NEXT= # remove / clear +``` + +Redeploy / restart GoTrue. + +**What happens:** + +- Metadata now advertises only the new certificate. +- `Cache-Control` returns to `max-age=600`. +- Signing switches to the new key immediately. +- If encrypted assertions are enabled, GoTrue no longer attempts the fallback decryption with + the old key (it is no longer configured). + +**Verify:** + +```bash +curl -s https:///auth/v1/sso/saml/metadata \ + | xmllint --xpath 'count(//md:KeyDescriptor[@use="signing"])' \ + --noout - 2>/dev/null +# Expected: 1 + +curl -s https:///auth/v1/settings \ + | jq '.saml_private_key_next_configured' +# Expected: false +``` + +Perform a test login to confirm end-to-end flow. + +--- + +## Encrypted assertions — additional notes + +When `GOTRUE_SAML_ALLOW_ENCRYPTED_ASSERTIONS=true`: + +- During the dual-key window (Step 2), GoTrue accepts assertions encrypted with **either** the + primary or the next (old) certificate. No action needed. +- The IdP may send assertions encrypted with the old certificate for up to the cache window + after Step 4. This is safe because the old key is gone from configuration and the IdP should + have already switched to the new certificate. If any IdP still sends assertions encrypted with + the old certificate after promotion, those assertions will fail. Contact the IdP admin to + force a metadata refresh. + +--- + +## Rollback + +| Phase | How to rollback | +|-------|----------------| +| After Step 2 (dual-key deployed, not yet promoted) | Clear `GOTRUE_SAML_PRIVATE_KEY_NEXT` and redeploy. No key material was changed at IdPs. | +| After Step 4 (new key promoted) | Restore the old key to `GOTRUE_SAML_PRIVATE_KEY`, set the new key in `GOTRUE_SAML_PRIVATE_KEY_NEXT`, redeploy. You are back to the dual-key window. Wait for IdPs to re-import the old certificate before relying on it. | + +> Avoid skipping the dual-key window. Promoting a new key before IdPs have cached the new +> certificate will break SP-initiated flows for the cache window duration. + +--- + +## Quick reference + +| Variable | Purpose | +|----------|---------| +| `GOTRUE_SAML_PRIVATE_KEY` | Active signing (and decryption) key. PKCS#1 DER, Base64-encoded. | +| `GOTRUE_SAML_PRIVATE_KEY_NEXT` | Incoming key during rotation. Advertised in metadata; used as decryption fallback. Clear after promotion. | +| `GOTRUE_SAML_ALLOW_ENCRYPTED_ASSERTIONS` | Enable encrypted assertion support. Both keys appear as `use="encryption"` descriptors when rotation is active. | diff --git a/internal/api/saml.go b/internal/api/saml.go index 21a600ad7a..bba7605eda 100644 --- a/internal/api/saml.go +++ b/internal/api/saml.go @@ -1,6 +1,8 @@ package api import ( + "crypto/rsa" + "crypto/x509" "encoding/base64" "encoding/xml" "net/http" @@ -12,39 +14,27 @@ import ( "github.com/crewjam/saml/samlsp" ) -// getSAMLServiceProvider generates a new service provider object with the -// (optionally) provided descriptor (metadata) for the identity provider. -func (a *API) getSAMLServiceProvider(identityProvider *saml.EntityDescriptor, idpInitiated bool) *saml.ServiceProvider { - var externalURL *url.URL - - if a.config.SAML.ExternalURL != "" { - url, err := url.ParseRequestURI(a.config.SAML.ExternalURL) - if err != nil { - // this should not fail as a.config should have been validated using #Validate() - panic(err) - } - - externalURL = url - } else { - url, err := url.ParseRequestURI(a.config.API.ExternalURL) - if err != nil { - // this should not fail as a.config should have been validated using #Validate() - panic(err) - } - - externalURL = url +// newSAMLServiceProvider constructs a ServiceProvider for the given IdP +// metadata, using the provided key/cert pair. Callers are responsible for +// passing the correct pair (primary or rotation fallback). +func (a *API) newSAMLServiceProvider(identityProvider *saml.EntityDescriptor, idpInitiated bool, key *rsa.PrivateKey, cert *x509.Certificate) *saml.ServiceProvider { + raw := a.config.SAML.ExternalURL + if raw == "" { + raw = a.config.API.ExternalURL } - - if !strings.HasSuffix(externalURL.Path, "/") { - externalURL.Path += "/" + u, err := url.ParseRequestURI(raw) + if err != nil { + panic(err) } - - externalURL.Path += "sso/" + if !strings.HasSuffix(u.Path, "/") { + u.Path += "/" + } + u.Path += "sso/" provider := samlsp.DefaultServiceProvider(samlsp.Options{ - URL: *externalURL, - Key: a.config.SAML.RSAPrivateKey, - Certificate: a.config.SAML.Certificate, + URL: *u, + Key: key, + Certificate: cert, SignRequest: true, AllowIDPInitiated: idpInitiated, IDPMetadata: identityProvider, @@ -57,7 +47,7 @@ func (a *API) getSAMLServiceProvider(identityProvider *saml.EntityDescriptor, id // SAMLMetadata serves GoTrue's SAML Service Provider metadata file. func (a *API) SAMLMetadata(w http.ResponseWriter, r *http.Request) error { - serviceProvider := a.getSAMLServiceProvider(nil, true) + serviceProvider := a.newSAMLServiceProvider(nil, true, a.config.SAML.RSAPrivateKey, a.config.SAML.Certificate) metadata := serviceProvider.Metadata() diff --git a/internal/api/samlacs.go b/internal/api/samlacs.go index 95d7cf336f..e0a35df85d 100644 --- a/internal/api/samlacs.go +++ b/internal/api/samlacs.go @@ -199,14 +199,31 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { } } - serviceProvider := a.getSAMLServiceProvider(idpMetadata, initiatedBy == "idp") + serviceProvider := a.newSAMLServiceProvider(idpMetadata, initiatedBy == "idp", config.SAML.RSAPrivateKey, config.SAML.Certificate) spAssertion, err := serviceProvider.ParseResponse(r, requestIds) if err != nil { - if ire, ok := err.(*saml.InvalidResponseError); ok { - return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "SAML Assertion is not valid %s", ire.Response).WithInternalError(ire.PrivateErr) + // During a key-rotation window the IdP may have cached the old (next) + // certificate from metadata and used it to encrypt the assertion. + // Retry with the old key in primary position to keep decryption working + // for the overlap period. Only attempt this when encrypted assertions + // are enabled, because only then can the IdP send encrypted payloads. + // If the retry also fails we return the original error so the operator + // sees a consistent diagnostic. + if config.SAML.AllowEncryptedAssertions && config.SAML.RSAPrivateKeyNext != nil { + fallbackSP := a.newSAMLServiceProvider(idpMetadata, initiatedBy == "idp", config.SAML.RSAPrivateKeyNext, config.SAML.CertificateNext) + if spAssertion2, retryErr := fallbackSP.ParseResponse(r, requestIds); retryErr == nil { + spAssertion = spAssertion2 + err = nil + } } - return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "SAML Assertion is not valid").WithInternalError(err) + if err != nil { + if ire, ok := err.(*saml.InvalidResponseError); ok { + return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "SAML Assertion is not valid %s", ire.Response).WithInternalError(ire.PrivateErr) + } + + return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "SAML Assertion is not valid").WithInternalError(err) + } } assertion := SAMLAssertion{ diff --git a/internal/api/samlacs_test.go b/internal/api/samlacs_test.go new file mode 100644 index 0000000000..ba385cea1d --- /dev/null +++ b/internal/api/samlacs_test.go @@ -0,0 +1,119 @@ +package api + +import ( + "crypto/x509" + tst "testing" + + "github.com/crewjam/saml" + "github.com/stretchr/testify/require" +) + +// TestNewSAMLServiceProvider verifies that newSAMLServiceProvider uses whichever +// key/cert pair it is given, and that distinct pairs produce distinct providers. +func TestNewSAMLServiceProvider(t *tst.T) { + api, err := newSAMLTestAPI(t, samlTestPrimaryKey, samlTestNextKey, false) + require.NoError(t, err) + + cfg := api.config.SAML + require.NotNil(t, cfg.RSAPrivateKey) + require.NotNil(t, cfg.Certificate) + require.NotNil(t, cfg.RSAPrivateKeyNext) + require.NotNil(t, cfg.CertificateNext) + + primary := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKey, cfg.Certificate) + require.Same(t, cfg.RSAPrivateKey, primary.Key) + require.Equal(t, cfg.Certificate.Raw, primary.Certificate.Raw) + + next := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKeyNext, cfg.CertificateNext) + require.Same(t, cfg.RSAPrivateKeyNext, next.Key) + require.Equal(t, cfg.CertificateNext.Raw, next.Certificate.Raw) + + require.NotSame(t, primary.Key, next.Key) + require.NotEqual(t, primary.Certificate.Raw, next.Certificate.Raw) +} + +// TestSAMLDecryptionFallback_NoRetryWithoutNextKey verifies that when +// RSAPrivateKeyNext is nil the retry path is not reached; a parse failure +// must return an error immediately (not panic or deadlock). +func TestSAMLDecryptionFallback_NoRetryWithoutNextKey(t *tst.T) { + // Build API with only a primary key (no next key). + api, err := newSAMLTestAPI(t, samlTestPrimaryKey, "", true) + require.NoError(t, err) + + require.Nil(t, api.config.SAML.RSAPrivateKeyNext, + "no next key should be configured") + + // The guard condition evaluated during retry must be false. + cfg := api.config.SAML + retryEligible := cfg.AllowEncryptedAssertions && cfg.RSAPrivateKeyNext != nil + require.False(t, retryEligible, + "retry must not be attempted when RSAPrivateKeyNext is nil") +} + +// TestSAMLDecryptionFallback_NoRetryWhenEncryptionDisabled verifies the guard +// condition is false when AllowEncryptedAssertions is false, even with a next +// key configured. Encrypted assertions can only be sent when the IdP knows +// the SP supports them, which requires AllowEncryptedAssertions = true in +// metadata. +func TestSAMLDecryptionFallback_NoRetryWhenEncryptionDisabled(t *tst.T) { + api, err := newSAMLTestAPI(t, samlTestPrimaryKey, samlTestNextKey, false) + require.NoError(t, err) + + cfg := api.config.SAML + require.False(t, cfg.AllowEncryptedAssertions) + require.NotNil(t, cfg.RSAPrivateKeyNext) + + retryEligible := cfg.AllowEncryptedAssertions && cfg.RSAPrivateKeyNext != nil + require.False(t, retryEligible, + "retry must not be attempted when AllowEncryptedAssertions is false") +} + +// TestSAMLDecryptionFallback_RetryCondition verifies that the guard condition +// is true when both AllowEncryptedAssertions is true AND RSAPrivateKeyNext is +// set — i.e., that the code will attempt the retry path. +func TestSAMLDecryptionFallback_RetryCondition(t *tst.T) { + api, err := newSAMLTestAPI(t, samlTestPrimaryKey, samlTestNextKey, true) + require.NoError(t, err) + + cfg := api.config.SAML + require.True(t, cfg.AllowEncryptedAssertions) + require.NotNil(t, cfg.RSAPrivateKeyNext) + + retryEligible := cfg.AllowEncryptedAssertions && cfg.RSAPrivateKeyNext != nil + require.True(t, retryEligible, + "retry should be eligible when AllowEncryptedAssertions=true and RSAPrivateKeyNext is set") +} + +// TestNewSAMLServiceProvider_SameURLBase verifies that both key pairs produce +// a ServiceProvider with the same ACS URL — wrong ACS URL would prevent +// assertion validation during the rotation fallback. +func TestNewSAMLServiceProvider_SameURLBase(t *tst.T) { + api, err := newSAMLTestAPI(t, samlTestPrimaryKey, samlTestNextKey, true) + require.NoError(t, err) + + cfg := api.config.SAML + + primary := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKey, cfg.Certificate) + withKey := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKeyNext, cfg.CertificateNext) + + // ACS URLs must be the same — rotation must not change the endpoint. + primaryACS := acsURLsFrom(primary) + withKeyACS := acsURLsFrom(withKey) + require.Equal(t, primaryACS, withKeyACS, + "ACS URLs must be identical regardless of which key is used") +} + +// acsURLsFrom collects all AssertionConsumerService Location strings from a SP. +func acsURLsFrom(sp *saml.ServiceProvider) []string { + var urls []string + meta := sp.Metadata() + for _, spd := range meta.SPSSODescriptors { + for _, acs := range spd.AssertionConsumerServices { + urls = append(urls, acs.Location) + } + } + return urls +} + +// Compile-time check: x509.Certificate.Raw is the field we compare. +var _ = (*x509.Certificate)(nil) diff --git a/internal/api/sso.go b/internal/api/sso.go index 3d50f9de0d..d0d4c47f7c 100644 --- a/internal/api/sso.go +++ b/internal/api/sso.go @@ -105,7 +105,7 @@ func (a *API) SingleSignOn(w http.ResponseWriter, r *http.Request) error { return apierrors.NewInternalServerError("Error parsing SAML Metadata for SAML provider").WithInternalError(err) } - serviceProvider := a.getSAMLServiceProvider(entityDescriptor, false /* <- idpInitiated */) + serviceProvider := a.newSAMLServiceProvider(entityDescriptor, false /* <- idpInitiated */, a.config.SAML.RSAPrivateKey, a.config.SAML.Certificate) authnRequest, err := serviceProvider.MakeAuthenticationRequest( serviceProvider.GetSSOBindingLocation(saml.HTTPRedirectBinding),