From 915febd8d507cbc8d0298945edf523bd1025dfa3 Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 30 Jul 2026 16:29:49 -0600 Subject: [PATCH 01/18] feat(scim): store a per-provider SCIM token hash --- internal/models/sso.go | 28 +++++++ internal/models/sso_test.go | 77 +++++++++++++++++++ ...dd_scim_token_hash_to_sso_providers.up.sql | 9 +++ 3 files changed, 114 insertions(+) create mode 100644 migrations/20260731000000_add_scim_token_hash_to_sso_providers.up.sql diff --git a/internal/models/sso.go b/internal/models/sso.go index 3a5be7d973..ca90de966a 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -1,8 +1,10 @@ package models import ( + "crypto/sha256" "database/sql" "database/sql/driver" + "encoding/hex" "encoding/json" "net/url" "reflect" @@ -23,6 +25,8 @@ type SSOProvider struct { 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"` + SCIMTokenHash *string `db:"scim_token_hash" json:"-"` + CreatedAt time.Time `db:"created_at" json:"created_at"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"` } @@ -39,6 +43,16 @@ func (p SSOProvider) Type() string { return "saml" } +func (p *SSOProvider) UpdateSCIMToken(token string) { + hash := toSHA256(token) + p.SCIMTokenHash = &hash +} + +func toSHA256(token string) string { + sum := sha256.Sum256([]byte(token)) + return hex.EncodeToString(sum[:]) +} + type SAMLAttribute struct { Name string `json:"name,omitempty"` Names []string `json:"names,omitempty"` @@ -222,6 +236,20 @@ func FindSSOProviderByResourceID(tx *storage.Connection, id string) (*SSOProvide return &ssoProvider, nil } +func FindSSOProviderBySCIMToken(tx *storage.Connection, token string) (*SSOProvider, error) { + var ssoProvider SSOProvider + + if err := tx.Q().Where("scim_token_hash = ?", toSHA256(token)).First(&ssoProvider); err != nil { + if errors.Cause(err) == sql.ErrNoRows { + return nil, SSOProviderNotFoundError{} + } + + return nil, errors.Wrap(err, "error finding SSO provider by SCIM token") + } + + return &ssoProvider, nil +} + func FindSSOProviderForEmailAddress(tx *storage.Connection, emailAddress string) (*SSOProvider, error) { parts := strings.Split(emailAddress, "@") emailDomain := strings.ToLower(parts[1]) diff --git a/internal/models/sso_test.go b/internal/models/sso_test.go index 523ad614c7..cd06cc1960 100644 --- a/internal/models/sso_test.go +++ b/internal/models/sso_test.go @@ -469,3 +469,80 @@ func (ts *SSOTestSuite) TestFindSSOProviderByResourceID() { require.Nil(ts.T(), got) } } + +func (ts *SSOTestSuite) TestUpdateSCIMToken() { + hashes := map[string]string{ + "scim_test_token": "dcbcd9ffd696ae1f2ee0f035fa17680d78175020a5fa1aadc758dbd681e0fe1d", + "scim_rotated_token": "289adb37f8946571bb4aea1e663281126c7f2d84d929ff09429fcaa1eb3f27bf", + } + + provider := &SSOProvider{ + SAMLProvider: SAMLProvider{ + EntityID: "https://example.com/saml/metadata/", + MetadataXML: "", + }, + } + require.Nil(ts.T(), provider.SCIMTokenHash) + + for token, hash := range hashes { + provider.UpdateSCIMToken(token) + require.NotNil(ts.T(), provider.SCIMTokenHash) + require.Equal(ts.T(), hash, *provider.SCIMTokenHash) + } +} + +func (ts *SSOTestSuite) TestFindSSOProviderBySCIMToken() { + token := "scim_test_token" + provider := &SSOProvider{ + SAMLProvider: SAMLProvider{ + EntityID: "https://example.com/saml/metadata/1", + MetadataXML: "", + }, + } + + provider.UpdateSCIMToken(token) + require.NoError(ts.T(), ts.db.Eager().Create(provider)) + + withoutToken := &SSOProvider{ + SAMLProvider: SAMLProvider{ + EntityID: "https://example.com/saml/metadata/2", + MetadataXML: "", + }, + } + require.NoError(ts.T(), ts.db.Eager().Create(withoutToken)) + + ts.Run("resolves the provider that owns the token", func() { + found, err := FindSSOProviderBySCIMToken(ts.db, token) + + require.NoError(ts.T(), err) + require.Equal(ts.T(), provider.ID, found.ID) + }) + + ts.Run("an unknown token resolves nothing", func() { + found, err := FindSSOProviderBySCIMToken(ts.db, "scim_unknown_token") + + require.Nil(ts.T(), found) + require.True(ts.T(), IsNotFoundError(err)) + }) + + ts.Run("an empty token does not match a provider without one", func() { + found, err := FindSSOProviderBySCIMToken(ts.db, "") + + require.Nil(ts.T(), found) + require.True(ts.T(), IsNotFoundError(err)) + }) + + ts.Run("rotation stops the previous token from resolving", func() { + newToken := "scim_rotated_token" + provider.UpdateSCIMToken(newToken) + require.NoError(ts.T(), ts.db.Update(provider)) + + found, err := FindSSOProviderBySCIMToken(ts.db, newToken) + require.NoError(ts.T(), err) + require.Equal(ts.T(), provider.ID, found.ID) + + found, err = FindSSOProviderBySCIMToken(ts.db, token) + require.Nil(ts.T(), found) + require.True(ts.T(), IsNotFoundError(err)) + }) +} diff --git a/migrations/20260731000000_add_scim_token_hash_to_sso_providers.up.sql b/migrations/20260731000000_add_scim_token_hash_to_sso_providers.up.sql new file mode 100644 index 0000000000..fd8afeadf7 --- /dev/null +++ b/migrations/20260731000000_add_scim_token_hash_to_sso_providers.up.sql @@ -0,0 +1,9 @@ +-- Holds the SHA-256 hex digest of the provider's SCIM token. +/* auth_migration: 20260731000000 */ +alter table only {{ index .Options "Namespace" }}.sso_providers + add column if not exists scim_token_hash text null; + +/* auth_migration: 20260731000000 */ +create unique index if not exists sso_providers_scim_token_hash_idx + on {{ index .Options "Namespace" }}.sso_providers (scim_token_hash) + where scim_token_hash is not null; From 6df5a18980d7cd11cf9590ee4a29aad5df516ac0 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 31 Jul 2026 11:47:48 -0600 Subject: [PATCH 02/18] feat(scim): add GET /scim/v2/Users/{id} --- internal/api/api.go | 4 +- internal/api/scim/authenticate.go | 71 ++++++ internal/api/scim/authenticate_test.go | 48 ++++ internal/api/scim/context.go | 27 +++ internal/api/scim/context_test.go | 60 +++++ internal/api/scim/core/endpoints.go | 1 + internal/api/scim/core/meta.go | 13 +- internal/api/scim/core/meta_test.go | 15 +- internal/api/scim/core/schemas.go | 2 + .../api/scim/core/service_provider_config.go | 2 +- internal/api/scim/core/user.go | 15 ++ internal/api/scim/core/user_test.go | 54 +++++ internal/api/scim/mapper.go | 38 ++++ internal/api/scim/mapper_test.go | 67 ++++++ internal/api/scim/server.go | 18 +- internal/api/scim/server_test.go | 12 +- internal/api/scim/users.go | 33 +++ internal/api/scim_test.go | 208 ++++++++++++++++++ internal/models/user.go | 19 ++ internal/models/user_test.go | 75 +++++++ 20 files changed, 767 insertions(+), 15 deletions(-) create mode 100644 internal/api/scim/authenticate.go create mode 100644 internal/api/scim/authenticate_test.go create mode 100644 internal/api/scim/context.go create mode 100644 internal/api/scim/context_test.go create mode 100644 internal/api/scim/core/user.go create mode 100644 internal/api/scim/core/user_test.go create mode 100644 internal/api/scim/mapper.go create mode 100644 internal/api/scim/mapper_test.go create mode 100644 internal/api/scim/users.go diff --git a/internal/api/api.go b/internal/api/api.go index beebb26c01..8353ec451d 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -138,7 +138,7 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne api.oauthServer = oauthserver.NewServer(globalConfig, db, api.tokenService) } - api.scim = scim.NewServer(globalConfig) + api.scim = scim.NewServer(globalConfig, db) if api.config.Password.HIBP.Enabled { httpClient := &http.Client{ @@ -458,6 +458,8 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne r.Get("/ServiceProviderConfig", api.scim.ServiceProviderConfig) r.Get("/ResourceTypes", api.scim.ResourceTypes) r.Get("/Schemas", api.scim.Schemas) + + r.WithBypass(api.scim.Authenticate).Get("/Users/{id}", api.scim.UserByID) }) }) diff --git a/internal/api/scim/authenticate.go b/internal/api/scim/authenticate.go new file mode 100644 index 0000000000..59ff25d2c1 --- /dev/null +++ b/internal/api/scim/authenticate.go @@ -0,0 +1,71 @@ +package scim + +import ( + "context" + "net/http" + "strings" + + "github.com/supabase/auth/internal/api/scim/protocol" + "github.com/supabase/auth/internal/models" + "github.com/supabase/auth/internal/observability" +) + +var providerKey = NewKey[*models.SSOProvider]("sso_provider") + +func (srv *Server) Authenticate(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx, ok := srv.authenticate(w, r) + if !ok { + return + } + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + +func (srv *Server) authenticate(w http.ResponseWriter, r *http.Request) (context.Context, bool) { + ctx := r.Context() + + token, ok := parseBearerToken(r.Header.Get("Authorization")) + if !ok { + unauthorized(w) + return nil, false + } + + provider, err := models.FindSSOProviderBySCIMToken(srv.db.WithContext(ctx), token) + if err != nil { + if models.IsNotFoundError(err) { + unauthorized(w) + return nil, false + } + srv.internalError(w, r, err) + return nil, false + } + + if !provider.IsEnabled() { + protocol.SendError(w, http.StatusForbidden, "", "SCIM is not available for this provider") + return nil, false + } + + observability.LogEntrySetField(r, "sso_provider_id", provider.ID.String()) + + return providerKey.With(ctx, provider), true +} + +func parseBearerToken(header string) (string, bool) { + scheme, rest, found := strings.Cut(header, " ") + if !found || !strings.EqualFold(scheme, "Bearer") { + return "", false + } + + token := strings.TrimSpace(rest) + if token == "" || strings.ContainsAny(token, " \t\r\n\v\f") { + return "", false + } + + return token, true +} + +func unauthorized(w http.ResponseWriter) error { + w.Header().Set("WWW-Authenticate", "Bearer") + return protocol.SendError(w, http.StatusUnauthorized, "", "A valid SCIM bearer token is required") +} diff --git a/internal/api/scim/authenticate_test.go b/internal/api/scim/authenticate_test.go new file mode 100644 index 0000000000..129f39801d --- /dev/null +++ b/internal/api/scim/authenticate_test.go @@ -0,0 +1,48 @@ +package scim + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseBearerToken(t *testing.T) { + // RFC 7235, Section 2.1: credentials = auth-scheme 1*SP token68. + // The scheme is case insensitive and one or more spaces may separate it + // from the token. + accepted := map[string]string{ + "Bearer tok": "tok", + "bearer tok": "tok", + "BEARER tok": "tok", + "Bearer tok": "tok", + "Bearer tok ": "tok", + "Bearer \ttok": "tok", + } + + for header, want := range accepted { + t.Run(header, func(t *testing.T) { + got, ok := parseBearerToken(header) + + require.True(t, ok) + require.Equal(t, want, got) + }) + } + + rejected := []string{ + "", + "Bearer", + "Bearer ", + "Basic tok", + "Bearertok", + "Bearer tok extra", + } + + for _, header := range rejected { + t.Run("rejects "+header, func(t *testing.T) { + got, ok := parseBearerToken(header) + + require.False(t, ok) + require.Empty(t, got) + }) + } +} diff --git a/internal/api/scim/context.go b/internal/api/scim/context.go new file mode 100644 index 0000000000..3611c66fa5 --- /dev/null +++ b/internal/api/scim/context.go @@ -0,0 +1,27 @@ +package scim + +import "context" + +type Key[T any] struct { + name string +} + +func NewKey[T any](name string) Key[T] { + return Key[T]{name: name} +} + +// String makes the key legible when a context is printed. Without it two +// keys of the same T are indistinguishable, since context renders the key by +// type name. +func (k Key[T]) String() string { + return "gotrue scim context key " + k.name +} + +func (k Key[T]) With(ctx context.Context, value T) context.Context { + return context.WithValue(ctx, k, value) +} + +func (k Key[T]) From(ctx context.Context) T { + value, _ := ctx.Value(k).(T) + return value +} diff --git a/internal/api/scim/context_test.go b/internal/api/scim/context_test.go new file mode 100644 index 0000000000..e5ad3f18d0 --- /dev/null +++ b/internal/api/scim/context_test.go @@ -0,0 +1,60 @@ +package scim + +import ( + "context" + "fmt" + "testing" + + "github.com/gofrs/uuid" + "github.com/stretchr/testify/require" + "github.com/supabase/auth/internal/models" +) + +func TestKey(t *testing.T) { + key := NewKey[*models.SSOProvider]("sso_provider") + + t.Run("round trips a typed value", func(t *testing.T) { + provider := &models.SSOProvider{ID: uuid.Must(uuid.NewV4())} + + ctx := key.With(context.Background(), provider) + + require.Equal(t, provider, key.From(ctx)) + }) + + t.Run("returns the zero value when absent", func(t *testing.T) { + require.Nil(t, key.From(context.Background())) + }) + + t.Run("keys of different types do not collide", func(t *testing.T) { + other := NewKey[string]("sso_provider") + + ctx := other.With(context.Background(), "not a provider") + + require.Nil(t, key.From(ctx)) + require.Equal(t, "not a provider", other.From(ctx)) + }) + + t.Run("keys of the same type and name share a slot", func(t *testing.T) { + provider := &models.SSOProvider{ID: uuid.Must(uuid.NewV4())} + + ctx := NewKey[*models.SSOProvider]("sso_provider").With(context.Background(), provider) + + require.Equal(t, provider, key.From(ctx)) + }) + + t.Run("keys of the same type but a different name do not collide", func(t *testing.T) { + first, second := NewKey[string]("first"), NewKey[string]("second") + + ctx := first.With(context.Background(), "one") + ctx = second.With(ctx, "two") + + require.Equal(t, "one", first.From(ctx)) + require.Equal(t, "two", second.From(ctx)) + }) + + t.Run("names the key when a context is printed", func(t *testing.T) { + ctx := NewKey[string]("first").With(context.Background(), "one") + + require.Contains(t, fmt.Sprint(ctx), "gotrue scim context key first") + }) +} diff --git a/internal/api/scim/core/endpoints.go b/internal/api/scim/core/endpoints.go index b1f9003dfb..ecd463660c 100644 --- a/internal/api/scim/core/endpoints.go +++ b/internal/api/scim/core/endpoints.go @@ -3,4 +3,5 @@ package core // The resource endpoints of RFC 7644, Section 3.2, relative to the base URL const ( EndpointServiceProviderConfig = "/ServiceProviderConfig" + EndpointUsers = "/Users" ) diff --git a/internal/api/scim/core/meta.go b/internal/api/scim/core/meta.go index a47e4a4b30..196325cc21 100644 --- a/internal/api/scim/core/meta.go +++ b/internal/api/scim/core/meta.go @@ -1,14 +1,23 @@ package core +import "time" + // Meta is the resource metadata common attribute defined in RFC 7643, Section 3.1. type Meta struct { ResourceType ResourceTypeName `json:"resourceType"` + Created time.Time `json:"created,omitzero"` + LastModified time.Time `json:"lastModified,omitzero"` Location string `json:"location,omitempty"` } -func NewMeta(baseURL string, resourceType ResourceTypeName, endpoint string) Meta { +func NewMeta(baseURL string, resourceType ResourceTypeName, endpoint, id string) Meta { + location := baseURL + endpoint + if id != "" { + location += "/" + id + } + return Meta{ ResourceType: resourceType, - Location: baseURL + endpoint, + Location: location, } } diff --git a/internal/api/scim/core/meta_test.go b/internal/api/scim/core/meta_test.go index 4b7383bd70..ac8d47817e 100644 --- a/internal/api/scim/core/meta_test.go +++ b/internal/api/scim/core/meta_test.go @@ -8,11 +8,20 @@ import ( ) func TestNewMeta(t *testing.T) { - t.Run("locates the resource at its endpoint", func(t *testing.T) { - meta := NewMeta("http://localhost:9999/scim/v2", ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig) + baseURL := "http://localhost:9999/scim/v2" + + t.Run("locates a resource that is its own endpoint", func(t *testing.T) { + meta := NewMeta(baseURL, ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig, "") require.Equal(t, ResourceTypeServiceProviderConfig, meta.ResourceType) - require.Equal(t, "http://localhost:9999/scim/v2/ServiceProviderConfig", meta.Location) + require.Equal(t, baseURL+"/ServiceProviderConfig", meta.Location) + }) + + t.Run("locates one resource of a collection", func(t *testing.T) { + meta := NewMeta(baseURL, ResourceTypeUser, EndpointUsers, "2819c223-7f76-453a-919d-413861904646") + + require.Equal(t, ResourceTypeUser, meta.ResourceType) + require.Equal(t, baseURL+"/Users/2819c223-7f76-453a-919d-413861904646", meta.Location) }) } diff --git a/internal/api/scim/core/schemas.go b/internal/api/scim/core/schemas.go index 128b2ea719..b324034caa 100644 --- a/internal/api/scim/core/schemas.go +++ b/internal/api/scim/core/schemas.go @@ -6,8 +6,10 @@ const ( schemaCore = schemaRoot + ":core:2.0" SchemaServiceProviderConfig SchemaURI = schemaCore + ":ServiceProviderConfig" + SchemaUser SchemaURI = schemaCore + ":User" ) const ( ResourceTypeServiceProviderConfig ResourceTypeName = "ServiceProviderConfig" + ResourceTypeUser ResourceTypeName = "User" ) diff --git a/internal/api/scim/core/service_provider_config.go b/internal/api/scim/core/service_provider_config.go index 26c64da947..3fcfb07420 100644 --- a/internal/api/scim/core/service_provider_config.go +++ b/internal/api/scim/core/service_provider_config.go @@ -65,6 +65,6 @@ func NewServiceProviderConfig(baseURL string, schemes ...*AuthenticationScheme) return &ServiceProviderConfig{ Schemas: []SchemaURI{SchemaServiceProviderConfig}, AuthenticationSchemes: schemes, - Meta: NewMeta(baseURL, ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig), + Meta: NewMeta(baseURL, ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig, ""), } } diff --git a/internal/api/scim/core/user.go b/internal/api/scim/core/user.go new file mode 100644 index 0000000000..c39237cc54 --- /dev/null +++ b/internal/api/scim/core/user.go @@ -0,0 +1,15 @@ +package core + +type Email struct { + Value string `json:"value"` + Primary bool `json:"primary"` +} + +// User is the core User resource defined in RFC 7643, Section 4.1. +type User struct { + Schemas []SchemaURI `json:"schemas"` + ID string `json:"id"` + UserName string `json:"userName"` + Emails []Email `json:"emails,omitempty"` + Meta Meta `json:"meta"` +} diff --git a/internal/api/scim/core/user_test.go b/internal/api/scim/core/user_test.go new file mode 100644 index 0000000000..4db1ba0a1d --- /dev/null +++ b/internal/api/scim/core/user_test.go @@ -0,0 +1,54 @@ +package core + +import ( + "encoding/json" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestUser(t *testing.T) { + created := time.Date(2026, 7, 21, 19, 41, 41, 0, time.UTC) + lastModified := time.Date(2026, 7, 22, 8, 12, 3, 0, time.UTC) + + user := User{ + Schemas: []SchemaURI{SchemaUser}, + ID: "2819c223-7f76-453a-919d-413861904646", + UserName: "bjensen@example.com", + Emails: []Email{{Value: "bjensen@example.com", Primary: true}}, + Meta: Meta{ + ResourceType: ResourceTypeUser, + Created: created, + LastModified: lastModified, + Location: "http://localhost:9999/scim/v2/Users/2819c223-7f76-453a-919d-413861904646", + }, + } + + t.Run("serializes to JSON correctly", func(t *testing.T) { + body, err := json.Marshal(user) + + require.NoError(t, err) + require.JSONEq(t, `{ + "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"], + "id": "2819c223-7f76-453a-919d-413861904646", + "userName": "bjensen@example.com", + "emails": [{"value": "bjensen@example.com", "primary": true}], + "meta": { + "resourceType": "User", + "created": "2026-07-21T19:41:41Z", + "lastModified": "2026-07-22T08:12:03Z", + "location": "http://localhost:9999/scim/v2/Users/2819c223-7f76-453a-919d-413861904646" + } + }`, string(body)) + }) + + t.Run("omits emails when there are none", func(t *testing.T) { + user.Emails = nil + + body, err := json.Marshal(user) + + require.NoError(t, err) + require.NotContains(t, string(body), "emails") + }) +} diff --git a/internal/api/scim/mapper.go b/internal/api/scim/mapper.go new file mode 100644 index 0000000000..355efbda5a --- /dev/null +++ b/internal/api/scim/mapper.go @@ -0,0 +1,38 @@ +package scim + +import ( + "github.com/supabase/auth/internal/api/scim/core" + "github.com/supabase/auth/internal/models" +) + +type Mapper[TIn, TOut any] interface { + MapFrom(in TIn) TOut +} + +type UserMapper struct { + baseURL string +} + +func NewUserMapper(baseURL string) UserMapper { + return UserMapper{baseURL: baseURL} +} + +func (m UserMapper) MapFrom(u *models.User) *core.User { + id, email := u.ID.String(), u.GetEmail() + + meta := core.NewMeta(m.baseURL, core.ResourceTypeUser, core.EndpointUsers, id) + meta.Created, meta.LastModified = u.CreatedAt.UTC(), u.UpdatedAt.UTC() + + user := &core.User{ + Schemas: []core.SchemaURI{core.SchemaUser}, + ID: id, + UserName: email, + Meta: meta, + } + + if email != "" { + user.Emails = []core.Email{{Value: email, Primary: true}} + } + + return user +} diff --git a/internal/api/scim/mapper_test.go b/internal/api/scim/mapper_test.go new file mode 100644 index 0000000000..b181e5dab7 --- /dev/null +++ b/internal/api/scim/mapper_test.go @@ -0,0 +1,67 @@ +package scim + +import ( + "testing" + "time" + + "github.com/gofrs/uuid" + "github.com/stretchr/testify/require" + "github.com/supabase/auth/internal/api/scim/core" + "github.com/supabase/auth/internal/models" + "github.com/supabase/auth/internal/storage" +) + +func TestUserMapper(t *testing.T) { + id := uuid.Must(uuid.FromString("2819c223-7f76-453a-919d-413861904646")) + createdAt := time.Date(2026, 7, 21, 19, 41, 41, 0, time.UTC) + updatedAt := time.Date(2026, 7, 22, 8, 12, 3, 0, time.UTC) + + newModel := func(email string) *models.User { + return &models.User{ + ID: id, + Email: storage.NullString(email), + CreatedAt: createdAt, + UpdatedAt: updatedAt, + } + } + + mapper := NewUserMapper("http://localhost:9999/scim/v2") + + t.Run("maps a user onto the core User resource", func(t *testing.T) { + user := mapper.MapFrom(newModel("bjensen@example.com")) + + require.Equal(t, []core.SchemaURI{core.SchemaUser}, user.Schemas) + require.Equal(t, id.String(), user.ID) + require.Equal(t, "bjensen@example.com", user.UserName) + require.Equal(t, []core.Email{{Value: "bjensen@example.com", Primary: true}}, user.Emails) + require.Equal(t, core.ResourceTypeUser, user.Meta.ResourceType) + }) + + t.Run("omits emails when the user has no email", func(t *testing.T) { + user := mapper.MapFrom(newModel("")) + + require.Empty(t, user.UserName) + require.Nil(t, user.Emails) + }) + + t.Run("builds the location from the base URL", func(t *testing.T) { + user := NewUserMapper("https://auth.example.com/scim/v2").MapFrom(newModel("bjensen@example.com")) + + require.Equal(t, "https://auth.example.com/scim/v2/Users/"+id.String(), user.Meta.Location) + }) + + t.Run("normalizes the timestamps to UTC", func(t *testing.T) { + model := newModel("bjensen@example.com") + model.CreatedAt = createdAt.In(time.FixedZone("MDT", -6*60*60)) + + user := mapper.MapFrom(model) + + require.Equal(t, time.UTC, user.Meta.Created.Location()) + require.True(t, user.Meta.Created.Equal(createdAt)) + require.Equal(t, updatedAt, user.Meta.LastModified) + }) + + t.Run("satisfies the Mapper interface", func(t *testing.T) { + var _ Mapper[*models.User, *core.User] = NewUserMapper("") + }) +} diff --git a/internal/api/scim/server.go b/internal/api/scim/server.go index d4b479caa7..1d7f0d0146 100644 --- a/internal/api/scim/server.go +++ b/internal/api/scim/server.go @@ -7,18 +7,27 @@ import ( "github.com/supabase/auth/internal/api/scim/core" "github.com/supabase/auth/internal/api/scim/protocol" "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/models" + "github.com/supabase/auth/internal/observability" + "github.com/supabase/auth/internal/storage" ) const BasePath = "/scim/v2" type Server struct { + db *storage.Connection + users Mapper[*models.User, *core.User] serviceProviderConfig *core.ServiceProviderConfig } -func NewServer(config *conf.GlobalConfiguration) *Server { +func NewServer(config *conf.GlobalConfiguration, db *storage.Connection) *Server { + baseURL := strings.TrimRight(config.API.ExternalURL, "/") + BasePath + return &Server{ + db: db, + users: NewUserMapper(baseURL), serviceProviderConfig: core.NewServiceProviderConfig( - strings.TrimRight(config.API.ExternalURL, "/")+BasePath, + baseURL, core.NewOAuthBearerToken().AsPrimary(), ), } @@ -40,6 +49,11 @@ func (srv *Server) NotFound(w http.ResponseWriter, r *http.Request) error { return protocol.SendError(w, http.StatusNotFound, "", "Endpoint or resource does not exist") } +func (srv *Server) internalError(w http.ResponseWriter, r *http.Request, err error) error { + observability.LogEntrySetField(r, "error", err.Error()) + return protocol.SendError(w, http.StatusInternalServerError, "", "Internal server error") +} + func list[T any](w http.ResponseWriter, r *http.Request, resources []T) error { if r.URL.Query().Get("filter") != "" { return protocol.SendError(w, http.StatusForbidden, "", "Filtering is not supported on this endpoint") diff --git a/internal/api/scim/server_test.go b/internal/api/scim/server_test.go index 773638bcdd..2522325a19 100644 --- a/internal/api/scim/server_test.go +++ b/internal/api/scim/server_test.go @@ -15,18 +15,18 @@ import ( //go:embed testdata/* var fixtures embed.FS +func newServerFor(externalURL string) *Server { + return NewServer(&conf.GlobalConfiguration{ + API: conf.APIConfiguration{ExternalURL: externalURL}, + }, nil) +} + func testFixture(t *testing.T, file string) string { data, err := fixtures.ReadFile("testdata/" + file) require.NoError(t, err) return string(data) } -func newServerFor(externalURL string) *Server { - return NewServer(&conf.GlobalConfiguration{ - API: conf.APIConfiguration{ExternalURL: externalURL}, - }) -} - func TestServer(t *testing.T) { srv := newServerFor("http://localhost:9999") require.NotNil(t, srv) diff --git a/internal/api/scim/users.go b/internal/api/scim/users.go new file mode 100644 index 0000000000..78cc44f8ec --- /dev/null +++ b/internal/api/scim/users.go @@ -0,0 +1,33 @@ +package scim + +import ( + "net/http" + + "github.com/go-chi/chi/v5" + "github.com/gofrs/uuid" + "github.com/supabase/auth/internal/api/scim/protocol" + "github.com/supabase/auth/internal/models" +) + +func (srv *Server) UserByID(w http.ResponseWriter, r *http.Request) error { + ctx := r.Context() + + id, err := uuid.FromString(chi.URLParam(r, "id")) + if err != nil { + return userNotFound(w) + } + + user, err := models.FindUserByIDAndSSOProviderID(srv.db.WithContext(ctx), id, providerKey.From(ctx).ID) + if err != nil { + if models.IsNotFoundError(err) { + return userNotFound(w) + } + return srv.internalError(w, r, err) + } + + return protocol.Send(w, http.StatusOK, srv.users.MapFrom(user)) +} + +func userNotFound(w http.ResponseWriter) error { + return protocol.SendError(w, http.StatusNotFound, "", "Resource not found") +} diff --git a/internal/api/scim_test.go b/internal/api/scim_test.go index a6a966823d..a66f5495b8 100644 --- a/internal/api/scim_test.go +++ b/internal/api/scim_test.go @@ -1,15 +1,19 @@ package api import ( + "fmt" "net/http" "net/http/httptest" "net/url" "testing" + "time" + "github.com/gofrs/uuid" "github.com/stretchr/testify/require" scimCore "github.com/supabase/auth/internal/api/scim/core" scimProtocol "github.com/supabase/auth/internal/api/scim/protocol" "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/models" "github.com/supabase/auth/internal/storage" ) @@ -128,3 +132,207 @@ func TestSCIM(t *testing.T) { }) }) } + +type scimTenant struct { + provider *models.SSOProvider + user *models.User + token string +} + +func seedSCIMTenant(t *testing.T, conn *storage.Connection, token, email string) *scimTenant { + t.Helper() + + id := uuid.Must(uuid.NewV4()).String() + provider := &models.SSOProvider{ + SAMLProvider: models.SAMLProvider{ + EntityID: "https://example.com/saml/metadata/" + id, + MetadataXML: "", + }, + SSODomains: []models.SSODomain{ + {Domain: id + ".local"}, + }, + } + provider.UpdateSCIMToken(token) + require.NoError(t, conn.Eager().Create(provider)) + + user, err := models.NewUser("", email, "", "authenticated", nil) + require.NoError(t, err) + user.IsSSOUser = true + require.NoError(t, conn.Create(user)) + + identity, err := models.NewIdentity(user, "sso:"+provider.ID.String(), map[string]interface{}{ + "sub": user.ID.String(), + "email": email, + }) + require.NoError(t, err) + require.NoError(t, conn.Create(identity)) + + return &scimTenant{provider: provider, user: user, token: token} +} + +func TestSCIMUsers(t *testing.T) { + var a, b *scimTenant + var conn *storage.Connection + + api, config, err := setupAPIForTestWithCallback(func(cfg *conf.GlobalConfiguration, c *storage.Connection) { + if cfg != nil { + cfg.Experimental.ScimEnabled = true + return + } + conn = c + require.NoError(t, models.TruncateAll(c)) + a = seedSCIMTenant(t, c, "scim_token_a", "a@example.com") + b = seedSCIMTenant(t, c, "scim_token_b", "b@example.com") + }) + require.NoError(t, err) + + get := func(id, token string) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+id, nil) + if token != "" { + r.Header.Set("Authorization", "Bearer "+token) + } + w := httptest.NewRecorder() + api.handler.ServeHTTP(w, r) + return w + } + + t.Run("returns the user that belongs to the token's provider", func(t *testing.T) { + w := get(a.user.ID.String(), a.token) + + require.Equal(t, http.StatusOK, w.Code) + require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) + require.JSONEq(t, fmt.Sprintf(`{ + "schemas": [%q], + "id": %q, + "userName": "a@example.com", + "emails": [{"value": "a@example.com", "primary": true}], + "meta": { + "resourceType": "User", + "created": %q, + "lastModified": %q, + "location": "%s/scim/v2/Users/%s" + } + }`, + scimCore.SchemaUser, + a.user.ID, + a.user.CreatedAt.UTC().Format(time.RFC3339Nano), + a.user.UpdatedAt.UTC().Format(time.RFC3339Nano), + config.API.ExternalURL, a.user.ID, + ), w.Body.String()) + }) + + t.Run("scopes each provider to its own users", func(t *testing.T) { + require.Equal(t, http.StatusOK, get(b.user.ID.String(), b.token).Code) + }) + + t.Run("hides a user belonging to another provider", func(t *testing.T) { + w := get(b.user.ID.String(), a.token) + + require.Equal(t, http.StatusNotFound, w.Code) + require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) + require.Contains(t, w.Body.String(), scimProtocol.SchemaError) + }) + + t.Run("returns the same 404 for an unknown id", func(t *testing.T) { + unknown := get(uuid.Must(uuid.NewV4()).String(), a.token) + other := get(b.user.ID.String(), a.token) + + require.Equal(t, http.StatusNotFound, unknown.Code) + require.Equal(t, other.Body.String(), unknown.Body.String()) + }) + + t.Run("returns 404 for a malformed id", func(t *testing.T) { + w := get("not-a-uuid", a.token) + + require.Equal(t, http.StatusNotFound, w.Code) + require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) + }) + + t.Run("requires a bearer token", func(t *testing.T) { + w := get(a.user.ID.String(), "") + + require.Equal(t, http.StatusUnauthorized, w.Code) + require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) + require.Equal(t, "Bearer", w.Header().Get("WWW-Authenticate")) + require.Contains(t, w.Body.String(), scimProtocol.SchemaError) + }) + + t.Run("rejects an unknown token", func(t *testing.T) { + w := get(a.user.ID.String(), "scim_nope") + + require.Equal(t, http.StatusUnauthorized, w.Code) + require.Equal(t, "Bearer", w.Header().Get("WWW-Authenticate")) + }) + + t.Run("rejects a disabled provider", func(t *testing.T) { + disabled := true + b.provider.Disabled = &disabled + require.NoError(t, conn.Update(b.provider)) + defer func() { + b.provider.Disabled = nil + require.NoError(t, conn.Update(b.provider)) + }() + + w := get(b.user.ID.String(), b.token) + + require.Equal(t, http.StatusForbidden, w.Code) + require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) + }) + + t.Run("stays hidden when the feature flag is off", func(t *testing.T) { + disabled, _, err := setupAPIForTest() + require.NoError(t, err) + + r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+a.user.ID.String(), nil) + r.Header.Set("Authorization", "Bearer "+a.token) + w := httptest.NewRecorder() + disabled.handler.ServeHTTP(w, r) + + require.Equal(t, http.StatusNotFound, w.Code) + require.Equal(t, "application/json", w.Header().Get("Content-Type")) + require.NotContains(t, w.Body.String(), scimProtocol.SchemaError) + }) +} + +func TestSCIMInfrastructureFailure(t *testing.T) { + var tenant *scimTenant + var conn *storage.Connection + + api, _, err := setupAPIForTestWithCallback(func(cfg *conf.GlobalConfiguration, c *storage.Connection) { + if cfg != nil { + cfg.Experimental.ScimEnabled = true + return + } + conn = c + require.NoError(t, models.TruncateAll(c)) + tenant = seedSCIMTenant(t, c, "scim_token_unreachable", "unreachable@example.com") + }) + require.NoError(t, err) + + rename := func(t *testing.T, from, to string) { + t.Helper() + require.NoError(t, conn.RawQuery("alter table "+from+" rename to "+to).Exec()) + } + + // Each table stands in for a database that fails one of the two queries a + // SCIM request makes, for a reason other than the row being absent. + for _, table := range []string{"sso_providers", "users"} { + t.Run("answers in the SCIM error form when "+table+" cannot be queried", func(t *testing.T) { + rename(t, table, table+"_renamed") + defer rename(t, table+"_renamed", table) + + r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+tenant.user.ID.String(), nil) + r.Header.Set("Authorization", "Bearer "+tenant.token) + w := httptest.NewRecorder() + api.handler.ServeHTTP(w, r) + + require.Equal(t, http.StatusInternalServerError, w.Code) + require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) + require.JSONEq(t, `{ + "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], + "detail": "Unexpected failure, please check server logs for more information", + "status": "500" + }`, w.Body.String()) + }) + } +} diff --git a/internal/models/user.go b/internal/models/user.go index f88a9729b8..6ce01a4e24 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -709,6 +709,25 @@ func FindUserByID(tx *storage.Connection, id uuid.UUID) (*User, error) { return findUser(tx, "instance_id = ? and id = ?", uuid.Nil, id) } +// FindUserByIDAndSSOProviderID finds a user matching the provided SSO provider ID and ID +func FindUserByIDAndSSOProviderID(tx *storage.Connection, id, ssoProviderID uuid.UUID) (*User, error) { + obj := &User{} + // Skip findUser's eager loading + query := tx.Q().Where( + "instance_id = ? and id = ? and deleted_at is null and is_sso_user = true and id in (select user_id from identities where provider = ?)", + uuid.Nil, id, "sso:"+ssoProviderID.String(), + ) + + if err := query.First(obj); err != nil { + if errors.Cause(err) == sql.ErrNoRows { + return nil, UserNotFoundError{} + } + return nil, errors.Wrap(err, "error finding user") + } + + return obj, nil +} + // FindUserWithRefreshToken finds a user from the provided refresh token. If // forUpdate is set to true, then the SELECT statement used by the query has // the form SELECT ... FOR UPDATE SKIP LOCKED. This means that a FOR UPDATE diff --git a/internal/models/user_test.go b/internal/models/user_test.go index 502392605e..9db56ff0f0 100644 --- a/internal/models/user_test.go +++ b/internal/models/user_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/gofrs/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -856,3 +857,77 @@ func (ts *UserTestSuite) TestAuthenticate() { }) } } + +func (ts *UserTestSuite) TestFindUserByIDAndSSOProviderID() { + providerA := uuid.Must(uuid.NewV4()) + providerB := uuid.Must(uuid.NewV4()) + + createSSOUser := func(email string, providerID uuid.UUID) *User { + u, err := NewUser("", email, "", "authenticated", nil) + require.NoError(ts.T(), err) + u.IsSSOUser = true + require.NoError(ts.T(), ts.db.Create(u)) + + identity, err := NewIdentity(u, "sso:"+providerID.String(), map[string]interface{}{ + "sub": u.ID.String(), + "email": email, + }) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.db.Create(identity)) + + return u + } + + userA := createSSOUser("a@example.com", providerA) + userB := createSSOUser("b@example.com", providerB) + + ts.Run("finds a user belonging to the provider", func() { + found, err := FindUserByIDAndSSOProviderID(ts.db, userA.ID, providerA) + + require.NoError(ts.T(), err) + require.Equal(ts.T(), userA.ID, found.ID) + }) + + ts.Run("does not find a user belonging to another provider", func() { + found, err := FindUserByIDAndSSOProviderID(ts.db, userB.ID, providerA) + + require.Nil(ts.T(), found) + require.True(ts.T(), IsNotFoundError(err)) + }) + + ts.Run("does not find an unknown id", func() { + found, err := FindUserByIDAndSSOProviderID(ts.db, uuid.Must(uuid.NewV4()), providerA) + + require.Nil(ts.T(), found) + require.True(ts.T(), IsNotFoundError(err)) + }) + + ts.Run("does not find a non-SSO user", func() { + u, err := NewUser("", "plain@example.com", "", "authenticated", nil) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.db.Create(u)) + + identity, err := NewIdentity(u, "sso:"+providerA.String(), map[string]interface{}{ + "sub": u.ID.String(), + "email": "plain@example.com", + }) + require.NoError(ts.T(), err) + require.NoError(ts.T(), ts.db.Create(identity)) + + found, err := FindUserByIDAndSSOProviderID(ts.db, u.ID, providerA) + + require.Nil(ts.T(), found) + require.True(ts.T(), IsNotFoundError(err)) + }) + + ts.Run("does not find a soft deleted user", func() { + deletedAt := time.Now() + userA.DeletedAt = &deletedAt + require.NoError(ts.T(), ts.db.Update(userA)) + + found, err := FindUserByIDAndSSOProviderID(ts.db, userA.ID, providerA) + + require.Nil(ts.T(), found) + require.True(ts.T(), IsNotFoundError(err)) + }) +} From b741d1878dcf24c4a8785f47c6de93ea9c5f0883 Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 6 Aug 2026 15:26:05 -0600 Subject: [PATCH 03/18] refactor(scim): delegate to current bearer token extractor --- internal/api/api.go | 2 +- internal/api/scim/authenticate.go | 19 ++-------- internal/api/scim/authenticate_test.go | 48 -------------------------- internal/api/scim/server.go | 10 ++++-- internal/api/scim/server_test.go | 2 +- 5 files changed, 11 insertions(+), 70 deletions(-) delete mode 100644 internal/api/scim/authenticate_test.go diff --git a/internal/api/api.go b/internal/api/api.go index 8353ec451d..75cc4cc04a 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -138,7 +138,7 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne api.oauthServer = oauthserver.NewServer(globalConfig, db, api.tokenService) } - api.scim = scim.NewServer(globalConfig, db) + api.scim = scim.NewServer(globalConfig, db, api.extractBearerToken) if api.config.Password.HIBP.Enabled { httpClient := &http.Client{ diff --git a/internal/api/scim/authenticate.go b/internal/api/scim/authenticate.go index 59ff25d2c1..9eb55ac9bb 100644 --- a/internal/api/scim/authenticate.go +++ b/internal/api/scim/authenticate.go @@ -3,7 +3,6 @@ package scim import ( "context" "net/http" - "strings" "github.com/supabase/auth/internal/api/scim/protocol" "github.com/supabase/auth/internal/models" @@ -25,8 +24,8 @@ func (srv *Server) Authenticate(next http.Handler) http.Handler { func (srv *Server) authenticate(w http.ResponseWriter, r *http.Request) (context.Context, bool) { ctx := r.Context() - token, ok := parseBearerToken(r.Header.Get("Authorization")) - if !ok { + token, err := srv.extract(r) + if err != nil { unauthorized(w) return nil, false } @@ -51,20 +50,6 @@ func (srv *Server) authenticate(w http.ResponseWriter, r *http.Request) (context return providerKey.With(ctx, provider), true } -func parseBearerToken(header string) (string, bool) { - scheme, rest, found := strings.Cut(header, " ") - if !found || !strings.EqualFold(scheme, "Bearer") { - return "", false - } - - token := strings.TrimSpace(rest) - if token == "" || strings.ContainsAny(token, " \t\r\n\v\f") { - return "", false - } - - return token, true -} - func unauthorized(w http.ResponseWriter) error { w.Header().Set("WWW-Authenticate", "Bearer") return protocol.SendError(w, http.StatusUnauthorized, "", "A valid SCIM bearer token is required") diff --git a/internal/api/scim/authenticate_test.go b/internal/api/scim/authenticate_test.go deleted file mode 100644 index 129f39801d..0000000000 --- a/internal/api/scim/authenticate_test.go +++ /dev/null @@ -1,48 +0,0 @@ -package scim - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestParseBearerToken(t *testing.T) { - // RFC 7235, Section 2.1: credentials = auth-scheme 1*SP token68. - // The scheme is case insensitive and one or more spaces may separate it - // from the token. - accepted := map[string]string{ - "Bearer tok": "tok", - "bearer tok": "tok", - "BEARER tok": "tok", - "Bearer tok": "tok", - "Bearer tok ": "tok", - "Bearer \ttok": "tok", - } - - for header, want := range accepted { - t.Run(header, func(t *testing.T) { - got, ok := parseBearerToken(header) - - require.True(t, ok) - require.Equal(t, want, got) - }) - } - - rejected := []string{ - "", - "Bearer", - "Bearer ", - "Basic tok", - "Bearertok", - "Bearer tok extra", - } - - for _, header := range rejected { - t.Run("rejects "+header, func(t *testing.T) { - got, ok := parseBearerToken(header) - - require.False(t, ok) - require.Empty(t, got) - }) - } -} diff --git a/internal/api/scim/server.go b/internal/api/scim/server.go index 1d7f0d0146..2136e579dc 100644 --- a/internal/api/scim/server.go +++ b/internal/api/scim/server.go @@ -14,18 +14,22 @@ import ( const BasePath = "/scim/v2" +type TokenExtractor func(r *http.Request) (string, error) + type Server struct { db *storage.Connection + extract TokenExtractor users Mapper[*models.User, *core.User] serviceProviderConfig *core.ServiceProviderConfig } -func NewServer(config *conf.GlobalConfiguration, db *storage.Connection) *Server { +func NewServer(config *conf.GlobalConfiguration, db *storage.Connection, extract TokenExtractor) *Server { baseURL := strings.TrimRight(config.API.ExternalURL, "/") + BasePath return &Server{ - db: db, - users: NewUserMapper(baseURL), + db: db, + extract: extract, + users: NewUserMapper(baseURL), serviceProviderConfig: core.NewServiceProviderConfig( baseURL, core.NewOAuthBearerToken().AsPrimary(), diff --git a/internal/api/scim/server_test.go b/internal/api/scim/server_test.go index 2522325a19..8c15804416 100644 --- a/internal/api/scim/server_test.go +++ b/internal/api/scim/server_test.go @@ -18,7 +18,7 @@ var fixtures embed.FS func newServerFor(externalURL string) *Server { return NewServer(&conf.GlobalConfiguration{ API: conf.APIConfiguration{ExternalURL: externalURL}, - }, nil) + }, nil, nil) } func testFixture(t *testing.T, file string) string { From 05874da829a2c50af8c3c47a17b1c3e2c6cf9b7c Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 6 Aug 2026 15:40:10 -0600 Subject: [PATCH 04/18] refactor(scim): use existing context key --- internal/api/scim/authenticate.go | 5 ++- internal/api/scim/context.go | 27 -------------- internal/api/scim/context_test.go | 60 ------------------------------- internal/api/scim/users.go | 4 ++- internal/api/shared/context.go | 16 +++++++++ 5 files changed, 21 insertions(+), 91 deletions(-) delete mode 100644 internal/api/scim/context.go delete mode 100644 internal/api/scim/context_test.go diff --git a/internal/api/scim/authenticate.go b/internal/api/scim/authenticate.go index 9eb55ac9bb..f2cf87a984 100644 --- a/internal/api/scim/authenticate.go +++ b/internal/api/scim/authenticate.go @@ -5,12 +5,11 @@ import ( "net/http" "github.com/supabase/auth/internal/api/scim/protocol" + "github.com/supabase/auth/internal/api/shared" "github.com/supabase/auth/internal/models" "github.com/supabase/auth/internal/observability" ) -var providerKey = NewKey[*models.SSOProvider]("sso_provider") - func (srv *Server) Authenticate(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx, ok := srv.authenticate(w, r) @@ -47,7 +46,7 @@ func (srv *Server) authenticate(w http.ResponseWriter, r *http.Request) (context observability.LogEntrySetField(r, "sso_provider_id", provider.ID.String()) - return providerKey.With(ctx, provider), true + return shared.WithSSOProvider(ctx, provider), true } func unauthorized(w http.ResponseWriter) error { diff --git a/internal/api/scim/context.go b/internal/api/scim/context.go deleted file mode 100644 index 3611c66fa5..0000000000 --- a/internal/api/scim/context.go +++ /dev/null @@ -1,27 +0,0 @@ -package scim - -import "context" - -type Key[T any] struct { - name string -} - -func NewKey[T any](name string) Key[T] { - return Key[T]{name: name} -} - -// String makes the key legible when a context is printed. Without it two -// keys of the same T are indistinguishable, since context renders the key by -// type name. -func (k Key[T]) String() string { - return "gotrue scim context key " + k.name -} - -func (k Key[T]) With(ctx context.Context, value T) context.Context { - return context.WithValue(ctx, k, value) -} - -func (k Key[T]) From(ctx context.Context) T { - value, _ := ctx.Value(k).(T) - return value -} diff --git a/internal/api/scim/context_test.go b/internal/api/scim/context_test.go deleted file mode 100644 index e5ad3f18d0..0000000000 --- a/internal/api/scim/context_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package scim - -import ( - "context" - "fmt" - "testing" - - "github.com/gofrs/uuid" - "github.com/stretchr/testify/require" - "github.com/supabase/auth/internal/models" -) - -func TestKey(t *testing.T) { - key := NewKey[*models.SSOProvider]("sso_provider") - - t.Run("round trips a typed value", func(t *testing.T) { - provider := &models.SSOProvider{ID: uuid.Must(uuid.NewV4())} - - ctx := key.With(context.Background(), provider) - - require.Equal(t, provider, key.From(ctx)) - }) - - t.Run("returns the zero value when absent", func(t *testing.T) { - require.Nil(t, key.From(context.Background())) - }) - - t.Run("keys of different types do not collide", func(t *testing.T) { - other := NewKey[string]("sso_provider") - - ctx := other.With(context.Background(), "not a provider") - - require.Nil(t, key.From(ctx)) - require.Equal(t, "not a provider", other.From(ctx)) - }) - - t.Run("keys of the same type and name share a slot", func(t *testing.T) { - provider := &models.SSOProvider{ID: uuid.Must(uuid.NewV4())} - - ctx := NewKey[*models.SSOProvider]("sso_provider").With(context.Background(), provider) - - require.Equal(t, provider, key.From(ctx)) - }) - - t.Run("keys of the same type but a different name do not collide", func(t *testing.T) { - first, second := NewKey[string]("first"), NewKey[string]("second") - - ctx := first.With(context.Background(), "one") - ctx = second.With(ctx, "two") - - require.Equal(t, "one", first.From(ctx)) - require.Equal(t, "two", second.From(ctx)) - }) - - t.Run("names the key when a context is printed", func(t *testing.T) { - ctx := NewKey[string]("first").With(context.Background(), "one") - - require.Contains(t, fmt.Sprint(ctx), "gotrue scim context key first") - }) -} diff --git a/internal/api/scim/users.go b/internal/api/scim/users.go index 78cc44f8ec..cbdca95d75 100644 --- a/internal/api/scim/users.go +++ b/internal/api/scim/users.go @@ -6,6 +6,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/gofrs/uuid" "github.com/supabase/auth/internal/api/scim/protocol" + "github.com/supabase/auth/internal/api/shared" "github.com/supabase/auth/internal/models" ) @@ -17,7 +18,8 @@ func (srv *Server) UserByID(w http.ResponseWriter, r *http.Request) error { return userNotFound(w) } - user, err := models.FindUserByIDAndSSOProviderID(srv.db.WithContext(ctx), id, providerKey.From(ctx).ID) + provider := shared.GetSSOProvider(ctx) + user, err := models.FindUserByIDAndSSOProviderID(srv.db.WithContext(ctx), id, provider.ID) if err != nil { if models.IsNotFoundError(err) { return userNotFound(w) diff --git a/internal/api/shared/context.go b/internal/api/shared/context.go index 81bfd47521..77cdb4529b 100644 --- a/internal/api/shared/context.go +++ b/internal/api/shared/context.go @@ -18,6 +18,7 @@ const ( UserKey ContextKey = "user" SessionKey ContextKey = "session" OAuthServerClientKey ContextKey = "oauth_server_client" + SSOProviderKey ContextKey = "sso_provider" ) // GetUser reads the user from the context - shared implementation @@ -70,3 +71,18 @@ func GetOAuthServerClient(ctx context.Context) *models.OAuthServerClient { } return obj.(*models.OAuthServerClient) } + +func GetSSOProvider(ctx context.Context) *models.SSOProvider { + if ctx == nil { + return nil + } + obj := ctx.Value(SSOProviderKey) + if obj == nil { + return nil + } + return obj.(*models.SSOProvider) +} + +func WithSSOProvider(ctx context.Context, s *models.SSOProvider) context.Context { + return context.WithValue(ctx, SSOProviderKey, s) +} From 3a868caedcc132f5f1034189feb925014eddeed3 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 08:36:16 -0600 Subject: [PATCH 05/18] refactor: convert ContextKey to generic lookup --- internal/api/context.go | 9 +---- internal/api/shared/context.go | 72 ++++++++++++++-------------------- 2 files changed, 32 insertions(+), 49 deletions(-) diff --git a/internal/api/context.go b/internal/api/context.go index f8367a4abc..834621724c 100644 --- a/internal/api/context.go +++ b/internal/api/context.go @@ -31,7 +31,6 @@ const ( adminUserKey = contextKey("admin_user") oauthTokenKey = contextKey("oauth_token") // for OAuth1.0, also known as request token oauthVerifierKey = contextKey("oauth_verifier") - ssoProviderKey = contextKey("sso_provider") externalHostKey = contextKey("external_host") oauthClientStateKey = contextKey("oauth_client_state_id") flowStateContextKey = contextKey("flow_state") @@ -239,15 +238,11 @@ func getOAuthVerifier(ctx context.Context) string { } func withSSOProvider(ctx context.Context, provider *models.SSOProvider) context.Context { - return context.WithValue(ctx, ssoProviderKey, provider) + return shared.WithSSOProvider(ctx, provider) } func getSSOProvider(ctx context.Context) *models.SSOProvider { - obj := ctx.Value(ssoProviderKey) - if obj == nil { - return nil - } - return obj.(*models.SSOProvider) + return shared.GetSSOProvider(ctx) } func withExternalHost(ctx context.Context, u *url.URL) context.Context { diff --git a/internal/api/shared/context.go b/internal/api/shared/context.go index 77cdb4529b..45e1c175b1 100644 --- a/internal/api/shared/context.go +++ b/internal/api/shared/context.go @@ -7,82 +7,70 @@ import ( ) // ContextKey is the type for context keys to avoid collisions -type ContextKey string +type ContextKey[T any] string -func (c ContextKey) String() string { +func (c ContextKey[T]) String() string { return "gotrue api context key " + string(c) } +func (key ContextKey[T]) Get(ctx context.Context) T { + var zero T + if ctx == nil { + return zero + } + obj := ctx.Value(key) + if obj == nil { + return zero + } + return obj.(T) +} + +func (key ContextKey[T]) With(ctx context.Context, t T) context.Context { + return context.WithValue(ctx, key, t) +} + // Context keys used across packages const ( - UserKey ContextKey = "user" - SessionKey ContextKey = "session" - OAuthServerClientKey ContextKey = "oauth_server_client" - SSOProviderKey ContextKey = "sso_provider" + UserKey ContextKey[*models.User] = "user" + SessionKey ContextKey[*models.Session] = "session" + OAuthServerClientKey ContextKey[*models.OAuthServerClient] = "oauth_server_client" + SSOProviderKey ContextKey[*models.SSOProvider] = "sso_provider" ) // GetUser reads the user from the context - shared implementation func GetUser(ctx context.Context) *models.User { - if ctx == nil { - return nil - } - obj := ctx.Value(UserKey) - if obj == nil { - return nil - } - return obj.(*models.User) + return UserKey.Get(ctx) } // WithUser adds the user to the context - shared implementation func WithUser(ctx context.Context, u *models.User) context.Context { - return context.WithValue(ctx, UserKey, u) + return UserKey.With(ctx, u) } // GetSession reads the session from the context - shared implementation func GetSession(ctx context.Context) *models.Session { - if ctx == nil { - return nil - } - obj := ctx.Value(SessionKey) - if obj == nil { - return nil - } - return obj.(*models.Session) + return SessionKey.Get(ctx) } // WithSession adds the session to the context - shared implementation func WithSession(ctx context.Context, s *models.Session) context.Context { - return context.WithValue(ctx, SessionKey, s) + return SessionKey.With(ctx, s) } // WithOAuthServerClient adds an OAuth server client to the context func WithOAuthServerClient(ctx context.Context, client *models.OAuthServerClient) context.Context { - return context.WithValue(ctx, OAuthServerClientKey, client) + return OAuthServerClientKey.With(ctx, client) } // GetOAuthServerClient retrieves an OAuth server client from the context func GetOAuthServerClient(ctx context.Context) *models.OAuthServerClient { - if ctx == nil { - return nil - } - obj := ctx.Value(OAuthServerClientKey) - if obj == nil { - return nil - } - return obj.(*models.OAuthServerClient) + return OAuthServerClientKey.Get(ctx) } func GetSSOProvider(ctx context.Context) *models.SSOProvider { - if ctx == nil { - return nil - } - obj := ctx.Value(SSOProviderKey) - if obj == nil { - return nil - } - return obj.(*models.SSOProvider) + return SSOProviderKey.Get(ctx) } func WithSSOProvider(ctx context.Context, s *models.SSOProvider) context.Context { - return context.WithValue(ctx, SSOProviderKey, s) + return SSOProviderKey.With(ctx, s) } From caef6e885811b9b4b97366093aba16a37d5448ce Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 10:41:46 -0600 Subject: [PATCH 06/18] refactor: rename self to match other declarations --- internal/api/shared/context.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/api/shared/context.go b/internal/api/shared/context.go index 45e1c175b1..be6572063b 100644 --- a/internal/api/shared/context.go +++ b/internal/api/shared/context.go @@ -13,20 +13,20 @@ func (c ContextKey[T]) String() string { return "gotrue api context key " + string(c) } -func (key ContextKey[T]) Get(ctx context.Context) T { +func (c ContextKey[T]) Get(ctx context.Context) T { var zero T if ctx == nil { return zero } - obj := ctx.Value(key) + obj := ctx.Value(c) if obj == nil { return zero } return obj.(T) } -func (key ContextKey[T]) With(ctx context.Context, t T) context.Context { - return context.WithValue(ctx, key, t) +func (c ContextKey[T]) With(ctx context.Context, t T) context.Context { + return context.WithValue(ctx, c, t) } // Context keys used across packages From 893274aa633d41fbe11c0a9c477abffb25d1c6ca Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 11:22:20 -0600 Subject: [PATCH 07/18] refactor: extract a provisioned user type --- internal/api/scim/core/user.go | 13 +++++ internal/api/scim/core/user_test.go | 18 ++++++ internal/api/scim/mapper.go | 41 +++++++++++-- internal/api/scim/mapper_test.go | 90 +++++++++++++++++++++++++---- internal/api/scim/server.go | 2 +- internal/api/scim/users.go | 2 +- internal/api/scim_test.go | 50 +++++++++++++++- internal/models/identity.go | 12 ++++ internal/models/provisioned_user.go | 20 +++++++ internal/models/sso.go | 29 ++++++++++ 10 files changed, 255 insertions(+), 22 deletions(-) create mode 100644 internal/models/provisioned_user.go diff --git a/internal/api/scim/core/user.go b/internal/api/scim/core/user.go index c39237cc54..0ad290d7e7 100644 --- a/internal/api/scim/core/user.go +++ b/internal/api/scim/core/user.go @@ -5,11 +5,24 @@ type Email struct { Primary bool `json:"primary"` } +// Name holds the components of the user's name, per RFC 7643, Section 4.1.1. +type Name struct { + Formatted string `json:"formatted,omitempty"` + FamilyName string `json:"familyName,omitempty"` + GivenName string `json:"givenName,omitempty"` + MiddleName string `json:"middleName,omitempty"` +} + +func (n Name) IsZero() bool { + return n == Name{} +} + // User is the core User resource defined in RFC 7643, Section 4.1. type User struct { Schemas []SchemaURI `json:"schemas"` ID string `json:"id"` UserName string `json:"userName"` + Name *Name `json:"name,omitempty"` Emails []Email `json:"emails,omitempty"` Meta Meta `json:"meta"` } diff --git a/internal/api/scim/core/user_test.go b/internal/api/scim/core/user_test.go index 4db1ba0a1d..d1003ad1be 100644 --- a/internal/api/scim/core/user_test.go +++ b/internal/api/scim/core/user_test.go @@ -51,4 +51,22 @@ func TestUser(t *testing.T) { require.NoError(t, err) require.NotContains(t, string(body), "emails") }) + + t.Run("omits the name when there is none", func(t *testing.T) { + user.Name = nil + + body, err := json.Marshal(user) + + require.NoError(t, err) + require.NotContains(t, string(body), `"name"`) + }) + + t.Run("serializes only the name components that are set", func(t *testing.T) { + user.Name = &Name{FamilyName: "Jensen", GivenName: "Barbara"} + + body, err := json.Marshal(user) + + require.NoError(t, err) + require.Contains(t, string(body), `"name":{"familyName":"Jensen","givenName":"Barbara"}`) + }) } diff --git a/internal/api/scim/mapper.go b/internal/api/scim/mapper.go index 355efbda5a..d91c332ece 100644 --- a/internal/api/scim/mapper.go +++ b/internal/api/scim/mapper.go @@ -17,22 +17,51 @@ func NewUserMapper(baseURL string) UserMapper { return UserMapper{baseURL: baseURL} } -func (m UserMapper) MapFrom(u *models.User) *core.User { - id, email := u.ID.String(), u.GetEmail() +func (m UserMapper) MapFrom(in models.ProvisionedUser) *core.User { + id := in.ID.String() meta := core.NewMeta(m.baseURL, core.ResourceTypeUser, core.EndpointUsers, id) - meta.Created, meta.LastModified = u.CreatedAt.UTC(), u.UpdatedAt.UTC() + meta.Created, meta.LastModified = in.CreatedAt.UTC(), in.UpdatedAt.UTC() user := &core.User{ Schemas: []core.SchemaURI{core.SchemaUser}, ID: id, - UserName: email, + UserName: userName(in), + Name: name(in), Meta: meta, } - if email != "" { - user.Emails = []core.Email{{Value: email, Primary: true}} + if address := email(in); address != "" { + user.Emails = []core.Email{{Value: address, Primary: true}} } return user } + +func email(in models.ProvisionedUser) string { + if email := in.Claim("email"); email != "" { + return email + } + return in.GetEmail() +} + +func userName(in models.ProvisionedUser) string { + if userName := in.Claim("preferred_username"); userName != "" { + return userName + } + return email(in) +} + +func name(in models.ProvisionedUser) *core.Name { + name := core.Name{ + Formatted: in.Claim("name"), + FamilyName: in.Claim("family_name"), + GivenName: in.Claim("given_name"), + MiddleName: in.Claim("middle_name"), + } + + if name.IsZero() { + return nil + } + return &name +} diff --git a/internal/api/scim/mapper_test.go b/internal/api/scim/mapper_test.go index b181e5dab7..eb5d035d62 100644 --- a/internal/api/scim/mapper_test.go +++ b/internal/api/scim/mapper_test.go @@ -16,19 +16,25 @@ func TestUserMapper(t *testing.T) { createdAt := time.Date(2026, 7, 21, 19, 41, 41, 0, time.UTC) updatedAt := time.Date(2026, 7, 22, 8, 12, 3, 0, time.UTC) - newModel := func(email string) *models.User { - return &models.User{ - ID: id, - Email: storage.NullString(email), - CreatedAt: createdAt, - UpdatedAt: updatedAt, + newModel := func(email string, claims map[string]interface{}) models.ProvisionedUser { + in := models.ProvisionedUser{ + User: &models.User{ + ID: id, + Email: storage.NullString(email), + CreatedAt: createdAt, + UpdatedAt: updatedAt, + }, } + if claims != nil { + in.Identity = &models.Identity{IdentityData: claims} + } + return in } mapper := NewUserMapper("http://localhost:9999/scim/v2") t.Run("maps a user onto the core User resource", func(t *testing.T) { - user := mapper.MapFrom(newModel("bjensen@example.com")) + user := mapper.MapFrom(newModel("bjensen@example.com", nil)) require.Equal(t, []core.SchemaURI{core.SchemaUser}, user.Schemas) require.Equal(t, id.String(), user.ID) @@ -38,20 +44,20 @@ func TestUserMapper(t *testing.T) { }) t.Run("omits emails when the user has no email", func(t *testing.T) { - user := mapper.MapFrom(newModel("")) + user := mapper.MapFrom(newModel("", nil)) require.Empty(t, user.UserName) require.Nil(t, user.Emails) }) t.Run("builds the location from the base URL", func(t *testing.T) { - user := NewUserMapper("https://auth.example.com/scim/v2").MapFrom(newModel("bjensen@example.com")) + user := NewUserMapper("https://auth.example.com/scim/v2").MapFrom(newModel("bjensen@example.com", nil)) require.Equal(t, "https://auth.example.com/scim/v2/Users/"+id.String(), user.Meta.Location) }) t.Run("normalizes the timestamps to UTC", func(t *testing.T) { - model := newModel("bjensen@example.com") + model := newModel("bjensen@example.com", nil) model.CreatedAt = createdAt.In(time.FixedZone("MDT", -6*60*60)) user := mapper.MapFrom(model) @@ -61,7 +67,69 @@ func TestUserMapper(t *testing.T) { require.Equal(t, updatedAt, user.Meta.LastModified) }) + t.Run("prefers the email the provider supplied over the user record", func(t *testing.T) { + user := mapper.MapFrom(newModel("stale@example.com", map[string]interface{}{ + "email": "bjensen@example.com", + })) + + require.Equal(t, "bjensen@example.com", user.UserName) + require.Equal(t, []core.Email{{Value: "bjensen@example.com", Primary: true}}, user.Emails) + }) + + t.Run("falls back to the user record when the provider supplied no email", func(t *testing.T) { + user := mapper.MapFrom(newModel("bjensen@example.com", map[string]interface{}{ + "sub": id.String(), + })) + + require.Equal(t, "bjensen@example.com", user.UserName) + require.Equal(t, []core.Email{{Value: "bjensen@example.com", Primary: true}}, user.Emails) + }) + + t.Run("prefers preferred_username for the userName", func(t *testing.T) { + user := mapper.MapFrom(newModel("", map[string]interface{}{ + "preferred_username": "bjensen", + "email": "bjensen@example.com", + })) + + require.Equal(t, "bjensen", user.UserName) + require.Equal(t, []core.Email{{Value: "bjensen@example.com", Primary: true}}, user.Emails) + }) + + t.Run("maps the name components the provider supplied", func(t *testing.T) { + user := mapper.MapFrom(newModel("", map[string]interface{}{ + "name": "Ms. Barbara Jane Jensen, III", + "family_name": "Jensen", + "given_name": "Barbara", + "middle_name": "Jane", + })) + + require.Equal(t, &core.Name{ + Formatted: "Ms. Barbara Jane Jensen, III", + FamilyName: "Jensen", + GivenName: "Barbara", + MiddleName: "Jane", + }, user.Name) + }) + + t.Run("omits the name when the provider supplied no components", func(t *testing.T) { + require.Nil(t, mapper.MapFrom(newModel("bjensen@example.com", nil)).Name) + require.Nil(t, mapper.MapFrom(newModel("bjensen@example.com", map[string]interface{}{ + "sub": id.String(), + })).Name) + }) + + t.Run("ignores claims that are not strings", func(t *testing.T) { + user := mapper.MapFrom(newModel("bjensen@example.com", map[string]interface{}{ + "email": 12345, + "given_name": []string{"Barbara"}, + "family_name": "Jensen", + })) + + require.Equal(t, "bjensen@example.com", user.UserName) + require.Equal(t, &core.Name{FamilyName: "Jensen"}, user.Name) + }) + t.Run("satisfies the Mapper interface", func(t *testing.T) { - var _ Mapper[*models.User, *core.User] = NewUserMapper("") + var _ Mapper[models.ProvisionedUser, *core.User] = NewUserMapper("") }) } diff --git a/internal/api/scim/server.go b/internal/api/scim/server.go index 2136e579dc..336a6988c8 100644 --- a/internal/api/scim/server.go +++ b/internal/api/scim/server.go @@ -19,7 +19,7 @@ type TokenExtractor func(r *http.Request) (string, error) type Server struct { db *storage.Connection extract TokenExtractor - users Mapper[*models.User, *core.User] + users Mapper[models.ProvisionedUser, *core.User] serviceProviderConfig *core.ServiceProviderConfig } diff --git a/internal/api/scim/users.go b/internal/api/scim/users.go index cbdca95d75..5afc3dbf6f 100644 --- a/internal/api/scim/users.go +++ b/internal/api/scim/users.go @@ -19,7 +19,7 @@ func (srv *Server) UserByID(w http.ResponseWriter, r *http.Request) error { } provider := shared.GetSSOProvider(ctx) - user, err := models.FindUserByIDAndSSOProviderID(srv.db.WithContext(ctx), id, provider.ID) + user, err := provider.FindProvisionedUserByID(srv.db.WithContext(ctx), id) if err != nil { if models.IsNotFoundError(err) { return userNotFound(w) diff --git a/internal/api/scim_test.go b/internal/api/scim_test.go index a66f5495b8..bd972aedf6 100644 --- a/internal/api/scim_test.go +++ b/internal/api/scim_test.go @@ -139,7 +139,7 @@ type scimTenant struct { token string } -func seedSCIMTenant(t *testing.T, conn *storage.Connection, token, email string) *scimTenant { +func seedSCIMTenant(t *testing.T, conn *storage.Connection, token, email string, extraClaims ...map[string]interface{}) *scimTenant { t.Helper() id := uuid.Must(uuid.NewV4()).String() @@ -160,10 +160,17 @@ func seedSCIMTenant(t *testing.T, conn *storage.Connection, token, email string) user.IsSSOUser = true require.NoError(t, conn.Create(user)) - identity, err := models.NewIdentity(user, "sso:"+provider.ID.String(), map[string]interface{}{ + claims := map[string]interface{}{ "sub": user.ID.String(), "email": email, - }) + } + for _, extra := range extraClaims { + for key, value := range extra { + claims[key] = value + } + } + + identity, err := models.NewIdentity(user, provider.ProviderType(), claims) require.NoError(t, err) require.NoError(t, conn.Create(identity)) @@ -225,6 +232,43 @@ func TestSCIMUsers(t *testing.T) { require.Equal(t, http.StatusOK, get(b.user.ID.String(), b.token).Code) }) + t.Run("maps the attributes the provider supplied", func(t *testing.T) { + c := seedSCIMTenant(t, conn, "scim_token_c", "stale@example.com", map[string]interface{}{ + "email": "bjensen@example.com", + "preferred_username": "bjensen", + "name": "Ms. Barbara Jane Jensen, III", + "family_name": "Jensen", + "given_name": "Barbara", + }) + + w := get(c.user.ID.String(), c.token) + + require.Equal(t, http.StatusOK, w.Code) + require.JSONEq(t, fmt.Sprintf(`{ + "schemas": [%q], + "id": %q, + "userName": "bjensen", + "name": { + "formatted": "Ms. Barbara Jane Jensen, III", + "familyName": "Jensen", + "givenName": "Barbara" + }, + "emails": [{"value": "bjensen@example.com", "primary": true}], + "meta": { + "resourceType": "User", + "created": %q, + "lastModified": %q, + "location": "%s/scim/v2/Users/%s" + } + }`, + scimCore.SchemaUser, + c.user.ID, + c.user.CreatedAt.UTC().Format(time.RFC3339Nano), + c.user.UpdatedAt.UTC().Format(time.RFC3339Nano), + config.API.ExternalURL, c.user.ID, + ), w.Body.String()) + }) + t.Run("hides a user belonging to another provider", func(t *testing.T) { w := get(b.user.ID.String(), a.token) diff --git a/internal/models/identity.go b/internal/models/identity.go index 1f5ee5f853..9c7af20fbc 100644 --- a/internal/models/identity.go +++ b/internal/models/identity.go @@ -93,6 +93,18 @@ func FindIdentityByIdAndProvider(tx *storage.Connection, providerId, provider st return identity, nil } +// FindIdentityByUserIDAndProvider searches for the identity linking a user to a provider. +func FindIdentityByUserIDAndProvider(tx *storage.Connection, userID uuid.UUID, provider string) (*Identity, error) { + identity := &Identity{} + if err := tx.Q().Where("user_id = ? AND provider = ?", userID, provider).First(identity); err != nil { + if errors.Cause(err) == sql.ErrNoRows { + return nil, IdentityNotFoundError{} + } + return nil, errors.Wrap(err, "error finding identity") + } + return identity, nil +} + // FindIdentitiesByUserID returns all identities associated to a user ID. func FindIdentitiesByUserID(tx *storage.Connection, userID uuid.UUID) ([]*Identity, error) { identities := []*Identity{} diff --git a/internal/models/provisioned_user.go b/internal/models/provisioned_user.go new file mode 100644 index 0000000000..e27ab5356b --- /dev/null +++ b/internal/models/provisioned_user.go @@ -0,0 +1,20 @@ +package models + +// ProvisionedUser is a user as seen through one SSO provider. The identity +// carries the claims that provider supplied, so attributes read from it are +// scoped to that provider rather than to the user record, which is shared +// across every provider the user is linked to. +type ProvisionedUser struct { + *User + Identity *Identity +} + +// Claim returns the string claim the provider supplied under key, or an empty +// string when it is absent or not a string. +func (p ProvisionedUser) Claim(key string) string { + if p.Identity == nil { + return "" + } + value, _ := p.Identity.IdentityData[key].(string) + return value +} diff --git a/internal/models/sso.go b/internal/models/sso.go index ca90de966a..0b09bccd8d 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -48,6 +48,35 @@ func (p *SSOProvider) UpdateSCIMToken(token string) { p.SCIMTokenHash = &hash } +func (p *SSOProvider) ProviderType() string { + return "sso:" + p.ID.String() +} + +func (p *SSOProvider) FindUserByID(tx *storage.Connection, id uuid.UUID) (*User, error) { + return FindUserByIDAndSSOProviderID(tx, id, p.ID) +} + +// FindIdentityByUserID returns the identity linking the user to this provider. +func (p *SSOProvider) FindIdentityByUserID(tx *storage.Connection, userID uuid.UUID) (*Identity, error) { + return FindIdentityByUserIDAndProvider(tx, userID, p.ProviderType()) +} + +// FindProvisionedUserByID returns the user together with the identity linking +// them to this provider. +func (p *SSOProvider) FindProvisionedUserByID(tx *storage.Connection, id uuid.UUID) (ProvisionedUser, error) { + user, err := p.FindUserByID(tx, id) + if err != nil { + return ProvisionedUser{}, err + } + + identity, err := p.FindIdentityByUserID(tx, user.ID) + if err != nil { + return ProvisionedUser{}, err + } + + return ProvisionedUser{User: user, Identity: identity}, nil +} + func toSHA256(token string) string { sum := sha256.Sum256([]byte(token)) return hex.EncodeToString(sum[:]) From cad340ada0f9b7a9580b366e4214b81a94bcc50e Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 11:25:53 -0600 Subject: [PATCH 08/18] refactor: delegate to srv.NotFound() --- internal/api/scim/users.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/internal/api/scim/users.go b/internal/api/scim/users.go index 5afc3dbf6f..e8b09ca822 100644 --- a/internal/api/scim/users.go +++ b/internal/api/scim/users.go @@ -15,21 +15,17 @@ func (srv *Server) UserByID(w http.ResponseWriter, r *http.Request) error { id, err := uuid.FromString(chi.URLParam(r, "id")) if err != nil { - return userNotFound(w) + return srv.NotFound(w, r) } provider := shared.GetSSOProvider(ctx) user, err := provider.FindProvisionedUserByID(srv.db.WithContext(ctx), id) if err != nil { if models.IsNotFoundError(err) { - return userNotFound(w) + return srv.NotFound(w, r) } return srv.internalError(w, r, err) } return protocol.Send(w, http.StatusOK, srv.users.MapFrom(user)) } - -func userNotFound(w http.ResponseWriter) error { - return protocol.SendError(w, http.StatusNotFound, "", "Resource not found") -} From cffafd719b6a92f01dae13d5c5f6761dc31dfc69 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 11:50:20 -0600 Subject: [PATCH 09/18] refactor: extract methods for loading a provisioned user --- internal/api/scim/core/user.go | 6 +--- internal/api/scim/core/user_test.go | 4 +-- internal/api/scim/mapper.go | 50 +++++++---------------------- internal/api/scim/mapper_test.go | 25 ++++++--------- internal/api/scim/server.go | 2 +- internal/models/provisioned_user.go | 22 +++++++++---- internal/models/sso.go | 11 +++---- 7 files changed, 43 insertions(+), 77 deletions(-) diff --git a/internal/api/scim/core/user.go b/internal/api/scim/core/user.go index 0ad290d7e7..ca58472cd1 100644 --- a/internal/api/scim/core/user.go +++ b/internal/api/scim/core/user.go @@ -13,16 +13,12 @@ type Name struct { MiddleName string `json:"middleName,omitempty"` } -func (n Name) IsZero() bool { - return n == Name{} -} - // User is the core User resource defined in RFC 7643, Section 4.1. type User struct { Schemas []SchemaURI `json:"schemas"` ID string `json:"id"` UserName string `json:"userName"` - Name *Name `json:"name,omitempty"` + Name Name `json:"name,omitzero"` Emails []Email `json:"emails,omitempty"` Meta Meta `json:"meta"` } diff --git a/internal/api/scim/core/user_test.go b/internal/api/scim/core/user_test.go index d1003ad1be..551c84de59 100644 --- a/internal/api/scim/core/user_test.go +++ b/internal/api/scim/core/user_test.go @@ -53,7 +53,7 @@ func TestUser(t *testing.T) { }) t.Run("omits the name when there is none", func(t *testing.T) { - user.Name = nil + user.Name = Name{} body, err := json.Marshal(user) @@ -62,7 +62,7 @@ func TestUser(t *testing.T) { }) t.Run("serializes only the name components that are set", func(t *testing.T) { - user.Name = &Name{FamilyName: "Jensen", GivenName: "Barbara"} + user.Name = Name{FamilyName: "Jensen", GivenName: "Barbara"} body, err := json.Marshal(user) diff --git a/internal/api/scim/mapper.go b/internal/api/scim/mapper.go index d91c332ece..c67db7c944 100644 --- a/internal/api/scim/mapper.go +++ b/internal/api/scim/mapper.go @@ -17,51 +17,23 @@ func NewUserMapper(baseURL string) UserMapper { return UserMapper{baseURL: baseURL} } -func (m UserMapper) MapFrom(in models.ProvisionedUser) *core.User { +func (m UserMapper) MapFrom(in *models.ProvisionedUser) *core.User { id := in.ID.String() meta := core.NewMeta(m.baseURL, core.ResourceTypeUser, core.EndpointUsers, id) meta.Created, meta.LastModified = in.CreatedAt.UTC(), in.UpdatedAt.UTC() - user := &core.User{ + return &core.User{ Schemas: []core.SchemaURI{core.SchemaUser}, ID: id, - UserName: userName(in), - Name: name(in), - Meta: meta, + UserName: in.UserName(), + Name: core.Name{ + Formatted: in.Claim("name"), + FamilyName: in.Claim("family_name"), + GivenName: in.Claim("given_name"), + MiddleName: in.Claim("middle_name"), + }, + Emails: []core.Email{{Value: in.Email(), Primary: true}}, + Meta: meta, } - - if address := email(in); address != "" { - user.Emails = []core.Email{{Value: address, Primary: true}} - } - - return user -} - -func email(in models.ProvisionedUser) string { - if email := in.Claim("email"); email != "" { - return email - } - return in.GetEmail() -} - -func userName(in models.ProvisionedUser) string { - if userName := in.Claim("preferred_username"); userName != "" { - return userName - } - return email(in) -} - -func name(in models.ProvisionedUser) *core.Name { - name := core.Name{ - Formatted: in.Claim("name"), - FamilyName: in.Claim("family_name"), - GivenName: in.Claim("given_name"), - MiddleName: in.Claim("middle_name"), - } - - if name.IsZero() { - return nil - } - return &name } diff --git a/internal/api/scim/mapper_test.go b/internal/api/scim/mapper_test.go index eb5d035d62..f0babc0716 100644 --- a/internal/api/scim/mapper_test.go +++ b/internal/api/scim/mapper_test.go @@ -16,8 +16,8 @@ func TestUserMapper(t *testing.T) { createdAt := time.Date(2026, 7, 21, 19, 41, 41, 0, time.UTC) updatedAt := time.Date(2026, 7, 22, 8, 12, 3, 0, time.UTC) - newModel := func(email string, claims map[string]interface{}) models.ProvisionedUser { - in := models.ProvisionedUser{ + newModel := func(email string, claims map[string]interface{}) *models.ProvisionedUser { + in := &models.ProvisionedUser{ User: &models.User{ ID: id, Email: storage.NullString(email), @@ -43,13 +43,6 @@ func TestUserMapper(t *testing.T) { require.Equal(t, core.ResourceTypeUser, user.Meta.ResourceType) }) - t.Run("omits emails when the user has no email", func(t *testing.T) { - user := mapper.MapFrom(newModel("", nil)) - - require.Empty(t, user.UserName) - require.Nil(t, user.Emails) - }) - t.Run("builds the location from the base URL", func(t *testing.T) { user := NewUserMapper("https://auth.example.com/scim/v2").MapFrom(newModel("bjensen@example.com", nil)) @@ -86,7 +79,7 @@ func TestUserMapper(t *testing.T) { }) t.Run("prefers preferred_username for the userName", func(t *testing.T) { - user := mapper.MapFrom(newModel("", map[string]interface{}{ + user := mapper.MapFrom(newModel("bjensen@example.com", map[string]interface{}{ "preferred_username": "bjensen", "email": "bjensen@example.com", })) @@ -96,14 +89,14 @@ func TestUserMapper(t *testing.T) { }) t.Run("maps the name components the provider supplied", func(t *testing.T) { - user := mapper.MapFrom(newModel("", map[string]interface{}{ + user := mapper.MapFrom(newModel("bjensen@example.com", map[string]interface{}{ "name": "Ms. Barbara Jane Jensen, III", "family_name": "Jensen", "given_name": "Barbara", "middle_name": "Jane", })) - require.Equal(t, &core.Name{ + require.Equal(t, core.Name{ Formatted: "Ms. Barbara Jane Jensen, III", FamilyName: "Jensen", GivenName: "Barbara", @@ -112,8 +105,8 @@ func TestUserMapper(t *testing.T) { }) t.Run("omits the name when the provider supplied no components", func(t *testing.T) { - require.Nil(t, mapper.MapFrom(newModel("bjensen@example.com", nil)).Name) - require.Nil(t, mapper.MapFrom(newModel("bjensen@example.com", map[string]interface{}{ + require.Equal(t, core.Name{}, mapper.MapFrom(newModel("bjensen@example.com", nil)).Name) + require.Equal(t, core.Name{}, mapper.MapFrom(newModel("bjensen@example.com", map[string]interface{}{ "sub": id.String(), })).Name) }) @@ -126,10 +119,10 @@ func TestUserMapper(t *testing.T) { })) require.Equal(t, "bjensen@example.com", user.UserName) - require.Equal(t, &core.Name{FamilyName: "Jensen"}, user.Name) + require.Equal(t, core.Name{FamilyName: "Jensen"}, user.Name) }) t.Run("satisfies the Mapper interface", func(t *testing.T) { - var _ Mapper[models.ProvisionedUser, *core.User] = NewUserMapper("") + var _ Mapper[*models.ProvisionedUser, *core.User] = NewUserMapper("") }) } diff --git a/internal/api/scim/server.go b/internal/api/scim/server.go index 336a6988c8..38abbe19ed 100644 --- a/internal/api/scim/server.go +++ b/internal/api/scim/server.go @@ -19,7 +19,7 @@ type TokenExtractor func(r *http.Request) (string, error) type Server struct { db *storage.Connection extract TokenExtractor - users Mapper[models.ProvisionedUser, *core.User] + users Mapper[*models.ProvisionedUser, *core.User] serviceProviderConfig *core.ServiceProviderConfig } diff --git a/internal/models/provisioned_user.go b/internal/models/provisioned_user.go index e27ab5356b..615ae45499 100644 --- a/internal/models/provisioned_user.go +++ b/internal/models/provisioned_user.go @@ -1,17 +1,25 @@ package models -// ProvisionedUser is a user as seen through one SSO provider. The identity -// carries the claims that provider supplied, so attributes read from it are -// scoped to that provider rather than to the user record, which is shared -// across every provider the user is linked to. type ProvisionedUser struct { *User Identity *Identity } -// Claim returns the string claim the provider supplied under key, or an empty -// string when it is absent or not a string. -func (p ProvisionedUser) Claim(key string) string { +func (u *ProvisionedUser) Email() string { + if email := u.Claim("email"); email != "" { + return email + } + return u.GetEmail() +} + +func (u *ProvisionedUser) UserName() string { + if userName := u.Claim("preferred_username"); userName != "" { + return userName + } + return u.Email() +} + +func (p *ProvisionedUser) Claim(key string) string { if p.Identity == nil { return "" } diff --git a/internal/models/sso.go b/internal/models/sso.go index 0b09bccd8d..a4d55db0e6 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -56,25 +56,22 @@ func (p *SSOProvider) FindUserByID(tx *storage.Connection, id uuid.UUID) (*User, return FindUserByIDAndSSOProviderID(tx, id, p.ID) } -// FindIdentityByUserID returns the identity linking the user to this provider. func (p *SSOProvider) FindIdentityByUserID(tx *storage.Connection, userID uuid.UUID) (*Identity, error) { return FindIdentityByUserIDAndProvider(tx, userID, p.ProviderType()) } -// FindProvisionedUserByID returns the user together with the identity linking -// them to this provider. -func (p *SSOProvider) FindProvisionedUserByID(tx *storage.Connection, id uuid.UUID) (ProvisionedUser, error) { +func (p *SSOProvider) FindProvisionedUserByID(tx *storage.Connection, id uuid.UUID) (*ProvisionedUser, error) { user, err := p.FindUserByID(tx, id) if err != nil { - return ProvisionedUser{}, err + return &ProvisionedUser{}, err } identity, err := p.FindIdentityByUserID(tx, user.ID) if err != nil { - return ProvisionedUser{}, err + return &ProvisionedUser{}, err } - return ProvisionedUser{User: user, Identity: identity}, nil + return &ProvisionedUser{User: user, Identity: identity}, nil } func toSHA256(token string) string { From a8508779b80522d2eb347b64ca151e6144d00382 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 11:52:56 -0600 Subject: [PATCH 10/18] refactor: rename Email() to PrimaryEmail() --- internal/api/scim/mapper.go | 2 +- internal/models/provisioned_user.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/api/scim/mapper.go b/internal/api/scim/mapper.go index c67db7c944..861cb78bee 100644 --- a/internal/api/scim/mapper.go +++ b/internal/api/scim/mapper.go @@ -33,7 +33,7 @@ func (m UserMapper) MapFrom(in *models.ProvisionedUser) *core.User { GivenName: in.Claim("given_name"), MiddleName: in.Claim("middle_name"), }, - Emails: []core.Email{{Value: in.Email(), Primary: true}}, + Emails: []core.Email{{Value: in.PrimaryEmail(), Primary: true}}, Meta: meta, } } diff --git a/internal/models/provisioned_user.go b/internal/models/provisioned_user.go index 615ae45499..6571cf1ddb 100644 --- a/internal/models/provisioned_user.go +++ b/internal/models/provisioned_user.go @@ -5,7 +5,7 @@ type ProvisionedUser struct { Identity *Identity } -func (u *ProvisionedUser) Email() string { +func (u *ProvisionedUser) PrimaryEmail() string { if email := u.Claim("email"); email != "" { return email } @@ -16,7 +16,7 @@ func (u *ProvisionedUser) UserName() string { if userName := u.Claim("preferred_username"); userName != "" { return userName } - return u.Email() + return u.PrimaryEmail() } func (p *ProvisionedUser) Claim(key string) string { From 03f420975838280540b79efa1f0519e356da4f97 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 12:04:07 -0600 Subject: [PATCH 11/18] refactor: extract method Meta#As to attach timestamps --- internal/api/scim/core/meta.go | 9 +++++++-- internal/api/scim/core/service_provider_config.go | 2 +- internal/api/scim/mapper.go | 5 +---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/api/scim/core/meta.go b/internal/api/scim/core/meta.go index 196325cc21..93b2a427ce 100644 --- a/internal/api/scim/core/meta.go +++ b/internal/api/scim/core/meta.go @@ -10,14 +10,19 @@ type Meta struct { Location string `json:"location,omitempty"` } -func NewMeta(baseURL string, resourceType ResourceTypeName, endpoint, id string) Meta { +func NewMeta(baseURL string, resourceType ResourceTypeName, endpoint, id string) *Meta { location := baseURL + endpoint if id != "" { location += "/" + id } - return Meta{ + return &Meta{ ResourceType: resourceType, Location: location, } } + +func (m *Meta) At(created, updated time.Time) *Meta { + m.Created, m.LastModified = created.UTC(), updated.UTC() + return m +} diff --git a/internal/api/scim/core/service_provider_config.go b/internal/api/scim/core/service_provider_config.go index 3fcfb07420..90a11a3cba 100644 --- a/internal/api/scim/core/service_provider_config.go +++ b/internal/api/scim/core/service_provider_config.go @@ -65,6 +65,6 @@ func NewServiceProviderConfig(baseURL string, schemes ...*AuthenticationScheme) return &ServiceProviderConfig{ Schemas: []SchemaURI{SchemaServiceProviderConfig}, AuthenticationSchemes: schemes, - Meta: NewMeta(baseURL, ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig, ""), + Meta: *NewMeta(baseURL, ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig, ""), } } diff --git a/internal/api/scim/mapper.go b/internal/api/scim/mapper.go index 861cb78bee..7edf2d732f 100644 --- a/internal/api/scim/mapper.go +++ b/internal/api/scim/mapper.go @@ -20,9 +20,6 @@ func NewUserMapper(baseURL string) UserMapper { func (m UserMapper) MapFrom(in *models.ProvisionedUser) *core.User { id := in.ID.String() - meta := core.NewMeta(m.baseURL, core.ResourceTypeUser, core.EndpointUsers, id) - meta.Created, meta.LastModified = in.CreatedAt.UTC(), in.UpdatedAt.UTC() - return &core.User{ Schemas: []core.SchemaURI{core.SchemaUser}, ID: id, @@ -34,6 +31,6 @@ func (m UserMapper) MapFrom(in *models.ProvisionedUser) *core.User { MiddleName: in.Claim("middle_name"), }, Emails: []core.Email{{Value: in.PrimaryEmail(), Primary: true}}, - Meta: meta, + Meta: *core.NewMeta(m.baseURL, core.ResourceTypeUser, core.EndpointUsers, id).At(in.CreatedAt, in.UpdatedAt), } } From 3d861eb0ce06155523707f0f75d5fa7b98288ad0 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 12:25:42 -0600 Subject: [PATCH 12/18] refactor: rever to value type instead of pointer --- internal/api/scim/core/meta.go | 6 +++--- internal/api/scim/core/service_provider_config.go | 2 +- internal/api/scim/mapper.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/api/scim/core/meta.go b/internal/api/scim/core/meta.go index 93b2a427ce..5ee199781e 100644 --- a/internal/api/scim/core/meta.go +++ b/internal/api/scim/core/meta.go @@ -10,19 +10,19 @@ type Meta struct { Location string `json:"location,omitempty"` } -func NewMeta(baseURL string, resourceType ResourceTypeName, endpoint, id string) *Meta { +func NewMeta(baseURL string, resourceType ResourceTypeName, endpoint, id string) Meta { location := baseURL + endpoint if id != "" { location += "/" + id } - return &Meta{ + return Meta{ ResourceType: resourceType, Location: location, } } -func (m *Meta) At(created, updated time.Time) *Meta { +func (m Meta) At(created, updated time.Time) Meta { m.Created, m.LastModified = created.UTC(), updated.UTC() return m } diff --git a/internal/api/scim/core/service_provider_config.go b/internal/api/scim/core/service_provider_config.go index 90a11a3cba..3fcfb07420 100644 --- a/internal/api/scim/core/service_provider_config.go +++ b/internal/api/scim/core/service_provider_config.go @@ -65,6 +65,6 @@ func NewServiceProviderConfig(baseURL string, schemes ...*AuthenticationScheme) return &ServiceProviderConfig{ Schemas: []SchemaURI{SchemaServiceProviderConfig}, AuthenticationSchemes: schemes, - Meta: *NewMeta(baseURL, ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig, ""), + Meta: NewMeta(baseURL, ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig, ""), } } diff --git a/internal/api/scim/mapper.go b/internal/api/scim/mapper.go index 7edf2d732f..645924f42b 100644 --- a/internal/api/scim/mapper.go +++ b/internal/api/scim/mapper.go @@ -31,6 +31,6 @@ func (m UserMapper) MapFrom(in *models.ProvisionedUser) *core.User { MiddleName: in.Claim("middle_name"), }, Emails: []core.Email{{Value: in.PrimaryEmail(), Primary: true}}, - Meta: *core.NewMeta(m.baseURL, core.ResourceTypeUser, core.EndpointUsers, id).At(in.CreatedAt, in.UpdatedAt), + Meta: core.NewMeta(m.baseURL, core.ResourceTypeUser, core.EndpointUsers, id).At(in.CreatedAt, in.UpdatedAt), } } From 043fe88d898127bbf889c1702775912505cf0567 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 14:27:11 -0600 Subject: [PATCH 13/18] refactor: extract method for sso provider key --- internal/api/samlacs.go | 2 +- internal/models/sso.go | 7 ++++++- internal/models/user.go | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/internal/api/samlacs.go b/internal/api/samlacs.go index e0a35df85d..5c44f8eeb2 100644 --- a/internal/api/samlacs.go +++ b/internal/api/samlacs.go @@ -311,7 +311,7 @@ func (a *API) handleSamlAcs(w http.ResponseWriter, r *http.Request) error { } } - providerType := "sso:" + ssoProvider.ID.String() + providerType := ssoProvider.ProviderType() if err := a.triggerBeforeUserCreatedExternal( r, db, &userProvidedData, providerType); err != nil { return err diff --git a/internal/models/sso.go b/internal/models/sso.go index a4d55db0e6..adacb719a9 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -49,7 +49,12 @@ func (p *SSOProvider) UpdateSCIMToken(token string) { } func (p *SSOProvider) ProviderType() string { - return "sso:" + p.ID.String() + return SSOProviderType(p.ID) +} + +// SSOProviderType is the identities.provider value for users of an SSO provider. +func SSOProviderType(id uuid.UUID) string { + return "sso:" + id.String() } func (p *SSOProvider) FindUserByID(tx *storage.Connection, id uuid.UUID) (*User, error) { diff --git a/internal/models/user.go b/internal/models/user.go index 6ce01a4e24..acf25c4e01 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -715,7 +715,7 @@ func FindUserByIDAndSSOProviderID(tx *storage.Connection, id, ssoProviderID uuid // Skip findUser's eager loading query := tx.Q().Where( "instance_id = ? and id = ? and deleted_at is null and is_sso_user = true and id in (select user_id from identities where provider = ?)", - uuid.Nil, id, "sso:"+ssoProviderID.String(), + uuid.Nil, id, SSOProviderType(ssoProviderID), ) if err := query.First(obj); err != nil { From fda4de5570d7029ea80e7bd7d0a0b0696112bf4b Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 15:27:21 -0600 Subject: [PATCH 14/18] refactor(scim): extract Resource interface --- .../api/scim/core/authentication_scheme.go | 29 +++++++++ internal/api/scim/core/core.go | 3 - internal/api/scim/core/endpoints.go | 7 -- internal/api/scim/core/feature.go | 16 +++++ internal/api/scim/core/meta.go | 19 ++---- internal/api/scim/core/meta_test.go | 65 ++++++++++++++++--- internal/api/scim/core/resource.go | 9 +++ internal/api/scim/core/resource_type.go | 31 +++++++++ internal/api/scim/core/resource_type_test.go | 24 +++++++ internal/api/scim/core/schemas.go | 5 -- .../api/scim/core/service_provider_config.go | 47 +------------- .../scim/core/service_provider_config_test.go | 4 +- internal/api/scim/core/user_test.go | 2 +- internal/api/scim/mapper.go | 6 +- internal/api/scim/mapper_test.go | 6 +- internal/models/provisioned_user.go | 24 ++++++- internal/models/sso.go | 4 +- 17 files changed, 203 insertions(+), 98 deletions(-) create mode 100644 internal/api/scim/core/authentication_scheme.go delete mode 100644 internal/api/scim/core/endpoints.go create mode 100644 internal/api/scim/core/feature.go create mode 100644 internal/api/scim/core/resource.go create mode 100644 internal/api/scim/core/resource_type.go create mode 100644 internal/api/scim/core/resource_type_test.go diff --git a/internal/api/scim/core/authentication_scheme.go b/internal/api/scim/core/authentication_scheme.go new file mode 100644 index 0000000000..d1af828eb5 --- /dev/null +++ b/internal/api/scim/core/authentication_scheme.go @@ -0,0 +1,29 @@ +package core + +type AuthenticationSchemeType string + +const ( + AuthenticationSchemeOAuthBearerToken AuthenticationSchemeType = "oauthbearertoken" +) + +type AuthenticationScheme struct { + Type AuthenticationSchemeType `json:"type"` + Name string `json:"name"` + Description string `json:"description"` + SpecURI string `json:"specUri,omitempty"` + Primary bool `json:"primary"` +} + +func NewOAuthBearerToken() *AuthenticationScheme { + return &AuthenticationScheme{ + Type: AuthenticationSchemeOAuthBearerToken, + Name: "OAuth Bearer Token", + Description: "Authentication scheme using the OAuth Bearer Token Standard", + SpecURI: "http://www.rfc-editor.org/info/rfc6750", + } +} + +func (scheme *AuthenticationScheme) AsPrimary() *AuthenticationScheme { + scheme.Primary = true + return scheme +} diff --git a/internal/api/scim/core/core.go b/internal/api/scim/core/core.go index d625dab4e1..3b8f92437b 100644 --- a/internal/api/scim/core/core.go +++ b/internal/api/scim/core/core.go @@ -3,6 +3,3 @@ package core // SchemaURI identifies a SCIM schema type SchemaURI string - -// ResourceTypeName names a resource type -type ResourceTypeName string diff --git a/internal/api/scim/core/endpoints.go b/internal/api/scim/core/endpoints.go deleted file mode 100644 index ecd463660c..0000000000 --- a/internal/api/scim/core/endpoints.go +++ /dev/null @@ -1,7 +0,0 @@ -package core - -// The resource endpoints of RFC 7644, Section 3.2, relative to the base URL -const ( - EndpointServiceProviderConfig = "/ServiceProviderConfig" - EndpointUsers = "/Users" -) diff --git a/internal/api/scim/core/feature.go b/internal/api/scim/core/feature.go new file mode 100644 index 0000000000..42c14bf960 --- /dev/null +++ b/internal/api/scim/core/feature.go @@ -0,0 +1,16 @@ +package core + +type SupportedFeature struct { + Supported bool `json:"supported"` +} + +type BulkFeature struct { + Supported bool `json:"supported"` + MaxOperations int `json:"maxOperations"` + MaxPayloadSize int `json:"maxPayloadSize"` +} + +type FilterFeature struct { + Supported bool `json:"supported"` + MaxResults int `json:"maxResults"` +} diff --git a/internal/api/scim/core/meta.go b/internal/api/scim/core/meta.go index 5ee199781e..c035221890 100644 --- a/internal/api/scim/core/meta.go +++ b/internal/api/scim/core/meta.go @@ -2,7 +2,6 @@ package core import "time" -// Meta is the resource metadata common attribute defined in RFC 7643, Section 3.1. type Meta struct { ResourceType ResourceTypeName `json:"resourceType"` Created time.Time `json:"created,omitzero"` @@ -10,19 +9,15 @@ type Meta struct { Location string `json:"location,omitempty"` } -func NewMeta(baseURL string, resourceType ResourceTypeName, endpoint, id string) Meta { - location := baseURL + endpoint - if id != "" { - location += "/" + id - } - - return Meta{ - ResourceType: resourceType, - Location: location, - } +func NewMeta(baseURL string, resourceType ResourceType) Meta { + return resourceType.Meta(baseURL) } -func (m Meta) At(created, updated time.Time) Meta { +func (m Meta) For(r Resource) Meta { + created, updated := r.Timestamps() + + m.Location += "/" + r.ResourceID() m.Created, m.LastModified = created.UTC(), updated.UTC() + return m } diff --git a/internal/api/scim/core/meta_test.go b/internal/api/scim/core/meta_test.go index ac8d47817e..ad227564f8 100644 --- a/internal/api/scim/core/meta_test.go +++ b/internal/api/scim/core/meta_test.go @@ -3,32 +3,81 @@ package core import ( "encoding/json" "testing" + "time" + "github.com/gofrs/uuid" "github.com/stretchr/testify/require" ) +type resource struct { + id string + created, updated time.Time +} + +func (s resource) ResourceID() string { return s.id } +func (s resource) ResourceType() ResourceType { + return ResourceType{Name: "Resource", Endpoint: "/Resources"} +} +func (s resource) Timestamps() (created, updated time.Time) { return s.created, s.updated } + func TestNewMeta(t *testing.T) { baseURL := "http://localhost:9999/scim/v2" + id := uuid.Must(uuid.NewV4()).String() + created := time.Date(2026, 7, 21, 19, 41, 41, 0, time.UTC) + updated := time.Date(2026, 7, 22, 8, 12, 3, 0, time.UTC) t.Run("locates a resource that is its own endpoint", func(t *testing.T) { - meta := NewMeta(baseURL, ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig, "") + meta := NewMeta(baseURL, ResourceTypeServiceProviderConfig) - require.Equal(t, ResourceTypeServiceProviderConfig, meta.ResourceType) + require.Equal(t, ResourceTypeServiceProviderConfig.Name, meta.ResourceType) require.Equal(t, baseURL+"/ServiceProviderConfig", meta.Location) + require.Zero(t, meta.Created) + require.Zero(t, meta.LastModified) }) t.Run("locates one resource of a collection", func(t *testing.T) { - meta := NewMeta(baseURL, ResourceTypeUser, EndpointUsers, "2819c223-7f76-453a-919d-413861904646") + meta := NewMeta(baseURL, ResourceTypeUser).For(resource{ + id: id, + created: created, + updated: updated, + }) + + require.Equal(t, ResourceTypeUser.Name, meta.ResourceType) + require.Equal(t, baseURL+"/Users/"+id, meta.Location) + require.Equal(t, created, meta.Created) + require.Equal(t, updated, meta.LastModified) + }) + + t.Run("normalizes the timestamps to UTC", func(t *testing.T) { + meta := NewMeta(baseURL, ResourceTypeUser).For(resource{ + id: id, + created: created.In(time.FixedZone("MDT", -6*60*60)), + updated: updated.In(time.FixedZone("MDT", -6*60*60)), + }) + + require.Equal(t, time.UTC, meta.Created.Location()) + require.Equal(t, time.UTC, meta.LastModified.Location()) + require.True(t, meta.Created.Equal(created)) + require.True(t, meta.LastModified.Equal(updated)) + }) + + t.Run("leaves the receiver untouched", func(t *testing.T) { + collection := NewMeta(baseURL, ResourceTypeUser) + + first := collection.For(resource{id: "first"}) + second := collection.For(resource{id: "second"}) - require.Equal(t, ResourceTypeUser, meta.ResourceType) - require.Equal(t, baseURL+"/Users/2819c223-7f76-453a-919d-413861904646", meta.Location) + require.Equal(t, baseURL+"/Users/first", first.Location) + require.Equal(t, baseURL+"/Users/second", second.Location) + require.Equal(t, baseURL+"/Users", collection.Location) + require.Zero(t, collection.Created) }) } func TestMeta(t *testing.T) { t.Run("serializes to JSON correctly", func(t *testing.T) { body, err := json.Marshal(Meta{ - ResourceType: ResourceTypeServiceProviderConfig, + ResourceType: "ServiceProviderConfig", Location: "http://localhost:9999/scim/v2/ServiceProviderConfig", }) @@ -40,9 +89,9 @@ func TestMeta(t *testing.T) { }) t.Run("omits the location when it is empty", func(t *testing.T) { - body, err := json.Marshal(Meta{ResourceType: ResourceTypeServiceProviderConfig}) + body, err := json.Marshal(Meta{ResourceType: "Example"}) require.NoError(t, err) - require.JSONEq(t, `{"resourceType": "ServiceProviderConfig"}`, string(body)) + require.JSONEq(t, `{"resourceType": "Example"}`, string(body)) }) } diff --git a/internal/api/scim/core/resource.go b/internal/api/scim/core/resource.go new file mode 100644 index 0000000000..8cfe8a727b --- /dev/null +++ b/internal/api/scim/core/resource.go @@ -0,0 +1,9 @@ +package core + +import "time" + +type Resource interface { + ResourceID() string + ResourceType() ResourceType + Timestamps() (created, updated time.Time) +} diff --git a/internal/api/scim/core/resource_type.go b/internal/api/scim/core/resource_type.go new file mode 100644 index 0000000000..234a9eee8a --- /dev/null +++ b/internal/api/scim/core/resource_type.go @@ -0,0 +1,31 @@ +package core + +var ( + ResourceTypeServiceProviderConfig = ResourceType{ + Name: "ServiceProviderConfig", + Endpoint: "/ServiceProviderConfig", + } + + ResourceTypeUser = ResourceType{ + Name: "User", + Endpoint: "/Users", + } +) + +type ResourceTypeName string + +type ResourceType struct { + Name ResourceTypeName + Endpoint string +} + +func (r ResourceType) Meta(baseURL string) Meta { + return Meta{ + ResourceType: r.Name, + Location: r.Location(baseURL), + } +} + +func (r ResourceType) Location(baseURL string) string { + return baseURL + r.Endpoint +} diff --git a/internal/api/scim/core/resource_type_test.go b/internal/api/scim/core/resource_type_test.go new file mode 100644 index 0000000000..1abc4b9ae3 --- /dev/null +++ b/internal/api/scim/core/resource_type_test.go @@ -0,0 +1,24 @@ +package core + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestResourceType(t *testing.T) { + r := ResourceType{Name: "Resource", Endpoint: "/Resources"} + + t.Run("Meta", func(t *testing.T) { + baseURL := "http://localhost:9999/scim/v2" + + t.Run("locates a resource that is its own endpoint", func(t *testing.T) { + meta := r.Meta(baseURL) + + require.Equal(t, ResourceTypeName("Resource"), meta.ResourceType) + require.Equal(t, baseURL+"/Resources", meta.Location) + require.Zero(t, meta.Created) + require.Zero(t, meta.LastModified) + }) + }) +} diff --git a/internal/api/scim/core/schemas.go b/internal/api/scim/core/schemas.go index b324034caa..026d49c6b5 100644 --- a/internal/api/scim/core/schemas.go +++ b/internal/api/scim/core/schemas.go @@ -8,8 +8,3 @@ const ( SchemaServiceProviderConfig SchemaURI = schemaCore + ":ServiceProviderConfig" SchemaUser SchemaURI = schemaCore + ":User" ) - -const ( - ResourceTypeServiceProviderConfig ResourceTypeName = "ServiceProviderConfig" - ResourceTypeUser ResourceTypeName = "User" -) diff --git a/internal/api/scim/core/service_provider_config.go b/internal/api/scim/core/service_provider_config.go index 3fcfb07420..c684c6c9a9 100644 --- a/internal/api/scim/core/service_provider_config.go +++ b/internal/api/scim/core/service_provider_config.go @@ -1,49 +1,5 @@ package core -type SupportedFeature struct { - Supported bool `json:"supported"` -} - -type BulkFeature struct { - Supported bool `json:"supported"` - MaxOperations int `json:"maxOperations"` - MaxPayloadSize int `json:"maxPayloadSize"` -} - -type FilterFeature struct { - Supported bool `json:"supported"` - MaxResults int `json:"maxResults"` -} - -type AuthenticationSchemeType string - -const ( - AuthenticationSchemeOAuthBearerToken AuthenticationSchemeType = "oauthbearertoken" -) - -// AuthenticationScheme is the authentication scheme of RFC 7643, Section 5. -type AuthenticationScheme struct { - Type AuthenticationSchemeType `json:"type"` - Name string `json:"name"` - Description string `json:"description"` - SpecURI string `json:"specUri,omitempty"` - Primary bool `json:"primary"` -} - -func NewOAuthBearerToken() *AuthenticationScheme { - return &AuthenticationScheme{ - Type: AuthenticationSchemeOAuthBearerToken, - Name: "OAuth Bearer Token", - Description: "Authentication scheme using the OAuth Bearer Token Standard", - SpecURI: "http://www.rfc-editor.org/info/rfc6750", - } -} - -func (scheme *AuthenticationScheme) AsPrimary() *AuthenticationScheme { - scheme.Primary = true - return scheme -} - // ServiceProviderConfig is the schema defined in RFC 7643, Section 5. type ServiceProviderConfig struct { Schemas []SchemaURI `json:"schemas"` @@ -61,10 +17,9 @@ func NewServiceProviderConfig(baseURL string, schemes ...*AuthenticationScheme) if schemes == nil { schemes = []*AuthenticationScheme{} } - return &ServiceProviderConfig{ Schemas: []SchemaURI{SchemaServiceProviderConfig}, AuthenticationSchemes: schemes, - Meta: NewMeta(baseURL, ResourceTypeServiceProviderConfig, EndpointServiceProviderConfig, ""), + Meta: ResourceTypeServiceProviderConfig.Meta(baseURL), } } diff --git a/internal/api/scim/core/service_provider_config_test.go b/internal/api/scim/core/service_provider_config_test.go index 03ff2dca95..552cce38bc 100644 --- a/internal/api/scim/core/service_provider_config_test.go +++ b/internal/api/scim/core/service_provider_config_test.go @@ -23,8 +23,8 @@ func TestNewServiceProviderConfig(t *testing.T) { config := NewServiceProviderConfig(baseURL) - require.Equal(t, ResourceTypeServiceProviderConfig, config.Meta.ResourceType) - require.Equal(t, baseURL+EndpointServiceProviderConfig, config.Meta.Location) + require.Equal(t, ResourceTypeServiceProviderConfig.Name, config.Meta.ResourceType) + require.Equal(t, baseURL+"/ServiceProviderConfig", config.Meta.Location) }) t.Run("supports none of the optional protocol features", func(t *testing.T) { diff --git a/internal/api/scim/core/user_test.go b/internal/api/scim/core/user_test.go index 551c84de59..080cde1405 100644 --- a/internal/api/scim/core/user_test.go +++ b/internal/api/scim/core/user_test.go @@ -18,7 +18,7 @@ func TestUser(t *testing.T) { UserName: "bjensen@example.com", Emails: []Email{{Value: "bjensen@example.com", Primary: true}}, Meta: Meta{ - ResourceType: ResourceTypeUser, + ResourceType: ResourceTypeUser.Name, Created: created, LastModified: lastModified, Location: "http://localhost:9999/scim/v2/Users/2819c223-7f76-453a-919d-413861904646", diff --git a/internal/api/scim/mapper.go b/internal/api/scim/mapper.go index 645924f42b..f1433e6ba2 100644 --- a/internal/api/scim/mapper.go +++ b/internal/api/scim/mapper.go @@ -18,11 +18,9 @@ func NewUserMapper(baseURL string) UserMapper { } func (m UserMapper) MapFrom(in *models.ProvisionedUser) *core.User { - id := in.ID.String() - return &core.User{ Schemas: []core.SchemaURI{core.SchemaUser}, - ID: id, + ID: in.ResourceID(), UserName: in.UserName(), Name: core.Name{ Formatted: in.Claim("name"), @@ -31,6 +29,6 @@ func (m UserMapper) MapFrom(in *models.ProvisionedUser) *core.User { MiddleName: in.Claim("middle_name"), }, Emails: []core.Email{{Value: in.PrimaryEmail(), Primary: true}}, - Meta: core.NewMeta(m.baseURL, core.ResourceTypeUser, core.EndpointUsers, id).At(in.CreatedAt, in.UpdatedAt), + Meta: in.ResourceType().Meta(m.baseURL).For(in), } } diff --git a/internal/api/scim/mapper_test.go b/internal/api/scim/mapper_test.go index f0babc0716..c037760c9f 100644 --- a/internal/api/scim/mapper_test.go +++ b/internal/api/scim/mapper_test.go @@ -40,7 +40,7 @@ func TestUserMapper(t *testing.T) { require.Equal(t, id.String(), user.ID) require.Equal(t, "bjensen@example.com", user.UserName) require.Equal(t, []core.Email{{Value: "bjensen@example.com", Primary: true}}, user.Emails) - require.Equal(t, core.ResourceTypeUser, user.Meta.ResourceType) + require.Equal(t, core.ResourceTypeUser.Name, user.Meta.ResourceType) }) t.Run("builds the location from the base URL", func(t *testing.T) { @@ -121,8 +121,4 @@ func TestUserMapper(t *testing.T) { require.Equal(t, "bjensen@example.com", user.UserName) require.Equal(t, core.Name{FamilyName: "Jensen"}, user.Name) }) - - t.Run("satisfies the Mapper interface", func(t *testing.T) { - var _ Mapper[*models.ProvisionedUser, *core.User] = NewUserMapper("") - }) } diff --git a/internal/models/provisioned_user.go b/internal/models/provisioned_user.go index 6571cf1ddb..6ade5a687a 100644 --- a/internal/models/provisioned_user.go +++ b/internal/models/provisioned_user.go @@ -1,5 +1,11 @@ package models +import ( + "time" + + "github.com/supabase/auth/internal/api/scim/core" +) + type ProvisionedUser struct { *User Identity *Identity @@ -19,10 +25,22 @@ func (u *ProvisionedUser) UserName() string { return u.PrimaryEmail() } -func (p *ProvisionedUser) Claim(key string) string { - if p.Identity == nil { +func (u *ProvisionedUser) Claim(key string) string { + if u.Identity == nil { return "" } - value, _ := p.Identity.IdentityData[key].(string) + value, _ := u.Identity.IdentityData[key].(string) return value } + +func (u *ProvisionedUser) ResourceID() string { + return u.ID.String() +} + +func (u *ProvisionedUser) ResourceType() core.ResourceType { + return core.ResourceTypeUser +} + +func (u *ProvisionedUser) Timestamps() (created, updated time.Time) { + return u.CreatedAt, u.UpdatedAt +} diff --git a/internal/models/sso.go b/internal/models/sso.go index adacb719a9..8bfe10c865 100644 --- a/internal/models/sso.go +++ b/internal/models/sso.go @@ -68,12 +68,12 @@ func (p *SSOProvider) FindIdentityByUserID(tx *storage.Connection, userID uuid.U func (p *SSOProvider) FindProvisionedUserByID(tx *storage.Connection, id uuid.UUID) (*ProvisionedUser, error) { user, err := p.FindUserByID(tx, id) if err != nil { - return &ProvisionedUser{}, err + return nil, err } identity, err := p.FindIdentityByUserID(tx, user.ID) if err != nil { - return &ProvisionedUser{}, err + return nil, err } return &ProvisionedUser{User: user, Identity: identity}, nil From 3322570545a43153fcbd68a713250f3eb429d76d Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 15:30:44 -0600 Subject: [PATCH 15/18] test: fix incorrect assertion --- internal/api/scim_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/scim_test.go b/internal/api/scim_test.go index bd972aedf6..10d0de3699 100644 --- a/internal/api/scim_test.go +++ b/internal/api/scim_test.go @@ -374,7 +374,7 @@ func TestSCIMInfrastructureFailure(t *testing.T) { require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) require.JSONEq(t, `{ "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], - "detail": "Unexpected failure, please check server logs for more information", + "detail": "Internal server error", "status": "500" }`, w.Body.String()) }) From cd67db129d78d9cfbd942f286fb9b917ee959ee3 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 15:55:50 -0600 Subject: [PATCH 16/18] test: extract scim_users_test.go --- internal/api/scim_test.go | 252 -------------------------------- internal/api/scim_users_test.go | 237 ++++++++++++++++++++++++++++++ 2 files changed, 237 insertions(+), 252 deletions(-) create mode 100644 internal/api/scim_users_test.go diff --git a/internal/api/scim_test.go b/internal/api/scim_test.go index 10d0de3699..a6a966823d 100644 --- a/internal/api/scim_test.go +++ b/internal/api/scim_test.go @@ -1,19 +1,15 @@ package api import ( - "fmt" "net/http" "net/http/httptest" "net/url" "testing" - "time" - "github.com/gofrs/uuid" "github.com/stretchr/testify/require" scimCore "github.com/supabase/auth/internal/api/scim/core" scimProtocol "github.com/supabase/auth/internal/api/scim/protocol" "github.com/supabase/auth/internal/conf" - "github.com/supabase/auth/internal/models" "github.com/supabase/auth/internal/storage" ) @@ -132,251 +128,3 @@ func TestSCIM(t *testing.T) { }) }) } - -type scimTenant struct { - provider *models.SSOProvider - user *models.User - token string -} - -func seedSCIMTenant(t *testing.T, conn *storage.Connection, token, email string, extraClaims ...map[string]interface{}) *scimTenant { - t.Helper() - - id := uuid.Must(uuid.NewV4()).String() - provider := &models.SSOProvider{ - SAMLProvider: models.SAMLProvider{ - EntityID: "https://example.com/saml/metadata/" + id, - MetadataXML: "", - }, - SSODomains: []models.SSODomain{ - {Domain: id + ".local"}, - }, - } - provider.UpdateSCIMToken(token) - require.NoError(t, conn.Eager().Create(provider)) - - user, err := models.NewUser("", email, "", "authenticated", nil) - require.NoError(t, err) - user.IsSSOUser = true - require.NoError(t, conn.Create(user)) - - claims := map[string]interface{}{ - "sub": user.ID.String(), - "email": email, - } - for _, extra := range extraClaims { - for key, value := range extra { - claims[key] = value - } - } - - identity, err := models.NewIdentity(user, provider.ProviderType(), claims) - require.NoError(t, err) - require.NoError(t, conn.Create(identity)) - - return &scimTenant{provider: provider, user: user, token: token} -} - -func TestSCIMUsers(t *testing.T) { - var a, b *scimTenant - var conn *storage.Connection - - api, config, err := setupAPIForTestWithCallback(func(cfg *conf.GlobalConfiguration, c *storage.Connection) { - if cfg != nil { - cfg.Experimental.ScimEnabled = true - return - } - conn = c - require.NoError(t, models.TruncateAll(c)) - a = seedSCIMTenant(t, c, "scim_token_a", "a@example.com") - b = seedSCIMTenant(t, c, "scim_token_b", "b@example.com") - }) - require.NoError(t, err) - - get := func(id, token string) *httptest.ResponseRecorder { - r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+id, nil) - if token != "" { - r.Header.Set("Authorization", "Bearer "+token) - } - w := httptest.NewRecorder() - api.handler.ServeHTTP(w, r) - return w - } - - t.Run("returns the user that belongs to the token's provider", func(t *testing.T) { - w := get(a.user.ID.String(), a.token) - - require.Equal(t, http.StatusOK, w.Code) - require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) - require.JSONEq(t, fmt.Sprintf(`{ - "schemas": [%q], - "id": %q, - "userName": "a@example.com", - "emails": [{"value": "a@example.com", "primary": true}], - "meta": { - "resourceType": "User", - "created": %q, - "lastModified": %q, - "location": "%s/scim/v2/Users/%s" - } - }`, - scimCore.SchemaUser, - a.user.ID, - a.user.CreatedAt.UTC().Format(time.RFC3339Nano), - a.user.UpdatedAt.UTC().Format(time.RFC3339Nano), - config.API.ExternalURL, a.user.ID, - ), w.Body.String()) - }) - - t.Run("scopes each provider to its own users", func(t *testing.T) { - require.Equal(t, http.StatusOK, get(b.user.ID.String(), b.token).Code) - }) - - t.Run("maps the attributes the provider supplied", func(t *testing.T) { - c := seedSCIMTenant(t, conn, "scim_token_c", "stale@example.com", map[string]interface{}{ - "email": "bjensen@example.com", - "preferred_username": "bjensen", - "name": "Ms. Barbara Jane Jensen, III", - "family_name": "Jensen", - "given_name": "Barbara", - }) - - w := get(c.user.ID.String(), c.token) - - require.Equal(t, http.StatusOK, w.Code) - require.JSONEq(t, fmt.Sprintf(`{ - "schemas": [%q], - "id": %q, - "userName": "bjensen", - "name": { - "formatted": "Ms. Barbara Jane Jensen, III", - "familyName": "Jensen", - "givenName": "Barbara" - }, - "emails": [{"value": "bjensen@example.com", "primary": true}], - "meta": { - "resourceType": "User", - "created": %q, - "lastModified": %q, - "location": "%s/scim/v2/Users/%s" - } - }`, - scimCore.SchemaUser, - c.user.ID, - c.user.CreatedAt.UTC().Format(time.RFC3339Nano), - c.user.UpdatedAt.UTC().Format(time.RFC3339Nano), - config.API.ExternalURL, c.user.ID, - ), w.Body.String()) - }) - - t.Run("hides a user belonging to another provider", func(t *testing.T) { - w := get(b.user.ID.String(), a.token) - - require.Equal(t, http.StatusNotFound, w.Code) - require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) - require.Contains(t, w.Body.String(), scimProtocol.SchemaError) - }) - - t.Run("returns the same 404 for an unknown id", func(t *testing.T) { - unknown := get(uuid.Must(uuid.NewV4()).String(), a.token) - other := get(b.user.ID.String(), a.token) - - require.Equal(t, http.StatusNotFound, unknown.Code) - require.Equal(t, other.Body.String(), unknown.Body.String()) - }) - - t.Run("returns 404 for a malformed id", func(t *testing.T) { - w := get("not-a-uuid", a.token) - - require.Equal(t, http.StatusNotFound, w.Code) - require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) - }) - - t.Run("requires a bearer token", func(t *testing.T) { - w := get(a.user.ID.String(), "") - - require.Equal(t, http.StatusUnauthorized, w.Code) - require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) - require.Equal(t, "Bearer", w.Header().Get("WWW-Authenticate")) - require.Contains(t, w.Body.String(), scimProtocol.SchemaError) - }) - - t.Run("rejects an unknown token", func(t *testing.T) { - w := get(a.user.ID.String(), "scim_nope") - - require.Equal(t, http.StatusUnauthorized, w.Code) - require.Equal(t, "Bearer", w.Header().Get("WWW-Authenticate")) - }) - - t.Run("rejects a disabled provider", func(t *testing.T) { - disabled := true - b.provider.Disabled = &disabled - require.NoError(t, conn.Update(b.provider)) - defer func() { - b.provider.Disabled = nil - require.NoError(t, conn.Update(b.provider)) - }() - - w := get(b.user.ID.String(), b.token) - - require.Equal(t, http.StatusForbidden, w.Code) - require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) - }) - - t.Run("stays hidden when the feature flag is off", func(t *testing.T) { - disabled, _, err := setupAPIForTest() - require.NoError(t, err) - - r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+a.user.ID.String(), nil) - r.Header.Set("Authorization", "Bearer "+a.token) - w := httptest.NewRecorder() - disabled.handler.ServeHTTP(w, r) - - require.Equal(t, http.StatusNotFound, w.Code) - require.Equal(t, "application/json", w.Header().Get("Content-Type")) - require.NotContains(t, w.Body.String(), scimProtocol.SchemaError) - }) -} - -func TestSCIMInfrastructureFailure(t *testing.T) { - var tenant *scimTenant - var conn *storage.Connection - - api, _, err := setupAPIForTestWithCallback(func(cfg *conf.GlobalConfiguration, c *storage.Connection) { - if cfg != nil { - cfg.Experimental.ScimEnabled = true - return - } - conn = c - require.NoError(t, models.TruncateAll(c)) - tenant = seedSCIMTenant(t, c, "scim_token_unreachable", "unreachable@example.com") - }) - require.NoError(t, err) - - rename := func(t *testing.T, from, to string) { - t.Helper() - require.NoError(t, conn.RawQuery("alter table "+from+" rename to "+to).Exec()) - } - - // Each table stands in for a database that fails one of the two queries a - // SCIM request makes, for a reason other than the row being absent. - for _, table := range []string{"sso_providers", "users"} { - t.Run("answers in the SCIM error form when "+table+" cannot be queried", func(t *testing.T) { - rename(t, table, table+"_renamed") - defer rename(t, table+"_renamed", table) - - r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+tenant.user.ID.String(), nil) - r.Header.Set("Authorization", "Bearer "+tenant.token) - w := httptest.NewRecorder() - api.handler.ServeHTTP(w, r) - - require.Equal(t, http.StatusInternalServerError, w.Code) - require.Equal(t, scimProtocol.MediaType, w.Header().Get("Content-Type")) - require.JSONEq(t, `{ - "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], - "detail": "Internal server error", - "status": "500" - }`, w.Body.String()) - }) - } -} diff --git a/internal/api/scim_users_test.go b/internal/api/scim_users_test.go new file mode 100644 index 0000000000..86b1496d9c --- /dev/null +++ b/internal/api/scim_users_test.go @@ -0,0 +1,237 @@ +package api + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gofrs/uuid" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + scimCore "github.com/supabase/auth/internal/api/scim/core" + scimProtocol "github.com/supabase/auth/internal/api/scim/protocol" + "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/models" + "github.com/supabase/auth/internal/storage" +) + +type scimTenant struct { + provider *models.SSOProvider + user *models.User + token string +} + +type SCIMUsersTestSuite struct { + suite.Suite + API *API + Config *conf.GlobalConfiguration + TenantA *scimTenant + TenantB *scimTenant +} + +func TestSCIMUsers(t *testing.T) { + api, config, err := setupAPIForTestWithCallback(func(cfg *conf.GlobalConfiguration, _ *storage.Connection) { + if cfg != nil { + cfg.Experimental.ScimEnabled = true + } + }) + require.NoError(t, err) + defer api.db.Close() + + suite.Run(t, &SCIMUsersTestSuite{API: api, Config: config}) +} + +func (ts *SCIMUsersTestSuite) SetupTest() { + require.NoError(ts.T(), models.TruncateAll(ts.API.db)) + + ts.TenantA = seedSCIMTenant(ts.T(), ts.API.db, "scim_token_a", "a@example.com") + ts.TenantB = seedSCIMTenant(ts.T(), ts.API.db, "scim_token_b", "b@example.com") +} + +func (ts *SCIMUsersTestSuite) get(id, token string) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+id, nil) + if token != "" { + r.Header.Set("Authorization", "Bearer "+token) + } + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, r) + return w +} + +func (ts *SCIMUsersTestSuite) TestGetUser() { + ts.Run("returns the user that belongs to the token's provider", func() { + w := ts.get(ts.TenantA.user.ID.String(), ts.TenantA.token) + + require.Equal(ts.T(), http.StatusOK, w.Code) + require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) + require.JSONEq(ts.T(), fmt.Sprintf(`{ + "schemas": [%q], + "id": %q, + "userName": "a@example.com", + "emails": [{"value": "a@example.com", "primary": true}], + "meta": { + "resourceType": "User", + "created": %q, + "lastModified": %q, + "location": "%s/scim/v2/Users/%s" + } + }`, + scimCore.SchemaUser, + ts.TenantA.user.ID, + ts.TenantA.user.CreatedAt.UTC().Format(time.RFC3339Nano), + ts.TenantA.user.UpdatedAt.UTC().Format(time.RFC3339Nano), + ts.Config.API.ExternalURL, ts.TenantA.user.ID, + ), w.Body.String()) + }) + + ts.Run("scopes each provider to its own users", func() { + require.Equal(ts.T(), http.StatusOK, ts.get(ts.TenantB.user.ID.String(), ts.TenantB.token).Code) + }) + + ts.Run("maps the attributes the provider supplied", func() { + c := seedSCIMTenant(ts.T(), ts.API.db, "scim_token_c", "stale@example.com", map[string]interface{}{ + "email": "bjensen@example.com", + "preferred_username": "bjensen", + "name": "Ms. Barbara Jane Jensen, III", + "family_name": "Jensen", + "given_name": "Barbara", + }) + + w := ts.get(c.user.ID.String(), c.token) + + require.Equal(ts.T(), http.StatusOK, w.Code) + require.JSONEq(ts.T(), fmt.Sprintf(`{ + "schemas": [%q], + "id": %q, + "userName": "bjensen", + "name": { + "formatted": "Ms. Barbara Jane Jensen, III", + "familyName": "Jensen", + "givenName": "Barbara" + }, + "emails": [{"value": "bjensen@example.com", "primary": true}], + "meta": { + "resourceType": "User", + "created": %q, + "lastModified": %q, + "location": "%s/scim/v2/Users/%s" + } + }`, + scimCore.SchemaUser, + c.user.ID, + c.user.CreatedAt.UTC().Format(time.RFC3339Nano), + c.user.UpdatedAt.UTC().Format(time.RFC3339Nano), + ts.Config.API.ExternalURL, c.user.ID, + ), w.Body.String()) + }) +} + +func (ts *SCIMUsersTestSuite) TestTenantIsolation() { + ts.Run("hides a user belonging to another provider", func() { + w := ts.get(ts.TenantB.user.ID.String(), ts.TenantA.token) + + require.Equal(ts.T(), http.StatusNotFound, w.Code) + require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) + require.Contains(ts.T(), w.Body.String(), scimProtocol.SchemaError) + }) + + ts.Run("returns the same 404 for an unknown id", func() { + unknown := ts.get(uuid.Must(uuid.NewV4()).String(), ts.TenantA.token) + other := ts.get(ts.TenantB.user.ID.String(), ts.TenantA.token) + + require.Equal(ts.T(), http.StatusNotFound, unknown.Code) + require.Equal(ts.T(), other.Body.String(), unknown.Body.String()) + }) + + ts.Run("returns 404 for a malformed id", func() { + w := ts.get("not-a-uuid", ts.TenantA.token) + + require.Equal(ts.T(), http.StatusNotFound, w.Code) + require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) + }) +} + +func (ts *SCIMUsersTestSuite) TestAuthentication() { + ts.Run("requires a bearer token", func() { + w := ts.get(ts.TenantA.user.ID.String(), "") + + require.Equal(ts.T(), http.StatusUnauthorized, w.Code) + require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) + require.Equal(ts.T(), "Bearer", w.Header().Get("WWW-Authenticate")) + require.Contains(ts.T(), w.Body.String(), scimProtocol.SchemaError) + }) + + ts.Run("rejects an unknown token", func() { + w := ts.get(ts.TenantA.user.ID.String(), "scim_nope") + + require.Equal(ts.T(), http.StatusUnauthorized, w.Code) + require.Equal(ts.T(), "Bearer", w.Header().Get("WWW-Authenticate")) + }) +} + +// Kept out of TestAuthentication because it mutates the seeded provider, which +// SetupTest only restores between suite methods. +func (ts *SCIMUsersTestSuite) TestRejectsADisabledProvider() { + disabled := true + ts.TenantB.provider.Disabled = &disabled + require.NoError(ts.T(), ts.API.db.Update(ts.TenantB.provider)) + + w := ts.get(ts.TenantB.user.ID.String(), ts.TenantB.token) + + require.Equal(ts.T(), http.StatusForbidden, w.Code) + require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) +} + +func (ts *SCIMUsersTestSuite) TestStaysHiddenWhenTheFeatureFlagIsOff() { + disabled, _, err := setupAPIForTest() + require.NoError(ts.T(), err) + + r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+ts.TenantA.user.ID.String(), nil) + r.Header.Set("Authorization", "Bearer "+ts.TenantA.token) + w := httptest.NewRecorder() + disabled.handler.ServeHTTP(w, r) + + require.Equal(ts.T(), http.StatusNotFound, w.Code) + require.Equal(ts.T(), "application/json", w.Header().Get("Content-Type")) + require.NotContains(ts.T(), w.Body.String(), scimProtocol.SchemaError) +} + +func seedSCIMTenant(t *testing.T, conn *storage.Connection, token, email string, extraClaims ...map[string]interface{}) *scimTenant { + t.Helper() + + id := uuid.Must(uuid.NewV4()).String() + provider := &models.SSOProvider{ + SAMLProvider: models.SAMLProvider{ + EntityID: "https://example.com/saml/metadata/" + id, + MetadataXML: "", + }, + SSODomains: []models.SSODomain{ + {Domain: id + ".local"}, + }, + } + provider.UpdateSCIMToken(token) + require.NoError(t, conn.Eager().Create(provider)) + + user, err := models.NewUser("", email, "", "authenticated", nil) + require.NoError(t, err) + user.IsSSOUser = true + require.NoError(t, conn.Create(user)) + + claims := map[string]interface{}{ + "sub": user.ID.String(), + "email": email, + } + for _, extra := range extraClaims { + for key, value := range extra { + claims[key] = value + } + } + + identity, err := models.NewIdentity(user, provider.ProviderType(), claims) + require.NoError(t, err) + require.NoError(t, conn.Create(identity)) + + return &scimTenant{provider: provider, user: user, token: token} +} From b2f16e3c6b5152efa5f148af713476092be1733e Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 16:32:01 -0600 Subject: [PATCH 17/18] test: generate n users for two different tenants in tests --- internal/api/scim_users_test.go | 112 +++++++++++++++++++++----------- 1 file changed, 75 insertions(+), 37 deletions(-) diff --git a/internal/api/scim_users_test.go b/internal/api/scim_users_test.go index 86b1496d9c..83d755c19a 100644 --- a/internal/api/scim_users_test.go +++ b/internal/api/scim_users_test.go @@ -2,6 +2,7 @@ package api import ( "fmt" + "math/rand/v2" "net/http" "net/http/httptest" "testing" @@ -17,9 +18,14 @@ import ( "github.com/supabase/auth/internal/storage" ) -type scimTenant struct { +const ( + minTenantUsers = 2 + maxTenantUsers = 5 +) + +type tenant struct { provider *models.SSOProvider - user *models.User + users []*models.User token string } @@ -27,8 +33,8 @@ type SCIMUsersTestSuite struct { suite.Suite API *API Config *conf.GlobalConfiguration - TenantA *scimTenant - TenantB *scimTenant + TenantA *tenant + TenantB *tenant } func TestSCIMUsers(t *testing.T) { @@ -46,8 +52,8 @@ func TestSCIMUsers(t *testing.T) { func (ts *SCIMUsersTestSuite) SetupTest() { require.NoError(ts.T(), models.TruncateAll(ts.API.db)) - ts.TenantA = seedSCIMTenant(ts.T(), ts.API.db, "scim_token_a", "a@example.com") - ts.TenantB = seedSCIMTenant(ts.T(), ts.API.db, "scim_token_b", "b@example.com") + ts.TenantA = seedSCIMTenant(ts.T(), ts.API.db, "example.com") + ts.TenantB = seedSCIMTenant(ts.T(), ts.API.db, "example.org") } func (ts *SCIMUsersTestSuite) get(id, token string) *httptest.ResponseRecorder { @@ -62,15 +68,17 @@ func (ts *SCIMUsersTestSuite) get(id, token string) *httptest.ResponseRecorder { func (ts *SCIMUsersTestSuite) TestGetUser() { ts.Run("returns the user that belongs to the token's provider", func() { - w := ts.get(ts.TenantA.user.ID.String(), ts.TenantA.token) + user := ts.TenantA.users[0] + + w := ts.get(user.ID.String(), ts.TenantA.token) require.Equal(ts.T(), http.StatusOK, w.Code) require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) require.JSONEq(ts.T(), fmt.Sprintf(`{ "schemas": [%q], "id": %q, - "userName": "a@example.com", - "emails": [{"value": "a@example.com", "primary": true}], + "userName": %q, + "emails": [{"value": %q, "primary": true}], "meta": { "resourceType": "User", "created": %q, @@ -79,19 +87,36 @@ func (ts *SCIMUsersTestSuite) TestGetUser() { } }`, scimCore.SchemaUser, - ts.TenantA.user.ID, - ts.TenantA.user.CreatedAt.UTC().Format(time.RFC3339Nano), - ts.TenantA.user.UpdatedAt.UTC().Format(time.RFC3339Nano), - ts.Config.API.ExternalURL, ts.TenantA.user.ID, + user.ID, + user.GetEmail(), + user.GetEmail(), + user.CreatedAt.UTC().Format(time.RFC3339Nano), + user.UpdatedAt.UTC().Format(time.RFC3339Nano), + ts.Config.API.ExternalURL, user.ID, ), w.Body.String()) }) + ts.Run("returns every user the tenant provisioned", func() { + require.Greater(ts.T(), len(ts.TenantA.users), 1) + + for _, user := range ts.TenantA.users { + w := ts.get(user.ID.String(), ts.TenantA.token) + + require.Equal(ts.T(), http.StatusOK, w.Code, "user %s", user.ID) + require.Contains(ts.T(), w.Body.String(), user.GetEmail()) + } + }) + ts.Run("scopes each provider to its own users", func() { - require.Equal(ts.T(), http.StatusOK, ts.get(ts.TenantB.user.ID.String(), ts.TenantB.token).Code) + require.Equal(ts.T(), http.StatusOK, ts.get(ts.TenantA.users[0].ID.String(), ts.TenantA.token).Code) + require.Equal(ts.T(), http.StatusNotFound, ts.get(ts.TenantA.users[0].ID.String(), ts.TenantB.token).Code) + + require.Equal(ts.T(), http.StatusOK, ts.get(ts.TenantB.users[0].ID.String(), ts.TenantB.token).Code) + require.Equal(ts.T(), http.StatusNotFound, ts.get(ts.TenantB.users[0].ID.String(), ts.TenantA.token).Code) }) ts.Run("maps the attributes the provider supplied", func() { - c := seedSCIMTenant(ts.T(), ts.API.db, "scim_token_c", "stale@example.com", map[string]interface{}{ + user := seedSCIMUser(ts.T(), ts.API.db, ts.TenantA.provider, "stale@example.com", map[string]interface{}{ "email": "bjensen@example.com", "preferred_username": "bjensen", "name": "Ms. Barbara Jane Jensen, III", @@ -99,7 +124,7 @@ func (ts *SCIMUsersTestSuite) TestGetUser() { "given_name": "Barbara", }) - w := ts.get(c.user.ID.String(), c.token) + w := ts.get(user.ID.String(), ts.TenantA.token) require.Equal(ts.T(), http.StatusOK, w.Code) require.JSONEq(ts.T(), fmt.Sprintf(`{ @@ -120,26 +145,28 @@ func (ts *SCIMUsersTestSuite) TestGetUser() { } }`, scimCore.SchemaUser, - c.user.ID, - c.user.CreatedAt.UTC().Format(time.RFC3339Nano), - c.user.UpdatedAt.UTC().Format(time.RFC3339Nano), - ts.Config.API.ExternalURL, c.user.ID, + user.ID, + user.CreatedAt.UTC().Format(time.RFC3339Nano), + user.UpdatedAt.UTC().Format(time.RFC3339Nano), + ts.Config.API.ExternalURL, user.ID, ), w.Body.String()) }) } func (ts *SCIMUsersTestSuite) TestTenantIsolation() { - ts.Run("hides a user belonging to another provider", func() { - w := ts.get(ts.TenantB.user.ID.String(), ts.TenantA.token) + ts.Run("hides every user belonging to another provider", func() { + for _, user := range ts.TenantB.users { + w := ts.get(user.ID.String(), ts.TenantA.token) - require.Equal(ts.T(), http.StatusNotFound, w.Code) - require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) - require.Contains(ts.T(), w.Body.String(), scimProtocol.SchemaError) + require.Equal(ts.T(), http.StatusNotFound, w.Code, "user %s", user.ID) + require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) + require.Contains(ts.T(), w.Body.String(), scimProtocol.SchemaError) + } }) ts.Run("returns the same 404 for an unknown id", func() { unknown := ts.get(uuid.Must(uuid.NewV4()).String(), ts.TenantA.token) - other := ts.get(ts.TenantB.user.ID.String(), ts.TenantA.token) + other := ts.get(ts.TenantB.users[0].ID.String(), ts.TenantA.token) require.Equal(ts.T(), http.StatusNotFound, unknown.Code) require.Equal(ts.T(), other.Body.String(), unknown.Body.String()) @@ -155,7 +182,7 @@ func (ts *SCIMUsersTestSuite) TestTenantIsolation() { func (ts *SCIMUsersTestSuite) TestAuthentication() { ts.Run("requires a bearer token", func() { - w := ts.get(ts.TenantA.user.ID.String(), "") + w := ts.get(ts.TenantA.users[0].ID.String(), "") require.Equal(ts.T(), http.StatusUnauthorized, w.Code) require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) @@ -164,21 +191,19 @@ func (ts *SCIMUsersTestSuite) TestAuthentication() { }) ts.Run("rejects an unknown token", func() { - w := ts.get(ts.TenantA.user.ID.String(), "scim_nope") + w := ts.get(ts.TenantA.users[0].ID.String(), uuid.Must(uuid.NewV4()).String()) require.Equal(ts.T(), http.StatusUnauthorized, w.Code) require.Equal(ts.T(), "Bearer", w.Header().Get("WWW-Authenticate")) }) } -// Kept out of TestAuthentication because it mutates the seeded provider, which -// SetupTest only restores between suite methods. func (ts *SCIMUsersTestSuite) TestRejectsADisabledProvider() { disabled := true ts.TenantB.provider.Disabled = &disabled require.NoError(ts.T(), ts.API.db.Update(ts.TenantB.provider)) - w := ts.get(ts.TenantB.user.ID.String(), ts.TenantB.token) + w := ts.get(ts.TenantB.users[0].ID.String(), ts.TenantB.token) require.Equal(ts.T(), http.StatusForbidden, w.Code) require.Equal(ts.T(), scimProtocol.MediaType, w.Header().Get("Content-Type")) @@ -188,7 +213,7 @@ func (ts *SCIMUsersTestSuite) TestStaysHiddenWhenTheFeatureFlagIsOff() { disabled, _, err := setupAPIForTest() require.NoError(ts.T(), err) - r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+ts.TenantA.user.ID.String(), nil) + r := httptest.NewRequest(http.MethodGet, "/scim/v2/Users/"+ts.TenantA.users[0].ID.String(), nil) r.Header.Set("Authorization", "Bearer "+ts.TenantA.token) w := httptest.NewRecorder() disabled.handler.ServeHTTP(w, r) @@ -198,7 +223,7 @@ func (ts *SCIMUsersTestSuite) TestStaysHiddenWhenTheFeatureFlagIsOff() { require.NotContains(ts.T(), w.Body.String(), scimProtocol.SchemaError) } -func seedSCIMTenant(t *testing.T, conn *storage.Connection, token, email string, extraClaims ...map[string]interface{}) *scimTenant { +func seedSCIMTenant(t *testing.T, conn *storage.Connection, domain string) *tenant { t.Helper() id := uuid.Must(uuid.NewV4()).String() @@ -207,13 +232,26 @@ func seedSCIMTenant(t *testing.T, conn *storage.Connection, token, email string, EntityID: "https://example.com/saml/metadata/" + id, MetadataXML: "", }, - SSODomains: []models.SSODomain{ - {Domain: id + ".local"}, - }, + SSODomains: []models.SSODomain{{Domain: domain}}, } + token := uuid.Must(uuid.NewV4()).String() provider.UpdateSCIMToken(token) require.NoError(t, conn.Eager().Create(provider)) + count := minTenantUsers + rand.IntN(maxTenantUsers-minTenantUsers+1) + + users := make([]*models.User, 0, count) + for range count { + email := uuid.Must(uuid.NewV4()).String() + "@" + domain + users = append(users, seedSCIMUser(t, conn, provider, email)) + } + + return &tenant{provider: provider, users: users, token: token} +} + +func seedSCIMUser(t *testing.T, conn *storage.Connection, provider *models.SSOProvider, email string, extraClaims ...map[string]interface{}) *models.User { + t.Helper() + user, err := models.NewUser("", email, "", "authenticated", nil) require.NoError(t, err) user.IsSSOUser = true @@ -233,5 +271,5 @@ func seedSCIMTenant(t *testing.T, conn *storage.Connection, token, email string, require.NoError(t, err) require.NoError(t, conn.Create(identity)) - return &scimTenant{provider: provider, user: user, token: token} + return user } From 4a99624d8d5aa7ae12024b92504470f0a9ac75b8 Mon Sep 17 00:00:00 2001 From: mo khan Date: Fri, 7 Aug 2026 17:43:37 -0600 Subject: [PATCH 18/18] test: update the claims to match an actual saml assertion --- internal/api/scim_users_test.go | 50 +++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/internal/api/scim_users_test.go b/internal/api/scim_users_test.go index 83d755c19a..39e21b5ac9 100644 --- a/internal/api/scim_users_test.go +++ b/internal/api/scim_users_test.go @@ -27,6 +27,7 @@ type tenant struct { provider *models.SSOProvider users []*models.User token string + domain string } type SCIMUsersTestSuite struct { @@ -115,13 +116,47 @@ func (ts *SCIMUsersTestSuite) TestGetUser() { require.Equal(ts.T(), http.StatusNotFound, ts.get(ts.TenantB.users[0].ID.String(), ts.TenantA.token).Code) }) - ts.Run("maps the attributes the provider supplied", func() { + ts.Run("omits name when the provider has no attribute mapping", func() { + user := ts.TenantA.users[0] + + w := ts.get(user.ID.String(), ts.TenantA.token) + + require.Equal(ts.T(), http.StatusOK, w.Code) + require.NotContains(ts.T(), w.Body.String(), `"name"`) + require.Contains(ts.T(), w.Body.String(), fmt.Sprintf(`"userName":%q`, user.GetEmail())) + }) + + ts.Run("keeps custom claims out of the response", func() { + user := seedSCIMUser(ts.T(), ts.API.db, ts.TenantA.provider, "custom@"+ts.TenantA.domain, map[string]interface{}{ + "custom_claims": map[string]interface{}{"department": "engineering"}, + }) + + w := ts.get(user.ID.String(), ts.TenantA.token) + + require.Equal(ts.T(), http.StatusOK, w.Code) + require.NotContains(ts.T(), w.Body.String(), "department") + require.NotContains(ts.T(), w.Body.String(), "engineering") + }) + + ts.Run("identifies the resource by user id, not by the NameID", func() { + opaque := seedSCIMUser(ts.T(), ts.API.db, ts.TenantA.provider, "persistent@"+ts.TenantA.domain, map[string]interface{}{ + "sub": uuid.Must(uuid.NewV4()).String(), + }) + + w := ts.get(opaque.ID.String(), ts.TenantA.token) + + require.Equal(ts.T(), http.StatusOK, w.Code) + require.Contains(ts.T(), w.Body.String(), fmt.Sprintf(`"id":%q`, opaque.ID)) + }) + + ts.Run("maps the attributes when the provider has an attribute mapping", func() { user := seedSCIMUser(ts.T(), ts.API.db, ts.TenantA.provider, "stale@example.com", map[string]interface{}{ "email": "bjensen@example.com", "preferred_username": "bjensen", "name": "Ms. Barbara Jane Jensen, III", "family_name": "Jensen", "given_name": "Barbara", + "middle_name": "Jane", }) w := ts.get(user.ID.String(), ts.TenantA.token) @@ -134,7 +169,8 @@ func (ts *SCIMUsersTestSuite) TestGetUser() { "name": { "formatted": "Ms. Barbara Jane Jensen, III", "familyName": "Jensen", - "givenName": "Barbara" + "givenName": "Barbara", + "middleName": "Jane" }, "emails": [{"value": "bjensen@example.com", "primary": true}], "meta": { @@ -246,7 +282,7 @@ func seedSCIMTenant(t *testing.T, conn *storage.Connection, domain string) *tena users = append(users, seedSCIMUser(t, conn, provider, email)) } - return &tenant{provider: provider, users: users, token: token} + return &tenant{provider: provider, users: users, token: token, domain: domain} } func seedSCIMUser(t *testing.T, conn *storage.Connection, provider *models.SSOProvider, email string, extraClaims ...map[string]interface{}) *models.User { @@ -258,8 +294,12 @@ func seedSCIMUser(t *testing.T, conn *storage.Connection, provider *models.SSOPr require.NoError(t, conn.Create(user)) claims := map[string]interface{}{ - "sub": user.ID.String(), - "email": email, + "iss": provider.SAMLProvider.EntityID, + "sub": email, + "email": email, + "email_verified": true, + "phone_verified": false, + "custom_claims": map[string]interface{}{}, } for _, extra := range extraClaims { for key, value := range extra {