From f06bb75310572eec03a55921370c3aea2ca2dbe9 Mon Sep 17 00:00:00 2001 From: linoman <2051016+linoman@users.noreply.github.com> Date: Mon, 20 Oct 2025 13:30:27 +0200 Subject: [PATCH] [release-12.2.1] Backport 112615 to release 12.2.1 (#112636) Update validation of non-provisioned users rejection (#112615) * Update validation of non-provisioned users rejection * Align tests (cherry picked from commit 0e4237b77586f585a290a8d450d341a6c88143be) --- .../authn/authnimpl/sync/user_sync.go | 16 ++++++++-- .../authn/authnimpl/sync/user_sync_test.go | 32 +++++++------------ 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index 01315058a9d..69bee5c35d2 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/pkg/services/authn/authnimpl/sync/user_sync.go @@ -186,6 +186,13 @@ func (s *UserSync) ValidateUserProvisioningHook(ctx context.Context, currentIden return nil } + effectiveReject := s.shouldRejectNonProvisionedUsers(ctx, currentIdentity) + + if !effectiveReject { + log.Debug("Skip provisioning validation, non-provisioned users are allowed") + return nil + } + log.Debug("Validating user provisioning") ctx, span := s.tracer.Start(ctx, "user.sync.ValidateUserProvisioningHook") defer span.End() @@ -225,7 +232,7 @@ func (s *UserSync) ValidateUserProvisioningHook(ctx context.Context, currentIden } // Reject non-provisioned users if configured to do so - if s.shouldRejectNonProvisionedUsers(ctx, currentIdentity) { + if effectiveReject { log.Error("Failed to authenticate user, user is not provisioned") return errUserNotProvisioned.Errorf("user is not provisioned") } @@ -500,8 +507,11 @@ func (s *UserSync) updateUserAttributes(ctx context.Context, usr *user.User, id attribute.String("identity.ID", id.ID), attribute.String("identity.ExternalUID", id.ExternalUID), ) - if usr.IsProvisioned { - s.log.Debug("User is provisioned", "id.UID", id.UID) + + ctxLogger := s.log.FromContext(ctx) + + if s.shouldRejectNonProvisionedUsers(ctx, id) && usr.IsProvisioned && id.AuthenticatedBy != login.GrafanaComAuthModule { + ctxLogger.Debug("User is provisioned", "id.UID", id.UID) needsConnectionCreation = false authInfo, err := s.authInfoService.GetAuthInfo(ctx, &login.GetAuthInfoQuery{UserId: usr.ID, AuthModule: id.AuthenticatedBy}) if err != nil { diff --git a/pkg/services/authn/authnimpl/sync/user_sync_test.go b/pkg/services/authn/authnimpl/sync/user_sync_test.go index e9710d53ff9..dd19836b0a5 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/user_sync_test.go @@ -203,8 +203,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService quota.Service } type args struct { - ctx context.Context - id *authn.Identity + id *authn.Identity } tests := []struct { name string @@ -221,7 +220,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ Login: "test", Name: "test", @@ -255,7 +253,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ Login: "test", Name: "test", @@ -295,7 +292,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ Login: "test", Name: "test", @@ -335,7 +331,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ AuthID: "2032", AuthenticatedBy: "oauth", @@ -379,7 +374,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ Login: "test", Name: "test", @@ -405,7 +399,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ Login: "test_create", Name: "test_create", @@ -454,7 +447,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ Login: "test_mod", Name: "test_mod", @@ -499,7 +491,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ Login: "test", Name: "test", @@ -546,7 +537,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ AuthID: "1", AuthenticatedBy: login.SAMLAuthModule, @@ -564,7 +554,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ AuthID: "1", AuthenticatedBy: login.SAMLAuthModule, @@ -582,7 +571,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ AuthID: "1", AuthenticatedBy: login.SAMLAuthModule, @@ -600,7 +588,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ AuthID: "1", AuthenticatedBy: login.SAMLAuthModule, @@ -655,7 +642,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ AuthID: "id_from_saml_assertion", AuthenticatedBy: "saml", @@ -717,7 +703,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ AuthID: "id_from_saml_assertion", AuthenticatedBy: "saml", @@ -776,7 +761,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ AuthID: "id_from_saml_assertion", AuthenticatedBy: "saml", @@ -862,7 +846,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { quotaService: "atest.FakeQuotaService{}, }, args: args{ - ctx: context.Background(), id: &authn.Identity{ AuthenticatedBy: "saml", // No AuthID or ExternalUID for this non-SCIM path, will lookup by email/login @@ -900,8 +883,12 @@ 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(), nil) - err := s.SyncUserHook(tt.args.ctx, tt.args.id, nil) + cfg := setting.NewCfg() + cfg.Raw.Section("auth.scim").Key("user_sync_enabled").SetValue("true") + cfg.Raw.Section("auth.scim").Key("reject_non_provisioned_users").SetValue("true") + + s := ProvideUserSync(tt.fields.userService, userProtection, tt.fields.authInfoService, tt.fields.quotaService, tracing.InitializeTracerForTest(), featuremgmt.WithFeatures(), cfg, nil) + err := s.SyncUserHook(context.Background(), tt.args.id, nil) if tt.wantErr { require.Error(t, err) return @@ -1428,6 +1415,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) { userSyncServiceSetup: func() *UserSync { userSyncService := initUserSyncService() userSyncService.isUserProvisioningEnabled = true + userSyncService.rejectNonProvisionedUsers = true userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}} userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: ""}} return userSyncService @@ -1447,6 +1435,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) { userSyncServiceSetup: func() *UserSync { userSyncService := initUserSyncService() userSyncService.isUserProvisioningEnabled = true + userSyncService.rejectNonProvisionedUsers = true userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}} userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: ""}} return userSyncService @@ -1466,6 +1455,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) { userSyncServiceSetup: func() *UserSync { userSyncService := initUserSyncService() userSyncService.isUserProvisioningEnabled = true + userSyncService.rejectNonProvisionedUsers = true userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}} userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: "db-uid"}} return userSyncService @@ -1898,7 +1888,7 @@ func TestUserSync_GetUsageStats(t *testing.T) { func TestUserSync_SCIMLoginUsageStatSet(t *testing.T) { userSync := initUserSyncService() - userSync.rejectNonProvisionedUsers = false + userSync.rejectNonProvisionedUsers = true userSync.isUserProvisioningEnabled = true userSync.userService = &usertest.FakeUserService{ ExpectedUser: &user.User{