From 539b41358427a876272aefe304724e70d9c157ef Mon Sep 17 00:00:00 2001 From: linoman <2051016+linoman@users.noreply.github.com> Date: Mon, 25 Aug 2025 16:21:10 +0200 Subject: [PATCH] SAML catalog: Add metrics for saml catalog logins (#109904) * Add samlCatalog metric * Add samlCatalog metric to stats * Define hook for successful SamlCatalog metrics * Register new hook * Add tests * Rework the collected stats and split it into versions --- pkg/services/authn/authnimpl/registration.go | 1 + .../authn/authnimpl/sync/user_sync.go | 37 ++++++++ .../authn/authnimpl/sync/user_sync_test.go | 88 +++++++++++++++++++ 3 files changed, 126 insertions(+) diff --git a/pkg/services/authn/authnimpl/registration.go b/pkg/services/authn/authnimpl/registration.go index ce0ed9a9e89..ea7afcd921f 100644 --- a/pkg/services/authn/authnimpl/registration.go +++ b/pkg/services/authn/authnimpl/registration.go @@ -152,6 +152,7 @@ func ProvideRegistration( authnSvc.RegisterPostAuthHook(rbacSync.SyncPermissionsHook, 120) authnSvc.RegisterPostLoginHook(orgSync.SetDefaultOrgHook, 140) + authnSvc.RegisterPostLoginHook(userSync.CatalogLoginHook, 145) authnSvc.RegisterPostLoginHook(rbacSync.ClearUserPermissionCacheHook, 170) nsSync := sync.ProvideNamespaceSync(cfg) diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index a82b1434152..01315058a9d 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/pkg/services/authn/authnimpl/sync/user_sync.go @@ -5,8 +5,10 @@ import ( "errors" "fmt" "strconv" + "sync" "sync/atomic" + "github.com/Masterminds/semver/v3" claims "github.com/grafana/authlib/types" "go.opentelemetry.io/otel/attribute" "golang.org/x/sync/singleflight" @@ -128,6 +130,7 @@ type UserSync struct { scimUtil *scimutil.SCIMUtil staticConfig *StaticSCIMConfig scimSuccessfulLogin atomic.Bool + samlCatalogStats sync.Map } // GetUsageStats implements registry.ProvidesUsageStats @@ -138,9 +141,43 @@ func (s *UserSync) GetUsageStats(ctx context.Context) map[string]any { } else { stats["stats.features.scim.has_successful_login.count"] = 0 } + + s.samlCatalogStats.Range(func(key, value interface{}) bool { + version := key.(string) + flag := value.(*atomic.Bool) + if flag.Load() { + stats[fmt.Sprintf("stats.features.saml.catalog_version_%s.count", version)] = 1 + } else { + stats[fmt.Sprintf("stats.features.saml.catalog_version_%s.count", version)] = 0 + } + return true + }) return stats } +func (s *UserSync) setSamlCatalogVersion(version string) { + value, loaded := s.samlCatalogStats.LoadOrStore(version, &atomic.Bool{}) + flag := value.(*atomic.Bool) + flag.Store(true) + + if !loaded { + s.log.Info("New SAML catalog version detected", "version", version) + } +} + +func (s *UserSync) CatalogLoginHook(_ context.Context, identity *authn.Identity, r *authn.Request, err error) { + if err != nil || identity == nil || !identity.ClientParams.SyncUser || r == nil { + return + } + catalogVersion := r.GetMeta("catalog_version") + if _, err := semver.NewVersion(catalogVersion); err != nil { + s.log.Warn("The SAML catalog used for this login has an incorrect version format", "catalogVersion", catalogVersion) + return + } + + s.setSamlCatalogVersion(catalogVersion) +} + // ValidateUserProvisioningHook validates if a user should be allowed access based on provisioning status and configuration func (s *UserSync) ValidateUserProvisioningHook(ctx context.Context, currentIdentity *authn.Identity, _ *authn.Request) error { log := s.log.FromContext(ctx).New("auth_module", currentIdentity.AuthenticatedBy, "auth_id", currentIdentity.AuthID) diff --git a/pkg/services/authn/authnimpl/sync/user_sync_test.go b/pkg/services/authn/authnimpl/sync/user_sync_test.go index 49e50f55fdc..e9710d53ff9 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/user_sync_test.go @@ -3,6 +3,7 @@ package sync import ( "context" "errors" + "fmt" "strconv" "testing" @@ -975,6 +976,93 @@ func TestUserSync_FetchSyncedUserHook(t *testing.T) { } } +func TestUserSync_CatalogLoginHook(t *testing.T) { + type testCase struct { + name string + identity *authn.Identity + expectFlagSet bool + catalogVersion string + } + + tests := []testCase{ + { + name: "should skip hook when SyncUser flag is not enabled", + identity: &authn.Identity{ + ClientParams: authn.ClientParams{ + SyncUser: false, + }, + }, + expectFlagSet: false, + }, + { + name: "should skip hook when request is nil", + identity: &authn.Identity{ + ClientParams: authn.ClientParams{ + SyncUser: true, + }, + }, + }, + { + name: "should skip hook when catalog version is not set", + identity: &authn.Identity{ + ClientParams: authn.ClientParams{ + SyncUser: true, + }, + }, + expectFlagSet: false, + }, + { + name: "should not set loginflag when catalog version is set incorrectly", + identity: &authn.Identity{ + ClientParams: authn.ClientParams{ + SyncUser: true, + }, + }, + catalogVersion: "v0aplha1", + expectFlagSet: false, + }, + { + name: "should not set loginflag when catalog version is empty", + identity: &authn.Identity{ + ClientParams: authn.ClientParams{ + SyncUser: true, + }, + }, + expectFlagSet: false, + }, + { + name: "should set successful loginflag when catalog version is set correctly", + identity: &authn.Identity{ + ClientParams: authn.ClientParams{ + SyncUser: true, + }, + }, + catalogVersion: "1.0.0", + expectFlagSet: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := UserSync{ + tracer: tracing.InitializeTracerForTest(), + log: log.New("test"), + } + + req := authn.Request{} + if tt.catalogVersion != "" { + req.SetMeta("catalog_version", tt.catalogVersion) + } + + s.CatalogLoginHook(context.Background(), tt.identity, &req, nil) + usageStats := s.GetUsageStats(context.Background()) + countIndex := fmt.Sprintf("stats.features.saml.catalog_version_%s.count", tt.catalogVersion) + countResult := usageStats[countIndex] != nil && usageStats[countIndex].(int) == 1 + assert.Equal(t, tt.expectFlagSet, countResult) + }) + } +} + func TestUserSync_EnableDisabledUserHook(t *testing.T) { type testCase struct { desc string