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/.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/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. 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) 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/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/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() 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/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) + }) +} 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 {