public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[release-12.2.1] Backport 112615 to release 12.2.1 (#112636)

Browse Source 
Update validation of non-provisioned users rejection (#112615)

* Update validation of non-provisioned users rejection

* Align tests

(cherry picked from commit 0e4237b775)
This commit is contained in:
linoman
2025-10-20 13:30:27 +02:00
committed by GitHub
parent 64b98e9866
commit f06bb75310
2 changed files with 24 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/authn/authnimpl/sync/user_sync.go
+13 -3
View File
@@ -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 {
pkg/services/authn/authnimpl/sync/user_sync_test.go
+11 -21
View File
@@ -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: &quotatest.FakeQuotaService{},
},
args: args{
ctx: context.Background(),
id: &authn.Identity{
Login: "test",
Name: "test",
@@ -255,7 +253,6 @@ func TestUserSync_SyncUserHook(t *testing.T) {
quotaService: &quotatest.FakeQuotaService{},
},
args: args{
ctx: context.Background(),
id: &authn.Identity{
Login: "test",
Name: "test",
@@ -295,7 +292,6 @@ func TestUserSync_SyncUserHook(t *testing.T) {
quotaService: &quotatest.FakeQuotaService{},
},
args: args{
ctx: context.Background(),
id: &authn.Identity{
Login: "test",
Name: "test",
@@ -335,7 +331,6 @@ func TestUserSync_SyncUserHook(t *testing.T) {
quotaService: &quotatest.FakeQuotaService{},
},
args: args{
ctx: context.Background(),
id: &authn.Identity{
AuthID: "2032",
AuthenticatedBy: "oauth",
@@ -379,7 +374,6 @@ func TestUserSync_SyncUserHook(t *testing.T) {
quotaService: &quotatest.FakeQuotaService{},
},
args: args{
ctx: context.Background(),
id: &authn.Identity{
Login: "test",
Name: "test",
@@ -405,7 +399,6 @@ func TestUserSync_SyncUserHook(t *testing.T) {
quotaService: &quotatest.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: &quotatest.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: &quotatest.FakeQuotaService{},
},
args: args{
ctx: context.Background(),
id: &authn.Identity{
Login: "test",
Name: "test",
@@ -546,7 +537,6 @@ func TestUserSync_SyncUserHook(t *testing.T) {
quotaService: &quotatest.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: &quotatest.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: &quotatest.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: &quotatest.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: &quotatest.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: &quotatest.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: &quotatest.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: &quotatest.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{