SCIM: Add flag for rejecting non provisioned users from logging in (#108568)

add flag for rejecting non provisioned users from logging in
This commit is contained in:
Mihai Doarna
2025-07-28 11:31:33 +03:00
committed by GitHub
parent 3a285d9b16
commit f9b34baa35
4 changed files with 181 additions and 128 deletions
@@ -1092,11 +1092,24 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
},
},
{
desc: "it should skip validation if allowedNonProvisionedUsers is enabled",
desc: "it should skip validation if rejectNonProvisionedUsers is disabled",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = true
userSyncService.rejectNonProvisionedUsers = false
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{
ID: 1,
IsProvisioned: false,
},
}
userSyncService.authInfoService = &authinfotest.FakeService{
ExpectedUserAuth: &login.UserAuth{
UserId: 1,
AuthModule: login.GenericOAuthModule,
AuthId: "1",
},
}
return userSyncService
},
identity: &authn.Identity{
@@ -1111,7 +1124,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
desc: "it should skip validation if the user is authenticated via GrafanaComAuthModule",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = false
userSyncService.rejectNonProvisionedUsers = true
userSyncService.isUserProvisioningEnabled = true
return userSyncService
},
@@ -1127,7 +1140,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
desc: "it should fail to validate the identity with the provisioned user, unexpected error",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = false
userSyncService.rejectNonProvisionedUsers = true
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{
ExpectedError: errors.New("random error"),
@@ -1148,7 +1161,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
desc: "it should fail to validate the identity with the provisioned user, no user found",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = false
userSyncService.rejectNonProvisionedUsers = true
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{}
return userSyncService
@@ -1167,7 +1180,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
desc: "it should fail to validate the provisioned user.ExternalUID with the identity.ExternalUID - empty ExternalUID",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = false
userSyncService.rejectNonProvisionedUsers = true
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{
@@ -1198,7 +1211,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
desc: "it should fail to validate the provisioned user.ExternalUID with the identity.ExternalUID - different ExternalUID",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = false
userSyncService.rejectNonProvisionedUsers = true
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{
@@ -1230,7 +1243,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
desc: "it should successfully validate the provisioned user.ExternalUID with the identity.ExternalUID",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = false
userSyncService.rejectNonProvisionedUsers = true
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{
@@ -1259,10 +1272,10 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
expectedErr: nil,
},
{
desc: "it should failed to validate a non provisioned user when retrieved from the database",
desc: "it should fail to validate a non provisioned user when configured to reject non provisioned users",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = false
userSyncService.rejectNonProvisionedUsers = true
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{
@@ -1290,6 +1303,38 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
},
expectedErr: errUserNotProvisioned.Errorf("user is not provisioned"),
},
{
desc: "it should skip to validate a non provisioned user when configured to allow non provisioned users",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.rejectNonProvisionedUsers = false
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{
ID: 1,
IsProvisioned: false,
},
}
userSyncService.authInfoService = &authinfotest.FakeService{
ExpectedUserAuth: &login.UserAuth{
UserId: 1,
AuthModule: login.SAMLAuthModule,
AuthId: "1",
ExternalUID: "random-external-uid",
},
}
return userSyncService
},
identity: &authn.Identity{
AuthenticatedBy: login.SAMLAuthModule,
AuthID: "1",
ExternalUID: "different-external-uid",
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
expectedErr: nil,
},
{
desc: "ValidateProvisioning: DB ExternalUID is empty, Incoming ExternalUID is empty - expect mismatch (stricter logic)",
userSyncServiceSetup: func() *UserSync {
@@ -1351,7 +1396,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
desc: "it should skip ExternalUID validation for a SAML-provisioned user accessed by a non-SAML method with an empty incoming ExternalUID",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = false
userSyncService.rejectNonProvisionedUsers = false
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{
@@ -1380,7 +1425,7 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
desc: "it should fail validation when a provisioned user is accessed by SAML with an empty incoming ExternalUID",
userSyncServiceSetup: func() *UserSync {
userSyncService := initUserSyncService()
userSyncService.allowNonProvisionedUsers = false
userSyncService.rejectNonProvisionedUsers = true
userSyncService.isUserProvisioningEnabled = true
userSyncService.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{
@@ -1425,10 +1470,10 @@ func TestUserSync_SCIMUtilIntegration(t *testing.T) {
// Mock SCIM utility for testing
type mockSCIMUtil struct {
userSyncEnabled bool
nonProvisionedUsersAllowed bool
shouldUseDynamicConfig bool
shouldReturnError bool
userSyncEnabled bool
nonProvisionedUsersRejected bool
shouldUseDynamicConfig bool
shouldReturnError bool
}
createMockSCIMUtil := func(mockCfg *mockSCIMUtil) *scimutil.SCIMUtil {
@@ -1453,9 +1498,9 @@ func TestUserSync_SCIMUtilIntegration(t *testing.T) {
"namespace": "default",
},
"spec": map[string]interface{}{
"enableUserSync": mockCfg.userSyncEnabled,
"enableGroupSync": false, // Not used for this test
"allowNonProvisionedUsers": mockCfg.nonProvisionedUsersAllowed,
"enableUserSync": mockCfg.userSyncEnabled,
"enableGroupSync": false, // Not used for this test
"rejectNonProvisionedUsers": mockCfg.nonProvisionedUsersRejected,
},
},
}
@@ -1467,13 +1512,13 @@ func TestUserSync_SCIMUtilIntegration(t *testing.T) {
}
tests := []struct {
name string
identity *authn.Identity
staticConfig *StaticSCIMConfig
mockSCIMUtil *mockSCIMUtil
expectedUserSyncEnabled bool
expectedNonProvisionedAllowed bool
expectedError error
name string
identity *authn.Identity
staticConfig *StaticSCIMConfig
mockSCIMUtil *mockSCIMUtil
expectedUserSyncEnabled bool
expectedNonProvisionedRejected bool
expectedError error
}{
{
name: "SCIM util nil - uses static config",
@@ -1483,11 +1528,11 @@ func TestUserSync_SCIMUtilIntegration(t *testing.T) {
},
staticConfig: &StaticSCIMConfig{
IsUserProvisioningEnabled: true,
AllowNonProvisionedUsers: false,
RejectNonProvisionedUsers: false,
},
mockSCIMUtil: nil, // No SCIM util
expectedUserSyncEnabled: true,
expectedNonProvisionedAllowed: false,
mockSCIMUtil: nil, // No SCIM util
expectedUserSyncEnabled: true,
expectedNonProvisionedRejected: false,
},
{
name: "SCIM util with dynamic config - user sync enabled",
@@ -1497,15 +1542,15 @@ func TestUserSync_SCIMUtilIntegration(t *testing.T) {
},
staticConfig: &StaticSCIMConfig{
IsUserProvisioningEnabled: false, // Static disabled
AllowNonProvisionedUsers: false,
RejectNonProvisionedUsers: true,
},
mockSCIMUtil: &mockSCIMUtil{
userSyncEnabled: true, // Dynamic enabled
nonProvisionedUsersAllowed: true,
shouldUseDynamicConfig: true,
userSyncEnabled: true, // Dynamic enabled
nonProvisionedUsersRejected: true,
shouldUseDynamicConfig: true,
},
expectedUserSyncEnabled: true,
expectedNonProvisionedAllowed: true,
expectedUserSyncEnabled: true,
expectedNonProvisionedRejected: true,
},
{
name: "SCIM util with dynamic config - user sync disabled",
@@ -1515,15 +1560,15 @@ func TestUserSync_SCIMUtilIntegration(t *testing.T) {
},
staticConfig: &StaticSCIMConfig{
IsUserProvisioningEnabled: true, // Static enabled
AllowNonProvisionedUsers: true,
RejectNonProvisionedUsers: true,
},
mockSCIMUtil: &mockSCIMUtil{
userSyncEnabled: false, // Dynamic disabled
nonProvisionedUsersAllowed: false,
shouldUseDynamicConfig: true,
userSyncEnabled: false, // Dynamic disabled
nonProvisionedUsersRejected: false,
shouldUseDynamicConfig: true,
},
expectedUserSyncEnabled: false,
expectedNonProvisionedAllowed: false,
expectedUserSyncEnabled: false,
expectedNonProvisionedRejected: false,
},
{
name: "SCIM util with error - falls back to static config",
@@ -1533,13 +1578,13 @@ func TestUserSync_SCIMUtilIntegration(t *testing.T) {
},
staticConfig: &StaticSCIMConfig{
IsUserProvisioningEnabled: true,
AllowNonProvisionedUsers: false,
RejectNonProvisionedUsers: false,
},
mockSCIMUtil: &mockSCIMUtil{
shouldReturnError: true,
},
expectedUserSyncEnabled: true,
expectedNonProvisionedAllowed: false,
expectedUserSyncEnabled: true,
expectedNonProvisionedRejected: false,
},
}
@@ -1559,14 +1604,14 @@ func TestUserSync_SCIMUtilIntegration(t *testing.T) {
}
assert.Equal(t, tt.expectedUserSyncEnabled, userSyncEnabled, "User sync enabled mismatch")
// Test non-provisioned users allowed check
var nonProvisionedAllowed bool
// Test non-provisioned users rejected check
var nonProvisionedReject bool
if userSync.scimUtil != nil {
nonProvisionedAllowed = userSync.scimUtil.AreNonProvisionedUsersAllowed(ctx, orgID, tt.staticConfig.AllowNonProvisionedUsers)
nonProvisionedReject = userSync.scimUtil.AreNonProvisionedUsersRejected(ctx, orgID, tt.staticConfig.RejectNonProvisionedUsers)
} else {
nonProvisionedAllowed = tt.staticConfig.AllowNonProvisionedUsers
nonProvisionedReject = tt.staticConfig.RejectNonProvisionedUsers
}
assert.Equal(t, tt.expectedNonProvisionedAllowed, nonProvisionedAllowed, "Non-provisioned users allowed mismatch")
assert.Equal(t, tt.expectedNonProvisionedRejected, nonProvisionedReject, "Non-provisioned users rejected mismatch")
})
}
}
@@ -1765,7 +1810,7 @@ func TestUserSync_GetUsageStats(t *testing.T) {
func TestUserSync_SCIMLoginUsageStatSet(t *testing.T) {
userSync := initUserSyncService()
userSync.allowNonProvisionedUsers = false
userSync.rejectNonProvisionedUsers = false
userSync.isUserProvisioningEnabled = true
userSync.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{