From 11ea1dfba8cff13499d832b7af937855fc14d787 Mon Sep 17 00:00:00 2001 From: Moustafa Baiou Date: Wed, 2 Jul 2025 13:26:27 -0400 Subject: [PATCH 1/5] Alerting: Fix notification policy conflicts originating from provenance mismatch (#107343) * Alerting: Fix notification policy conflicts originating from provenance mismatch Sometimes the provenance field on the notification policy route can mismatch with the actual provenance in the provenance store. This leads to 409s when trying to update the notification policy via the k8s apis. We avoid this by ignoring the provenance field when calculating the fingerprint. Co-authored-by: Matt Jacobson * fix tests which validate stability --------- Co-authored-by: Matt Jacobson --- pkg/services/ngalert/provisioning/notification_policies.go | 1 - .../ngalert/provisioning/notification_policies_test.go | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/services/ngalert/provisioning/notification_policies.go b/pkg/services/ngalert/provisioning/notification_policies.go index fed23d7bc69..f6563fe45ac 100644 --- a/pkg/services/ngalert/provisioning/notification_policies.go +++ b/pkg/services/ngalert/provisioning/notification_policies.go @@ -277,7 +277,6 @@ func writeToHash(sum hash.Hash, r *definitions.Route) { writeDuration(r.GroupWait) writeDuration(r.GroupInterval) writeDuration(r.RepeatInterval) - writeString(string(r.Provenance)) for _, route := range r.Routes { writeToHash(sum, route) } diff --git a/pkg/services/ngalert/provisioning/notification_policies_test.go b/pkg/services/ngalert/provisioning/notification_policies_test.go index f32d6908fd6..e52460c5c04 100644 --- a/pkg/services/ngalert/provisioning/notification_policies_test.go +++ b/pkg/services/ngalert/provisioning/notification_policies_test.go @@ -428,13 +428,14 @@ func TestRoute_Fingerprint(t *testing.T) { } t.Run("stable across code changes", func(t *testing.T) { - expectedFingerprint := "7faba12778df93b8" // If this is a valid fingerprint generation change, update the expected value. + expectedFingerprint := "450c06a7f4a66675" // If this is a valid fingerprint generation change, update the expected value. assert.Equal(t, expectedFingerprint, calculateRouteFingerprint(baseRouteGen())) }) t.Run("unstable across field modification", func(t *testing.T) { fingerprint := calculateRouteFingerprint(baseRouteGen()) excludedFields := map[string]struct{}{ - "Routes": {}, + "Routes": {}, + "Provenance": {}, } reflectVal := reflect.ValueOf(&completelyDifferentRoute).Elem() From d57155a19beeb60a495b3b79a42a4ae1a25862e6 Mon Sep 17 00:00:00 2001 From: Kevin Minehart <5140827+kminehart@users.noreply.github.com> Date: Wed, 2 Jul 2025 13:43:59 -0500 Subject: [PATCH 2/5] CI: Support large build IDs in artifact builds (#107533) * CI: Support large build IDs in artifact builds * Use run_id instead of run_number in prerelease builds --- .github/actions/build-package/action.yml | 2 +- .github/workflows/release-build.yml | 6 +++--- pkg/build/daggerbuild/msi/wxs.go | 3 +++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/actions/build-package/action.yml b/.github/actions/build-package/action.yml index 5b9275c95fe..123484071e7 100644 --- a/.github/actions/build-package/action.yml +++ b/.github/actions/build-package/action.yml @@ -30,7 +30,7 @@ inputs: build-id: type: string description: an identifier number which can be traced back to the workflow run. - default: ${{github.run_number}} + default: ${{github.run_id}} required: false patches-repo: type: string diff --git a/.github/workflows/release-build.yml b/.github/workflows/release-build.yml index 2ee344e8ebd..54c81f1091f 100644 --- a/.github/workflows/release-build.yml +++ b/.github/workflows/release-build.yml @@ -59,7 +59,7 @@ jobs: run: jq -r .version package.json | sed -s "s/pre/${BUILD_ID}/g" > VERSION env: REF_NAME: ${{ github.ref_name }} - BUILD_ID: ${{ github.run_number }} + BUILD_ID: ${{ github.run_id }} - id: output run: | echo "version=$(cat VERSION)" >> "$GITHUB_OUTPUT" @@ -90,7 +90,7 @@ jobs: env: REF: ${{ github.ref_name }} VERSION: ${{ needs.setup.outputs.version }} - BUILD_ID: ${{ github.run_number }} + BUILD_ID: ${{ github.run_id }} BUCKET: grafana-prerelease GRAFANA_COMMIT: ${{ needs.setup.outputs.grafana-commit }} with: @@ -163,7 +163,7 @@ jobs: version: ${{ needs.setup.outputs.version }} output: artifacts-${{ matrix.name }}.txt verify: true - build-id: ${{ github.run_number }} + build-id: ${{ github.run_id }} - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 with: name: artifacts-list-${{ matrix.name }} diff --git a/pkg/build/daggerbuild/msi/wxs.go b/pkg/build/daggerbuild/msi/wxs.go index bad26bb5a37..443545de56a 100644 --- a/pkg/build/daggerbuild/msi/wxs.go +++ b/pkg/build/daggerbuild/msi/wxs.go @@ -50,6 +50,9 @@ func WxsVersion(ersion string) string { v = "0" } + if len(v) > 5 { + v = v[len(v)-5:] + } return fmt.Sprintf("%s.%s.%s.%s", major, minor, patch, v) } return fmt.Sprintf("%s.%s.%s.0", major, minor, patch) From b483a04aecadad01010b7aff88dbd4efe333af79 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Wed, 2 Jul 2025 21:35:24 +0200 Subject: [PATCH 3/5] Alerting: Send merged configuration to the remote alertmanager (#107004) --- .../api/tooling/definitions/alertmanager.go | 18 +- pkg/services/ngalert/notifier/crypto.go | 4 +- pkg/services/ngalert/remote/alertmanager.go | 47 +++- .../ngalert/remote/alertmanager_test.go | 223 ++++++++++++++- .../client/alertmanager_configuration.go | 4 +- .../alertmanager/alertmanager_scenario.go | 9 + pkg/tests/alertmanager/mimir.go | 67 +++++ .../alerting/api_remote_alertmanager_test.go | 262 ++++++++++++++++++ pkg/tests/testinfra/testinfra.go | 14 + 9 files changed, 621 insertions(+), 27 deletions(-) create mode 100644 pkg/tests/alertmanager/mimir.go create mode 100644 pkg/tests/api/alerting/api_remote_alertmanager_test.go diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go index ed568763235..dfd39e73f56 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go @@ -1051,21 +1051,9 @@ func (c *GettableApiAlertingConfig) UnmarshalYAML(value *yaml.Node) error { func (c *GettableApiAlertingConfig) validate() error { receivers := make(map[string]struct{}, len(c.Receivers)) - var hasGrafReceivers, hasAMReceivers bool - for _, r := range c.Receivers { - receivers[r.Name] = struct{}{} - switch r.Type() { - case GrafanaReceiverType: - hasGrafReceivers = true - case AlertmanagerReceiverType: - hasAMReceivers = true - default: - continue - } - } - - if hasGrafReceivers && hasAMReceivers { - return fmt.Errorf("cannot mix Alertmanager & Grafana receiver types") + // Populate the receivers map with defined receiver names + for _, receiver := range c.Receivers { + receivers[receiver.Name] = struct{}{} } for _, receiver := range AllReceivers(c.Route.AsAMRoute()) { diff --git a/pkg/services/ngalert/notifier/crypto.go b/pkg/services/ngalert/notifier/crypto.go index 76762dffc68..7a80a70c27a 100644 --- a/pkg/services/ngalert/notifier/crypto.go +++ b/pkg/services/ngalert/notifier/crypto.go @@ -256,9 +256,11 @@ func (c *alertmanagerCrypto) EncryptExtraConfigs(ctx context.Context, config *de func (c *alertmanagerCrypto) DecryptExtraConfigs(ctx context.Context, config *definitions.PostableUserConfig) error { for i := range config.ExtraConfigs { + // Check if the config is encrypted by trying to base64 decode it encryptedValue, err := base64.StdEncoding.DecodeString(config.ExtraConfigs[i].AlertmanagerConfig) if err != nil { - return fmt.Errorf("failed to base64 decode extra configuration: %w", err) + // If it can't be base64 decoded, assume it's already decrypted and skip + continue } decryptedValue, err := c.secrets.Decrypt(ctx, encryptedValue) diff --git a/pkg/services/ngalert/remote/alertmanager.go b/pkg/services/ngalert/remote/alertmanager.go index 006577edb55..56cf9d8ec26 100644 --- a/pkg/services/ngalert/remote/alertmanager.go +++ b/pkg/services/ngalert/remote/alertmanager.go @@ -50,6 +50,7 @@ func NoopAutogenFn(_ context.Context, _ log.Logger, _ int64, _ *apimodels.Postab type Crypto interface { Decrypt(ctx context.Context, payload []byte) ([]byte, error) + DecryptExtraConfigs(ctx context.Context, config *apimodels.PostableUserConfig) error } type Alertmanager struct { @@ -282,10 +283,16 @@ func (am *Alertmanager) CompareAndSendConfiguration(ctx context.Context, config if err := am.autogenFn(ctx, am.log, am.orgID, &c.AlertmanagerConfig, true); err != nil { return err } + decryptedCfg, err := am.decryptConfiguration(ctx, c) if err != nil { return err } + + // Decrypt and merge extra configs + if err := am.mergeExtraConfigs(ctx, decryptedCfg); err != nil { + return fmt.Errorf("unable to merge extra configurations: %w", err) + } rawDecrypted, err := json.Marshal(decryptedCfg) if err != nil { return fmt.Errorf("unable to marshal decrypted configuration: %w", err) @@ -297,7 +304,7 @@ func (am *Alertmanager) CompareAndSendConfiguration(ctx context.Context, config return nil } - return am.sendConfiguration(ctx, decryptedCfg, config.ConfigurationHash, config.CreatedAt, am.isDefaultConfiguration(configHash)) + return am.sendConfiguration(ctx, decryptedCfg, fmt.Sprintf("%x", configHash), config.CreatedAt, am.isDefaultConfiguration(configHash)) } func (am *Alertmanager) isDefaultConfiguration(configHash [16]byte) bool { @@ -342,6 +349,27 @@ func decrypter(ctx context.Context, crypto Crypto) models.DecryptFn { } } +// mergeExtraConfigs decrypts and applies merged configuration if extra configs exist. +func (am *Alertmanager) mergeExtraConfigs(ctx context.Context, config *apimodels.PostableUserConfig) error { + if len(config.ExtraConfigs) == 0 { + return nil + } + + if err := am.crypto.DecryptExtraConfigs(ctx, config); err != nil { + return fmt.Errorf("unable to decrypt extra configs: %w", err) + } + + mergeResult, err := config.GetMergedAlertmanagerConfig() + if err != nil { + return fmt.Errorf("unable to get merged Alertmanager configuration: %w", err) + } + config.AlertmanagerConfig = mergeResult.Config + // Clear ExtraConfigs to avoid re-processing them later + config.ExtraConfigs = nil + + return nil +} + func (am *Alertmanager) sendConfiguration(ctx context.Context, decrypted *apimodels.PostableUserConfig, hash string, createdAt int64, isDefault bool) error { am.metrics.ConfigSyncsTotal.Inc() if err := am.mimirClient.CreateGrafanaAlertmanagerConfig( @@ -380,13 +408,6 @@ func (am *Alertmanager) SendState(ctx context.Context) error { // SaveAndApplyConfig decrypts and sends a configuration to the remote Alertmanager. func (am *Alertmanager) SaveAndApplyConfig(ctx context.Context, cfg *apimodels.PostableUserConfig) error { - // Get the hash for the encrypted configuration. - rawCfg, err := json.Marshal(cfg) - if err != nil { - return err - } - hash := fmt.Sprintf("%x", md5.Sum(rawCfg)) - // Add auto-generated routes and decrypt before sending. if err := am.autogenFn(ctx, am.log, am.orgID, &cfg.AlertmanagerConfig, false); err != nil { return err @@ -396,6 +417,16 @@ func (am *Alertmanager) SaveAndApplyConfig(ctx context.Context, cfg *apimodels.P return err } + if err := am.mergeExtraConfigs(ctx, decryptedCfg); err != nil { + return fmt.Errorf("unable to merge extra configurations: %w", err) + } + + rawCfg, err := json.Marshal(decryptedCfg) + if err != nil { + return err + } + hash := fmt.Sprintf("%x", md5.Sum(rawCfg)) + return am.sendConfiguration(ctx, decryptedCfg, hash, time.Now().Unix(), false) } diff --git a/pkg/services/ngalert/remote/alertmanager_test.go b/pkg/services/ngalert/remote/alertmanager_test.go index 8ebb5de8826..3bbcb03de6b 100644 --- a/pkg/services/ngalert/remote/alertmanager_test.go +++ b/pkg/services/ngalert/remote/alertmanager_test.go @@ -11,12 +11,15 @@ import ( "net/http" "net/http/httptest" "os" + "slices" "strings" "testing" "time" "github.com/go-openapi/strfmt" amv2 "github.com/prometheus/alertmanager/api/v2/models" + "github.com/prometheus/alertmanager/config" + "github.com/prometheus/alertmanager/pkg/labels" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" @@ -361,6 +364,15 @@ func TestCompareAndSendConfiguration(t *testing.T) { require.NoError(t, err) require.NoError(t, testAutogenFn(nil, nil, 0, &cfgWithAutogenRoutes.AlertmanagerConfig, false)) + // Calculate hashes for expected configurations + cfgWithDecryptedSecretBytes, err := json.Marshal(cfgWithDecryptedSecret) + require.NoError(t, err) + cfgWithDecryptedSecretHash := fmt.Sprintf("%x", md5.Sum(cfgWithDecryptedSecretBytes)) + + cfgWithAutogenRoutesBytes, err := json.Marshal(cfgWithAutogenRoutes) + require.NoError(t, err) + cfgWithAutogenRoutesHash := fmt.Sprintf("%x", md5.Sum(cfgWithAutogenRoutesBytes)) + tests := []struct { name string config string @@ -402,6 +414,7 @@ func TestCompareAndSendConfiguration(t *testing.T) { NoopAutogenFn, &client.UserGrafanaConfig{ GrafanaAlertmanagerConfig: cfgWithDecryptedSecret, + Hash: cfgWithDecryptedSecretHash, }, nil, }, @@ -411,6 +424,7 @@ func TestCompareAndSendConfiguration(t *testing.T) { testAutogenFn, &client.UserGrafanaConfig{ GrafanaAlertmanagerConfig: cfgWithAutogenRoutes, + Hash: cfgWithAutogenRoutesHash, }, nil, }, @@ -561,6 +575,210 @@ func Test_isDefaultConfiguration(t *testing.T) { } } +func TestApplyConfigWithExtraConfigs(t *testing.T) { + const tenantID = "test" + + var configSent client.UserGrafanaConfig + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, tenantID, r.Header.Get(client.MimirTenantHeader)) + require.Equal(t, "true", r.Header.Get(client.RemoteAlertmanagerHeader)) + + if r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/config") { + require.NoError(t, json.NewDecoder(r.Body).Decode(&configSent)) + } + + w.Header().Add("content-type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(map[string]string{"status": "success"})) + })) + defer server.Close() + + var cfg apimodels.PostableUserConfig + require.NoError(t, json.Unmarshal([]byte(testGrafanaConfig), &cfg)) + + cfg.ExtraConfigs = []apimodels.ExtraConfiguration{ + { + Identifier: "test-external", + MergeMatchers: []*labels.Matcher{ + { + Type: labels.MatchEqual, + Name: "test", + Value: "value", + }, + }, + TemplateFiles: map[string]string{}, + AlertmanagerConfig: `global: + smtp_smarthost: localhost:587 + smtp_from: alerts@grafana.com +route: + receiver: extra-receiver +receivers: + - name: extra-receiver + email_configs: + - to: alerts@grafana.com`, + }, + } + + secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(db.InitTestDB(t))) + tc := notifier.NewCrypto(secretsService, nil, log.NewNopLogger()) + ctx := context.Background() + + c := AlertmanagerConfig{ + OrgID: 1, + TenantID: tenantID, + URL: server.URL, + DefaultConfig: defaultGrafanaConfig, + PromoteConfig: true, + } + + store := ngfakes.NewFakeKVStore(t) + fstore := notifier.NewFileStore(1, store) + require.NoError(t, store.Set(ctx, c.OrgID, "alertmanager", notifier.SilencesFilename, "")) + require.NoError(t, store.Set(ctx, c.OrgID, "alertmanager", notifier.NotificationLogFilename, "")) + + m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) + am, err := NewAlertmanager(ctx, c, fstore, tc, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + require.NoError(t, err) + + err = am.SaveAndApplyConfig(ctx, &cfg) + require.NoError(t, err) + + require.Equal(t, len(configSent.GrafanaAlertmanagerConfig.AlertmanagerConfig.Receivers), 2) + + var extraReceiver *apimodels.PostableApiReceiver + for _, rcv := range configSent.GrafanaAlertmanagerConfig.AlertmanagerConfig.Receivers { + if rcv.Name == "extra-receiver" { + extraReceiver = rcv + break + } + } + require.NotNil(t, extraReceiver) + require.Len(t, extraReceiver.EmailConfigs, 1) + require.Equal(t, "alerts@grafana.com", extraReceiver.EmailConfigs[0].To) + + // Verify the config hash + expectedConfigBytes, err := json.Marshal(configSent.GrafanaAlertmanagerConfig) + require.NoError(t, err) + expectedHash := fmt.Sprintf("%x", md5.Sum(expectedConfigBytes)) + require.Equal(t, expectedHash, configSent.Hash) +} + +func TestCompareAndSendConfigurationWithExtraConfigs(t *testing.T) { + const tenantID = "test" + + var configSent client.UserGrafanaConfig + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, tenantID, r.Header.Get(client.MimirTenantHeader)) + require.Equal(t, "true", r.Header.Get(client.RemoteAlertmanagerHeader)) + + if r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/config") { + require.NoError(t, json.NewDecoder(r.Body).Decode(&configSent)) + } else if r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/config") { + // If this is a GET method, Grafana requests the current configuration to compare. + // Return an empty config to ensure it gets replaced + w.Header().Add("content-type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(client.UserGrafanaConfig{ + GrafanaAlertmanagerConfig: &apimodels.PostableUserConfig{}, + })) + return + } + + w.Header().Add("content-type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(map[string]string{"status": "success"})) + })) + defer server.Close() + + cfg := apimodels.PostableUserConfig{ + AlertmanagerConfig: apimodels.PostableApiAlertingConfig{ + Config: apimodels.Config{ + Route: &apimodels.Route{ + Receiver: "grafana-default-email", + }, + }, + Receivers: []*apimodels.PostableApiReceiver{ + { + Receiver: config.Receiver{Name: "grafana-default-email"}, + PostableGrafanaReceivers: apimodels.PostableGrafanaReceivers{ + GrafanaManagedReceivers: []*apimodels.PostableGrafanaReceiver{ + { + Name: "email receiver", + Type: "email", + Settings: apimodels.RawMessage(`{"addresses":""}`), + }, + }, + }, + }, + }, + }, + ExtraConfigs: []apimodels.ExtraConfiguration{ + { + Identifier: "test-external", + MergeMatchers: []*labels.Matcher{ + { + Type: labels.MatchEqual, + Name: "test", + Value: "test", + }, + }, + AlertmanagerConfig: `global: + smtp_smarthost: localhost:587 + smtp_from: alerts@grafana.com +route: + receiver: extra-receiver +receivers: + - name: extra-receiver + email_configs: + - to: alerts@grafana.com`, + }, + }, + } + + secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(db.InitTestDB(t))) + tc := notifier.NewCrypto(secretsService, nil, log.NewNopLogger()) + ctx := context.Background() + + // Encrypt extra configs since this tests the database path + err := tc.EncryptExtraConfigs(ctx, &cfg) + require.NoError(t, err) + + c := AlertmanagerConfig{ + OrgID: 1, + TenantID: tenantID, + URL: server.URL, + DefaultConfig: defaultGrafanaConfig, + PromoteConfig: true, + } + + store := ngfakes.NewFakeKVStore(t) + fstore := notifier.NewFileStore(1, store) + require.NoError(t, store.Set(ctx, c.OrgID, "alertmanager", notifier.SilencesFilename, "")) + require.NoError(t, store.Set(ctx, c.OrgID, "alertmanager", notifier.NotificationLogFilename, "")) + + m := metrics.NewRemoteAlertmanagerMetrics(prometheus.NewRegistry()) + am, err := NewAlertmanager(ctx, c, fstore, tc, NoopAutogenFn, m, tracing.InitializeTracerForTest()) + require.NoError(t, err) + + configJSON, err := json.Marshal(cfg) + require.NoError(t, err) + config := &ngmodels.AlertConfiguration{ + AlertmanagerConfiguration: string(configJSON), + } + + err = am.CompareAndSendConfiguration(ctx, config) + require.NoError(t, err) + + require.Equal(t, len(configSent.GrafanaAlertmanagerConfig.AlertmanagerConfig.Receivers), 2) + found := slices.ContainsFunc(configSent.GrafanaAlertmanagerConfig.AlertmanagerConfig.Receivers, func(rcv *apimodels.PostableApiReceiver) bool { + return strings.Contains(rcv.Name, "extra-receiver") + }) + require.True(t, found) + + // Verify the config hash + expectedConfigBytes, err := json.Marshal(configSent.GrafanaAlertmanagerConfig) + require.NoError(t, err) + expectedHash := fmt.Sprintf("%x", md5.Sum(expectedConfigBytes)) + require.Equal(t, expectedHash, configSent.Hash) +} + func TestIntegrationRemoteAlertmanagerConfiguration(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") @@ -705,7 +923,10 @@ func TestIntegrationRemoteAlertmanagerConfiguration(t *testing.T) { require.NoError(t, err) require.JSONEq(t, testGrafanaConfigWithSecret, string(got)) - require.Equal(t, fmt.Sprintf("%x", md5.Sum(encryptedConfig)), config.Hash) + + // Verify that the hash is calculated from the final configuration, including simplified routing + expectedHash := fmt.Sprintf("%x", md5.Sum(got)) + require.Equal(t, expectedHash, config.Hash, "Hash should be calculated from the final processed configuration") require.False(t, config.Default) // An error while adding auto-generated rutes should be returned. diff --git a/pkg/services/ngalert/remote/client/alertmanager_configuration.go b/pkg/services/ngalert/remote/client/alertmanager_configuration.go index eb955bf987e..164a3949011 100644 --- a/pkg/services/ngalert/remote/client/alertmanager_configuration.go +++ b/pkg/services/ngalert/remote/client/alertmanager_configuration.go @@ -3,10 +3,10 @@ package client import ( "bytes" "context" - "encoding/json" "fmt" "net/http" + "github.com/grafana/alerting/definition" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" ) @@ -53,7 +53,7 @@ func (mc *Mimir) GetGrafanaAlertmanagerConfig(ctx context.Context) (*UserGrafana } func (mc *Mimir) CreateGrafanaAlertmanagerConfig(ctx context.Context, cfg *apimodels.PostableUserConfig, hash string, createdAt int64, isDefault bool) error { - payload, err := json.Marshal(&UserGrafanaConfig{ + payload, err := definition.MarshalJSONWithSecrets(&UserGrafanaConfig{ GrafanaAlertmanagerConfig: cfg, Hash: hash, CreatedAt: createdAt, diff --git a/pkg/tests/alertmanager/alertmanager_scenario.go b/pkg/tests/alertmanager/alertmanager_scenario.go index 7855a111e40..2745c938da1 100644 --- a/pkg/tests/alertmanager/alertmanager_scenario.go +++ b/pkg/tests/alertmanager/alertmanager_scenario.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/e2e" gapi "github.com/grafana/grafana-api-golang-client" + "github.com/grafana/grafana/pkg/services/ngalert/remote/client" "github.com/stretchr/testify/require" ) @@ -43,6 +44,7 @@ type AlertmanagerScenario struct { Webhook *WebhookService Postgres *PostgresService Loki *LokiService + Mimir *MimirService } func NewAlertmanagerScenario() (*AlertmanagerScenario, error) { @@ -381,3 +383,10 @@ func mapInstancePeers(is []string) map[string][]string { return mIs } + +func (s *AlertmanagerScenario) NewMimirClient(tenantID string) (client.MimirClient, error) { + if s.Mimir == nil { + return nil, fmt.Errorf("mimir service not started") + } + return NewMimirClient("http://"+s.Mimir.HTTPEndpoint(), tenantID) +} diff --git a/pkg/tests/alertmanager/mimir.go b/pkg/tests/alertmanager/mimir.go new file mode 100644 index 00000000000..a3d99855879 --- /dev/null +++ b/pkg/tests/alertmanager/mimir.go @@ -0,0 +1,67 @@ +package alertmanager + +import ( + "fmt" + "net/url" + + "github.com/grafana/e2e" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/ngalert/metrics" + "github.com/grafana/grafana/pkg/services/ngalert/remote/client" + "github.com/prometheus/client_golang/prometheus" +) + +const ( + mimirImage = "grafana/mimir:r348-017076d8" + + mimirBinary = "/bin/mimir" + mimirHTTPPort = 33667 + mimirGRPCPort = 33668 +) + +type MimirService struct { + *e2e.HTTPService +} + +func NewMimirService(name string) *MimirService { + flags := map[string]string{ + "-target": "alertmanager", + "-server.http-listen-port": fmt.Sprintf("%d", mimirHTTPPort), + "-server.grpc-listen-port": fmt.Sprintf("%d", mimirGRPCPort), + "-alertmanager.web.external-url": "http://localhost:8080/alertmanager", + "-alertmanager-storage.backend": "filesystem", + "-alertmanager-storage.filesystem.dir": "/tmp/mimir/alertmanager", + "-alertmanager.grafana-alertmanager-compatibility-enabled": "true", + } + + return &MimirService{ + HTTPService: e2e.NewHTTPService( + name, + mimirImage, + e2e.NewCommandWithoutEntrypoint(mimirBinary, e2e.BuildArgs(flags)...), + e2e.NewHTTPReadinessProbe(mimirHTTPPort, "/ready", 200, 299), + mimirHTTPPort, + ), + } +} + +func NewMimirClient(mimirURL, tenantID string) (client.MimirClient, error) { + u, err := url.Parse(mimirURL) + if err != nil { + return nil, err + } + + cfg := &client.Config{ + URL: u, + TenantID: tenantID, + Password: "", // No password needed for test + Logger: log.NewNopLogger(), + } + + registry := prometheus.NewRegistry() + metrics := metrics.NewRemoteAlertmanagerMetrics(registry) + tracer := tracing.InitializeTracerForTest() + + return client.New(cfg, metrics, tracer) +} diff --git a/pkg/tests/api/alerting/api_remote_alertmanager_test.go b/pkg/tests/api/alerting/api_remote_alertmanager_test.go new file mode 100644 index 00000000000..9a5975a62b7 --- /dev/null +++ b/pkg/tests/api/alerting/api_remote_alertmanager_test.go @@ -0,0 +1,262 @@ +package alerting + +import ( + "context" + "fmt" + "net/http" + "net/url" + "testing" + "time" + + "github.com/stretchr/testify/require" + + apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" + "github.com/grafana/grafana/pkg/tests/alertmanager" + "github.com/grafana/grafana/pkg/tests/testinfra" +) + +// TestIntegrationRemoteAlertmanagerConfigUpload tests that when we post an alertmanager +// configuration to Grafana with remote alertmanager enabled, it gets uploaded to the remote Mimir. +func TestIntegrationRemoteAlertmanagerConfigUpload(t *testing.T) { + testinfra.SQLiteIntegrationTest(t) + + s, err := alertmanager.NewAlertmanagerScenario() + require.NoError(t, err) + defer s.Close() + + s.Mimir = alertmanager.NewMimirService("mimir") + require.NoError(t, s.StartAndWaitReady(s.Mimir)) + + mimirEndpoint := "http://" + s.Mimir.HTTPEndpoint() + + dir, gpath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + DisableLegacyAlerting: true, + EnableUnifiedAlerting: true, + DisableAnonymous: true, + AppModeProduction: true, + EnableFeatureToggles: []string{ + "alertmanagerRemotePrimary", + "alertingImportAlertmanagerAPI", + }, + RemoteAlertmanagerURL: mimirEndpoint, + }) + + grafanaListedAddr, _ := testinfra.StartGrafanaEnv(t, dir, gpath) + + apiClient := newAlertingApiClient(grafanaListedAddr, "admin", "admin") + mimirClient, err := alertmanager.NewMimirClient(mimirEndpoint, "1") + require.NoError(t, err) + + // Wait for Grafana to be ready + require.Eventually(t, func() bool { + _, status, _ := apiClient.GetAlertmanagerConfigWithStatus(t) + return status == http.StatusOK + }, 30*time.Second, time.Second, "Grafana failed to start") + + // Check that the initial Mimir config contains the default Grafana configuration + initialMimirConfig, err := mimirClient.GetGrafanaAlertmanagerConfig(context.Background()) + require.NoError(t, err) + require.NotNil(t, initialMimirConfig) // Grafana automatically syncs default config to remote alertmanager + require.NotNil(t, initialMimirConfig.GrafanaAlertmanagerConfig) + + // Initially there is just the default grafana-default-email receiver + receivers := initialMimirConfig.GrafanaAlertmanagerConfig.AlertmanagerConfig.Receivers + require.Len(t, receivers, 1) + require.Equal(t, "grafana-default-email", receivers[0].Name) + + // Now upload a new extra config and check that it gets uploaded to Mimir + testAlertmanagerConfigYAML := ` +route: + group_by: ['alertname'] + group_wait: 10s + group_interval: 10s + repeat_interval: 1h + receiver: extra-slack + +receivers: +- name: extra-slack + slack_configs: + - api_url: 'http://localhost/slack' + channel: '#alerts' + title: 'Alerts' +` + + headers := map[string]string{ + "Content-Type": "application/yaml", + "X-Grafana-Alerting-Config-Identifier": "external-system", + "X-Grafana-Alerting-Merge-Matchers": "environment=production,team=backend", + } + + amConfig := apimodels.AlertmanagerUserConfig{ + AlertmanagerConfig: testAlertmanagerConfigYAML, + TemplateFiles: map[string]string{ + "test.tmpl": `{{ define "test.template" }}Test template for remote sync{{ end }}`, + }, + } + + // Post the configuration to Grafana + response := apiClient.ConvertPrometheusPostAlertmanagerConfig(t, amConfig, headers) + require.Equal(t, "success", response.Status) + + _, status, _ := apiClient.GetAlertmanagerConfigWithStatus(t) + require.Equal(t, http.StatusOK, status) + + // Check that the configuration was successfully sent to Mimir and contains the new receiver + finalMimirConfig, err := mimirClient.GetGrafanaAlertmanagerConfig(context.Background()) + require.NoError(t, err) + require.NotNil(t, finalMimirConfig) + require.NotNil(t, finalMimirConfig.GrafanaAlertmanagerConfig) + + receivers = finalMimirConfig.GrafanaAlertmanagerConfig.AlertmanagerConfig.Receivers + require.Len(t, receivers, 2) + + var foundDefault, foundExtraSlack bool + for _, receiver := range receivers { + switch receiver.Name { + case "grafana-default-email": + foundDefault = true + require.Len(t, receiver.GrafanaManagedReceivers, 1) + require.Equal(t, "email receiver", receiver.GrafanaManagedReceivers[0].Name) + require.Equal(t, "email", receiver.GrafanaManagedReceivers[0].Type) + case "extra-slack": + foundExtraSlack = true + require.Len(t, receiver.SlackConfigs, 1) + require.NotNil(t, receiver.SlackConfigs[0].APIURL) + require.Equal(t, "#alerts", receiver.SlackConfigs[0].Channel) + } + } + require.True(t, foundDefault, "Default receiver not found") + require.True(t, foundExtraSlack, "Extra slack receiver not found") +} + +// TestIntegrationRemoteAlertmanagerHistoricalConfigActivation tests that when we activate +// a historical alertmanager configuration with extra configs, it gets properly decrypted +// and uploaded to the remote Mimir. +func TestIntegrationRemoteAlertmanagerHistoricalConfigActivation(t *testing.T) { + testinfra.SQLiteIntegrationTest(t) + + s, err := alertmanager.NewAlertmanagerScenario() + require.NoError(t, err) + defer s.Close() + + s.Mimir = alertmanager.NewMimirService("mimir") + require.NoError(t, s.StartAndWaitReady(s.Mimir)) + + mimirEndpoint := "http://" + s.Mimir.HTTPEndpoint() + + dir, gpath := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ + DisableLegacyAlerting: true, + EnableUnifiedAlerting: true, + DisableAnonymous: true, + AppModeProduction: true, + EnableFeatureToggles: []string{ + "alertmanagerRemotePrimary", + "alertingImportAlertmanagerAPI", + }, + RemoteAlertmanagerURL: mimirEndpoint, + }) + + grafanaListedAddr, _ := testinfra.StartGrafanaEnv(t, dir, gpath) + + apiClient := newAlertingApiClient(grafanaListedAddr, "admin", "admin") + mimirClient, err := alertmanager.NewMimirClient(mimirEndpoint, "1") + require.NoError(t, err) + + require.Eventually(t, func() bool { + _, status, _ := apiClient.GetAlertmanagerConfigWithStatus(t) + return status == http.StatusOK + }, 30*time.Second, time.Second, "Grafana failed to start") + + // Upload configuration with extra configs + testAlertmanagerConfigYAML := ` +route: + group_by: ['alertname'] + group_wait: 10s + group_interval: 10s + repeat_interval: 1h + receiver: old-slack + +receivers: +- name: old-slack + slack_configs: + - api_url: 'http://localhost/slack' + channel: '#alerts' +` + + headers := map[string]string{ + "Content-Type": "application/yaml", + "X-Grafana-Alerting-Config-Identifier": "historical-system", + "X-Grafana-Alerting-Merge-Matchers": "environment=test,team=platform", + } + + amConfig := apimodels.AlertmanagerUserConfig{ + AlertmanagerConfig: testAlertmanagerConfigYAML, + TemplateFiles: map[string]string{ + "historical.tmpl": `{{ define "historical.template" }}Historical template{{ end }}`, + }, + } + + response := apiClient.ConvertPrometheusPostAlertmanagerConfig(t, amConfig, headers) + require.Equal(t, "success", response.Status) + + // Get the configuration history to find the most recent config + historyResponse := getAlertmanagerConfigHistory(t, apiClient) + require.NotEmpty(t, historyResponse) + + var mostRecentID int64 + for _, entry := range historyResponse { + if entry.ID > mostRecentID { + mostRecentID = entry.ID + } + } + require.Greater(t, mostRecentID, int64(0), "Should have found a historical configuration") + + // Activate the historical configuration + activateHistoricalConfiguration(t, apiClient, mostRecentID) + + // Verify the configuration + finalMimirConfig, err := mimirClient.GetGrafanaAlertmanagerConfig(context.Background()) + require.NoError(t, err) + require.NotNil(t, finalMimirConfig) + require.NotNil(t, finalMimirConfig.GrafanaAlertmanagerConfig) + + receivers := finalMimirConfig.GrafanaAlertmanagerConfig.AlertmanagerConfig.Receivers + require.Len(t, receivers, 2) + + found := false + for _, receiver := range receivers { + if receiver.Name == "old-slack" { + found = true + require.Len(t, receiver.SlackConfigs, 1) + break + } + } + require.True(t, found) +} + +func getAlertmanagerConfigHistory(t *testing.T, client apiClient) []apimodels.GettableHistoricUserConfig { + t.Helper() + u, err := url.Parse(fmt.Sprintf("%s/api/alertmanager/grafana/config/history", client.url)) + require.NoError(t, err) + + req, err := http.NewRequest(http.MethodGet, u.String(), nil) + require.NoError(t, err) + + history, _, _ := sendRequestJSON[[]apimodels.GettableHistoricUserConfig](t, req, http.StatusOK) + return history +} + +func activateHistoricalConfiguration(t *testing.T, client apiClient, configID int64) { + t.Helper() + u, err := url.Parse(fmt.Sprintf("%s/api/alertmanager/grafana/config/history/%d/_activate", client.url, configID)) + require.NoError(t, err) + + req, err := http.NewRequest(http.MethodPost, u.String(), nil) + require.NoError(t, err) + + response, statusCode, body := sendRequestJSON[map[string]string](t, req, http.StatusAccepted) + if statusCode != http.StatusAccepted { + t.Fatalf("Expected status code %d but got %d. Response body: %s", http.StatusAccepted, statusCode, body) + } + require.Equal(t, "configuration activated", response["message"]) +} diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index 5310a1813ad..74aba425b91 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -477,6 +477,17 @@ func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { _, err = grafanaComSection.NewKey("api_url", opts.GrafanaComAPIURL) require.NoError(t, err) } + + if opts.RemoteAlertmanagerURL != "" { + remoteAlertmanagerSection, err := getOrCreateSection("remote.alertmanager") + require.NoError(t, err) + _, err = remoteAlertmanagerSection.NewKey("enabled", "true") + require.NoError(t, err) + _, err = remoteAlertmanagerSection.NewKey("url", opts.RemoteAlertmanagerURL) + require.NoError(t, err) + _, err = remoteAlertmanagerSection.NewKey("tenant", "1") + require.NoError(t, err) + } if opts.GrafanaComSSOAPIToken != "" { grafanaComSection, err := getOrCreateSection("grafana_com") require.NoError(t, err) @@ -571,6 +582,9 @@ type GrafanaOpts struct { // When "unified-grpc" is selected it will also start the grpc server APIServerStorageType options.StorageType + + // Remote alertmanager configuration + RemoteAlertmanagerURL string } func CreateUser(t *testing.T, store db.DB, cfg *setting.Cfg, cmd user.CreateUserCommand) *user.User { From 61efc8b60994e315fd6ae023b55c02f149a71421 Mon Sep 17 00:00:00 2001 From: Pepe Cano <825430+ppcano@users.noreply.github.com> Date: Wed, 2 Jul 2025 21:46:29 +0200 Subject: [PATCH 4/5] docs(alerting): clarify usage of different Alertmanagers and fix misleading details (#107498) * docs(alerting): clarify usage of different Alertmanagers and fix misleading details * address review changes --- docs/sources/alerting/set-up/configure-alertmanager/_index.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/sources/alerting/set-up/configure-alertmanager/_index.md b/docs/sources/alerting/set-up/configure-alertmanager/_index.md index 873d9a5ffc7..0702c7dfa5f 100644 --- a/docs/sources/alerting/set-up/configure-alertmanager/_index.md +++ b/docs/sources/alerting/set-up/configure-alertmanager/_index.md @@ -83,8 +83,6 @@ Grafana can be configured to handle alert notifications using various Alertmanag The Cloud Alertmanager is available exclusively in Grafana Cloud and can handle both Grafana-managed and data source-managed alerts. - Some Grafana Cloud services, such as **Kubernetes Monitoring** and **Synthetic Monitoring** use the Cloud Alertmanager to create and manage alerts. - - **Other Alertmanagers**: Grafana Alerting also supports sending alerts to other Alertmanagers, such as the [Prometheus Alertmanager](https://prometheus.io/docs/alerting/latest/alertmanager/), which can handle both Grafana-managed and data source-managed alerts. Grafana Alerting supports using a combination of Alertmanagers and can [enable other Alertmanagers to receive Grafana-managed alerts](#enable-an-alertmanager-to-receive-grafana-managed-alerts). The decision often depends on your alerting setup and where your alerts are generated. From b6eacc929a41c01f91a90a26a5a18754b3701958 Mon Sep 17 00:00:00 2001 From: colin-stuart Date: Wed, 2 Jul 2025 15:23:06 -0500 Subject: [PATCH 5/5] Auth: Check SCIM dynamic settings when syncing users (#107487) * Auth: Check SCIM dynamic settings when syncing users * remove enterprise package import * add CODEOWNERS * lint * use default namespace & update tests * add optional dynamic AllowNonProvisionedUsers setting * update test case with allowNonProvisionedUsers --- .github/CODEOWNERS | 1 + pkg/services/authn/authnimpl/registration.go | 3 +- .../authn/authnimpl/sync/user_sync.go | 36 +- .../authn/authnimpl/sync/user_sync_test.go | 349 ++++++++- pkg/services/scimutil/README.md | 181 +++++ pkg/services/scimutil/scim_util.go | 144 ++++ pkg/services/scimutil/scim_util_test.go | 704 ++++++++++++++++++ 7 files changed, 1405 insertions(+), 13 deletions(-) create mode 100644 pkg/services/scimutil/README.md create mode 100644 pkg/services/scimutil/scim_util.go create mode 100644 pkg/services/scimutil/scim_util_test.go diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 232d737f538..3e78adb40cb 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -710,6 +710,7 @@ playwright.config.ts @grafana/plugins-platform-frontend /pkg/services/anonymous/ @grafana/identity-access-team /pkg/services/auth/ @grafana/identity-squad /pkg/services/authn/ @grafana/identity-squad +/pkg/services/scimutil/ @grafana/identity-squad /pkg/services/authz/ @grafana/access-squad /pkg/services/signingkeys/ @grafana/identity-squad /pkg/services/dashboards/accesscontrol.go @grafana/access-squad diff --git a/pkg/services/authn/authnimpl/registration.go b/pkg/services/authn/authnimpl/registration.go index 7231889c72c..e8fa578ad39 100644 --- a/pkg/services/authn/authnimpl/registration.go +++ b/pkg/services/authn/authnimpl/registration.go @@ -131,7 +131,8 @@ func ProvideRegistration( } // FIXME (jguer): move to User package - userSync := sync.ProvideUserSync(userService, userProtectionService, authInfoService, quotaService, tracer, features, cfg) + // Pass nil for k8sClient - it will be handled gracefully in the SCIMSettingsUtil + userSync := sync.ProvideUserSync(userService, userProtectionService, authInfoService, quotaService, tracer, features, cfg, nil) orgSync := sync.ProvideOrgSync(userService, orgService, accessControlService, cfg, tracer) authnSvc.RegisterPostAuthHook(userSync.SyncUserHook, 10) authnSvc.RegisterPostAuthHook(userSync.EnableUserHook, 20) diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index ebbb6500206..66879861146 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/pkg/services/authn/authnimpl/sync/user_sync.go @@ -13,11 +13,13 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/apiserver/client" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/quota" + "github.com/grafana/grafana/pkg/services/scimutil" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" ) @@ -79,13 +81,25 @@ var ( errSignupNotAllowed = errors.New("system administrator has disabled signup") ) +// StaticSCIMConfig represents the static SCIM configuration from config.ini +type StaticSCIMConfig struct { + AllowNonProvisionedUsers bool + IsUserProvisioningEnabled bool +} + func ProvideUserSync(userService user.Service, userProtectionService login.UserProtectionService, authInfoService login.AuthInfoService, quotaService quota.Service, tracer tracing.Tracer, features featuremgmt.FeatureToggles, cfg *setting.Cfg, + k8sClient client.K8sHandler, ) *UserSync { scimSection := cfg.Raw.Section("auth.scim") + staticConfig := &StaticSCIMConfig{ + AllowNonProvisionedUsers: scimSection.Key("allow_non_provisioned_users").MustBool(false), + IsUserProvisioningEnabled: scimSection.Key("user_sync_enabled").MustBool(false), + } + return &UserSync{ - allowNonProvisionedUsers: scimSection.Key("allow_non_provisioned_users").MustBool(false), - isUserProvisioningEnabled: scimSection.Key("user_sync_enabled").MustBool(false), + allowNonProvisionedUsers: staticConfig.AllowNonProvisionedUsers, + isUserProvisioningEnabled: staticConfig.IsUserProvisioningEnabled, userService: userService, authInfoService: authInfoService, userProtectionService: userProtectionService, @@ -94,6 +108,8 @@ func ProvideUserSync(userService user.Service, userProtectionService login.UserP tracer: tracer, features: features, lastSeenSF: &singleflight.Group{}, + scimUtil: scimutil.NewSCIMUtil(k8sClient), + staticConfig: staticConfig, } } @@ -108,6 +124,8 @@ type UserSync struct { tracer tracing.Tracer features featuremgmt.FeatureToggles lastSeenSF *singleflight.Group + scimUtil *scimutil.SCIMUtil + staticConfig *StaticSCIMConfig } // ValidateUserProvisioningHook validates if a user should be allowed access based on provisioning status and configuration @@ -163,12 +181,22 @@ func (s *UserSync) ValidateUserProvisioningHook(ctx context.Context, currentIden func (s *UserSync) skipProvisioningValidation(ctx context.Context, currentIdentity *authn.Identity) bool { log := s.log.FromContext(ctx).New("auth_module", currentIdentity.AuthenticatedBy, "auth_id", currentIdentity.AuthID, "id", currentIdentity.ID) - if !s.isUserProvisioningEnabled { + // Use dynamic SCIM settings if available, otherwise fall back to static config + effectiveUserSyncEnabled := s.isUserProvisioningEnabled + effectiveAllowNonProvisionedUsers := s.allowNonProvisionedUsers + + if s.scimUtil != nil { + orgID := currentIdentity.GetOrgID() + effectiveUserSyncEnabled = s.scimUtil.IsUserSyncEnabled(ctx, orgID, s.staticConfig.IsUserProvisioningEnabled) + effectiveAllowNonProvisionedUsers = s.scimUtil.AreNonProvisionedUsersAllowed(ctx, orgID, s.staticConfig.AllowNonProvisionedUsers) + } + + if !effectiveUserSyncEnabled { log.Debug("User provisioning is disabled, skipping validation") return true } - if s.allowNonProvisionedUsers { + if effectiveAllowNonProvisionedUsers { log.Debug("Non-provisioned users are allowed, skipping validation") return true } diff --git a/pkg/services/authn/authnimpl/sync/user_sync_test.go b/pkg/services/authn/authnimpl/sync/user_sync_test.go index 74f5e2121b9..73bb14d6bc4 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/user_sync_test.go @@ -21,9 +21,13 @@ import ( "github.com/grafana/grafana/pkg/services/login/authinfotest" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/quota/quotatest" + "github.com/grafana/grafana/pkg/services/scimutil" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/usertest" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/storage/unified/resourcepb" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) func ptrString(s string) *string { @@ -39,10 +43,10 @@ func TestUserSync_SyncUserHook(t *testing.T) { authFakeNil := &authinfotest.FakeService{ ExpectedError: user.ErrUserNotFound, - SetAuthInfoFn: func(ctx context.Context, cmd *login.SetAuthInfoCommand) error { + SetAuthInfoFn: func(_ context.Context, _ *login.SetAuthInfoCommand) error { return nil }, - UpdateAuthInfoFn: func(ctx context.Context, cmd *login.UpdateAuthInfoCommand) error { + UpdateAuthInfoFn: func(_ context.Context, _ *login.UpdateAuthInfoCommand) error { return nil }, } @@ -87,7 +91,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { userServiceNil := &usertest.FakeUserService{ ExpectedError: user.ErrUserNotFound, - CreateFn: func(ctx context.Context, cmd *user.CreateUserCommand) (*user.User, error) { + CreateFn: func(_ context.Context, cmd *user.CreateUserCommand) (*user.User, error) { return &user.User{ ID: 2, UID: "2", @@ -103,7 +107,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { // mockUpdateFn helps assert the UpdateUserCommand contents. // expectNoUpdateForOtherAttributes is true for SCIM users where only IsGrafanaAdmin should sync from SAML. mockUpdateFn := func(t *testing.T, expectedCmd *user.UpdateUserCommand, expectNoUpdateForOtherAttributes bool, originalUserEmail string) func(context.Context, *user.UpdateUserCommand) error { - return func(ctx context.Context, cmd *user.UpdateUserCommand) error { + return func(_ context.Context, cmd *user.UpdateUserCommand) error { if expectedCmd == nil { t.Errorf("userService.Update was called unexpectedly") return nil @@ -183,8 +187,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { ExternalUID: externalUID, UserId: userID, }, - SetAuthInfoFn: func(ctx context.Context, cmd *login.SetAuthInfoCommand) error { return nil }, - UpdateAuthInfoFn: func(ctx context.Context, cmd *login.UpdateAuthInfoCommand) error { return nil }, + SetAuthInfoFn: func(_ context.Context, _ *login.SetAuthInfoCommand) error { return nil }, + UpdateAuthInfoFn: func(_ context.Context, _ *login.UpdateAuthInfoCommand) error { return nil }, } } @@ -895,7 +899,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := ProvideUserSync(tt.fields.userService, userProtection, tt.fields.authInfoService, tt.fields.quotaService, tracing.InitializeTracerForTest(), featuremgmt.WithFeatures(), setting.NewCfg()) + s := ProvideUserSync(tt.fields.userService, userProtection, tt.fields.authInfoService, tt.fields.quotaService, tracing.InitializeTracerForTest(), featuremgmt.WithFeatures(), setting.NewCfg(), nil) err := s.SyncUserHook(tt.args.ctx, tt.args.id, nil) if tt.wantErr { require.Error(t, err) @@ -922,6 +926,7 @@ func TestUserSync_SyncUserRetryFetch(t *testing.T) { tracing.NewNoopTracerService(), featuremgmt.WithFeatures(), setting.NewCfg(), + nil, ) email := "test@test.com" @@ -1014,7 +1019,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) { t.Run(tt.desc, func(t *testing.T) { userSvc := usertest.NewUserServiceFake() called := false - userSvc.UpdateFn = func(ctx context.Context, cmd *user.UpdateUserCommand) error { + userSvc.UpdateFn = func(_ context.Context, _ *user.UpdateUserCommand) error { called = true return nil } @@ -1251,6 +1256,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) { SyncUser: true, }, }, + expectedErr: nil, }, { desc: "it should failed to validate a non provisioned user when retrieved from the database", @@ -1412,3 +1418,330 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) { }) } } + +func TestUserSync_SCIMUtilIntegration(t *testing.T) { + ctx := context.Background() + orgID := int64(1) + + // Mock SCIM utility for testing + type mockSCIMUtil struct { + userSyncEnabled bool + nonProvisionedUsersAllowed bool + shouldUseDynamicConfig bool + shouldReturnError bool + } + + createMockSCIMUtil := func(mockCfg *mockSCIMUtil) *scimutil.SCIMUtil { + if mockCfg == nil { + return nil + } + + // Create a mock K8s client that returns the expected behavior + mockK8sClient := &MockK8sHandler{} + + if mockCfg.shouldReturnError { + mockK8sClient.On("Get", ctx, "default", orgID, mock.AnythingOfType("v1.GetOptions"), mock.Anything). + Return(nil, errors.New("k8s error")) + } else if mockCfg.shouldUseDynamicConfig { + // Create a mock SCIM config with the desired settings + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "scim.grafana.com/v0alpha1", + "kind": "SCIMConfig", + "metadata": map[string]interface{}{ + "name": "test-config", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "enableUserSync": mockCfg.userSyncEnabled, + "enableGroupSync": false, // Not used for this test + "allowNonProvisionedUsers": mockCfg.nonProvisionedUsersAllowed, + }, + }, + } + mockK8sClient.On("Get", ctx, "default", orgID, mock.AnythingOfType("v1.GetOptions"), mock.Anything). + Return(obj, nil) + } + + return scimutil.NewSCIMUtil(mockK8sClient) + } + + tests := []struct { + name string + identity *authn.Identity + staticConfig *StaticSCIMConfig + mockSCIMUtil *mockSCIMUtil + expectedUserSyncEnabled bool + expectedNonProvisionedAllowed bool + expectedError error + }{ + { + name: "SCIM util nil - uses static config", + identity: &authn.Identity{ + OrgID: orgID, + ID: "test-user", + }, + staticConfig: &StaticSCIMConfig{ + IsUserProvisioningEnabled: true, + AllowNonProvisionedUsers: false, + }, + mockSCIMUtil: nil, // No SCIM util + expectedUserSyncEnabled: true, + expectedNonProvisionedAllowed: false, + }, + { + name: "SCIM util with dynamic config - user sync enabled", + identity: &authn.Identity{ + OrgID: orgID, + ID: "test-user", + }, + staticConfig: &StaticSCIMConfig{ + IsUserProvisioningEnabled: false, // Static disabled + AllowNonProvisionedUsers: false, + }, + mockSCIMUtil: &mockSCIMUtil{ + userSyncEnabled: true, // Dynamic enabled + nonProvisionedUsersAllowed: true, + shouldUseDynamicConfig: true, + }, + expectedUserSyncEnabled: true, + expectedNonProvisionedAllowed: true, + }, + { + name: "SCIM util with dynamic config - user sync disabled", + identity: &authn.Identity{ + OrgID: orgID, + ID: "test-user", + }, + staticConfig: &StaticSCIMConfig{ + IsUserProvisioningEnabled: true, // Static enabled + AllowNonProvisionedUsers: true, + }, + mockSCIMUtil: &mockSCIMUtil{ + userSyncEnabled: false, // Dynamic disabled + nonProvisionedUsersAllowed: false, + shouldUseDynamicConfig: true, + }, + expectedUserSyncEnabled: false, + expectedNonProvisionedAllowed: false, + }, + { + name: "SCIM util with error - falls back to static config", + identity: &authn.Identity{ + OrgID: orgID, + ID: "test-user", + }, + staticConfig: &StaticSCIMConfig{ + IsUserProvisioningEnabled: true, + AllowNonProvisionedUsers: false, + }, + mockSCIMUtil: &mockSCIMUtil{ + shouldReturnError: true, + }, + expectedUserSyncEnabled: true, + expectedNonProvisionedAllowed: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create UserSync service with mock SCIM util + userSync := &UserSync{ + scimUtil: createMockSCIMUtil(tt.mockSCIMUtil), + } + + // Test user sync enabled check + var userSyncEnabled bool + if userSync.scimUtil != nil { + userSyncEnabled = userSync.scimUtil.IsUserSyncEnabled(ctx, orgID, tt.staticConfig.IsUserProvisioningEnabled) + } else { + userSyncEnabled = tt.staticConfig.IsUserProvisioningEnabled + } + assert.Equal(t, tt.expectedUserSyncEnabled, userSyncEnabled, "User sync enabled mismatch") + + // Test non-provisioned users allowed check + var nonProvisionedAllowed bool + if userSync.scimUtil != nil { + nonProvisionedAllowed = userSync.scimUtil.AreNonProvisionedUsersAllowed(ctx, orgID, tt.staticConfig.AllowNonProvisionedUsers) + } else { + nonProvisionedAllowed = tt.staticConfig.AllowNonProvisionedUsers + } + assert.Equal(t, tt.expectedNonProvisionedAllowed, nonProvisionedAllowed, "Non-provisioned users allowed mismatch") + }) + } +} + +// MockK8sHandler is a mock implementation for testing +type MockK8sHandler struct { + mock.Mock +} + +func (m *MockK8sHandler) GetNamespace(orgID int64) string { + args := m.Called(orgID) + return args.String(0) +} + +func (m *MockK8sHandler) Get(ctx context.Context, name string, orgID int64, opts metav1.GetOptions, subresource ...string) (*unstructured.Unstructured, error) { + args := m.Called(ctx, name, orgID, opts, subresource) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*unstructured.Unstructured), args.Error(1) +} + +// Add other required methods with empty implementations for the mock +func (m *MockK8sHandler) Create(ctx context.Context, obj *unstructured.Unstructured, orgID int64, opts metav1.CreateOptions) (*unstructured.Unstructured, error) { + args := m.Called(ctx, obj, orgID, opts) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*unstructured.Unstructured), args.Error(1) +} + +func (m *MockK8sHandler) Update(ctx context.Context, obj *unstructured.Unstructured, orgID int64, opts metav1.UpdateOptions) (*unstructured.Unstructured, error) { + args := m.Called(ctx, obj, orgID, opts) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*unstructured.Unstructured), args.Error(1) +} + +func (m *MockK8sHandler) Delete(ctx context.Context, name string, orgID int64, options metav1.DeleteOptions) error { + args := m.Called(ctx, name, orgID, options) + return args.Error(0) +} + +func (m *MockK8sHandler) DeleteCollection(ctx context.Context, orgID int64) error { + args := m.Called(ctx, orgID) + return args.Error(0) +} + +func (m *MockK8sHandler) List(ctx context.Context, orgID int64, options metav1.ListOptions) (*unstructured.UnstructuredList, error) { + args := m.Called(ctx, orgID, options) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*unstructured.UnstructuredList), args.Error(1) +} + +func (m *MockK8sHandler) Search(ctx context.Context, orgID int64, in *resourcepb.ResourceSearchRequest) (*resourcepb.ResourceSearchResponse, error) { + args := m.Called(ctx, orgID, in) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*resourcepb.ResourceSearchResponse), args.Error(1) +} + +func (m *MockK8sHandler) GetStats(ctx context.Context, orgID int64) (*resourcepb.ResourceStatsResponse, error) { + args := m.Called(ctx, orgID) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*resourcepb.ResourceStatsResponse), args.Error(1) +} + +func (m *MockK8sHandler) GetUsersFromMeta(ctx context.Context, userMeta []string) (map[string]*user.User, error) { + args := m.Called(ctx, userMeta) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(map[string]*user.User), args.Error(1) +} + +func TestUserSync_NamespaceMappingLogic(t *testing.T) { + ctx := context.Background() + + // Test the actual namespace mapping logic + tests := []struct { + name string + stackID string + orgID int64 + expectedNamespace string + description string + }{ + { + name: "Cloud instance with valid stackID", + stackID: "75", + orgID: 123, + expectedNamespace: "stacks-75", + description: "Should use stack-based namespace for cloud instances", + }, + { + name: "Cloud instance with different stackID", + stackID: "99", + orgID: 123, + expectedNamespace: "stacks-99", + description: "Should use different stack-based namespace for different stackID", + }, + { + name: "Cloud instance with invalid stackID", + stackID: "invalid", + orgID: 456, + expectedNamespace: "stacks-0", + description: "Should fallback to stacks-0 for invalid stackID", + }, + { + name: "On-prem instance (no stackID)", + stackID: "", + orgID: 456, + expectedNamespace: "org-456", + description: "Should use org-based namespace for on-prem instances", + }, + { + name: "On-prem instance with different orgID", + stackID: "", + orgID: 789, + expectedNamespace: "org-789", + description: "Should use correct orgID in namespace", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock K8s client + mockK8sClient := &MockK8sHandler{} + + // Mock the GetNamespace method to simulate the actual namespace mapping logic + mockK8sClient.On("GetNamespace", tt.orgID).Return(tt.expectedNamespace) + + // Set up a successful SCIM config response + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "scim.grafana.com/v0alpha1", + "kind": "SCIMConfig", + "metadata": map[string]interface{}{ + "name": "default", + "namespace": tt.expectedNamespace, + }, + "spec": map[string]interface{}{ + "enableUserSync": true, + "enableGroupSync": false, + }, + }, + } + mockK8sClient.On("Get", ctx, "default", tt.orgID, mock.AnythingOfType("v1.GetOptions"), mock.Anything). + Return(obj, nil) + + // Create SCIM util with the mock client + scimUtil := scimutil.NewSCIMUtil(mockK8sClient) + + // Test the namespace mapping + actualNamespace := mockK8sClient.GetNamespace(tt.orgID) + assert.Equal(t, tt.expectedNamespace, actualNamespace, + "Namespace mapping failed: %s", tt.description) + + // Test that the SCIM util works with the mapped namespace + userSyncEnabled := scimUtil.IsUserSyncEnabled(ctx, tt.orgID, false) + assert.True(t, userSyncEnabled, + "SCIM util should work with namespace %s: %s", tt.expectedNamespace, tt.description) + + // Verify that the correct API path would be constructed + // This is implicit in the mock setup, but we can verify the components + assert.Equal(t, "default", obj.GetName(), "Resource name should be 'default'") + assert.Equal(t, tt.expectedNamespace, obj.GetNamespace(), "Namespace should match expected") + + // Verify the mock expectations + mockK8sClient.AssertExpectations(t) + }) + } +} diff --git a/pkg/services/scimutil/README.md b/pkg/services/scimutil/README.md new file mode 100644 index 00000000000..939d42789c2 --- /dev/null +++ b/pkg/services/scimutil/README.md @@ -0,0 +1,181 @@ +# SCIM Utility + +This package provides utility functions for checking SCIM dynamic app platform settings using the `client.K8sHandler`. It allows both the `authimpl` and `saml` packages to check SCIM settings with dynamic configuration support and static fallback. + +## API Reference + +### SCIMUtil + +The main utility struct that provides methods for checking SCIM settings. + +```go +type SCIMUtil struct { + k8sClient client.K8sHandler + logger log.Logger +} +``` + +### Methods + +#### NewSCIMUtil +Creates a new SCIMUtil instance. + +```go +func NewSCIMUtil(k8sClient client.K8sHandler) *SCIMUtil +``` + +#### IsUserSyncEnabled +Checks if SCIM user sync is enabled using dynamic configuration with static fallback. + +```go +func (s *SCIMUtil) IsUserSyncEnabled(ctx context.Context, orgID int64, staticEnabled bool) bool +``` + +#### AreNonProvisionedUsersAllowed +Checks if non-provisioned users are allowed using dynamic configuration with static fallback. + +```go +func (s *SCIMUtil) AreNonProvisionedUsersAllowed(ctx context.Context, orgID int64, staticAllowed bool) bool +``` + +**Note**: This field defaults to `false` when not present in the dynamic configuration. + +## Usage + +### Basic Usage + +```go +import ( + "context" + "github.com/grafana/grafana/pkg/services/apiserver/client" + "github.com/grafana/grafana/pkg/services/scimutil" +) + +// Create a new SCIM utility instance +scimUtil := scimutil.NewSCIMUtil(k8sClient) + +// Check if user sync is enabled (with dynamic config support) +userSyncEnabled := scimUtil.IsUserSyncEnabled(ctx, orgID, staticConfig.IsUserProvisioningEnabled) + +// Check if non-provisioned users are allowed (with dynamic config support) +nonProvisionedAllowed := scimUtil.AreNonProvisionedUsersAllowed(ctx, orgID, staticConfig.AllowNonProvisionedUsers) +``` + +### In authimpl Package + +The `authimpl` package uses this utility in the `UserSync` struct to check SCIM settings during user provisioning validation: + +```go +// In user_sync.go +type UserSync struct { + // ... other fields ... + scimUtil *scim_util.SCIMUtil + staticConfig *StaticSCIMConfig +} + +func (s *UserSync) skipProvisioningValidation(ctx context.Context, currentIdentity *authn.Identity) bool { + // Use dynamic SCIM settings if available, otherwise fall back to static config + effectiveUserSyncEnabled := s.isUserProvisioningEnabled + effectiveAllowNonProvisionedUsers := s.allowNonProvisionedUsers + + if s.scimUtil != nil { + orgID := currentIdentity.GetOrgID() + effectiveUserSyncEnabled = s.scimUtil.IsUserSyncEnabled(ctx, orgID, s.staticConfig.IsUserProvisioningEnabled) + effectiveAllowNonProvisionedUsers = s.scimUtil.AreNonProvisionedUsersAllowed(ctx, orgID, s.staticConfig.AllowNonProvisionedUsers) + } + + // ... rest of validation logic ... +} +``` + +### In SAML Package + +The SAML package can use this utility to check SCIM settings during authentication: + +```go +// In saml package +type SCIMHelper struct { + scimUtil *scim_util.SCIMUtil +} + +func (h *SCIMHelper) CheckUserSyncEnabled(ctx context.Context, orgID int64, staticEnabled bool) bool { + if h.scimUtil == nil { + return staticEnabled + } + return h.scimUtil.IsUserSyncEnabled(ctx, orgID, staticEnabled) +} +``` + +## Dynamic Configuration + +The utility supports dynamic SCIM configuration through the Kubernetes API. It will: + +1. First attempt to fetch SCIM settings from the dynamic configuration (SCIMConfig resource) +2. If dynamic configuration is not available or fails, fall back to static configuration from `config.ini` +3. Log the source of configuration being used for debugging + +### Configuration Sources + +- **Dynamic**: SCIMConfig resource in Kubernetes (org-specific) + - Resource name: `default` + - API Group: `scim.grafana.com/v0alpha1` + - Kind: `SCIMConfig` +- **Static**: `auth.scim` section in `config.ini` (global) + +### SCIMConfig Resource Structure + +```yaml +apiVersion: scim.grafana.com/v0alpha1 +kind: SCIMConfig +metadata: + name: default + namespace: +spec: + enableUserSync: true # Controls user provisioning + enableGroupSync: false # Controls group/team provisioning + allowNonProvisionedUsers: false # Controls whether non-provisioned users are allowed (optional) +``` + +## Error Handling + +The utility gracefully handles errors and falls back to static configuration when: +- K8s client is not configured +- SCIMConfig resource is not found +- Network errors occur +- Invalid configuration is encountered +- Missing or malformed spec in SCIMConfig resource + +All errors are logged for debugging purposes with appropriate log levels: +- `Debug`: Normal operation messages +- `Warn`: Fallback scenarios and non-critical errors +- `Error`: Invalid configuration or unexpected errors + +## Implementation Details + +This package is designed to work with the open-source Grafana build and does not depend on enterprise-only SCIM API types. It uses a simplified `SCIMConfigSpec` struct that contains only the essential configuration fields: + +```go +type SCIMConfigSpec struct { + EnableUserSync bool `json:"enableUserSync"` + EnableGroupSync bool `json:"enableGroupSync"` + AllowNonProvisionedUsers *bool `json:"allowNonProvisionedUsers,omitempty"` +} +``` + +The `AllowNonProvisionedUsers` field is optional and defaults to `false` when not present in the configuration. + +The utility directly works with Kubernetes unstructured objects and extracts the configuration values without requiring the full SCIM API types. + +## Testing + +The package includes comprehensive tests covering: +- All combinations of user sync, group sync, and non-provisioned users settings +- Error scenarios and fallback behavior +- Integration scenarios with both dynamic and static configurations +- Mock implementations for the K8s client interface +- Optional field handling for `allowNonProvisionedUsers` + +Run tests with: +```bash +go test ./pkg/services/scimutil +``` \ No newline at end of file diff --git a/pkg/services/scimutil/scim_util.go b/pkg/services/scimutil/scim_util.go new file mode 100644 index 00000000000..022277ba994 --- /dev/null +++ b/pkg/services/scimutil/scim_util.go @@ -0,0 +1,144 @@ +package scimutil + +import ( + "context" + "errors" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/apiserver/client" +) + +// SCIMUtil provides utility functions for checking SCIM dynamic app platform settings +type SCIMUtil struct { + k8sClient client.K8sHandler + logger log.Logger +} + +// NewSCIMUtil creates a new SCIMUtil instance +func NewSCIMUtil(k8sClient client.K8sHandler) *SCIMUtil { + return &SCIMUtil{ + k8sClient: k8sClient, + logger: log.New("scim.util"), + } +} + +// IsUserSyncEnabled checks if SCIM user sync is enabled using dynamic configuration with static fallback +func (s *SCIMUtil) IsUserSyncEnabled(ctx context.Context, orgID int64, staticEnabled bool) bool { + if s.k8sClient == nil { + s.logger.Debug("K8s client not configured, using static SCIM config for user sync") + return staticEnabled + } + + dynamicEnabled, dynamicConfigFetched := s.fetchDynamicSCIMSetting(ctx, orgID, "user") + + if dynamicConfigFetched { + s.logger.Debug("Using dynamic SCIM config for user sync", "orgID", orgID, "enabled", dynamicEnabled) + return dynamicEnabled + } + + // Fallback to static config if dynamic config wasn't fetched successfully + s.logger.Debug("Using static SCIM config for user sync", "orgID", orgID, "enabled", staticEnabled) + return staticEnabled +} + +// AreNonProvisionedUsersAllowed checks if non-provisioned users are allowed using dynamic configuration with static fallback +func (s *SCIMUtil) AreNonProvisionedUsersAllowed(ctx context.Context, orgID int64, staticAllowed bool) bool { + if s.k8sClient == nil { + s.logger.Debug("K8s client not configured, using static SCIM config for non-provisioned users") + return staticAllowed + } + + dynamicAllowed, dynamicConfigFetched := s.fetchDynamicSCIMSetting(ctx, orgID, "allowNonProvisionedUsers") + + if dynamicConfigFetched { + s.logger.Debug("Using dynamic SCIM config for user sync", "orgID", orgID, "enabled", dynamicAllowed) + return dynamicAllowed + } + + // Fallback to static config if dynamic config wasn't fetched successfully + s.logger.Debug("Using static SCIM config for user sync", "orgID", orgID, "enabled", staticAllowed) + return staticAllowed +} + +// fetchDynamicSCIMSetting attempts to retrieve a specific dynamic SCIM configuration setting +func (s *SCIMUtil) fetchDynamicSCIMSetting(ctx context.Context, orgID int64, settingType string) (settingEnabled bool, dynamicConfigFetched bool) { + if s.k8sClient == nil { + s.logger.Warn("K8s client not configured, dynamic SCIM config lookup skipped", "orgID", orgID, "settingType", settingType) + return false, false + } + + scimConfig, err := s.getOrgSCIMConfig(ctx, orgID) + if err != nil { + s.logger.Warn("Failed to fetch dynamic SCIMConfig resource, will attempt fallback to static config", "orgID", orgID, "error", err) + return false, false + } + + var enabled bool + switch settingType { + case "user": + enabled = scimConfig.EnableUserSync + case "group": + enabled = scimConfig.EnableGroupSync + case "allowNonProvisionedUsers": + if scimConfig.AllowNonProvisionedUsers != nil { + enabled = *scimConfig.AllowNonProvisionedUsers + } else { + enabled = false + } + default: + s.logger.Error("Invalid setting type provided to fetchDynamicSCIMSetting", "settingType", settingType) + return false, false + } + + return enabled, true +} + +// getOrgSCIMConfig fetches and converts the SCIMConfig for an org +func (s *SCIMUtil) getOrgSCIMConfig(ctx context.Context, orgID int64) (*SCIMConfigSpec, error) { + if s.k8sClient == nil { + return nil, errors.New("k8s client not configured") + } + + unstructuredObj, err := s.k8sClient.Get(ctx, "default", orgID, metav1.GetOptions{}) + if err != nil { + return nil, err + } + + return s.unstructuredToSCIMConfig(unstructuredObj) +} + +// SCIMConfigSpec represents the spec part of a SCIMConfig resource +type SCIMConfigSpec struct { + EnableUserSync bool `json:"enableUserSync"` + EnableGroupSync bool `json:"enableGroupSync"` + AllowNonProvisionedUsers *bool `json:"allowNonProvisionedUsers,omitempty"` +} + +// unstructuredToSCIMConfig converts an unstructured object to a SCIMConfigSpec +func (s *SCIMUtil) unstructuredToSCIMConfig(obj *unstructured.Unstructured) (*SCIMConfigSpec, error) { + if obj == nil { + return nil, errors.New("nil unstructured object") + } + + // Convert spec + spec, found, err := unstructured.NestedMap(obj.Object, "spec") + if err != nil { + return nil, err + } + if !found { + return nil, errors.New("spec not found in SCIMConfig") + } + + enableUserSync, _, _ := unstructured.NestedBool(spec, "enableUserSync") + enableGroupSync, _, _ := unstructured.NestedBool(spec, "enableGroupSync") + allowNonProvisionedUsers, _, _ := unstructured.NestedBool(spec, "allowNonProvisionedUsers") + + return &SCIMConfigSpec{ + EnableUserSync: enableUserSync, + EnableGroupSync: enableGroupSync, + AllowNonProvisionedUsers: &allowNonProvisionedUsers, + }, nil +} diff --git a/pkg/services/scimutil/scim_util_test.go b/pkg/services/scimutil/scim_util_test.go new file mode 100644 index 00000000000..2e371d32fd8 --- /dev/null +++ b/pkg/services/scimutil/scim_util_test.go @@ -0,0 +1,704 @@ +package scimutil + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/grafana/grafana/pkg/services/apiserver/client" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/storage/unified/resourcepb" + "github.com/grafana/grafana/pkg/util" +) + +// MockK8sHandler is a mock implementation of client.K8sHandler for testing +type MockK8sHandler struct { + mock.Mock +} + +func (m *MockK8sHandler) GetNamespace(orgID int64) string { + args := m.Called(orgID) + return args.String(0) +} + +func (m *MockK8sHandler) Get(ctx context.Context, name string, orgID int64, opts metav1.GetOptions, subresource ...string) (*unstructured.Unstructured, error) { + args := m.Called(ctx, name, orgID, opts, subresource) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*unstructured.Unstructured), args.Error(1) +} + +func (m *MockK8sHandler) Create(ctx context.Context, obj *unstructured.Unstructured, orgID int64, opts metav1.CreateOptions) (*unstructured.Unstructured, error) { + args := m.Called(ctx, obj, orgID, opts) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*unstructured.Unstructured), args.Error(1) +} + +func (m *MockK8sHandler) Update(ctx context.Context, obj *unstructured.Unstructured, orgID int64, opts metav1.UpdateOptions) (*unstructured.Unstructured, error) { + args := m.Called(ctx, obj, orgID, opts) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*unstructured.Unstructured), args.Error(1) +} + +func (m *MockK8sHandler) Delete(ctx context.Context, name string, orgID int64, options metav1.DeleteOptions) error { + args := m.Called(ctx, name, orgID, options) + return args.Error(0) +} + +func (m *MockK8sHandler) DeleteCollection(ctx context.Context, orgID int64) error { + args := m.Called(ctx, orgID) + return args.Error(0) +} + +func (m *MockK8sHandler) List(ctx context.Context, orgID int64, options metav1.ListOptions) (*unstructured.UnstructuredList, error) { + args := m.Called(ctx, orgID, options) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*unstructured.UnstructuredList), args.Error(1) +} + +func (m *MockK8sHandler) Search(ctx context.Context, orgID int64, in *resourcepb.ResourceSearchRequest) (*resourcepb.ResourceSearchResponse, error) { + args := m.Called(ctx, orgID, in) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*resourcepb.ResourceSearchResponse), args.Error(1) +} + +func (m *MockK8sHandler) GetStats(ctx context.Context, orgID int64) (*resourcepb.ResourceStatsResponse, error) { + args := m.Called(ctx, orgID) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*resourcepb.ResourceStatsResponse), args.Error(1) +} + +func (m *MockK8sHandler) GetUsersFromMeta(ctx context.Context, userMeta []string) (map[string]*user.User, error) { + args := m.Called(ctx, userMeta) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(map[string]*user.User), args.Error(1) +} + +func TestNewSCIMUtil(t *testing.T) { + tests := []struct { + name string + k8sClient client.K8sHandler + }{ + { + name: "with k8s client", + k8sClient: &MockK8sHandler{}, + }, + { + name: "without k8s client", + k8sClient: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + util := NewSCIMUtil(tt.k8sClient) + assert.NotNil(t, util) + assert.Equal(t, tt.k8sClient, util.k8sClient) + assert.NotNil(t, util.logger) + }) + } +} + +func TestSCIMUtil_IsUserSyncEnabled(t *testing.T) { + ctx := context.Background() + orgID := int64(1) + + tests := []struct { + name string + k8sClient client.K8sHandler + staticEnabled bool + expectedResult bool + setupMock func(*MockK8sHandler) + }{ + { + name: "k8s client nil - returns static config", + k8sClient: nil, + staticEnabled: true, + expectedResult: true, + }, + { + name: "k8s client nil - returns static config false", + k8sClient: nil, + staticEnabled: false, + expectedResult: false, + }, + { + name: "k8s client error - falls back to static config", + k8sClient: &MockK8sHandler{}, + staticEnabled: true, + setupMock: func(mockHandler *MockK8sHandler) { + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(nil, errors.New("k8s error")) + }, + expectedResult: true, + }, + { + name: "dynamic config user sync enabled", + k8sClient: &MockK8sHandler{}, + staticEnabled: false, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(true, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + expectedResult: true, + }, + { + name: "dynamic config user sync disabled", + k8sClient: &MockK8sHandler{}, + staticEnabled: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(false, true) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + expectedResult: false, + }, + { + name: "dynamic config both settings disabled", + k8sClient: &MockK8sHandler{}, + staticEnabled: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(false, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + expectedResult: false, + }, + { + name: "dynamic config both settings enabled", + k8sClient: &MockK8sHandler{}, + staticEnabled: false, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(true, true) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + expectedResult: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setupMock != nil { + tt.setupMock(tt.k8sClient.(*MockK8sHandler)) + } + + util := NewSCIMUtil(tt.k8sClient) + result := util.IsUserSyncEnabled(ctx, orgID, tt.staticEnabled) + + assert.Equal(t, tt.expectedResult, result) + + if tt.k8sClient != nil { + tt.k8sClient.(*MockK8sHandler).AssertExpectations(t) + } + }) + } +} + +func TestSCIMUtil_AreNonProvisionedUsersAllowed(t *testing.T) { + ctx := context.Background() + orgID := int64(1) + + tests := []struct { + name string + k8sClient client.K8sHandler + staticAllowed bool + expectedResult bool + setupMock func(*MockK8sHandler) + }{ + { + name: "k8s client nil - returns static config", + k8sClient: nil, + staticAllowed: true, + expectedResult: true, + }, + { + name: "k8s client nil - returns static config false", + k8sClient: nil, + staticAllowed: false, + expectedResult: false, + }, + { + name: "k8s client error - falls back to static config", + k8sClient: &MockK8sHandler{}, + staticAllowed: true, + setupMock: func(mockHandler *MockK8sHandler) { + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(nil, errors.New("k8s error")) + }, + expectedResult: true, + }, + { + name: "dynamic config user sync enabled - non-provisioned users allowed", + k8sClient: &MockK8sHandler{}, + staticAllowed: false, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfigWithNonProvisioned(true, false, true) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + expectedResult: true, + }, + { + name: "dynamic config user sync disabled - non-provisioned users not allowed", + k8sClient: &MockK8sHandler{}, + staticAllowed: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfigWithNonProvisioned(false, true, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + expectedResult: false, + }, + { + name: "dynamic config both settings disabled - non-provisioned users not allowed", + k8sClient: &MockK8sHandler{}, + staticAllowed: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfigWithNonProvisioned(false, false, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + expectedResult: false, + }, + { + name: "dynamic config both settings enabled - non-provisioned users allowed", + k8sClient: &MockK8sHandler{}, + staticAllowed: false, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfigWithNonProvisioned(true, true, true) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + expectedResult: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setupMock != nil { + tt.setupMock(tt.k8sClient.(*MockK8sHandler)) + } + + util := NewSCIMUtil(tt.k8sClient) + result := util.AreNonProvisionedUsersAllowed(ctx, orgID, tt.staticAllowed) + + assert.Equal(t, tt.expectedResult, result) + + if tt.k8sClient != nil { + tt.k8sClient.(*MockK8sHandler).AssertExpectations(t) + } + }) + } +} + +func TestSCIMUtil_fetchDynamicSCIMSetting(t *testing.T) { + ctx := context.Background() + orgID := int64(1) + + tests := []struct { + name string + k8sClient client.K8sHandler + settingType string + expectedEnabled bool + expectedDynamicFetched bool + setupMock func(*MockK8sHandler) + }{ + { + name: "k8s client nil", + k8sClient: nil, + settingType: "user", + expectedEnabled: false, + expectedDynamicFetched: false, + }, + { + name: "invalid setting type", + k8sClient: &MockK8sHandler{}, + settingType: "invalid", + expectedEnabled: false, + expectedDynamicFetched: false, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(true, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "k8s client error", + k8sClient: &MockK8sHandler{}, + settingType: "user", + expectedEnabled: false, + expectedDynamicFetched: false, + setupMock: func(mockHandler *MockK8sHandler) { + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(nil, errors.New("k8s error")) + }, + }, + { + name: "user sync setting enabled", + k8sClient: &MockK8sHandler{}, + settingType: "user", + expectedEnabled: true, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(true, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "user sync setting disabled", + k8sClient: &MockK8sHandler{}, + settingType: "user", + expectedEnabled: false, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(false, true) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "group sync setting enabled", + k8sClient: &MockK8sHandler{}, + settingType: "group", + expectedEnabled: true, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(false, true) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "group sync setting disabled", + k8sClient: &MockK8sHandler{}, + settingType: "group", + expectedEnabled: false, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(true, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "user sync setting - both settings disabled", + k8sClient: &MockK8sHandler{}, + settingType: "user", + expectedEnabled: false, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(false, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "user sync setting - both settings enabled", + k8sClient: &MockK8sHandler{}, + settingType: "user", + expectedEnabled: true, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(true, true) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "group sync setting - both settings disabled", + k8sClient: &MockK8sHandler{}, + settingType: "group", + expectedEnabled: false, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(false, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "group sync setting - both settings enabled", + k8sClient: &MockK8sHandler{}, + settingType: "group", + expectedEnabled: true, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(true, true) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "allowNonProvisionedUsers setting enabled", + k8sClient: &MockK8sHandler{}, + settingType: "allowNonProvisionedUsers", + expectedEnabled: true, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfigWithNonProvisioned(false, false, true) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + { + name: "allowNonProvisionedUsers setting disabled", + k8sClient: &MockK8sHandler{}, + settingType: "allowNonProvisionedUsers", + expectedEnabled: false, + expectedDynamicFetched: true, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfigWithNonProvisioned(true, true, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setupMock != nil { + tt.setupMock(tt.k8sClient.(*MockK8sHandler)) + } + + util := NewSCIMUtil(tt.k8sClient) + enabled, dynamicFetched := util.fetchDynamicSCIMSetting(ctx, orgID, tt.settingType) + + assert.Equal(t, tt.expectedEnabled, enabled) + assert.Equal(t, tt.expectedDynamicFetched, dynamicFetched) + + if tt.k8sClient != nil { + tt.k8sClient.(*MockK8sHandler).AssertExpectations(t) + } + }) + } +} + +func TestSCIMUtil_getOrgSCIMConfig(t *testing.T) { + ctx := context.Background() + orgID := int64(1) + + tests := []struct { + name string + k8sClient client.K8sHandler + expectedError error + setupMock func(*MockK8sHandler) + }{ + { + name: "k8s client nil", + k8sClient: nil, + expectedError: errors.New("k8s client not configured"), + }, + { + name: "k8s client error", + k8sClient: &MockK8sHandler{}, + expectedError: errors.New("k8s error"), + setupMock: func(mockHandler *MockK8sHandler) { + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(nil, errors.New("k8s error")) + }, + }, + { + name: "successful fetch", + k8sClient: &MockK8sHandler{}, + setupMock: func(mockHandler *MockK8sHandler) { + obj := createMockSCIMConfig(true, false) + mockHandler.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setupMock != nil { + tt.setupMock(tt.k8sClient.(*MockK8sHandler)) + } + + util := NewSCIMUtil(tt.k8sClient) + config, err := util.getOrgSCIMConfig(ctx, orgID) + + if tt.expectedError != nil { + assert.Error(t, err) + assert.Nil(t, config) + } else { + assert.NoError(t, err) + assert.NotNil(t, config) + assert.Equal(t, true, config.EnableUserSync) + assert.Equal(t, false, config.EnableGroupSync) + } + + if tt.k8sClient != nil { + tt.k8sClient.(*MockK8sHandler).AssertExpectations(t) + } + }) + } +} + +func TestSCIMUtil_unstructuredToSCIMConfig(t *testing.T) { + tests := []struct { + name string + obj *unstructured.Unstructured + expectedError bool + expectedSpec SCIMConfigSpec + }{ + { + name: "nil object", + obj: nil, + expectedError: true, + }, + { + name: "valid object with both settings enabled", + obj: createMockSCIMConfig(true, true), + expectedSpec: SCIMConfigSpec{ + EnableUserSync: true, + EnableGroupSync: true, + AllowNonProvisionedUsers: util.Pointer(false), + }, + }, + { + name: "valid object with both settings disabled", + obj: createMockSCIMConfig(false, false), + expectedSpec: SCIMConfigSpec{ + EnableUserSync: false, + EnableGroupSync: false, + AllowNonProvisionedUsers: util.Pointer(false), + }, + }, + { + name: "valid object with mixed settings", + obj: createMockSCIMConfig(true, false), + expectedSpec: SCIMConfigSpec{ + EnableUserSync: true, + EnableGroupSync: false, + AllowNonProvisionedUsers: util.Pointer(false), + }, + }, + { + name: "valid object with allowNonProvisionedUsers enabled", + obj: createMockSCIMConfigWithNonProvisioned(false, false, true), + expectedSpec: SCIMConfigSpec{ + EnableUserSync: false, + EnableGroupSync: false, + AllowNonProvisionedUsers: util.Pointer(true), + }, + }, + { + name: "object with missing spec", + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "scim.grafana.com/v0alpha1", + "kind": "SCIMConfig", + "metadata": map[string]interface{}{ + "name": "test-config", + "namespace": "default", + }, + }, + }, + expectedError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + util := NewSCIMUtil(nil) + config, err := util.unstructuredToSCIMConfig(tt.obj) + + if tt.expectedError { + assert.Error(t, err) + assert.Nil(t, config) + } else { + assert.NoError(t, err) + assert.NotNil(t, config) + assert.Equal(t, tt.expectedSpec, *config) + } + }) + } +} + +// Helper function to create a mock SCIMConfig unstructured object +func createMockSCIMConfig(userSyncEnabled, groupSyncEnabled bool) *unstructured.Unstructured { + return createMockSCIMConfigWithNonProvisioned(userSyncEnabled, groupSyncEnabled, false) +} + +// Helper function to create a mock SCIMConfig unstructured object with non-provisioned users setting +func createMockSCIMConfigWithNonProvisioned(userSyncEnabled, groupSyncEnabled, allowNonProvisionedUsers bool) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "scim.grafana.com/v0alpha1", + "kind": "SCIMConfig", + "metadata": map[string]interface{}{ + "name": "test-config", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "enableUserSync": userSyncEnabled, + "enableGroupSync": groupSyncEnabled, + "allowNonProvisionedUsers": allowNonProvisionedUsers, + }, + }, + } +} + +// Test integration scenarios +func TestSCIMUtil_Integration(t *testing.T) { + ctx := context.Background() + orgID := int64(1) + + t.Run("full workflow with dynamic config", func(t *testing.T) { + mockClient := &MockK8sHandler{} + obj := createMockSCIMConfigWithNonProvisioned(true, false, true) + mockClient.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(obj, nil) + + util := NewSCIMUtil(mockClient) + + // Test user sync enabled + userSyncEnabled := util.IsUserSyncEnabled(ctx, orgID, false) + assert.True(t, userSyncEnabled) + + // Test non-provisioned users allowed + nonProvisionedAllowed := util.AreNonProvisionedUsersAllowed(ctx, orgID, false) + assert.True(t, nonProvisionedAllowed) + + mockClient.AssertExpectations(t) + }) + + t.Run("full workflow with static fallback", func(t *testing.T) { + mockClient := &MockK8sHandler{} + mockClient.On("Get", ctx, "default", orgID, metav1.GetOptions{}, mock.Anything). + Return(nil, errors.New("k8s error")) + + util := NewSCIMUtil(mockClient) + + // Test user sync falls back to static config + userSyncEnabled := util.IsUserSyncEnabled(ctx, orgID, true) + assert.True(t, userSyncEnabled) + + // Test non-provisioned users falls back to static config + nonProvisionedAllowed := util.AreNonProvisionedUsersAllowed(ctx, orgID, true) + assert.True(t, nonProvisionedAllowed) + + mockClient.AssertExpectations(t) + }) +}