From 82ba69bdd43b0c26e03a5b207f69cbd91e0ea559 Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Tue, 15 Jul 2025 16:49:40 -0700 Subject: [PATCH 1/4] feat: add support for managing SSO providers by resource_id - Updated API for SSO providers to allow get, put, delete by `resource_id` - Extended `loadSSOProvider` to lookup by either `idp_id` or `resource_id` - Added optional `resource_id` field to `SSOProvider` model - Implemented `FindSSOProviderByResourceID` in model layer - Renamed `FindAllSAMLProviders` to `FindAllSSOProviders` - Included pagination & filtering support in and `FindSSOProviderByFilter` - Note: this is not currently being used, waiting to hear teams feedback - Included full E2E test coverage for the new `/resources/{resource_id}` admin endpoints. --- internal/api/api.go | 10 +- internal/api/ssoadmin.go | 50 +++- internal/api/ssoadmin_test.go | 480 ++++++++++++++++++++++++++++++++++ internal/models/sso.go | 61 ++++- 4 files changed, 586 insertions(+), 15 deletions(-) create mode 100644 internal/api/ssoadmin_test.go diff --git a/internal/api/api.go b/internal/api/api.go index a09d045cb6..fca5f81d39 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -292,8 +292,16 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne r.Delete("/", api.adminSSOProvidersDelete) }) }) - }) + r.Route("/resources", func(r *router) { + r.Route("/{resource_id}", func(r *router) { + r.Use(api.loadSSOProvider) + r.Get("/", api.adminSSOProvidersGet) + r.Put("/", api.adminSSOProvidersUpdate) + r.Delete("/", api.adminSSOProvidersDelete) + }) + }) + }) }) }) diff --git a/internal/api/ssoadmin.go b/internal/api/ssoadmin.go index 682423a568..1d0f16c8f7 100644 --- a/internal/api/ssoadmin.go +++ b/internal/api/ssoadmin.go @@ -19,22 +19,31 @@ import ( "github.com/supabase/auth/internal/utilities" ) -// loadSSOProvider looks for an idp_id parameter in the URL route and loads the SSO provider -// with that ID (or resource ID) and adds it to the context. +// loadSSOProvider looks for an idp_id or resource_id parameter in the URL route +// and loads the SSO provider into the the context. func (a *API) loadSSOProvider(w http.ResponseWriter, r *http.Request) (context.Context, error) { ctx := r.Context() db := a.db.WithContext(ctx) - idpParam := chi.URLParam(r, "idp_id") - - idpID, err := uuid.FromString(idpParam) - if err != nil { - // idpParam is not UUIDv4 - return nil, apierrors.NewNotFoundError(apierrors.ErrorCodeSSOProviderNotFound, "SSO Identity Provider not found") + var ( + provider *models.SSOProvider + err error + ) + switch { + case chi.URLParam(r, "idp_id") != "": + idpParam := chi.URLParam(r, "idp_id") + idpID, idpErr := uuid.FromString(idpParam) + if idpErr != nil { + return nil, apierrors.NewNotFoundError(apierrors.ErrorCodeSSOProviderNotFound, "SSO Identity Provider not found") + } + provider, err = models.FindSSOProviderByID(db, idpID) + case chi.URLParam(r, "resource_id") != "": + resourceID := chi.URLParam(r, "resource_id") + provider, err = models.FindSSOProviderByResourceID(db, resourceID) + default: + err = apierrors.NewNotFoundError(apierrors.ErrorCodeSSOProviderNotFound, "SSO Identity Provider not found") } - // idpParam is a UUIDv4 - provider, err := models.FindSSOProviderByID(db, idpID) if err != nil { if models.IsNotFoundError(err) { return nil, apierrors.NewNotFoundError(apierrors.ErrorCodeSSOProviderNotFound, "SSO Identity Provider not found") @@ -44,7 +53,6 @@ func (a *API) loadSSOProvider(w http.ResponseWriter, r *http.Request) (context.C } observability.LogEntrySetField(r, "sso_provider_id", provider.ID.String()) - return withSSOProvider(r.Context(), provider), nil } @@ -54,7 +62,7 @@ func (a *API) adminSSOProvidersList(w http.ResponseWriter, r *http.Request) erro ctx := r.Context() db := a.db.WithContext(ctx) - providers, err := models.FindAllSAMLProviders(db) + providers, err := models.FindAllSSOProviders(db) if err != nil { return err } @@ -77,6 +85,7 @@ type CreateSSOProviderParams struct { Domains []string `json:"domains"` AttributeMapping models.SAMLAttributeMapping `json:"attribute_mapping"` NameIDFormat string `json:"name_id_format"` + ResourceID *string `json:"resource_id,omitempty"` } func (p *CreateSSOProviderParams) validate(forUpdate bool) error { @@ -230,6 +239,9 @@ func (a *API) adminSSOProvidersCreate(w http.ResponseWriter, r *http.Request) er }, } + if params.ResourceID != nil { + provider.ResourceID = params.ResourceID + } if params.MetadataURL != "" { provider.SAMLProvider.MetadataURL = ¶ms.MetadataURL } @@ -374,6 +386,20 @@ func (a *API) adminSSOProvidersUpdate(w http.ResponseWriter, r *http.Request) er } } + if params.ResourceID != nil { + resourceID := *params.ResourceID + switch { + case resourceID == "" && provider.ResourceID != nil: + provider.ResourceID = nil + modified = true + case resourceID != "" && + (provider.ResourceID == nil || + *provider.ResourceID != resourceID): + provider.ResourceID = &resourceID + modified = true + } + } + if modified { if err := db.Transaction(func(tx *storage.Connection) error { if terr := tx.Eager().Update(provider); terr != nil { diff --git a/internal/api/ssoadmin_test.go b/internal/api/ssoadmin_test.go new file mode 100644 index 0000000000..5d7485210c --- /dev/null +++ b/internal/api/ssoadmin_test.go @@ -0,0 +1,480 @@ +package api_test + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "testing" + "time" + + "github.com/crewjam/saml" + "github.com/stretchr/testify/require" + "github.com/supabase/auth/internal/api" + "github.com/supabase/auth/internal/api/apierrors" + "github.com/supabase/auth/internal/e2e" + "github.com/supabase/auth/internal/e2e/e2eapi" + "github.com/supabase/auth/internal/models" +) + +const testMetadataXMLTemplate = `MIIDqjCCApKgAwIBAgIGAZfq8svbMA0GCSqGSIb3DQEBCwUAMIGVMQswCQYDVQQGEwJVUzETMBEG +A1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5jaXNjbzENMAsGA1UECgwET2t0YTEU +MBIGA1UECwwLU1NPUHJvdmlkZXIxFjAUBgNVBAMMDXRyaWFsLTgxNzg2MTQxHDAaBgkqhkiG9w0B +CQEWDWluZm9Ab2t0YS5jb20wHhcNMjUwNzA4MTY1MDA5WhcNMzUwNzA4MTY1MTA5WjCBlTELMAkG +A1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDTAL +BgNVBAoMBE9rdGExFDASBgNVBAsMC1NTT1Byb3ZpZGVyMRYwFAYDVQQDDA10cmlhbC04MTc4NjE0 +MRwwGgYJKoZIhvcNAQkBFg1pbmZvQG9rdGEuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB +CgKCAQEAviGBsGh17McdQQPdTkI5Stw9wh3SdTpqXQ2C7Us8GMXKmV5/07Gh1tgNKeQDACgdox9v +nhXyNBjAW5ANITAQAsDwD8k9unZhksEGe5A/v4/Reas8gtYXMj2iVO1TaaM3ZIGamCbMtDybsX+a +6HCcnWv0LcGXuQLqNApWcKdZ9mNiAMAf3ATucwced8Yl950FsXobEf6bVFiOpIoL5tE4AjOfgfrK +Vm+p9PxuQh4vl4j8Iw9UkCyiLOwcJyo5XikfM7BeYsoU9WVV85/pXuI6vWMk4zsOfVjsLRhsEI7K +jbjkIxClLB43V+YOa+IN03Zmsxulp4Tm7AEvw3rlDIUpcQIDAQABMA0GCSqGSIb3DQEBCwUAA4IB +AQAPpGUfNkFzDP2Xzzd8Y+MoWZshbZYLANWcNNIyb5ajqX7CU/vJmOniYpZVp0f7n3Yu6DV28KiL +AvB46YNtsAtuUtEendEiIO5vcef5o0I4pKUz9RVT1WZIycCHbW9/qhPHpuRVVgpJcp2hleq7eisl +TYE5YIIH7ovMWAc0RuC3YqNttdq8ampcTxDer9VSVsFFXK/TqxcB7rwgCv6Q9jqf/7dsVKKFyIuS +NvmYIdjXXfEV0OsftSN/+s+UqtDEqEXR0Bmd51k0OJUm+2iNyu8Nh5Sr2M2475Gk2PSdPbzYdEoi +nfcdcTq+SxjLzHdQyv2U8TiLwZhRNXcrY8kCqgbyurn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress` + +func TestE2EAdmin(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + + t.Run("Admin", func(t *testing.T) { + globalCfg := e2e.Must(e2e.Config()) + + inst, err := e2eapi.New(globalCfg) + require.NoError(t, err) + defer inst.Close() + + t.Run("SSO", func(t *testing.T) { + getTestMetadata := func(label string) string { + return fmt.Sprintf(testMetadataXMLTemplate, label) + } + getTestAttributes := func() map[string]models.SAMLAttribute { + return map[string]models.SAMLAttribute{ + "TestE2EAdmin": models.SAMLAttribute{ + Default: true, + }, + "customAttr": models.SAMLAttribute{ + Default: "somevalue", + }, + "email": models.SAMLAttribute{ + Name: "user.email", + }, + "first_name": models.SAMLAttribute{ + Name: "user.firstName", + }, + "last_name": models.SAMLAttribute{ + Name: "user.lastName", + }, + "user_name": models.SAMLAttribute{ + Name: "user.login", + }, + } + } + + checkAPIError := func(t *testing.T, b []byte) { + apiError := new(apierrors.HTTPError) + if err := json.Unmarshal(b, &apiError); err != nil { + return + } + + if apiError.Message != "" { + require.NoError(t, apiError) + } + } + + checkHTTPRes := func(t *testing.T, httpRes *http.Response, exp any) { + require.GreaterOrEqual(t, httpRes.StatusCode, 200) + require.LessOrEqual(t, httpRes.StatusCode, 201) + + body, err := io.ReadAll(httpRes.Body) + require.NoError(t, err) + + checkAPIError(t, body) + + rdr := bytes.NewReader(body) + err = json.NewDecoder(rdr).Decode(exp) + require.NoError(t, err) + } + + // basic check + checkProvider := func(t *testing.T, pr *models.SSOProvider) { + const zeroID = "00000000-0000-0000-0000-000000000000" + require.NotNil(t, pr) + require.NotNil(t, pr.ID) + require.NotEqual(t, pr.ID.String(), zeroID) + + require.True(t, len(pr.ID.String()) > 0) + require.NotNil(t, + pr.SAMLProvider.NameIDFormat) + require.Equal(t, + *pr.SAMLProvider.NameIDFormat, + string(saml.EmailAddressNameIDFormat)) + + require.True(t, len(pr.SSODomains) > 0) + for _, domain := range pr.SSODomains { + require.NotEmpty(t, domain.Domain) + } + + require.True(t, len(pr.SSODomains) > 0) + require.LessOrEqual(t, pr.CreatedAt, pr.UpdatedAt) + } + + checkProviderMap := func(t *testing.T, m map[string]*models.SSOProvider) { + for k, v := range m { + require.Equal(t, k, v.ID.String()) + checkProvider(t, v) + } + } + + equalProviderParams := func(t *testing.T, + params *api.CreateSSOProviderParams, + pr *models.SSOProvider, + ) { + checkProvider(t, pr) + + require.NotNil(t, params) + + if params.ResourceID != nil { + require.NotNil(t, pr.ResourceID) + require.Equal(t, *params.ResourceID, *pr.ResourceID) + } + + if len(params.Domains) > 0 { + require.Equal(t, len(params.Domains), len(pr.SSODomains)) + } + + if params.NameIDFormat != "" { + require.NotNil(t, pr.SAMLProvider.NameIDFormat) + require.Equal(t, + *pr.SAMLProvider.NameIDFormat, + params.NameIDFormat) + } + + if len(params.Domains) > 0 { + require.Equal(t, len(params.Domains), len(pr.SSODomains)) + for _, domain := range params.Domains { + var ok bool + for _, v := range pr.SSODomains { + if domain == v.Domain { + ok = true + } + } + require.True(t, ok, "Could not find domain %s", domain) + } + } + + if len(params.AttributeMapping.Keys) > 0 { + require.True(t, + params.AttributeMapping.Equal( + &pr.SAMLProvider.AttributeMapping)) + } + } + + equalProvider := func(t *testing.T, a, b *models.SSOProvider) { + checkProvider(t, a) + checkProvider(t, b) + + require.Equal(t, a.ID, b.ID) + require.Equal(t, + a.ResourceID, + b.ResourceID) + + require.Equal(t, a.SSODomains, b.SSODomains) + + require.Equal(t, + a.SAMLProvider.ID, + b.SAMLProvider.ID) + require.Equal(t, + a.SAMLProvider.EntityID, + b.SAMLProvider.EntityID) + require.True(t, + a.SAMLProvider.AttributeMapping.Equal( + &b.SAMLProvider.AttributeMapping)) + require.Equal(t, + a.SAMLProvider.NameIDFormat, + b.SAMLProvider.NameIDFormat) + require.Equal(t, + a.SAMLProvider.CreatedAt, + b.SAMLProvider.CreatedAt) + require.Equal(t, + a.SAMLProvider.UpdatedAt, + b.SAMLProvider.UpdatedAt) + } + + equalProviderMaps := func(t *testing.T, a, b map[string]*models.SSOProvider) { + for expKey, expPr := range a { + gotPr, ok := b[expKey] + require.True(t, ok) + equalProvider(t, expPr, gotPr) + } + } + + listToMap := func(list []*models.SSOProvider) map[string]*models.SSOProvider { + out := make(map[string]*models.SSOProvider) + for _, pr := range list { + out[pr.ID.String()] = pr + } + return out + } + + listProviders := func(t *testing.T) map[string]*models.SSOProvider { + httpReq, err := http.NewRequestWithContext( + ctx, "GET", "/admin/sso/providers", nil) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + + var res struct { + Items []*models.SSOProvider `json:"items"` + } + checkHTTPRes(t, httpRes, &res) + + prMap := listToMap(res.Items) + checkProviderMap(t, prMap) + return prMap + } + + getProvider := func( + t *testing.T, + urlSegment string, + ) (res *models.SSOProvider) { + t.Run("Get/"+urlSegment, func(t *testing.T) { + url := "/admin/sso/" + urlSegment + httpReq, err := http.NewRequestWithContext( + ctx, "GET", url, nil) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + + res = new(models.SSOProvider) + checkHTTPRes(t, httpRes, res) + checkProvider(t, res) + }) + return + } + + updateProvider := func( + t *testing.T, + urlSegment string, + req *api.CreateSSOProviderParams, + ) (res *models.SSOProvider) { + t.Run("Update/"+urlSegment, func(t *testing.T) { + url := "/admin/sso/" + urlSegment + body := new(bytes.Buffer) + err := json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + httpReq, err := http.NewRequestWithContext( + ctx, "PUT", url, body) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + + res = new(models.SSOProvider) + checkHTTPRes(t, httpRes, res) + checkProvider(t, res) + equalProviderParams(t, req, res) + }) + return + } + + deleteProvider := func( + t *testing.T, + urlSegment string, + ) (res *models.SSOProvider) { + t.Run("Delete/"+urlSegment, func(t *testing.T) { + url := "/admin/sso/" + urlSegment + httpReq, err := http.NewRequestWithContext( + ctx, "DELETE", url, nil) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + + res = new(models.SSOProvider) + checkHTTPRes(t, httpRes, res) + checkProvider(t, res) + }) + return + } + + cleanProviders := func(t *testing.T, name string) { + t.Run("Clean/"+name, func(t *testing.T) { + prMap := listProviders(t) + for _, pr := range prMap { + keys := pr.SAMLProvider.AttributeMapping.Keys + if _, ok := keys["TestE2EAdmin"]; ok { + deleteProvider(t, "providers/"+pr.ID.String()) + } + } + }) + } + + // cleanup providers before & after tests + cleanProviders(t, "Before") + defer cleanProviders(t, "After") + + createProvider := func( + t *testing.T, + req *api.CreateSSOProviderParams, + ) (res *models.SSOProvider) { + t.Run("Create", func(t *testing.T) { + body := new(bytes.Buffer) + err := json.NewEncoder(body).Encode(req) + require.NoError(t, err) + + httpReq, err := http.NewRequestWithContext( + ctx, "POST", "/admin/sso/providers", body) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + + res = new(models.SSOProvider) + checkHTTPRes(t, httpRes, res) + + checkProvider(t, res) + equalProviderParams(t, req, res) + }) + return + + } + + t.Run("ByProviderID", func(t *testing.T) { + const label = "by-provider-id" + + createReq := &api.CreateSSOProviderParams{ + Type: "saml", + MetadataURL: "", + MetadataXML: getTestMetadata(label), + Domains: []string{ + label + ".local", + }, + AttributeMapping: models.SAMLAttributeMapping{ + Keys: getTestAttributes(), + }, + NameIDFormat: string(saml.EmailAddressNameIDFormat), + } + createRes := createProvider(t, createReq) + equalProviderParams(t, createReq, createRes) + + providerSeg := "providers/" + createRes.ID.String() + getRes := getProvider(t, providerSeg) + equalProvider(t, createRes, getRes) + + updateReq := &api.CreateSSOProviderParams{ + Domains: []string{ + label + ".local", + label + "-new.local", + }, + } + updateRes := updateProvider(t, providerSeg, updateReq) + getRes = getProvider(t, providerSeg) + equalProvider(t, updateRes, getRes) + + { + currentProviderMap := map[string]*models.SSOProvider{ + getRes.ID.String(): getRes, + } + prMap := listProviders(t) + equalProviderMaps(t, currentProviderMap, prMap) + } + + { + deleteRes := deleteProvider(t, providerSeg) + equalProvider(t, getRes, deleteRes) + + url := "/admin/sso/" + providerSeg + httpReq, err := http.NewRequestWithContext( + ctx, "GET", url, nil) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + require.Equal(t, 404, httpRes.StatusCode) + } + }) + + t.Run("ByResourceID", func(t *testing.T) { + const label = "by-resource-id" + + resourceID := label + ":default" + createReq := &api.CreateSSOProviderParams{ + Type: "saml", + ResourceID: &resourceID, + MetadataURL: "", + MetadataXML: getTestMetadata(label), + Domains: []string{ + label + ".local", + }, + AttributeMapping: models.SAMLAttributeMapping{ + Keys: getTestAttributes(), + }, + NameIDFormat: string(saml.EmailAddressNameIDFormat), + } + createRes := createProvider(t, createReq) + equalProviderParams(t, createReq, createRes) + + resourceSeg := "resources/" + resourceID + getRes := getProvider(t, resourceSeg) + equalProvider(t, createRes, getRes) + + { + updateReq := &api.CreateSSOProviderParams{ + Domains: []string{ + label + ".local", + label + "-new.local", + }, + } + updateRes := updateProvider(t, resourceSeg, updateReq) + getRes = getProvider(t, resourceSeg) + equalProvider(t, updateRes, getRes) + } + + var lastProvider *models.SSOProvider + { + resourceID = label + ":testing" + updateReq := &api.CreateSSOProviderParams{ + ResourceID: &resourceID, + } + updateRes := updateProvider(t, resourceSeg, updateReq) + + resourceSeg = "resources/" + resourceID + getRes = getProvider(t, resourceSeg) + equalProvider(t, updateRes, getRes) + + lastProvider = getRes + } + + { + currentProviderMap := map[string]*models.SSOProvider{ + lastProvider.ID.String(): lastProvider, + } + prMap := listProviders(t) + equalProviderMaps(t, currentProviderMap, prMap) + } + + { + deleteRes := deleteProvider(t, resourceSeg) + equalProvider(t, lastProvider, deleteRes) + + url := "/admin/sso/" + resourceSeg + httpReq, err := http.NewRequestWithContext( + ctx, "GET", url, nil) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + require.Equal(t, 404, httpRes.StatusCode) + } + }) + }) + }) +} diff --git a/internal/models/sso.go b/internal/models/sso.go index 28c2429acb..aa026d14d5 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -4,6 +4,7 @@ import ( "database/sql" "database/sql/driver" "encoding/json" + "math" "reflect" "strings" "time" @@ -16,7 +17,8 @@ import ( ) type SSOProvider struct { - ID uuid.UUID `db:"id" json:"id"` + ID uuid.UUID `db:"id" json:"id"` + ResourceID *string `db:"resource_id" json:"resource_id,omitempty"` SAMLProvider SAMLProvider `has_one:"saml_providers" fk_id:"sso_provider_id" json:"saml,omitempty"` SSODomains []SSODomain `has_many:"sso_domains" fk_id:"sso_provider_id" json:"domains"` @@ -199,6 +201,19 @@ func FindSSOProviderByID(tx *storage.Connection, id uuid.UUID) (*SSOProvider, er return nil, errors.Wrap(err, "error finding SAML SSO provider by ID") } + return &ssoProvider, nil +} + +func FindSSOProviderByResourceID(tx *storage.Connection, id string) (*SSOProvider, error) { + var ssoProvider SSOProvider + + if err := tx.Eager().Q().Where("resource_id = ?", id).First(&ssoProvider); err != nil { + if errors.Cause(err) == sql.ErrNoRows { + return nil, SSOProviderNotFoundError{} + } + + return nil, errors.Wrap(err, "error finding SAML SSO provider by Resource ID") + } return &ssoProvider, nil } @@ -233,7 +248,7 @@ func FindSSOProviderByDomain(tx *storage.Connection, domain string) (*SSOProvide return &ssoProvider, nil } -func FindAllSAMLProviders(tx *storage.Connection) ([]SSOProvider, error) { +func FindAllSSOProviders(tx *storage.Connection) ([]SSOProvider, error) { var providers []SSOProvider if err := tx.Eager().All(&providers); err != nil { @@ -247,6 +262,48 @@ func FindAllSAMLProviders(tx *storage.Connection) ([]SSOProvider, error) { return providers, nil } +// FindSSOProviderByFilter finds SSO Providers with the matching filter. +func FindSSOProviderByFilter( + tx *storage.Connection, + pageParams *Pagination, + sortParams *SortParams, + filter map[string]string, +) ([]*SSOProvider, error) { + ssoProviders := []*SSOProvider{} + q := tx.Eager().Q() + + // allow listing resources by prefix + if v, ok := filter["resource_id_prefix"]; ok { + q = q.Where("resource_id LIKE ?%", v) + } + + if sortParams != nil && len(sortParams.Fields) > 0 { + for _, field := range sortParams.Fields { + q = q.Order(field.Name + " " + string(field.Dir)) + } + } + + var err error + if pageParams != nil { + page := uint64ToInt(pageParams.Page, 0) + perPage := uint64ToInt(pageParams.PerPage, 50) + + err = q.Paginate(page, int(perPage)).All(&ssoProviders) + pageParams.Count = uint64(q.Paginator.TotalEntriesSize) + } else { + err = q.All(&ssoProviders) + } + + return ssoProviders, err +} + +func uint64ToInt(v uint64, def int) int { + if v > 0 && v < math.MaxInt { + def = int(v) + } + return def +} + func FindSAMLRelayStateByID(tx *storage.Connection, id uuid.UUID) (*SAMLRelayState, error) { var state SAMLRelayState From ebdb96e084dad3c3702d6f34a484a209f1bef1ee Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Wed, 16 Jul 2025 08:51:06 -0700 Subject: [PATCH 2/4] feat: support filtering SSO providers by resource_id & resource_id_prefix This change simplifies self-service SSO flows by enabling lookup of SSO providers via a `resource_id`, which maps to an infrastructure org ID. This removes the need for syncing external mappings. Key changes: - Extended `loadSSOProvider` to accept `resource_`-prefixed `idp_id` values - Removed previously introduced `/resources` routes - Added filtering to the `/admin/sso/providers` via `?resource_id{,_prefix}=` - Reworked `FindAllSSOProvidersByFilter` to drop pagination/sorting - Updated E2E test suite for full SSO coverage --- internal/api/api.go | 9 -- internal/api/ssoadmin.go | 34 ++++--- internal/api/ssoadmin_test.go | 162 +++++++++++++++++++++------------- internal/models/sso.go | 45 +++------- 4 files changed, 135 insertions(+), 115 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index fca5f81d39..17ce68d7b6 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -287,15 +287,6 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne r.Route("/{idp_id}", func(r *router) { r.Use(api.loadSSOProvider) - r.Get("/", api.adminSSOProvidersGet) - r.Put("/", api.adminSSOProvidersUpdate) - r.Delete("/", api.adminSSOProvidersDelete) - }) - }) - r.Route("/resources", func(r *router) { - r.Route("/{resource_id}", func(r *router) { - r.Use(api.loadSSOProvider) - r.Get("/", api.adminSSOProvidersGet) r.Put("/", api.adminSSOProvidersUpdate) r.Delete("/", api.adminSSOProvidersDelete) diff --git a/internal/api/ssoadmin.go b/internal/api/ssoadmin.go index 1d0f16c8f7..3c720495e0 100644 --- a/internal/api/ssoadmin.go +++ b/internal/api/ssoadmin.go @@ -19,8 +19,9 @@ import ( "github.com/supabase/auth/internal/utilities" ) -// loadSSOProvider looks for an idp_id or resource_id parameter in the URL route -// and loads the SSO provider into the the context. +// loadSSOProvider looks for an idp_id and first checks it for a "resource_" +// prefix, if present the provider is loaded by resource_id. Otherwise the +// provider is loaded by id. func (a *API) loadSSOProvider(w http.ResponseWriter, r *http.Request) (context.Context, error) { ctx := r.Context() db := a.db.WithContext(ctx) @@ -29,19 +30,19 @@ func (a *API) loadSSOProvider(w http.ResponseWriter, r *http.Request) (context.C provider *models.SSOProvider err error ) + + const resourcePrefix = "resource_" + idpParam := chi.URLParam(r, "idp_id") switch { - case chi.URLParam(r, "idp_id") != "": - idpParam := chi.URLParam(r, "idp_id") + case strings.HasPrefix(idpParam, resourcePrefix): + resourceID := strings.TrimPrefix(idpParam, resourcePrefix) + provider, err = models.FindSSOProviderByResourceID(db, resourceID) + default: idpID, idpErr := uuid.FromString(idpParam) if idpErr != nil { return nil, apierrors.NewNotFoundError(apierrors.ErrorCodeSSOProviderNotFound, "SSO Identity Provider not found") } provider, err = models.FindSSOProviderByID(db, idpID) - case chi.URLParam(r, "resource_id") != "": - resourceID := chi.URLParam(r, "resource_id") - provider, err = models.FindSSOProviderByResourceID(db, resourceID) - default: - err = apierrors.NewNotFoundError(apierrors.ErrorCodeSSOProviderNotFound, "SSO Identity Provider not found") } if err != nil { @@ -56,13 +57,24 @@ func (a *API) loadSSOProvider(w http.ResponseWriter, r *http.Request) (context.C return withSSOProvider(r.Context(), provider), nil } -// adminSSOProvidersList lists all SAML SSO Identity Providers in the system. Does +// adminSSOProvidersList lists all SSO Identity Providers in the system. Does // not deal with pagination at this time. func (a *API) adminSSOProvidersList(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() db := a.db.WithContext(ctx) - providers, err := models.FindAllSSOProviders(db) + filter := make(map[string]string) + for _, filterCol := range []string{ + // see: FindAllSSOProvidersByFilter in internal/models/sso.go + "resource_id", + "resource_id_prefix", + } { + if filterVal := r.URL.Query().Get(filterCol); filterVal != "" { + filter[filterCol] = filterVal + } + } + + providers, err := models.FindAllSSOProvidersByFilter(db, filter) if err != nil { return err } diff --git a/internal/api/ssoadmin_test.go b/internal/api/ssoadmin_test.go index 5d7485210c..36a16e1603 100644 --- a/internal/api/ssoadmin_test.go +++ b/internal/api/ssoadmin_test.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "net/url" "testing" "time" @@ -221,9 +222,14 @@ func TestE2EAdmin(t *testing.T) { return out } - listProviders := func(t *testing.T) map[string]*models.SSOProvider { + listProvidersWithFilter := func( + t *testing.T, + filter url.Values, + ) map[string]*models.SSOProvider { + url := "/admin/sso/providers?" + filter.Encode() + httpReq, err := http.NewRequestWithContext( - ctx, "GET", "/admin/sso/providers", nil) + ctx, "GET", url, nil) require.NoError(t, err) httpRes, err := inst.DoAdmin(httpReq) @@ -239,6 +245,10 @@ func TestE2EAdmin(t *testing.T) { return prMap } + listProviders := func(t *testing.T) map[string]*models.SSOProvider { + return listProvidersWithFilter(t, nil) + } + getProvider := func( t *testing.T, urlSegment string, @@ -344,7 +354,6 @@ func TestE2EAdmin(t *testing.T) { equalProviderParams(t, req, res) }) return - } t.Run("ByProviderID", func(t *testing.T) { @@ -405,75 +414,104 @@ func TestE2EAdmin(t *testing.T) { t.Run("ByResourceID", func(t *testing.T) { const label = "by-resource-id" - resourceID := label + ":default" - createReq := &api.CreateSSOProviderParams{ - Type: "saml", - ResourceID: &resourceID, - MetadataURL: "", - MetadataXML: getTestMetadata(label), - Domains: []string{ - label + ".local", - }, - AttributeMapping: models.SAMLAttributeMapping{ - Keys: getTestAttributes(), - }, - NameIDFormat: string(saml.EmailAddressNameIDFormat), - } - createRes := createProvider(t, createReq) - equalProviderParams(t, createReq, createRes) + currentProviderMap := make(map[string]*models.SSOProvider) + suffixes := []string{"live", "testing"} + for _, suffix := range suffixes { + t.Run("WithSuffix/"+suffix, func(t *testing.T) { + resourceID := label + ":" + suffix + createReq := &api.CreateSSOProviderParams{ + Type: "saml", + ResourceID: &resourceID, + MetadataURL: "", + MetadataXML: getTestMetadata(label + "-" + suffix), + Domains: []string{ + label + "-" + suffix + ".local", + }, + AttributeMapping: models.SAMLAttributeMapping{ + Keys: getTestAttributes(), + }, + NameIDFormat: string(saml.EmailAddressNameIDFormat), + } - resourceSeg := "resources/" + resourceID - getRes := getProvider(t, resourceSeg) - equalProvider(t, createRes, getRes) + createRes := createProvider(t, createReq) + equalProviderParams(t, createReq, createRes) - { - updateReq := &api.CreateSSOProviderParams{ - Domains: []string{ - label + ".local", - label + "-new.local", - }, - } - updateRes := updateProvider(t, resourceSeg, updateReq) - getRes = getProvider(t, resourceSeg) - equalProvider(t, updateRes, getRes) + resourceSeg := "providers/resource_" + resourceID + getRes := getProvider(t, resourceSeg) + equalProvider(t, createRes, getRes) + + { + updateReq := &api.CreateSSOProviderParams{ + Domains: []string{ + label + "-" + suffix + ".local", + label + "-" + suffix + "-new.local", + }, + } + updateRes := updateProvider(t, resourceSeg, updateReq) + getRes = getProvider(t, resourceSeg) + equalProvider(t, updateRes, getRes) + + currentProviderMap[getRes.ID.String()] = getRes + } + }) } - var lastProvider *models.SSOProvider - { - resourceID = label + ":testing" - updateReq := &api.CreateSSOProviderParams{ - ResourceID: &resourceID, - } - updateRes := updateProvider(t, resourceSeg, updateReq) + t.Run("ListByFilter", func(t *testing.T) { - resourceSeg = "resources/" + resourceID - getRes = getProvider(t, resourceSeg) - equalProvider(t, updateRes, getRes) + t.Run("NoFilter", func(t *testing.T) { + prMap := listProviders(t) + equalProviderMaps(t, currentProviderMap, prMap) + }) - lastProvider = getRes - } + t.Run("WithResourceID", func(t *testing.T) { + for _, pr := range currentProviderMap { + q := make(url.Values) + q.Add("resource_id", *pr.ResourceID) - { - currentProviderMap := map[string]*models.SSOProvider{ - lastProvider.ID.String(): lastProvider, - } - prMap := listProviders(t) - equalProviderMaps(t, currentProviderMap, prMap) - } + prMap := listProvidersWithFilter(t, q) + require.Len(t, prMap, 1) - { - deleteRes := deleteProvider(t, resourceSeg) - equalProvider(t, lastProvider, deleteRes) + got, ok := prMap[pr.ID.String()] + require.True(t, ok) + require.NotNil(t, got) - url := "/admin/sso/" + resourceSeg - httpReq, err := http.NewRequestWithContext( - ctx, "GET", url, nil) - require.NoError(t, err) + equalProvider(t, pr, got) + } + }) - httpRes, err := inst.DoAdmin(httpReq) - require.NoError(t, err) - require.Equal(t, 404, httpRes.StatusCode) - } + t.Run("WithResourceIDPrefix", func(t *testing.T) { + q := make(url.Values) + q.Add("resource_id_prefix", label+":") + + prMap := listProvidersWithFilter(t, q) + require.Len(t, prMap, 2) + + for _, pr := range currentProviderMap { + got, ok := prMap[pr.ID.String()] + require.True(t, ok) + require.NotNil(t, got) + + equalProvider(t, pr, got) + } + }) + }) + + t.Run("Delete", func(t *testing.T) { + for _, pr := range currentProviderMap { + resourceSeg := "providers/resource_" + *pr.ResourceID + deleteRes := deleteProvider(t, resourceSeg) + equalProvider(t, pr, deleteRes) + + url := "/admin/sso/" + resourceSeg + httpReq, err := http.NewRequestWithContext( + ctx, "GET", url, nil) + require.NoError(t, err) + + httpRes, err := inst.DoAdmin(httpReq) + require.NoError(t, err) + require.Equal(t, 404, httpRes.StatusCode) + } + }) }) }) }) diff --git a/internal/models/sso.go b/internal/models/sso.go index aa026d14d5..c68dc3788b 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -4,7 +4,6 @@ import ( "database/sql" "database/sql/driver" "encoding/json" - "math" "reflect" "strings" "time" @@ -262,46 +261,26 @@ func FindAllSSOProviders(tx *storage.Connection) ([]SSOProvider, error) { return providers, nil } -// FindSSOProviderByFilter finds SSO Providers with the matching filter. -func FindSSOProviderByFilter( +// FindAllSSOProvidersByFilter finds SSO Providers with the matching filter. +func FindAllSSOProvidersByFilter( tx *storage.Connection, - pageParams *Pagination, - sortParams *SortParams, filter map[string]string, ) ([]*SSOProvider, error) { ssoProviders := []*SSOProvider{} - q := tx.Eager().Q() - // allow listing resources by prefix - if v, ok := filter["resource_id_prefix"]; ok { - q = q.Where("resource_id LIKE ?%", v) + q := tx.Eager().Q() + if v, ok := filter["resource_id"]; ok { + q = q.Where("resource_id = ?", v) + } else if v, ok := filter["resource_id_prefix"]; ok { + q = q.Where("resource_id LIKE ?", v+"%") } - - if sortParams != nil && len(sortParams.Fields) > 0 { - for _, field := range sortParams.Fields { - q = q.Order(field.Name + " " + string(field.Dir)) + if err := q.All(&ssoProviders); err != nil { + if errors.Cause(err) == sql.ErrNoRows { + return nil, nil } + return nil, errors.Wrap(err, "error loading all SAML SSO providers") } - - var err error - if pageParams != nil { - page := uint64ToInt(pageParams.Page, 0) - perPage := uint64ToInt(pageParams.PerPage, 50) - - err = q.Paginate(page, int(perPage)).All(&ssoProviders) - pageParams.Count = uint64(q.Paginator.TotalEntriesSize) - } else { - err = q.All(&ssoProviders) - } - - return ssoProviders, err -} - -func uint64ToInt(v uint64, def int) int { - if v > 0 && v < math.MaxInt { - def = int(v) - } - return def + return ssoProviders, nil } func FindSAMLRelayStateByID(tx *storage.Connection, id uuid.UUID) (*SAMLRelayState, error) { From a86b8c057ee0b9e6bc485e757ce260d02fc9d2dd Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Fri, 18 Jul 2025 09:27:34 -0700 Subject: [PATCH 3/4] feat: support a disabled flag for SSO Providers The sso_providers.disabled column was added to the SSOProvider. --- internal/api/apierrors/errorcode.go | 1 + internal/api/samlacs.go | 12 +++++ internal/api/sso.go | 40 ++++++++------- internal/api/ssoadmin.go | 17 ++++++- internal/api/ssoadmin_test.go | 51 ++++++++++++++++++- internal/models/sso.go | 10 ++-- ...82212_add_disabled_to_sso_providers.up.sql | 9 ++++ 7 files changed, 116 insertions(+), 24 deletions(-) create mode 100644 migrations/20250717082212_add_disabled_to_sso_providers.up.sql diff --git a/internal/api/apierrors/errorcode.go b/internal/api/apierrors/errorcode.go index 2a151924ba..b764de80de 100644 --- a/internal/api/apierrors/errorcode.go +++ b/internal/api/apierrors/errorcode.go @@ -57,6 +57,7 @@ const ( ErrorCodeSAMLAssertionNoEmail ErrorCode = "saml_assertion_no_email" ErrorCodeUserAlreadyExists ErrorCode = "user_already_exists" ErrorCodeSSOProviderNotFound ErrorCode = "sso_provider_not_found" + ErrorCodeSSOProviderDisabled ErrorCode = "sso_provider_disabled" ErrorCodeSAMLMetadataFetchFailed ErrorCode = "saml_metadata_fetch_failed" ErrorCodeSAMLIdPAlreadyExists ErrorCode = "saml_idp_already_exists" ErrorCodeSSODomainAlreadyExists ErrorCode = "sso_domain_already_exists" diff --git a/internal/api/samlacs.go b/internal/api/samlacs.go index 104511b171..df840f0636 100644 --- a/internal/api/samlacs.go +++ b/internal/api/samlacs.go @@ -103,6 +103,12 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { return apierrors.NewInternalServerError("Unable to find SSO Provider from SAML RelayState") } + if !ssoProvider.IsEnabled() { + return apierrors.NewNotFoundError( + apierrors.ErrorCodeSSOProviderDisabled, + "SSO Provider assigned for this domain is currently disabled") + } + initiatedBy = "sp" entityId = ssoProvider.SAMLProvider.EntityID redirectTo = relayState.RedirectTo @@ -156,6 +162,12 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { return err } + if !ssoProvider.IsEnabled() { + return apierrors.NewNotFoundError( + apierrors.ErrorCodeSSOProviderDisabled, + "SSO Provider assigned for this domain is currently disabled") + } + idpMetadata, err := ssoProvider.SAMLProvider.EntityDescriptor() if err != nil { return err diff --git a/internal/api/sso.go b/internal/api/sso.go index 83667cfec5..9b803f4ce0 100644 --- a/internal/api/sso.go +++ b/internal/api/sso.go @@ -52,25 +52,8 @@ func (a *API) SingleSignOn(w http.ResponseWriter, r *http.Request) error { if hasProviderID, err = params.validate(); err != nil { return err } - codeChallengeMethod := params.CodeChallengeMethod - codeChallenge := params.CodeChallenge - - if err := validatePKCEParams(codeChallengeMethod, codeChallenge); err != nil { - return err - } - flowType := getFlowFromChallenge(params.CodeChallenge) - var flowStateID *uuid.UUID - flowStateID = nil - if isPKCEFlow(flowType) { - flowState, err := generateFlowState(db, models.SSOSAML.String(), models.SSOSAML, codeChallengeMethod, codeChallenge, nil) - if err != nil { - return err - } - flowStateID = &flowState.ID - } var ssoProvider *models.SSOProvider - if hasProviderID { ssoProvider, err = models.FindSSOProviderByID(db, params.ProviderID) if models.IsNotFoundError(err) { @@ -87,6 +70,29 @@ func (a *API) SingleSignOn(w http.ResponseWriter, r *http.Request) error { } } + if !ssoProvider.IsEnabled() { + return apierrors.NewNotFoundError( + apierrors.ErrorCodeSSOProviderDisabled, + "SSO Provider is currently disabled") + } + + codeChallengeMethod := params.CodeChallengeMethod + codeChallenge := params.CodeChallenge + + if err := validatePKCEParams(codeChallengeMethod, codeChallenge); err != nil { + return err + } + flowType := getFlowFromChallenge(params.CodeChallenge) + var flowStateID *uuid.UUID + flowStateID = nil + if isPKCEFlow(flowType) { + flowState, err := generateFlowState(db, models.SSOSAML.String(), models.SSOSAML, codeChallengeMethod, codeChallenge, nil) + if err != nil { + return err + } + flowStateID = &flowState.ID + } + entityDescriptor, err := ssoProvider.SAMLProvider.EntityDescriptor() if err != nil { return apierrors.NewInternalServerError("Error parsing SAML Metadata for SAML provider").WithInternalError(err) diff --git a/internal/api/ssoadmin.go b/internal/api/ssoadmin.go index 3c720495e0..42538774c7 100644 --- a/internal/api/ssoadmin.go +++ b/internal/api/ssoadmin.go @@ -97,7 +97,9 @@ type CreateSSOProviderParams struct { Domains []string `json:"domains"` AttributeMapping models.SAMLAttributeMapping `json:"attribute_mapping"` NameIDFormat string `json:"name_id_format"` - ResourceID *string `json:"resource_id,omitempty"` + + ResourceID *string `json:"resource_id,omitempty"` + Disabled *bool `json:"disabled,omitempty"` } func (p *CreateSSOProviderParams) validate(forUpdate bool) error { @@ -244,6 +246,7 @@ func (a *API) adminSSOProvidersCreate(w http.ResponseWriter, r *http.Request) er } provider := &models.SSOProvider{ + // TODO handle Name, Description, Attribute Mapping SAMLProvider: models.SAMLProvider{ EntityID: metadata.EntityID, @@ -254,10 +257,12 @@ func (a *API) adminSSOProvidersCreate(w http.ResponseWriter, r *http.Request) er if params.ResourceID != nil { provider.ResourceID = params.ResourceID } + if params.Disabled != nil { + provider.Disabled = params.Disabled + } if params.MetadataURL != "" { provider.SAMLProvider.MetadataURL = ¶ms.MetadataURL } - if params.NameIDFormat != "" { provider.SAMLProvider.NameIDFormat = ¶ms.NameIDFormat } @@ -412,6 +417,14 @@ func (a *API) adminSSOProvidersUpdate(w http.ResponseWriter, r *http.Request) er } } + if params.Disabled != nil { + disabled := *params.Disabled + if provider.Disabled == nil || *provider.Disabled != disabled { + provider.Disabled = &disabled + modified = true + } + } + if modified { if err := db.Transaction(func(tx *storage.Connection) error { if terr := tx.Eager().Update(provider); terr != nil { diff --git a/internal/api/ssoadmin_test.go b/internal/api/ssoadmin_test.go index 36a16e1603..871ded447e 100644 --- a/internal/api/ssoadmin_test.go +++ b/internal/api/ssoadmin_test.go @@ -435,12 +435,16 @@ func TestE2EAdmin(t *testing.T) { createRes := createProvider(t, createReq) equalProviderParams(t, createReq, createRes) + require.Nil(t, createRes.Disabled) + require.True(t, createRes.IsEnabled()) resourceSeg := "providers/resource_" + resourceID getRes := getProvider(t, resourceSeg) equalProvider(t, createRes, getRes) + require.Nil(t, getRes.Disabled) + require.True(t, getRes.IsEnabled()) - { + t.Run("AddDomain", func(t *testing.T) { updateReq := &api.CreateSSOProviderParams{ Domains: []string{ label + "-" + suffix + ".local", @@ -448,11 +452,54 @@ func TestE2EAdmin(t *testing.T) { }, } updateRes := updateProvider(t, resourceSeg, updateReq) + require.Nil(t, updateRes.Disabled) + require.True(t, updateRes.IsEnabled()) + getRes = getProvider(t, resourceSeg) equalProvider(t, updateRes, getRes) + require.Nil(t, getRes.Disabled) + require.True(t, getRes.IsEnabled()) currentProviderMap[getRes.ID.String()] = getRes - } + }) + + disabled := true + t.Run("DisabledFlag/true", func(t *testing.T) { + updateReq := &api.CreateSSOProviderParams{ + Disabled: &disabled, + } + updateRes := updateProvider(t, resourceSeg, updateReq) + require.NotNil(t, updateRes.Disabled) + require.True(t, *updateRes.Disabled) + require.False(t, updateRes.IsEnabled()) + + getRes = getProvider(t, resourceSeg) + equalProvider(t, updateRes, getRes) + require.NotNil(t, getRes.Disabled) + require.True(t, *getRes.Disabled) + require.False(t, getRes.IsEnabled()) + + currentProviderMap[getRes.ID.String()] = getRes + }) + + disabled = false + t.Run("DisabledFlag/false", func(t *testing.T) { + updateReq := &api.CreateSSOProviderParams{ + Disabled: &disabled, + } + updateRes := updateProvider(t, resourceSeg, updateReq) + require.NotNil(t, updateRes.Disabled) + require.False(t, *updateRes.Disabled) + require.True(t, updateRes.IsEnabled()) + + getRes = getProvider(t, resourceSeg) + equalProvider(t, updateRes, getRes) + require.NotNil(t, getRes.Disabled) + require.False(t, *getRes.Disabled) + require.True(t, getRes.IsEnabled()) + + currentProviderMap[getRes.ID.String()] = getRes + }) }) } diff --git a/internal/models/sso.go b/internal/models/sso.go index c68dc3788b..b048291ab3 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -16,9 +16,9 @@ import ( ) type SSOProvider struct { - ID uuid.UUID `db:"id" json:"id"` - ResourceID *string `db:"resource_id" json:"resource_id,omitempty"` - + ID uuid.UUID `db:"id" json:"id"` + ResourceID *string `db:"resource_id" json:"resource_id,omitempty"` + Disabled *bool `db:"disabled" json:"disabled"` SAMLProvider SAMLProvider `has_one:"saml_providers" fk_id:"sso_provider_id" json:"saml,omitempty"` SSODomains []SSODomain `has_many:"sso_domains" fk_id:"sso_provider_id" json:"domains"` @@ -26,6 +26,10 @@ type SSOProvider struct { UpdatedAt time.Time `db:"updated_at" json:"updated_at"` } +func (p SSOProvider) IsEnabled() bool { + return p.Disabled == nil || !*p.Disabled +} + func (p SSOProvider) TableName() string { return "sso_providers" } diff --git a/migrations/20250717082212_add_disabled_to_sso_providers.up.sql b/migrations/20250717082212_add_disabled_to_sso_providers.up.sql new file mode 100644 index 0000000000..1328539474 --- /dev/null +++ b/migrations/20250717082212_add_disabled_to_sso_providers.up.sql @@ -0,0 +1,9 @@ +do $$ begin + + alter table only {{ index .Options "Namespace" }}.sso_providers + add column if not exists disabled boolean null; + + create index if not exists sso_providers_resource_id_pattern_idx + on {{ index .Options "Namespace" }}.sso_providers + (resource_id text_pattern_ops); +end $$; From e5ec7da2ddbe70601eab8f132a4b3eacf3060aed Mon Sep 17 00:00:00 2001 From: Chris Stockton Date: Fri, 18 Jul 2025 14:43:37 -0700 Subject: [PATCH 4/4] chore: write unit tests --- internal/api/ssoadmin.go | 13 +- internal/api/ssoadmin_test.go | 9 +- internal/models/sso.go | 12 +- internal/models/sso_test.go | 244 +++++++++++++++++++++++++++++++++- 4 files changed, 260 insertions(+), 18 deletions(-) diff --git a/internal/api/ssoadmin.go b/internal/api/ssoadmin.go index 42538774c7..1b1d9519c7 100644 --- a/internal/api/ssoadmin.go +++ b/internal/api/ssoadmin.go @@ -63,18 +63,7 @@ func (a *API) adminSSOProvidersList(w http.ResponseWriter, r *http.Request) erro ctx := r.Context() db := a.db.WithContext(ctx) - filter := make(map[string]string) - for _, filterCol := range []string{ - // see: FindAllSSOProvidersByFilter in internal/models/sso.go - "resource_id", - "resource_id_prefix", - } { - if filterVal := r.URL.Query().Get(filterCol); filterVal != "" { - filter[filterCol] = filterVal - } - } - - providers, err := models.FindAllSSOProvidersByFilter(db, filter) + providers, err := models.FindAllSSOProvidersByFilter(db, r.URL.Query()) if err != nil { return err } diff --git a/internal/api/ssoadmin_test.go b/internal/api/ssoadmin_test.go index 871ded447e..4bedd009d1 100644 --- a/internal/api/ssoadmin_test.go +++ b/internal/api/ssoadmin_test.go @@ -246,7 +246,14 @@ func TestE2EAdmin(t *testing.T) { } listProviders := func(t *testing.T) map[string]*models.SSOProvider { - return listProvidersWithFilter(t, nil) + prMap := listProvidersWithFilter(t, nil) + for k, pr := range prMap { + keys := pr.SAMLProvider.AttributeMapping.Keys + if _, ok := keys["TestE2EAdmin"]; !ok { + delete(prMap, k) + } + } + return prMap } getProvider := func( diff --git a/internal/models/sso.go b/internal/models/sso.go index b048291ab3..3a5be7d973 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -4,6 +4,7 @@ import ( "database/sql" "database/sql/driver" "encoding/json" + "net/url" "reflect" "strings" "time" @@ -265,17 +266,22 @@ func FindAllSSOProviders(tx *storage.Connection) ([]SSOProvider, error) { return providers, nil } +const ( + resourceIDFilter = "resource_id" + resourceIDPrefixFilter = "resource_id_prefix" +) + // FindAllSSOProvidersByFilter finds SSO Providers with the matching filter. func FindAllSSOProvidersByFilter( tx *storage.Connection, - filter map[string]string, + queryValues url.Values, ) ([]*SSOProvider, error) { ssoProviders := []*SSOProvider{} q := tx.Eager().Q() - if v, ok := filter["resource_id"]; ok { + if v := queryValues.Get(resourceIDFilter); v != "" { q = q.Where("resource_id = ?", v) - } else if v, ok := filter["resource_id_prefix"]; ok { + } else if v := queryValues.Get(resourceIDPrefixFilter); v != "" { q = q.Where("resource_id LIKE ?", v+"%") } if err := q.All(&ssoProviders); err != nil { diff --git a/internal/models/sso_test.go b/internal/models/sso_test.go index b6c9656309..dd6f8c8ee5 100644 --- a/internal/models/sso_test.go +++ b/internal/models/sso_test.go @@ -1,8 +1,11 @@ package models import ( - tst "testing" + "net/url" + "slices" + "testing" + "github.com/gofrs/uuid" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/supabase/auth/internal/conf" @@ -20,7 +23,7 @@ func (ts *SSOTestSuite) SetupTest() { TruncateAll(ts.db) } -func TestSSO(t *tst.T) { +func TestSSO(t *testing.T) { globalConfig, err := conf.LoadGlobal(modelsTestConfig) require.NoError(t, err) @@ -230,3 +233,240 @@ func (ts *SSOTestSuite) TestFindSAMLProviderByEntityID() { } } } + +func (ts *SSOTestSuite) TestFindSSOProviderByResourceID() { + const ( + g1Prefix = "group_one:" + g1Test = g1Prefix + "test" + g1Live = g1Prefix + "live" + + g2Prefix = "group_two:" + g2Test = g2Prefix + "test" + g2Live = g2Prefix + "live" + ) + + genStr := func() string { + return uuid.Must(uuid.NewV4()).String() + } + + genProvider := func(resourceID string) *SSOProvider { + str := genStr() + pr := &SSOProvider{ + SAMLProvider: SAMLProvider{ + EntityID: "https://example.com/saml/metadata/" + str, + MetadataXML: "", + }, + SSODomains: []SSODomain{ + { + Domain: str + ".local", + }, + }, + } + if resourceID != "" { + pr.ResourceID = &resourceID + } + return pr + } + + var ( + ssoProviderG1Test = genProvider(g1Test) + ssoProviderG1Live = genProvider(g1Live) + ssoProviderG2Test = genProvider(g2Test) + ssoProviderG2Live = genProvider(g2Live) + ssoProviderNoResourceID1 = genProvider("") + ssoProviderNoResourceID2 = genProvider("") + ) + + noProviders := []*SSOProvider{ + ssoProviderNoResourceID1, + ssoProviderNoResourceID2, + } + g1Providers := []*SSOProvider{ + ssoProviderG1Test, + ssoProviderG1Live, + } + g2Providers := []*SSOProvider{ + ssoProviderG2Test, + ssoProviderG2Live, + } + gProviders := slices.Concat(g1Providers, g2Providers) + allProviders := slices.Concat(g1Providers, g2Providers, noProviders) + + defer func() { + for _, pr := range allProviders { + ts.db.Eager().Destroy(pr) + } + }() + + for i := range allProviders { + pr := allProviders[i] + require.NoError(ts.T(), ts.db.Eager().Create(pr)) + ts.db.Eager().Q().Where("id = ?", pr.ID).First(pr) + } + + type testsSpec struct { + query url.Values + exp []*SSOProvider + } + + tests := []testsSpec{ + + // no filters + { + query: nil, + exp: allProviders, + }, + + // resource_id + { + query: url.Values{"resource_id": []string{g1Prefix}}, + exp: nil, + }, + { + query: url.Values{"resource_id": []string{g1Test}}, + exp: []*SSOProvider{ssoProviderG1Test}, + }, + { + query: url.Values{"resource_id": []string{g1Live}}, + exp: []*SSOProvider{ssoProviderG1Live}, + }, + { + query: url.Values{"resource_id": []string{g2Test}}, + exp: []*SSOProvider{ssoProviderG2Test}, + }, + { + query: url.Values{"resource_id": []string{g2Live}}, + exp: []*SSOProvider{ssoProviderG2Live}, + }, + + // resource_id - negative + { + query: url.Values{"resource_id": []string{g1Test[:len(g1Test)-1]}}, + exp: nil, + }, + + // resource_id_prefix + { + query: url.Values{"resource_id_prefix": []string{g1Prefix}}, + exp: g1Providers, + }, + { + query: url.Values{"resource_id_prefix": []string{g2Prefix}}, + exp: g2Providers, + }, + + // resource_id_prefix - partial + { + query: url.Values{"resource_id_prefix": []string{"group"}}, + exp: gProviders, + }, + { + query: url.Values{"resource_id_prefix": []string{"group_one:"}}, + exp: g1Providers, + }, + { + query: url.Values{"resource_id_prefix": []string{"group_two:"}}, + exp: g2Providers, + }, + { + query: url.Values{"resource_id_prefix": []string{g1Test[:len(g1Test)-2]}}, + exp: []*SSOProvider{ssoProviderG1Test}, + }, + { + query: url.Values{"resource_id_prefix": []string{g1Live[:len(g1Live)-2]}}, + exp: []*SSOProvider{ssoProviderG1Live}, + }, + + // resource_id_prefix - exact matches + { + query: url.Values{"resource_id_prefix": []string{g1Test}}, + exp: []*SSOProvider{ssoProviderG1Test}, + }, + { + query: url.Values{"resource_id_prefix": []string{g1Live}}, + exp: []*SSOProvider{ssoProviderG1Live}, + }, + { + query: url.Values{"resource_id_prefix": []string{g2Test}}, + exp: []*SSOProvider{ssoProviderG2Test}, + }, + { + query: url.Values{"resource_id_prefix": []string{g2Live}}, + exp: []*SSOProvider{ssoProviderG2Live}, + }, + + { + query: url.Values{"resource_id_prefix": []string{"invalid:"}}, + exp: nil, + }, + { + query: url.Values{"resource_id_prefix": []string{"invalid:"}}, + exp: nil, + }, + } + + check := func(t *testing.T, exp, got []*SSOProvider) { + t.Helper() + + require.Len(t, got, len(exp)) + + isEqual := func(a, b *SSOProvider) bool { + return a.ID == b.ID && a.ResourceID == b.ResourceID + } + equal := slices.EqualFunc(exp, got, isEqual) + if !equal { + require.Equal(t, exp, got) + } + } + + for _, test := range tests { + ts.Run("FindAllSSOProvidersByFilter/query='"+test.query.Encode()+"'", func() { + prs, err := FindAllSSOProvidersByFilter(ts.db, test.query) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), prs) + check(ts.T(), test.exp, prs) + }) + } + + for _, exp := range allProviders { + + { + got, err := FindSSOProviderByID(ts.db, exp.ID) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), got) + check(ts.T(), []*SSOProvider{exp}, []*SSOProvider{got}) + } + + if exp.ResourceID != nil { + got, err := FindSSOProviderByResourceID(ts.db, *exp.ResourceID) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), got) + check(ts.T(), []*SSOProvider{exp}, []*SSOProvider{got}) + } + + for _, domain := range exp.SSODomains { + got, err := FindSSOProviderByDomain(ts.db, domain.Domain) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), got) + check(ts.T(), []*SSOProvider{exp}, []*SSOProvider{got}) + } + } + + { + got, err := FindSSOProviderByResourceID(ts.db, "") + require.Error(ts.T(), err) + require.Nil(ts.T(), got) + } + + { + got, err := FindSSOProviderByID(ts.db, uuid.Nil) + require.Error(ts.T(), err) + require.Nil(ts.T(), got) + } + + { + got, err := FindSSOProviderByDomain(ts.db, "_test_invalid_") + require.Error(ts.T(), err) + require.Nil(ts.T(), got) + } +}