diff --git a/internal/api/api.go b/internal/api/api.go index 8caa068888..6c26fe5571 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -354,6 +354,15 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne r.With(api.limitHandler(api.limiterOpts.SAMLAssertion)). Post("/acs", api.SamlAcs) }) + + // Dedicated endpoints for SSO providers with an isolated scope, + // whose ACS URL and entity ID are scoped to their own ID. + r.Route("/{sso_provider_id}/saml", func(r *router) { + r.Get("/metadata", api.SAMLMetadata) + + r.With(api.limitHandler(api.limiterOpts.SAMLAssertion)). + Post("/acs", api.SamlAcs) + }) }) r.Route("/admin", func(r *router) { diff --git a/internal/api/saml.go b/internal/api/saml.go index bba7605eda..5eb762b259 100644 --- a/internal/api/saml.go +++ b/internal/api/saml.go @@ -12,12 +12,51 @@ import ( "github.com/crewjam/saml" "github.com/crewjam/saml/samlsp" + "github.com/go-chi/chi/v5" + "github.com/gofrs/uuid" + "github.com/supabase/auth/internal/api/apierrors" + "github.com/supabase/auth/internal/models" ) +// loadSAMLDedicatedScopeID reads the optional {sso_provider_id} route +// parameter used by the per-provider SAML metadata and ACS endpoints. It +// returns nil when the request was made against the shared, non-scoped +// endpoint. When a provider ID is present, the provider must exist and have +// dedicated scope enabled, otherwise it doesn't have a dedicated endpoint. +func (a *API) loadSAMLDedicatedScopeID(r *http.Request) (*uuid.UUID, error) { + idParam := chi.URLParam(r, "sso_provider_id") + if idParam == "" { + return nil, nil + } + + id, err := uuid.FromString(idParam) + if err != nil { + return nil, apierrors.NewNotFoundError(apierrors.ErrorCodeSSOProviderNotFound, "No such SSO provider") + } + + ssoProvider, err := models.FindSSOProviderByID(a.db.WithContext(r.Context()), id) + if models.IsNotFoundError(err) { + return nil, apierrors.NewNotFoundError(apierrors.ErrorCodeSSOProviderNotFound, "No such SSO provider") + } else if err != nil { + return nil, err + } + + if !ssoProvider.HasDedicatedScope() { + return nil, apierrors.NewNotFoundError(apierrors.ErrorCodeSSOProviderNotFound, "SSO provider does not use a dedicated scope") + } + + return &ssoProvider.ID, nil +} + // 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 { +// +// When dedicatedScopeID is not nil, the ACS URL (and, by extension, the +// entity ID GoTrue presents to the IdP, which defaults to its metadata URL) +// are scoped to that SSO provider's ID instead of using the URLs shared by +// every SAML provider. The same key/cert pair is still used. +func (a *API) newSAMLServiceProvider(identityProvider *saml.EntityDescriptor, idpInitiated bool, key *rsa.PrivateKey, cert *x509.Certificate, dedicatedScopeID *uuid.UUID) *saml.ServiceProvider { raw := a.config.SAML.ExternalURL if raw == "" { raw = a.config.API.ExternalURL @@ -30,6 +69,9 @@ func (a *API) newSAMLServiceProvider(identityProvider *saml.EntityDescriptor, id u.Path += "/" } u.Path += "sso/" + if dedicatedScopeID != nil { + u.Path += dedicatedScopeID.String() + "/" + } provider := samlsp.DefaultServiceProvider(samlsp.Options{ URL: *u, @@ -47,7 +89,12 @@ func (a *API) newSAMLServiceProvider(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.newSAMLServiceProvider(nil, true, a.config.SAML.RSAPrivateKey, a.config.SAML.Certificate) + dedicatedScopeID, err := a.loadSAMLDedicatedScopeID(r) + if err != nil { + return err + } + + serviceProvider := a.newSAMLServiceProvider(nil, true, a.config.SAML.RSAPrivateKey, a.config.SAML.Certificate, dedicatedScopeID) metadata := serviceProvider.Metadata() diff --git a/internal/api/samlacs.go b/internal/api/samlacs.go index e0a35df85d..0fbd72a761 100644 --- a/internal/api/samlacs.go +++ b/internal/api/samlacs.go @@ -11,6 +11,7 @@ import ( "github.com/crewjam/saml" "github.com/fatih/structs" + "github.com/go-chi/chi/v5" "github.com/gofrs/uuid" "github.com/supabase/auth/internal/api/apierrors" "github.com/supabase/auth/internal/api/provider" @@ -74,6 +75,7 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { entityId := "" initiatedBy := "" redirectTo := "" + peekDestination := "" var requestIds []string var flowState *models.FlowState @@ -149,6 +151,7 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { initiatedBy = "idp" entityId = peekResponse.Issuer.Value + peekDestination = peekResponse.Destination redirectTo = relayStateValue } else { // RelayState can't be identified, so SAML flow can't continue @@ -173,6 +176,21 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { return err } + // The scope this request arrived on is taken from the URL path alone, so + // that the ACS URL used below for cryptographic validation matches + // exactly what the IdP posted to. Whether ssoProvider (looked up above + // using an unauthenticated Issuer/entity ID peeked from the raw XML) is + // actually allowed to use that scope is checked only after the assertion + // has been cryptographically validated, further down. + var dedicatedScopeID *uuid.UUID + if routeScopeIDParam := chi.URLParam(r, "sso_provider_id"); routeScopeIDParam != "" { + routeScopeID, err := uuid.FromString(routeScopeIDParam) + if err != nil { + return apierrors.NewNotFoundError(apierrors.ErrorCodeSAMLIdPNotFound, "SSO Provider does not have a SAML connection at this ACS URL") + } + dedicatedScopeID = &routeScopeID + } + samlMetadataModified := false if ssoProvider.SAMLProvider.MetadataURL == nil { @@ -199,7 +217,28 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { } } - serviceProvider := a.newSAMLServiceProvider(idpMetadata, initiatedBy == "idp", config.SAML.RSAPrivateKey, config.SAML.Certificate) + serviceProvider := a.newSAMLServiceProvider(idpMetadata, initiatedBy == "idp", config.SAML.RSAPrivateKey, config.SAML.Certificate, dedicatedScopeID) + + // SECURITY: ACS confusion check. The SAML spec makes Destination + // optional on a Response (many real IdPs omit it, especially for + // IdP-initiated flows), so an empty Destination is not itself a + // violation and must not block the flow. But when an IdP does declare + // one, it must name the exact ACS URL of the endpoint this request was + // actually POSTed to -- whether that's the global, shared ACS URL or a + // provider's dedicated-scope one. The global ACS endpoint is not a + // catch-all for every provider's assertions: without this check, a + // Response built for (or replayed against) one endpoint could otherwise + // be accepted on another, relying solely on crewjam/saml's internal + // SubjectConfirmationData.Recipient check (nested inside the assertion, + // enforced later in ParseResponse) to catch the mismatch. That check is + // unconditional and would in fact already catch this once Recipient is + // present, but we assert the invariant explicitly and up front here, as + // our own first-class defense, rather than depending entirely on a + // third-party library's internal behavior. + if initiatedBy == "idp" && peekDestination != "" && peekDestination != serviceProvider.AcsURL.String() { + return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "SAML Response Destination does not match this SSO Provider's ACS URL") + } + spAssertion, err := serviceProvider.ParseResponse(r, requestIds) if err != nil { // During a key-rotation window the IdP may have cached the old (next) @@ -210,7 +249,7 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { // 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) + fallbackSP := a.newSAMLServiceProvider(idpMetadata, initiatedBy == "idp", config.SAML.RSAPrivateKeyNext, config.SAML.CertificateNext, dedicatedScopeID) if spAssertion2, retryErr := fallbackSP.ParseResponse(r, requestIds); retryErr == nil { spAssertion = spAssertion2 err = nil @@ -226,6 +265,15 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { } } + // Only now that the assertion has been cryptographically validated do we + // check that ssoProvider is actually allowed to use the scope this + // request arrived on. This must not happen earlier, since until this + // point ssoProvider was resolved using unauthenticated data (the Issuer + // peeked from the raw, unverified XML in the IdP-initiated case). + if (dedicatedScopeID != nil) != ssoProvider.HasDedicatedScope() || (dedicatedScopeID != nil && *dedicatedScopeID != ssoProvider.ID) { + return apierrors.NewNotFoundError(apierrors.ErrorCodeSAMLIdPNotFound, "SSO Provider does not have a SAML connection at this ACS URL") + } + assertion := SAMLAssertion{ spAssertion, } diff --git a/internal/api/samlacs_test.go b/internal/api/samlacs_test.go index ba385cea1d..072b1e5cd8 100644 --- a/internal/api/samlacs_test.go +++ b/internal/api/samlacs_test.go @@ -5,6 +5,7 @@ import ( tst "testing" "github.com/crewjam/saml" + "github.com/gofrs/uuid" "github.com/stretchr/testify/require" ) @@ -20,11 +21,11 @@ func TestNewSAMLServiceProvider(t *tst.T) { require.NotNil(t, cfg.RSAPrivateKeyNext) require.NotNil(t, cfg.CertificateNext) - primary := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKey, cfg.Certificate) + primary := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKey, cfg.Certificate, nil) 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) + next := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKeyNext, cfg.CertificateNext, nil) require.Same(t, cfg.RSAPrivateKeyNext, next.Key) require.Equal(t, cfg.CertificateNext.Raw, next.Certificate.Raw) @@ -93,8 +94,8 @@ func TestNewSAMLServiceProvider_SameURLBase(t *tst.T) { cfg := api.config.SAML - primary := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKey, cfg.Certificate) - withKey := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKeyNext, cfg.CertificateNext) + primary := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKey, cfg.Certificate, nil) + withKey := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKeyNext, cfg.CertificateNext, nil) // ACS URLs must be the same — rotation must not change the endpoint. primaryACS := acsURLsFrom(primary) @@ -103,6 +104,35 @@ func TestNewSAMLServiceProvider_SameURLBase(t *tst.T) { "ACS URLs must be identical regardless of which key is used") } +// TestNewSAMLServiceProvider_DedicatedScope verifies that passing a dedicated +// scope ID scopes the ACS URL (and therefore the entity ID, which defaults +// to the metadata URL) to that ID, while the key/cert pair is unaffected. +func TestNewSAMLServiceProvider_DedicatedScope(t *tst.T) { + api, err := newSAMLTestAPI(t, samlTestPrimaryKey, samlTestNextKey, false) + require.NoError(t, err) + + cfg := api.config.SAML + + scopeID, err := uuid.NewV4() + require.NoError(t, err) + + global := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKey, cfg.Certificate, nil) + scoped := api.newSAMLServiceProvider(nil, false, cfg.RSAPrivateKey, cfg.Certificate, &scopeID) + + globalACS := acsURLsFrom(global) + scopedACS := acsURLsFrom(scoped) + + require.NotEqual(t, globalACS, scopedACS, "dedicated scope must change the ACS URL") + for _, u := range scopedACS { + require.Contains(t, u, scopeID.String(), "scoped ACS URL must contain the provider ID") + } + + require.NotEqual(t, global.Metadata().EntityID, scoped.Metadata().EntityID, + "dedicated scope must also change the entity ID presented to the IdP") + + require.Equal(t, scoped.Key, global.Key, "the signing key must be unchanged by isolated scope") +} + // acsURLsFrom collects all AssertionConsumerService Location strings from a SP. func acsURLsFrom(sp *saml.ServiceProvider) []string { var urls []string diff --git a/internal/api/sso.go b/internal/api/sso.go index d0d4c47f7c..2f8cb59828 100644 --- a/internal/api/sso.go +++ b/internal/api/sso.go @@ -105,7 +105,12 @@ 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.newSAMLServiceProvider(entityDescriptor, false /* <- idpInitiated */, a.config.SAML.RSAPrivateKey, a.config.SAML.Certificate) + var dedicatedScopeID *uuid.UUID + if ssoProvider.HasDedicatedScope() { + dedicatedScopeID = &ssoProvider.ID + } + + serviceProvider := a.newSAMLServiceProvider(entityDescriptor, false /* <- idpInitiated */, a.config.SAML.RSAPrivateKey, a.config.SAML.Certificate, dedicatedScopeID) authnRequest, err := serviceProvider.MakeAuthenticationRequest( serviceProvider.GetSSOBindingLocation(saml.HTTPRedirectBinding), diff --git a/internal/api/sso_test.go b/internal/api/sso_test.go index bb67753671..6cca08fba9 100644 --- a/internal/api/sso_test.go +++ b/internal/api/sso_test.go @@ -401,6 +401,49 @@ func (ts *SSOTestSuite) TestAdminCreateSSOProvider() { } } +func (ts *SSOTestSuite) TestAdminCreateSSOProviderWithDedicatedScope() { + body, err := json.Marshal(map[string]interface{}{ + "type": "saml", + "metadata_xml": validSAMLIDPMetadata("https://accounts.google.com/o/saml2?idpid=EXAMPLE-DEDICATED-SCOPE"), + "dedicated_scope": true, + }) + require.NoError(ts.T(), err) + + req := httptest.NewRequest(http.MethodPost, "http://localhost/admin/sso/providers", bytes.NewBuffer(body)) + req.Header.Set("Authorization", "Bearer "+ts.AdminJWT) + w := httptest.NewRecorder() + + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusCreated, w.Code) + + response, err := io.ReadAll(w.Body) + require.NoError(ts.T(), err) + + var provider struct { + ID string `json:"id"` + SAML struct { + DedicatedScope bool `json:"dedicated_scope"` + } `json:"saml"` + } + require.NoError(ts.T(), json.Unmarshal(response, &provider)) + require.True(ts.T(), provider.SAML.DedicatedScope) + + // dedicated_scope cannot be changed after creation, even to the same value + updateBody, err := json.Marshal(map[string]interface{}{ + "dedicated_scope": true, + }) + require.NoError(ts.T(), err) + + req = httptest.NewRequest(http.MethodPut, "http://localhost/admin/sso/providers/"+provider.ID, bytes.NewBuffer(updateBody)) + req.Header.Set("Authorization", "Bearer "+ts.AdminJWT) + w = httptest.NewRecorder() + + ts.API.handler.ServeHTTP(w, req) + + require.Equal(ts.T(), http.StatusBadRequest, w.Code) +} + func (ts *SSOTestSuite) TestAdminUpdateSSOProvider() { providers := []struct { ID string @@ -489,6 +532,13 @@ func (ts *SSOTestSuite) TestAdminUpdateSSOProvider() { }, }, }, + { + ID: providers[1].ID, + Status: http.StatusBadRequest, // dedicated_scope cannot be changed after creation + Request: map[string]interface{}{ + "dedicated_scope": true, + }, + }, } for _, example := range examples { diff --git a/internal/api/ssoadmin.go b/internal/api/ssoadmin.go index 5f0c73741a..403d7379a1 100644 --- a/internal/api/ssoadmin.go +++ b/internal/api/ssoadmin.go @@ -87,8 +87,9 @@ type CreateSSOProviderParams struct { AttributeMapping models.SAMLAttributeMapping `json:"attribute_mapping"` NameIDFormat string `json:"name_id_format"` - ResourceID *string `json:"resource_id,omitempty"` - Disabled *bool `json:"disabled,omitempty"` + ResourceID *string `json:"resource_id,omitempty"` + Disabled *bool `json:"disabled,omitempty"` + DedicatedScope *bool `json:"dedicated_scope,omitempty"` } func (p *CreateSSOProviderParams) validate(forUpdate bool) error { @@ -126,6 +127,10 @@ func (p *CreateSSOProviderParams) validate(forUpdate bool) error { }, ", ")) } + if forUpdate && p.DedicatedScope != nil { + return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "dedicated_scope cannot be changed after the SSO provider is created, as doing so changes the entity ID presented to the Identity Provider") + } + return nil } @@ -249,6 +254,9 @@ func (a *API) adminSSOProvidersCreate(w http.ResponseWriter, r *http.Request) er if params.Disabled != nil { provider.Disabled = params.Disabled } + if params.DedicatedScope != nil { + provider.SAMLProvider.DedicatedScope = params.DedicatedScope + } if params.MetadataURL != "" { provider.SAMLProvider.MetadataURL = ¶ms.MetadataURL } @@ -414,6 +422,9 @@ func (a *API) adminSSOProvidersUpdate(w http.ResponseWriter, r *http.Request) er } } + // dedicated_scope cannot be changed on update; validate() above already + // rejects the request if params.DedicatedScope is set. + if modified { if err := db.Transaction(func(tx *storage.Connection) error { if terr := tx.Eager().Update(provider); terr != nil { diff --git a/internal/models/sso.go b/internal/models/sso.go index 3a5be7d973..ea51533f7c 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -31,6 +31,13 @@ func (p SSOProvider) IsEnabled() bool { return p.Disabled == nil || !*p.Disabled } +// HasDedicatedScope reports whether this provider's SAML ACS URL and entity +// ID are scoped to its own provider ID, instead of using the ones shared by +// all SAML providers in this project. It still uses the same signing key. +func (p SSOProvider) HasDedicatedScope() bool { + return p.SAMLProvider.HasDedicatedScope() +} + func (p SSOProvider) TableName() string { return "sso_providers" } @@ -129,10 +136,23 @@ type SAMLProvider struct { NameIDFormat *string `db:"name_id_format" json:"name_id_format,omitempty"` + // DedicatedScope, once set, cannot be changed: flipping it changes the + // entity ID this SP presents to the IdP (it defaults to the SP metadata + // URL, which becomes scoped to the provider's ID), so toggling it after + // the IdP has been configured would break the connection. + DedicatedScope *bool `db:"dedicated_scope" json:"dedicated_scope,omitempty"` + CreatedAt time.Time `db:"created_at" json:"-"` UpdatedAt time.Time `db:"updated_at" json:"-"` } +// HasDedicatedScope reports whether this provider's SAML ACS URL and entity +// ID are scoped to its own provider ID, instead of using the ones shared by +// all SAML providers in this project. It still uses the same signing key. +func (p SAMLProvider) HasDedicatedScope() bool { + return p.DedicatedScope != nil && *p.DedicatedScope +} + func (p SAMLProvider) TableName() string { return "saml_providers" } diff --git a/migrations/20261030120000_add_dedicated_scope_to_saml_providers.up.sql b/migrations/20261030120000_add_dedicated_scope_to_saml_providers.up.sql new file mode 100644 index 0000000000..60fcd715bb --- /dev/null +++ b/migrations/20261030120000_add_dedicated_scope_to_saml_providers.up.sql @@ -0,0 +1,7 @@ +/* auth_migration: 20261030120000 */ +do $$ begin + + alter table only {{ index .Options "Namespace" }}.saml_providers + add column if not exists dedicated_scope boolean null; + +end $$; diff --git a/openapi.yaml b/openapi.yaml index 41c20d1dde..5851d4bc25 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -2143,6 +2143,10 @@ paths: format: hostname attribute_mapping: $ref: "#/components/schemas/SAMLAttributeMappingSchema" + dedicated_scope: + type: boolean + description: > + When `true`, this provider's SAML ACS URL and entity ID are scoped to its own provider ID instead of the ones shared by all SAML providers in this project. The same signing key is still used. This cannot be changed once the provider is created. responses: 200: description: SSO provider was created. @@ -2192,7 +2196,7 @@ paths: put: summary: Update details about a SSO provider. description: > - You can only update only one of `metadata_url` or `metadata_xml` at once. The SAML Metadata represented by these updates must advertize the same Identity Provider EntityID. Do not include the `domains` or `attribute_mapping` property to keep the existing database values. + You can only update only one of `metadata_url` or `metadata_xml` at once. The SAML Metadata represented by these updates must advertize the same Identity Provider EntityID. Do not include the `domains` or `attribute_mapping` property to keep the existing database values. `dedicated_scope` cannot be changed once the provider is created, and this request fails if it is included. tags: - admin security: @@ -3665,6 +3669,10 @@ components: type: string attribute_mapping: $ref: "#/components/schemas/SAMLAttributeMappingSchema" + dedicated_scope: + type: boolean + description: > + When `true`, this provider's SAML ACS URL and entity ID are scoped to its own provider ID instead of the ones shared by all SAML providers in this project. The same signing key is still used. This cannot be changed once the provider is created. AccessTokenResponseSchema: type: object